From owner-freebsd-emulation@FreeBSD.ORG Sun Sep 6 15:46:35 2009 Return-Path: Delivered-To: freebsd-emulation@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 7357D106568B for ; Sun, 6 Sep 2009 15:46:35 +0000 (UTC) (envelope-from nox@jelal.kn-bremen.de) Received: from smtp.kn-bremen.de (gelbbaer.kn-bremen.de [78.46.108.116]) by mx1.freebsd.org (Postfix) with ESMTP id 2F3D18FC12 for ; Sun, 6 Sep 2009 15:46:34 +0000 (UTC) Received: by smtp.kn-bremen.de (Postfix, from userid 10) id C2F7C1E00367; Sun, 6 Sep 2009 17:46:33 +0200 (CEST) Received: from triton8.kn-bremen.de (noident@localhost [127.0.0.1]) by triton8.kn-bremen.de (8.14.3/8.14.3) with ESMTP id n86FhLpY088547; Sun, 6 Sep 2009 17:43:21 +0200 (CEST) (envelope-from nox@triton8.kn-bremen.de) Received: (from nox@localhost) by triton8.kn-bremen.de (8.14.3/8.14.3/Submit) id n86FhF6Z088546; Sun, 6 Sep 2009 17:43:15 +0200 (CEST) (envelope-from nox) From: Juergen Lock Date: Sun, 6 Sep 2009 17:43:15 +0200 To: Mark McLoughlin Message-ID: <20090906154315.GA88187@triton8.kn-bremen.de> References: <4AA11B9F.9050101@codemonkey.ws> <20090904201347.GA77929@triton8.kn-bremen.de> <1252241425.3191.81.camel@blaa> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1252241425.3191.81.camel@blaa> User-Agent: Mutt/1.5.20 (2009-06-14) Cc: freebsd-emulation@freebsd.org, Juergen Lock , Anthony Liguori , qemu-devel@nongnu.org Subject: Re: close tapfd before running down_script [was Re: [Qemu-devel] ANNOUNCE: Release 0.11.0-rc2 of QEMU] X-BeenThere: freebsd-emulation@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Development of Emulators of other operating systems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 06 Sep 2009 15:46:35 -0000 On Sun, Sep 06, 2009 at 01:50:25PM +0100, Mark McLoughlin wrote: > On Fri, 2009-09-04 at 22:13 +0200, Juergen Lock wrote: > > > The second change is a small patch to tap_cleanup that makes it close > > the tap fd before calling the ifdown script instead of after, otherwise > > FreeBSD's tap driver may hit a KASSERT in case the ifdown script does > > something like an `ifconfig tap0 destroy'... > > > > Index: qemu/net.c > > @@ -1643,12 +1643,13 @@ static void tap_cleanup(VLANClientState > > > > qemu_purge_queued_packets(vc); > > > > - if (s->down_script[0]) > > - launch_script(s->down_script, s->down_script_arg, s->fd); > > - > > tap_read_poll(s, 0); > > tap_write_poll(s, 0); > > close(s->fd); > > + > > + if (s->down_script[0]) > > + launch_script(s->down_script, s->down_script_arg, -1); > > + > > qemu_free(s); > > } > > > > I don't know if there are use cases where the ifdown script needs the > > tap fd still open, otherwise I guess this can also be committed upstream. > > And in case you want to: :) > > > > Signed-off-by: Juergen Lock > > I don't ever use the the down script myself, but a couple of things to > bear in mind: > > a) 0.9.1 never actually closed the tap fd and since 0.10.0 we've > been closing the fd after calling the script > > b) where qemu creates the tap interface, by closing the tap fd before > the script we'd be destroying the interface before passing the > interface name to the script > Ah, then that sounds like a difference between Linux and FreeBSD, on FreeBSD the tap interface continues to exist after closing the fd... (which is why these scripts call `ifconfig tapX destroy' in the first place.) > The current behaviour seems right to me. Could you explain your use case > a bit more? Maybe post the up and down scripts? Here's the thread that started this: http://lists.freebsd.org/pipermail/freebsd-emulation/2009-August/006700.html And here is FreeBSD's tap_destroy() fn with the mentioned KASSERT: http://fxr.watson.org/fxr/source/net/if_tap.c#L213 Anyway, sounds like the patch is the wrong thing to do on Linux hosts so I guess I'll just keep it private to the FreeBSD port(s)... (Or should I repost it with an #ifdef __FreeBSD__ etc?) Thanx, Juergen