* [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).