From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34860) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bMsU5-00052E-JQ for qemu-devel@nongnu.org; Tue, 12 Jul 2016 03:52:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bMsU2-0007Ra-C7 for qemu-devel@nongnu.org; Tue, 12 Jul 2016 03:52:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58508) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bMsU1-0007RM-Rk for qemu-devel@nongnu.org; Tue, 12 Jul 2016 03:51:58 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2420685541 for ; Tue, 12 Jul 2016 07:51:57 +0000 (UTC) References: <20160711144847.16651-1-marcandre.lureau@redhat.com> <57849FA1.3050705@redhat.com> From: Paolo Bonzini Message-ID: <71cb921e-afa5-6344-46b0-fdecfcc03a3d@redhat.com> Date: Tue, 12 Jul 2016 09:51:52 +0200 MIME-Version: 1.0 In-Reply-To: <57849FA1.3050705@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] tap: use an exit notifier to call down_script List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang , marcandre.lureau@redhat.com, qemu-devel@nongnu.org Cc: akong@redhat.com On 12/07/2016 09:43, Jason Wang wrote: >=20 >=20 > On 2016=E5=B9=B407=E6=9C=8811=E6=97=A5 22:48, marcandre.lureau@redhat.c= om wrote: >> From: Marc-Andr=C3=A9 Lureau >> >> We would like to move back net_cleanup() at the end of main function, >> like it used to be until f30dbae63a46f23116715dff8d130c, but minimum >> tap cleanup is necessary regarless at exit() time. Use an exit notifie= r >> to call TAP down_script. If net_cleanup() is called first, then remove >> the exit notifier as it will become a dangling pointer otherwise. >> >> Signed-off-by: Marc-Andr=C3=A9 Lureau >> Suggested-by: Paolo Bonzini >=20 > This looks like a tap specific solution. But at least slirp wants to do > some cleanup too. Right, specifically slirp_smb_cleanup. > We need a generic solution. Does it work if we use > atexit(net_chr_cleanup) and first cleanup net and then chr at that > function? I think exit notifiers _are_ a generic solution. Most pieces of QEMU don't care about cleanup at exit(), because the operating system takes care of it (freeing memory, closing file descriptors, etc.). Tap, slirp and chardev are exceptions, and they can use the exit notifiers. I'll send a patch for slirp. Paolo >> --- >> net/tap.c | 27 ++++++++++++++++++++------- >> 1 file changed, 20 insertions(+), 7 deletions(-) >> >> diff --git a/net/tap.c b/net/tap.c >> index 676bad4..e9c32f3 100644 >> --- a/net/tap.c >> +++ b/net/tap.c >> @@ -58,6 +58,7 @@ typedef struct TAPState { >> bool enabled; >> VHostNetState *vhost_net; >> unsigned host_vnet_hdr_len; >> + Notifier exit; >> } TAPState; >> static void launch_script(const char *setup_script, const char >> *ifname, >> @@ -292,10 +293,22 @@ static void tap_set_offload(NetClientState *nc, >> int csum, int tso4, >> tap_fd_set_offload(s->fd, csum, tso4, tso6, ecn, ufo); >> } >> +static void tap_exit_notify(Notifier *notifier, void *data) >> +{ >> + TAPState *s =3D container_of(notifier, TAPState, exit); >> + Error *err =3D NULL; >> + >> + if (s->down_script[0]) { >> + launch_script(s->down_script, s->down_script_arg, s->fd, &err= ); >> + if (err) { >> + error_report_err(err); >> + } >> + } >> +} >> + >> static void tap_cleanup(NetClientState *nc) >> { >> TAPState *s =3D DO_UPCAST(TAPState, nc, nc); >> - Error *err =3D NULL; >> if (s->vhost_net) { >> vhost_net_cleanup(s->vhost_net); >> @@ -304,12 +317,8 @@ static void tap_cleanup(NetClientState *nc) >> qemu_purge_queued_packets(nc); >> - if (s->down_script[0]) { >> - launch_script(s->down_script, s->down_script_arg, s->fd, &err= ); >> - if (err) { >> - error_report_err(err); >> - } >> - } >> + tap_exit_notify(&s->exit, NULL); >> + qemu_remove_exit_notifier(&s->exit); >> tap_read_poll(s, false); >> tap_write_poll(s, false); >> @@ -379,6 +388,10 @@ static TAPState *net_tap_fd_init(NetClientState >> *peer, >> } >> tap_read_poll(s, true); >> s->vhost_net =3D NULL; >> + >> + s->exit.notify =3D tap_exit_notify; >> + qemu_add_exit_notifier(&s->exit); >> + >> return s; >> } >> =20 >=20