qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/7] vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously
@ 2023-10-02 20:32 Laszlo Ersek
  2023-10-02 20:32 ` [PATCH v3 1/7] vhost-user: strip superfluous whitespace Laszlo Ersek
                   ` (7 more replies)
  0 siblings, 8 replies; 15+ messages in thread
From: Laszlo Ersek @ 2023-10-02 20:32 UTC (permalink / raw)
  To: qemu-devel, lersek

v2:

- http://mid.mail-archive.com/20230830134055.106812-1-lersek@redhat.com
- https://patchwork.ozlabs.org/project/qemu-devel/cover/20230830134055.106812-1-lersek@redhat.com/

v3 picks up tags from Phil, Eugenio and Albert, and updates the commit
message on patch#7 according to Eugenio's comments.

Retested.

Laszlo Ersek (7):
  vhost-user: strip superfluous whitespace
  vhost-user: tighten "reply_supported" scope in "set_vring_addr"
  vhost-user: factor out "vhost_user_write_sync"
  vhost-user: flatten "enforce_reply" into "vhost_user_write_sync"
  vhost-user: hoist "write_sync", "get_features", "get_u64"
  vhost-user: allow "vhost_set_vring" to wait for a reply
  vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously

 hw/virtio/vhost-user.c | 216 ++++++++++----------
 1 file changed, 108 insertions(+), 108 deletions(-)


base-commit: 36e9aab3c569d4c9ad780473596e18479838d1aa

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3 1/7] vhost-user: strip superfluous whitespace
  2023-10-02 20:32 [PATCH v3 0/7] vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously Laszlo Ersek
@ 2023-10-02 20:32 ` Laszlo Ersek
  2023-10-04  9:06   ` Michael S. Tsirkin
  2023-10-02 20:32 ` [PATCH v3 2/7] vhost-user: tighten "reply_supported" scope in "set_vring_addr" Laszlo Ersek
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Laszlo Ersek @ 2023-10-02 20:32 UTC (permalink / raw)
  To: qemu-devel, lersek
  Cc: Michael S. Tsirkin, Eugenio Perez Martin, German Maglione,
	Liu Jiang, Sergio Lopez Pascual, Stefano Garzarella

Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:vhost)
Cc: Eugenio Perez Martin <eperezma@redhat.com>
Cc: German Maglione <gmaglione@redhat.com>
Cc: Liu Jiang <gerry@linux.alibaba.com>
Cc: Sergio Lopez Pascual <slp@redhat.com>
Cc: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Albert Esteve <aesteve@redhat.com>
Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
---

Notes:
    v3:
    
    - pick up R-b from Phil and Eugenio, T-b from Albert
    
    v2:
    
    - pick up Stefano's R-b

 hw/virtio/vhost-user.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 8dcf049d422b..b4b677c1ce66 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -398,7 +398,7 @@ static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg,
      * operations such as configuring device memory mappings or issuing device
      * resets, which affect the whole device instead of individual VQs,
      * vhost-user messages should only be sent once.
-     * 
+     *
      * Devices with multiple vhost_devs are given an associated dev->vq_index
      * so per_device requests are only sent if vq_index is 0.
      */


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 2/7] vhost-user: tighten "reply_supported" scope in "set_vring_addr"
  2023-10-02 20:32 [PATCH v3 0/7] vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously Laszlo Ersek
  2023-10-02 20:32 ` [PATCH v3 1/7] vhost-user: strip superfluous whitespace Laszlo Ersek
@ 2023-10-02 20:32 ` Laszlo Ersek
  2023-10-02 20:32 ` [PATCH v3 3/7] vhost-user: factor out "vhost_user_write_sync" Laszlo Ersek
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2023-10-02 20:32 UTC (permalink / raw)
  To: qemu-devel, lersek
  Cc: Michael S. Tsirkin, Eugenio Perez Martin, German Maglione,
	Liu Jiang, Sergio Lopez Pascual, Stefano Garzarella

In the vhost_user_set_vring_addr() function, we calculate
"reply_supported" unconditionally, even though we'll only need it if
"wait_for_reply" is also true.

Restrict the scope of "reply_supported" to the minimum.

This is purely refactoring -- no observable change.

Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:vhost)
Cc: Eugenio Perez Martin <eperezma@redhat.com>
Cc: German Maglione <gmaglione@redhat.com>
Cc: Liu Jiang <gerry@linux.alibaba.com>
Cc: Sergio Lopez Pascual <slp@redhat.com>
Cc: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Tested-by: Albert Esteve <aesteve@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
---

Notes:
    v3:
    
    - pick up R-b from Phil and Eugenio, T-b from Albert
    
    v2:
    
    - pick up Stefano's R-b

 hw/virtio/vhost-user.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index b4b677c1ce66..64eac317bfb2 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1331,17 +1331,18 @@ static int vhost_user_set_vring_addr(struct vhost_dev *dev,
         .hdr.size = sizeof(msg.payload.addr),
     };
 
-    bool reply_supported = virtio_has_feature(dev->protocol_features,
-                                              VHOST_USER_PROTOCOL_F_REPLY_ACK);
-
     /*
      * wait for a reply if logging is enabled to make sure
      * backend is actually logging changes
      */
     bool wait_for_reply = addr->flags & (1 << VHOST_VRING_F_LOG);
 
-    if (reply_supported && wait_for_reply) {
-        msg.hdr.flags |= VHOST_USER_NEED_REPLY_MASK;
+    if (wait_for_reply) {
+        bool reply_supported = virtio_has_feature(dev->protocol_features,
+                                          VHOST_USER_PROTOCOL_F_REPLY_ACK);
+        if (reply_supported) {
+            msg.hdr.flags |= VHOST_USER_NEED_REPLY_MASK;
+        }
     }
 
     ret = vhost_user_write(dev, &msg, NULL, 0);


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 3/7] vhost-user: factor out "vhost_user_write_sync"
  2023-10-02 20:32 [PATCH v3 0/7] vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously Laszlo Ersek
  2023-10-02 20:32 ` [PATCH v3 1/7] vhost-user: strip superfluous whitespace Laszlo Ersek
  2023-10-02 20:32 ` [PATCH v3 2/7] vhost-user: tighten "reply_supported" scope in "set_vring_addr" Laszlo Ersek
@ 2023-10-02 20:32 ` Laszlo Ersek
  2023-10-02 20:32 ` [PATCH v3 4/7] vhost-user: flatten "enforce_reply" into "vhost_user_write_sync" Laszlo Ersek
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2023-10-02 20:32 UTC (permalink / raw)
  To: qemu-devel, lersek
  Cc: Michael S. Tsirkin, Eugenio Perez Martin, German Maglione,
	Liu Jiang, Sergio Lopez Pascual, Stefano Garzarella

The tails of the "vhost_user_set_vring_addr" and "vhost_user_set_u64"
functions are now byte-for-byte identical. Factor the common tail out to a
new function called "vhost_user_write_sync".

This is purely refactoring -- no observable change.

Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:vhost)
Cc: Eugenio Perez Martin <eperezma@redhat.com>
Cc: German Maglione <gmaglione@redhat.com>
Cc: Liu Jiang <gerry@linux.alibaba.com>
Cc: Sergio Lopez Pascual <slp@redhat.com>
Cc: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Tested-by: Albert Esteve <aesteve@redhat.com>
Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
---

Notes:
    v3:
    
    - pick up R-b from Eugenio, T-b from Albert
    
    v2:
    
    - pick up R-b's from Phil and Stefano
    
    - rename "vhost_user_write_msg" to "vhost_user_write_sync" (in code and
      commit message) [Stefano]

 hw/virtio/vhost-user.c | 66 +++++++++-----------
 1 file changed, 28 insertions(+), 38 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 64eac317bfb2..1476b36f0a6e 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1320,10 +1320,35 @@ static int enforce_reply(struct vhost_dev *dev,
     return vhost_user_get_features(dev, &dummy);
 }
 
+/* Note: "msg->hdr.flags" may be modified. */
+static int vhost_user_write_sync(struct vhost_dev *dev, VhostUserMsg *msg,
+                                 bool wait_for_reply)
+{
+    int ret;
+
+    if (wait_for_reply) {
+        bool reply_supported = virtio_has_feature(dev->protocol_features,
+                                          VHOST_USER_PROTOCOL_F_REPLY_ACK);
+        if (reply_supported) {
+            msg->hdr.flags |= VHOST_USER_NEED_REPLY_MASK;
+        }
+    }
+
+    ret = vhost_user_write(dev, msg, NULL, 0);
+    if (ret < 0) {
+        return ret;
+    }
+
+    if (wait_for_reply) {
+        return enforce_reply(dev, msg);
+    }
+
+    return 0;
+}
+
 static int vhost_user_set_vring_addr(struct vhost_dev *dev,
                                      struct vhost_vring_addr *addr)
 {
-    int ret;
     VhostUserMsg msg = {
         .hdr.request = VHOST_USER_SET_VRING_ADDR,
         .hdr.flags = VHOST_USER_VERSION,
@@ -1337,24 +1362,7 @@ static int vhost_user_set_vring_addr(struct vhost_dev *dev,
      */
     bool wait_for_reply = addr->flags & (1 << VHOST_VRING_F_LOG);
 
-    if (wait_for_reply) {
-        bool reply_supported = virtio_has_feature(dev->protocol_features,
-                                          VHOST_USER_PROTOCOL_F_REPLY_ACK);
-        if (reply_supported) {
-            msg.hdr.flags |= VHOST_USER_NEED_REPLY_MASK;
-        }
-    }
-
-    ret = vhost_user_write(dev, &msg, NULL, 0);
-    if (ret < 0) {
-        return ret;
-    }
-
-    if (wait_for_reply) {
-        return enforce_reply(dev, &msg);
-    }
-
-    return 0;
+    return vhost_user_write_sync(dev, &msg, wait_for_reply);
 }
 
 static int vhost_user_set_u64(struct vhost_dev *dev, int request, uint64_t u64,
@@ -1366,26 +1374,8 @@ static int vhost_user_set_u64(struct vhost_dev *dev, int request, uint64_t u64,
         .payload.u64 = u64,
         .hdr.size = sizeof(msg.payload.u64),
     };
-    int ret;
 
-    if (wait_for_reply) {
-        bool reply_supported = virtio_has_feature(dev->protocol_features,
-                                          VHOST_USER_PROTOCOL_F_REPLY_ACK);
-        if (reply_supported) {
-            msg.hdr.flags |= VHOST_USER_NEED_REPLY_MASK;
-        }
-    }
-
-    ret = vhost_user_write(dev, &msg, NULL, 0);
-    if (ret < 0) {
-        return ret;
-    }
-
-    if (wait_for_reply) {
-        return enforce_reply(dev, &msg);
-    }
-
-    return 0;
+    return vhost_user_write_sync(dev, &msg, wait_for_reply);
 }
 
 static int vhost_user_set_status(struct vhost_dev *dev, uint8_t status)


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 4/7] vhost-user: flatten "enforce_reply" into "vhost_user_write_sync"
  2023-10-02 20:32 [PATCH v3 0/7] vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously Laszlo Ersek
                   ` (2 preceding siblings ...)
  2023-10-02 20:32 ` [PATCH v3 3/7] vhost-user: factor out "vhost_user_write_sync" Laszlo Ersek
@ 2023-10-02 20:32 ` Laszlo Ersek
  2023-10-02 20:32 ` [PATCH v3 5/7] vhost-user: hoist "write_sync", "get_features", "get_u64" Laszlo Ersek
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2023-10-02 20:32 UTC (permalink / raw)
  To: qemu-devel, lersek
  Cc: Michael S. Tsirkin, Eugenio Perez Martin, German Maglione,
	Liu Jiang, Sergio Lopez Pascual, Stefano Garzarella

At this point, only "vhost_user_write_sync" calls "enforce_reply"; embed
the latter into the former.

This is purely refactoring -- no observable change.

Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:vhost)
Cc: Eugenio Perez Martin <eperezma@redhat.com>
Cc: German Maglione <gmaglione@redhat.com>
Cc: Liu Jiang <gerry@linux.alibaba.com>
Cc: Sergio Lopez Pascual <slp@redhat.com>
Cc: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Tested-by: Albert Esteve <aesteve@redhat.com>
Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
---

Notes:
    v3:
    
    - pick up R-b from Eugenio, T-b from Albert
    
    v2:
    
    - pick up R-b's from Phil and Stefano
    
    - rename "vhost_user_write_msg" to "vhost_user_write_sync" (in code
      context and commit message) [Stefano]

 hw/virtio/vhost-user.c | 32 ++++++++------------
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 1476b36f0a6e..4129ba72e408 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1302,24 +1302,6 @@ static int vhost_user_get_features(struct vhost_dev *dev, uint64_t *features)
     return 0;
 }
 
-static int enforce_reply(struct vhost_dev *dev,
-                         const VhostUserMsg *msg)
-{
-    uint64_t dummy;
-
-    if (msg->hdr.flags & VHOST_USER_NEED_REPLY_MASK) {
-        return process_message_reply(dev, msg);
-    }
-
-   /*
-    * We need to wait for a reply but the backend does not
-    * support replies for the command we just sent.
-    * Send VHOST_USER_GET_FEATURES which makes all backends
-    * send a reply.
-    */
-    return vhost_user_get_features(dev, &dummy);
-}
-
 /* Note: "msg->hdr.flags" may be modified. */
 static int vhost_user_write_sync(struct vhost_dev *dev, VhostUserMsg *msg,
                                  bool wait_for_reply)
@@ -1340,7 +1322,19 @@ static int vhost_user_write_sync(struct vhost_dev *dev, VhostUserMsg *msg,
     }
 
     if (wait_for_reply) {
-        return enforce_reply(dev, msg);
+        uint64_t dummy;
+
+        if (msg->hdr.flags & VHOST_USER_NEED_REPLY_MASK) {
+            return process_message_reply(dev, msg);
+        }
+
+       /*
+        * We need to wait for a reply but the backend does not
+        * support replies for the command we just sent.
+        * Send VHOST_USER_GET_FEATURES which makes all backends
+        * send a reply.
+        */
+        return vhost_user_get_features(dev, &dummy);
     }
 
     return 0;


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 5/7] vhost-user: hoist "write_sync", "get_features", "get_u64"
  2023-10-02 20:32 [PATCH v3 0/7] vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously Laszlo Ersek
                   ` (3 preceding siblings ...)
  2023-10-02 20:32 ` [PATCH v3 4/7] vhost-user: flatten "enforce_reply" into "vhost_user_write_sync" Laszlo Ersek
@ 2023-10-02 20:32 ` Laszlo Ersek
  2023-10-02 20:32 ` [PATCH v3 6/7] vhost-user: allow "vhost_set_vring" to wait for a reply Laszlo Ersek
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2023-10-02 20:32 UTC (permalink / raw)
  To: qemu-devel, lersek
  Cc: Michael S. Tsirkin, Eugenio Perez Martin, German Maglione,
	Liu Jiang, Sergio Lopez Pascual, Stefano Garzarella

In order to avoid a forward-declaration for "vhost_user_write_sync" in a
subsequent patch, hoist "vhost_user_write_sync" ->
"vhost_user_get_features" -> "vhost_user_get_u64" just above
"vhost_set_vring".

This is purely code movement -- no observable change.

Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:vhost)
Cc: Eugenio Perez Martin <eperezma@redhat.com>
Cc: German Maglione <gmaglione@redhat.com>
Cc: Liu Jiang <gerry@linux.alibaba.com>
Cc: Sergio Lopez Pascual <slp@redhat.com>
Cc: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Tested-by: Albert Esteve <aesteve@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
---

Notes:
    v3:
    
    - pick up R-b from Phil and Eugenio, T-b from Albert
    
    v2:
    
    - pick up R-b from Stefano
    
    - rename "vhost_user_write_msg" to "vhost_user_write_sync" (in code and
      commit message) [Stefano]

 hw/virtio/vhost-user.c | 170 ++++++++++----------
 1 file changed, 85 insertions(+), 85 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 4129ba72e408..c79b6f77cdca 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1083,6 +1083,91 @@ static int vhost_user_set_vring_endian(struct vhost_dev *dev,
     return vhost_user_write(dev, &msg, NULL, 0);
 }
 
+static int vhost_user_get_u64(struct vhost_dev *dev, int request, uint64_t *u64)
+{
+    int ret;
+    VhostUserMsg msg = {
+        .hdr.request = request,
+        .hdr.flags = VHOST_USER_VERSION,
+    };
+
+    if (vhost_user_per_device_request(request) && dev->vq_index != 0) {
+        return 0;
+    }
+
+    ret = vhost_user_write(dev, &msg, NULL, 0);
+    if (ret < 0) {
+        return ret;
+    }
+
+    ret = vhost_user_read(dev, &msg);
+    if (ret < 0) {
+        return ret;
+    }
+
+    if (msg.hdr.request != request) {
+        error_report("Received unexpected msg type. Expected %d received %d",
+                     request, msg.hdr.request);
+        return -EPROTO;
+    }
+
+    if (msg.hdr.size != sizeof(msg.payload.u64)) {
+        error_report("Received bad msg size.");
+        return -EPROTO;
+    }
+
+    *u64 = msg.payload.u64;
+
+    return 0;
+}
+
+static int vhost_user_get_features(struct vhost_dev *dev, uint64_t *features)
+{
+    if (vhost_user_get_u64(dev, VHOST_USER_GET_FEATURES, features) < 0) {
+        return -EPROTO;
+    }
+
+    return 0;
+}
+
+/* Note: "msg->hdr.flags" may be modified. */
+static int vhost_user_write_sync(struct vhost_dev *dev, VhostUserMsg *msg,
+                                 bool wait_for_reply)
+{
+    int ret;
+
+    if (wait_for_reply) {
+        bool reply_supported = virtio_has_feature(dev->protocol_features,
+                                          VHOST_USER_PROTOCOL_F_REPLY_ACK);
+        if (reply_supported) {
+            msg->hdr.flags |= VHOST_USER_NEED_REPLY_MASK;
+        }
+    }
+
+    ret = vhost_user_write(dev, msg, NULL, 0);
+    if (ret < 0) {
+        return ret;
+    }
+
+    if (wait_for_reply) {
+        uint64_t dummy;
+
+        if (msg->hdr.flags & VHOST_USER_NEED_REPLY_MASK) {
+            return process_message_reply(dev, msg);
+        }
+
+       /*
+        * We need to wait for a reply but the backend does not
+        * support replies for the command we just sent.
+        * Send VHOST_USER_GET_FEATURES which makes all backends
+        * send a reply.
+        */
+        return vhost_user_get_features(dev, &dummy);
+    }
+
+    return 0;
+}
+
 static int vhost_set_vring(struct vhost_dev *dev,
                            unsigned long int request,
                            struct vhost_vring_state *ring)
@@ -1255,91 +1340,6 @@ static int vhost_user_set_vring_err(struct vhost_dev *dev,
     return vhost_set_vring_file(dev, VHOST_USER_SET_VRING_ERR, file);
 }
 
-static int vhost_user_get_u64(struct vhost_dev *dev, int request, uint64_t *u64)
-{
-    int ret;
-    VhostUserMsg msg = {
-        .hdr.request = request,
-        .hdr.flags = VHOST_USER_VERSION,
-    };
-
-    if (vhost_user_per_device_request(request) && dev->vq_index != 0) {
-        return 0;
-    }
-
-    ret = vhost_user_write(dev, &msg, NULL, 0);
-    if (ret < 0) {
-        return ret;
-    }
-
-    ret = vhost_user_read(dev, &msg);
-    if (ret < 0) {
-        return ret;
-    }
-
-    if (msg.hdr.request != request) {
-        error_report("Received unexpected msg type. Expected %d received %d",
-                     request, msg.hdr.request);
-        return -EPROTO;
-    }
-
-    if (msg.hdr.size != sizeof(msg.payload.u64)) {
-        error_report("Received bad msg size.");
-        return -EPROTO;
-    }
-
-    *u64 = msg.payload.u64;
-
-    return 0;
-}
-
-static int vhost_user_get_features(struct vhost_dev *dev, uint64_t *features)
-{
-    if (vhost_user_get_u64(dev, VHOST_USER_GET_FEATURES, features) < 0) {
-        return -EPROTO;
-    }
-
-    return 0;
-}
-
-/* Note: "msg->hdr.flags" may be modified. */
-static int vhost_user_write_sync(struct vhost_dev *dev, VhostUserMsg *msg,
-                                 bool wait_for_reply)
-{
-    int ret;
-
-    if (wait_for_reply) {
-        bool reply_supported = virtio_has_feature(dev->protocol_features,
-                                          VHOST_USER_PROTOCOL_F_REPLY_ACK);
-        if (reply_supported) {
-            msg->hdr.flags |= VHOST_USER_NEED_REPLY_MASK;
-        }
-    }
-
-    ret = vhost_user_write(dev, msg, NULL, 0);
-    if (ret < 0) {
-        return ret;
-    }
-
-    if (wait_for_reply) {
-        uint64_t dummy;
-
-        if (msg->hdr.flags & VHOST_USER_NEED_REPLY_MASK) {
-            return process_message_reply(dev, msg);
-        }
-
-       /*
-        * We need to wait for a reply but the backend does not
-        * support replies for the command we just sent.
-        * Send VHOST_USER_GET_FEATURES which makes all backends
-        * send a reply.
-        */
-        return vhost_user_get_features(dev, &dummy);
-    }
-
-    return 0;
-}
-
 static int vhost_user_set_vring_addr(struct vhost_dev *dev,
                                      struct vhost_vring_addr *addr)
 {


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 6/7] vhost-user: allow "vhost_set_vring" to wait for a reply
  2023-10-02 20:32 [PATCH v3 0/7] vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously Laszlo Ersek
                   ` (4 preceding siblings ...)
  2023-10-02 20:32 ` [PATCH v3 5/7] vhost-user: hoist "write_sync", "get_features", "get_u64" Laszlo Ersek
@ 2023-10-02 20:32 ` Laszlo Ersek
  2023-10-02 20:32 ` [PATCH v3 7/7] vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously Laszlo Ersek
  2023-10-18 11:26 ` [PATCH v3 0/7] " Laszlo Ersek
  7 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2023-10-02 20:32 UTC (permalink / raw)
  To: qemu-devel, lersek
  Cc: Michael S. Tsirkin, Eugenio Perez Martin, German Maglione,
	Liu Jiang, Sergio Lopez Pascual, Stefano Garzarella

The "vhost_set_vring" function already centralizes the common parts of
"vhost_user_set_vring_num", "vhost_user_set_vring_base" and
"vhost_user_set_vring_enable". We'll want to allow some of those callers
to wait for a reply.

Therefore, rebase "vhost_set_vring" from just "vhost_user_write" to
"vhost_user_write_sync", exposing the "wait_for_reply" parameter.

This is purely refactoring -- there is no observable change. That's
because:

- all three callers pass in "false" for "wait_for_reply", which disables
  all logic in "vhost_user_write_sync" except the call to
  "vhost_user_write";

- the fds=NULL and fd_num=0 arguments of the original "vhost_user_write"
  call inside "vhost_set_vring" are hard-coded within
  "vhost_user_write_sync".

Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:vhost)
Cc: Eugenio Perez Martin <eperezma@redhat.com>
Cc: German Maglione <gmaglione@redhat.com>
Cc: Liu Jiang <gerry@linux.alibaba.com>
Cc: Sergio Lopez Pascual <slp@redhat.com>
Cc: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Tested-by: Albert Esteve <aesteve@redhat.com>
Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
---

Notes:
    v3:
    
    - pick up R-b from Eugenio, T-b from Albert
    
    v2:
    
    - pick up R-b's from Phil and Stefano
    
    - rename "vhost_user_write_msg" to "vhost_user_write_sync" (in code and
      commit message) [Stefano]

 hw/virtio/vhost-user.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index c79b6f77cdca..18e15a9bb359 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1170,7 +1170,8 @@ static int vhost_user_write_sync(struct vhost_dev *dev, VhostUserMsg *msg,
 
 static int vhost_set_vring(struct vhost_dev *dev,
                            unsigned long int request,
-                           struct vhost_vring_state *ring)
+                           struct vhost_vring_state *ring,
+                           bool wait_for_reply)
 {
     VhostUserMsg msg = {
         .hdr.request = request,
@@ -1179,13 +1180,13 @@ static int vhost_set_vring(struct vhost_dev *dev,
         .hdr.size = sizeof(msg.payload.state),
     };
 
-    return vhost_user_write(dev, &msg, NULL, 0);
+    return vhost_user_write_sync(dev, &msg, wait_for_reply);
 }
 
 static int vhost_user_set_vring_num(struct vhost_dev *dev,
                                     struct vhost_vring_state *ring)
 {
-    return vhost_set_vring(dev, VHOST_USER_SET_VRING_NUM, ring);
+    return vhost_set_vring(dev, VHOST_USER_SET_VRING_NUM, ring, false);
 }
 
 static void vhost_user_host_notifier_free(VhostUserHostNotifier *n)
@@ -1216,7 +1217,7 @@ static void vhost_user_host_notifier_remove(VhostUserHostNotifier *n,
 static int vhost_user_set_vring_base(struct vhost_dev *dev,
                                      struct vhost_vring_state *ring)
 {
-    return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring);
+    return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring, false);
 }
 
 static int vhost_user_set_vring_enable(struct vhost_dev *dev, int enable)
@@ -1234,7 +1235,7 @@ static int vhost_user_set_vring_enable(struct vhost_dev *dev, int enable)
             .num   = enable,
         };
 
-        ret = vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, &state);
+        ret = vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, &state, false);
         if (ret < 0) {
             /*
              * Restoring the previous state is likely infeasible, as well as


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 7/7] vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously
  2023-10-02 20:32 [PATCH v3 0/7] vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously Laszlo Ersek
                   ` (5 preceding siblings ...)
  2023-10-02 20:32 ` [PATCH v3 6/7] vhost-user: allow "vhost_set_vring" to wait for a reply Laszlo Ersek
@ 2023-10-02 20:32 ` Laszlo Ersek
  2023-10-18 11:26 ` [PATCH v3 0/7] " Laszlo Ersek
  7 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2023-10-02 20:32 UTC (permalink / raw)
  To: qemu-devel, lersek
  Cc: Michael S. Tsirkin, Eugenio Perez Martin, German Maglione,
	Liu Jiang, Sergio Lopez Pascual, Stefano Garzarella

(1) The virtio-1.2 specification
<http://docs.oasis-open.org/virtio/virtio/v1.2/virtio-v1.2.html> writes:

> 3     General Initialization And Device Operation
> 3.1   Device Initialization
> 3.1.1 Driver Requirements: Device Initialization
>
> [...]
>
> 7. Perform device-specific setup, including discovery of virtqueues for
>    the device, optional per-bus setup, reading and possibly writing the
>    device’s virtio configuration space, and population of virtqueues.
>
> 8. Set the DRIVER_OK status bit. At this point the device is “live”.

and

> 4         Virtio Transport Options
> 4.1       Virtio Over PCI Bus
> 4.1.4     Virtio Structure PCI Capabilities
> 4.1.4.3   Common configuration structure layout
> 4.1.4.3.2 Driver Requirements: Common configuration structure layout
>
> [...]
>
> The driver MUST configure the other virtqueue fields before enabling the
> virtqueue with queue_enable.
>
> [...]

(The same statements are present in virtio-1.0 identically, at
<http://docs.oasis-open.org/virtio/virtio/v1.0/virtio-v1.0.html>.)

These together mean that the following sub-sequence of steps is valid for
a virtio-1.0 guest driver:

(1.1) set "queue_enable" for the needed queues as the final part of device
initialization step (7),

(1.2) set DRIVER_OK in step (8),

(1.3) immediately start sending virtio requests to the device.

(2) When vhost-user is enabled, and the VHOST_USER_F_PROTOCOL_FEATURES
special virtio feature is negotiated, then virtio rings start in disabled
state, according to
<https://qemu-project.gitlab.io/qemu/interop/vhost-user.html#ring-states>.
In this case, explicit VHOST_USER_SET_VRING_ENABLE messages are needed for
enabling vrings.

Therefore setting "queue_enable" from the guest (1.1) -- which is
technically "buffered" on the QEMU side until the guest sets DRIVER_OK
(1.2) -- is a *control plane* operation, which -- after (1.2) -- travels
from the guest through QEMU to the vhost-user backend, using a unix domain
socket.

Whereas sending a virtio request (1.3) is a *data plane* operation, which
evades QEMU -- it travels from guest to the vhost-user backend via
eventfd.

This means that operations ((1.1) + (1.2)) and (1.3) travel through
different channels, and their relative order can be reversed, as perceived
by the vhost-user backend.

That's exactly what happens when OVMF's virtiofs driver (VirtioFsDxe) runs
against the Rust-language virtiofsd version 1.7.2. (Which uses version
0.10.1 of the vhost-user-backend crate, and version 0.8.1 of the vhost
crate.)

Namely, when VirtioFsDxe binds a virtiofs device, it goes through the
device initialization steps (i.e., control plane operations), and
immediately sends a FUSE_INIT request too (i.e., performs a data plane
operation). In the Rust-language virtiofsd, this creates a race between
two components that run *concurrently*, i.e., in different threads or
processes:

- Control plane, handling vhost-user protocol messages:

  The "VhostUserSlaveReqHandlerMut::set_vring_enable" method
  [crates/vhost-user-backend/src/handler.rs] handles
  VHOST_USER_SET_VRING_ENABLE messages, and updates each vring's "enabled"
  flag according to the message processed.

- Data plane, handling virtio / FUSE requests:

  The "VringEpollHandler::handle_event" method
  [crates/vhost-user-backend/src/event_loop.rs] handles the incoming
  virtio / FUSE request, consuming the virtio kick at the same time. If
  the vring's "enabled" flag is set, the virtio / FUSE request is
  processed genuinely. If the vring's "enabled" flag is clear, then the
  virtio / FUSE request is discarded.

Note that OVMF enables the queue *first*, and sends FUSE_INIT *second*.
However, if the data plane processor in virtiofsd wins the race, then it
sees the FUSE_INIT *before* the control plane processor took notice of
VHOST_USER_SET_VRING_ENABLE and green-lit the queue for the data plane
processor. Therefore the latter drops FUSE_INIT on the floor, and goes
back to waiting for further virtio / FUSE requests with epoll_wait.
Meanwhile OVMF is stuck waiting for the FUSET_INIT response -- a deadlock.

The deadlock is not deterministic. OVMF hangs infrequently during first
boot. However, OVMF hangs almost certainly during reboots from the UEFI
shell.

The race can be "reliably masked" by inserting a very small delay -- a
single debug message -- at the top of "VringEpollHandler::handle_event",
i.e., just before the data plane processor checks the "enabled" field of
the vring. That delay suffices for the control plane processor to act upon
VHOST_USER_SET_VRING_ENABLE.

We can deterministically prevent the race in QEMU, by blocking OVMF inside
step (1.2) -- i.e., in the write to the device status register that
"unleashes" queue enablement -- until VHOST_USER_SET_VRING_ENABLE actually
*completes*. That way OVMF's VCPU cannot advance to the FUSE_INIT
submission before virtiofsd's control plane processor takes notice of the
queue being enabled.

Wait for VHOST_USER_SET_VRING_ENABLE completion by:

- setting the NEED_REPLY flag on VHOST_USER_SET_VRING_ENABLE, and waiting
  for the reply, if the VHOST_USER_PROTOCOL_F_REPLY_ACK vhost-user feature
  has been negotiated, or

- performing a separate VHOST_USER_GET_FEATURES *exchange*, which requires
  a backend response regardless of VHOST_USER_PROTOCOL_F_REPLY_ACK.

Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:vhost)
Cc: Eugenio Perez Martin <eperezma@redhat.com>
Cc: German Maglione <gmaglione@redhat.com>
Cc: Liu Jiang <gerry@linux.alibaba.com>
Cc: Sergio Lopez Pascual <slp@redhat.com>
Cc: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Tested-by: Albert Esteve <aesteve@redhat.com>
[lersek@redhat.com: work Eugenio's explanation into the commit message,
 about QEMU containing step (1.1) until step (1.2)]
Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
---

Notes:
    v3:
    
    - pick up R-b from Eugenio, T-b from Albert
    
    - clarify commit message (also give permanent credit for the
      clarification; I feel the change is important enough) [Eugenio]
    
    v2:
    
    - pick up R-b from Stefano
    
    - update virtio spec reference from 1.0 to 1.2 (also keep the 1.0 ref)
      in the commit message; re-check the quotes / section headers [Stefano]
    
    - summarize commit message in code comment [Stefano]

 hw/virtio/vhost-user.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 18e15a9bb359..41842eb023b5 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1235,7 +1235,21 @@ static int vhost_user_set_vring_enable(struct vhost_dev *dev, int enable)
             .num   = enable,
         };
 
-        ret = vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, &state, false);
+        /*
+         * SET_VRING_ENABLE travels from guest to QEMU to vhost-user backend /
+         * control plane thread via unix domain socket. Virtio requests travel
+         * from guest to vhost-user backend / data plane thread via eventfd.
+         * Even if the guest enables the ring first, and pushes its first virtio
+         * request second (conforming to the virtio spec), the data plane thread
+         * in the backend may see the virtio request before the control plane
+         * thread sees the queue enablement. This causes (in fact, requires) the
+         * data plane thread to discard the virtio request (it arrived on a
+         * seemingly disabled queue). To prevent this out-of-order delivery,
+         * don't let the guest proceed to pushing the virtio request until the
+         * backend control plane acknowledges enabling the queue -- IOW, pass
+         * wait_for_reply=true below.
+         */
+        ret = vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, &state, true);
         if (ret < 0) {
             /*
              * Restoring the previous state is likely infeasible, as well as

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 1/7] vhost-user: strip superfluous whitespace
  2023-10-02 20:32 ` [PATCH v3 1/7] vhost-user: strip superfluous whitespace Laszlo Ersek
@ 2023-10-04  9:06   ` Michael S. Tsirkin
  2023-10-04 10:08     ` Laszlo Ersek
  0 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2023-10-04  9:06 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: qemu-devel, Eugenio Perez Martin, German Maglione, Liu Jiang,
	Sergio Lopez Pascual, Stefano Garzarella

On Mon, Oct 02, 2023 at 10:32:15PM +0200, Laszlo Ersek wrote:
> Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:vhost)

why the (supporter:vhost) part? not all scripts will cope
well with text after the mail. If you really want to keep
it around, I think you should add a hash tag # before that -
more tools know to ignore that.


> Cc: Eugenio Perez Martin <eperezma@redhat.com>
> Cc: German Maglione <gmaglione@redhat.com>
> Cc: Liu Jiang <gerry@linux.alibaba.com>
> Cc: Sergio Lopez Pascual <slp@redhat.com>
> Cc: Stefano Garzarella <sgarzare@redhat.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Tested-by: Albert Esteve <aesteve@redhat.com>
> Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
> ---
> 
> Notes:
>     v3:
>     
>     - pick up R-b from Phil and Eugenio, T-b from Albert
>     
>     v2:
>     
>     - pick up Stefano's R-b
> 
>  hw/virtio/vhost-user.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 8dcf049d422b..b4b677c1ce66 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -398,7 +398,7 @@ static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg,
>       * operations such as configuring device memory mappings or issuing device
>       * resets, which affect the whole device instead of individual VQs,
>       * vhost-user messages should only be sent once.
> -     * 
> +     *
>       * Devices with multiple vhost_devs are given an associated dev->vq_index
>       * so per_device requests are only sent if vq_index is 0.
>       */
> 



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 1/7] vhost-user: strip superfluous whitespace
  2023-10-04  9:06   ` Michael S. Tsirkin
@ 2023-10-04 10:08     ` Laszlo Ersek
  2023-10-04 12:54       ` Michael S. Tsirkin
  0 siblings, 1 reply; 15+ messages in thread
From: Laszlo Ersek @ 2023-10-04 10:08 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: qemu-devel, Eugenio Perez Martin, German Maglione, Liu Jiang,
	Sergio Lopez Pascual, Stefano Garzarella

On 10/4/23 11:06, Michael S. Tsirkin wrote:
> On Mon, Oct 02, 2023 at 10:32:15PM +0200, Laszlo Ersek wrote:
>> Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:vhost)
> 
> why the (supporter:vhost) part? not all scripts will cope
> well with text after the mail. If you really want to keep
> it around, I think you should add a hash tag # before that -
> more tools know to ignore that.

It looked too tiresome to strip all these comments, plus I expected
that, if the get_maintainer.pl script output these lines, they were fit
for inclusion in "Cc:" tags in the commit message.

If they're not, then the tool should indeed insert a # in-between, or
else provide the explanation for each name+email printed on separate
(preceding) lines, potentially prefixed with "#". That makes for easy
human reading and also for easy machine reading (filtering them out).

Laszlo

> 
> 
>> Cc: Eugenio Perez Martin <eperezma@redhat.com>
>> Cc: German Maglione <gmaglione@redhat.com>
>> Cc: Liu Jiang <gerry@linux.alibaba.com>
>> Cc: Sergio Lopez Pascual <slp@redhat.com>
>> Cc: Stefano Garzarella <sgarzare@redhat.com>
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Tested-by: Albert Esteve <aesteve@redhat.com>
>> Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
>> ---
>>
>> Notes:
>>     v3:
>>     
>>     - pick up R-b from Phil and Eugenio, T-b from Albert
>>     
>>     v2:
>>     
>>     - pick up Stefano's R-b
>>
>>  hw/virtio/vhost-user.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>> index 8dcf049d422b..b4b677c1ce66 100644
>> --- a/hw/virtio/vhost-user.c
>> +++ b/hw/virtio/vhost-user.c
>> @@ -398,7 +398,7 @@ static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg,
>>       * operations such as configuring device memory mappings or issuing device
>>       * resets, which affect the whole device instead of individual VQs,
>>       * vhost-user messages should only be sent once.
>> -     * 
>> +     *
>>       * Devices with multiple vhost_devs are given an associated dev->vq_index
>>       * so per_device requests are only sent if vq_index is 0.
>>       */
>>
> 



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 1/7] vhost-user: strip superfluous whitespace
  2023-10-04 10:08     ` Laszlo Ersek
@ 2023-10-04 12:54       ` Michael S. Tsirkin
  2023-10-04 13:28         ` Laszlo Ersek
  0 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2023-10-04 12:54 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: qemu-devel, Eugenio Perez Martin, German Maglione, Liu Jiang,
	Sergio Lopez Pascual, Stefano Garzarella

On Wed, Oct 04, 2023 at 12:08:52PM +0200, Laszlo Ersek wrote:
> On 10/4/23 11:06, Michael S. Tsirkin wrote:
> > On Mon, Oct 02, 2023 at 10:32:15PM +0200, Laszlo Ersek wrote:
> >> Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:vhost)
> > 
> > why the (supporter:vhost) part? not all scripts will cope
> > well with text after the mail. If you really want to keep
> > it around, I think you should add a hash tag # before that -
> > more tools know to ignore that.
> 
> It looked too tiresome to strip all these comments, plus I expected
> that, if the get_maintainer.pl script output these lines, they were fit
> for inclusion in "Cc:" tags in the commit message.
> 
> If they're not, then the tool should indeed insert a # in-between, or
> else provide the explanation for each name+email printed on separate
> (preceding) lines, potentially prefixed with "#". That makes for easy
> human reading and also for easy machine reading (filtering them out).
> 
> Laszlo

/me shrugs

get_maintainer.pl doesn't output Cc tags either. Just pipe to
sed 's/(.*//' ?

> > 
> > 
> >> Cc: Eugenio Perez Martin <eperezma@redhat.com>
> >> Cc: German Maglione <gmaglione@redhat.com>
> >> Cc: Liu Jiang <gerry@linux.alibaba.com>
> >> Cc: Sergio Lopez Pascual <slp@redhat.com>
> >> Cc: Stefano Garzarella <sgarzare@redhat.com>
> >> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> >> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >> Tested-by: Albert Esteve <aesteve@redhat.com>
> >> Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
> >> ---
> >>
> >> Notes:
> >>     v3:
> >>     
> >>     - pick up R-b from Phil and Eugenio, T-b from Albert
> >>     
> >>     v2:
> >>     
> >>     - pick up Stefano's R-b
> >>
> >>  hw/virtio/vhost-user.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> >> index 8dcf049d422b..b4b677c1ce66 100644
> >> --- a/hw/virtio/vhost-user.c
> >> +++ b/hw/virtio/vhost-user.c
> >> @@ -398,7 +398,7 @@ static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg,
> >>       * operations such as configuring device memory mappings or issuing device
> >>       * resets, which affect the whole device instead of individual VQs,
> >>       * vhost-user messages should only be sent once.
> >> -     * 
> >> +     *
> >>       * Devices with multiple vhost_devs are given an associated dev->vq_index
> >>       * so per_device requests are only sent if vq_index is 0.
> >>       */
> >>
> > 



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 1/7] vhost-user: strip superfluous whitespace
  2023-10-04 12:54       ` Michael S. Tsirkin
@ 2023-10-04 13:28         ` Laszlo Ersek
  0 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2023-10-04 13:28 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: qemu-devel, Eugenio Perez Martin, German Maglione, Liu Jiang,
	Sergio Lopez Pascual, Stefano Garzarella

On 10/4/23 14:54, Michael S. Tsirkin wrote:
> On Wed, Oct 04, 2023 at 12:08:52PM +0200, Laszlo Ersek wrote:
>> On 10/4/23 11:06, Michael S. Tsirkin wrote:
>>> On Mon, Oct 02, 2023 at 10:32:15PM +0200, Laszlo Ersek wrote:
>>>> Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:vhost)
>>>
>>> why the (supporter:vhost) part? not all scripts will cope
>>> well with text after the mail. If you really want to keep
>>> it around, I think you should add a hash tag # before that -
>>> more tools know to ignore that.
>>
>> It looked too tiresome to strip all these comments, plus I expected
>> that, if the get_maintainer.pl script output these lines, they were fit
>> for inclusion in "Cc:" tags in the commit message.
>>
>> If they're not, then the tool should indeed insert a # in-between, or
>> else provide the explanation for each name+email printed on separate
>> (preceding) lines, potentially prefixed with "#". That makes for easy
>> human reading and also for easy machine reading (filtering them out).
>>
>> Laszlo
> 
> /me shrugs
> 
> get_maintainer.pl doesn't output Cc tags either. Just pipe to
> sed 's/(.*//' ?

Yes, I'll seek to remember that.

Laszlo

> 
>>>
>>>
>>>> Cc: Eugenio Perez Martin <eperezma@redhat.com>
>>>> Cc: German Maglione <gmaglione@redhat.com>
>>>> Cc: Liu Jiang <gerry@linux.alibaba.com>
>>>> Cc: Sergio Lopez Pascual <slp@redhat.com>
>>>> Cc: Stefano Garzarella <sgarzare@redhat.com>
>>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>>>> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> Tested-by: Albert Esteve <aesteve@redhat.com>
>>>> Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
>>>> ---
>>>>
>>>> Notes:
>>>>     v3:
>>>>     
>>>>     - pick up R-b from Phil and Eugenio, T-b from Albert
>>>>     
>>>>     v2:
>>>>     
>>>>     - pick up Stefano's R-b
>>>>
>>>>  hw/virtio/vhost-user.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>>>> index 8dcf049d422b..b4b677c1ce66 100644
>>>> --- a/hw/virtio/vhost-user.c
>>>> +++ b/hw/virtio/vhost-user.c
>>>> @@ -398,7 +398,7 @@ static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg,
>>>>       * operations such as configuring device memory mappings or issuing device
>>>>       * resets, which affect the whole device instead of individual VQs,
>>>>       * vhost-user messages should only be sent once.
>>>> -     * 
>>>> +     *
>>>>       * Devices with multiple vhost_devs are given an associated dev->vq_index
>>>>       * so per_device requests are only sent if vq_index is 0.
>>>>       */
>>>>
>>>
> 



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 0/7] vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously
  2023-10-02 20:32 [PATCH v3 0/7] vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously Laszlo Ersek
                   ` (6 preceding siblings ...)
  2023-10-02 20:32 ` [PATCH v3 7/7] vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously Laszlo Ersek
@ 2023-10-18 11:26 ` Laszlo Ersek
  2023-10-18 11:41   ` Stefan Hajnoczi
  2023-10-18 11:59   ` Michael S. Tsirkin
  7 siblings, 2 replies; 15+ messages in thread
From: Laszlo Ersek @ 2023-10-18 11:26 UTC (permalink / raw)
  To: Michael S. Tsirkin, Stefan Hajnoczi; +Cc: qemu devel list, Gerd Hoffmann

Hi Michael,

still waiting for you to pick this up, please.

In
<http://mid.mail-archive.com/20231004122927-mutt-send-email-mst@kernel.org>,
you wrote:

> OK. I'll need to do another PR soonish since a bunch of patchsets
> which I wanted in this PR had issues and I had to drop them.
> v3 will be there.

(Alt. link:
<https://lists.gnu.org/archive/html/qemu-devel/2023-10/msg01164.html>.)

That was on 04 Oct 2023 -- exactly two weeks ago :(

Stefan, can you perhaps apply this v3 series directly from the list?

Thanks,
Laszlo

On 10/2/23 22:32, Laszlo Ersek wrote:
> v2:
> 
> - http://mid.mail-archive.com/20230830134055.106812-1-lersek@redhat.com
> - https://patchwork.ozlabs.org/project/qemu-devel/cover/20230830134055.106812-1-lersek@redhat.com/
> 
> v3 picks up tags from Phil, Eugenio and Albert, and updates the commit
> message on patch#7 according to Eugenio's comments.
> 
> Retested.
> 
> Laszlo Ersek (7):
>   vhost-user: strip superfluous whitespace
>   vhost-user: tighten "reply_supported" scope in "set_vring_addr"
>   vhost-user: factor out "vhost_user_write_sync"
>   vhost-user: flatten "enforce_reply" into "vhost_user_write_sync"
>   vhost-user: hoist "write_sync", "get_features", "get_u64"
>   vhost-user: allow "vhost_set_vring" to wait for a reply
>   vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously
> 
>  hw/virtio/vhost-user.c | 216 ++++++++++----------
>  1 file changed, 108 insertions(+), 108 deletions(-)
> 
> 
> base-commit: 36e9aab3c569d4c9ad780473596e18479838d1aa



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 0/7] vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously
  2023-10-18 11:26 ` [PATCH v3 0/7] " Laszlo Ersek
@ 2023-10-18 11:41   ` Stefan Hajnoczi
  2023-10-18 11:59   ` Michael S. Tsirkin
  1 sibling, 0 replies; 15+ messages in thread
From: Stefan Hajnoczi @ 2023-10-18 11:41 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: Michael S. Tsirkin, Stefan Hajnoczi, qemu devel list,
	Gerd Hoffmann

On Wed, 18 Oct 2023 at 07:26, Laszlo Ersek <lersek@redhat.com> wrote:
>
> Hi Michael,
>
> still waiting for you to pick this up, please.
>
> In
> <http://mid.mail-archive.com/20231004122927-mutt-send-email-mst@kernel.org>,
> you wrote:
>
> > OK. I'll need to do another PR soonish since a bunch of patchsets
> > which I wanted in this PR had issues and I had to drop them.
> > v3 will be there.
>
> (Alt. link:
> <https://lists.gnu.org/archive/html/qemu-devel/2023-10/msg01164.html>.)
>
> That was on 04 Oct 2023 -- exactly two weeks ago :(
>
> Stefan, can you perhaps apply this v3 series directly from the list?

Michael has been active over the past few days, so I think he'll
respond and send a pull request with your patches.

Stefan

>
> Thanks,
> Laszlo
>
> On 10/2/23 22:32, Laszlo Ersek wrote:
> > v2:
> >
> > - http://mid.mail-archive.com/20230830134055.106812-1-lersek@redhat.com
> > - https://patchwork.ozlabs.org/project/qemu-devel/cover/20230830134055.106812-1-lersek@redhat.com/
> >
> > v3 picks up tags from Phil, Eugenio and Albert, and updates the commit
> > message on patch#7 according to Eugenio's comments.
> >
> > Retested.
> >
> > Laszlo Ersek (7):
> >   vhost-user: strip superfluous whitespace
> >   vhost-user: tighten "reply_supported" scope in "set_vring_addr"
> >   vhost-user: factor out "vhost_user_write_sync"
> >   vhost-user: flatten "enforce_reply" into "vhost_user_write_sync"
> >   vhost-user: hoist "write_sync", "get_features", "get_u64"
> >   vhost-user: allow "vhost_set_vring" to wait for a reply
> >   vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously
> >
> >  hw/virtio/vhost-user.c | 216 ++++++++++----------
> >  1 file changed, 108 insertions(+), 108 deletions(-)
> >
> >
> > base-commit: 36e9aab3c569d4c9ad780473596e18479838d1aa
>
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 0/7] vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously
  2023-10-18 11:26 ` [PATCH v3 0/7] " Laszlo Ersek
  2023-10-18 11:41   ` Stefan Hajnoczi
@ 2023-10-18 11:59   ` Michael S. Tsirkin
  1 sibling, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2023-10-18 11:59 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: Stefan Hajnoczi, qemu devel list, Gerd Hoffmann

On Wed, Oct 18, 2023 at 01:26:30PM +0200, Laszlo Ersek wrote:
> Hi Michael,
> 
> still waiting for you to pick this up, please.
> 
> In
> <http://mid.mail-archive.com/20231004122927-mutt-send-email-mst@kernel.org>,
> you wrote:
> 
> > OK. I'll need to do another PR soonish since a bunch of patchsets
> > which I wanted in this PR had issues and I had to drop them.
> > v3 will be there.
> 
> (Alt. link:
> <https://lists.gnu.org/archive/html/qemu-devel/2023-10/msg01164.html>.)
> 
> That was on 04 Oct 2023 -- exactly two weeks ago :(

It's been a bit wild here, sorry about the delay.
I think I'm set for now and I'm testing it.

> Stefan, can you perhaps apply this v3 series directly from the list?
> 
> Thanks,
> Laszlo
> 
> On 10/2/23 22:32, Laszlo Ersek wrote:
> > v2:
> > 
> > - http://mid.mail-archive.com/20230830134055.106812-1-lersek@redhat.com
> > - https://patchwork.ozlabs.org/project/qemu-devel/cover/20230830134055.106812-1-lersek@redhat.com/
> > 
> > v3 picks up tags from Phil, Eugenio and Albert, and updates the commit
> > message on patch#7 according to Eugenio's comments.
> > 
> > Retested.
> > 
> > Laszlo Ersek (7):
> >   vhost-user: strip superfluous whitespace
> >   vhost-user: tighten "reply_supported" scope in "set_vring_addr"
> >   vhost-user: factor out "vhost_user_write_sync"
> >   vhost-user: flatten "enforce_reply" into "vhost_user_write_sync"
> >   vhost-user: hoist "write_sync", "get_features", "get_u64"
> >   vhost-user: allow "vhost_set_vring" to wait for a reply
> >   vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously
> > 
> >  hw/virtio/vhost-user.c | 216 ++++++++++----------
> >  1 file changed, 108 insertions(+), 108 deletions(-)
> > 
> > 
> > base-commit: 36e9aab3c569d4c9ad780473596e18479838d1aa



^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-10-18 11:59 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-02 20:32 [PATCH v3 0/7] vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously Laszlo Ersek
2023-10-02 20:32 ` [PATCH v3 1/7] vhost-user: strip superfluous whitespace Laszlo Ersek
2023-10-04  9:06   ` Michael S. Tsirkin
2023-10-04 10:08     ` Laszlo Ersek
2023-10-04 12:54       ` Michael S. Tsirkin
2023-10-04 13:28         ` Laszlo Ersek
2023-10-02 20:32 ` [PATCH v3 2/7] vhost-user: tighten "reply_supported" scope in "set_vring_addr" Laszlo Ersek
2023-10-02 20:32 ` [PATCH v3 3/7] vhost-user: factor out "vhost_user_write_sync" Laszlo Ersek
2023-10-02 20:32 ` [PATCH v3 4/7] vhost-user: flatten "enforce_reply" into "vhost_user_write_sync" Laszlo Ersek
2023-10-02 20:32 ` [PATCH v3 5/7] vhost-user: hoist "write_sync", "get_features", "get_u64" Laszlo Ersek
2023-10-02 20:32 ` [PATCH v3 6/7] vhost-user: allow "vhost_set_vring" to wait for a reply Laszlo Ersek
2023-10-02 20:32 ` [PATCH v3 7/7] vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously Laszlo Ersek
2023-10-18 11:26 ` [PATCH v3 0/7] " Laszlo Ersek
2023-10-18 11:41   ` Stefan Hajnoczi
2023-10-18 11:59   ` Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).