From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54064) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XREZB-0006lU-VH for qemu-devel@nongnu.org; Tue, 09 Sep 2014 02:06:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XREZ6-0007p0-Re for qemu-devel@nongnu.org; Tue, 09 Sep 2014 02:06:13 -0400 Message-ID: <540E98BF.1060707@redhat.com> Date: Tue, 09 Sep 2014 14:05:51 +0800 From: Jason Wang MIME-Version: 1.0 References: <1409845811-27943-1-git-send-email-stefanha@redhat.com> <1409845811-27943-7-git-send-email-stefanha@redhat.com> In-Reply-To: <1409845811-27943-7-git-send-email-stefanha@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PULL 6/7] net: complete all queued packets on VM stop List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi , qemu-devel@nongnu.org Cc: Peter Maydell , qemu-stable@nongnu.org, "Michael S. Tsirkin" On 09/04/2014 11:50 PM, Stefan Hajnoczi wrote: > From: "Michael S. Tsirkin" > > This completes all packets, ensuring that callbacks > will not run when VM is stopped. > > Cc: qemu-stable@nongnu.org > Cc: Jason Wang > Signed-off-by: Michael S. Tsirkin > Signed-off-by: Stefan Hajnoczi > --- > net/net.c | 33 ++++++++++++++++++++++++++++++++- > 1 file changed, 32 insertions(+), 1 deletion(-) > > diff --git a/net/net.c b/net/net.c > index 962c05f..7acc162 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -48,6 +48,7 @@ > # define CONFIG_NET_BRIDGE > #endif > > +static VMChangeStateEntry *net_change_state_entry; > static QTAILQ_HEAD(, NetClientState) net_clients; > > const char *host_net_devices[] = { > @@ -511,7 +512,8 @@ void qemu_purge_queued_packets(NetClientState *nc) > qemu_net_queue_purge(nc->peer->incoming_queue, nc); > } > > -void qemu_flush_queued_packets(NetClientState *nc) > +static > +void qemu_flush_or_purge_queued_packets(NetClientState *nc, bool purge) > { > nc->receive_disabled = 0; > > @@ -525,9 +527,17 @@ void qemu_flush_queued_packets(NetClientState *nc) > * the file descriptor (for tap, for example). > */ > qemu_notify_event(); > + } else if (purge) { > + /* Unable to empty the queue, purge remaining packets */ > + qemu_net_queue_purge(nc->incoming_queue, nc); > } > } > > +void qemu_flush_queued_packets(NetClientState *nc) > +{ > + qemu_flush_or_purge_queued_packets(nc, false); > +} > + > static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender, > unsigned flags, > const uint8_t *buf, int size, > @@ -1175,6 +1185,22 @@ void qmp_set_link(const char *name, bool up, Error **errp) > } > } > > +static void net_vm_change_state_handler(void *opaque, int running, > + RunState state) > +{ > + /* Complete all queued packets, to guarantee we don't modify > + * state later when VM is not running. > + */ > + if (!running) { > + NetClientState *nc; > + NetClientState *tmp; > + > + QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) { > + qemu_flush_or_purge_queued_packets(nc, true); > + } > + } > +} Something like net_drain_all_queue() in do_vm_stop() looks simpler. And doing this is tricky if it depends on other handlers to be called first. E.g virtio-net vm change state handler will set vm_running to false. And net_vm_change_state_handler() will be called after this. This means virtio_net_flush_tx() will still hit the assert since it will be called by packet cb (virtio_net_tx_complete()). > + > void net_cleanup(void) > { > NetClientState *nc; > @@ -1190,6 +1216,8 @@ void net_cleanup(void) > qemu_del_net_client(nc); > } > } > + > + qemu_del_vm_change_state_handler(net_change_state_entry); > } > > void net_check_clients(void) > @@ -1275,6 +1303,9 @@ int net_init_clients(void) > #endif > } > > + net_change_state_entry = > + qemu_add_vm_change_state_handler(net_vm_change_state_handler, NULL); > + > QTAILQ_INIT(&net_clients); > > if (qemu_opts_foreach(qemu_find_opts("netdev"), net_init_netdev, NULL, 1) == -1)