[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 3/7] vhost-user: factor out "vhost_user_write_msg"
From: |
Laszlo Ersek |
Subject: |
Re: [PATCH 3/7] vhost-user: factor out "vhost_user_write_msg" |
Date: |
Wed, 30 Aug 2023 11:14:28 +0200 |
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!
- [PATCH 0/7] vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously, Laszlo Ersek, 2023/08/27
- [PATCH 4/7] vhost-user: flatten "enforce_reply" into "vhost_user_write_msg", Laszlo Ersek, 2023/08/27
- [PATCH 5/7] vhost-user: hoist "write_msg", "get_features", "get_u64", Laszlo Ersek, 2023/08/27
- [PATCH 6/7] vhost-user: allow "vhost_set_vring" to wait for a reply, Laszlo Ersek, 2023/08/27