Patchwork Support for multiple keyboard device

login
register
mail settings
Submitter Shahar Havivi
Date March 11, 2010, 9:57 p.m.
Message ID <20100311215712.GA6606@redhat.com>
Download mbox | patch
Permalink /patch/47634/
State New
Headers show

Comments

Shahar Havivi - March 11, 2010, 9:57 p.m.
Currently qemu use the last keyboard device that added,
When removing keyboard (via device_del kbd) you get segfault next time
you try to write in the client.

i.e. start qemu
  x86_64-softmmu/qemu-system-x86_64 -usb -device usb-kbd,id=kbd
switch to monitor
  device_del kbd
switch back to client, segfault

This patch fix the segfault and add list of all the keyboard handle much
like the mouse device does.


Signed-off-by: Shahar Havivi <shaharh@redhat.com>
---
 console.h    |    9 +++++-
 hw/usb-hid.c |    9 ++++--
 input.c      |   81 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 88 insertions(+), 11 deletions(-)
Anthony Liguori - March 17, 2010, 5:55 p.m.
On 03/11/2010 03:57 PM, Shahar Havivi wrote:
> Currently qemu use the last keyboard device that added,
> When removing keyboard (via device_del kbd) you get segfault next time
> you try to write in the client.
>
> i.e. start qemu
>    x86_64-softmmu/qemu-system-x86_64 -usb -device usb-kbd,id=kbd
> switch to monitor
>    device_del kbd
> switch back to client, segfault
>
> This patch fix the segfault and add list of all the keyboard handle much
> like the mouse device does.
>
>
> Signed-off-by: Shahar Havivi<shaharh@redhat.com>
>    

It's a good idea, but I'd like to commit my rework of input.c first 
(which I'll do in a couple hours).  A few comments:

> ---
>   console.h    |    9 +++++-
>   hw/usb-hid.c |    9 ++++--
>   input.c      |   81 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
>   3 files changed, 88 insertions(+), 11 deletions(-)
>
> diff --git a/console.h b/console.h
> index 71e8ff2..e250008 100644
> --- a/console.h
> +++ b/console.h
> @@ -38,7 +38,14 @@ typedef struct QEMUPutLEDEntry {
>       QTAILQ_ENTRY(QEMUPutLEDEntry) next;
>   } QEMUPutLEDEntry;
>
> -void qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque);
> +typedef struct QEMUPutKbdEntry {
> +    QEMUPutKBDEvent *qemu_put_kbd_event;
> +    void *qemu_put_kbd_event_opaque;
> +    struct QEMUPutKbdEntry *next;
> +} QEMUPutKbdEntry;
> +
> +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/hw/usb-hid.c b/hw/usb-hid.c
> index 2e4e647..1dd0cc9 100644
> --- a/hw/usb-hid.c
> +++ b/hw/usb-hid.c
> @@ -55,6 +55,7 @@ typedef struct USBKeyboardState {
>       uint8_t leds;
>       uint8_t key[16];
>       int keys;
> +    QEMUPutKbdEntry *eh_entry;
>   } USBKeyboardState;
>
>   typedef struct USBHIDState {
> @@ -635,7 +636,7 @@ static void usb_keyboard_handle_reset(USBDevice *dev)
>   {
>       USBHIDState *s = (USBHIDState *)dev;
>
> -    qemu_add_kbd_event_handler(usb_keyboard_event, s);
> +    s->kbd.eh_entry = qemu_add_kbd_event_handler(usb_keyboard_event, s);
>       s->protocol = 1;
>   }
>
> @@ -856,9 +857,11 @@ static void usb_hid_handle_destroy(USBDevice *dev)
>   {
>       USBHIDState *s = (USBHIDState *)dev;
>
> -    if (s->kind != USB_KEYBOARD)
> +    if (s->kind != USB_KEYBOARD) {
>           qemu_remove_mouse_event_handler(s->ptr.eh_entry);
> -    /* TODO: else */
> +    } else {
> +        qemu_remove_kbd_event_handler(s->kbd.eh_entry);
> +    }
>   }
>
>   static int usb_hid_initfn(USBDevice *dev, int kind)
> diff --git a/input.c b/input.c
> index baaa4c6..90b6cfb 100644
> --- a/input.c
> +++ b/input.c
> @@ -29,16 +29,82 @@
>   #include "qjson.h"
>
>
> -static QEMUPutKBDEvent *qemu_put_kbd_event;
> -static void *qemu_put_kbd_event_opaque;
> +static QEMUPutKbdEntry *qemu_put_kbd_event_head;
> +static QEMUPutKbdEntry *qemu_put_kbd_event_current;
>   static QEMUPutMouseEntry *qemu_put_mouse_event_head;
>   static QEMUPutMouseEntry *qemu_put_mouse_event_current;
>   static QTAILQ_HEAD(, QEMUPutLEDEntry) led_handlers = QTAILQ_HEAD_INITIALIZER(led_handlers);
>
> -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 *s, *cursor;
> +
> +    cursor = qemu_put_kbd_event_head;
> +    while (cursor) {
> +        if (cursor->qemu_put_kbd_event == func&&
> +            cursor->qemu_put_kbd_event_opaque == opaque) {
> +
> +            qemu_put_kbd_event_current = cursor;
> +            return cursor;
> +        }
> +        cursor = cursor->next;
> +    }
> +
> +    s = qemu_mallocz(sizeof(QEMUPutKbdEntry));
> +
> +    s->qemu_put_kbd_event_opaque = opaque;
> +    s->qemu_put_kbd_event = func;
> +    s->next = NULL;
> +
> +    if (!qemu_put_kbd_event_head) {
> +        qemu_put_kbd_event_head = s;
> +        qemu_put_kbd_event_current = s;
> +        return s;
> +    }
> +
> +    cursor = qemu_put_kbd_event_head;
> +    while (cursor->next) {
> +        cursor = cursor->next;
> +    }
> +
> +    cursor->next = s;
> +    qemu_put_kbd_event_current = s;
> +
> +    return s;
> +}
> +
> +void qemu_remove_kbd_event_handler(QEMUPutKbdEntry *entry)
> +{
> +    QEMUPutKbdEntry *prev = NULL, *cursor;
> +
> +    if (!qemu_put_kbd_event_head || !entry) {
> +        return;
> +    }
> +
> +    cursor = qemu_put_kbd_event_head;
> +    while (cursor&&  cursor != entry) {
> +        prev = cursor;
> +        cursor = cursor->next;
> +    }
> +
> +    if (cursor == NULL) {
> +        return;
> +    } else if (prev == NULL) {
> +        qemu_put_kbd_event_head = cursor->next;
> +        if (qemu_put_kbd_event_current == entry) {
> +            qemu_put_kbd_event_current = cursor->next;
> +        }
> +        qemu_free(entry);
> +        return;
> +    }
> +
> +    prev->next = entry->next;
> +
> +    if (qemu_put_kbd_event_current == entry) {
> +        qemu_put_kbd_event_current = prev;
> +    }
> +
> +    qemu_free(entry);
>   }
>
>   QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func,
> @@ -126,8 +192,9 @@ void qemu_remove_led_event_handler(QEMUPutLEDEntry *entry)
>
>   void kbd_put_keycode(int keycode)
>   {
> -    if (qemu_put_kbd_event) {
> -        qemu_put_kbd_event(qemu_put_kbd_event_opaque, keycode);
> +    if (qemu_put_kbd_event_current) {
> +        qemu_put_kbd_event_current->qemu_put_kbd_event(
> +            qemu_put_kbd_event_current->qemu_put_kbd_event_opaque, keycode);
>       }
>   }
>
>    

Shouldn't we have a keyboard_set monitor command (much like the 
mouse_set command)?  Also, it would be helpful to have an info keyboard.

Regards,

Anthony Liguori

Patch

diff --git a/console.h b/console.h
index 71e8ff2..e250008 100644
--- a/console.h
+++ b/console.h
@@ -38,7 +38,14 @@  typedef struct QEMUPutLEDEntry {
     QTAILQ_ENTRY(QEMUPutLEDEntry) next;
 } QEMUPutLEDEntry;
 
-void qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque);
+typedef struct QEMUPutKbdEntry {
+    QEMUPutKBDEvent *qemu_put_kbd_event;
+    void *qemu_put_kbd_event_opaque;
+    struct QEMUPutKbdEntry *next;
+} QEMUPutKbdEntry;
+
+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/hw/usb-hid.c b/hw/usb-hid.c
index 2e4e647..1dd0cc9 100644
--- a/hw/usb-hid.c
+++ b/hw/usb-hid.c
@@ -55,6 +55,7 @@  typedef struct USBKeyboardState {
     uint8_t leds;
     uint8_t key[16];
     int keys;
+    QEMUPutKbdEntry *eh_entry;
 } USBKeyboardState;
 
 typedef struct USBHIDState {
@@ -635,7 +636,7 @@  static void usb_keyboard_handle_reset(USBDevice *dev)
 {
     USBHIDState *s = (USBHIDState *)dev;
 
-    qemu_add_kbd_event_handler(usb_keyboard_event, s);
+    s->kbd.eh_entry = qemu_add_kbd_event_handler(usb_keyboard_event, s);
     s->protocol = 1;
 }
 
@@ -856,9 +857,11 @@  static void usb_hid_handle_destroy(USBDevice *dev)
 {
     USBHIDState *s = (USBHIDState *)dev;
 
-    if (s->kind != USB_KEYBOARD)
+    if (s->kind != USB_KEYBOARD) {
         qemu_remove_mouse_event_handler(s->ptr.eh_entry);
-    /* TODO: else */
+    } else {
+        qemu_remove_kbd_event_handler(s->kbd.eh_entry);
+    }
 }
 
 static int usb_hid_initfn(USBDevice *dev, int kind)
diff --git a/input.c b/input.c
index baaa4c6..90b6cfb 100644
--- a/input.c
+++ b/input.c
@@ -29,16 +29,82 @@ 
 #include "qjson.h"
 
 
-static QEMUPutKBDEvent *qemu_put_kbd_event;
-static void *qemu_put_kbd_event_opaque;
+static QEMUPutKbdEntry *qemu_put_kbd_event_head;
+static QEMUPutKbdEntry *qemu_put_kbd_event_current;
 static QEMUPutMouseEntry *qemu_put_mouse_event_head;
 static QEMUPutMouseEntry *qemu_put_mouse_event_current;
 static QTAILQ_HEAD(, QEMUPutLEDEntry) led_handlers = QTAILQ_HEAD_INITIALIZER(led_handlers);
 
-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 *s, *cursor;
+
+    cursor = qemu_put_kbd_event_head;
+    while (cursor) {
+        if (cursor->qemu_put_kbd_event == func &&
+            cursor->qemu_put_kbd_event_opaque == opaque) {
+
+            qemu_put_kbd_event_current = cursor;
+            return cursor;
+        }
+        cursor = cursor->next;
+    }
+
+    s = qemu_mallocz(sizeof(QEMUPutKbdEntry));
+
+    s->qemu_put_kbd_event_opaque = opaque;
+    s->qemu_put_kbd_event = func;
+    s->next = NULL;
+
+    if (!qemu_put_kbd_event_head) {
+        qemu_put_kbd_event_head = s;
+        qemu_put_kbd_event_current = s;
+        return s;
+    }
+
+    cursor = qemu_put_kbd_event_head;
+    while (cursor->next) {
+        cursor = cursor->next;
+    }
+
+    cursor->next = s;
+    qemu_put_kbd_event_current = s;
+
+    return s;
+}
+
+void qemu_remove_kbd_event_handler(QEMUPutKbdEntry *entry)
+{
+    QEMUPutKbdEntry *prev = NULL, *cursor;
+
+    if (!qemu_put_kbd_event_head || !entry) {
+        return;
+    }
+
+    cursor = qemu_put_kbd_event_head;
+    while (cursor && cursor != entry) {
+        prev = cursor;
+        cursor = cursor->next;
+    }
+
+    if (cursor == NULL) {
+        return;
+    } else if (prev == NULL) {
+        qemu_put_kbd_event_head = cursor->next;
+        if (qemu_put_kbd_event_current == entry) {
+            qemu_put_kbd_event_current = cursor->next;
+        }
+        qemu_free(entry);
+        return;
+    }
+
+    prev->next = entry->next;
+
+    if (qemu_put_kbd_event_current == entry) {
+        qemu_put_kbd_event_current = prev;
+    }
+
+    qemu_free(entry);
 }
 
 QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func,
@@ -126,8 +192,9 @@  void qemu_remove_led_event_handler(QEMUPutLEDEntry *entry)
 
 void kbd_put_keycode(int keycode)
 {
-    if (qemu_put_kbd_event) {
-        qemu_put_kbd_event(qemu_put_kbd_event_opaque, keycode);
+    if (qemu_put_kbd_event_current) {
+        qemu_put_kbd_event_current->qemu_put_kbd_event(
+            qemu_put_kbd_event_current->qemu_put_kbd_event_opaque, keycode);
     }
 }