diff mbox series

[6/7] vhost-user: allow "vhost_set_vring" to wait for a reply

Message ID 20230827182937.146450-7-lersek@redhat.com
State New
Headers show
Series vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously | expand

Commit Message

Laszlo Ersek Aug. 27, 2023, 6:29 p.m. UTC
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_msg", 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_msg" 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_msg".

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>
---
 hw/virtio/vhost-user.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Philippe Mathieu-Daudé Aug. 28, 2023, 10:49 p.m. UTC | #1
On 27/8/23 20:29, Laszlo Ersek wrote:
> 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_msg", 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_msg" 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_msg".
> 
> 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>
> ---
>   hw/virtio/vhost-user.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Stefano Garzarella Aug. 30, 2023, 8:32 a.m. UTC | #2
On Sun, Aug 27, 2023 at 08:29:36PM +0200, Laszlo Ersek wrote:
>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_msg", 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_msg" 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_msg".
>
>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>
>---
> hw/virtio/vhost-user.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
diff mbox series

Patch

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index cadafebd0767..beb4b832245e 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1170,7 +1170,8 @@  static int vhost_user_write_msg(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_msg(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