Re: [PATCH v2] virtio: Add function name to error messages

[Markus Armbruster <armbru@redhat.com>]

Alessandro Ratti <alessandro@0x65c.net> writes:

> 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.

A need for function names and such in error messages suggests the error
messages are crap.

Consider the example above.  From users' point of view, the message
changes from gobbledygook to more verbose gobbledygook.  Was the error
the user's fault?  The guest's?  Something else's?  What can the user do
about it now?  If you need a developer to answer such questions, the
user interface is *dire*.

Clue: virtio_error() sets vdev->broken to true.  Did the device just
stop working?  If yes, shouldn't we tell the user?

Note that __func__ does not materially improve things even for developer
when the error message template string is unique.  Almost all are.

Fun example: "Region caches not initialized".  Three instances:

hw/virtio/virtio.c:        virtio_error(vdev, "Region caches not initialized");
hw/virtio/virtio.c:        virtio_error(vdev, "Region caches not initialized");
hw/virtio/virtio.c:            error_setg(errp, "Region caches not initialized");

Your patch adds __func__ in two out of three cases.  I'm not asking you
to add it to the third case, I'm only mentioning this to illustrate the
depth of the error reporting swamp around here.

I'll shut up now :)

> 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
>
> Signed-off-by: Alessandro Ratti <alessandro@0x65c.net>