[PATCH v2 0/2] qdev: Consolidate qdev_get_human_name() into qdev_get_printable_name()

[PATCH v2 0/2] qdev: Consolidate qdev_get_human_name() into qdev_get_printable_name()

[Alessandro Ratti]

v2 addresses Markus's review of v1:

- Split into two patches: improve qdev_get_printable_name() first,
  then remove qdev_get_human_name()
- Fix comments to accurately describe qdev_get_dev_path() behavior
- Drop unnecessary comments and the "<unknown device>" placeholder
- Rename @vdev to @dev, add g_assert(dev) precondition
- Update doc comment in qdev.h to reflect the new fallback chain
- Rebase on current master now that the prerequisite
  qdev_get_printable_name() changes have landed

Formatting bus-specific paths as "<bus> device <dev-path>" (e.g.
"PCI device 0000:00:00.0") is left for a follow-up series.

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


Alessandro Ratti (2):
  hw/qdev: Clarify fallback order in qdev_get_printable_name()
  hw/qdev: Remove qdev_get_human_name()

 hw/block/block.c       |  5 ++---
 hw/core/qdev.c         | 37 ++++++++++++-------------------------
 include/hw/core/qdev.h | 25 +++++--------------------
 3 files changed, 19 insertions(+), 48 deletions(-)

-- 
2.53.0

[PATCH v2 1/2] 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, rename
the @vdev parameter to @dev for consistency with surrounding code,
and add g_assert(dev) to match qdev_get_human_name()'s precondition.

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         | 29 ++++++++++++-----------------
 include/hw/core/qdev.h | 11 +++++------
 2 files changed, 17 insertions(+), 23 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index e48616b2c6..f9fb7966dd 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -411,33 +411,28 @@ 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);
+    g_assert(dev != NULL);
+
+    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.
+     * Final fallback: return the canonical QOM path
+     * (e.g. /machine/peripheral-anon/device[0]).
      */
-    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..2da7327144 100644
--- a/include/hw/core/qdev.h
+++ b/include/hw/core/qdev.h
@@ -1087,18 +1087,17 @@ char *qdev_get_dev_path(DeviceState *dev);
 
 /**
  * qdev_get_printable_name: Return human readable name for device
- * @dev: Device to get name of
+ * @dev: Device to get name of. Must not be NULL.
  *
  * 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.
+ * user-facing error messages.
  *
  * 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">.
+ * Otherwise we try the bus-specific device path (which will be
+ * the PCI address for PCI devices, for example). If all else fails
+ * we return the canonical QOM path.
  */
 const char *qdev_get_printable_name(DeviceState *dev);
 
-- 
2.53.0

Re: [PATCH v2 1/2] 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, rename
> the @vdev parameter to @dev for consistency with surrounding code,
> and add g_assert(dev) to match qdev_get_human_name()'s precondition.
>
> 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         | 29 ++++++++++++-----------------
>  include/hw/core/qdev.h | 11 +++++------
>  2 files changed, 17 insertions(+), 23 deletions(-)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index e48616b2c6..f9fb7966dd 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -411,33 +411,28 @@ 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);
> +    g_assert(dev != NULL);

This replaces crash on dev->id below by an assertion failure.  I
wouldn't bother.

> +
> +    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.
> +     * Final fallback: return the canonical QOM path
> +     * (e.g. /machine/peripheral-anon/device[0]).
>       */

Is the comment useful?

> -    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..2da7327144 100644
> --- a/include/hw/core/qdev.h
> +++ b/include/hw/core/qdev.h
> @@ -1087,18 +1087,17 @@ char *qdev_get_dev_path(DeviceState *dev);
>  
>  /**
>   * qdev_get_printable_name: Return human readable name for device
> - * @dev: Device to get name of
> + * @dev: Device to get name of. Must not be NULL.
>   *
>   * 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.
> + * user-facing error messages.
>   *
>   * 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">.
> + * Otherwise we try the bus-specific device path (which will be
> + * the PCI address for PCI devices, for example). If all else fails
> + * we return the canonical QOM path.

I prefer imperative mood for describing function behavior:

      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.

Bonus: obviously not the longwinded, flowery prose LLMs barf out[*].

>   */
>  const char *qdev_get_printable_name(DeviceState *dev);


[*] Would not be welcome in QEMU at this time; see section "Use of
AI-generated content" in docs/devel/code-provenance.rst.

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

[Alessandro Ratti]

On Tue, 17 Mar 2026 at 07:46, Markus Armbruster <armbru@redhat.com> wrote:
>
> 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, rename
> > the @vdev parameter to @dev for consistency with surrounding code,
> > and add g_assert(dev) to match qdev_get_human_name()'s precondition.
> >
> > 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         | 29 ++++++++++++-----------------
> >  include/hw/core/qdev.h | 11 +++++------
> >  2 files changed, 17 insertions(+), 23 deletions(-)
> >
> > diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> > index e48616b2c6..f9fb7966dd 100644
> > --- a/hw/core/qdev.c
> > +++ b/hw/core/qdev.c
> > @@ -411,33 +411,28 @@ 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);
> > +    g_assert(dev != NULL);
>
> This replaces crash on dev->id below by an assertion failure.  I
> wouldn't bother.

I'll drop it.

>
> > +
> > +    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.
> > +     * Final fallback: return the canonical QOM path
> > +     * (e.g. /machine/peripheral-anon/device[0]).
> >       */
>
> Is the comment useful?

Fair enough. I'll drop it.

> > -    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..2da7327144 100644
> > --- a/include/hw/core/qdev.h
> > +++ b/include/hw/core/qdev.h
> > @@ -1087,18 +1087,17 @@ char *qdev_get_dev_path(DeviceState *dev);
> >
> >  /**
> >   * qdev_get_printable_name: Return human readable name for device
> > - * @dev: Device to get name of
> > + * @dev: Device to get name of. Must not be NULL.
> >   *
> >   * 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.
> > + * user-facing error messages.
> >   *
> >   * 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">.
> > + * Otherwise we try the bus-specific device path (which will be
> > + * the PCI address for PCI devices, for example). If all else fails
> > + * we return the canonical QOM path.
>
> I prefer imperative mood for describing function behavior:
>
>       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.

I'll adopt as-is.

> Bonus: obviously not the longwinded, flowery prose LLMs barf out[*].
>
> >   */
> >  const char *qdev_get_printable_name(DeviceState *dev);
>
>
> [*] Would not be welcome in QEMU at this time; see section "Use of
> AI-generated content" in docs/devel/code-provenance.rst.

Noted, thanks.

[PATCH v2 2/2] hw/qdev: Remove qdev_get_human_name()

[Alessandro Ratti]

Remove qdev_get_human_name() and switch its two callers in
hw/block/block.c to qdev_get_printable_name().

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
device path first, providing more informative output.

Narrow the scope of dev_id in blk_check_size_and_read_all() to the
blocks where it is actually used.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Alessandro Ratti <alessandro@0x65c.net>
---
 hw/block/block.c       |  5 ++---
 hw/core/qdev.c         |  8 --------
 include/hw/core/qdev.h | 14 --------------
 3 files changed, 2 insertions(+), 25 deletions(-)

diff --git a/hw/block/block.c b/hw/block/block.c
index f187fa025d..84e5298e2f 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -65,7 +65,6 @@ bool blk_check_size_and_read_all(BlockBackend *blk, DeviceState *dev,
 {
     int64_t blk_len;
     int ret;
-    g_autofree char *dev_id = NULL;
 
     if (cpr_is_incoming()) {
         return true;
@@ -78,7 +77,7 @@ bool blk_check_size_and_read_all(BlockBackend *blk, DeviceState *dev,
         return false;
     }
     if (blk_len != size) {
-        dev_id = qdev_get_human_name(dev);
+        g_autofree const char *dev_id = qdev_get_printable_name(dev);
         error_setg(errp, "%s device '%s' requires %" HWADDR_PRIu
                    " bytes, %s block backend provides %" PRIu64 " bytes",
                    object_get_typename(OBJECT(dev)), dev_id, size,
@@ -95,7 +94,7 @@ bool blk_check_size_and_read_all(BlockBackend *blk, DeviceState *dev,
     assert(size <= BDRV_REQUEST_MAX_BYTES);
     ret = blk_pread_nonzeroes(blk, size, buf);
     if (ret < 0) {
-        dev_id = qdev_get_human_name(dev);
+        g_autofree const char *dev_id = qdev_get_printable_name(dev);
         error_setg_errno(errp, -ret, "can't read %s block backend"
                          " for %s device '%s'",
                          blk_name(blk), object_get_typename(OBJECT(dev)),
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index f9fb7966dd..f464f63521 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -862,14 +862,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/include/hw/core/qdev.h b/include/hw/core/qdev.h
index 2da7327144..cc847c20c1 100644
--- a/include/hw/core/qdev.h
+++ b/include/hw/core/qdev.h
@@ -1045,20 +1045,6 @@ void qdev_create_fake_machine(void);
  */
 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.
- *
- * Use g_free() to free it.
- */
-char *qdev_get_human_name(DeviceState *dev);
-
 /* FIXME: make this a link<> */
 bool qdev_set_parent_bus(DeviceState *dev, BusState *bus, Error **errp);
 
-- 
2.53.0

Re: [PATCH v2 2/2] hw/qdev: Remove qdev_get_human_name()

[Markus Armbruster]

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

> Remove qdev_get_human_name() and switch its two callers in
> hw/block/block.c to qdev_get_printable_name().
>
> 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
> device path first, providing more informative output.
>
> Narrow the scope of dev_id in blk_check_size_and_read_all() to the
> blocks where it is actually used.
>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Alessandro Ratti <alessandro@0x65c.net>

This replaces two ways to describe a device to the user by one.  Good.

One half of the uses is unchanged.  Good.

The other half can now show a bus-specific device path instead of the
canonical QOM path.  This path can be difficult to interpret: we have
some twenty .get_dev_path() methods, and each of them can format however
it wants.  I need to guess the format to make sense of the value.  I'm
inclined to call this a regression.

To address this, please insert another patch before this one that
changes

        device <dev-path>

  to

        <bus> device <dev-path>

This removes the guesswork, and actually satisfies the claim "more
informative output".

If you have reasons to do it in a follow-up patch instead, explain them
briefly.

qdev_get_human_name() is a better name than qdev_get_printable_name().
Please consider renaming the function so we keep the better name.

Re: [PATCH v2 2/2] hw/qdev: Remove qdev_get_human_name()

[Alessandro Ratti]

On Tue, 17 Mar 2026 at 08:08, Markus Armbruster <armbru@redhat.com> wrote:
>
> Alessandro Ratti <alessandro@0x65c.net> writes:
>
> > Remove qdev_get_human_name() and switch its two callers in
> > hw/block/block.c to qdev_get_printable_name().
> >
> > 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
> > device path first, providing more informative output.
> >
> > Narrow the scope of dev_id in blk_check_size_and_read_all() to the
> > blocks where it is actually used.
> >
> > Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> > Signed-off-by: Alessandro Ratti <alessandro@0x65c.net>
>
> This replaces two ways to describe a device to the user by one.  Good.
>
> One half of the uses is unchanged.  Good.
>
> The other half can now show a bus-specific device path instead of the
> canonical QOM path.  This path can be difficult to interpret: we have
> some twenty .get_dev_path() methods, and each of them can format however
> it wants.  I need to guess the format to make sense of the value.  I'm
> inclined to call this a regression.
>
> To address this, please insert another patch before this one that
> changes
>
>         device <dev-path>
>
>   to
>
>         <bus> device <dev-path>
>
> This removes the guesswork, and actually satisfies the claim "more
> informative output".
>

Will do. I've prototyped this using object_get_typename() on
dev->parent_bus to get the bus type.

A complication: a few buses (virtio-pci-bus, ufs-bus) delegate
get_dev_path() to their parent unchanged, so the path format doesn't
match the immediate bus type. I detect delegation by comparing the
child's path to the parent bus's path, and format as
"<child-bus> on <parent-bus> device <path>".

Tested with i440fx + virtio-blk + UFS + USB + SCSI:

  PCI device 0000:00:04.0
  virtio-pci-bus on PCI device 0000:00:04.0
  ufs-bus on PCI device 0000:00:04.0
  SCSI device 0000:00:04.0/0:0:0
  usb-bus device 0000:00:03.0/1

And with Q35:

  PCIE device 0000:00:04.0
  virtio-pci-bus on PCIE device 0000:00:04.0
  ufs-bus on PCIE device 0000:00:04.0
  SCSI device 0000:00:04.0/0:0:0
  usb-bus device 0000:00:03.0/1

If my understanding is correct, VMBus delegates to sysbus which has
no get_dev_path(), so it falls through to the QOM path.

Note: object_get_typename() gives "PCI" on i440fx and "PCIE" on Q35
for the same address format. Not sure if that's confusing enough to
warrant normalizing.

> qdev_get_human_name() is a better name than qdev_get_printable_name().
> Please consider renaming the function so we keep the better name.
>

Ack, will rename.

If my proposal to add "<bus> device" prefix sounds solid,
I'll send a v3 with the series restructured as:

  1. Replace "<unknown device>" with canonical QOM path, clean up comments
  2. Add "<bus> device" prefix to bus-specific paths
  3. Consolidate into qdev_get_human_name()

Thanks for your time and consideration.

Best regards,
Alessandro

Re: [PATCH v2 2/2] hw/qdev: Remove qdev_get_human_name()

[Markus Armbruster]

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

> On Tue, 17 Mar 2026 at 08:08, Markus Armbruster <armbru@redhat.com> wrote:
>>
>> Alessandro Ratti <alessandro@0x65c.net> writes:
>>
>> > Remove qdev_get_human_name() and switch its two callers in
>> > hw/block/block.c to qdev_get_printable_name().
>> >
>> > 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
>> > device path first, providing more informative output.
>> >
>> > Narrow the scope of dev_id in blk_check_size_and_read_all() to the
>> > blocks where it is actually used.
>> >
>> > Suggested-by: Peter Maydell <peter.maydell@linaro.org>
>> > Signed-off-by: Alessandro Ratti <alessandro@0x65c.net>
>>
>> This replaces two ways to describe a device to the user by one.  Good.
>>
>> One half of the uses is unchanged.  Good.
>>
>> The other half can now show a bus-specific device path instead of the
>> canonical QOM path.  This path can be difficult to interpret: we have
>> some twenty .get_dev_path() methods, and each of them can format however
>> it wants.  I need to guess the format to make sense of the value.  I'm
>> inclined to call this a regression.
>>
>> To address this, please insert another patch before this one that
>> changes
>>
>>         device <dev-path>
>>
>>   to
>>
>>         <bus> device <dev-path>
>>
>> This removes the guesswork, and actually satisfies the claim "more
>> informative output".
>>
>
> Will do. I've prototyped this using object_get_typename() on
> dev->parent_bus to get the bus type.
>
> A complication: a few buses (virtio-pci-bus, ufs-bus) delegate
> get_dev_path() to their parent unchanged,

Like so:

    static char *ufs_bus_get_dev_path(DeviceState *dev)
    {
        BusState *bus = qdev_get_parent_bus(dev);

        return qdev_get_dev_path(bus->parent);
    }

>                                           so the path format doesn't
> match the immediate bus type.

.get_dev_path()'s contract:

    /**
     * qdev_get_dev_path(): Return the path of a device on its bus
     * @dev: device to get the path of
     *
     * Returns: A newly allocated string containing the dev path of
     * @dev. The caller must free this with g_free().
     * The format of the string depends on the bus; for instance a
     * PCI device's path will be in the format::
     *
     *   Domain:00:Slot.Function:Slot.Function....:Slot.Function
     *
     * and a SCSI device's path will be::
     *
     *   channel:ID:LUN
     *
     * (possibly prefixed by the path of the SCSI controller).
     *
     * If @dev is NULL or not on a bus, returns NULL.
     */

This looks decent at a glance, but it's actually somewhat woolly.

>                               I detect delegation by comparing the
> child's path to the parent bus's path, and format as
> "<child-bus> on <parent-bus> device <path>".
>
> Tested with i440fx + virtio-blk + UFS + USB + SCSI:
>
>   PCI device 0000:00:04.0
>   virtio-pci-bus on PCI device 0000:00:04.0
>   ufs-bus on PCI device 0000:00:04.0
>   SCSI device 0000:00:04.0/0:0:0
>   usb-bus device 0000:00:03.0/1
>
> And with Q35:
>
>   PCIE device 0000:00:04.0
>   virtio-pci-bus on PCIE device 0000:00:04.0
>   ufs-bus on PCIE device 0000:00:04.0
>   SCSI device 0000:00:04.0/0:0:0
>   usb-bus device 0000:00:03.0/1
>
> If my understanding is correct, VMBus delegates to sysbus which has
> no get_dev_path(), so it falls through to the QOM path.
>
> Note: object_get_typename() gives "PCI" on i440fx and "PCIE" on Q35
> for the same address format. Not sure if that's confusing enough to
> warrant normalizing.
>
>> qdev_get_human_name() is a better name than qdev_get_printable_name().
>> Please consider renaming the function so we keep the better name.
>>
>
> Ack, will rename.
>
> If my proposal to add "<bus> device" prefix sounds solid,
> I'll send a v3 with the series restructured as:
>
>   1. Replace "<unknown device>" with canonical QOM path, clean up comments
>   2. Add "<bus> device" prefix to bus-specific paths
>   3. Consolidate into qdev_get_human_name()
>
> Thanks for your time and consideration.
>
> Best regards,
> Alessandro

Re: [PATCH v2 2/2] hw/qdev: Remove qdev_get_human_name()

[Markus Armbruster]

Ignore this one, butterfingers.  Complete reply coming.

Re: [PATCH v2 2/2] hw/qdev: Remove qdev_get_human_name()

[Markus Armbruster]

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

> On Tue, 17 Mar 2026 at 08:08, Markus Armbruster <armbru@redhat.com> wrote:
>>
>> Alessandro Ratti <alessandro@0x65c.net> writes:
>>
>> > Remove qdev_get_human_name() and switch its two callers in
>> > hw/block/block.c to qdev_get_printable_name().
>> >
>> > 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
>> > device path first, providing more informative output.
>> >
>> > Narrow the scope of dev_id in blk_check_size_and_read_all() to the
>> > blocks where it is actually used.
>> >
>> > Suggested-by: Peter Maydell <peter.maydell@linaro.org>
>> > Signed-off-by: Alessandro Ratti <alessandro@0x65c.net>
>>
>> This replaces two ways to describe a device to the user by one.  Good.
>>
>> One half of the uses is unchanged.  Good.
>>
>> The other half can now show a bus-specific device path instead of the
>> canonical QOM path.  This path can be difficult to interpret: we have
>> some twenty .get_dev_path() methods, and each of them can format however
>> it wants.  I need to guess the format to make sense of the value.  I'm
>> inclined to call this a regression.
>>
>> To address this, please insert another patch before this one that
>> changes
>>
>>         device <dev-path>
>>
>>   to
>>
>>         <bus> device <dev-path>
>>
>> This removes the guesswork, and actually satisfies the claim "more
>> informative output".
>>
>
> Will do. I've prototyped this using object_get_typename() on
> dev->parent_bus to get the bus type.
>
> A complication: a few buses (virtio-pci-bus, ufs-bus) delegate
> get_dev_path() to their parent unchanged,

Like so:

    static char *ufs_bus_get_dev_path(DeviceState *dev)
    {
        BusState *bus = qdev_get_parent_bus(dev);

        return qdev_get_dev_path(bus->parent);
    }

>                                           so the path format doesn't
> match the immediate bus type.

Well, what *is* the immediate bus type's path format?

Does it even have one?

>                               I detect delegation by comparing the
> child's path to the parent bus's path, and format as
> "<child-bus> on <parent-bus> device <path>".

Hmm.

> Tested with i440fx + virtio-blk + UFS + USB + SCSI:
>
>   PCI device 0000:00:04.0
>   virtio-pci-bus on PCI device 0000:00:04.0
>   ufs-bus on PCI device 0000:00:04.0
>   SCSI device 0000:00:04.0/0:0:0
>   usb-bus device 0000:00:03.0/1
>
> And with Q35:
>
>   PCIE device 0000:00:04.0
>   virtio-pci-bus on PCIE device 0000:00:04.0
>   ufs-bus on PCIE device 0000:00:04.0
>   SCSI device 0000:00:04.0/0:0:0

Note the dev path format PARENT-DEV-PATH/BUS-ADDRESS.

The parent device happens to be a PCI device, so PARENT-DEV-PATH is a
PCI address, which is unique within the machine.

>   usb-bus device 0000:00:03.0/1

Likewise.

ufs-bus does just PARENT-DEV-PATH.  Clashes with the parent device's dev
path, and also with sibling buses if the parent device provides more
than one ufs-bus.  Prefixing with the bus type disambiguates parent and
child, but not siblings.  I don't like this at all.

The stupidest patch I can think of: PARENT-DEV-PATH/NUMBER, where NUMBER
enumerates the siblings.

> If my understanding is correct, VMBus delegates to sysbus which has
> no get_dev_path(), so it falls through to the QOM path.

.get_dev_path()'s contract:

    /**
     * qdev_get_dev_path(): Return the path of a device on its bus
     * @dev: device to get the path of
     *
     * Returns: A newly allocated string containing the dev path of
     * @dev. The caller must free this with g_free().
     * The format of the string depends on the bus; for instance a
     * PCI device's path will be in the format::
     *
     *   Domain:00:Slot.Function:Slot.Function....:Slot.Function
     *
     * and a SCSI device's path will be::
     *
     *   channel:ID:LUN
     *
     * (possibly prefixed by the path of the SCSI controller).
     *
     * If @dev is NULL or not on a bus, returns NULL.
     */

This looks decent at first glance, but it's actually quite woolly.

What's a "path on a bus"?

Is it unique within the machine?

For what it's worth, the PCI device path shown as example is.

The SCSI device path isn't, unless it's actually "prefixed by the path
of the SCSI controller", *and* that path is unique.  Code:

    static char *scsibus_get_dev_path(DeviceState *dev)
    {
        SCSIDevice *d = SCSI_DEVICE(dev);
        DeviceState *hba = dev->parent_bus->parent;
        char *id;
        char *path;

        id = qdev_get_dev_path(hba);
        if (id) {
            path = g_strdup_printf("%s/%d:%d:%d", id, d->channel, d->id, d->lun);
        } else {
            path = g_strdup_printf("%d:%d:%d", d->channel, d->id, d->lun);
        }
        g_free(id);
        return path;
    }

The returned path is unique within the machine only when
get_dev_path(hba) is non-null and unique.

I smell a dung pile.

I think your idea to add "on PARENT-DEV-PATH" papers over
.get_dev_path() defects we should fix instead.  I'm not demanding *you*
fix them all.

How would you code qdev_get_printable_name() if qdev_get_dev_path()
either returns null or a string that together with the bus type uniquely
identifies the device?

> Note: object_get_typename() gives "PCI" on i440fx and "PCIE" on Q35
> for the same address format. Not sure if that's confusing enough to
> warrant normalizing.

Feels okay to me.

>> qdev_get_human_name() is a better name than qdev_get_printable_name().
>> Please consider renaming the function so we keep the better name.
>>
>
> Ack, will rename.
>
> If my proposal to add "<bus> device" prefix sounds solid,
> I'll send a v3 with the series restructured as:
>
>   1. Replace "<unknown device>" with canonical QOM path, clean up comments
>   2. Add "<bus> device" prefix to bus-specific paths
>   3. Consolidate into qdev_get_human_name()

Sounds good!

> Thanks for your time and consideration.
>
> Best regards,
> Alessandro

Thank you!