Re: [Qemu-devel] [PATCH v3 07/15] vfio: Report warnings with warn_report(), not error_printf()

[Markus Armbruster <armbru@redhat.com>]

Alex Williamson <alex.williamson@redhat.com> writes:

> On Wed, 17 Apr 2019 21:06:33 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Cc: Alex Williamson <alex.williamson@redhat.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  hw/vfio/pci.c | 19 +++++++++++++------
>>  1 file changed, 13 insertions(+), 6 deletions(-)
>> 
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index 504019c458..0142819ea6 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -947,8 +947,10 @@ static void vfio_pci_size_rom(VFIOPCIDevice *vdev)
>>      if (vdev->pdev.romfile || !vdev->pdev.rom_bar) {
>>          /* Since pci handles romfile, just print a message and return */
>>          if (vfio_blacklist_opt_rom(vdev) && vdev->pdev.romfile) {
>> -            error_printf("Warning : Device at %s is known to cause system instability issues during option rom execution. Proceeding anyway since user specified romfile\n",
>> -                         vdev->vbasedev.name);
>> +            warn_report("Device at %s is known to cause system instability"
>> +                        " issues during option rom execution",
>> +                        vdev->vbasedev.name);
>> +            error_printf("Proceeding anyway since user specified romfile\n");
>
> I'm confused, the original warning is "this device is know to have
> issues, proceeding because you asked me to".  Are we categorizing the
> first half as a warning and the latter as random uncategorized error
> spew?  Did an automated script chunk it this way because of the period
> and strict application of the "single phrase" specification of
> warn_report()?  If this is the recommended semantics, I'm not sure how
> I'd know to generate this myself for similar situations.  Should we
> instead try to express this in something acceptable as a single
> phrase?  Thanks,

This is an instance of the following error reporting pattern:

    concise error / warning message (one line, single phrase)
    additional information (free format)

We use error_report() / warn_report() for the former (this adds
decorations to the message), and error_printf() for the latter.

"git-grep -w error_printf" will lead you to other instances.  Recommend
to look with this series applied, because it removes misuses of
error_printf().

Particularly relevant instances are error_report_err() and
warn_report_err(): these print the error proper with error_report() /
warn_report_err(), and the hint, if any, with error_printf().

Clearer now?