On Sat, 7 Mar 2026 at 14:39, Peter Maydell <[email protected]> wrote: > > On Wed, 24 Sept 2025 at 10:37, Daniel P. Berrangé <[email protected]> wrote: > > > > On Wed, Sep 24, 2025 at 11:14:04AM +0200, Alessandro Ratti wrote: > > > Improve error reporting when virtqueue ring mapping fails by including a > > > device identifier in the error message. > > > > > > Introduce a helper qdev_get_printable_name() in qdev-core, which returns > > > either: > > > > > > - the device ID, if explicitly provided (e.g. -device ...,id=foo) > > > - the QOM path from qdev_get_dev_path(dev) otherwise > > > - "<unknown device>" as a fallback when no identifier is present > > > > > > This makes it easier to identify which device triggered the error in > > > multi-device setups or when debugging complex guest configurations. > > > > > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/230 > > > Buglink: https://bugs.launchpad.net/qemu/+bug/1919021 > > > > > > Suggested-by: Markus Armbruster <[email protected]> > > > Signed-off-by: Alessandro Ratti <[email protected]> > > > --- > > > hw/core/qdev.c | 29 +++++++++++++++++++++++++++++ > > > hw/virtio/virtio.c | 15 ++++++++++++--- > > > include/hw/qdev-core.h | 1 + > > > 3 files changed, 42 insertions(+), 3 deletions(-) > > > > Reviewed-by: Daniel P. Berrangé <[email protected]> > > Hi; I just found this commit (now e209d4d7a31b9) in the course > of finding a memory leak in it. I'm about to send patches to fix > that, but in the meantime: > > We now have two different functions for "give me a nice string > that I can use in an error message about this device": > > * qdev_get_human_name(), used only in hw/block/block.c > * qdev_get_printable_name(), used only in hw/virtio/virtio.c > > Can we please have *one* function for this purpose? > > I see from the thread that the criticism of qdev_get_human_name() > is that "it often returns opaque paths like > > /machine/peripheral-anon/device[0]/virtio-backend > > which are less informative in user-facing logs compared to PCI IDs or > user-specified names". > > So could we instead make block.c use qdev_get_printable_name(), and > remove qdev_get_human_name() ? It also is using the result only > to construct an error string for the user. >
Hi Peter, Thank you for catching the memory leak and for the feedback on the API design. You're right that having two similar functions is confusing and unnecessarily duplicates functionality. I agree that consolidating to a single function makes sense. I can send a follow-up patch to: 1. Replace the usage of qdev_get_human_name() in hw/block/block.c with qdev_get_printable_name() 2. Remove qdev_get_human_name() entirely 3. Fix the memory leak in qdev_get_printable_name() (unless you already have a patch ready to send) Alternatively, if you prefer to handle the consolidation as part of your memory leak fix series, please feel free to do so - just let me know. Thank you for your time and for improving the code. Best regards, Alessandro
