Re: [PATCH v2 16/24] vhost: vhost-user: update vhost_dev_virtqueue_stop()

[Jason Wang]

在 2022/8/16 09:06, Kangjie Xu 写道:
> Update vhost_dev_virtqueue_stop() for vhost-user scenario.
>
> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
>   hw/virtio/vhost.c | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index fc3f550c76..a0d6824353 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1908,10 +1908,29 @@ int vhost_net_set_backend(struct vhost_dev *hdev,
>   void vhost_dev_virtqueue_stop(struct vhost_dev *hdev, VirtIODevice *vdev,
>                                 int idx)
>   {
> +    const VhostOps *vhost_ops = hdev->vhost_ops;
> +    struct vhost_vring_state state;
> +    int r;
> +
> +    assert(vhost_ops);
> +
> +    if (vhost_ops->vhost_reset_vring) {
> +        state.index = hdev->vq_index + idx;
> +        r = vhost_ops->vhost_reset_vring(hdev, &state);
> +        if (r < 0) {
> +            goto err_queue_reset;
> +        }
> +    }
> +


So this worries me:

1) having two similar functions

vhost_virtqueue_stop()

and

vhost_dev_virtqueue_stop()

It can easily confuse the people who want stop the device.

I think we need rename vhost_dev_virtqueue_stop() to 
vhost_virqtueue_reset() since it has different semantic compared to stop:

1) stop means the virtqueue state is reserved, e.g the index could be 
synced via get_vring_base()
2) reset means the virqtueue state is lost

Thanks


>       vhost_virtqueue_unmap(hdev,
>                             vdev,
>                             hdev->vqs + idx,
>                             idx);
> +
> +    return;
> +
> +err_queue_reset:
> +    error_report("Error when stopping the qeuue.");
>   }
>   
>   int vhost_dev_virtqueue_restart(struct vhost_dev *hdev, VirtIODevice *vdev,