[PATCH] virtio: improve virtqueue mapping error messages

[PATCH] virtio: improve virtqueue mapping error messages

[Alessandro Ratti]

Hi all,

Following up on the previous discussion in this thread [1], I'm submitting a
cleaned-up version of the patch to improve error messages in
virtio_init_region_cache() when virtqueue ring mapping fails.

This version introduces a helper (virtio_get_pretty_dev_name()) that returns
either:

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

This improves the clarity of error messages, especially when debugging
misconfigured guests or setups with multiple virtio devices.

## Example 1: PCI device (fallback to QOM path returns PCI ID)

~/qemu/build$ ./qemu-system-x86_64 \ 
    -enable-kvm \ 
    -m 512 \ 
    -drive if=virtio,file=/tmp/some.img,format=raw
qemu-system-x86_64: Failed to map descriptor ring for device 0000:00:04.0: invalid guest physical address or corrupted queue setup

## Example 2: Non-PCI virtio-mmio device (QOM path fallback)

~/qemu/build$ ./qemu-system-aarch64 -M virt -cpu cortex-a57 -m 512 -nographic \
    -append "console=ttyAMA0" \
    -drive if=none,file=/tmp/some.img,format=raw,id=hd0 \
    -device virtio-blk-device,drive=hd0 \
    -kernel /tmp/vmlinuz-5.15.0-qcomlt-arm64

qemu-system-aarch64: Failed to map descriptor ring for device virtio-mmio@000000000a003e00: invalid guest physical address or corrupted queue setup\ 

## Example 3: Explicit device ID (dev->id path)

~/qemu/build$ ./qemu-system-aarch64 -M virt -cpu cortex-a57 -m 512 -nographic \
    -append "console=ttyAMA0" \
    -drive if=none,file=/tmp/some.img,format=raw,id=hd0 \
    -device virtio-blk-device,drive=hd0,id=foo \
    -kernel /tmp/vmlinuz-5.15.0-qcomlt-arm64

qemu-system-aarch64: Failed to map available ring for device foo: possible queue misconfiguration or overlapping memory region

As discussed, I considered using qdev_get_human_name() but it often returns
opaque paths like 

   /machine/peripheral-anon/device[0]/virtio-backend 

Which are less informative in user-facing logs compared to PCI IDs or
user-specified names.

The messages have been adjusted to be more descriptive and scoped to improving
this specific area of diagnostics. I hope this aligns with the direction
discussed — but of course, I’m open to further suggestions if something still
feels off.

Thank you for your time and consideration.

Best regards,
Alessandro Ratti

[1]: https://lore.kernel.org/qemu-devel/20250915100701.224156-1-alessandro@0x65c.net/

[PATCH] virtio: improve virtqueue mapping error messages

[Alessandro Ratti]

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

[Alessandro Ratti]

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

[Alessandro Ratti]

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

[Daniel P. Berrangé]

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

[Alessandro Ratti]

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

[Alessandro Ratti]

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

[Daniel P. Berrangé]

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

Re: [PATCH v3] virtio: improve virtqueue mapping error messages

[Peter Maydell]

On Wed, 24 Sept 2025 at 10:37, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> 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>

Hi; I just found this commit (now e209d4d7a31b9) in the course
of finding a memory leak in it. I'm about to send patches to fix
that, but in the meantime:

We now have two different functions for "give me a nice string
that I can use in an error message about this device":

 * qdev_get_human_name(), used only in hw/block/block.c
 * qdev_get_printable_name(), used only in hw/virtio/virtio.c

Can we please have *one* function for this purpose?

I see from the thread that the criticism of qdev_get_human_name()
is that "it often returns opaque paths like

   /machine/peripheral-anon/device[0]/virtio-backend

which are less informative in user-facing logs compared to PCI IDs or
user-specified names".

So could we instead make block.c use qdev_get_printable_name(), and
remove qdev_get_human_name() ? It also is using the result only
to construct an error string for the user.

thanks
-- PMM

Re: [PATCH v3] virtio: improve virtqueue mapping error messages

[Alessandro Ratti]

On Sat, 7 Mar 2026 at 14:39, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Wed, 24 Sept 2025 at 10:37, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > 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>
>
> Hi; I just found this commit (now e209d4d7a31b9) in the course
> of finding a memory leak in it. I'm about to send patches to fix
> that, but in the meantime:
>
> We now have two different functions for "give me a nice string
> that I can use in an error message about this device":
>
>  * qdev_get_human_name(), used only in hw/block/block.c
>  * qdev_get_printable_name(), used only in hw/virtio/virtio.c
>
> Can we please have *one* function for this purpose?
>
> I see from the thread that the criticism of qdev_get_human_name()
> is that "it often returns opaque paths like
>
>    /machine/peripheral-anon/device[0]/virtio-backend
>
> which are less informative in user-facing logs compared to PCI IDs or
> user-specified names".
>
> So could we instead make block.c use qdev_get_printable_name(), and
> remove qdev_get_human_name() ? It also is using the result only
> to construct an error string for the user.
>

Hi Peter,

Thank you for catching the memory leak and for the feedback on the API design.

You're right that having two similar functions is confusing and unnecessarily
duplicates functionality. I agree that consolidating to a single function makes
sense.

I can send a follow-up patch to:

1. Replace the usage of qdev_get_human_name() in hw/block/block.c with
   qdev_get_printable_name()
2. Remove qdev_get_human_name() entirely
3. Fix the memory leak in qdev_get_printable_name() (unless you already have
   a patch ready to send)

Alternatively, if you prefer to handle the consolidation as part of your
memory leak fix series, please feel free to do so - just let me know.

Thank you for your time and for improving the code.

Best regards,
Alessandro

Re: [PATCH v3] virtio: improve virtqueue mapping error messages

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Wed, 24 Sept 2025 at 10:37, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>
>> 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>
>
> Hi; I just found this commit (now e209d4d7a31b9) in the course
> of finding a memory leak in it. I'm about to send patches to fix
> that, but in the meantime:
>
> We now have two different functions for "give me a nice string
> that I can use in an error message about this device":
>
>  * qdev_get_human_name(), used only in hw/block/block.c
>  * qdev_get_printable_name(), used only in hw/virtio/virtio.c
>
> Can we please have *one* function for this purpose?

Sensible request.

> I see from the thread that the criticism of qdev_get_human_name()
> is that "it often returns opaque paths like
>
>    /machine/peripheral-anon/device[0]/virtio-backend
>
> which are less informative in user-facing logs compared to PCI IDs or
> user-specified names".
>
> So could we instead make block.c use qdev_get_printable_name(), and
> remove qdev_get_human_name() ? It also is using the result only
> to construct an error string for the user.

No, not without significant improvements to qdev_get_printable_name().

Let's have a closer look.


First qdev_get_human_name():

    char *qdev_get_human_name(DeviceState *dev)
    {
        g_assert(dev != NULL);

        return dev->id ?
               g_strdup(dev->id) : object_get_canonical_path(OBJECT(dev));
    }

This returns the device's ID if it has one, else its canonical QOM path.

Intepreting the value is easy enough: if it starts with '/', it's a
canonical QOM path, else a qdev ID.

Devices plugged by the user have an ID if the user specified one.  If
they don't have one, the canonical path will be
/machine/peripheral-anon/device[N], where N is a non-negative integer.
Gibberish, but easy enough to avoid: specify an ID.  Management
applications should do that always.

Onboard devices do not have an ID.  The return value could be something
relatively reasonable like /machine/soc/cpu, or gibberish like
/machine/unattached/device[N].


Next qdev_get_printable_name():

    const char *qdev_get_printable_name(DeviceState *vdev)

Nitpick: @vdev is an unusual identifier.  We normally use @dev around
here.

    {
        /*
         * 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;
        }

Same as qdev_get_human_name() so far.

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

This returns a "name" in a bus-specific format if the device is plugged
into a bus that provides the get_dev_path() method.  Many buses do.

For instance, the PCI bus's method is cibus_get_dev_path().  It appears
to return a path of the form

    Domain:00:Slot.Function:Slot.Function....:Slot.Function.

which is commonly just a PCI address.

The comment is nonsense: this has nothing to do with "the canonical QOM
device path".

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

This fallback is is unnecessarily bad.  The canonical QOM path always
exists and would be far better.

    }

Intepreting the value can be difficult: we have some twenty
.get_dev_path() methods, and each of them can format however it wants.
You need to guess the format to make sense of the value.


Neither function is ideal.  But in their current state, I *strongly*
prefer qdev_get_human_name(), because I hate having to decipher a QOM
path less than having to guess what kind of device this might be, then
figure out what format its .get_dev_path() uses.

If someone posts a patch to fix the shortcomings of
qdev_get_printable_name() I outlined above, we can talk.

Re: [PATCH v3] virtio: improve virtqueue mapping error messages

[Peter Maydell]

On Tue, 10 Mar 2026 at 11:03, Markus Armbruster <armbru@redhat.com> wrote:
> Neither function is ideal.  But in their current state, I *strongly*
> prefer qdev_get_human_name(), because I hate having to decipher a QOM
> path less than having to guess what kind of device this might be, then
> figure out what format its .get_dev_path() uses.
>
> If someone posts a patch to fix the shortcomings of
> qdev_get_printable_name() I outlined above, we can talk.

How do you feel about the proposed unification of the two
functions done in this patch?

https://lore.kernel.org/qemu-devel/20260308160040.354186-2-alessandro@0x65c.net/

thanks
-- PMM

Re: [PATCH v3] virtio: improve virtqueue mapping error messages

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Tue, 10 Mar 2026 at 11:03, Markus Armbruster <armbru@redhat.com> wrote:
>> Neither function is ideal.  But in their current state, I *strongly*
>> prefer qdev_get_human_name(), because I hate having to decipher a QOM
>> path less than having to guess what kind of device this might be, then
>> figure out what format its .get_dev_path() uses.
>>
>> If someone posts a patch to fix the shortcomings of
>> qdev_get_printable_name() I outlined above, we can talk.
>
> How do you feel about the proposed unification of the two
> functions done in this patch?
>
> https://lore.kernel.org/qemu-devel/20260308160040.354186-2-alessandro@0x65c.net/

Thanks for the pointer, I'll reply there.