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

Reply via email to