* [Qemu-devel] [RFC v1 0/3] Implement a warning_report function @ 2017-06-27 20:45 Alistair Francis 2017-06-27 20:45 ` [Qemu-devel] [RFC v1 1/3] util/qemu-error: Rename error_print_loc() to be more generic Alistair Francis ` (3 more replies) 0 siblings, 4 replies; 17+ messages in thread From: Alistair Francis @ 2017-06-27 20:45 UTC (permalink / raw) To: qemu-devel, armbru; +Cc: alistair.francis, alistair23 QEMU currently has a standard method to report errors with error_repot(). This ensure a sane and standard format when printing errors. This series is attempting to add the same functionality for warnings. At the moment only one error is being converted, I wanted to get the implementation nailed down a little bit before I started converting others. This patch renames error_print_loc() function to be more clear, but I didn't bother renaming the others. It seems silly to change error_printf() to error_warning_printf() and printf is already taken so I just left it as is. I also didn't change the current error output as that would probably break backwards compatibilty for anyone who is parsing logs. Alistair Francis (3): util/qemu-error: Rename error_print_loc() to be more generic util/qemu-error: Add a warning_report() function char-socket: Report TCP socket waiting as a warning chardev/char-socket.c | 2 +- include/qemu/error-report.h | 2 ++ util/qemu-error.c | 36 ++++++++++++++++++++++++++++++++++-- 3 files changed, 37 insertions(+), 3 deletions(-) -- 2.11.0 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [RFC v1 1/3] util/qemu-error: Rename error_print_loc() to be more generic 2017-06-27 20:45 [Qemu-devel] [RFC v1 0/3] Implement a warning_report function Alistair Francis @ 2017-06-27 20:45 ` Alistair Francis 2017-06-28 4:32 ` Philippe Mathieu-Daudé 2017-06-27 20:45 ` [Qemu-devel] [RFC v1 2/3] util/qemu-error: Add a warning_report() function Alistair Francis ` (2 subsequent siblings) 3 siblings, 1 reply; 17+ messages in thread From: Alistair Francis @ 2017-06-27 20:45 UTC (permalink / raw) To: qemu-devel, armbru; +Cc: alistair.francis, alistair23 Rename the error_print_loc() function in preperation for using it to print warnings as well. Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> --- util/qemu-error.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/util/qemu-error.c b/util/qemu-error.c index b331f8f4a4..1c5e35ecdb 100644 --- a/util/qemu-error.c +++ b/util/qemu-error.c @@ -146,7 +146,7 @@ const char *error_get_progname(void) /* * Print current location to current monitor if we have one, else to stderr. */ -static void error_print_loc(void) +static void print_loc(void) { const char *sep = ""; int i; @@ -197,7 +197,7 @@ void error_vreport(const char *fmt, va_list ap) g_free(timestr); } - error_print_loc(); + print_loc(); error_vprintf(fmt, ap); error_printf("\n"); } -- 2.11.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [RFC v1 1/3] util/qemu-error: Rename error_print_loc() to be more generic 2017-06-27 20:45 ` [Qemu-devel] [RFC v1 1/3] util/qemu-error: Rename error_print_loc() to be more generic Alistair Francis @ 2017-06-28 4:32 ` Philippe Mathieu-Daudé 0 siblings, 0 replies; 17+ messages in thread From: Philippe Mathieu-Daudé @ 2017-06-28 4:32 UTC (permalink / raw) To: Alistair Francis Cc: qemu-devel@nongnu.org Developers, Markus Armbruster, Alistair Francis On Tue, Jun 27, 2017 at 5:45 PM, Alistair Francis <alistair.francis@xilinx.com> wrote: > Rename the error_print_loc() function in preperation for using it to preparation > print warnings as well. > > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > > util/qemu-error.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/util/qemu-error.c b/util/qemu-error.c > index b331f8f4a4..1c5e35ecdb 100644 > --- a/util/qemu-error.c > +++ b/util/qemu-error.c > @@ -146,7 +146,7 @@ const char *error_get_progname(void) > /* > * Print current location to current monitor if we have one, else to stderr. > */ > -static void error_print_loc(void) > +static void print_loc(void) > { > const char *sep = ""; > int i; > @@ -197,7 +197,7 @@ void error_vreport(const char *fmt, va_list ap) > g_free(timestr); > } > > - error_print_loc(); > + print_loc(); > error_vprintf(fmt, ap); > error_printf("\n"); > } > -- > 2.11.0 > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [RFC v1 2/3] util/qemu-error: Add a warning_report() function 2017-06-27 20:45 [Qemu-devel] [RFC v1 0/3] Implement a warning_report function Alistair Francis 2017-06-27 20:45 ` [Qemu-devel] [RFC v1 1/3] util/qemu-error: Rename error_print_loc() to be more generic Alistair Francis @ 2017-06-27 20:45 ` Alistair Francis 2017-06-27 22:21 ` Thomas Huth 2017-06-28 9:04 ` Daniel P. Berrange 2017-06-27 20:45 ` [Qemu-devel] [RFC v1 3/3] char-socket: Report TCP socket waiting as a warning Alistair Francis 2017-06-28 9:07 ` [Qemu-devel] [RFC v1 0/3] Implement a warning_report function Daniel P. Berrange 3 siblings, 2 replies; 17+ messages in thread From: Alistair Francis @ 2017-06-27 20:45 UTC (permalink / raw) To: qemu-devel, armbru; +Cc: alistair.francis, alistair23 Add a functino which can be used similarly to error_report() execpt to inform the users about warnings instead of errors. The warning print does not include the timestamp and instead will preface the messages with a 'warning: '. Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> --- include/qemu/error-report.h | 2 ++ util/qemu-error.c | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h index 3001865896..c2600d2298 100644 --- a/include/qemu/error-report.h +++ b/include/qemu/error-report.h @@ -36,7 +36,9 @@ void error_vprintf_unless_qmp(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); void error_printf_unless_qmp(const char *fmt, ...) GCC_FMT_ATTR(1, 2); void error_set_progname(const char *argv0); void error_vreport(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); +void warning_vreport(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); void error_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2); +void warning_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2); const char *error_get_progname(void); extern bool enable_timestamp_msg; diff --git a/util/qemu-error.c b/util/qemu-error.c index 1c5e35ecdb..2edd752fec 100644 --- a/util/qemu-error.c +++ b/util/qemu-error.c @@ -203,6 +203,23 @@ void error_vreport(const char *fmt, va_list ap) } /* + * Print a warning message ot the current monitor if we have one, else to + * stderr. This follows similar formating and use cases as error_vreport() + * except for these two differentce: + * - It prefixes the message with 'warning: ' to indicate it is only a + * warning. + * - It does not print the timestamp. + */ +void warning_vreport(const char *fmt, va_list ap) +{ + error_vprintf("warning: ", ap); + + print_loc(); + error_vprintf(fmt, ap); + error_printf("\n"); +} + +/* * Print an error message to current monitor if we have one, else to stderr. * Format arguments like sprintf(). The resulting message should be a * single phrase, with no newline or trailing punctuation. @@ -217,3 +234,18 @@ void error_report(const char *fmt, ...) error_vreport(fmt, ap); va_end(ap); } + +/* + * Print an warning message to current monitor if we have one, else to stderr. + * This follows the same formating and use cases as error_report() + * except it prefixes the message with 'warning: ' to indicate it is only a + * warning. + */ +void warning_report(const char *fmt, ...) +{ + va_list ap; + + va_start(ap, fmt); + warning_vreport(fmt, ap); + va_end(ap); +} -- 2.11.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [RFC v1 2/3] util/qemu-error: Add a warning_report() function 2017-06-27 20:45 ` [Qemu-devel] [RFC v1 2/3] util/qemu-error: Add a warning_report() function Alistair Francis @ 2017-06-27 22:21 ` Thomas Huth 2017-06-28 0:16 ` Alistair Francis 2017-06-28 9:04 ` Daniel P. Berrange 1 sibling, 1 reply; 17+ messages in thread From: Thomas Huth @ 2017-06-27 22:21 UTC (permalink / raw) To: Alistair Francis, qemu-devel, armbru; +Cc: alistair23 On 27.06.2017 22:45, Alistair Francis wrote: > Add a functino which can be used similarly to error_report() execpt to > inform the users about warnings instead of errors. s/functino/function/ s/execpt/except/ And could we maybe call the function "warn_report" or something else instead? I often already have trouble with error_report() to fit the string into one line of the scarce 80 columns space ... so the shorter the name of the function, the more characters can be squeezed into the string in one line later ;-) Thomas ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [RFC v1 2/3] util/qemu-error: Add a warning_report() function 2017-06-27 22:21 ` Thomas Huth @ 2017-06-28 0:16 ` Alistair Francis 0 siblings, 0 replies; 17+ messages in thread From: Alistair Francis @ 2017-06-28 0:16 UTC (permalink / raw) To: Thomas Huth Cc: Alistair Francis, qemu-devel@nongnu.org Developers, Markus Armbruster On Tue, Jun 27, 2017 at 3:21 PM, Thomas Huth <thuth@redhat.com> wrote: > On 27.06.2017 22:45, Alistair Francis wrote: >> Add a functino which can be used similarly to error_report() execpt to >> inform the users about warnings instead of errors. > > s/functino/function/ > s/execpt/except/ Thanks, I'll fix these two in the next version. > > And could we maybe call the function "warn_report" or something else > instead? I often already have trouble with error_report() to fit the > string into one line of the scarce 80 columns space ... so the shorter > the name of the function, the more characters can be squeezed into the > string in one line later ;-) Good idea, I'll change it to warn_report(). Thanks, Alistair > > Thomas ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [RFC v1 2/3] util/qemu-error: Add a warning_report() function 2017-06-27 20:45 ` [Qemu-devel] [RFC v1 2/3] util/qemu-error: Add a warning_report() function Alistair Francis 2017-06-27 22:21 ` Thomas Huth @ 2017-06-28 9:04 ` Daniel P. Berrange 2017-06-28 16:16 ` Alistair Francis 1 sibling, 1 reply; 17+ messages in thread From: Daniel P. Berrange @ 2017-06-28 9:04 UTC (permalink / raw) To: Alistair Francis; +Cc: qemu-devel, armbru, alistair23 On Tue, Jun 27, 2017 at 01:45:45PM -0700, Alistair Francis wrote: > Add a functino which can be used similarly to error_report() execpt to > inform the users about warnings instead of errors. > > The warning print does not include the timestamp and instead will > preface the messages with a 'warning: '. Not including the timestamp is a bug IMHO. If I've turned on timestamps, I expect all messages to have the timestamp. I'm not particularly convinced by adding the 'warning: ' prefix either, particularly given the scenario you are using this in, is not actually a warning - its just a informative message. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [RFC v1 2/3] util/qemu-error: Add a warning_report() function 2017-06-28 9:04 ` Daniel P. Berrange @ 2017-06-28 16:16 ` Alistair Francis 2017-06-28 16:19 ` Daniel P. Berrange 0 siblings, 1 reply; 17+ messages in thread From: Alistair Francis @ 2017-06-28 16:16 UTC (permalink / raw) To: Daniel P. Berrange Cc: Alistair Francis, qemu-devel@nongnu.org Developers, Markus Armbruster On Wed, Jun 28, 2017 at 2:04 AM, Daniel P. Berrange <berrange@redhat.com> wrote: > On Tue, Jun 27, 2017 at 01:45:45PM -0700, Alistair Francis wrote: >> Add a functino which can be used similarly to error_report() execpt to >> inform the users about warnings instead of errors. >> >> The warning print does not include the timestamp and instead will >> preface the messages with a 'warning: '. > > Not including the timestamp is a bug IMHO. If I've turned on timestamps, > I expect all messages to have the timestamp. That's fine, I'm happy to add it back in. I just wasn't sure. > > I'm not particularly convinced by adding the 'warning: ' prefix either, > particularly given the scenario you are using this in, is not actually > a warning - its just a informative message. Maybe it makes more sense to add an extra argument to error_report() that can be used to specify error, warning or information. The same way qemu_log_mask() works. That was Edgar's idea in reply to one of the other patches. Does that sound more useful? Thanks, Alistair > > > Regards, > Daniel > -- > |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [RFC v1 2/3] util/qemu-error: Add a warning_report() function 2017-06-28 16:16 ` Alistair Francis @ 2017-06-28 16:19 ` Daniel P. Berrange 2017-06-29 7:38 ` Markus Armbruster 0 siblings, 1 reply; 17+ messages in thread From: Daniel P. Berrange @ 2017-06-28 16:19 UTC (permalink / raw) To: Alistair Francis; +Cc: qemu-devel@nongnu.org Developers, Markus Armbruster On Wed, Jun 28, 2017 at 09:16:45AM -0700, Alistair Francis wrote: > On Wed, Jun 28, 2017 at 2:04 AM, Daniel P. Berrange <berrange@redhat.com> wrote: > > On Tue, Jun 27, 2017 at 01:45:45PM -0700, Alistair Francis wrote: > >> Add a functino which can be used similarly to error_report() execpt to > >> inform the users about warnings instead of errors. > >> > >> The warning print does not include the timestamp and instead will > >> preface the messages with a 'warning: '. > > > > Not including the timestamp is a bug IMHO. If I've turned on timestamps, > > I expect all messages to have the timestamp. > > That's fine, I'm happy to add it back in. I just wasn't sure. > > > > > I'm not particularly convinced by adding the 'warning: ' prefix either, > > particularly given the scenario you are using this in, is not actually > > a warning - its just a informative message. > > Maybe it makes more sense to add an extra argument to error_report() > that can be used to specify error, warning or information. The same > way qemu_log_mask() works. That was Edgar's idea in reply to one of > the other patches. > > Does that sound more useful? I'd suggest renaming the current 'error_report' to 'message_report' and making it take an extra arg that accepts a enum flag INFO | WARNING | ERROR. Then add macros for error_report, warning_report, info_report that call message_report with the right enum. That way you don't have to update any of the existing code that calls error_report. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [RFC v1 2/3] util/qemu-error: Add a warning_report() function 2017-06-28 16:19 ` Daniel P. Berrange @ 2017-06-29 7:38 ` Markus Armbruster 0 siblings, 0 replies; 17+ messages in thread From: Markus Armbruster @ 2017-06-29 7:38 UTC (permalink / raw) To: Daniel P. Berrange; +Cc: Alistair Francis, qemu-devel@nongnu.org Developers "Daniel P. Berrange" <berrange@redhat.com> writes: > On Wed, Jun 28, 2017 at 09:16:45AM -0700, Alistair Francis wrote: >> On Wed, Jun 28, 2017 at 2:04 AM, Daniel P. Berrange <berrange@redhat.com> wrote: >> > On Tue, Jun 27, 2017 at 01:45:45PM -0700, Alistair Francis wrote: >> >> Add a functino which can be used similarly to error_report() execpt to >> >> inform the users about warnings instead of errors. >> >> >> >> The warning print does not include the timestamp and instead will >> >> preface the messages with a 'warning: '. >> > >> > Not including the timestamp is a bug IMHO. If I've turned on timestamps, >> > I expect all messages to have the timestamp. >> >> That's fine, I'm happy to add it back in. I just wasn't sure. >> >> > >> > I'm not particularly convinced by adding the 'warning: ' prefix either, >> > particularly given the scenario you are using this in, is not actually >> > a warning - its just a informative message. >> >> Maybe it makes more sense to add an extra argument to error_report() >> that can be used to specify error, warning or information. The same >> way qemu_log_mask() works. That was Edgar's idea in reply to one of >> the other patches. >> >> Does that sound more useful? > > I'd suggest renaming the current 'error_report' to 'message_report' and > making it take an extra arg that accepts a enum flag INFO | WARNING | ERROR. > Then add macros for error_report, warning_report, info_report that call > message_report with the right enum. That way you don't have to update any > of the existing code that calls error_report. *Functions*, please, not macros. Macros would bloat the code for no benefit at all. ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [RFC v1 3/3] char-socket: Report TCP socket waiting as a warning 2017-06-27 20:45 [Qemu-devel] [RFC v1 0/3] Implement a warning_report function Alistair Francis 2017-06-27 20:45 ` [Qemu-devel] [RFC v1 1/3] util/qemu-error: Rename error_print_loc() to be more generic Alistair Francis 2017-06-27 20:45 ` [Qemu-devel] [RFC v1 2/3] util/qemu-error: Add a warning_report() function Alistair Francis @ 2017-06-27 20:45 ` Alistair Francis 2017-06-27 21:10 ` Edgar E. Iglesias 2017-06-28 9:07 ` [Qemu-devel] [RFC v1 0/3] Implement a warning_report function Daniel P. Berrange 3 siblings, 1 reply; 17+ messages in thread From: Alistair Francis @ 2017-06-27 20:45 UTC (permalink / raw) To: qemu-devel, armbru; +Cc: alistair.francis, alistair23 When QEMU is waiting for a TCP socket connection it reports that message as an error. This isn't an error it is a warnign so let's change the report to use warning_report() instead. Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> --- chardev/char-socket.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chardev/char-socket.c b/chardev/char-socket.c index ccc499cfa1..5a56628e74 100644 --- a/chardev/char-socket.c +++ b/chardev/char-socket.c @@ -765,7 +765,7 @@ static int tcp_chr_wait_connected(Chardev *chr, Error **errp) * in TLS and telnet cases, only wait for an accepted socket */ while (!s->ioc) { if (s->is_listen) { - error_report("QEMU waiting for connection on: %s", + warning_report("QEMU waiting for connection on: %s", chr->filename); qio_channel_set_blocking(QIO_CHANNEL(s->listen_ioc), true, NULL); tcp_chr_accept(QIO_CHANNEL(s->listen_ioc), G_IO_IN, chr); -- 2.11.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [RFC v1 3/3] char-socket: Report TCP socket waiting as a warning 2017-06-27 20:45 ` [Qemu-devel] [RFC v1 3/3] char-socket: Report TCP socket waiting as a warning Alistair Francis @ 2017-06-27 21:10 ` Edgar E. Iglesias 2017-06-28 0:17 ` Alistair Francis 0 siblings, 1 reply; 17+ messages in thread From: Edgar E. Iglesias @ 2017-06-27 21:10 UTC (permalink / raw) To: Alistair Francis; +Cc: qemu-devel, armbru, alistair23 On Tue, Jun 27, 2017 at 01:45:48PM -0700, Alistair Francis wrote: > When QEMU is waiting for a TCP socket connection it reports that message as > an error. This isn't an error it is a warnign so let's change the report to > use warning_report() instead. Hi Alistair, Isn't this more like an informational message rather than a warning? A warning would indicate to me that something unexpected and perhaps wrong happened. In this case, there's nothing wrong going on. We may need more classes, like 'info:' and/or maybe others... Cheers, Edgar > > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> > --- > > chardev/char-socket.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/chardev/char-socket.c b/chardev/char-socket.c > index ccc499cfa1..5a56628e74 100644 > --- a/chardev/char-socket.c > +++ b/chardev/char-socket.c > @@ -765,7 +765,7 @@ static int tcp_chr_wait_connected(Chardev *chr, Error **errp) > * in TLS and telnet cases, only wait for an accepted socket */ > while (!s->ioc) { > if (s->is_listen) { > - error_report("QEMU waiting for connection on: %s", > + warning_report("QEMU waiting for connection on: %s", > chr->filename); > qio_channel_set_blocking(QIO_CHANNEL(s->listen_ioc), true, NULL); > tcp_chr_accept(QIO_CHANNEL(s->listen_ioc), G_IO_IN, chr); > -- > 2.11.0 > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [RFC v1 3/3] char-socket: Report TCP socket waiting as a warning 2017-06-27 21:10 ` Edgar E. Iglesias @ 2017-06-28 0:17 ` Alistair Francis 2017-06-28 6:15 ` Edgar E. Iglesias 0 siblings, 1 reply; 17+ messages in thread From: Alistair Francis @ 2017-06-28 0:17 UTC (permalink / raw) To: Edgar E. Iglesias Cc: Alistair Francis, qemu-devel@nongnu.org Developers, Markus Armbruster On Tue, Jun 27, 2017 at 2:10 PM, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > On Tue, Jun 27, 2017 at 01:45:48PM -0700, Alistair Francis wrote: >> When QEMU is waiting for a TCP socket connection it reports that message as >> an error. This isn't an error it is a warnign so let's change the report to >> use warning_report() instead. > > Hi Alistair, > > Isn't this more like an informational message rather than a warning? > > A warning would indicate to me that something unexpected and perhaps wrong happened. > In this case, there's nothing wrong going on. > > We may need more classes, like 'info:' and/or maybe others... Hey Edgar, That is a good point. I can add a info_report() as well then that copies the warning_report() function but prefixes with 'info: '. Thanks, Alistair > > Cheers, > Edgar > > > >> >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> >> --- >> >> chardev/char-socket.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/chardev/char-socket.c b/chardev/char-socket.c >> index ccc499cfa1..5a56628e74 100644 >> --- a/chardev/char-socket.c >> +++ b/chardev/char-socket.c >> @@ -765,7 +765,7 @@ static int tcp_chr_wait_connected(Chardev *chr, Error **errp) >> * in TLS and telnet cases, only wait for an accepted socket */ >> while (!s->ioc) { >> if (s->is_listen) { >> - error_report("QEMU waiting for connection on: %s", >> + warning_report("QEMU waiting for connection on: %s", >> chr->filename); >> qio_channel_set_blocking(QIO_CHANNEL(s->listen_ioc), true, NULL); >> tcp_chr_accept(QIO_CHANNEL(s->listen_ioc), G_IO_IN, chr); >> -- >> 2.11.0 >> >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [RFC v1 3/3] char-socket: Report TCP socket waiting as a warning 2017-06-28 0:17 ` Alistair Francis @ 2017-06-28 6:15 ` Edgar E. Iglesias 2017-06-28 16:18 ` Alistair Francis 0 siblings, 1 reply; 17+ messages in thread From: Edgar E. Iglesias @ 2017-06-28 6:15 UTC (permalink / raw) To: Alistair Francis; +Cc: qemu-devel@nongnu.org Developers, Markus Armbruster On Tue, Jun 27, 2017 at 05:17:58PM -0700, Alistair Francis wrote: > On Tue, Jun 27, 2017 at 2:10 PM, Edgar E. Iglesias > <edgar.iglesias@gmail.com> wrote: > > On Tue, Jun 27, 2017 at 01:45:48PM -0700, Alistair Francis wrote: > >> When QEMU is waiting for a TCP socket connection it reports that message as > >> an error. This isn't an error it is a warnign so let's change the report to > >> use warning_report() instead. > > > > Hi Alistair, > > > > Isn't this more like an informational message rather than a warning? > > > > A warning would indicate to me that something unexpected and perhaps wrong happened. > > In this case, there's nothing wrong going on. > > > > We may need more classes, like 'info:' and/or maybe others... > > Hey Edgar, > > That is a good point. I can add a info_report() as well then that > copies the warning_report() function but prefixes with 'info: '. Hi, Yes, or another way is to pass the class of message as an argument, e.g like qemu_log_mask does it. Best regards, Edgar > > Thanks, > Alistair > > > > > Cheers, > > Edgar > > > > > > > >> > >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> > >> --- > >> > >> chardev/char-socket.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/chardev/char-socket.c b/chardev/char-socket.c > >> index ccc499cfa1..5a56628e74 100644 > >> --- a/chardev/char-socket.c > >> +++ b/chardev/char-socket.c > >> @@ -765,7 +765,7 @@ static int tcp_chr_wait_connected(Chardev *chr, Error **errp) > >> * in TLS and telnet cases, only wait for an accepted socket */ > >> while (!s->ioc) { > >> if (s->is_listen) { > >> - error_report("QEMU waiting for connection on: %s", > >> + warning_report("QEMU waiting for connection on: %s", > >> chr->filename); > >> qio_channel_set_blocking(QIO_CHANNEL(s->listen_ioc), true, NULL); > >> tcp_chr_accept(QIO_CHANNEL(s->listen_ioc), G_IO_IN, chr); > >> -- > >> 2.11.0 > >> > >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [RFC v1 3/3] char-socket: Report TCP socket waiting as a warning 2017-06-28 6:15 ` Edgar E. Iglesias @ 2017-06-28 16:18 ` Alistair Francis 0 siblings, 0 replies; 17+ messages in thread From: Alistair Francis @ 2017-06-28 16:18 UTC (permalink / raw) To: Edgar E. Iglesias Cc: Alistair Francis, qemu-devel@nongnu.org Developers, Markus Armbruster On Tue, Jun 27, 2017 at 11:15 PM, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > On Tue, Jun 27, 2017 at 05:17:58PM -0700, Alistair Francis wrote: >> On Tue, Jun 27, 2017 at 2:10 PM, Edgar E. Iglesias >> <edgar.iglesias@gmail.com> wrote: >> > On Tue, Jun 27, 2017 at 01:45:48PM -0700, Alistair Francis wrote: >> >> When QEMU is waiting for a TCP socket connection it reports that message as >> >> an error. This isn't an error it is a warnign so let's change the report to >> >> use warning_report() instead. >> > >> > Hi Alistair, >> > >> > Isn't this more like an informational message rather than a warning? >> > >> > A warning would indicate to me that something unexpected and perhaps wrong happened. >> > In this case, there's nothing wrong going on. >> > >> > We may need more classes, like 'info:' and/or maybe others... >> >> Hey Edgar, >> >> That is a good point. I can add a info_report() as well then that >> copies the warning_report() function but prefixes with 'info: '. > > > Hi, > > Yes, or another way is to pass the class of message as an argument, > e.g like qemu_log_mask does it. Ah! That is a good idea. Something like this: report(ERROR, "message, ...) report(WARN, "message, ...) report(INFO, "message, ...) which gets converted to adding prefixes to the message. Thanks, Alistair > > Best regards, > Edgar > > > > >> >> Thanks, >> Alistair >> >> > >> > Cheers, >> > Edgar >> > >> > >> > >> >> >> >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> >> >> --- >> >> >> >> chardev/char-socket.c | 2 +- >> >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> >> >> diff --git a/chardev/char-socket.c b/chardev/char-socket.c >> >> index ccc499cfa1..5a56628e74 100644 >> >> --- a/chardev/char-socket.c >> >> +++ b/chardev/char-socket.c >> >> @@ -765,7 +765,7 @@ static int tcp_chr_wait_connected(Chardev *chr, Error **errp) >> >> * in TLS and telnet cases, only wait for an accepted socket */ >> >> while (!s->ioc) { >> >> if (s->is_listen) { >> >> - error_report("QEMU waiting for connection on: %s", >> >> + warning_report("QEMU waiting for connection on: %s", >> >> chr->filename); >> >> qio_channel_set_blocking(QIO_CHANNEL(s->listen_ioc), true, NULL); >> >> tcp_chr_accept(QIO_CHANNEL(s->listen_ioc), G_IO_IN, chr); >> >> -- >> >> 2.11.0 >> >> >> >> > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [RFC v1 0/3] Implement a warning_report function 2017-06-27 20:45 [Qemu-devel] [RFC v1 0/3] Implement a warning_report function Alistair Francis ` (2 preceding siblings ...) 2017-06-27 20:45 ` [Qemu-devel] [RFC v1 3/3] char-socket: Report TCP socket waiting as a warning Alistair Francis @ 2017-06-28 9:07 ` Daniel P. Berrange 2017-06-28 15:49 ` Alistair Francis 3 siblings, 1 reply; 17+ messages in thread From: Daniel P. Berrange @ 2017-06-28 9:07 UTC (permalink / raw) To: Alistair Francis; +Cc: qemu-devel, armbru, alistair23 On Tue, Jun 27, 2017 at 01:45:41PM -0700, Alistair Francis wrote: > QEMU currently has a standard method to report errors with > error_repot(). This ensure a sane and standard format when printing > errors. This series is attempting to add the same functionality for > warnings. I'm not seeing the obvious benefit in this change. Despite its name 'error_report' is ultimately just a clever way to run printf() on a text string. It can be used for errors, warnings, information alike. If anything the current method is simply misnamed, and could be changed to 'message_report'. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [RFC v1 0/3] Implement a warning_report function 2017-06-28 9:07 ` [Qemu-devel] [RFC v1 0/3] Implement a warning_report function Daniel P. Berrange @ 2017-06-28 15:49 ` Alistair Francis 0 siblings, 0 replies; 17+ messages in thread From: Alistair Francis @ 2017-06-28 15:49 UTC (permalink / raw) To: Daniel P. Berrange Cc: Alistair Francis, qemu-devel@nongnu.org Developers, Markus Armbruster On Wed, Jun 28, 2017 at 2:07 AM, Daniel P. Berrange <berrange@redhat.com> wrote: > On Tue, Jun 27, 2017 at 01:45:41PM -0700, Alistair Francis wrote: >> QEMU currently has a standard method to report errors with >> error_repot(). This ensure a sane and standard format when printing >> errors. This series is attempting to add the same functionality for >> warnings. > > I'm not seeing the obvious benefit in this change. Despite its name > 'error_report' is ultimately just a clever way to run printf() > on a text string. It can be used for errors, warnings, information > alike. If anything the current method is simply misnamed, and could > be changed to 'message_report'. I agree that this new method results in the same information being printed to the same place as error_report(). What I am trying to achieve though is a consistent structure for messages. So that every error, warning and information message that QEMU prints looks the same and are all obvious. The final end goal is that when including QEMU in other systems (like an Eclipse GUI) all of the different messages can be clearly understood and automatically parsed. The problem I think we have now is that some messages just showing information to the user (like in patch 3) are treated the same as errors, when they really are not errors. I have seen a lot of confusion caused by QEMU printing errors and information to stderr with no obvious way to differentiate which is which. Thanks, Alistair > > Regards, > Daniel > -- > |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2017-06-29 7:39 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-06-27 20:45 [Qemu-devel] [RFC v1 0/3] Implement a warning_report function Alistair Francis 2017-06-27 20:45 ` [Qemu-devel] [RFC v1 1/3] util/qemu-error: Rename error_print_loc() to be more generic Alistair Francis 2017-06-28 4:32 ` Philippe Mathieu-Daudé 2017-06-27 20:45 ` [Qemu-devel] [RFC v1 2/3] util/qemu-error: Add a warning_report() function Alistair Francis 2017-06-27 22:21 ` Thomas Huth 2017-06-28 0:16 ` Alistair Francis 2017-06-28 9:04 ` Daniel P. Berrange 2017-06-28 16:16 ` Alistair Francis 2017-06-28 16:19 ` Daniel P. Berrange 2017-06-29 7:38 ` Markus Armbruster 2017-06-27 20:45 ` [Qemu-devel] [RFC v1 3/3] char-socket: Report TCP socket waiting as a warning Alistair Francis 2017-06-27 21:10 ` Edgar E. Iglesias 2017-06-28 0:17 ` Alistair Francis 2017-06-28 6:15 ` Edgar E. Iglesias 2017-06-28 16:18 ` Alistair Francis 2017-06-28 9:07 ` [Qemu-devel] [RFC v1 0/3] Implement a warning_report function Daniel P. Berrange 2017-06-28 15:49 ` Alistair Francis
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.