* [Qemu-devel] [PATCH build-fix v1 1/1] error: Don't use error_report() for assertion msgs. @ 2014-01-15 2:29 Peter Crosthwaite 2014-01-15 2:55 ` Andreas Färber 2014-01-15 14:19 ` Luiz Capitulino 0 siblings, 2 replies; 6+ messages in thread From: Peter Crosthwaite @ 2014-01-15 2:29 UTC (permalink / raw) To: qemu-devel; +Cc: edgar.iglesias, kwolf, aliguori, lcapitulino Use fprintf(stderr instead. This removes dependency of libqemuutil.a on the monitor. We can further justify this change, in that this code path should only trigger under a fatal error condition. fprintf-stderr is probably the appropriate medium as under a fatal error conidition the monitor itself may be down and out for the count. So assertion failure messages should go lowest common denominator - straight to stderr. Fixes the build as reported by Kevin Wolf. Issue debugged and change suggested by Luiz Capitulino. Issue introduced by 5d24ee70bcbcf578614193526bcd5ed30a8eb16c. Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> --- util/error.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/util/error.c b/util/error.c index f11f1d5..7c7650c 100644 --- a/util/error.c +++ b/util/error.c @@ -44,7 +44,7 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...) err->err_class = err_class; if (errp == &error_abort) { - error_report("%s", error_get_pretty(err)); + fprintf(stderr, "%s", error_get_pretty(err)); abort(); } @@ -80,7 +80,7 @@ void error_set_errno(Error **errp, int os_errno, ErrorClass err_class, err->err_class = err_class; if (errp == &error_abort) { - error_report("%s", error_get_pretty(err)); + fprintf(stderr, "%s", error_get_pretty(err)); abort(); } @@ -125,7 +125,7 @@ void error_set_win32(Error **errp, int win32_err, ErrorClass err_class, err->err_class = err_class; if (errp == &error_abort) { - error_report("%s", error_get_pretty(err)); + fprintf(stderr, "%s", error_get_pretty(err)); abort(); } @@ -171,7 +171,7 @@ void error_free(Error *err) void error_propagate(Error **dst_err, Error *local_err) { if (local_err && dst_err == &error_abort) { - error_report("%s", error_get_pretty(local_err)); + fprintf(stderr, "%s", error_get_pretty(local_err)); abort(); } else if (dst_err && !*dst_err) { *dst_err = local_err; -- 1.8.5.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH build-fix v1 1/1] error: Don't use error_report() for assertion msgs. 2014-01-15 2:29 [Qemu-devel] [PATCH build-fix v1 1/1] error: Don't use error_report() for assertion msgs Peter Crosthwaite @ 2014-01-15 2:55 ` Andreas Färber 2014-01-15 3:31 ` Peter Crosthwaite 2014-01-15 10:01 ` Markus Armbruster 2014-01-15 14:19 ` Luiz Capitulino 1 sibling, 2 replies; 6+ messages in thread From: Andreas Färber @ 2014-01-15 2:55 UTC (permalink / raw) To: Peter Crosthwaite, qemu-devel Cc: edgar.iglesias, kwolf, aliguori, lcapitulino Am 15.01.2014 03:29, schrieb Peter Crosthwaite: > Use fprintf(stderr instead. This removes dependency of libqemuutil.a > on the monitor. > > We can further justify this change, in that this code path should only > trigger under a fatal error condition. fprintf-stderr is probably the > appropriate medium as under a fatal error conidition the monitor itself > may be down and out for the count. So assertion failure messages should > go lowest common denominator - straight to stderr. Actually I thought the point of error_report() was to add an appropriate prefix "qemu-system-foo: ..." to the error message and an optional timestamp, not to send it to the monitor... > > Fixes the build as reported by Kevin Wolf. Issue debugged and change > suggested by Luiz Capitulino. Issue introduced by > 5d24ee70bcbcf578614193526bcd5ed30a8eb16c. > > Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > --- > > util/error.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/util/error.c b/util/error.c > index f11f1d5..7c7650c 100644 > --- a/util/error.c > +++ b/util/error.c > @@ -44,7 +44,7 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...) > err->err_class = err_class; > > if (errp == &error_abort) { > - error_report("%s", error_get_pretty(err)); > + fprintf(stderr, "%s", error_get_pretty(err)); You need to add \n if you do this. Andreas > abort(); > } > > @@ -80,7 +80,7 @@ void error_set_errno(Error **errp, int os_errno, ErrorClass err_class, > err->err_class = err_class; > > if (errp == &error_abort) { > - error_report("%s", error_get_pretty(err)); > + fprintf(stderr, "%s", error_get_pretty(err)); > abort(); > } > > @@ -125,7 +125,7 @@ void error_set_win32(Error **errp, int win32_err, ErrorClass err_class, > err->err_class = err_class; > > if (errp == &error_abort) { > - error_report("%s", error_get_pretty(err)); > + fprintf(stderr, "%s", error_get_pretty(err)); > abort(); > } > > @@ -171,7 +171,7 @@ void error_free(Error *err) > void error_propagate(Error **dst_err, Error *local_err) > { > if (local_err && dst_err == &error_abort) { > - error_report("%s", error_get_pretty(local_err)); > + fprintf(stderr, "%s", error_get_pretty(local_err)); > abort(); > } else if (dst_err && !*dst_err) { > *dst_err = local_err; > -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH build-fix v1 1/1] error: Don't use error_report() for assertion msgs. 2014-01-15 2:55 ` Andreas Färber @ 2014-01-15 3:31 ` Peter Crosthwaite 2014-01-15 3:34 ` Peter Crosthwaite 2014-01-15 10:01 ` Markus Armbruster 1 sibling, 1 reply; 6+ messages in thread From: Peter Crosthwaite @ 2014-01-15 3:31 UTC (permalink / raw) To: Andreas Färber Cc: Edgar Iglesias, Kevin Wolf, qemu-devel@nongnu.org Developers, Anthony Liguori, Luiz Capitulino On Wed, Jan 15, 2014 at 12:55 PM, Andreas Färber <afaerber@suse.de> wrote: > Am 15.01.2014 03:29, schrieb Peter Crosthwaite: >> Use fprintf(stderr instead. This removes dependency of libqemuutil.a >> on the monitor. >> >> We can further justify this change, in that this code path should only >> trigger under a fatal error condition. fprintf-stderr is probably the >> appropriate medium as under a fatal error conidition the monitor itself >> may be down and out for the count. So assertion failure messages should >> go lowest common denominator - straight to stderr. > > Actually I thought the point of error_report() was to add an appropriate > prefix "qemu-system-foo: ..." to the error message and an optional > timestamp, not to send it to the monitor... > Well this patch routes it away from the monitor. Never implemented prefixing though. My main motivations were: 1: Text reduction. No need to define local_err and assert them constantly. 2: Full backtraceability of the error. Regards, Peter >> >> Fixes the build as reported by Kevin Wolf. Issue debugged and change >> suggested by Luiz Capitulino. Issue introduced by >> 5d24ee70bcbcf578614193526bcd5ed30a8eb16c. >> >> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >> --- >> >> util/error.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/util/error.c b/util/error.c >> index f11f1d5..7c7650c 100644 >> --- a/util/error.c >> +++ b/util/error.c >> @@ -44,7 +44,7 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...) >> err->err_class = err_class; >> >> if (errp == &error_abort) { >> - error_report("%s", error_get_pretty(err)); >> + fprintf(stderr, "%s", error_get_pretty(err)); > > You need to add \n if you do this. > > Andreas > >> abort(); >> } >> >> @@ -80,7 +80,7 @@ void error_set_errno(Error **errp, int os_errno, ErrorClass err_class, >> err->err_class = err_class; >> >> if (errp == &error_abort) { >> - error_report("%s", error_get_pretty(err)); >> + fprintf(stderr, "%s", error_get_pretty(err)); >> abort(); >> } >> >> @@ -125,7 +125,7 @@ void error_set_win32(Error **errp, int win32_err, ErrorClass err_class, >> err->err_class = err_class; >> >> if (errp == &error_abort) { >> - error_report("%s", error_get_pretty(err)); >> + fprintf(stderr, "%s", error_get_pretty(err)); >> abort(); >> } >> >> @@ -171,7 +171,7 @@ void error_free(Error *err) >> void error_propagate(Error **dst_err, Error *local_err) >> { >> if (local_err && dst_err == &error_abort) { >> - error_report("%s", error_get_pretty(local_err)); >> + fprintf(stderr, "%s", error_get_pretty(local_err)); >> abort(); >> } else if (dst_err && !*dst_err) { >> *dst_err = local_err; >> > > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH build-fix v1 1/1] error: Don't use error_report() for assertion msgs. 2014-01-15 3:31 ` Peter Crosthwaite @ 2014-01-15 3:34 ` Peter Crosthwaite 0 siblings, 0 replies; 6+ messages in thread From: Peter Crosthwaite @ 2014-01-15 3:34 UTC (permalink / raw) To: Andreas Färber Cc: Edgar Iglesias, Kevin Wolf, qemu-devel@nongnu.org Developers, Anthony Liguori, Luiz Capitulino On Wed, Jan 15, 2014 at 1:31 PM, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > On Wed, Jan 15, 2014 at 12:55 PM, Andreas Färber <afaerber@suse.de> wrote: >> Am 15.01.2014 03:29, schrieb Peter Crosthwaite: >>> Use fprintf(stderr instead. This removes dependency of libqemuutil.a >>> on the monitor. >>> >>> We can further justify this change, in that this code path should only >>> trigger under a fatal error condition. fprintf-stderr is probably the >>> appropriate medium as under a fatal error conidition the monitor itself >>> may be down and out for the count. So assertion failure messages should >>> go lowest common denominator - straight to stderr. >> >> Actually I thought the point of error_report() was to add an appropriate >> prefix "qemu-system-foo: ..." to the error message and an optional >> timestamp, not to send it to the monitor... >> > > Well this patch routes it away from the monitor. Never implemented > prefixing though. My main motivations were: > > 1: Text reduction. No need to define local_err and assert them constantly. > 2: Full backtraceability of the error. > Sry misread your mail, you were commenting error_report not error_abort as I read it if your wondering why my mail makes no sense at all. Sry for the noise :S > Regards, > Peter > >>> >>> Fixes the build as reported by Kevin Wolf. Issue debugged and change >>> suggested by Luiz Capitulino. Issue introduced by >>> 5d24ee70bcbcf578614193526bcd5ed30a8eb16c. >>> >>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >>> --- >>> >>> util/error.c | 8 ++++---- >>> 1 file changed, 4 insertions(+), 4 deletions(-) >>> >>> diff --git a/util/error.c b/util/error.c >>> index f11f1d5..7c7650c 100644 >>> --- a/util/error.c >>> +++ b/util/error.c >>> @@ -44,7 +44,7 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...) >>> err->err_class = err_class; >>> >>> if (errp == &error_abort) { >>> - error_report("%s", error_get_pretty(err)); >>> + fprintf(stderr, "%s", error_get_pretty(err)); >> >> You need to add \n if you do this. >> Fixed. V2 en-route. Regards, Peter >> Andreas >> >>> abort(); >>> } >>> >>> @@ -80,7 +80,7 @@ void error_set_errno(Error **errp, int os_errno, ErrorClass err_class, >>> err->err_class = err_class; >>> >>> if (errp == &error_abort) { >>> - error_report("%s", error_get_pretty(err)); >>> + fprintf(stderr, "%s", error_get_pretty(err)); >>> abort(); >>> } >>> >>> @@ -125,7 +125,7 @@ void error_set_win32(Error **errp, int win32_err, ErrorClass err_class, >>> err->err_class = err_class; >>> >>> if (errp == &error_abort) { >>> - error_report("%s", error_get_pretty(err)); >>> + fprintf(stderr, "%s", error_get_pretty(err)); >>> abort(); >>> } >>> >>> @@ -171,7 +171,7 @@ void error_free(Error *err) >>> void error_propagate(Error **dst_err, Error *local_err) >>> { >>> if (local_err && dst_err == &error_abort) { >>> - error_report("%s", error_get_pretty(local_err)); >>> + fprintf(stderr, "%s", error_get_pretty(local_err)); >>> abort(); >>> } else if (dst_err && !*dst_err) { >>> *dst_err = local_err; >>> >> >> >> -- >> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany >> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg >> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH build-fix v1 1/1] error: Don't use error_report() for assertion msgs. 2014-01-15 2:55 ` Andreas Färber 2014-01-15 3:31 ` Peter Crosthwaite @ 2014-01-15 10:01 ` Markus Armbruster 1 sibling, 0 replies; 6+ messages in thread From: Markus Armbruster @ 2014-01-15 10:01 UTC (permalink / raw) To: Andreas Färber Cc: edgar.iglesias, kwolf, Peter Crosthwaite, qemu-devel, lcapitulino, aliguori Andreas Färber <afaerber@suse.de> writes: > Am 15.01.2014 03:29, schrieb Peter Crosthwaite: >> Use fprintf(stderr instead. This removes dependency of libqemuutil.a >> on the monitor. >> >> We can further justify this change, in that this code path should only >> trigger under a fatal error condition. fprintf-stderr is probably the >> appropriate medium as under a fatal error conidition the monitor itself >> may be down and out for the count. So assertion failure messages should >> go lowest common denominator - straight to stderr. > > Actually I thought the point of error_report() was to add an appropriate > prefix "qemu-system-foo: ..." to the error message and an optional > timestamp, not to send it to the monitor... Exactly. Unwanted loss of functionality. Please try adding stubs/mon-set-error.o to whatever is missing cur_mon instead. >> Fixes the build as reported by Kevin Wolf. Issue debugged and change >> suggested by Luiz Capitulino. Issue introduced by >> 5d24ee70bcbcf578614193526bcd5ed30a8eb16c. >> >> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >> --- >> >> util/error.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/util/error.c b/util/error.c >> index f11f1d5..7c7650c 100644 >> --- a/util/error.c >> +++ b/util/error.c >> @@ -44,7 +44,7 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...) >> err->err_class = err_class; >> >> if (errp == &error_abort) { >> - error_report("%s", error_get_pretty(err)); >> + fprintf(stderr, "%s", error_get_pretty(err)); > > You need to add \n if you do this. Yup. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH build-fix v1 1/1] error: Don't use error_report() for assertion msgs. 2014-01-15 2:29 [Qemu-devel] [PATCH build-fix v1 1/1] error: Don't use error_report() for assertion msgs Peter Crosthwaite 2014-01-15 2:55 ` Andreas Färber @ 2014-01-15 14:19 ` Luiz Capitulino 1 sibling, 0 replies; 6+ messages in thread From: Luiz Capitulino @ 2014-01-15 14:19 UTC (permalink / raw) To: Peter Crosthwaite; +Cc: edgar.iglesias, kwolf, qemu-devel, aliguori On Tue, 14 Jan 2014 18:29:50 -0800 Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > Use fprintf(stderr instead. This removes dependency of libqemuutil.a > on the monitor. > > We can further justify this change, in that this code path should only > trigger under a fatal error condition. fprintf-stderr is probably the > appropriate medium as under a fatal error conidition the monitor itself > may be down and out for the count. So assertion failure messages should > go lowest common denominator - straight to stderr. > > Fixes the build as reported by Kevin Wolf. Issue debugged and change > suggested by Luiz Capitulino. Issue introduced by > 5d24ee70bcbcf578614193526bcd5ed30a8eb16c. Thanks for doing this Peter! I missed the prefix feature (well pointed by Markus), but at least we unblocked other people's testing and can fix it w/o rushing. > > Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > --- > > util/error.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/util/error.c b/util/error.c > index f11f1d5..7c7650c 100644 > --- a/util/error.c > +++ b/util/error.c > @@ -44,7 +44,7 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...) > err->err_class = err_class; > > if (errp == &error_abort) { > - error_report("%s", error_get_pretty(err)); > + fprintf(stderr, "%s", error_get_pretty(err)); > abort(); > } > > @@ -80,7 +80,7 @@ void error_set_errno(Error **errp, int os_errno, ErrorClass err_class, > err->err_class = err_class; > > if (errp == &error_abort) { > - error_report("%s", error_get_pretty(err)); > + fprintf(stderr, "%s", error_get_pretty(err)); > abort(); > } > > @@ -125,7 +125,7 @@ void error_set_win32(Error **errp, int win32_err, ErrorClass err_class, > err->err_class = err_class; > > if (errp == &error_abort) { > - error_report("%s", error_get_pretty(err)); > + fprintf(stderr, "%s", error_get_pretty(err)); > abort(); > } > > @@ -171,7 +171,7 @@ void error_free(Error *err) > void error_propagate(Error **dst_err, Error *local_err) > { > if (local_err && dst_err == &error_abort) { > - error_report("%s", error_get_pretty(local_err)); > + fprintf(stderr, "%s", error_get_pretty(local_err)); > abort(); > } else if (dst_err && !*dst_err) { > *dst_err = local_err; ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-01-15 14:19 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-01-15 2:29 [Qemu-devel] [PATCH build-fix v1 1/1] error: Don't use error_report() for assertion msgs Peter Crosthwaite 2014-01-15 2:55 ` Andreas Färber 2014-01-15 3:31 ` Peter Crosthwaite 2014-01-15 3:34 ` Peter Crosthwaite 2014-01-15 10:01 ` Markus Armbruster 2014-01-15 14:19 ` Luiz Capitulino
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).