On Tue, 23 Sept 2025 at 18:17, Daniel P. Berrangé <[email protected]> wrote:
>
[...]
> >  hw/virtio/virtio.c | 46 +++++++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 43 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index 9a81ad912e..f5adc381a4 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -235,6 +235,37 @@ static void virtio_virtqueue_reset_region_cache(struct 
> > VirtQueue *vq)
> >      }
> >  }
> >
> > +static const char *virtio_get_pretty_dev_name(VirtIODevice *vdev)
>
> I'd suggest this be  'const char *qdev_get_printable_name(DeviceState *dev)'
> and live in  the same header & source files as qdev_get_dev_path.
>
> I used 'printable' rather than 'pretty' as I'm not sure I'd claim
> that qdev_get_dev_path() results can be said to be pretty :-)

Thanks for the review and the suggestion.

Fair enough :) — I've renamed the helper to qdev_get_printable_name() and moved
it next to qdev_get_dev_path() in `hw/core/qdev.c`, as you recommended.

[...]
> > +    /*
> > +     * Final fallback: if all else fails, return a placeholder string.
> > +     * This ensures the error message always contains a valid string.
> > +     */
> > +    return "<unknow device>";
>
> s/unknow/unknown/
>

Fixed, thanks for catching that!

[...]
>
> This part all looks good

Glad to hear! I've sent out v3 with the changes above. Let me know if you
have any further thoughts or improvements in mind.


Thanks again for your time and helpful reviews.

Best regards,
Alessandro

---

Changes since v2:
- Renamed helper to qdev_get_printable_name()
- Moved helper to appropriate source/header location
- Fixed typo in fallback string
- Incorporated review feedback from Daniel Barrangé

Reply via email to