Re: [Qemu-devel] [RFC v3 2/3] qemu-error: Implement a more generic error reporting

[Alistair Francis]

On Thu, Jul 6, 2017 at 7:44 AM, Markus Armbruster <address@hidden> wrote:
> "Daniel P. Berrange" <address@hidden> writes:
>
>> On Thu, Jul 06, 2017 at 02:20:51PM +0200, Markus Armbruster wrote:
>>> "Daniel P. Berrange" <address@hidden> writes:
>>>
>>> > On Thu, Jul 06, 2017 at 01:27:15PM +0200, Markus Armbruster wrote:
>>> >> "Daniel P. Berrange" <address@hidden> writes:
> [...]
>>> >> > Do we really need to care about compatibility of the precise way we 
>>> >> > output
>>> >> > error messages. It has never been something we call a "stable API", as 
>>> >> > we
>>> >> > don't guarantee error message text will remain the same across 
>>> >> > releases. So
>>> >> > anyone relying on scraping QEMU stderr to match some error message has 
>>> >> > always
>>> >> > been liable to break.
>>> >> >
>>> >> > IOW, just add an "error: " prefix to the text
>>> >>
>>> >> I agree the error message format isn't ABI.
>>> >>
>>> >> But what would adding "error: " buy us?
>>> >
>>> > It would clearly distinguish errors from any other output on stderr, which
>>> > may not be error related (for example SPICE commonly pollutes stderr with
>>> > lots of messages).
>>>
>>> Changing the current error message format
>>>
>>>     <TIMESTAMP><PROGNAME>:<LOCATION><MSG>
>>>
>>> to
>>>
>>>     <TIMESTAMP><PROGNAME>:<LOCATION>error: <MSG>
>>>
>>> makes recognizing error messages a bit easier, but it also makes them
>>> even longer.  Can't we make do with recognizing <PROGNAME>:?
>>
>> I'm not convinced 7 extra characters is a big deal compared with the
>> size of the timestamps, program name, location & error message itself.
>> We'll already have such a prefix for info & warnings, so consistency is
>> good IMHO.
>
> I don't see the value of consistency here.  What matters is whether
> errors are easy to spot and recognize.
>
> GCC doesn't put "error:" into its error messages.  Clang does.  Feels
> like a matter of taste to me.
>
> If adding "error:" solves a problem people have, let's do it.  If not, I
> don't see why we should change what we have.

Ok, adding "error: " actually helps with what I was originally trying
to do, so I have added it in my next version.

I have also removed the qmsg_* prefix and updated the commit message.

Thanks,
Alistair

>
>> If line length is a concern, perhaps we should make the error printing
>> function able to intelligently line wrap at 80 chars, taking into account
>> the size of the metadata (timestamp, program, location, msg type prefix).
>
> Uh, that cure feels worse than the disease :)