"Daniel P. Berrange" <berra...@redhat.com> writes:
> On Thu, Jul 06, 2017 at 01:27:15PM +0200, Markus Armbruster wrote:
>> "Daniel P. Berrange" <berra...@redhat.com> writes:
>>
>> > On Thu, Jul 06, 2017 at 08:15:54AM +0200, Markus Armbruster wrote:
>> >> Alistair Francis <alistair.fran...@xilinx.com> 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 <alistair.fran...@xilinx.com>
>> >
>> >> > 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?
>
> 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>:?
If SPICE babbles to stderr, it needs a gag.
