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

[Felipe Franciosi]

> On 19 Sep 2016, at 18:42, Michael S. Tsirkin <address@hidden> wrote:
> 
> 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!

No problem! Done! :)

http://lists.nongnu.org/archive/html/qemu-devel/2016-09/msg05582.html

Felipe

> 
> 
> 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
>>