From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1JsipI-0003Ct-Lh for qemu-devel@nongnu.org; Sun, 04 May 2008 14:20:44 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1JsipH-0003Ch-32 for qemu-devel@nongnu.org; Sun, 04 May 2008 14:20:43 -0400 Received: from [199.232.76.173] (port=51941 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JsipG-0003Ce-T7 for qemu-devel@nongnu.org; Sun, 04 May 2008 14:20:42 -0400 Received: from mx1.redhat.com ([66.187.233.31]) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1JsipH-0002q9-8W for qemu-devel@nongnu.org; Sun, 04 May 2008 14:20:43 -0400 Date: Sun, 4 May 2008 19:20:17 +0100 From: "Daniel P. Berrange" Subject: Re: [Qemu-devel] PATCH: Put Psuedo-TTY in rawmode for char devices Message-ID: <20080504182017.GA21786@redhat.com> References: <20080422005057.GD3649@redhat.com> <20080501171424.GF21335@redhat.com> <20080503230850.GA6745@volta.aurel32.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20080503230850.GA6745@volta.aurel32.net> Reply-To: "Daniel P. Berrange" , qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aurelien Jarno Cc: qemu-devel@nongnu.org On Sun, May 04, 2008 at 01:08:50AM +0200, Aurelien Jarno wrote: > On Thu, May 01, 2008 at 06:14:24PM +0100, Daniel P. Berrange wrote: > > Re-posting patch for review/inclusion... > > > > On Tue, Apr 22, 2008 at 01:50:57AM +0100, Daniel P. Berrange wrote: > > > If running a QEMU instance with a serial/parallel device connected to a > > > Psuedo-TTY, eg '-serial pty', every \r\n sequence output by the guest > > > is getting translated into a \n\n sequence by the TTY layer. So clients > > > interacting with the serial port via a TTY done get the correct \r\n > > > sequence and text marches to the right and wraps. This is because the > > > TTY is not put into rawmode when QEMU sets it up. > > > > > > The following patch is a re-diff of a patch applied to Xen's QEMU code. > > > It uses cfmakeraw() to ensure the TTY is put into rawmode, thus avoiding > > > the incorrect \r\n translations. It also switches to tcsetattr() on the > > > slave_fd instead of master_fd - although this is effectively the same on > > > Linux, only slave_fd works on Solaris. Finally it stops using the 'name' > > > arg to openpty() which is a security risk because its buffer size is > > > undefined. Instead it makes use of the ptsname() function. > > > > cfmakeraw() is Linux specific, though also available on most BSD > systems. It is unavailable on Solaris, and probably the same on WIN32. Win32 is not a problem because the qemu_chr_open_pty() function is already conditional on #if defined(__linux__) || defined(__sun__) That, said it is also already broken on Sun because the call to openpty() is conditionalized on #if defined(__linux__) so on Sun it will never even attempt to open the PTY. So this revised patch adds an impl of openty() and cfmakeraw() for Solaris. I don't have a Solaris system on which to test compile this, but the code is taken from the Xen fork of QEMU where it has been tested, so it ought to do the job. The updated patch also closes the slave_fd file descriptor since only the master_fd is needed on the QEMU end. So with this new patch it should correctly work on Linux and Solaris, which are the only platforms qemu_chr_open_pty() is compiled under. Xen also further extends the conditional around qemu_chr_open_pty() to allow it to compile on __NetBSD__ and __OpenBSD__. If desired I can include that, but it is unclear if those needed further code changes or not - someone might like to just try compiling with this patch on a BSD variant & see if it works. Signed-off-by: Daniel P. Berrange Regards, Daniel. Index: vl.c =================================================================== --- vl.c (revision 4332) +++ vl.c (working copy) @@ -2269,28 +2269,80 @@ return chr; } +#ifdef __sun__ +/* Once Solaris has openpty(), this is going to be removed. */ +int openpty(int *amaster, int *aslave, char *name, + struct termios *termp, struct winsize *winp) +{ + const char *slave; + int mfd = -1, sfd = -1; + + *amaster = *aslave = -1; + + mfd = open("/dev/ptmx", O_RDWR | O_NOCTTY); + if (mfd < 0) + goto err; + + if (grantpt(mfd) == -1 || unlockpt(mfd) == -1) + goto err; + + if ((slave = ptsname(mfd)) == NULL) + goto err; + + if ((sfd = open(slave, O_RDONLY | O_NOCTTY)) == -1) + goto err; + + if (ioctl(sfd, I_PUSH, "ptem") == -1 || + (termp != NULL && tcgetattr(sfd, termp) < 0)) + goto err; + + if (amaster) + *amaster = mfd; + if (aslave) + *aslave = sfd; + if (winp) + ioctl(sfd, TIOCSWINSZ, winp); + + return 0; + +err: + if (sfd != -1) + close(sfd); + close(mfd); + return -1; +} + +void cfmakeraw (struct termios *termios_p) +{ + termios_p->c_iflag &= + ~(IGNBRK|BRKINT|PARMRK|ISTRIP|INLCR|IGNCR|ICRNL|IXON); + termios_p->c_oflag &= ~OPOST; + termios_p->c_lflag &= ~(ECHO|ECHONL|ICANON|ISIG|IEXTEN); + termios_p->c_cflag &= ~(CSIZE|PARENB); + termios_p->c_cflag |= CS8; + + termios_p->c_cc[VMIN] = 0; + termios_p->c_cc[VTIME] = 0; +} +#endif + #if defined(__linux__) || defined(__sun__) static CharDriverState *qemu_chr_open_pty(void) { struct termios tty; - char slave_name[1024]; int master_fd, slave_fd; -#if defined(__linux__) - /* Not satisfying */ - if (openpty(&master_fd, &slave_fd, slave_name, NULL, NULL) < 0) { + if (openpty(&master_fd, &slave_fd, NULL, NULL, NULL) < 0) { return NULL; } -#endif - /* Disabling local echo and line-buffered output */ - tcgetattr (master_fd, &tty); - tty.c_lflag &= ~(ECHO|ICANON|ISIG); - tty.c_cc[VMIN] = 1; - tty.c_cc[VTIME] = 0; - tcsetattr (master_fd, TCSAFLUSH, &tty); + /* Set raw attributes on the pty. */ + cfmakeraw(&tty); + tcsetattr(slave_fd, TCSAFLUSH, &tty); - fprintf(stderr, "char device redirected to %s\n", slave_name); + close(slave_fd); + + fprintf(stderr, "char device redirected to %s\n", ptsname(master_fd)); return qemu_chr_open_fd(master_fd, master_fd); } -- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|