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_msg".
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>
---
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..36f99b66a644 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_msg(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_msg(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_msg(dev, &msg, wait_for_reply);
}
static int vhost_user_set_status(struct vhost_dev *dev, uint8_t status)
On Sun, Aug 27, 2023 at 08:29:33PM +0200, Laszlo Ersek wrote: >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_msg". > >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> >--- > 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..36f99b66a644 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_msg(struct vhost_dev *dev, VhostUserMsg *msg, >+ bool wait_for_reply) The difference between vhost_user_write() and vhost_user_write_msg() is not immediately obvious from the function name, so I would propose something different, like vhost_user_write_sync() or vhost_user_write_wait(). Anyway, I'm not good with names and don't have a strong opinion, so this version is fine with me as well :-) Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
On 8/30/23 10:31, Stefano Garzarella wrote: > On Sun, Aug 27, 2023 at 08:29:33PM +0200, Laszlo Ersek wrote: >> 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_msg". >> >> 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> >> --- >> 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..36f99b66a644 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_msg(struct vhost_dev *dev, VhostUserMsg >> *msg, >> + bool wait_for_reply) > > The difference between vhost_user_write() and vhost_user_write_msg() is > not immediately obvious from the function name, so I would propose > something different, like vhost_user_write_sync() or > vhost_user_write_wait(). I'm mostly OK with either variant; I think I may have thought of _sync myself, but didn't like it because the wait would be *optional*, dependent on caller choice. And I didn't like vhost_user_write_maybe_wait() either; that one seemed awkward / too verbose. Let's see what others prefer. :) > > Anyway, I'm not good with names and don't have a strong opinion, so this > version is fine with me as well :-) > > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> > Thanks!
On 8/30/23 11:14, Laszlo Ersek wrote: > On 8/30/23 10:31, Stefano Garzarella wrote: >> On Sun, Aug 27, 2023 at 08:29:33PM +0200, Laszlo Ersek wrote: >>> 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_msg". >>> >>> 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> >>> --- >>> 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..36f99b66a644 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_msg(struct vhost_dev *dev, VhostUserMsg >>> *msg, >>> + bool wait_for_reply) >> >> The difference between vhost_user_write() and vhost_user_write_msg() is >> not immediately obvious from the function name, so I would propose >> something different, like vhost_user_write_sync() or >> vhost_user_write_wait(). > > I'm mostly OK with either variant; I think I may have thought of _sync > myself, but didn't like it because the wait would be *optional*, > dependent on caller choice. And I didn't like > vhost_user_write_maybe_wait() either; that one seemed awkward / too verbose. > > Let's see what others prefer. :) ... I went with vhost_user_write_sync. > >> >> Anyway, I'm not good with names and don't have a strong opinion, so this >> version is fine with me as well :-) >> >> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> >> > > Thanks!
On 27/8/23 20:29, Laszlo Ersek wrote: > 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_msg". > > 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> > --- > hw/virtio/vhost-user.c | 66 +++++++++----------- > 1 file changed, 28 insertions(+), 38 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
© 2016 - 2026 Red Hat, Inc.