Re: [Qemu-devel] sane char device writes?

[Thomas Huth]

On 23.11.2016 19:09, Michal Suchánek wrote:
> Hello,
> 
> 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.
> 
> While the abort has been resolved and spapr_vty.c should truncate the
> data now getting the data through is still not possible.
> 
> 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):
> 
> static void kbd_send_chars(void *opaque)
> {
>     QemuConsole *s = opaque;
>     int len;
>     uint8_t buf[16];
> 
>     len = qemu_chr_be_can_write(s->chr);
>     if (len > s->out_fifo.count)
>         len = s->out_fifo.count;
>     if (len > 0) {
>         if (len > sizeof(buf))
>             len = 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); }
> }
> 
> The corresponding piece of code in gtk.c is AFAICT
> 
> static gboolean  (VteTerminal *terminal, gchar *text, guint size,
>                          gpointer user_data)
> {
>     VirtualConsole *vc = user_data;
> 
>     if (vc->vte.echo) {
>         VteTerminal *term = VTE_TERMINAL(vc->vte.terminal);
>         int i;
>         for (i = 0; i < size; i++) {
>             uint8_t c = text[i];
>             if (c >= 128 || isprint(c)) {
>                 /* 8-bit characters are considered printable.  */
>                 vte_terminal_feed(term, &text[i], 1);
>             } else if (c == '\r' || c == '\n') {
>                 vte_terminal_feed(term, "\r\n", 2);
>             } else {
>                 char ctrl[2] = { '^', 0};
>                 ctrl[1] = text[i] ^ 64;
>                 vte_terminal_feed(term, ctrl, 2);
>             }
>         }
>     }
> 
>     qemu_chr_be_write(vc->vte.chr, (uint8_t  *)text, (unsigned
>     int)size); return TRUE;
> }
> 
> meaning there is no loop to split the submitted text buffer.
> 
> 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.
> 
> 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
> 
> 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.
> 
> 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