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