From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KNPao-0005cp-9e for qemu-devel@nongnu.org; Mon, 28 Jul 2008 06:04:38 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KNPaj-0005c0-Uw for qemu-devel@nongnu.org; Mon, 28 Jul 2008 06:04:37 -0400 Received: from [199.232.76.173] (port=47641 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KNPaj-0005bw-La for qemu-devel@nongnu.org; Mon, 28 Jul 2008 06:04:33 -0400 Received: from mx1.redhat.com ([66.187.233.31]:34314) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KNPai-0004sD-1m for qemu-devel@nongnu.org; Mon, 28 Jul 2008 06:04:33 -0400 Message-ID: <488D988C.3040209@redhat.com> Date: Mon, 28 Jul 2008 11:59:40 +0200 From: Gerd Hoffmann MIME-Version: 1.0 References: <488688E3.105@codemonkey.ws> <20080723082413.GA2291@redhat.com> <48871A7E.5030501@redhat.com> <20080723121510.GJ2291@redhat.com> <48872979.4050107@redhat.com> <48873F17.4030101@redhat.com> <4888A1BB.3010807@codemonkey.ws> <4889BC1D.1030305@redhat.com> <4889EB79.80903@codemonkey.ws> In-Reply-To: <4889EB79.80903@codemonkey.ws> Content-Type: multipart/mixed; boundary="------------070308090602090003040209" Subject: [Qemu-devel] Re: [PATCH 2/3] Always use nonblocking mode for qemu_chr_open_fd. Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: qemu-devel@nongnu.org, Paul Brook This is a multi-part message in MIME format. --------------070308090602090003040209 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit > You've got whitespace damage. You're mixing tabs and spaces. Please > only use spaces. Fixed. >> + if ((size == -1 && EIO == errno) || >> + (size == 0)) { >> > > errno == EIO Fixed. >> + /* >> + * Short timeout here: just need wait long enougth that qemu makes >> + * it through the poll loop once. When reconnected we want a >> + * short timeout so we notice it almost instantly. Otherwise >> + * read() gives us -EIO instantly, making pty_chr_state() reset the >> + * timeout to the normal (much longer) poll interval before the >> + * timer triggers. >> + */ >> + qemu_mod_timer(s->timer, qemu_get_clock(rt_clock) + 10); >> > > Perhaps what you really want here is a bottom half? Looked promising, tried, doesn't work. bh is called *before* the pty file handle is checked at least once for being readable. >> + if (s->connected) { >> + fprintf(stderr,"%s: %s disconnected\n", __FUNCTION__, >> ptsname(s->fd)); >> > > Obviously this debugging has to go. Dropped. >> + /* Try reading again ... */ >> +#if 0 >> + fprintf(stderr,"%s: %s checking ...\n", __FUNCTION__, >> ptsname(s->fd)); >> +#endif ... this too while being at it ;) >> + if (!s) { >> + free(chr); >> > > qemu_free() Fixed. New patch attached. cheers, Gerd -- http://kraxel.fedorapeople.org/xenner/ --------------070308090602090003040209 Content-Type: text/x-patch; name="0003-Attempt-to-detect-unconnected-ptys.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="0003-Attempt-to-detect-unconnected-ptys.patch" >>From 4362b66782ac68d8e1efb04fea1befeb20587677 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 23 Jul 2008 16:35:40 +0200 Subject: [PATCH 03/23] Attempt to detect unconnected ptys. This patch moves the pty char device imlementation away from the generic filehandle code. It tries to detect as good as possible whenever there is someone connected to the slave pty device and only send data down the road in case someone is listening. Unfortunaly we have to poll via timer once in a while to check the status because we have to use read() on the master pty to figure the status (returns -EIO when unconnected). Poll intervall for an idle guest is one second, when the guest sends data to the virtual device linked to the pty we check more frequently. The point for all of this is to avoid qemu blocking and not responding any more. Writing to the master pty handle succeeds even when nobody is connected to (and reading from) to the slave end of the pty. The kernel just bufferes the writes. And as soon as the kernel buffer is full the write() call blocks forever ... Signed-off-by: Gerd Hoffmann --- vl.c | 149 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 145 insertions(+), 4 deletions(-) diff --git a/vl.c b/vl.c index 23b5243..8801615 100644 --- a/vl.c +++ b/vl.c @@ -2474,21 +2474,162 @@ void cfmakeraw (struct termios *termios_p) #endif #if defined(__linux__) || defined(__sun__) + +typedef struct { + int fd; + int connected; + int polling; + int read_bytes; + QEMUTimer *timer; +} PtyCharDriver; + +static void pty_chr_update_read_handler(CharDriverState *chr); +static void pty_chr_state(CharDriverState *chr, int connected); + +static int pty_chr_write(CharDriverState *chr, const uint8_t *buf, int len) +{ + PtyCharDriver *s = chr->opaque; + + if (!s->connected) { + /* guest sends data, check for (re-)connect */ + pty_chr_update_read_handler(chr); + return 0; + } + return unix_write(s->fd, buf, len); +} + +static int pty_chr_read_poll(void *opaque) +{ + CharDriverState *chr = opaque; + PtyCharDriver *s = chr->opaque; + + s->read_bytes = qemu_chr_can_read(chr); + return s->read_bytes; +} + +static void pty_chr_read(void *opaque) +{ + CharDriverState *chr = opaque; + PtyCharDriver *s = chr->opaque; + int size, len; + uint8_t buf[1024]; + + len = sizeof(buf); + if (len > s->read_bytes) + len = s->read_bytes; + if (len == 0) + return; + size = read(s->fd, buf, len); + if ((size == -1 && errno == EIO) || + (size == 0)) { + pty_chr_state(chr, 0); + return; + } + if (size > 0) { + pty_chr_state(chr, 1); + qemu_chr_read(chr, buf, size); + } +} + +static void pty_chr_update_read_handler(CharDriverState *chr) +{ + PtyCharDriver *s = chr->opaque; + + qemu_set_fd_handler2(s->fd, pty_chr_read_poll, + pty_chr_read, NULL, chr); + s->polling = 1; + /* + * Short timeout here: just need wait long enougth that qemu makes + * it through the poll loop once. When reconnected we want a + * short timeout so we notice it almost instantly. Otherwise + * read() gives us -EIO instantly, making pty_chr_state() reset the + * timeout to the normal (much longer) poll interval before the + * timer triggers. + */ + qemu_mod_timer(s->timer, qemu_get_clock(rt_clock) + 10); +} + +static void pty_chr_state(CharDriverState *chr, int connected) +{ + PtyCharDriver *s = chr->opaque; + + if (!connected) { + qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); + s->connected = 0; + s->polling = 0; + /* (re-)connect poll interval for idle guests: once per second. + * We check more frequently in case the guests sends data to + * the virtual device linked to our pty. */ + qemu_mod_timer(s->timer, qemu_get_clock(rt_clock) + 1000); + } else { + if (!s->connected) + qemu_chr_reset(chr); + s->connected = 1; + } +} + +void pty_chr_timer(void *opaque) +{ + struct CharDriverState *chr = opaque; + PtyCharDriver *s = chr->opaque; + + if (s->connected) + return; + if (s->polling) { + /* If we arrive here without polling being cleared due + * read returning -EIO, then we are (re-)connected */ + pty_chr_state(chr, 1); + return; + } + + /* Next poll ... */ + pty_chr_update_read_handler(chr); +} + +static void pty_chr_close(struct CharDriverState *chr) +{ + PtyCharDriver *s = chr->opaque; + + qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); + close(s->fd); + qemu_free(s); +} + static CharDriverState *qemu_chr_open_pty(void) { + CharDriverState *chr; + PtyCharDriver *s; struct termios tty; - int master_fd, slave_fd; + int slave_fd; - if (openpty(&master_fd, &slave_fd, NULL, NULL, NULL) < 0) { + chr = qemu_mallocz(sizeof(CharDriverState)); + if (!chr) + return NULL; + s = qemu_mallocz(sizeof(PtyCharDriver)); + if (!s) { + qemu_free(chr); + return NULL; + } + + if (openpty(&s->fd, &slave_fd, NULL, NULL, NULL) < 0) { return NULL; } /* Set raw attributes on the pty. */ cfmakeraw(&tty); tcsetattr(slave_fd, TCSAFLUSH, &tty); + close(slave_fd); + + fprintf(stderr, "char device redirected to %s\n", ptsname(s->fd)); - fprintf(stderr, "char device redirected to %s\n", ptsname(master_fd)); - return qemu_chr_open_fd(master_fd, master_fd); + chr->opaque = s; + chr->chr_write = pty_chr_write; + chr->chr_update_read_handler = pty_chr_update_read_handler; + chr->chr_close = pty_chr_close; + + s->timer = qemu_new_timer(rt_clock, pty_chr_timer, chr); + + return chr; } static void tty_serial_init(int fd, int speed, -- 1.5.4.1 --------------070308090602090003040209--