| Message ID | CAJ+F1CLuPkWjG3Hn8hfaxTApn6+ouqh5nJZNWSQYX1C3_HRFqA@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/06/2017 08:58 AM, Marc-André Lureau wrote: > Hi > > On Mon, Jun 5, 2017 at 10:37 PM Alistair Francis < > alistair.francis@xilinx.com> wrote: > >> When QEMU is waiting for a TCP socket connection it reports that message as >> an error. This isn't an error though, so let's change the report to just >> use qemu_log(). >> >> > I don't think this is a good idea, since stdout output my be mixed with > console or other expected output. > > In fact, it used to be on stdout, and got moved to stderr: This is somehow confusing. I don't think it is worth having another qemu_log_stderr() function rather than using error_report() but this very call might deserve a comment explaining this unusual use. What do you think? > > commit fdca2124adc293f84f2b7aaf0df43faa6b6bf420 > Author: Gerd Hoffmann <kraxel@redhat.com> > Date: Mon Jun 24 08:39:49 2013 +0200 > > qemu-char: print notification to stderr > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> > > diff --git a/qemu-char.c b/qemu-char.c > index e3b3224886..371f6308c5 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -2666,8 +2666,8 @@ static CharDriverState *qemu_chr_open_socket_fd(int > fd, bool do_nodelay, > } > > if (is_listen && is_waitconnect) { > - printf("QEMU waiting for connection on: %s\n", > - chr->filename); > + fprintf(stderr, "QEMU waiting for connection on: %s\n", > + chr->filename); > > > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> >> --- >> >> chardev/char-socket.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/chardev/char-socket.c b/chardev/char-socket.c >> index ccc499cfa1..a9884fa85b 100644 >> --- a/chardev/char-socket.c >> +++ b/chardev/char-socket.c >> @@ -27,6 +27,7 @@ >> #include "io/channel-tls.h" >> #include "qemu/error-report.h" >> #include "qapi/error.h" >> +#include "qemu/log.h" >> #include "qapi/clone-visitor.h" >> >> #include "chardev/char-io.h" >> @@ -765,7 +766,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", >> + qemu_log("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 >> >> >> -- > Marc-André Lureau >
On Tue, Jun 6, 2017 at 6:37 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > On 06/06/2017 08:58 AM, Marc-André Lureau wrote: >> >> Hi >> >> On Mon, Jun 5, 2017 at 10:37 PM Alistair Francis < >> alistair.francis@xilinx.com> wrote: >> >>> When QEMU is waiting for a TCP socket connection it reports that message >>> as >>> an error. This isn't an error though, so let's change the report to just >>> use qemu_log(). >>> >>> >> I don't think this is a good idea, since stdout output my be mixed with >> console or other expected output. >> >> In fact, it used to be on stdout, and got moved to stderr: > > > This is somehow confusing. I don't think it is worth having another > qemu_log_stderr() function rather than using error_report() but this very > call might deserve a comment explaining this unusual use. What do you think? The problem with stderr is that this isn't an error. Some uses of QEMU (inside Eclipse for example) flag everything printed on stderr as red which confuses users that they are seeing an error when they really aren't. Also all the uses of this message that I have seen (there are probably others though) stops QEMU until the connection is made, which means it doesn't matter if it is mixed up in console output. I just think that this message to most users isn't helpful and they probably don't even read it, so printing to stderr seems like a bit much. Thanks, Alistair > > >> >> commit fdca2124adc293f84f2b7aaf0df43faa6b6bf420 >> Author: Gerd Hoffmann <kraxel@redhat.com> >> Date: Mon Jun 24 08:39:49 2013 +0200 >> >> qemu-char: print notification to stderr >> >> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> >> Reviewed-by: Laszlo Ersek <lersek@redhat.com> >> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> >> >> diff --git a/qemu-char.c b/qemu-char.c >> index e3b3224886..371f6308c5 100644 >> --- a/qemu-char.c >> +++ b/qemu-char.c >> @@ -2666,8 +2666,8 @@ static CharDriverState *qemu_chr_open_socket_fd(int >> fd, bool do_nodelay, >> } >> >> if (is_listen && is_waitconnect) { >> - printf("QEMU waiting for connection on: %s\n", >> - chr->filename); >> + fprintf(stderr, "QEMU waiting for connection on: %s\n", >> + chr->filename); >> >> >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> >>> >>> --- >>> >>> chardev/char-socket.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/chardev/char-socket.c b/chardev/char-socket.c >>> index ccc499cfa1..a9884fa85b 100644 >>> --- a/chardev/char-socket.c >>> +++ b/chardev/char-socket.c >>> @@ -27,6 +27,7 @@ >>> #include "io/channel-tls.h" >>> #include "qemu/error-report.h" >>> #include "qapi/error.h" >>> +#include "qemu/log.h" >>> #include "qapi/clone-visitor.h" >>> >>> #include "chardev/char-io.h" >>> @@ -765,7 +766,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", >>> + qemu_log("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 >>> >>> >>> -- >> >> Marc-André Lureau >> >
On 06/06/2017 18:30, Alistair Francis wrote: >> >> This is somehow confusing. I don't think it is worth having another >> qemu_log_stderr() function rather than using error_report() but this very >> call might deserve a comment explaining this unusual use. What do you think? > > The problem with stderr is that this isn't an error. Some uses of QEMU > (inside Eclipse for example) flag everything printed on stderr as red > which confuses users that they are seeing an error when they really > aren't. But they are wrong. Would it work for you to work around it, by making QEMU use a client (connect) rather than a server (listen) socket? > Also all the uses of this message that I have seen (there are probably > others though) stops QEMU until the connection is made, which means it > doesn't matter if it is mixed up in console output. It may not mix up, but it would break programs expecting only console output to be on stdout. > I just think that this message to most users isn't helpful and they > probably don't even read it, so printing to stderr seems like a bit > much. I would compare it with dd's output. It goes to stderr even though it's not an error. Paolo
On Tue, Jun 6, 2017 at 3:14 PM, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > On 06/06/2017 18:30, Alistair Francis wrote: >>> >>> This is somehow confusing. I don't think it is worth having another >>> qemu_log_stderr() function rather than using error_report() but this very >>> call might deserve a comment explaining this unusual use. What do you think? >> >> The problem with stderr is that this isn't an error. Some uses of QEMU >> (inside Eclipse for example) flag everything printed on stderr as red >> which confuses users that they are seeing an error when they really >> aren't. > > But they are wrong. Would it work for you to work around it, by making > QEMU use a client (connect) rather than a server (listen) socket? No, we use QEMU as both so we will always have this issue. > >> Also all the uses of this message that I have seen (there are probably >> others though) stops QEMU until the connection is made, which means it >> doesn't matter if it is mixed up in console output. > > It may not mix up, but it would break programs expecting only console > output to be on stdout. Is that something we guarantee that stdout will only ever be program output? Thanks, Alistair > >> I just think that this message to most users isn't helpful and they >> probably don't even read it, so printing to stderr seems like a bit >> much. > > I would compare it with dd's output. It goes to stderr even though it's > not an error. > > Paolo >
Paolo Bonzini <pbonzini@redhat.com> writes: > On 06/06/2017 18:30, Alistair Francis wrote: >>> >>> This is somehow confusing. I don't think it is worth having another >>> qemu_log_stderr() function rather than using error_report() but this very >>> call might deserve a comment explaining this unusual use. What do you think? >> >> The problem with stderr is that this isn't an error. Some uses of QEMU >> (inside Eclipse for example) flag everything printed on stderr as red >> which confuses users that they are seeing an error when they really >> aren't. > > But they are wrong. Concur. We also print warnings and informational messages to stderr. We should make errors easy to recognize. Fortunately, error_report() prints errors to stderr in a rigid format. Unfortunately, error messages bypassing error_report() still exist in places. We suck. The format is timestamp-if-enabled progname ':' location message timestamp-if-enabled is normally empty. With -msg timestamp=on, it's the current time in ISO 8601 format, followed by a space. progname is the program name (main()'s argv[0]). location is either empty, or a reference to the command line or a configuration file. See error_vreport() for details. [...]
On Wed, Jun 7, 2017 at 12:19 AM, Markus Armbruster <armbru@redhat.com> wrote: > Paolo Bonzini <pbonzini@redhat.com> writes: > >> On 06/06/2017 18:30, Alistair Francis wrote: >>>> >>>> This is somehow confusing. I don't think it is worth having another >>>> qemu_log_stderr() function rather than using error_report() but this very >>>> call might deserve a comment explaining this unusual use. What do you think? >>> >>> The problem with stderr is that this isn't an error. Some uses of QEMU >>> (inside Eclipse for example) flag everything printed on stderr as red >>> which confuses users that they are seeing an error when they really >>> aren't. >> >> But they are wrong. > > Concur. We also print warnings and informational messages to stderr. > > We should make errors easy to recognize. Fortunately, error_report() > prints errors to stderr in a rigid format. Unfortunately, error > messages bypassing error_report() still exist in places. We suck. > > The format is > > timestamp-if-enabled progname ':' location message > > timestamp-if-enabled is normally empty. With -msg timestamp=on, it's > the current time in ISO 8601 format, followed by a space. > > progname is the program name (main()'s argv[0]). > > location is either empty, or a reference to the command line or a > configuration file. > > See error_vreport() for details. Ok, but this isn't an error, it's more information. So it sounds like we should still print to stderr but not print in the format described above? Thanks, Alistair > > [...] >
Alistair Francis <alistair.francis@xilinx.com> writes: > On Wed, Jun 7, 2017 at 12:19 AM, Markus Armbruster <armbru@redhat.com> wrote: >> Paolo Bonzini <pbonzini@redhat.com> writes: >> >>> On 06/06/2017 18:30, Alistair Francis wrote: >>>>> >>>>> This is somehow confusing. I don't think it is worth having another >>>>> qemu_log_stderr() function rather than using error_report() but this very >>>>> call might deserve a comment explaining this unusual use. What do you think? >>>> >>>> The problem with stderr is that this isn't an error. Some uses of QEMU >>>> (inside Eclipse for example) flag everything printed on stderr as red >>>> which confuses users that they are seeing an error when they really >>>> aren't. >>> >>> But they are wrong. >> >> Concur. We also print warnings and informational messages to stderr. >> >> We should make errors easy to recognize. Fortunately, error_report() >> prints errors to stderr in a rigid format. Unfortunately, error >> messages bypassing error_report() still exist in places. We suck. >> >> The format is >> >> timestamp-if-enabled progname ':' location message >> >> timestamp-if-enabled is normally empty. With -msg timestamp=on, it's >> the current time in ISO 8601 format, followed by a space. >> >> progname is the program name (main()'s argv[0]). >> >> location is either empty, or a reference to the command line or a >> configuration file. >> >> See error_vreport() for details. > > Ok, but this isn't an error, it's more information. So it sounds like > we should still print to stderr but not print in the format described > above? Yes. I explained the error message format to show how to distinguish actual errors from other stuff.
On Wed, Jun 7, 2017 at 11:03 PM, Markus Armbruster <armbru@redhat.com> wrote: > Alistair Francis <alistair.francis@xilinx.com> writes: > >> On Wed, Jun 7, 2017 at 12:19 AM, Markus Armbruster <armbru@redhat.com> wrote: >>> Paolo Bonzini <pbonzini@redhat.com> writes: >>> >>>> On 06/06/2017 18:30, Alistair Francis wrote: >>>>>> >>>>>> This is somehow confusing. I don't think it is worth having another >>>>>> qemu_log_stderr() function rather than using error_report() but this very >>>>>> call might deserve a comment explaining this unusual use. What do you think? >>>>> >>>>> The problem with stderr is that this isn't an error. Some uses of QEMU >>>>> (inside Eclipse for example) flag everything printed on stderr as red >>>>> which confuses users that they are seeing an error when they really >>>>> aren't. >>>> >>>> But they are wrong. >>> >>> Concur. We also print warnings and informational messages to stderr. >>> >>> We should make errors easy to recognize. Fortunately, error_report() >>> prints errors to stderr in a rigid format. Unfortunately, error >>> messages bypassing error_report() still exist in places. We suck. >>> >>> The format is >>> >>> timestamp-if-enabled progname ':' location message >>> >>> timestamp-if-enabled is normally empty. With -msg timestamp=on, it's >>> the current time in ISO 8601 format, followed by a space. >>> >>> progname is the program name (main()'s argv[0]). >>> >>> location is either empty, or a reference to the command line or a >>> configuration file. >>> >>> See error_vreport() for details. >> >> Ok, but this isn't an error, it's more information. So it sounds like >> we should still print to stderr but not print in the format described >> above? > > Yes. > > I explained the error message format to show how to distinguish actual > errors from other stuff. Sorry, I should have been more clear. I meant we should not use the error_report() function here. I don't think we have any warning_report() function though, is that something worth having?
Alistair Francis <alistair.francis@xilinx.com> writes: > On Wed, Jun 7, 2017 at 11:03 PM, Markus Armbruster <armbru@redhat.com> wrote: >> Alistair Francis <alistair.francis@xilinx.com> writes: >> >>> On Wed, Jun 7, 2017 at 12:19 AM, Markus Armbruster <armbru@redhat.com> wrote: >>>> Paolo Bonzini <pbonzini@redhat.com> writes: >>>> >>>>> On 06/06/2017 18:30, Alistair Francis wrote: >>>>>>> >>>>>>> This is somehow confusing. I don't think it is worth having another >>>>>>> qemu_log_stderr() function rather than using error_report() but this very >>>>>>> call might deserve a comment explaining this unusual use. What do you think? >>>>>> >>>>>> The problem with stderr is that this isn't an error. Some uses of QEMU >>>>>> (inside Eclipse for example) flag everything printed on stderr as red >>>>>> which confuses users that they are seeing an error when they really >>>>>> aren't. >>>>> >>>>> But they are wrong. >>>> >>>> Concur. We also print warnings and informational messages to stderr. >>>> >>>> We should make errors easy to recognize. Fortunately, error_report() >>>> prints errors to stderr in a rigid format. Unfortunately, error >>>> messages bypassing error_report() still exist in places. We suck. >>>> >>>> The format is >>>> >>>> timestamp-if-enabled progname ':' location message >>>> >>>> timestamp-if-enabled is normally empty. With -msg timestamp=on, it's >>>> the current time in ISO 8601 format, followed by a space. >>>> >>>> progname is the program name (main()'s argv[0]). >>>> >>>> location is either empty, or a reference to the command line or a >>>> configuration file. >>>> >>>> See error_vreport() for details. >>> >>> Ok, but this isn't an error, it's more information. So it sounds like >>> we should still print to stderr but not print in the format described >>> above? >> >> Yes. >> >> I explained the error message format to show how to distinguish actual >> errors from other stuff. > > Sorry, I should have been more clear. I meant we should not use the > error_report() function here. I don't think we have any > warning_report() function though, is that something worth having? So far we simply use error_printf() for such things. A function to report a warning would let us report them more uniformly, but only if we actually use it uniformly. In other words, adding one without also converting the existing warnings to use it would create yet another open-ended incremental conversion job. Are we up to it?
On Thu, Jun 8, 2017 at 10:56 AM, Markus Armbruster <armbru@redhat.com> wrote: > Alistair Francis <alistair.francis@xilinx.com> writes: > >> On Wed, Jun 7, 2017 at 11:03 PM, Markus Armbruster <armbru@redhat.com> wrote: >>> Alistair Francis <alistair.francis@xilinx.com> writes: >>> >>>> On Wed, Jun 7, 2017 at 12:19 AM, Markus Armbruster <armbru@redhat.com> wrote: >>>>> Paolo Bonzini <pbonzini@redhat.com> writes: >>>>> >>>>>> On 06/06/2017 18:30, Alistair Francis wrote: >>>>>>>> >>>>>>>> This is somehow confusing. I don't think it is worth having another >>>>>>>> qemu_log_stderr() function rather than using error_report() but this very >>>>>>>> call might deserve a comment explaining this unusual use. What do you think? >>>>>>> >>>>>>> The problem with stderr is that this isn't an error. Some uses of QEMU >>>>>>> (inside Eclipse for example) flag everything printed on stderr as red >>>>>>> which confuses users that they are seeing an error when they really >>>>>>> aren't. >>>>>> >>>>>> But they are wrong. >>>>> >>>>> Concur. We also print warnings and informational messages to stderr. >>>>> >>>>> We should make errors easy to recognize. Fortunately, error_report() >>>>> prints errors to stderr in a rigid format. Unfortunately, error >>>>> messages bypassing error_report() still exist in places. We suck. >>>>> >>>>> The format is >>>>> >>>>> timestamp-if-enabled progname ':' location message >>>>> >>>>> timestamp-if-enabled is normally empty. With -msg timestamp=on, it's >>>>> the current time in ISO 8601 format, followed by a space. >>>>> >>>>> progname is the program name (main()'s argv[0]). >>>>> >>>>> location is either empty, or a reference to the command line or a >>>>> configuration file. >>>>> >>>>> See error_vreport() for details. >>>> >>>> Ok, but this isn't an error, it's more information. So it sounds like >>>> we should still print to stderr but not print in the format described >>>> above? >>> >>> Yes. >>> >>> I explained the error message format to show how to distinguish actual >>> errors from other stuff. >> >> Sorry, I should have been more clear. I meant we should not use the >> error_report() function here. I don't think we have any >> warning_report() function though, is that something worth having? > > So far we simply use error_printf() for such things. > > A function to report a warning would let us report them more uniformly, > but only if we actually use it uniformly. In other words, adding one > without also converting the existing warnings to use it would create yet > another open-ended incremental conversion job. Are we up to it? Yeah! Why not. I am happy to give it a shot changing some errors to warnings. First thing though, what is the format for printing warnings? Thanks, Alistair >
Alistair Francis <alistair.francis@xilinx.com> writes: > On Thu, Jun 8, 2017 at 10:56 AM, Markus Armbruster <armbru@redhat.com> wrote: >> Alistair Francis <alistair.francis@xilinx.com> writes: >> >>> On Wed, Jun 7, 2017 at 11:03 PM, Markus Armbruster <armbru@redhat.com> wrote: >>>> Alistair Francis <alistair.francis@xilinx.com> writes: >>>> >>>>> On Wed, Jun 7, 2017 at 12:19 AM, Markus Armbruster <armbru@redhat.com> wrote: >>>>>> Paolo Bonzini <pbonzini@redhat.com> writes: >>>>>> >>>>>>> On 06/06/2017 18:30, Alistair Francis wrote: >>>>>>>>> >>>>>>>>> This is somehow confusing. I don't think it is worth having another >>>>>>>>> qemu_log_stderr() function rather than using error_report() but this very >>>>>>>>> call might deserve a comment explaining this unusual use. What do you think? >>>>>>>> >>>>>>>> The problem with stderr is that this isn't an error. Some uses of QEMU >>>>>>>> (inside Eclipse for example) flag everything printed on stderr as red >>>>>>>> which confuses users that they are seeing an error when they really >>>>>>>> aren't. >>>>>>> >>>>>>> But they are wrong. >>>>>> >>>>>> Concur. We also print warnings and informational messages to stderr. >>>>>> >>>>>> We should make errors easy to recognize. Fortunately, error_report() >>>>>> prints errors to stderr in a rigid format. Unfortunately, error >>>>>> messages bypassing error_report() still exist in places. We suck. >>>>>> >>>>>> The format is >>>>>> >>>>>> timestamp-if-enabled progname ':' location message >>>>>> >>>>>> timestamp-if-enabled is normally empty. With -msg timestamp=on, it's >>>>>> the current time in ISO 8601 format, followed by a space. >>>>>> >>>>>> progname is the program name (main()'s argv[0]). >>>>>> >>>>>> location is either empty, or a reference to the command line or a >>>>>> configuration file. >>>>>> >>>>>> See error_vreport() for details. >>>>> >>>>> Ok, but this isn't an error, it's more information. So it sounds like >>>>> we should still print to stderr but not print in the format described >>>>> above? >>>> >>>> Yes. >>>> >>>> I explained the error message format to show how to distinguish actual >>>> errors from other stuff. >>> >>> Sorry, I should have been more clear. I meant we should not use the >>> error_report() function here. I don't think we have any >>> warning_report() function though, is that something worth having? >> >> So far we simply use error_printf() for such things. >> >> A function to report a warning would let us report them more uniformly, >> but only if we actually use it uniformly. In other words, adding one >> without also converting the existing warnings to use it would create yet >> another open-ended incremental conversion job. Are we up to it? > > Yeah! Why not. I am happy to give it a shot changing some errors to warnings. > > First thing though, what is the format for printing warnings? We make one up. For what it's worth, gcc uses the same format as for errors with the message prefixed either by "warning: " or by "error: ". Also common is prefixing warnings, but not errors. We already have several error_report() calls with messages that start with (a variation of) "warning: ".
On Thu, Jun 8, 2017 at 10:16 PM, Markus Armbruster <armbru@redhat.com> wrote: > Alistair Francis <alistair.francis@xilinx.com> writes: > >> On Thu, Jun 8, 2017 at 10:56 AM, Markus Armbruster <armbru@redhat.com> wrote: >>> Alistair Francis <alistair.francis@xilinx.com> writes: >>> >>>> On Wed, Jun 7, 2017 at 11:03 PM, Markus Armbruster <armbru@redhat.com> wrote: >>>>> Alistair Francis <alistair.francis@xilinx.com> writes: >>>>> >>>>>> On Wed, Jun 7, 2017 at 12:19 AM, Markus Armbruster <armbru@redhat.com> wrote: >>>>>>> Paolo Bonzini <pbonzini@redhat.com> writes: >>>>>>> >>>>>>>> On 06/06/2017 18:30, Alistair Francis wrote: >>>>>>>>>> >>>>>>>>>> This is somehow confusing. I don't think it is worth having another >>>>>>>>>> qemu_log_stderr() function rather than using error_report() but this very >>>>>>>>>> call might deserve a comment explaining this unusual use. What do you think? >>>>>>>>> >>>>>>>>> The problem with stderr is that this isn't an error. Some uses of QEMU >>>>>>>>> (inside Eclipse for example) flag everything printed on stderr as red >>>>>>>>> which confuses users that they are seeing an error when they really >>>>>>>>> aren't. >>>>>>>> >>>>>>>> But they are wrong. >>>>>>> >>>>>>> Concur. We also print warnings and informational messages to stderr. >>>>>>> >>>>>>> We should make errors easy to recognize. Fortunately, error_report() >>>>>>> prints errors to stderr in a rigid format. Unfortunately, error >>>>>>> messages bypassing error_report() still exist in places. We suck. >>>>>>> >>>>>>> The format is >>>>>>> >>>>>>> timestamp-if-enabled progname ':' location message >>>>>>> >>>>>>> timestamp-if-enabled is normally empty. With -msg timestamp=on, it's >>>>>>> the current time in ISO 8601 format, followed by a space. >>>>>>> >>>>>>> progname is the program name (main()'s argv[0]). >>>>>>> >>>>>>> location is either empty, or a reference to the command line or a >>>>>>> configuration file. >>>>>>> >>>>>>> See error_vreport() for details. >>>>>> >>>>>> Ok, but this isn't an error, it's more information. So it sounds like >>>>>> we should still print to stderr but not print in the format described >>>>>> above? >>>>> >>>>> Yes. >>>>> >>>>> I explained the error message format to show how to distinguish actual >>>>> errors from other stuff. >>>> >>>> Sorry, I should have been more clear. I meant we should not use the >>>> error_report() function here. I don't think we have any >>>> warning_report() function though, is that something worth having? >>> >>> So far we simply use error_printf() for such things. >>> >>> A function to report a warning would let us report them more uniformly, >>> but only if we actually use it uniformly. In other words, adding one >>> without also converting the existing warnings to use it would create yet >>> another open-ended incremental conversion job. Are we up to it? >> >> Yeah! Why not. I am happy to give it a shot changing some errors to warnings. >> >> First thing though, what is the format for printing warnings? > > We make one up. > > For what it's worth, gcc uses the same format as for errors with the > message prefixed either by "warning: " or by "error: ". Also common is > prefixing warnings, but not errors. Ok, I think that is what I'm going to do. I will send a patch series out soon that basically copies the error_report() format but prefixes it with warning. I also removed the timestamp for the warning, although I'm not sure if that is what we want to do. I'll CC you when I send it out. Thanks, Alistair > > We already have several error_report() calls with messages that start > with (a variation of) "warning: ". >
diff --git a/qemu-char.c b/qemu-char.c index e3b3224886..371f6308c5 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2666,8 +2666,8 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay, } if (is_listen && is_waitconnect) { - printf("QEMU waiting for connection on: %s\n", - chr->filename); + fprintf(stderr, "QEMU waiting for connection on: %s\n", + chr->filename);