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 <arm...@redhat.com>
> Signed-off-by: Alessandro Ratti <alessan...@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 :|
