Re: [PATCH 45/67] ui/vc: remove kby_put_keysym() and update function calls

[Akihiko Odaki]

On 2023/09/04 21:42, Marc-André Lureau wrote:
> Hi
>
> On Thu, Aug 31, 2023 at 12:59 AM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
> > On 2023/08/30 18:38, marcandre.lureau@redhat.com wrote:
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > The function calls to `kbd_put_keysym` have been updated to now call
> > > `kbd_put_keysym_console` with a NULL console parameter.
> > >
> > > Like most console functions, NULL argument is now for the active console.
> > >
> > > This will allow to rename the text console functions in a consistent manner.
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > >    include/ui/console.h |  1 -
> > >    ui/console.c         | 14 ++++++------
> > >    ui/curses.c          |  2 +-
> > >    ui/vnc.c             | 54 ++++++++++++++++++++++----------------------
> > >    ui/cocoa.m           |  2 +-
> > >    5 files changed, 36 insertions(+), 37 deletions(-)
> > >
> > > diff --git a/include/ui/console.h b/include/ui/console.h
> > > index 1ccd432b4d..9c362f0e87 100644
> > > --- a/include/ui/console.h
> > > +++ b/include/ui/console.h
> > > @@ -115,7 +115,6 @@ bool qemu_mouse_set(int index, Error **errp);
> > >    void kbd_put_keysym_console(QemuTextConsole *s, int keysym);
> > >    bool kbd_put_qcode_console(QemuTextConsole *s, int qcode, bool ctrl);
> > >    void kbd_put_string_console(QemuTextConsole *s, const char *str, int len);
> > > -void kbd_put_keysym(int keysym);
> > >
> > >    /* Touch devices */
> > >    typedef struct touch_slot {
> > > diff --git a/ui/console.c b/ui/console.c
> > > index a98adbb1b2..6068e02928 100644
> > > --- a/ui/console.c
> > > +++ b/ui/console.c
> > > @@ -1141,6 +1141,13 @@ void kbd_put_keysym_console(QemuTextConsole *s, int 
> > > keysym)
> > >        int c;
> > >        uint32_t num_free;
> > >
> > > +    if (!s) {
> > > +        if (!QEMU_IS_TEXT_CONSOLE(active_console)) {
> > > +            return;
> > > +        }
> > > +        s = QEMU_TEXT_CONSOLE(active_console);
> > > +    }
> > > +
> > >        switch(keysym) {
> > >        case QEMU_KEY_CTRL_UP:
> > >            console_scroll(s, -1);
> > > @@ -1231,13 +1238,6 @@ void kbd_put_string_console(QemuTextConsole *s, const 
> > > char *str, int len)
> > >        }
> > >    }
> > >
> > > -void kbd_put_keysym(int keysym)
> > > -{
> > > -    if (QEMU_IS_TEXT_CONSOLE(active_console)) {
> > > -        kbd_put_keysym_console(QEMU_TEXT_CONSOLE(active_console), keysym);
> > > -    }
> > > -}
> > > -
> > >    static void text_console_invalidate(void *opaque)
> > >    {
> > >        QemuTextConsole *s = QEMU_TEXT_CONSOLE(opaque);
> > > diff --git a/ui/curses.c b/ui/curses.c
> > > index de962faa7c..4ddbbae7cd 100644
> > > --- a/ui/curses.c
> > > +++ b/ui/curses.c
> > > @@ -400,7 +400,7 @@ static void curses_refresh(DisplayChangeListener *dcl)
> > >                if (keysym == -1)
> > >                    keysym = chr;
> > >
> > > -            kbd_put_keysym(keysym);
> > > +            kbd_put_keysym_console(NULL, keysym);
> > >            }
> > >        }
> > >    }
> > > diff --git a/ui/vnc.c b/ui/vnc.c
> > > index 92964dcc0c..1fa4456744 100644
> > > --- a/ui/vnc.c
> > > +++ b/ui/vnc.c
> > > @@ -1945,88 +1945,88 @@ static void do_key_event(VncState *vs, int down, int 
> > > keycode, int sym)
> > >                case 0xb8:                          /* Right ALT */
> > >                    break;
> > >                case 0xc8:
> > > -                kbd_put_keysym(QEMU_KEY_UP);
> > > +                kbd_put_keysym_console(NULL, QEMU_KEY_UP);
> >
> > vs->vd->dcl.con should be used instead. There may be VNC connections for
> > consoles other than the "active console" and in such a case
> > vs->vd->dcl.con != NULL. Unfortunately it seems that ui/vnc is very
> > buggy in such a situation.
>
> That's not how the console & VNC server code works. VNC server will
> send the key event to the hw anyway. But if the active_console is
> text/vc, then it also sends (some) of the key events to it. There is
> no "per-client" console either, the console switch is global
> (console_select()).

Actually vnc is capable of binding a VNC server instance to a particular 
console instead of active console controlled by global console_select() 
switch since commit 1d0d59fe29 ("vnc: allow binding servers to qemu 
consoles").

> Anyway, this patch is quite systematic. Further complicated changes
> should be done later. please ack/r-b

I see.

Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>

> thanks