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

["Alex Bennée" <alex.bennee@linaro.org>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Mon, Sep 22, 2025 at 12:37:57PM +0200, Markus Armbruster wrote:
>> 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.
>
> I pretty much agree. If we take that view forwards, then I think our
> coding guidelines should explicitly state something like
>
>  "Function names must never be included in error messages.
>
>   The messages need to be sufficiently descriptive in their
>   text, such that including function names is redundant"

Ahh I missed the fact this ends up as an error_report. I think having
function names in debug output is fine.

It does however miss important information like which VirtIO device is
actually failing, despite having vdev passed down to the function.

>
>> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/230
>
> This is interesting as it shows a link to a previously proposed patch:
>
>   https://patchwork.kernel.org/project/qemu-devel/patch/20220414112902.41390-1-codeguy.moteen@gmail.com/
>
> this old patch just expanded the error messages to include 'Virtio '
> in their text. I'm not going to claim this made new error messages
> hugely user friendly, but I think that old patch approach was at
> least conceptually better & preferrable to the function name
> addition.
>
>> > Buglink: https://bugs.launchpad.net/qemu/+bug/1919021
>> >
>> > Signed-off-by: Alessandro Ratti <alessandro@0x65c.net>
>
> With regards,
> Daniel

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro