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

[Michael S. Tsirkin]

On Mon, Aug 18, 2014 at 05:51:31PM +0800, 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)" <address@hidden>
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> Signed-off-by: Jason Wang <address@hidden>


This doesn't seem to apply to master.
Can you rebase please?
> --
> 
> 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 006576d..72084ba 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -223,6 +223,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(tap_get_vhost_net(ncs[i].peer), dev, i * 2);
>  
> @@ -231,12 +237,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:
> @@ -254,16 +254,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(tap_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(tap_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 9e336ad..d74514a 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -969,7 +969,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);
>  }
> @@ -981,7 +980,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