Message ID | 20100311215712.GA6606@redhat.com |
---|---|
State | New |
Headers | show |
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
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); } }
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(-)