From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49250) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c9ooh-0008M5-Vs for qemu-devel@nongnu.org; Thu, 24 Nov 2016 02:51:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c9ooe-0002Pi-Qk for qemu-devel@nongnu.org; Thu, 24 Nov 2016 02:51:35 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41220) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c9ooe-0002Ol-Hz for qemu-devel@nongnu.org; Thu, 24 Nov 2016 02:51:32 -0500 References: <20161123190920.49f18f94@kitsune.suse.cz> From: Thomas Huth Message-ID: Date: Thu, 24 Nov 2016 08:51:26 +0100 MIME-Version: 1.0 In-Reply-To: <20161123190920.49f18f94@kitsune.suse.cz> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] sane char device writes? List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?Michal_Such=c3=a1nek?= , qemu-devel@nongnu.org Cc: Paolo Bonzini , Gerd Hoffmann On 23.11.2016 19:09, Michal Such=C3=A1nek wrote: > Hello, >=20 > I have reported the issue with qemu aborting in spapr_vty.c because > gtk.c submitted more data than can be sent to the emulated serial port. >=20 > While the abort has been resolved and spapr_vty.c should truncate the > data now getting the data through is still not possible. >=20 > Looking in the code I see that console.c has this code (which is only > piece of code in UI corresponding the the gtk part I found): >=20 > static void kbd_send_chars(void *opaque) > { > QemuConsole *s =3D opaque; > int len; > uint8_t buf[16]; >=20 > len =3D qemu_chr_be_can_write(s->chr); > if (len > s->out_fifo.count) > len =3D s->out_fifo.count; > if (len > 0) { > if (len > sizeof(buf)) > len =3D sizeof(buf); > qemu_fifo_read(&s->out_fifo, buf, len); > qemu_chr_be_write(s->chr, buf, len); > } > /* characters are pending: we send them a bit later (XXX: > horrible, should change char device API) */ > if (s->out_fifo.count > 0) { > timer_mod(s->kbd_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) > + 1); } > } >=20 > The corresponding piece of code in gtk.c is AFAICT >=20 > static gboolean (VteTerminal *terminal, gchar *text, guint size, > gpointer user_data) > { > VirtualConsole *vc =3D user_data; >=20 > if (vc->vte.echo) { > VteTerminal *term =3D VTE_TERMINAL(vc->vte.terminal); > int i; > for (i =3D 0; i < size; i++) { > uint8_t c =3D text[i]; > if (c >=3D 128 || isprint(c)) { > /* 8-bit characters are considered printable. */ > vte_terminal_feed(term, &text[i], 1); > } else if (c =3D=3D '\r' || c =3D=3D '\n') { > vte_terminal_feed(term, "\r\n", 2); > } else { > char ctrl[2] =3D { '^', 0}; > ctrl[1] =3D text[i] ^ 64; > vte_terminal_feed(term, ctrl, 2); > } > } > } >=20 > qemu_chr_be_write(vc->vte.chr, (uint8_t *)text, (unsigned > int)size); return TRUE; > } >=20 > meaning there is no loop to split the submitted text buffer. >=20 > gd_vc_in is VTE callback handling input so I suspect it either handles > it or not and it cannot say it handled only part of the "commit" event. >=20 > So for this to work an extra buffer would have to be stored in gtk.c > somewhere, and possibly similar timer trick used as in console.c >=20 > Any ideas how to do this without introducing too much insanity? > > Presumably using a GTK timer for repeating gd_vc_in the handler would > run in the same GTK UI thread as the "commit" signal handler and > excessive locking would not be required. >=20 > The data passed to gd_vc_in is presumably freed when it ends so it > would have to be copied somewhere. It's quite possible to create a > static list in gd_vc_in or some extra field in VirtualConsole. Not sure how the best solution should really look like, but Paolo suggested something here: http://lists.gnu.org/archive/html/qemu-devel/2016-11/msg02222.html ... so I'm putting him on CC: ... maybe he's got some spare minutes to elaborate on his idea. Thomas