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