Re: [Qemu-devel] [PATCH] Avoid additional GET_FEATURES call on vhost-user

[Michael S. Tsirkin]

Thanks for the reminder. 2.8 is open now so I can integrate this.
I would appreciate it if you could rebase this top of

        commit d1b4259f1ab18af24e6a297edb6a8f71691f3256
        Author: Maxime Coquelin <address@hidden>
        Date:   Tue Sep 13 15:30:30 2016 +0200
            virtio-bus: Plug devices after features are negotiated

test and repost if it still works.

Thanks!


On Mon, Sep 19, 2016 at 05:25:49PM +0000, Felipe Franciosi wrote:
> Gentle ping.
> 
>  
> 
> Thanks,
> 
> Felipe
> 
>  
> 
> From: Marc-André Lureau <address@hidden>
> Date: Thursday, 25 August 2016 at 12:14
> To: Felipe Franciosi <address@hidden>, "address@hidden"
> <address@hidden>, "Michael S. Tsirkin" <address@hidden>
> Subject: Re: [Qemu-devel] [PATCH] Avoid additional GET_FEATURES call on
> vhost-user
> 
>  
> 
>     Hi
> 
>      
> 
>     On Sat, Aug 20, 2016 at 3:28 AM Felipe Franciosi <address@hidden>
>     wrote:
> 
>         Vhost-user requires an early GET_FEATURES call to determine if the
>         slave supports protocol feature negotiation. An extra GET_FEATURES
>         call is made after vhost_backend_init() to actually set the device
>         features.
> 
>         This patch moves the actual setting of the device features to both
>         implementations (kernel/user) of vhost_backend_init(), therefore
>         eliminating the need for a second call.
> 
>         Signed-off-by: Felipe Franciosi <address@hidden>
> 
> 
> 
>     Reviewed-by: Marc-André Lureau <address@hidden>
> 
>      
> 
>         ---
>          hw/virtio/vhost-backend.c | 27 ++++++++++++++++++---------
>          hw/virtio/vhost-user.c    |  1 +
>          hw/virtio/vhost.c         |  9 ---------
>          3 files changed, 19 insertions(+), 18 deletions(-)
> 
>         diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
>         index 7681f15..a519fe2 100644
>         --- a/hw/virtio/vhost-backend.c
>         +++ b/hw/virtio/vhost-backend.c
>         @@ -25,15 +25,6 @@ static int vhost_kernel_call(struct vhost_dev *dev,
>         unsigned long int request,
>              return ioctl(fd, request, arg);
>          }
> 
>         -static int vhost_kernel_init(struct vhost_dev *dev, void *opaque)
>         -{
>         -    assert(dev->vhost_ops->backend_type == 
> VHOST_BACKEND_TYPE_KERNEL);
>         -
>         -    dev->opaque = opaque;
>         -
>         -    return 0;
>         -}
>         -
>          static int vhost_kernel_cleanup(struct vhost_dev *dev)
>          {
>              int fd = (uintptr_t) dev->opaque;
>         @@ -172,6 +163,24 @@ static int vhost_kernel_get_vq_index(struct
>         vhost_dev *dev, int idx)
>              return idx - dev->vq_index;
>          }
> 
>         +static int vhost_kernel_init(struct vhost_dev *dev, void *opaque)
>         +{
>         +    uint64_t features;
>         +    int r;
>         +
>         +    assert(dev->vhost_ops->backend_type == 
> VHOST_BACKEND_TYPE_KERNEL);
>         +
>         +    dev->opaque = opaque;
>         +
>         +    r = vhost_kernel_get_features(dev, &features);
>         +    if (r < 0) {
>         +        return r;
>         +    }
>         +    dev->features = features;
>         +
>         +    return 0;
>         +}
>         +
>          static const VhostOps kernel_ops = {
>                  .backend_type = VHOST_BACKEND_TYPE_KERNEL,
>                  .vhost_backend_init = vhost_kernel_init,
>         diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>         index b57454a..684f6d7 100644
>         --- a/hw/virtio/vhost-user.c
>         +++ b/hw/virtio/vhost-user.c
>         @@ -578,6 +578,7 @@ static int vhost_user_init(struct vhost_dev *dev,
>         void *opaque)
>              if (err < 0) {
>                  return err;
>              }
>         +    dev->features = features;
> 
>              if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES))
>         {
>                  dev->backend_features |= 1ULL <<
>         VHOST_USER_F_PROTOCOL_FEATURES;
>         diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>         index 3d0c807..cb9870a 100644
>         --- a/hw/virtio/vhost.c
>         +++ b/hw/virtio/vhost.c
>         @@ -1037,7 +1037,6 @@ static void vhost_virtqueue_cleanup(struct
>         vhost_virtqueue *vq)
>          int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
>                             VhostBackendType backend_type, uint32_t
>         busyloop_timeout)
>          {
>         -    uint64_t features;
>              int i, r, n_initialized_vqs = 0;
> 
>              hdev->migration_blocker = NULL;
>         @@ -1063,12 +1062,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void
>         *opaque,
>                  goto fail;
>              }
> 
>         -    r = hdev->vhost_ops->vhost_get_features(hdev, &features);
>         -    if (r < 0) {
>         -        VHOST_OPS_DEBUG("vhost_get_features failed");
>         -        goto fail;
>         -    }
>         -
>              for (i = 0; i < hdev->nvqs; ++i, ++n_initialized_vqs) {
>                  r = vhost_virtqueue_init(hdev, hdev->vqs + i, hdev->vq_index 
> +
>         i);
>                  if (r < 0) {
>         @@ -1086,8 +1079,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void
>         *opaque,
>                  }
>              }
> 
>         -    hdev->features = features;
>         -
>              hdev->memory_listener = (MemoryListener) {
>                  .begin = vhost_begin,
>                  .commit = vhost_commit,
>         --
>         1.9.5
> 
> 
>     --
> 
>     Marc-André Lureau
>