[Qemu-devel] [PATCH V2] vhost_net: start/stop guest notifiers properly

[Qemu-devel] [PATCH V2] vhost_net: start/stop guest notifiers properly

[Jason Wang]

commit a9f98bb5ebe6fb1869321dcc58e72041ae626ad8 vhost: multiqueue
support changed the order of stopping the device. Previously
vhost_dev_stop would disable backend and only afterwards, unset guest
notifiers. We now unset guest notifiers while vhost is still
active. This can lose interrupts causing guest networking to fail.

Additionally, remove the hdev->started assert in vhost.c since we may
want to start the guest notifiers before vhost starts and stop the
guest notifiers after vhost is stopped.

In particular, this has been observed during migration.

Reported-by: "Zhangjie (HZ)" <zhangjie14@huawei.com>
Cc: William Dauchy <wdauchy@gmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>

--

Zhang Jie, please test this patch to see if it fixes the issue.
---
 hw/net/vhost_net.c | 20 ++++++++++----------
 hw/virtio/vhost.c  |  2 --
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index f87c798..41d5158 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -308,6 +308,12 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
         goto err;
     }
 
+    r = k->set_guest_notifiers(qbus->parent, total_queues * 2, true);
+    if (r < 0) {
+        error_report("Error binding guest notifier: %d", -r);
+        goto err;
+    }
+
     for (i = 0; i < total_queues; i++) {
         r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev, i * 2);
 
@@ -316,12 +322,6 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
         }
     }
 
-    r = k->set_guest_notifiers(qbus->parent, total_queues * 2, true);
-    if (r < 0) {
-        error_report("Error binding guest notifier: %d", -r);
-        goto err;
-    }
-
     return 0;
 
 err:
@@ -339,16 +339,16 @@ void vhost_net_stop(VirtIODevice *dev, NetClientState *ncs,
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
     int i, r;
 
+    for (i = 0; i < total_queues; i++) {
+        vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev);
+    }
+
     r = k->set_guest_notifiers(qbus->parent, total_queues * 2, false);
     if (r < 0) {
         fprintf(stderr, "vhost guest notifier cleanup failed: %d\n", r);
         fflush(stderr);
     }
     assert(r >= 0);
-
-    for (i = 0; i < total_queues; i++) {
-        vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev);
-    }
 }
 
 void vhost_net_cleanup(struct vhost_net *net)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index e55fe1c..5d7c40a 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -976,7 +976,6 @@ void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
 bool vhost_virtqueue_pending(struct vhost_dev *hdev, int n)
 {
     struct vhost_virtqueue *vq = hdev->vqs + n - hdev->vq_index;
-    assert(hdev->started);
     assert(n >= hdev->vq_index && n < hdev->vq_index + hdev->nvqs);
     return event_notifier_test_and_clear(&vq->masked_notifier);
 }
@@ -988,7 +987,6 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n,
     struct VirtQueue *vvq = virtio_get_queue(vdev, n);
     int r, index = n - hdev->vq_index;
 
-    assert(hdev->started);
     assert(n >= hdev->vq_index && n < hdev->vq_index + hdev->nvqs);
 
     struct vhost_vring_file file = {
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH V2] vhost_net: start/stop guest notifiers properly

[Jason Wang]

On 08/19/2014 11:02 AM, Jason Wang wrote:
> commit a9f98bb5ebe6fb1869321dcc58e72041ae626ad8 vhost: multiqueue
> support changed the order of stopping the device. Previously
> vhost_dev_stop would disable backend and only afterwards, unset guest
> notifiers. We now unset guest notifiers while vhost is still
> active. This can lose interrupts causing guest networking to fail.
>
> Additionally, remove the hdev->started assert in vhost.c since we may
> want to start the guest notifiers before vhost starts and stop the
> guest notifiers after vhost is stopped.
>
> In particular, this has been observed during migration.
>
> Reported-by: "Zhangjie (HZ)" <zhangjie14@huawei.com>
> Cc: William Dauchy <wdauchy@gmail.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
>
> --

Looks like the patch breaks multiqueue test. I will post V3.