From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34909) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y8Z8O-0006cx-De for qemu-devel@nongnu.org; Tue, 06 Jan 2015 13:45:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y8Z8J-0003QI-Ad for qemu-devel@nongnu.org; Tue, 06 Jan 2015 13:45:40 -0500 Received: from mx6-phx2.redhat.com ([209.132.183.39]:59283) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y8Z8J-0003QA-1c for qemu-devel@nongnu.org; Tue, 06 Jan 2015 13:45:35 -0500 Date: Tue, 6 Jan 2015 13:45:32 -0500 (EST) From: Gal Hammer Message-ID: <1317037476.3836745.1420569932187.JavaMail.zimbra@redhat.com> In-Reply-To: References: <1420449663-4542-1-git-send-email-ghammer@redhat.com> <54ABF19F.7090207@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] char: disable stdio echo on resume from suspend. Reply-To: Gal Hammer List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Paolo Bonzini , QEMU Developers ----- Original Message ----- > From: "Peter Maydell" > To: "Gal Hammer" > Cc: "Paolo Bonzini" , "QEMU Developers" > Sent: Tuesday, January 6, 2015 4:36:19 PM > Subject: Re: [Qemu-devel] [PATCH] char: disable stdio echo on resume from suspend. > > 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. I've missed the fact that qemu_chr_fe_set_echo(chr, true) is called if the monitor is in qmp mode. I'll post a new version of this patch. > > 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.] No problem. I'll use sigaction() and won't check the signal identifier in the handler function. > thanks > -- PMM > > Gal.