From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33349) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y8V9W-0005A0-04 for qemu-devel@nongnu.org; Tue, 06 Jan 2015 09:30:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y8V9S-0002mL-Ky for qemu-devel@nongnu.org; Tue, 06 Jan 2015 09:30:33 -0500 Received: from mx1.redhat.com ([209.132.183.28]:42286) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y8V9S-0002lp-7q for qemu-devel@nongnu.org; Tue, 06 Jan 2015 09:30:30 -0500 Message-ID: <54ABF19F.7090207@redhat.com> Date: Tue, 06 Jan 2015 16:30:55 +0200 From: Gal Hammer MIME-Version: 1.0 References: <1420449663-4542-1-git-send-email-ghammer@redhat.com> In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] char: disable stdio echo on resume from suspend. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Paolo Bonzini , QEMU Developers On 06/01/2015 15:49, Peter Maydell wrote: > On 5 January 2015 at 09:21, Gal Hammer 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 >> --- >> 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.