[Qemu-devel] [PATCH v1 1/1] char-socket: Don't report TCP socket waiting as an error

[Qemu-devel] [PATCH v1 1/1] char-socket: Don't report TCP socket waiting as an error

[Alistair Francis]

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().

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

Re: [Qemu-devel] [PATCH v1 1/1] char-socket: Don't report TCP socket waiting as an error

[Philippe Mathieu-Daudé]

On 06/05/2017 03:34 PM, Alistair Francis 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().
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>
>  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);
>

Re: [Qemu-devel] [PATCH v1 1/1] char-socket: Don't report TCP socket waiting as an error

[Marc-André Lureau]

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:

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

Re: [Qemu-devel] [PATCH v1 1/1] char-socket: Don't report TCP socket waiting as an error

[Philippe Mathieu-Daudé]

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
>

Re: [Qemu-devel] [PATCH v1 1/1] char-socket: Don't report TCP socket waiting as an error

[Alistair Francis]

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
>>
>

Re: [Qemu-devel] [PATCH v1 1/1] char-socket: Don't report TCP socket waiting as an error

[Paolo Bonzini]

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

Re: [Qemu-devel] [PATCH v1 1/1] char-socket: Don't report TCP socket waiting as an error

[Alistair Francis]

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
>

Re: [Qemu-devel] [PATCH v1 1/1] char-socket: Don't report TCP socket waiting as an error

[Markus Armbruster]

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.

[...]

Re: [Qemu-devel] [PATCH v1 1/1] char-socket: Don't report TCP socket waiting as an error

[Alistair Francis]

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

>
> [...]
>

Re: [Qemu-devel] [PATCH v1 1/1] char-socket: Don't report TCP socket waiting as an error

[Markus Armbruster]

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.

Re: [Qemu-devel] [PATCH v1 1/1] char-socket: Don't report TCP socket waiting as an error

[Alistair Francis]

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?

Re: [Qemu-devel] [PATCH v1 1/1] char-socket: Don't report TCP socket waiting as an error

[Markus Armbruster]

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?

Re: [Qemu-devel] [PATCH v1 1/1] char-socket: Don't report TCP socket waiting as an error

[Alistair Francis]

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

>

Re: [Qemu-devel] [PATCH v1 1/1] char-socket: Don't report TCP socket waiting as an error

[Markus Armbruster]

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: ".

Re: [Qemu-devel] [PATCH v1 1/1] char-socket: Don't report TCP socket waiting as an error

[Alistair Francis]

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: ".
>