[PATCH 0/2] ui/gtk: prevent QEMU lock up

[PATCH 0/2] ui/gtk: prevent QEMU lock up

[Volker Rümelin]

Since commit 8eb13bbbac ("ui/gtk: vte: fix sending multiple
characeters") it's very easy to lock up QEMU with the gtk ui.
If you configure a guest with a serial device and the guest
doesn't listen on this device, QEMU will lock up after
entering two characters in the serial console.

Patch 1/2 is a bug fix for 6.1.

Patch 2/2 is a preview for 6.2. If there are no objections, I
will resend this patch for the 6.2 development cycle together
with changes to ui/console similar to the changes in patch 1/2.
This will remove the QEMUFIFO code in ui/console.c.

Volker Rümelin (2):
   ui/gtk: add a keyboard fifo to the VTE consoles
   ui/gtk: drop chars if the chardev frontend makes no progress

  include/ui/gtk.h |  7 ++++
  ui/gtk.c         | 84 +++++++++++++++++++++++++++++++++++++++++++-----
  2 files changed, 83 insertions(+), 8 deletions(-)

-- 
2.26.2

[PATCH for 6.1 1/2] ui/gtk: add a keyboard fifo to the VTE consoles

[Volker Rümelin]

Since commit 8eb13bbbac ("ui/gtk: vte: fix sending multiple
characeters") it's very easy to lock up QEMU with the gtk ui.
If you configure a guest with a serial device and the guest
doesn't listen on this device, QEMU will lock up after
entering two characters in the serial console.

To fix this problem copy the function kbd_send_chars() and
related code from ui/console.c to ui/gtk.c. kbd_send_chars()
doesn't lock up because it uses a timer instead of a busy loop
for the write retries.

Fixes: 8eb13bbbac ("ui/gtk: vte: fix sending multiple characeters")
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 include/ui/gtk.h |  5 +++++
 ui/gtk.c         | 53 ++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index 9516670ebc..4714218376 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -25,6 +25,9 @@
 #include "ui/egl-helpers.h"
 #include "ui/egl-context.h"
 #endif
+#ifdef CONFIG_VTE
+#include "qemu/fifo8.h"
+#endif
 
 #define MAX_VCS 10
 
@@ -62,6 +65,8 @@ typedef struct VirtualVteConsole {
     GtkWidget *scrollbar;
     GtkWidget *terminal;
     Chardev *chr;
+    QEMUTimer *kbd_timer;
+    Fifo8 out_fifo;
     bool echo;
 } VirtualVteConsole;
 #endif
diff --git a/ui/gtk.c b/ui/gtk.c
index 376b4d528d..b95b077b65 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -48,6 +48,7 @@
 #include <locale.h>
 #if defined(CONFIG_VTE)
 #include <vte/vte.h>
+#include "chardev/char-fe.h"
 #endif
 #include <math.h>
 
@@ -1710,10 +1711,46 @@ static const TypeInfo char_gd_vc_type_info = {
     .class_init = char_gd_vc_class_init,
 };
 
+static void gd_vc_send_chars(VirtualConsole *vc)
+{
+    uint32_t len, avail;
+    const uint8_t *buf;
+
+    len = qemu_chr_be_can_write(vc->vte.chr);
+    avail = fifo8_num_used(&vc->vte.out_fifo);
+    if (len > avail) {
+        len = avail;
+    }
+    while (len > 0) {
+        uint32_t size;
+
+        buf = fifo8_pop_buf(&vc->vte.out_fifo, len, &size);
+        qemu_chr_be_write(vc->vte.chr, (uint8_t *)buf, size);
+        len -= size;
+        avail -= size;
+    }
+    /*
+     * characters are pending: we send them a bit later (XXX:
+     * horrible, should change char device API)
+     */
+    if (avail > 0) {
+        timer_mod(vc->vte.kbd_timer,
+                  qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1);
+    }
+}
+
+static void gd_vc_timer_send_chars(void *opaque)
+{
+    VirtualConsole *vc = opaque;
+
+    gd_vc_send_chars(vc);
+}
+
 static gboolean gd_vc_in(VteTerminal *terminal, gchar *text, guint size,
                          gpointer user_data)
 {
     VirtualConsole *vc = user_data;
+    CharBackend *be = vc->vte.chr->be;
 
     if (vc->vte.echo) {
         VteTerminal *term = VTE_TERMINAL(vc->vte.terminal);
@@ -1733,16 +1770,13 @@ static gboolean gd_vc_in(VteTerminal *terminal, gchar *text, guint size,
         }
     }
 
-    int remaining = size;
-    uint8_t* p = (uint8_t *)text;
-    while (remaining > 0) {
-        int can_write = qemu_chr_be_can_write(vc->vte.chr);
-        int written = MIN(remaining, can_write);
-        qemu_chr_be_write(vc->vte.chr, p, written);
+    if (be && be->chr_read) {
+        uint32_t free = fifo8_num_free(&vc->vte.out_fifo);
 
-        remaining -= written;
-        p += written;
+        fifo8_push_all(&vc->vte.out_fifo, (uint8_t *)text, MIN(free, size));
+        gd_vc_send_chars(vc);
     }
+
     return TRUE;
 }
 
@@ -1759,6 +1793,9 @@ static GSList *gd_vc_vte_init(GtkDisplayState *s, VirtualConsole *vc,
     vc->s = s;
     vc->vte.echo = vcd->echo;
     vc->vte.chr = chr;
+    fifo8_create(&vc->vte.out_fifo, 16);
+    vc->vte.kbd_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL,
+                                     gd_vc_timer_send_chars, vc);
     vcd->console = vc;
 
     snprintf(buffer, sizeof(buffer), "vc%d", idx);
-- 
2.26.2

[PATCH for 6.2 2/2] ui/gtk: drop chars if the chardev frontend makes no progress

[Volker Rümelin]

Currently there is no limit how long the virtual console chardev
backend retries to send remaining characters in the buffer.

Drop the characters after a timeout if the chardev frontend
doesn't accept characters.

The timeout was calculated from arbitrarily selected constants.
It was assumed that 50 baud is the slowest bit rate and that a
character has at maximum one start bit, eight data bits, one
parity bit and two stop bits. For a good safety margin, this
time was multiplied by eight. This results in a timeout of
1 / (50 bits / s) * 12 bits * 8 = 1.92 s.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 include/ui/gtk.h |  2 ++
 ui/gtk.c         | 47 +++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index 4714218376..2771b1aa91 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -67,6 +67,8 @@ typedef struct VirtualVteConsole {
     Chardev *chr;
     QEMUTimer *kbd_timer;
     Fifo8 out_fifo;
+    int64_t be_last_write;
+    bool be_can_write;
     bool echo;
 } VirtualVteConsole;
 #endif
diff --git a/ui/gtk.c b/ui/gtk.c
index b95b077b65..c22f4a0329 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1711,8 +1711,20 @@ static const TypeInfo char_gd_vc_type_info = {
     .class_init = char_gd_vc_class_init,
 };
 
+static uint32_t gd_vc_send_backoff(uint32_t elapsed)
+{
+    if (elapsed <= 500) {
+        return 1000;
+    } else if (elapsed >= 50000) {
+        return 100000;
+    }
+
+    return elapsed * 2;
+}
+
 static void gd_vc_send_chars(VirtualConsole *vc)
 {
+    int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
     uint32_t len, avail;
     const uint8_t *buf;
 
@@ -1721,21 +1733,39 @@ static void gd_vc_send_chars(VirtualConsole *vc)
     if (len > avail) {
         len = avail;
     }
-    while (len > 0) {
+    if (len > 0) {
         uint32_t size;
 
-        buf = fifo8_pop_buf(&vc->vte.out_fifo, len, &size);
-        qemu_chr_be_write(vc->vte.chr, (uint8_t *)buf, size);
-        len -= size;
-        avail -= size;
+        do {
+            buf = fifo8_pop_buf(&vc->vte.out_fifo, len, &size);
+            qemu_chr_be_write(vc->vte.chr, (uint8_t *)buf, size);
+            len -= size;
+            avail -= size;
+        } while (len > 0);
+
+        vc->vte.be_can_write = true;
+        vc->vte.be_last_write = now;
     }
     /*
      * characters are pending: we send them a bit later (XXX:
      * horrible, should change char device API)
      */
     if (avail > 0) {
-        timer_mod(vc->vte.kbd_timer,
-                  qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1);
+        if (vc->vte.be_can_write) {
+            uint64_t elapsed = (now - vc->vte.be_last_write) / SCALE_US;
+
+            if (elapsed < 1920000) {
+                timer_mod(vc->vte.kbd_timer,
+                          now / SCALE_US + gd_vc_send_backoff(elapsed));
+            } else {
+                /* no progress since 1.92s */
+                vc->vte.be_can_write = false;
+                fifo8_reset(&vc->vte.out_fifo);
+            }
+        } else {
+            /* the chardev frontend hasn't accepted chars in a long time */
+            fifo8_reset(&vc->vte.out_fifo);
+        }
     }
 }
 
@@ -1794,7 +1824,8 @@ static GSList *gd_vc_vte_init(GtkDisplayState *s, VirtualConsole *vc,
     vc->vte.echo = vcd->echo;
     vc->vte.chr = chr;
     fifo8_create(&vc->vte.out_fifo, 16);
-    vc->vte.kbd_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL,
+    vc->vte.be_can_write = false;
+    vc->vte.kbd_timer = timer_new_us(QEMU_CLOCK_VIRTUAL,
                                      gd_vc_timer_send_chars, vc);
     vcd->console = vc;
 
-- 
2.26.2

Re: [PATCH for 6.1 1/2] ui/gtk: add a keyboard fifo to the VTE consoles

[Peter Maydell]

On Sun, 18 Jul 2021 at 08:50, Volker Rümelin <vr_qemu@t-online.de> wrote:
>
> Since commit 8eb13bbbac ("ui/gtk: vte: fix sending multiple
> characeters") it's very easy to lock up QEMU with the gtk ui.
> If you configure a guest with a serial device and the guest
> doesn't listen on this device, QEMU will lock up after
> entering two characters in the serial console.
>
> To fix this problem copy the function kbd_send_chars() and
> related code from ui/console.c to ui/gtk.c. kbd_send_chars()
> doesn't lock up because it uses a timer instead of a busy loop
> for the write retries.
>
> Fixes: 8eb13bbbac ("ui/gtk: vte: fix sending multiple characeters")
> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> ---
>  include/ui/gtk.h |  5 +++++
>  ui/gtk.c         | 53 ++++++++++++++++++++++++++++++++++++++++--------
>  2 files changed, 50 insertions(+), 8 deletions(-)

This feels like maybe it's the kind of thing that should be handled
more generically rather than in one particular UI frontend ?

thanks
-- PMM

Re: [PATCH for 6.1 1/2] ui/gtk: add a keyboard fifo to the VTE consoles

[Daniel P. Berrangé]

On Sun, Jul 18, 2021 at 02:00:03PM +0100, Peter Maydell wrote:
> On Sun, 18 Jul 2021 at 08:50, Volker Rümelin <vr_qemu@t-online.de> wrote:
> >
> > Since commit 8eb13bbbac ("ui/gtk: vte: fix sending multiple
> > characeters") it's very easy to lock up QEMU with the gtk ui.
> > If you configure a guest with a serial device and the guest
> > doesn't listen on this device, QEMU will lock up after
> > entering two characters in the serial console.
> >
> > To fix this problem copy the function kbd_send_chars() and
> > related code from ui/console.c to ui/gtk.c. kbd_send_chars()
> > doesn't lock up because it uses a timer instead of a busy loop
> > for the write retries.
> >
> > Fixes: 8eb13bbbac ("ui/gtk: vte: fix sending multiple characeters")
> > Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> > ---
> >  include/ui/gtk.h |  5 +++++
> >  ui/gtk.c         | 53 ++++++++++++++++++++++++++++++++++++++++--------
> >  2 files changed, 50 insertions(+), 8 deletions(-)
> 
> This feels like maybe it's the kind of thing that should be handled
> more generically rather than in one particular UI frontend ?

IIUC none of the other frontends directly talk to the chardevs for the
serial ports. Instead they communicate via the text console interfaces.
So GTK is a bit special already here.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH for 6.1 1/2] ui/gtk: add a keyboard fifo to the VTE consoles

[Volker Rümelin]

Am 18.07.21 um 15:00 schrieb Peter Maydell:

> On Sun, 18 Jul 2021 at 08:50, Volker Rümelin <vr_qemu@t-online.de> wrote:
>> Since commit 8eb13bbbac ("ui/gtk: vte: fix sending multiple
>> characeters") it's very easy to lock up QEMU with the gtk ui.
>> If you configure a guest with a serial device and the guest
>> doesn't listen on this device, QEMU will lock up after
>> entering two characters in the serial console.
>>
>> To fix this problem copy the function kbd_send_chars() and
>> related code from ui/console.c to ui/gtk.c. kbd_send_chars()
>> doesn't lock up because it uses a timer instead of a busy loop
>> for the write retries.
>>
>> Fixes: 8eb13bbbac ("ui/gtk: vte: fix sending multiple characeters")
>> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
>> ---
>>   include/ui/gtk.h |  5 +++++
>>   ui/gtk.c         | 53 ++++++++++++++++++++++++++++++++++++++++--------
>>   2 files changed, 50 insertions(+), 8 deletions(-)
> This feels like maybe it's the kind of thing that should be handled
> more generically rather than in one particular UI frontend ?
>
> thanks
> -- PMM

All other UI frontends (except Spice) use the correct code in 
kbd_send_chars(). I think only the GTK UI code is wrong.

With best regards
Volker

Re: [PATCH for 6.1 1/2] ui/gtk: add a keyboard fifo to the VTE consoles

[Peter Maydell]

On Mon, 19 Jul 2021 at 19:22, Volker Rümelin <vr_qemu@t-online.de> wrote:
>
> Am 18.07.21 um 15:00 schrieb Peter Maydell:
>
> > On Sun, 18 Jul 2021 at 08:50, Volker Rümelin <vr_qemu@t-online.de> wrote:
> >> Since commit 8eb13bbbac ("ui/gtk: vte: fix sending multiple
> >> characeters") it's very easy to lock up QEMU with the gtk ui.
> >> If you configure a guest with a serial device and the guest
> >> doesn't listen on this device, QEMU will lock up after
> >> entering two characters in the serial console.
> >>
> >> To fix this problem copy the function kbd_send_chars() and
> >> related code from ui/console.c to ui/gtk.c. kbd_send_chars()
> >> doesn't lock up because it uses a timer instead of a busy loop
> >> for the write retries.
> >>
> >> Fixes: 8eb13bbbac ("ui/gtk: vte: fix sending multiple characeters")
> >> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> >> ---
> >>   include/ui/gtk.h |  5 +++++
> >>   ui/gtk.c         | 53 ++++++++++++++++++++++++++++++++++++++++--------
> >>   2 files changed, 50 insertions(+), 8 deletions(-)
> > This feels like maybe it's the kind of thing that should be handled
> > more generically rather than in one particular UI frontend ?

> All other UI frontends (except Spice) use the correct code in
> kbd_send_chars(). I think only the GTK UI code is wrong.

Why isn't GTK able to do things the same way all the other UI
frontends do, then ?

-- PMM

Re: [PATCH for 6.1 1/2] ui/gtk: add a keyboard fifo to the VTE consoles

[Daniel P. Berrangé]

On Mon, Jul 19, 2021 at 07:37:23PM +0100, Peter Maydell wrote:
> On Mon, 19 Jul 2021 at 19:22, Volker Rümelin <vr_qemu@t-online.de> wrote:
> >
> > Am 18.07.21 um 15:00 schrieb Peter Maydell:
> >
> > > On Sun, 18 Jul 2021 at 08:50, Volker Rümelin <vr_qemu@t-online.de> wrote:
> > >> Since commit 8eb13bbbac ("ui/gtk: vte: fix sending multiple
> > >> characeters") it's very easy to lock up QEMU with the gtk ui.
> > >> If you configure a guest with a serial device and the guest
> > >> doesn't listen on this device, QEMU will lock up after
> > >> entering two characters in the serial console.
> > >>
> > >> To fix this problem copy the function kbd_send_chars() and
> > >> related code from ui/console.c to ui/gtk.c. kbd_send_chars()
> > >> doesn't lock up because it uses a timer instead of a busy loop
> > >> for the write retries.
> > >>
> > >> Fixes: 8eb13bbbac ("ui/gtk: vte: fix sending multiple characeters")
> > >> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> > >> ---
> > >>   include/ui/gtk.h |  5 +++++
> > >>   ui/gtk.c         | 53 ++++++++++++++++++++++++++++++++++++++++--------
> > >>   2 files changed, 50 insertions(+), 8 deletions(-)
> > > This feels like maybe it's the kind of thing that should be handled
> > > more generically rather than in one particular UI frontend ?
> 
> > All other UI frontends (except Spice) use the correct code in
> > kbd_send_chars(). I think only the GTK UI code is wrong.
> 
> Why isn't GTK able to do things the same way all the other UI
> frontends do, then ?

In the other frontends the text consoles are not directly exposed, you
just switch into them using QEMU's console switching shortcuts.

In the GTK frontend, each text console is exposed directly as a notebook
tab in the UI, so you don't need to use QEMU's console switching.

IOW, the GTK frontend is more feature rich than the other frontends and
causes it to hit this mistake.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH for 6.1 1/2] ui/gtk: add a keyboard fifo to the VTE consoles

[Peter Maydell]

On Mon, 19 Jul 2021 at 19:41, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Mon, Jul 19, 2021 at 07:37:23PM +0100, Peter Maydell wrote:
> > On Mon, 19 Jul 2021 at 19:22, Volker Rümelin <vr_qemu@t-online.de> wrote:
> > >
> > > Am 18.07.21 um 15:00 schrieb Peter Maydell:
> > >
> > > > On Sun, 18 Jul 2021 at 08:50, Volker Rümelin <vr_qemu@t-online.de> wrote:
> > > >> Since commit 8eb13bbbac ("ui/gtk: vte: fix sending multiple
> > > >> characeters") it's very easy to lock up QEMU with the gtk ui.
> > > >> If you configure a guest with a serial device and the guest
> > > >> doesn't listen on this device, QEMU will lock up after
> > > >> entering two characters in the serial console.
> > > >>
> > > >> To fix this problem copy the function kbd_send_chars() and
> > > >> related code from ui/console.c to ui/gtk.c. kbd_send_chars()
> > > >> doesn't lock up because it uses a timer instead of a busy loop
> > > >> for the write retries.
> > > >>
> > > >> Fixes: 8eb13bbbac ("ui/gtk: vte: fix sending multiple characeters")
> > > >> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> > > >> ---
> > > >>   include/ui/gtk.h |  5 +++++
> > > >>   ui/gtk.c         | 53 ++++++++++++++++++++++++++++++++++++++++--------
> > > >>   2 files changed, 50 insertions(+), 8 deletions(-)
> > > > This feels like maybe it's the kind of thing that should be handled
> > > > more generically rather than in one particular UI frontend ?
> >
> > > All other UI frontends (except Spice) use the correct code in
> > > kbd_send_chars(). I think only the GTK UI code is wrong.
> >
> > Why isn't GTK able to do things the same way all the other UI
> > frontends do, then ?
>
> In the other frontends the text consoles are not directly exposed, you
> just switch into them using QEMU's console switching shortcuts.
>
> In the GTK frontend, each text console is exposed directly as a notebook
> tab in the UI, so you don't need to use QEMU's console switching.
>
> IOW, the GTK frontend is more feature rich than the other frontends and
> causes it to hit this mistake.

Then we should probably have generic code to support "the frontend
implements multiple tabs or whatever itself", so that the 2nd frontend
that decides it wants to add this feature doesn't have to implement
the same code again... (Not something we need to do for 6.1, to be
clear. I just tend to feel we don't do enough UI stuff in common
code and have weird disparities in featureset etc between our
various UI frontends.)

-- PMM

Re: [PATCH for 6.1 1/2] ui/gtk: add a keyboard fifo to the VTE consoles

[Gerd Hoffmann]

  Hi,

> +static void gd_vc_send_chars(VirtualConsole *vc)
> +{
> +    uint32_t len, avail;
> +    const uint8_t *buf;
> +
> +    len = qemu_chr_be_can_write(vc->vte.chr);
> +    avail = fifo8_num_used(&vc->vte.out_fifo);
> +    if (len > avail) {
> +        len = avail;
> +    }
> +    while (len > 0) {
> +        uint32_t size;
> +
> +        buf = fifo8_pop_buf(&vc->vte.out_fifo, len, &size);
> +        qemu_chr_be_write(vc->vte.chr, (uint8_t *)buf, size);
> +        len -= size;
> +        avail -= size;
> +    }
> +    /*
> +     * characters are pending: we send them a bit later (XXX:
> +     * horrible, should change char device API)
> +     */
> +    if (avail > 0) {
> +        timer_mod(vc->vte.kbd_timer,
> +                  qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1);
> +    }

There is ChardevClass->chr_accept_input() which gets called when you can
send more data, so there is no need to use a timer for that.

Typical workflow is to only read data when it can be pushed forward to
the guest, so when the guest stops reading data qemu stops doing so too,
effectively forwarding the stalls.  Which works fine for things like tcp
sockets.  Not so much for user input though.

So, yes, just throw away data is the only option we have here.  Adding a
reasonable-sized fifo makes sense too to cover bulky input, so you can
cut+paste a longish URL even if the guest accepts only a few chars at a
time (16550 fifo is 16 chars IIRC ...).

I would suggest to keep things simple, just throw away what you can't
store in the fifo, I don't see the point taking different actions
depending on how long the stalls are lasting (patch 2/2).

take care,
  Gerd

Re: [PATCH for 6.1 1/2] ui/gtk: add a keyboard fifo to the VTE consoles

[Volker Rümelin]

>> +static void gd_vc_send_chars(VirtualConsole *vc)
>> +{
>> +    uint32_t len, avail;
>> +    const uint8_t *buf;
>> +
>> +    len = qemu_chr_be_can_write(vc->vte.chr);
>> +    avail = fifo8_num_used(&vc->vte.out_fifo);
>> +    if (len > avail) {
>> +        len = avail;
>> +    }
>> +    while (len > 0) {
>> +        uint32_t size;
>> +
>> +        buf = fifo8_pop_buf(&vc->vte.out_fifo, len, &size);
>> +        qemu_chr_be_write(vc->vte.chr, (uint8_t *)buf, size);
>> +        len -= size;
>> +        avail -= size;
>> +    }
>> +    /*
>> +     * characters are pending: we send them a bit later (XXX:
>> +     * horrible, should change char device API)
>> +     */
>> +    if (avail > 0) {
>> +        timer_mod(vc->vte.kbd_timer,
>> +                  qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1);
>> +    }
> There is ChardevClass->chr_accept_input() which gets called when you can
> send more data, so there is no need to use a timer for that.

Oh, I didn't notice this callback function. With this, the retry timer 
and my attempt to quickly slow down the write retries are really not 
necessary.

>
> Typical workflow is to only read data when it can be pushed forward to
> the guest, so when the guest stops reading data qemu stops doing so too,
> effectively forwarding the stalls.  Which works fine for things like tcp
> sockets.  Not so much for user input though.
>
> So, yes, just throw away data is the only option we have here.  Adding a
> reasonable-sized fifo makes sense too to cover bulky input, so you can
> cut+paste a longish URL even if the guest accepts only a few chars at a
> time (16550 fifo is 16 chars IIRC ...).
>
> I would suggest to keep things simple, just throw away what you can't
> store in the fifo, I don't see the point taking different actions
> depending on how long the stalls are lasting (patch 2/2).

I will send a version 2 patch.

With best regards,
Volker