From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57632) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XPU4p-0005AZ-1J for qemu-devel@nongnu.org; Thu, 04 Sep 2014 06:15:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XPU4j-0003CL-Mf for qemu-devel@nongnu.org; Thu, 04 Sep 2014 06:15:38 -0400 Message-ID: <54083BB0.60202@redhat.com> Date: Thu, 04 Sep 2014 18:15:12 +0800 From: Jason Wang MIME-Version: 1.0 References: <1409816839-12309-1-git-send-email-mst@redhat.com> <1409816839-12309-2-git-send-email-mst@redhat.com> In-Reply-To: <1409816839-12309-2-git-send-email-mst@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/3] net: complete all queued packets on VM stop List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" , qemu-devel@nongnu.org Cc: qemu-stable@nongnu.org, Stefan Hajnoczi , Anthony Liguori On 09/04/2014 04:39 PM, Michael S. Tsirkin wrote: > 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 > --- > net/net.c | 33 ++++++++++++++++++++++++++++++++- > 1 file changed, 32 insertions(+), 1 deletion(-) > > diff --git a/net/net.c b/net/net.c > index 6d930ea..25fdb07 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -47,6 +47,7 @@ > # define CONFIG_NET_BRIDGE > #endif > > +static VMChangeStateEntry *net_change_state_entry; > static QTAILQ_HEAD(, NetClientState) net_clients; > > const char *host_net_devices[] = { > @@ -504,7 +505,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; > > @@ -518,9 +520,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, > @@ -1168,6 +1178,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); > + } > + } > +} > + > void net_cleanup(void) > { > NetClientState *nc; > @@ -1183,6 +1209,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) > @@ -1268,6 +1296,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) A problem is the dependency between state change handlers (e.g. virtio). Current virtio vmstate change handler will be called before this handler. Which means vdev->vm_running was false when we purge the queue, this will trigger the assert of vdev->vm_running in virtio_net_flush_tx().