1. Patchew
  2. QEMU
  3. View series

[PATCH] virtio: improve virtqueue mapping error messages

Alessandro Ratti posted 1 patch 5 days, 2 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250923130034.486370-2-alessandro@0x65c.net
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>
There is a newer version of this series
hw/virtio/virtio.c | 37 ++++++++++++++++++++++++++++++++++---
1 file changed, 34 insertions(+), 3 deletions(-)
[PATCH] virtio: improve virtqueue mapping error messages
Posted by Alessandro Ratti 5 days, 2 hours ago 
Improve error reporting when virtqueue ring mapping fails by including a
device identifier in the error message.

Introduce a helper virtio_get_pretty_dev_name() that returns either:

 - the device ID, if explicitly provided (e.g. -device ...,id=foo)
 - the QOM path from qdev_get_dev_path(dev) otherwise

This makes it easier to identify which device triggered the error in
multi-device setups or when debuggin complex guest configurations.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/230
Buglink: https://bugs.launchpad.net/qemu/+bug/1919021

Signed-off-by: Alessandro Ratti <alessandro@0x65c.net>
---
 hw/virtio/virtio.c | 37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 9a81ad912e..3b3ad2e0ac 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -235,6 +235,28 @@ static void virtio_virtqueue_reset_region_cache(struct VirtQueue *vq)
     }
 }
 
+static const char *virtio_get_pretty_dev_name(VirtIODevice *vdev)
+{
+    DeviceState *dev = DEVICE(vdev);
+
+    /*
+     * Return device ID if explicity set
+     * (e.g. -device virtio-blk-pci,id=foo)
+     * This allows users to correlate errors with their custom device
+     * names.
+     */
+    if (dev->id) {
+        return dev->id;
+    }
+    /*
+     * Fall back to the canonical QOM device path (eg. ID for PCI
+     * devices).
+     * This ensures the device is still uniquely and meaningfully
+     * identified.
+     */
+    return qdev_get_dev_path(dev);
+}
+
 void virtio_init_region_cache(VirtIODevice *vdev, int n)
 {
     VirtQueue *vq = &vdev->vq[n];
@@ -256,7 +278,10 @@ void virtio_init_region_cache(VirtIODevice *vdev, int n)
     len = address_space_cache_init(&new->desc, vdev->dma_as,
                                    addr, size, packed);
     if (len < size) {
-        virtio_error(vdev, "Cannot map desc");
+        virtio_error(vdev,
+                "Failed to map descriptor ring for device %s: "
+                "invalid guest physical address or corrupted queue setup",
+                virtio_get_pretty_dev_name(vdev));
         goto err_desc;
     }
 
@@ -264,7 +289,10 @@ void virtio_init_region_cache(VirtIODevice *vdev, int n)
     len = address_space_cache_init(&new->used, vdev->dma_as,
                                    vq->vring.used, size, true);
     if (len < size) {
-        virtio_error(vdev, "Cannot map used");
+        virtio_error(vdev,
+                "Failed to map used ring for device %s: "
+                "possible guest misconfiguration or insufficient memory",
+                virtio_get_pretty_dev_name(vdev));
         goto err_used;
     }
 
@@ -272,7 +300,10 @@ void virtio_init_region_cache(VirtIODevice *vdev, int n)
     len = address_space_cache_init(&new->avail, vdev->dma_as,
                                    vq->vring.avail, size, false);
     if (len < size) {
-        virtio_error(vdev, "Cannot map avail");
+        virtio_error(vdev,
+                "Failed to map avalaible ring for device %s: "
+                "possible queue misconfiguration or overlapping memory region",
+                virtio_get_pretty_dev_name(vdev));
         goto err_avail;
     }
 
-- 
2.39.5
[PATCH v2] virtio: improve virtqueue mapping error messages
Posted by Alessandro Ratti 4 days, 23 hours ago 
Hi,

Following up on the previous discussion [1], Markus Armbruster raised an
important point:

> Note that qdev_get_dev_path() may return null.  You need another fallback.

This v2 addresses that by falling back to the string "<unknown device>" in
the virtio_get_pretty_dev_name() helper if both the device ID and QOM path
are unavailable.

Markus also noted:

> For what it's worth, "qdev ID or QOM path" is how users specify devices in QMP.

With this change, the logic for device identification now mirrors the behavior
expected in QMP tooling — prioritizing user-provided IDs, falling back to
system-generated QOM paths, and ensuring that a meaningful identifier is always
present in error messages.

Thanks for your time and consideration.

Best regards,
Alessandro Ratti

[1]: https://lore.kernel.org/qemu-devel/87y0q58f97.fsf@pond.sub.org/
[PATCH v2] virtio: improve virtqueue mapping error messages
Posted by Alessandro Ratti 4 days, 23 hours ago 
Improve error reporting when virtqueue ring mapping fails by including a
device identifier in the error message.

Introduce a helper virtio_get_pretty_dev_name() that returns either:

 - the device ID, if explicitly provided (e.g. -device ...,id=foo)
 - the QOM path from qdev_get_dev_path(dev) otherwise
 - "<unknown device>" as a fallback when no identifier is present

This makes it easier to identify which device triggered the error in
multi-device setups or when debugging complex guest configurations.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/230
Buglink: https://bugs.launchpad.net/qemu/+bug/1919021

Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alessandro Ratti <alessandro@0x65c.net>
---
 hw/virtio/virtio.c | 46 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 9a81ad912e..f5adc381a4 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -235,6 +235,37 @@ static void virtio_virtqueue_reset_region_cache(struct VirtQueue *vq)
     }
 }
 
+static const char *virtio_get_pretty_dev_name(VirtIODevice *vdev)
+{
+    DeviceState *dev = DEVICE(vdev);
+
+    /*
+     * Return device ID if explicity set
+     * (e.g. -device virtio-blk-pci,id=foo)
+     * This allows users to correlate errors with their custom device
+     * names.
+     */
+    if (dev->id) {
+        return dev->id;
+    }
+    /*
+     * Fall back to the canonical QOM device path (eg. ID for PCI
+     * devices).
+     * This ensures the device is still uniquely and meaningfully
+     * identified.
+     */
+    const char *path = qdev_get_dev_path(dev);
+    if (path) {
+        return path;
+    }
+
+    /*
+     * Final fallback: if all else fails, return a placeholder string.
+     * This ensures the error message always contains a valid string.
+     */
+    return "<unknow device>";
+}
+
 void virtio_init_region_cache(VirtIODevice *vdev, int n)
 {
     VirtQueue *vq = &vdev->vq[n];
@@ -256,7 +287,10 @@ void virtio_init_region_cache(VirtIODevice *vdev, int n)
     len = address_space_cache_init(&new->desc, vdev->dma_as,
                                    addr, size, packed);
     if (len < size) {
-        virtio_error(vdev, "Cannot map desc");
+        virtio_error(vdev,
+                "Failed to map descriptor ring for device %s: "
+                "invalid guest physical address or corrupted queue setup",
+                virtio_get_pretty_dev_name(vdev));
         goto err_desc;
     }
 
@@ -264,7 +298,10 @@ void virtio_init_region_cache(VirtIODevice *vdev, int n)
     len = address_space_cache_init(&new->used, vdev->dma_as,
                                    vq->vring.used, size, true);
     if (len < size) {
-        virtio_error(vdev, "Cannot map used");
+        virtio_error(vdev,
+                "Failed to map used ring for device %s: "
+                "possible guest misconfiguration or insufficient memory",
+                virtio_get_pretty_dev_name(vdev));
         goto err_used;
     }
 
@@ -272,7 +309,10 @@ void virtio_init_region_cache(VirtIODevice *vdev, int n)
     len = address_space_cache_init(&new->avail, vdev->dma_as,
                                    vq->vring.avail, size, false);
     if (len < size) {
-        virtio_error(vdev, "Cannot map avail");
+        virtio_error(vdev,
+                "Failed to map avalaible ring for device %s: "
+                "possible queue misconfiguration or overlapping memory region",
+                virtio_get_pretty_dev_name(vdev));
         goto err_avail;
     }
 
-- 
2.39.5
Re: [PATCH v2] virtio: improve virtqueue mapping error messages
Posted by Daniel P. Berrangé 4 days, 22 hours ago 
On Tue, Sep 23, 2025 at 05:09:25PM +0200, Alessandro Ratti wrote:
> Improve error reporting when virtqueue ring mapping fails by including a
> device identifier in the error message.
> 
> Introduce a helper virtio_get_pretty_dev_name() that returns either:
> 
>  - the device ID, if explicitly provided (e.g. -device ...,id=foo)
>  - the QOM path from qdev_get_dev_path(dev) otherwise
>  - "<unknown device>" as a fallback when no identifier is present
> 
> This makes it easier to identify which device triggered the error in
> multi-device setups or when debugging complex guest configurations.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/230
> Buglink: https://bugs.launchpad.net/qemu/+bug/1919021
> 
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Alessandro Ratti <alessandro@0x65c.net>
> ---
>  hw/virtio/virtio.c | 46 +++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 43 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 9a81ad912e..f5adc381a4 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -235,6 +235,37 @@ static void virtio_virtqueue_reset_region_cache(struct VirtQueue *vq)
>      }
>  }
>  
> +static const char *virtio_get_pretty_dev_name(VirtIODevice *vdev)

I'd suggest this be  'const char *qdev_get_printable_name(DeviceState *dev)'
and live in  the same header & source files as qdev_get_dev_path.

I used 'printable' rather than 'pretty' as I'm not sure I'd claim
that qdev_get_dev_path() results can be said to be pretty :-)

> +{
> +    DeviceState *dev = DEVICE(vdev);
> +
> +    /*
> +     * Return device ID if explicity set
> +     * (e.g. -device virtio-blk-pci,id=foo)
> +     * This allows users to correlate errors with their custom device
> +     * names.
> +     */
> +    if (dev->id) {
> +        return dev->id;
> +    }
> +    /*
> +     * Fall back to the canonical QOM device path (eg. ID for PCI
> +     * devices).
> +     * This ensures the device is still uniquely and meaningfully
> +     * identified.
> +     */
> +    const char *path = qdev_get_dev_path(dev);
> +    if (path) {
> +        return path;
> +    }
> +
> +    /*
> +     * Final fallback: if all else fails, return a placeholder string.
> +     * This ensures the error message always contains a valid string.
> +     */
> +    return "<unknow device>";

s/unknow/unknown/

> +}
> +
>  void virtio_init_region_cache(VirtIODevice *vdev, int n)
>  {
>      VirtQueue *vq = &vdev->vq[n];
> @@ -256,7 +287,10 @@ void virtio_init_region_cache(VirtIODevice *vdev, int n)
>      len = address_space_cache_init(&new->desc, vdev->dma_as,
>                                     addr, size, packed);
>      if (len < size) {
> -        virtio_error(vdev, "Cannot map desc");
> +        virtio_error(vdev,
> +                "Failed to map descriptor ring for device %s: "
> +                "invalid guest physical address or corrupted queue setup",
> +                virtio_get_pretty_dev_name(vdev));
>          goto err_desc;
>      }
>  
> @@ -264,7 +298,10 @@ void virtio_init_region_cache(VirtIODevice *vdev, int n)
>      len = address_space_cache_init(&new->used, vdev->dma_as,
>                                     vq->vring.used, size, true);
>      if (len < size) {
> -        virtio_error(vdev, "Cannot map used");
> +        virtio_error(vdev,
> +                "Failed to map used ring for device %s: "
> +                "possible guest misconfiguration or insufficient memory",
> +                virtio_get_pretty_dev_name(vdev));
>          goto err_used;
>      }
>  
> @@ -272,7 +309,10 @@ void virtio_init_region_cache(VirtIODevice *vdev, int n)
>      len = address_space_cache_init(&new->avail, vdev->dma_as,
>                                     vq->vring.avail, size, false);
>      if (len < size) {
> -        virtio_error(vdev, "Cannot map avail");
> +        virtio_error(vdev,
> +                "Failed to map avalaible ring for device %s: "
> +                "possible queue misconfiguration or overlapping memory region",
> +                virtio_get_pretty_dev_name(vdev));
>          goto err_avail;
>      }

This part all looks good


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
[PATCH v3] virtio: improve virtqueue mapping error messages
Posted by Alessandro Ratti 4 days, 5 hours ago 
On Tue, 23 Sept 2025 at 18:17, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
[...]
> >  hw/virtio/virtio.c | 46 +++++++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 43 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index 9a81ad912e..f5adc381a4 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -235,6 +235,37 @@ static void virtio_virtqueue_reset_region_cache(struct VirtQueue *vq)
> >      }
> >  }
> >
> > +static const char *virtio_get_pretty_dev_name(VirtIODevice *vdev)
>
> I'd suggest this be  'const char *qdev_get_printable_name(DeviceState *dev)'
> and live in  the same header & source files as qdev_get_dev_path.
>
> I used 'printable' rather than 'pretty' as I'm not sure I'd claim
> that qdev_get_dev_path() results can be said to be pretty :-)

Thanks for the review and the suggestion.

Fair enough :) — I've renamed the helper to qdev_get_printable_name() and moved
it next to qdev_get_dev_path() in `hw/core/qdev.c`, as you recommended.

[...]
> > +    /*
> > +     * Final fallback: if all else fails, return a placeholder string.
> > +     * This ensures the error message always contains a valid string.
> > +     */
> > +    return "<unknow device>";
>
> s/unknow/unknown/
>

Fixed, thanks for catching that!

[...]
>
> This part all looks good

Glad to hear! I've sent out v3 with the changes above. Let me know if you
have any further thoughts or improvements in mind.


Thanks again for your time and helpful reviews.

Best regards,
Alessandro

---

Changes since v2:
- Renamed helper to qdev_get_printable_name()
- Moved helper to appropriate source/header location
- Fixed typo in fallback string
- Incorporated review feedback from Daniel Barrangé
[PATCH v3] virtio: improve virtqueue mapping error messages
Posted by Alessandro Ratti 4 days, 5 hours ago 
Improve error reporting when virtqueue ring mapping fails by including a
device identifier in the error message.

Introduce a helper qdev_get_printable_name() in qdev-core, which returns
either:

 - the device ID, if explicitly provided (e.g. -device ...,id=foo)
 - the QOM path from qdev_get_dev_path(dev) otherwise
 - "<unknown device>" as a fallback when no identifier is present

This makes it easier to identify which device triggered the error in
multi-device setups or when debugging complex guest configurations.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/230
Buglink: https://bugs.launchpad.net/qemu/+bug/1919021

Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alessandro Ratti <alessandro@0x65c.net>
---
 hw/core/qdev.c         | 29 +++++++++++++++++++++++++++++
 hw/virtio/virtio.c     | 15 ++++++++++++---
 include/hw/qdev-core.h |  1 +
 3 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index f600226176..fab42a7270 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -411,6 +411,35 @@ char *qdev_get_dev_path(DeviceState *dev)
     return NULL;
 }
 
+const char *qdev_get_printable_name(DeviceState *vdev)
+{
+    /*
+     * Return device ID if explicity set
+     * (e.g. -device virtio-blk-pci,id=foo)
+     * This allows users to correlate errors with their custom device
+     * names.
+     */
+    if (vdev->id) {
+        return vdev->id;
+    }
+    /*
+     * Fall back to the canonical QOM device path (eg. ID for PCI
+     * devices).
+     * This ensures the device is still uniquely and meaningfully
+     * identified.
+     */
+    const char *path = qdev_get_dev_path(vdev);
+    if (path) {
+        return path;
+    }
+
+    /*
+     * Final fallback: if all else fails, return a placeholder string.
+     * This ensures the error message always contains a valid string.
+     */
+    return "<unknown device>";
+}
+
 void qdev_add_unplug_blocker(DeviceState *dev, Error *reason)
 {
     dev->unplug_blockers = g_slist_prepend(dev->unplug_blockers, reason);
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 9a81ad912e..0caea5b8ce 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -256,7 +256,10 @@ void virtio_init_region_cache(VirtIODevice *vdev, int n)
     len = address_space_cache_init(&new->desc, vdev->dma_as,
                                    addr, size, packed);
     if (len < size) {
-        virtio_error(vdev, "Cannot map desc");
+        virtio_error(vdev,
+                "Failed to map descriptor ring for device %s: "
+                "invalid guest physical address or corrupted queue setup",
+                qdev_get_printable_name(DEVICE(vdev)));
         goto err_desc;
     }
 
@@ -264,7 +267,10 @@ void virtio_init_region_cache(VirtIODevice *vdev, int n)
     len = address_space_cache_init(&new->used, vdev->dma_as,
                                    vq->vring.used, size, true);
     if (len < size) {
-        virtio_error(vdev, "Cannot map used");
+        virtio_error(vdev,
+                "Failed to map used ring for device %s: "
+                "possible guest misconfiguration or insufficient memory",
+                qdev_get_printable_name(DEVICE(vdev)));
         goto err_used;
     }
 
@@ -272,7 +278,10 @@ void virtio_init_region_cache(VirtIODevice *vdev, int n)
     len = address_space_cache_init(&new->avail, vdev->dma_as,
                                    vq->vring.avail, size, false);
     if (len < size) {
-        virtio_error(vdev, "Cannot map avail");
+        virtio_error(vdev,
+                "Failed to map avalaible ring for device %s: "
+                "possible queue misconfiguration or overlapping memory region",
+                qdev_get_printable_name(DEVICE(vdev)));
         goto err_avail;
     }
 
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 530f3da702..a7bfb10dc7 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -1064,6 +1064,7 @@ bool qdev_set_parent_bus(DeviceState *dev, BusState *bus, Error **errp);
 extern bool qdev_hot_removed;
 
 char *qdev_get_dev_path(DeviceState *dev);
+const char *qdev_get_printable_name(DeviceState *dev);
 
 void qbus_set_hotplug_handler(BusState *bus, Object *handler);
 void qbus_set_bus_hotplug_handler(BusState *bus);
-- 
2.39.5
Re: [PATCH v3] virtio: improve virtqueue mapping error messages
Posted by Daniel P. Berrangé 4 days, 5 hours ago 
On Wed, Sep 24, 2025 at 11:14:04AM +0200, Alessandro Ratti wrote:
> Improve error reporting when virtqueue ring mapping fails by including a
> device identifier in the error message.
> 
> Introduce a helper qdev_get_printable_name() in qdev-core, which returns
> either:
> 
>  - the device ID, if explicitly provided (e.g. -device ...,id=foo)
>  - the QOM path from qdev_get_dev_path(dev) otherwise
>  - "<unknown device>" as a fallback when no identifier is present
> 
> This makes it easier to identify which device triggered the error in
> multi-device setups or when debugging complex guest configurations.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/230
> Buglink: https://bugs.launchpad.net/qemu/+bug/1919021
> 
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Alessandro Ratti <alessandro@0x65c.net>
> ---
>  hw/core/qdev.c         | 29 +++++++++++++++++++++++++++++
>  hw/virtio/virtio.c     | 15 ++++++++++++---
>  include/hw/qdev-core.h |  1 +
>  3 files changed, 42 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.