From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1MkHDV-00084B-3X
	for qemu-devel@nongnu.org; Sun, 06 Sep 2009 08:51:37 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1MkHDQ-00083t-A9
	for qemu-devel@nongnu.org; Sun, 06 Sep 2009 08:51:36 -0400
Received: from [199.232.76.173] (port=58278 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1MkHDQ-00083j-0x
	for qemu-devel@nongnu.org; Sun, 06 Sep 2009 08:51:32 -0400
Received: from mx1.redhat.com ([209.132.183.28]:65013)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <markmc@redhat.com>) id 1MkHDP-00056P-Dt
	for qemu-devel@nongnu.org; Sun, 06 Sep 2009 08:51:31 -0400
Subject: Re: close tapfd before running down_script [was Re: [Qemu-devel]
	ANNOUNCE: Release 0.11.0-rc2 of QEMU]
From: Mark McLoughlin <markmc@redhat.com>
In-Reply-To: <20090904201347.GA77929@triton8.kn-bremen.de>
References: <4AA11B9F.9050101@codemonkey.ws>
	<20090904201347.GA77929@triton8.kn-bremen.de>
Content-Type: text/plain
Date: Sun, 06 Sep 2009 13:50:25 +0100
Message-Id: <1252241425.3191.81.camel@blaa>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Reply-To: Mark McLoughlin <markmc@redhat.com>
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Juergen Lock <nox@jelal.kn-bremen.de>
Cc: freebsd-emulation@freebsd.org, qemu-devel@nongnu.org

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 <nox@jelal.kn-bremen.de>

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

The current behaviour seems right to me. Could you explain your use case
a bit more? Maybe post the up and down scripts?

Cheers,
Mark.