[Qemu-devel] [PATCH 1/2] add VirtIONet vhost_stopped flag to prevent multiple stops

[Dan Streetman]

From: Dan Streetman <ddstr...@canonical.com>

Buglink: https://launchpad.net/bugs/1823458

There is a race condition when using the vhost-user driver, between a guest
shutdown and the vhost-user interface being closed.  This is explained in
more detail at the bug link above; the short explanation is the vhost-user
device can be closed while the main thread is in the middle of stopping
the vhost_net.  In this case, the main thread handling shutdown will
enter virtio_net_vhost_status() and move into the n->vhost_started (else)
block, and call vhost_net_stop(); while it is running that function,
another thread is notified that the vhost-user device has been closed,
and (indirectly) calls into virtio_net_vhost_status() also.  Since the
vhost_net status hasn't yet changed, the second thread also enters
the n->vhost_started block, and also calls vhost_net_stop().  This
causes problems for the second thread when it tries to stop the network
that's already been stopped.

This adds a flag to the struct that's atomically set to prevent more than
one thread from calling vhost_net_stop().  The atomic_fetch_inc() is likely
overkill and probably could be done with a simple check-and-set, but
since it's a race condition there would still be a (very, very) small
window without using an atomic to set it.

Signed-off-by: Dan Streetman <ddstr...@canonical.com>
---
 hw/net/virtio-net.c            | 3 ++-
 include/hw/virtio/virtio-net.h | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index ffe0872fff..d36f50d5dd 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -13,6 +13,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/iov.h"
+#include "qemu/atomic.h"
 #include "hw/virtio/virtio.h"
 #include "net/net.h"
 #include "net/checksum.h"
@@ -240,7 +241,7 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t 
status)
                          "falling back on userspace virtio", -r);
             n->vhost_started = 0;
         }
-    } else {
+    } else if (atomic_fetch_inc(&n->vhost_stopped) == 0) {
         vhost_net_stop(vdev, n->nic->ncs, queues);
         n->vhost_started = 0;
     }
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index b96f0c643f..d03fd933d0 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -164,6 +164,7 @@ struct VirtIONet {
     uint8_t nouni;
     uint8_t nobcast;
     uint8_t vhost_started;
+    int vhost_stopped;
     struct {
         uint32_t in_use;
         uint32_t first_multi;
-- 
2.20.1