Re: [PATCH 1/2] vhost-user: Change one_time to per_device request

[Raphael Norwitz]

> On Jun 28, 2023, at 5:39 PM, Tom Lonergan <tom.lonergan@nutanix.com> wrote:
> 
> Some devices, like virtio-scsi, consist of one vhost_dev, while others, like
> virtio-net, contain multiple vhost_devs. The QEMU vhost-user code has a
> concept of one-time messages which is misleading. One-time messages are sent
> once per operation on the device, not once for the lifetime of the device.
> Therefore, as discussed in [1], vhost_user_one_time_request should be
> renamed to vhost_user_per_device_request and the relevant comments updated
> to match the real functionality.
> 
> [1] 
> https://lore.kernel.org/qemu-devel/20230127083027-mutt-send-email-mst@kernel.org/
> 

Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>

> Signed-off-by: Tom Lonergan <tom.lonergan@nutanix.com>
> ---
> hw/virtio/vhost-user.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index c4e0cbd702..65d6299343 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -367,7 +367,7 @@ static int process_message_reply(struct vhost_dev *dev,
>     return msg_reply.payload.u64 ? -EIO : 0;
> }
> 
> -static bool vhost_user_one_time_request(VhostUserRequest request)
> +static bool vhost_user_per_device_request(VhostUserRequest request)
> {
>     switch (request) {
>     case VHOST_USER_SET_OWNER:
> @@ -392,11 +392,17 @@ static int vhost_user_write(struct vhost_dev *dev, 
> VhostUserMsg *msg,
>     int ret, size = VHOST_USER_HDR_SIZE + msg->hdr.size;
> 
>     /*
> -     * For non-vring specific requests, like VHOST_USER_SET_MEM_TABLE,
> -     * we just need send it once in the first time. For later such
> -     * request, we just ignore it.
> +     * Some devices, like virtio-scsi, are implemented as a single vhost_dev,
> +     * while others, like virtio-net, contain multiple vhost_devs. For
> +     * 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.
>      */
> -    if (vhost_user_one_time_request(msg->hdr.request) && dev->vq_index != 0) 
> {
> +    if (vhost_user_per_device_request(msg->hdr.request)
> +        && dev->vq_index != 0) {
>         msg->hdr.flags &= ~VHOST_USER_NEED_REPLY_MASK;
>         return 0;
>     }
> @@ -1256,7 +1262,7 @@ static int vhost_user_get_u64(struct vhost_dev *dev, 
> int request, uint64_t *u64)
>         .hdr.flags = VHOST_USER_VERSION,
>     };
> 
> -    if (vhost_user_one_time_request(request) && dev->vq_index != 0) {
> +    if (vhost_user_per_device_request(request) && dev->vq_index != 0) {
>         return 0;
>     }
> 
> -- 
> 2.22.3
>