Re: [Qemu-devel] [PATCH 2/2] input: introduce keyboard handler list

[Markus Armbruster <armbru@redhat.com>]

Gerd Hoffmann <kraxel@redhat.com> writes:

> Add a linked list of keyboard handlers.  Added handlers will go
> to the head of the list.  Removed handlers will be zapped from
> the list.  The head of the list will be used for events.
>
> This fixes the keyboard-dead-after-usb-kbd-unplug issue, key events
> will be re-routed to the ps/2 kbd instead of being discarded.

Patterned after the mouse code, which already has such a list.

For mice, we have a monitor command mouse_set to select the active one.
Useful for keyboards, too?  I dimly recall somebody trying something
like that in the past.

> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/hid.c             |    4 ++--
>  hw/hid.h             |    1 +
>  include/ui/console.h |    6 ++++--
>  ui/input.c           |   37 +++++++++++++++++++++++++------------
>  4 files changed, 32 insertions(+), 16 deletions(-)
>
> diff --git a/hw/hid.c b/hw/hid.c
> index 89b5415..6be00ec 100644
> --- a/hw/hid.c
> +++ b/hw/hid.c
> @@ -415,7 +415,7 @@ void hid_free(HIDState *hs)
>  {
>      switch (hs->kind) {
>      case HID_KEYBOARD:
> -        qemu_remove_kbd_event_handler();
> +        qemu_remove_kbd_event_handler(hs->kbd.eh_entry);
>          break;
>      case HID_MOUSE:
>      case HID_TABLET:
> @@ -431,7 +431,7 @@ void hid_init(HIDState *hs, int kind, HIDEventFunc event)
>      hs->event = event;
>  
>      if (hs->kind == HID_KEYBOARD) {
> -        qemu_add_kbd_event_handler(hid_keyboard_event, hs);
> +        hs->kbd.eh_entry = qemu_add_kbd_event_handler(hid_keyboard_event, hs);
>      } else if (hs->kind == HID_MOUSE) {
>          hs->ptr.eh_entry = qemu_add_mouse_event_handler(hid_pointer_event, hs,
>                                                          0, "QEMU HID Mouse");
> diff --git a/hw/hid.h b/hw/hid.h
> index 56c71ed..2567879 100644
> --- a/hw/hid.h
> +++ b/hw/hid.h
> @@ -31,6 +31,7 @@ typedef struct HIDKeyboardState {
>      uint8_t leds;
>      uint8_t key[16];
>      int32_t keys;
> +    QEMUPutKbdEntry *eh_entry;
>  } HIDKeyboardState;
>  
>  struct HIDState {
> diff --git a/include/ui/console.h b/include/ui/console.h
> index ce5a550..fd39d94 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -28,10 +28,12 @@ typedef void QEMUPutLEDEvent(void *opaque, int ledstate);
>  typedef void QEMUPutMouseEvent(void *opaque, int dx, int dy, int dz, int buttons_state);
>  
>  typedef struct QEMUPutMouseEntry QEMUPutMouseEntry;
> +typedef struct QEMUPutKbdEntry QEMUPutKbdEntry;
>  typedef struct QEMUPutLEDEntry QEMUPutLEDEntry;
>  
> -void qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque);
> -void qemu_remove_kbd_event_handler(void);
> +QEMUPutKbdEntry *qemu_add_kbd_event_handler(QEMUPutKBDEvent *func,
> +                                            void *opaque);
> +void qemu_remove_kbd_event_handler(QEMUPutKbdEntry *entry);
>  QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func,
>                                                  void *opaque, int absolute,
>                                                  const char *name);
> diff --git a/ui/input.c b/ui/input.c
> index 87a23df..59d0578 100644
> --- a/ui/input.c
> +++ b/ui/input.c
> @@ -41,18 +41,25 @@ struct QEMUPutMouseEntry {
>      QTAILQ_ENTRY(QEMUPutMouseEntry) node;
>  };
>  
> +struct QEMUPutKbdEntry {
> +    QEMUPutLEDEvent *put_kbd;

Sure it's not QEMUPutKbdEvent?

> +    void *opaque;
> +    QTAILQ_ENTRY(QEMUPutKbdEntry) next;
> +};
> +
>  struct QEMUPutLEDEntry {
>      QEMUPutLEDEvent *put_led;
>      void *opaque;
>      QTAILQ_ENTRY(QEMUPutLEDEntry) next;
>  };
>  
> -static QEMUPutKBDEvent *qemu_put_kbd_event;
> -static void *qemu_put_kbd_event_opaque;
> -static QTAILQ_HEAD(, QEMUPutLEDEntry) led_handlers = QTAILQ_HEAD_INITIALIZER(led_handlers);
> +static QTAILQ_HEAD(, QEMUPutLEDEntry) led_handlers =
> +    QTAILQ_HEAD_INITIALIZER(led_handlers);

Unrelated style cleanup.  Tolerable.

> +static QTAILQ_HEAD(, QEMUPutKbdEntry) kbd_handlers =
> +    QTAILQ_HEAD_INITIALIZER(kbd_handlers);
>  static QTAILQ_HEAD(, QEMUPutMouseEntry) mouse_handlers =
>      QTAILQ_HEAD_INITIALIZER(mouse_handlers);
> -static NotifierList mouse_mode_notifiers = 
> +static NotifierList mouse_mode_notifiers =
>      NOTIFIER_LIST_INITIALIZER(mouse_mode_notifiers);

Unrelated whitespace cleanup.  Tolerable.

>  
>  static const int key_defs[] = {
> @@ -306,16 +313,20 @@ void qmp_send_key(KeyValueList *keys, bool has_hold_time, int64_t hold_time,
>                     muldiv64(get_ticks_per_sec(), hold_time, 1000));
>  }
>  
> -void qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque)
> +QEMUPutKbdEntry *qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque)
>  {
> -    qemu_put_kbd_event_opaque = opaque;
> -    qemu_put_kbd_event = func;
> +    QEMUPutKbdEntry *entry;
> +
> +    entry = g_malloc0(sizeof(QEMUPutKbdEntry));
> +    entry->put_kbd = func;
> +    entry->opaque = opaque;
> +    QTAILQ_INSERT_HEAD(&kbd_handlers, entry, next);
> +    return entry;
>  }
>  
> -void qemu_remove_kbd_event_handler(void)
> +void qemu_remove_kbd_event_handler(QEMUPutKbdEntry *entry)
>  {
> -    qemu_put_kbd_event_opaque = NULL;
> -    qemu_put_kbd_event = NULL;
> +    QTAILQ_REMOVE(&kbd_handlers, entry, next);
>  }
>  
>  static void check_mode_change(void)
> @@ -399,11 +410,13 @@ void qemu_remove_led_event_handler(QEMUPutLEDEntry *entry)
>  
>  void kbd_put_keycode(int keycode)
>  {
> +    QEMUPutKbdEntry *entry = QTAILQ_FIRST(&kbd_handlers);
> +
>      if (!runstate_is_running() && !runstate_check(RUN_STATE_SUSPENDED)) {
>          return;
>      }
> -    if (qemu_put_kbd_event) {
> -        qemu_put_kbd_event(qemu_put_kbd_event_opaque, keycode);
> +    if (entry) {
> +        entry->put_kbd(entry->opaque, keycode);
>      }
>  }