[2/7] vhost-user: tighten "reply_supported" scope in "set_vring_addr"

Commit Message

Laszlo Ersek Aug. 27, 2023, 6:29 p.m. UTC

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" <[e-mail]> (supporter:vhost)
Cc: Eugenio Perez Martin <[e-mail]>
Cc: German Maglione <[e-mail]>
Cc: Liu Jiang <[e-mail]>
Cc: Sergio Lopez Pascual <[e-mail]>
Cc: Stefano Garzarella <[e-mail]>
Signed-off-by: Laszlo Ersek <[e-mail]>
---
 hw/virtio/vhost-user.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Stefano Garzarella Aug. 30, 2023, 8:27 a.m. UTC | #1

On Sun, Aug 27, 2023 at 08:29:32PM +0200, Laszlo Ersek wrote:
>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" <[e-mail]> (supporter:vhost)
>Cc: Eugenio Perez Martin <[e-mail]>
>Cc: German Maglione <[e-mail]>
>Cc: Liu Jiang <[e-mail]>
>Cc: Sergio Lopez Pascual <[e-mail]>
>Cc: Stefano Garzarella <[e-mail]>
>Signed-off-by: Laszlo Ersek <[e-mail]>
>---
> hw/virtio/vhost-user.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)

Reviewed-by: Stefano Garzarella <[e-mail]>

Philippe Mathieu-Daudé Aug. 30, 2023, 3:04 p.m. UTC | #2

On 27/8/23 20:29, Laszlo Ersek wrote:
> 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" <[e-mail]> (supporter:vhost)
> Cc: Eugenio Perez Martin <[e-mail]>
> Cc: German Maglione <[e-mail]>
> Cc: Liu Jiang <[e-mail]>
> Cc: Sergio Lopez Pascual <[e-mail]>
> Cc: Stefano Garzarella <[e-mail]>
> Signed-off-by: Laszlo Ersek <[e-mail]>
> ---
>   hw/virtio/vhost-user.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <[e-mail]>

Patch

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