From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34288) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y8VFT-00077d-Vg for qemu-devel@nongnu.org; Tue, 06 Jan 2015 09:36:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y8VFQ-0004UK-Jl for qemu-devel@nongnu.org; Tue, 06 Jan 2015 09:36:43 -0500 Received: from mail-la0-f48.google.com ([209.85.215.48]:40303) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y8VFQ-0004UD-CL for qemu-devel@nongnu.org; Tue, 06 Jan 2015 09:36:40 -0500 Received: by mail-la0-f48.google.com with SMTP id gf13so19617308lab.21 for ; Tue, 06 Jan 2015 06:36:39 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <54ABF19F.7090207@redhat.com> References: <1420449663-4542-1-git-send-email-ghammer@redhat.com> <54ABF19F.7090207@redhat.com> From: Peter Maydell Date: Tue, 6 Jan 2015 14:36:19 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 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: Gal Hammer Cc: Paolo Bonzini , QEMU Developers On 6 January 2015 at 14:30, Gal Hammer wrote: > 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). >>> + /* 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. But qemu_chr_open_stdio also registers this function as the chr_set_echo pointer in the CharDriverState, so if the user of the CharDriverState calls qemu_chr_fe_set_echo(chr, true) then we'll set the echo to on. And then if we ^Z and resume, your patch will end up setting the echo to off, which seems wrong. > 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. Right, if the echo was disabled we want to make sure it is disabled. But if the echo wasn't disabled we don't want to disable it. >>> + } >>> +} >>> + >>> 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. Only when we're setting a signal to SIG_IGN, I think. signal()'s semantics aren't portable, which is why we use sigaction(). [See the Linux signal(2) manpage for some discussion of this.] thanks -- PMM