hw/virtio/virtio.c | 46 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 43 insertions(+), 3 deletions(-)
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.5On 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 :|
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é
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.5On 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 :|
© 2016 - 2025 Red Hat, Inc.