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

[Gal Hammer]

On 06/01/2015 15:49, Peter Maydell wrote:
> On 5 January 2015 at 09:21, Gal Hammer <address@hidden> 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 <address@hidden>
> > ---
> >   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.