Re: [PATCH v2] hw/virtio: propagate vhost_virtqueue_mask() errors from start path
"Michael S. Tsirkin" <[email protected]> Sun, 2 Aug 2026 14:42:08 -0400
| Newsgroups | org.nongnu.qemu-devel |
|---|---|
| Message-ID | <[email protected]> |
On Thu, Jul 16, 2026 at 11:33:03AM +0800, Bin Guo wrote: > vhost_virtqueue_start() unmasks the call notifier by calling > vhost_virtqueue_mask(), whose vhost_set_vring_call ioctl can fail > (closed vhost-user socket, kernel ENOMEM, revoked guest_notifier fd) > but whose void signature throws the error away. vhost_dev_start() > then reports success while the backend has no valid call eventfd for > that vq, leaving the guest with a working kick path but no virtqueue > interrupts -- a half-up state harder to diagnose than a clean failure. Is there a real problem though? So what if socket closed one second after we sent the fd, does it matter? And what does it mean for a guest_notifier fd to be revoked? Is qemu likely to survive long after we started getting ENOMEM for allocations of a hundred of bytes from the kernel? I do not object to the patch on principle but let's get it clear how it was tested, what is being fixed and why? > Make vhost_virtqueue_mask() return int and handle the error in the > start path via the existing fail unwind. Other callers reach the > function through VirtioDeviceClass.guest_notifier_mask, whose void > signature offers no upward error channel; they invoke it as a > statement and remain unchanged. > > Signed-off-by: Bin Guo <[email protected]> > --- > hw/virtio/vhost.c | 9 ++++++--- > include/hw/virtio/vhost.h | 2 +- > 2 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > index af41841b52..ad22c37c33 100644 > --- a/hw/virtio/vhost.c > +++ b/hw/virtio/vhost.c > @@ -1470,8 +1470,10 @@ int vhost_virtqueue_start(struct vhost_dev *dev, > * will do it later. > */ > if (!vdev->use_guest_notifier_mask) { > - /* TODO: check and handle errors. */ > - vhost_virtqueue_mask(dev, vdev, idx, false); > + r = vhost_virtqueue_mask(dev, vdev, idx, false); > + if (r < 0) { > + goto fail; > + } > } > > if (k->query_guest_notifiers && > @@ -1918,7 +1920,7 @@ bool vhost_virtqueue_pending(struct vhost_dev *hdev, int n) > } > > /* Mask/unmask events from this vq. */ > -void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n, > +int vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n, > bool mask) > { > struct VirtQueue *vvq = virtio_get_queue(vdev, n); > @@ -1940,6 +1942,7 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n, > if (r < 0) { > error_report("vhost_set_vring_call failed %d", -r); > } > + return r; > } > > bool vhost_config_pending(struct vhost_dev *hdev) > diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h > index 684bafcaad..1f62332d60 100644 > --- a/include/hw/virtio/vhost.h > +++ b/include/hw/virtio/vhost.h > @@ -312,7 +312,7 @@ bool vhost_virtqueue_pending(struct vhost_dev *hdev, int n); > > /* Mask/unmask events from this vq. > */ > -void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n, > +int vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n, > bool mask); > > /** > -- > 2.50.1 (Apple Git-155)