Re: [Qemu-devel] [PATCH] char: disable stdio echo on resume from suspend.

[Gal Hammer <ghammer@redhat.com>]

On 06/01/2015 15:49, Peter Maydell wrote:
> On 5 January 2015 at 09:21, Gal Hammer <ghammer@redhat.com> wrote:
>> The monitor's auto-completion feature stopped working when stdio is used
>> as an input and qemu was resumed after it was suspended (using ctrl-z).
>>
>> Signed-off-by: Gal Hammer <ghammer@redhat.com>
>> ---
>>   qemu-char.c | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/qemu-char.c b/qemu-char.c
>> index ef84b53..786df33 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -1113,12 +1113,22 @@ static int old_fd0_flags;
>>   static bool stdio_in_use;
>>   static bool stdio_allow_signal;
>>
>> +static void qemu_chr_set_echo_stdio(CharDriverState *chr, bool echo);
>> +
>>   static void term_exit(void)
>>   {
>>       tcsetattr (0, TCSANOW, &oldtty);
>>       fcntl(0, F_SETFL, old_fd0_flags);
>>   }
>>
>> +static void term_stdio_handler(int sig)
>> +{
>> +    if (sig == SIGCONT) {
>
> ...why do we need this check? We don't register the function
> for any other signals...

It's a caution and not a must. It can be removed if you think it is 
redundant.

>
>> +        /* echo should be off after resume from suspend. */
>> +        qemu_chr_set_echo_stdio(NULL, false);
>
> Should echo really be always off, even if the thing using the
> char device had set it to on?

That's what the function qemu_chr_open_stdio() do, always set the stdio 
char device echo to off. I didn't change the current behavior I just 
restore it.

As I understood from my tests, the auto-complete feature doesn't work if 
the echo is enabled because pressing the tab key prints a tab char.

>> +    }
>> +}
>> +
>>   static void qemu_chr_set_echo_stdio(CharDriverState *chr, bool echo)
>>   {
>>       struct termios tty;
>> @@ -1165,6 +1175,7 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
>>       tcgetattr(0, &oldtty);
>>       qemu_set_nonblock(0);
>>       atexit(term_exit);
>> +    signal(SIGCONT, term_stdio_handler);
>
> This should probably be using sigaction() which is what we use
> elsewhere for signal handler registration.

signal() is used in the code. Although I can switch to sigaction() and 
earn few more LOC ;-).

>
> -- PMM
>

     Gal.