Re: [Qemu-devel] [PATCH v10 5/7] vhost: introduce vhost_backend_get_vq_index method

[Jason Wang <jasowang@redhat.com>]

On 09/18/2015 10:58 PM, Yuanhan Liu wrote:
> Minusing the idx with the base(dev->vq_index) for vhost-kernel, and
> then adding it back for vhost-user doesn't seem right. Here introduces
> a new method vhost_backend_get_vq_index() for getting the right vq
> index for following vhost messages calls.
>
> Suggested-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

Reviewed-by: Jason Wang <jasowang@redhat.com>

> ---
>  hw/virtio/vhost-backend.c         | 10 +++++++++-
>  hw/virtio/vhost-user.c            | 12 ++++++++++--
>  hw/virtio/vhost.c                 | 15 ++++++---------
>  include/hw/virtio/vhost-backend.h |  2 ++
>  4 files changed, 27 insertions(+), 12 deletions(-)
>
> diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
> index 4d68a27..72d1392 100644
> --- a/hw/virtio/vhost-backend.c
> +++ b/hw/virtio/vhost-backend.c
> @@ -42,11 +42,19 @@ static int vhost_kernel_cleanup(struct vhost_dev *dev)
>      return close(fd);
>  }
>  
> +static int vhost_kernel_get_vq_index(struct vhost_dev *dev, int idx)
> +{
> +    assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
> +
> +    return idx - dev->vq_index;
> +}
> +
>  static const VhostOps kernel_ops = {
>          .backend_type = VHOST_BACKEND_TYPE_KERNEL,
>          .vhost_call = vhost_kernel_call,
>          .vhost_backend_init = vhost_kernel_init,
> -        .vhost_backend_cleanup = vhost_kernel_cleanup
> +        .vhost_backend_cleanup = vhost_kernel_cleanup,
> +        .vhost_backend_get_vq_index = vhost_kernel_get_vq_index,
>  };
>  
>  int vhost_set_backend_type(struct vhost_dev *dev, VhostBackendType backend_type)
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 694fde5..5018fd6 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -393,9 +393,17 @@ static int vhost_user_cleanup(struct vhost_dev *dev)
>      return 0;
>  }
>  
> +static int vhost_user_get_vq_index(struct vhost_dev *dev, int idx)
> +{
> +    assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
> +
> +    return idx;
> +}
> +
>  const VhostOps user_ops = {
>          .backend_type = VHOST_BACKEND_TYPE_USER,
>          .vhost_call = vhost_user_call,
>          .vhost_backend_init = vhost_user_init,
> -        .vhost_backend_cleanup = vhost_user_cleanup
> -        };
> +        .vhost_backend_cleanup = vhost_user_cleanup,
> +        .vhost_backend_get_vq_index = vhost_user_get_vq_index,
> +};
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index a08c36b..7a7812d 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -719,7 +719,7 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
>  {
>      hwaddr s, l, a;
>      int r;
> -    int vhost_vq_index = idx - dev->vq_index;
> +    int vhost_vq_index = dev->vhost_ops->vhost_backend_get_vq_index(dev, idx);
>      struct vhost_vring_file file = {
>          .index = vhost_vq_index
>      };
> @@ -728,7 +728,6 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
>      };
>      struct VirtQueue *vvq = virtio_get_queue(vdev, idx);
>  
> -    assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
>  
>      vq->num = state.num = virtio_queue_get_num(vdev, idx);
>      r = dev->vhost_ops->vhost_call(dev, VHOST_SET_VRING_NUM, &state);
> @@ -822,12 +821,12 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
>                                      struct vhost_virtqueue *vq,
>                                      unsigned idx)
>  {
> -    int vhost_vq_index = idx - dev->vq_index;
> +    int vhost_vq_index = dev->vhost_ops->vhost_backend_get_vq_index(dev, idx);
>      struct vhost_vring_state state = {
>          .index = vhost_vq_index,
>      };
>      int r;
> -    assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
> +
>      r = dev->vhost_ops->vhost_call(dev, VHOST_GET_VRING_BASE, &state);
>      if (r < 0) {
>          fprintf(stderr, "vhost VQ %d ring restore failed: %d\n", idx, r);
> @@ -1066,17 +1065,15 @@ 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;
> +    struct vhost_vring_file file;
>  
> -    assert(n >= hdev->vq_index && n < hdev->vq_index + hdev->nvqs);
> -
> -    struct vhost_vring_file file = {
> -        .index = index
> -    };
>      if (mask) {
>          file.fd = event_notifier_get_fd(&hdev->vqs[index].masked_notifier);
>      } else {
>          file.fd = event_notifier_get_fd(virtio_queue_get_guest_notifier(vvq));
>      }
> +
> +    file.index = hdev->vhost_ops->vhost_backend_get_vq_index(hdev, n);
>      r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_VRING_CALL, &file);
>      assert(r >= 0);
>  }
> diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
> index e472f29..e1dfc6d 100644
> --- a/include/hw/virtio/vhost-backend.h
> +++ b/include/hw/virtio/vhost-backend.h
> @@ -24,12 +24,14 @@ typedef int (*vhost_call)(struct vhost_dev *dev, unsigned long int request,
>               void *arg);
>  typedef int (*vhost_backend_init)(struct vhost_dev *dev, void *opaque);
>  typedef int (*vhost_backend_cleanup)(struct vhost_dev *dev);
> +typedef int (*vhost_backend_get_vq_index)(struct vhost_dev *dev, int idx);
>  
>  typedef struct VhostOps {
>      VhostBackendType backend_type;
>      vhost_call vhost_call;
>      vhost_backend_init vhost_backend_init;
>      vhost_backend_cleanup vhost_backend_cleanup;
> +    vhost_backend_get_vq_index vhost_backend_get_vq_index;
>  } VhostOps;
>  
>  extern const VhostOps user_ops;