From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KMOqv-00046y-9w for qemu-devel@nongnu.org; Fri, 25 Jul 2008 11:05:05 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KMOqs-00045g-EU for qemu-devel@nongnu.org; Fri, 25 Jul 2008 11:05:04 -0400 Received: from [199.232.76.173] (port=51508 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KMOqs-00045d-8k for qemu-devel@nongnu.org; Fri, 25 Jul 2008 11:05:02 -0400 Received: from wr-out-0506.google.com ([64.233.184.232]:29491) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KMOqr-0002J8-O8 for qemu-devel@nongnu.org; Fri, 25 Jul 2008 11:05:01 -0400 Received: by wr-out-0506.google.com with SMTP id c46so2653462wra.18 for ; Fri, 25 Jul 2008 08:05:00 -0700 (PDT) Message-ID: <4889EB79.80903@codemonkey.ws> Date: Fri, 25 Jul 2008 10:04:25 -0500 From: Anthony Liguori 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> In-Reply-To: <4889BC1D.1030305@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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: Gerd Hoffmann Cc: qemu-devel@nongnu.org, Paul Brook Gerd Hoffmann wrote: > From 076b5cff55b57a64e5f8daf32186d924945f78b4 Mon Sep 17 00:00:00 2001 > From: Gerd Hoffmann > Date: Wed, 23 Jul 2008 16:35:40 +0200 > Subject: [PATCH 03/21] 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 ... > You've got whitespace damage. You're mixing tabs and spaces. Please only use spaces. > Signed-off-by: Gerd Hoffmann > --- > vl.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- > 1 files changed, 154 insertions(+), 4 deletions(-) > > diff --git a/vl.c b/vl.c > index 23b5243..faf7b95 100644 > --- a/vl.c > +++ b/vl.c > @@ -2474,21 +2474,171 @@ 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 && EIO == errno) || > + (size == 0)) { > errno == EIO > + qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); > + s->polling = 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); > Perhaps what you really want here is a bottom half? > +} > + > +static void pty_chr_state(CharDriverState *chr, int connected) > +{ > + PtyCharDriver *s = chr->opaque; > + > + if (!connected) { > + if (s->connected) { > + fprintf(stderr,"%s: %s disconnected\n", __FUNCTION__, ptsname(s->fd)); > Obviously this debugging has to go. > + s->connected = 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) { > + fprintf(stderr,"%s: %s connected\n", __FUNCTION__, ptsname(s->fd)); > + s->connected = 1; > + qemu_chr_reset(chr); > + } > + } > +} > + > +void pty_chr_timer(void *opaque) > +{ > + struct CharDriverState *chr = opaque; > + PtyCharDriver *s = chr->opaque; > + > + if (s->connected) { > + /* All fine, nothing to do. */ > + return; > + } > + if (s->polling) { > + /* Ran for a while without getting EIO for reads, > + * probably someone connected to the slave pty. */ > + pty_chr_state(chr, 1); > + return; > + } > + /* Try reading again ... */ > +#if 0 > + fprintf(stderr,"%s: %s checking ...\n", __FUNCTION__, ptsname(s->fd)); > +#endif > + 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; > + > + chr = qemu_mallocz(sizeof(CharDriverState)); > + if (!chr) > + return NULL; > + s = qemu_mallocz(sizeof(PtyCharDriver)); > + if (!s) { > + free(chr); > qemu_free() Regards, Anthony Liguori > + return NULL; > + } > > - if (openpty(&master_fd, &slave_fd, NULL, NULL, NULL) < 0) { > + 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)); > + > + 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; > > - fprintf(stderr, "char device redirected to %s\n", ptsname(master_fd)); > - return qemu_chr_open_fd(master_fd, master_fd); > + 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