On Sun, Oct 05, 2025 at 10:13:57PM +0200, Alessandro Ratti wrote:
> On Sun, 5 Oct 2025 at 21:17, Michael S. Tsirkin <[email protected]> wrote:
> >
> > From: Alessandro Ratti <[email protected]>
> >
> > Replace virtio_error() with a macro that automatically prepends the
> > calling function name to error messages. This provides better context
> > for debugging virtio issues by showing exactly which function
> > encountered the error.
> >
> > Before: "Invalid queue size: 1024"
> > After:  "virtio_queue_set_num: Invalid queue size: 1024"
> >
> > The implementation uses a macro to insert __func__ at compile time,
> > avoiding any runtime overhead while providing more specific error
> > context than a generic "virtio:" prefix.
> >
> > Also remove manual __func__ usage in virtio-balloon to avoid duplicate
> > function names in error messages.
> >
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/230
> > Buglink: https://bugs.launchpad.net/qemu/+bug/1919021
> > Reviewed-by: Alex Bennée <[email protected]>
> > Signed-off-by: Alessandro Ratti <[email protected]>
> > Reviewed-by: Michael S. Tsirkin <[email protected]>
> > Message-ID: <[email protected]>
> > Signed-off-by: Michael S. Tsirkin <[email protected]>
> > ---
> >  include/hw/virtio/virtio.h | 4 +++-
> >  hw/virtio/virtio-balloon.c | 2 +-
> >  hw/virtio/virtio.c         | 3 ++-
> >  3 files changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > index d97529c3f1..695bb56186 100644
> > --- a/include/hw/virtio/virtio.h
> > +++ b/include/hw/virtio/virtio.h
> > @@ -253,7 +253,9 @@ void virtio_init(VirtIODevice *vdev, uint16_t 
> > device_id, size_t config_size);
> >
> >  void virtio_cleanup(VirtIODevice *vdev);
> >
> > -void virtio_error(VirtIODevice *vdev, const char *fmt, ...) 
> > G_GNUC_PRINTF(2, 3);
> > +#define virtio_error(vdev, fmt, ...) \
> > +    virtio_error_impl(vdev, "%s: " fmt, __func__, ##__VA_ARGS__)
> > +void virtio_error_impl(VirtIODevice *, const char *fmt, ...) 
> > G_GNUC_PRINTF(2, 3);
> >
> >  /* Set the child bus name. */
> >  void virtio_device_set_child_bus_name(VirtIODevice *vdev, char *bus_name);
> > diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> > index db787d00b3..e443f71c01 100644
> > --- a/hw/virtio/virtio-balloon.c
> > +++ b/hw/virtio/virtio-balloon.c
> > @@ -697,7 +697,7 @@ virtio_balloon_free_page_hint_notify(NotifierWithReturn 
> > *n, void *data,
> >      case PRECOPY_NOTIFY_COMPLETE:
> >          break;
> >      default:
> > -        virtio_error(vdev, "%s: %d reason unknown", __func__, pnd->reason);
> > +        virtio_error(vdev, "%d reason unknown", pnd->reason);
> >      }
> >
> >      return 0;
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index de89e8104a..0f33ca5d90 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -3968,7 +3968,8 @@ void virtio_device_set_child_bus_name(VirtIODevice 
> > *vdev, char *bus_name)
> >      vdev->bus_name = g_strdup(bus_name);
> >  }
> >
> > -void G_GNUC_PRINTF(2, 3) virtio_error(VirtIODevice *vdev, const char *fmt, 
> > ...)
> > +void G_GNUC_PRINTF(2, 3)
> > +virtio_error_impl(VirtIODevice *vdev, const char *fmt, ...)
> >  {
> >      va_list ap;
> >
> > --
> > MST
> >
> >
> 
> Hi Michael,
> 
> Thanks for picking this up.
> 
> It seems that the version currently queued is actually the initial
> submission, which had previously been rejected following feedback from
> Markus Armbruster.
> I later submitted a corrected version ([v3]) which:
> 
> * Addresses all the feedback (including from Markus Armbruster and
> Daniel P. Berrangé).
> * Has Daniel’s formal Reviewed-by:
> 
> Patch: 
> https://lore.kernel.org/qemu-devel/[email protected]/
> Reviewed-by: https://lore.kernel.org/qemu-devel/[email protected]/
> 
> Would you mind updating the queue to reflect this version instead?
> 
> Thanks again for your time and all the work you do maintaining virtio!
> 
> Best regards,
> Alessandro


Oh right. The weird thing is that I reviewed v3 but because it was
a reply when i applied it using b4 it actually picked up original one.
Not good! pls start a new thread each time.


Reply via email to