[PATCH v2] hw/virtio: propagate vhost_virtqueue_mask() errors from start path

[PATCH v2] hw/virtio: propagate vhost_virtqueue_mask() errors from start path

[Bin Guo]

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 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 <guobin@linux.alibaba.com>
---
 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)

Re: [PATCH v2] hw/virtio: propagate vhost_virtqueue_mask() errors from start path

[Michael S. Tsirkin]

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 <guobin@linux.alibaba.com>
> ---
>  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)

Re: [PATCH v2] hw/virtio: propagate vhost_virtqueue_mask() errors from start path

[Philippe Mathieu-Daudé]

Hi,

On 16/7/26 05:33, 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 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 <guobin@linux.alibaba.com>
> ---
>   hw/virtio/vhost.c         | 9 ++++++---
>   include/hw/virtio/vhost.h | 2 +-
>   2 files changed, 7 insertions(+), 4 deletions(-)


> 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.

Please document the new returned value, i.e.:

   " * Return 0 on success, negative errno on failure."

(Good opportunity to convert to a proper docstring describing arguments)

>    */
> -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);
>   
>   /**