Re: [RFC PATCH 02/12] vhost: Move SVQ queue rewind to the destination

[Jason Wang]

On Sat, Jul 16, 2022 at 7:34 PM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> Migration with SVQ already migrate the inflight descriptors,

How is this done?

> so the
> destination can perform the work.
>
> This makes easier to migrate between backends or to recover them in
> vhost devices that support set in flight descriptors.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>  hw/virtio/vhost-vdpa.c | 24 +++++++++++-------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 18820498b3..4458c8d23e 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -1178,7 +1178,18 @@ static int vhost_vdpa_set_vring_base(struct vhost_dev 
> *dev,
>                                         struct vhost_vring_state *ring)
>  {
>      struct vhost_vdpa *v = dev->opaque;
> +    VirtQueue *vq = virtio_get_queue(dev->vdev, ring->index);
>
> +    /*
> +     * vhost-vdpa devices does not support in-flight requests. Set all of 
> them
> +     * as available.
> +     *
> +     * TODO: This is ok for networking, but other kinds of devices might
> +     * have problems with these retransmissions.
> +     */
> +    while (virtqueue_rewind(vq, 1)) {
> +        continue;
> +    }
>      if (v->shadow_vqs_enabled) {
>          /*
>           * Device vring base was set at device start. SVQ base is handled by
> @@ -1197,19 +1208,6 @@ static int vhost_vdpa_get_vring_base(struct vhost_dev 
> *dev,
>      int ret;
>
>      if (v->shadow_vqs_enabled) {
> -        VirtQueue *vq = virtio_get_queue(dev->vdev, ring->index);
> -
> -        /*
> -         * Setting base as last used idx, so destination will see as 
> available
> -         * all the entries that the device did not use, including the 
> in-flight
> -         * processing ones.
> -         *
> -         * TODO: This is ok for networking, but other kinds of devices might
> -         * have problems with these retransmissions.
> -         */
> -        while (virtqueue_rewind(vq, 1)) {
> -            continue;
> -        }

I think it's not a good idea to partially revert the code that was
just introduced in the previous patch (unless it can be backported to
-stable independently).

We probably need to squash the changes.

Thanks

>          ring->num = virtio_queue_get_last_avail_idx(dev->vdev, ring->index);
>          return 0;
>      }
> --
> 2.31.1
>