[PATCH v3 0/3] hw/qdev: Consolidate qdev_get_human_name() and qdev_get_printable_name()

[PATCH v3 0/3] hw/qdev: Consolidate qdev_get_human_name() and qdev_get_printable_name()

[Alessandro Ratti]

v3 addresses Markus's review of v2:

- Drop g_assert(dev) and final fallback comment
- Adopt imperative mood for doc comment (Markus's wording)
- Add "<bus-type> device <path>" prefix to bus-specific paths
  instead of deferring to a follow-up series
- Rename qdev_get_printable_name() to qdev_get_human_name()

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

Alessandro Ratti (3):
  hw/qdev: Clarify fallback order in qdev_get_printable_name()
  hw/qdev: Prefix bus type in qdev_get_printable_name() device paths
  hw/qdev: Consolidate qdev_get_printable_name() into
    qdev_get_human_name()

 hw/core/qdev.c         | 38 ++++++++++----------------------------
 hw/virtio/virtio.c     |  6 +++---
 include/hw/core/qdev.h | 28 +++++-----------------------
 3 files changed, 18 insertions(+), 54 deletions(-)

-- 
2.53.0

[PATCH v3 1/3] hw/qdev: Clarify fallback order in qdev_get_printable_name()

[Alessandro Ratti]

Replace the uninformative "<unknown device>" final fallback with the
canonical QOM path (e.g. /machine/peripheral-anon/device[0]).

Also clean up comments to accurately describe qdev_get_dev_path()
behavior, drop an unnecessary comment on the dev->id check, and rename
the @vdev parameter to @dev for consistency with surrounding code.

Update the doc comment in qdev.h to reflect the new fallback chain.

Signed-off-by: Alessandro Ratti <alessandro@0x65c.net>
---
 hw/core/qdev.c         | 26 +++++++-------------------
 include/hw/core/qdev.h |  8 +++-----
 2 files changed, 10 insertions(+), 24 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index e48616b2c6..c44616b4b8 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -411,33 +411,21 @@ char *qdev_get_dev_path(DeviceState *dev)
     return NULL;
 }
 
-const char *qdev_get_printable_name(DeviceState *vdev)
+const char *qdev_get_printable_name(DeviceState *dev)
 {
-    /*
-     * 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 g_strdup(vdev->id);
+    if (dev->id) {
+        return g_strdup(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.
+     * Fall back to a bus-specific device path, if the bus
+     * provides one (e.g. PCI address "0000:00:04.0").
      */
-    const char *path = qdev_get_dev_path(vdev);
+    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 g_strdup("<unknown device>");
+    return object_get_canonical_path(OBJECT(dev));
 }
 
 void qdev_add_unplug_blocker(DeviceState *dev, Error *reason)
diff --git a/include/hw/core/qdev.h b/include/hw/core/qdev.h
index f99a8979cc..b87497906a 100644
--- a/include/hw/core/qdev.h
+++ b/include/hw/core/qdev.h
@@ -1094,11 +1094,9 @@ char *qdev_get_dev_path(DeviceState *dev);
  * user-facing error messages. The function will never return NULL,
  * so the name can be used without further checking or fallbacks.
  *
- * If the device has an explicitly set ID (e.g. by the user on the
- * command line via "-device thisdev,id=myid") this is preferred.
- * Otherwise we try the canonical QOM device path (which will be
- * the PCI ID for PCI devices, for example). If all else fails
- * we will return the placeholder "<unknown device">.
+ * Return the device's ID if it has one.  Else, return the path of a
+ * device on its bus if it has one.  Else return its canonical QOM
+ * path.
  */
 const char *qdev_get_printable_name(DeviceState *dev);
 
-- 
2.53.0

Re: [PATCH v3 1/3] hw/qdev: Clarify fallback order in qdev_get_printable_name()

[Markus Armbruster]

Alessandro Ratti <alessandro@0x65c.net> writes:

> Replace the uninformative "<unknown device>" final fallback with the
> canonical QOM path (e.g. /machine/peripheral-anon/device[0]).
>
> Also clean up comments to accurately describe qdev_get_dev_path()
> behavior, drop an unnecessary comment on the dev->id check, and rename
> the @vdev parameter to @dev for consistency with surrounding code.
>
> Update the doc comment in qdev.h to reflect the new fallback chain.
>
> Signed-off-by: Alessandro Ratti <alessandro@0x65c.net>

Reviewed-by: Markus Armbruster <armbru@redhat.com>

[PATCH v3 2/3] hw/qdev: Prefix bus type in qdev_get_printable_name() device paths

[Alessandro Ratti]

Raw get_dev_path() output (e.g. "0000:00:04.0", "/1") is ambiguous
without knowing which bus produced it. Prefix the path with the bus
type name so error messages become self-describing.

Examples:
  - PCIE device 0000:00:04.0
  - virtio-pci-bus device 0000:00:03.0

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

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index c44616b4b8..904e710f8e 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -418,11 +418,13 @@ const char *qdev_get_printable_name(DeviceState *dev)
     }
     /*
      * Fall back to a bus-specific device path, if the bus
-     * provides one (e.g. PCI address "0000:00:04.0").
+     * provides one (e.g. "PCI device 0000:00:04.0").
      */
-    const char *path = qdev_get_dev_path(dev);
+    g_autofree char *path = qdev_get_dev_path(dev);
     if (path) {
-        return path;
+        const char *bus_type = object_get_typename(OBJECT(dev->parent_bus));
+        char *name = g_strdup_printf("%s device %s", bus_type, path);
+        return name;
     }
 
     return object_get_canonical_path(OBJECT(dev));
-- 
2.53.0

Re: [PATCH v3 2/3] hw/qdev: Prefix bus type in qdev_get_printable_name() device paths

[Markus Armbruster]

Alessandro Ratti <alessandro@0x65c.net> writes:

> Raw get_dev_path() output (e.g. "0000:00:04.0", "/1") is ambiguous

"can be ambiguous" would be more precise.

> without knowing which bus produced it. Prefix the path with the bus
> type name so error messages become self-describing.
>
> Examples:
>   - PCIE device 0000:00:04.0
>   - virtio-pci-bus device 0000:00:03.0
>
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Alessandro Ratti <alessandro@0x65c.net>
> ---
>  hw/core/qdev.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index c44616b4b8..904e710f8e 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -418,11 +418,13 @@ const char *qdev_get_printable_name(DeviceState *dev)
>      }
>      /*
>       * Fall back to a bus-specific device path, if the bus
> -     * provides one (e.g. PCI address "0000:00:04.0").
> +     * provides one (e.g. "PCI device 0000:00:04.0").
>       */
> -    const char *path = qdev_get_dev_path(dev);
> +    g_autofree char *path = qdev_get_dev_path(dev);
>      if (path) {
> -        return path;
> +        const char *bus_type = object_get_typename(OBJECT(dev->parent_bus));
> +        char *name = g_strdup_printf("%s device %s", bus_type, path);
> +        return name;
>      }
>  
>      return object_get_canonical_path(OBJECT(dev));

Reviewed-by: Markus Armbruster <armbru@redhat.com>

[PATCH v3 3/3] hw/qdev: Consolidate qdev_get_printable_name() into qdev_get_human_name()

[Alessandro Ratti]

Rename qdev_get_printable_name() to qdev_get_human_name(), remove
the old qdev_get_human_name() implementation, and switch the three
qdev_get_printable_name() callers in hw/virtio/virtio.c.

qdev_get_printable_name() subsumes qdev_get_human_name(): both
return the device ID when set and fall back to the canonical QOM
path, but qdev_get_printable_name() also tries the bus-specific
path first, providing more informative output.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Alessandro Ratti <alessandro@0x65c.net>
---
 hw/core/qdev.c         | 10 +---------
 hw/virtio/virtio.c     |  6 +++---
 include/hw/core/qdev.h | 26 +++++---------------------
 3 files changed, 9 insertions(+), 33 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 904e710f8e..93347f67bb 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -411,7 +411,7 @@ char *qdev_get_dev_path(DeviceState *dev)
     return NULL;
 }
 
-const char *qdev_get_printable_name(DeviceState *dev)
+char *qdev_get_human_name(DeviceState *dev)
 {
     if (dev->id) {
         return g_strdup(dev->id);
@@ -857,14 +857,6 @@ Object *machine_get_container(const char *name)
     return container;
 }
 
-char *qdev_get_human_name(DeviceState *dev)
-{
-    g_assert(dev != NULL);
-
-    return dev->id ?
-           g_strdup(dev->id) : object_get_canonical_path(OBJECT(dev));
-}
-
 static MachineInitPhase machine_phase;
 
 bool phase_check(MachineInitPhase phase)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 8fcf6cfd0b..63e2faee99 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -281,7 +281,7 @@ 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) {
-        g_autofree const char *devname = qdev_get_printable_name(DEVICE(vdev));
+        g_autofree char *devname = qdev_get_human_name(DEVICE(vdev));
 
         virtio_error(vdev,
                 "Failed to map descriptor ring for device %s: "
@@ -294,7 +294,7 @@ 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) {
-        g_autofree const char *devname = qdev_get_printable_name(DEVICE(vdev));
+        g_autofree char *devname = qdev_get_human_name(DEVICE(vdev));
 
         virtio_error(vdev,
                 "Failed to map used ring for device %s: "
@@ -307,7 +307,7 @@ 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) {
-        g_autofree const char *devname = qdev_get_printable_name(DEVICE(vdev));
+        g_autofree char *devname = qdev_get_human_name(DEVICE(vdev));
 
         virtio_error(vdev,
                 "Failed to map avalaible ring for device %s: "
diff --git a/include/hw/core/qdev.h b/include/hw/core/qdev.h
index b87497906a..43d8e58432 100644
--- a/include/hw/core/qdev.h
+++ b/include/hw/core/qdev.h
@@ -1049,13 +1049,12 @@ Object *machine_get_container(const char *name);
  * qdev_get_human_name() - Return a human-readable name for a device
  * @dev: The device. Must be a valid and non-NULL pointer.
  *
- * .. note::
- *    This function is intended for user friendly error messages.
- *
- * Returns: A newly allocated string containing the device id if not null,
- * else the object canonical path.
+ * Returns: A newly allocated string suitable for user-facing error
+ * messages.
  *
- * Use g_free() to free it.
+ * Return the device's ID if it has one.  Else, return the path of a
+ * device on its bus if it has one.  Else return its canonical QOM
+ * path.
  */
 char *qdev_get_human_name(DeviceState *dev);
 
@@ -1085,21 +1084,6 @@ extern bool qdev_hot_removed;
  */
 char *qdev_get_dev_path(DeviceState *dev);
 
-/**
- * qdev_get_printable_name: Return human readable name for device
- * @dev: Device to get name of
- *
- * Returns: A newly allocated string containing some human
- * readable name for the device, suitable for printing in
- * user-facing error messages. The function will never return NULL,
- * so the name can be used without further checking or fallbacks.
- *
- * Return the device's ID if it has one.  Else, return the path of a
- * device on its bus if it has one.  Else return its canonical QOM
- * path.
- */
-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.53.0

Re: [PATCH v3 3/3] hw/qdev: Consolidate qdev_get_printable_name() into qdev_get_human_name()

[Markus Armbruster]

Alessandro Ratti <alessandro@0x65c.net> writes:

> Rename qdev_get_printable_name() to qdev_get_human_name(), remove
> the old qdev_get_human_name() implementation, and switch the three
> qdev_get_printable_name() callers in hw/virtio/virtio.c.
>
> qdev_get_printable_name() subsumes qdev_get_human_name(): both
> return the device ID when set and fall back to the canonical QOM
> path, but qdev_get_printable_name() also tries the bus-specific
> path first, providing more informative output.
>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Alessandro Ratti <alessandro@0x65c.net>

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH v3 0/3] hw/qdev: Consolidate qdev_get_human_name() and qdev_get_printable_name()

[Paolo Bonzini]

Queued, thanks.

Paolo