Re: [Qemu-trivial] [Qemu-devel] [PATCH v6 04/29] hw/arm: Replace fprintf(stderr, "*\n" with error_report()

[Markus Armbruster <armbru@redhat.com>]

Alistair Francis <alistair.francis@xilinx.com> writes:

> On Fri, Dec 22, 2017 at 9:17 AM, Thomas Huth <thuth@redhat.com> wrote:
>> On 22.12.2017 16:37, Markus Armbruster wrote:
>>> Second thoughts...
>>>
>>> Alistair Francis <alistair.francis@xilinx.com> writes:
>> [...]
>>>>  #include "qemu/osdep.h"
>>>> +#include "qemu/error-report.h"
>>>>  #include "qapi/error.h"
>>>>  #include "qemu-common.h"
>>>>  #include "cpu.h"
>>>> @@ -1311,8 +1312,8 @@ static void omap_prcm_apll_update(struct omap_prcm_s *s)
>>>>      /* TODO: update clocks */
>>>>
>>>>      if (mode[0] == 1 || mode[0] == 2 || mode[1] == 1 || mode[1] == 2)
>>>> -        fprintf(stderr, "%s: bad EN_54M_PLL or bad EN_96M_PLL\n",
>>>> -                        __func__);
>>>> +        error_report("%s: bad EN_54M_PLL or bad EN_96M_PLL",
>>>> +                     __func__);
>>>>  }
>>>
>>> This one's different: we neither exit() nor return a "failed" status to
>>> the caller.
>>>
>>> We get here when the guest writes something funny to a certain
>>> memory-mapped I/O register.  In other words, it's guest misbehavior, not
>>> a user error.  I doubt it should be reported with error_report().
>>> Peter, do we have a canonical way to report or log  guest misbehavior?
>>
>> qemu_log_mask(LOG_GUEST_ERROR, ...) ?
>
> That seems like the best option to me.

Suggest:

1. Keep converting fatal errors (the ones that exit())

2. Keep converting recoverable errors (the ones that return failure)

3. You can leave the prints that are neither alone.  You can also
   convert to logging or tracing, as appropriate, but that requires
   understanding the code.

Makes sense?