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

"Michael S. Tsirkin" <[email protected]>
Newsgroups gmane.comp.emulators.qemu
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)
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.