[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 07/15] vfio: Report warnings with warn_report
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v3 07/15] vfio: Report warnings with warn_report(), not error_printf() |
Date: |
Thu, 18 Apr 2019 22:25:52 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Alex Williamson <address@hidden> writes:
> On Thu, 18 Apr 2019 08:18:56 +0200
> Markus Armbruster <address@hidden> wrote:
>
>> Alex Williamson <address@hidden> writes:
>>
>> > On Wed, 17 Apr 2019 21:06:33 +0200
>> > Markus Armbruster <address@hidden> wrote:
>> >
>> >> Cc: Alex Williamson <address@hidden>
>> >> Signed-off-by: Markus Armbruster <address@hidden>
>> >> ---
>> >> 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?
>
> I can't guarantee that I'd be able to reproduce these sorts of
> semantics without prompting, but yes, there does seem to be some method
> to the madness ;) Thanks,
Hopefully, presence of good examples, absence of bad examples, and
review will do the trick often enough.
> Acked-by: Alex Williamson <address@hidden>
Thanks!
- [Qemu-devel] [PATCH v3 15/15] monitor: Simplify how -device/device_add print help, (continued)
- [Qemu-devel] [PATCH v3 15/15] monitor: Simplify how -device/device_add print help, Markus Armbruster, 2019/04/17
- [Qemu-devel] [PATCH v3 10/15] monitor error: Make printf()-like functions return a value, Markus Armbruster, 2019/04/17
- [Qemu-devel] [PATCH v3 03/15] loader-fit: Wean off error_printf(), Markus Armbruster, 2019/04/17
- [Qemu-devel] [PATCH v3 02/15] block/ssh: Do not report read/write/flush errors to the user, Markus Armbruster, 2019/04/17
- [Qemu-devel] [PATCH v3 11/15] qemu-print: New qemu_printf(), qemu_vprintf() etc., Markus Armbruster, 2019/04/17
- [Qemu-devel] [PATCH v3 13/15] char: Make -chardev help print to stdout, Markus Armbruster, 2019/04/17
- [Qemu-devel] [PATCH v3 07/15] vfio: Report warnings with warn_report(), not error_printf(), Markus Armbruster, 2019/04/17