Re: [PATCH] vdpa: dont check vhost_vdpa->suspended when unsupported

[Jason Wang]

On Wed, Jun 7, 2023 at 11:37 PM Eugenio Perez Martin
<eperezma@redhat.com> wrote:
>
> On Wed, Jun 7, 2023 at 11:09 AM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
> >
> > When read the state of a virtqueue, vhost_vdpa need
> > to check whether the device is suspended.
> >
> > This commit verifies whether VHOST_BACKEND_F_SUSPEND is
> > negotiated when checking vhost_vdpa->suspended.
> >
>
> I'll add: Otherwise, qemu prints XXX error message.
>
> On second thought, not returning an error prevents the caller
> (vhost.c:vhost_virtqueue_stop) from recovering used idx from the
> guest.
>
> I'm not sure about the right fix for this, should we call
> virtio_queue_restore_last_avail_idx here?

It should be the duty of the caller:

E.g in vhost_virtqueue_stop() we had:

=>  r = dev->vhost_ops->vhost_get_vring_base(dev, &state);
    if (r < 0) {
        VHOST_OPS_DEBUG(r, "vhost VQ %u ring restore failed: %d", idx, r);
        /* Connection to the backend is broken, so let's sync internal
         * last avail idx to the device used idx.
         */
virtio_queue_restore_last_avail_idx(vdev, idx);
    } else {
        virtio_queue_set_last_avail_idx(vdev, idx, state.num);

Thansk

> Communicate that the backend
> cannot suspend so vhost.c can print a better error message?
>
> Thanks!
>
> > Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> > ---
> >  hw/virtio/vhost-vdpa.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> > index b3094e8a8b..ae176c06dd 100644
> > --- a/hw/virtio/vhost-vdpa.c
> > +++ b/hw/virtio/vhost-vdpa.c
> > @@ -1397,7 +1397,7 @@ static int vhost_vdpa_get_vring_base(struct vhost_dev 
> > *dev,
> >          return 0;
> >      }
> >
> > -    if (!v->suspended) {
> > +    if ((dev->backend_cap & BIT_ULL(VHOST_BACKEND_F_SUSPEND)) && 
> > (!v->suspended)) {
> >          /*
> >           * Cannot trust in value returned by device, let vhost recover used
> >           * idx from guest.
> > --
> > 2.39.1
> >
>