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

[Felipe Franciosi]

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<mailto: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<mailto:address@hidden>>


Reviewed-by: Marc-André Lureau <address@hidden<mailto: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