On Wed, Feb 7, 2024 at 5:27 PM Stefano Garzarella <sgarz...@redhat.com> wrote: > > vhost_vdpa_set_vring_ready() could already fail, but if Linux's > patch [1] will be merged, it may fail with more chance if > userspace does not activate virtqueues before DRIVER_OK when > VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK is not negotiated.
I wonder what happens if we just leave it as is. VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK: We do know enabling could be done after driver_ok. Without VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK: We don't know whether enabling could be done after driver_ok or not. Thanks > > So better check its return value anyway. > > [1] > https://lore.kernel.org/virtualization/20240206145154.118044-1-sgarz...@redhat.com/T/#u > > Signed-off-by: Stefano Garzarella <sgarz...@redhat.com> > --- > Note: This patch conflicts with [2], but the resolution is simple, > so for now I sent a patch for the current master, but I'll rebase > this patch if we merge the other one first. > > [2] > https://lore.kernel.org/qemu-devel/20240202132521.32714-1-kw...@redhat.com/ > --- > hw/virtio/vdpa-dev.c | 8 +++++++- > net/vhost-vdpa.c | 15 ++++++++++++--- > 2 files changed, 19 insertions(+), 4 deletions(-) > > diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c > index eb9ecea83b..d57cd76c18 100644 > --- a/hw/virtio/vdpa-dev.c > +++ b/hw/virtio/vdpa-dev.c > @@ -259,7 +259,11 @@ static int vhost_vdpa_device_start(VirtIODevice *vdev, > Error **errp) > goto err_guest_notifiers; > } > for (i = 0; i < s->dev.nvqs; ++i) { > - vhost_vdpa_set_vring_ready(&s->vdpa, i); > + ret = vhost_vdpa_set_vring_ready(&s->vdpa, i); > + if (ret < 0) { > + error_setg_errno(errp, -ret, "Error starting vring %d", i); > + goto err_dev_stop; > + } > } > s->started = true; > > @@ -274,6 +278,8 @@ static int vhost_vdpa_device_start(VirtIODevice *vdev, > Error **errp) > > return ret; > > +err_dev_stop: > + vhost_dev_stop(&s->dev, vdev, false); > err_guest_notifiers: > k->set_guest_notifiers(qbus->parent, s->dev.nvqs, false); > err_host_notifiers: > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c > index 3726ee5d67..e3d8036479 100644 > --- a/net/vhost-vdpa.c > +++ b/net/vhost-vdpa.c > @@ -381,7 +381,10 @@ static int vhost_vdpa_net_data_load(NetClientState *nc) > } > > for (int i = 0; i < v->dev->nvqs; ++i) { > - vhost_vdpa_set_vring_ready(v, i + v->dev->vq_index); > + int ret = vhost_vdpa_set_vring_ready(v, i + v->dev->vq_index); > + if (ret < 0) { > + return ret; > + } > } > return 0; > } > @@ -1213,7 +1216,10 @@ static int vhost_vdpa_net_cvq_load(NetClientState *nc) > > assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA); > > - vhost_vdpa_set_vring_ready(v, v->dev->vq_index); > + r = vhost_vdpa_set_vring_ready(v, v->dev->vq_index); > + if (unlikely(r < 0)) { > + return r; > + } > > if (v->shadow_vqs_enabled) { > n = VIRTIO_NET(v->dev->vdev); > @@ -1252,7 +1258,10 @@ static int vhost_vdpa_net_cvq_load(NetClientState *nc) > } > > for (int i = 0; i < v->dev->vq_index; ++i) { > - vhost_vdpa_set_vring_ready(v, i); > + r = vhost_vdpa_set_vring_ready(v, i); > + if (unlikely(r < 0)) { > + return r; > + } > } > > return 0; > -- > 2.43.0 >
