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

[Markus Armbruster]

"Daniel P. Berrange" <address@hidden> writes:

> On Thu, Jul 06, 2017 at 08:15:54AM +0200, Markus Armbruster wrote:
>> Alistair Francis <address@hidden> writes:
>> 
>> > This patch converts the existing error_vreport() function into a generic
>> > qmesg_vreport() function that takes an enum describing the
>> > information to be reported.
>> >
>> > As part of this change a new qmesg_report() function is added as well with 
>> > the
>> > same capability.
>> >
>> > To maintain full compatibility the original error_report() function is
>> > maintained and no changes to the way errors are printed have been made.
>> > To improve access to the new informaiton and warning options wrapper 
>> > functions
>> > similar to error_report() have been added for warnings and information
>> > printing.
>> >
>> > Signed-off-by: Alistair Francis <address@hidden>
>
>> > diff --git a/util/qemu-error.c b/util/qemu-error.c
>> > index 1c5e35ecdb..63fdc0e174 100644
>> > --- a/util/qemu-error.c
>> > +++ b/util/qemu-error.c
>> > @@ -179,17 +179,29 @@ static void print_loc(void)
>> >  
>> >  bool enable_timestamp_msg;
>> >  /*
>> > - * Print an error message to current monitor if we have one, else to 
>> > stderr.
>> > + * Print a message to current monitor if we have one, else to stderr.
>> >   * Format arguments like vsprintf().  The resulting message should be
>> >   * a single phrase, with no newline or trailing punctuation.
>> >   * Prepend the current location and append a newline.
>> >   * It's wrong to call this in a QMP monitor.  Use error_setg() there.
>> >   */
>> > -void error_vreport(const char *fmt, va_list ap)
>> > +void qmsg_vreport(report_type type, const char *fmt, va_list ap)
>> >  {
>> >      GTimeVal tv;
>> >      gchar *timestr;
>> >  
>> > +    switch (type) {
>> > +    case REPORT_TYPE_ERROR:
>> > +        /* To maintain compatibility we don't add anything here */
>> 
>> I feel the comment isn't going to be useful in the future.  Let's drop
>> it.
>
> 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?