Alessandro Ratti <[email protected]> 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 <[email protected]>
