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

Alessandro Ratti posted 2 patches 3 weeks, 5 days ago
Maintainers: John Snow <jsnow@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>
[PATCH v2 1/2] hw/qdev: Clarify fallback order in qdev_get_printable_name()
Posted by Alessandro Ratti 3 weeks, 5 days ago
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()
Posted by Markus Armbruster 3 weeks ago
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()
Posted by Alessandro Ratti 2 weeks, 6 days ago
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.