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

[Markus Armbruster <armbru@redhat.com>]

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