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é <address@hidden> wrote:
> On 06/06/2017 08:58 AM, Marc-André Lureau wrote:
>>
>> Hi
>>
>> On Mon, Jun 5, 2017 at 10:37 PM Alistair Francis <
>> address@hidden> 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 <address@hidden>
>> Date:   Mon Jun 24 08:39:49 2013 +0200
>>
>>     qemu-char: print notification to stderr
>>
>>     Signed-off-by: Gerd Hoffmann <address@hidden>
>>     Reviewed-by: Laszlo Ersek <address@hidden>
>>     Signed-off-by: Michael Tokarev <address@hidden>
>>
>> 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 <address@hidden>
>>>
>>> ---
>>>
>>>  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
>>
>