[Qemu-devel] [PATCH v2 03/15] char-pty: Drop "char device redirected to" message

Markus Armbruster posted 15 patches 6 years, 10 months ago
Maintainers: Christian Borntraeger <borntraeger@de.ibm.com>, Alex Williamson <alex.williamson@redhat.com>, Aleksandar Rikalo <arikalo@wavecomp.com>, Markus Armbruster <armbru@redhat.com>, Paul Burton <pburton@wavecomp.com>, Aurelien Jarno <aurelien@aurel32.net>, Kevin Wolf <kwolf@redhat.com>, David Hildenbrand <david@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Aleksandar Markovic <amarkovic@wavecomp.com>, Halil Pasic <pasic@linux.ibm.com>, Cornelia Huck <cohuck@redhat.com>, "Richard W.M. Jones" <rjones@redhat.com>, Max Reitz <mreitz@redhat.com>, Richard Henderson <rth@twiddle.net>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>
There is a newer version of this series
[Qemu-devel] [PATCH v2 03/15] char-pty: Drop "char device redirected to" message
Posted by Markus Armbruster 6 years, 10 months ago
char_pty_open() prints a "char device redirected to PTY_NAME (label
LABEL)" message to the current monitor or else to stderr.  No other
ChardevClass::open() prints anything on success.  Drop the message.

Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 chardev/char-pty.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index b034332edd..a48d3e5d20 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -211,8 +211,6 @@ static void char_pty_open(Chardev *chr,
     qemu_set_nonblock(master_fd);
 
     chr->filename = g_strdup_printf("pty:%s", pty_name);
-    error_printf("char device redirected to %s (label %s)\n",
-                 pty_name, chr->label);
 
     s = PTY_CHARDEV(chr);
     s->ioc = QIO_CHANNEL(qio_channel_file_new_fd(master_fd));
-- 
2.17.2


Re: [Qemu-devel] [PATCH v2 03/15] char-pty: Drop "char device redirected to" message
Posted by Paolo Bonzini 6 years, 10 months ago
On 11/04/19 16:52, Markus Armbruster wrote:
> char_pty_open() prints a "char device redirected to PTY_NAME (label
> LABEL)" message to the current monitor or else to stderr.  No other
> ChardevClass::open() prints anything on success.  Drop the message.
> 
> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  chardev/char-pty.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/chardev/char-pty.c b/chardev/char-pty.c
> index b034332edd..a48d3e5d20 100644
> --- a/chardev/char-pty.c
> +++ b/chardev/char-pty.c
> @@ -211,8 +211,6 @@ static void char_pty_open(Chardev *chr,
>      qemu_set_nonblock(master_fd);
>  
>      chr->filename = g_strdup_printf("pty:%s", pty_name);
> -    error_printf("char device redirected to %s (label %s)\n",
> -                 pty_name, chr->label);
>  
>      s = PTY_CHARDEV(chr);
>      s->ioc = QIO_CHANNEL(qio_channel_file_new_fd(master_fd));

The reason for the message is that the char device is completely useless
until the user knows the /dev/pts/N path[1].  You can get it with "info
chardev" (aka query-chardev for QMP) but there's an interesting chicken
and egg problem if the pty is for your monitor...

Paolo

[1] once you know it, you can use the monitor's readline interface with
e.g. "socat STDIO,cfmakeraw FILE:/dev/pts/1"

Re: [Qemu-devel] [PATCH v2 03/15] char-pty: Drop "char device redirected to" message
Posted by Markus Armbruster 6 years, 10 months ago
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 11/04/19 16:52, Markus Armbruster wrote:
>> char_pty_open() prints a "char device redirected to PTY_NAME (label
>> LABEL)" message to the current monitor or else to stderr.  No other
>> ChardevClass::open() prints anything on success.  Drop the message.
>> 
>> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>  chardev/char-pty.c | 2 --
>>  1 file changed, 2 deletions(-)
>> 
>> diff --git a/chardev/char-pty.c b/chardev/char-pty.c
>> index b034332edd..a48d3e5d20 100644
>> --- a/chardev/char-pty.c
>> +++ b/chardev/char-pty.c
>> @@ -211,8 +211,6 @@ static void char_pty_open(Chardev *chr,
>>      qemu_set_nonblock(master_fd);
>>  
>>      chr->filename = g_strdup_printf("pty:%s", pty_name);
>> -    error_printf("char device redirected to %s (label %s)\n",
>> -                 pty_name, chr->label);
>>  
>>      s = PTY_CHARDEV(chr);
>>      s->ioc = QIO_CHANNEL(qio_channel_file_new_fd(master_fd));
>
> The reason for the message is that the char device is completely useless
> until the user knows the /dev/pts/N path[1].  You can get it with "info
> chardev" (aka query-chardev for QMP) but there's an interesting chicken
> and egg problem if the pty is for your monitor...
>
> Paolo

During review of v1, I wrote:

    If we should decide the message is still useful enough to be worth
    keeping, I could direct it to stdout instead of dropping it.

No clear conclusion emerged, so I did nothing for v2.  If we conclude to
keep the message now, I'll gladly do that.

> [1] once you know it, you can use the monitor's readline interface with
> e.g. "socat STDIO,cfmakeraw FILE:/dev/pts/1"

There's also

    $ socat UNIX:/path/to/socket READLINE,history=$HOME/.hmp_history,prompt='(qemu) '

Lacks completion.  But then our very own reimplementation of readline
lacks any number of other features.

Re: [Qemu-devel] [PATCH v2 03/15] char-pty: Drop "char device redirected to" message
Posted by Markus Armbruster 6 years, 9 months ago
Markus Armbruster <armbru@redhat.com> writes:

> Paolo Bonzini <pbonzini@redhat.com> writes:
>
>> On 11/04/19 16:52, Markus Armbruster wrote:
>>> char_pty_open() prints a "char device redirected to PTY_NAME (label
>>> LABEL)" message to the current monitor or else to stderr.  No other
>>> ChardevClass::open() prints anything on success.  Drop the message.
>>> 
>>> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> ---
>>>  chardev/char-pty.c | 2 --
>>>  1 file changed, 2 deletions(-)
>>> 
>>> diff --git a/chardev/char-pty.c b/chardev/char-pty.c
>>> index b034332edd..a48d3e5d20 100644
>>> --- a/chardev/char-pty.c
>>> +++ b/chardev/char-pty.c
>>> @@ -211,8 +211,6 @@ static void char_pty_open(Chardev *chr,
>>>      qemu_set_nonblock(master_fd);
>>>  
>>>      chr->filename = g_strdup_printf("pty:%s", pty_name);
>>> -    error_printf("char device redirected to %s (label %s)\n",
>>> -                 pty_name, chr->label);
>>>  
>>>      s = PTY_CHARDEV(chr);
>>>      s->ioc = QIO_CHANNEL(qio_channel_file_new_fd(master_fd));
>>
>> The reason for the message is that the char device is completely useless
>> until the user knows the /dev/pts/N path[1].  You can get it with "info
>> chardev" (aka query-chardev for QMP) but there's an interesting chicken
>> and egg problem if the pty is for your monitor...
>>
>> Paolo
>
> During review of v1, I wrote:
>
>     If we should decide the message is still useful enough to be worth
>     keeping, I could direct it to stdout instead of dropping it.
>
> No clear conclusion emerged, so I did nothing for v2.  If we conclude to
> keep the message now, I'll gladly do that.

No further comments, no clear conclusion, so let's stick to the status
quo.  I'll drop this patch and add one to print the message to stdout.

[...]

Re: [Qemu-devel] [PATCH v2 03/15] char-pty: Drop "char device redirected to" message
Posted by Peter Krempa 6 years, 10 months ago
On Thu, Apr 11, 2019 at 16:52:44 +0200, Markus Armbruster wrote:
> char_pty_open() prints a "char device redirected to PTY_NAME (label
> LABEL)" message to the current monitor or else to stderr.  No other
> ChardevClass::open() prints anything on success.  Drop the message.
> 
> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  chardev/char-pty.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/chardev/char-pty.c b/chardev/char-pty.c
> index b034332edd..a48d3e5d20 100644
> --- a/chardev/char-pty.c
> +++ b/chardev/char-pty.c
> @@ -211,8 +211,6 @@ static void char_pty_open(Chardev *chr,
>      qemu_set_nonblock(master_fd);
>  
>      chr->filename = g_strdup_printf("pty:%s", pty_name);
> -    error_printf("char device redirected to %s (label %s)\n",
> -                 pty_name, chr->label);

Don't be scared by the one occurence of this string in a 'strstr' call
in libvirt. We thankfully deleted the code that was using it to extract
the PTY path a long time ago. The only leftover part is to skip it when
we are attempting to extract an useful error from qemu which would be
reported to stderr prior to initiating QMP so getting rid of the message
actually has no effect.

https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=932534e85f34a479c7eac174e997bfd9c85bd22d