From: "Michael S. Tsirkin" <mst@redhat.com>
To: Bin Guo <guobin@linux.alibaba.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [PATCH v2] hw/virtio: propagate vhost_virtqueue_mask() errors from start path
Date: Sun, 2 Aug 2026 14:42:08 -0400 [thread overview]
Message-ID: <20260802143439-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20260716033303.95963-1-guobin@linux.alibaba.com>
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)
next prev parent reply other threads:[~2026-08-02 18:43 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-16 3:33 [PATCH v2] hw/virtio: propagate vhost_virtqueue_mask() errors from start path Bin Guo
2026-08-02 18:42 ` Michael S. Tsirkin [this message]
2026-08-03 7:09 ` Philippe Mathieu-Daudé
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260802143439-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=guobin@linux.alibaba.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.