Hello, This patch consolidates qdev_get_human_name() and qdev_get_printable_name() into a single function, as suggested by Peter Maydell here [1]
This patch is based on Peter's recent series and should be applied after [2]: - "hw/qdev: Document qdev_get_dev_path()" - "hw: Make qdev_get_printable_name() consistently return freeable string" Thank you for your time and consideration. Best regards Alessandro [1]: https://lore.kernel.org/qemu-devel/[email protected]/T/#m89da9b4e30b7c84713ca4b6c323514c72897e649 [2]: https://lore.kernel.org/qemu-devel/[email protected]/T/#m962127cb58192e0b2095039cb2fb79145f2a7388 --- Remove qdev_get_human_name() and use qdev_get_printable_name() for all device identification in error messages. Both functions now behave similarly, with qdev_get_printable_name() preferring bus-specific paths (e.g., PCI addresses) when available before falling back to QOM canonical paths. This provides better context in error messages while maintaining the same level of detail. Error messages will now show device identifiers in this priority: 1. User-specified device IDs (e.g., -device virtio-blk,id=foo) 2. Bus-specific identifiers (e.g., PCI addresses like 0000:00:04.0) 3. QOM canonical paths as a last resort Suggested-by: Peter Maydell <[email protected]> Signed-off-by: Alessandro Ratti <[email protected]> --- hw/block/block.c | 5 ++--- hw/core/qdev.c | 19 ++++++++----------- include/hw/core/qdev.h | 13 ------------- 3 files changed, 10 insertions(+), 27 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 e48616b2c6..9ee98a0c39 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -434,10 +434,15 @@ const char *qdev_get_printable_name(DeviceState *vdev) } /* - * 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. + * While verbose (e.g., /machine/peripheral-anon/device[0]), this + * provides accurate device identification when neither a user-specified + * ID nor a bus-specific path is available. Only falls back to + * <unknown device> in the extremely rare case where even the QOM + * path is unavailable. */ - return g_strdup("<unknown device>"); + char *qom_path = object_get_canonical_path(OBJECT(vdev)); + return qom_path ? qom_path : g_strdup("<unknown device>"); } void qdev_add_unplug_blocker(DeviceState *dev, Error *reason) @@ -867,14 +872,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 f99a8979cc..09dafd3d59 100644 --- a/include/hw/core/qdev.h +++ b/include/hw/core/qdev.h @@ -1045,19 +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
