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.0Alessandro 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.
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
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!
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
© 2016 - 2026 Red Hat, Inc.