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.

Make vhost_virtqueue_mask() return int and handle the error in the
start path via the existing fail_vector 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 source-compatible.

Resolves a long-standing TODO at the unmask call.

Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
---
 hw/virtio/vhost.c         | 12 +++++++++---
 include/hw/virtio/vhost.h |  4 ++--
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index b9dc4ed13b..42e2fc9952 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1351,8 +1351,11 @@ 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) {
+            /* No call eventfd means no interrupts; bail out. */
+            goto fail_vector;
+        }
     }
 
     if (k->query_guest_notifiers &&
@@ -1815,7 +1818,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);
@@ -1836,7 +1839,10 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n,
     r = hdev->vhost_ops->vhost_set_vring_call(hdev, &file);
     if (r < 0) {
         error_report("vhost_set_vring_call failed %d", -r);
+        return r;
     }
+
+    return 0;
 }
 
 bool vhost_config_pending(struct vhost_dev *hdev)
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 89817bd848..96cc261822 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -317,8 +317,8 @@ 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,
-                          bool mask);
+int vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n,
+                         bool mask);
 
 /**
  * vhost_get_features_ex() - sanitize the extended features set
-- 
2.50.1 (Apple Git-155)

Hi,

On 4/6/26 14:44, 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.
> 
> Make vhost_virtqueue_mask() return int and handle the error in the
> start path via the existing fail_vector 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 source-compatible.
> 
> Resolves a long-standing TODO at the unmask call.
> 
> Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
> ---
>   hw/virtio/vhost.c         | 12 +++++++++---
>   include/hw/virtio/vhost.h |  4 ++--
>   2 files changed, 11 insertions(+), 5 deletions(-)


>   /* 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);
> @@ -1836,7 +1839,10 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n,
>       r = hdev->vhost_ops->vhost_set_vring_call(hdev, &file);
>       if (r < 0) {
>           error_report("vhost_set_vring_call failed %d", -r);

IIUC the intention behind this TODO is to convert to "a function
returning a boolean and taking Error** as last argument", replacing
this call with error_setg_errno().

See in "Error reporting system loosely patterned after Glib's GError"
rules in "qapi/error.h".

> +        return r;
>       }
> +
> +    return 0;
>   }

On Wed, 4 Jun 2026 14:57:41 +0200, Philippe Mathieu-Daudé wrote:
> IIUC the intention behind this TODO is to convert to a function
> returning a boolean and taking Error** as last argument, replacing
> this call with error_setg_errno().
> See in 'Error reporting system loosely patterned after Glib's GError'
> rules in 'qapi/error.h'.

Thanks for the suggestion. I considered changing the signature to
bool + Error **errp, but decided to keep the current int +
error_report() approach. vhost_set_vring_call is unlikely to fail
in practice, so this is defensive programming rather than a regular
error path. Changing the signature would propagate to ~15 call sites
across multiple files for what is a rare edge case, which seems
disproportionate. The patch already resolves the TODO and confines
the change to the start path.