| Message ID | 24a0619c9d6c07c5b10f5aff19478ff3040c4af8.1269373411.git.shaharh@redhat.com |
|---|---|
| State | New |
| Headers | show |
Shahar Havivi <shaharh@redhat.com> writes: > Two new monitor commands: adding ability to handle which keyboard qemu will > use and to see which keyboard are currently available. > > $ info keyboard > $ keyboard_set <index> > > Signed-off-by: Shahar Havivi <shaharh@redhat.com> > --- > console.h | 4 ++ > input.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > monitor.c | 8 ++++ > qemu-monitor.hx | 17 +++++++++ > qerror.c | 8 ++++ > 5 files changed, 141 insertions(+), 0 deletions(-) > > diff --git a/console.h b/console.h > index 16c9c3d..7efa88e 100644 > --- a/console.h > +++ b/console.h > @@ -85,6 +85,10 @@ void do_info_mice_print(Monitor *mon, const QObject *data); > void do_info_mice(Monitor *mon, QObject **ret_data); > void do_mouse_set(Monitor *mon, const QDict *qdict); > > +void do_info_keyboard_print(Monitor *mon, const QObject *data); > +void do_info_keyboard(Monitor *mon, QObject **ret_data); > +int do_keyboard_set(Monitor *mon, const QDict *qdict, QObject **ret_data); > + > /* keysym is a unicode code except for special keys (see QEMU_KEY_xxx > constants) */ > #define QEMU_KEY_ESC1(c) ((c) | 0xe100) > diff --git a/input.c b/input.c > index 563ecad..4daaeb0 100644 > --- a/input.c > +++ b/input.c > @@ -368,3 +368,107 @@ void qemu_remove_mouse_mode_change_notifier(Notifier *notify) > { > notifier_list_remove(&mouse_mode_notifiers, notify); > } > + > +static void info_keyboard_iter(QObject *data, void *opaque) > +{ > + QDict *kbd; > + Monitor *mon = opaque; > + > + kbd = qobject_to_qdict(data); > + monitor_printf(mon, "%c Keyboard #%" PRId64 ": %s\n", > + (qdict_get_bool(kbd, "current") ? '*' : ' '), > + qdict_get_int(kbd, "index"), qdict_get_str(kbd, "name")); > +} > + > +void do_info_keyboard_print(Monitor *mon, const QObject *data) > +{ > + QList *kbd_list; > + > + kbd_list = qobject_to_qlist(data); > + if (qlist_empty(kbd_list)) { > + monitor_printf(mon, "No keyboard devices connected\n"); > + return; > + } > + > + qlist_iter(kbd_list, info_keyboard_iter, mon); > +} > + > +/* > + * do_info_keyboard(): Show VM keyboard information > + * > + * Each keyboard is represented by a QDict, the returned QObject is > + * a QList of all keyboards. > + * > + * The keyboard QDict contains the following: > + * > + * - "name": keyboard's name > + * - "index": keyboard's index > + * - "current": true if this keyboard is receiving events, false otherwise > + * > + * Example: > + * > + * [ { "name": "QEMU USB Keyboard", "index": 0, "current": false }, > + * { "name": "QEMU PS/2 Keyboard", "index": 1, "current": true } ] > + */ > +void do_info_keyboard(Monitor *mon, QObject **ret_data) > +{ > + QEMUPutKbdEntry *cursor; > + QList *kbd_list; > + int index = 0; > + > + kbd_list = qlist_new(); > + > + if (!qemu_put_kbd_event_head) { > + goto out; > + } > + > + cursor = qemu_put_kbd_event_head; > + while (cursor != NULL) { > + QObject *obj; > + obj = qobject_from_jsonf("{ 'name': %s, 'index': %d, 'current': %i }", > + cursor->qemu_put_kbd_name, > + index, cursor == qemu_put_kbd_event_current); > + qlist_append_obj(kbd_list, obj); > + index++; > + cursor = cursor->next; > + } > +out: > + *ret_data = QOBJECT(kbd_list); > +} Keyboard indexes change when keyboards other than the last one get removed. Hmm. > + > +/* > + * do_keyboard_set(): Set active keyboard > + * > + * Argument qdict contains > + * - "index": the keyboard index to set > + * > + * Example: > + * > + * { "index": "0" } > + */ > +int do_keyboard_set(Monitor *mon, const QDict *qdict, QObject **ret_data) > +{ > + QEMUPutKbdEntry *cursor; > + int i = 0; > + int index = qdict_get_int(qdict, "index"); > + > + if (!qemu_put_kbd_event_head) { > + qerror_report(QERR_DEVICE_NOT_FOUND, "keyboard"); > + return -1; > + } > + > + cursor = qemu_put_kbd_event_head; > + while (cursor != NULL && index != i) { > + i++; > + cursor = cursor->next; > + } > + > + if (cursor != NULL) { > + qemu_put_kbd_event_current = cursor; > + } > + else { > + qerror_report(QERR_INVALID_PARAMETER, "index"); > + return -1; > + } > + return 0; > +} Since the index is not a stable identifier of keyboards, and we support multiple monitors, you're prone to remove the wrong keyboard. Monitor#1 Monitor#2 "info keyboard" to find the index unplug a keyboard, invalidating index keyboard_set <index> I think it's best to use a stable ID, like we do in other places. We commonly let the user specify it, e.g. as id=ID in -netdev, -device and elswhere. > diff --git a/monitor.c b/monitor.c > index 0448a70..cc95b3d 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -2783,6 +2783,14 @@ static const mon_cmd_t info_cmds[] = { > .mhandler.info_new = do_info_mice, > }, > { > + .name = "keyboard", > + .args_type = "", > + .params = "", > + .help = "show which guest keyboard is receiving events", > + .user_print = do_info_keyboard_print, > + .mhandler.info_new = do_info_keyboard, > + }, > + { > .name = "vnc", > .args_type = "", > .params = "", > diff --git a/qemu-monitor.hx b/qemu-monitor.hx > index 5308f36..e9beb12 100644 > --- a/qemu-monitor.hx > +++ b/qemu-monitor.hx > @@ -659,6 +659,23 @@ info mice > @end example > ETEXI > > + { > + .name = "keyboard_set", > + .args_type = "index:i", > + .params = "index", > + .help = "set which keyboard device receives events", > + .mhandler.cmd_new = do_keyboard_set, > + }, > + > +STEXI > +@item keyboard_set @var{index} > +@findex keyboard_set > +Set which keyboard device receives events at given @var{index}, index > +can be obtained with > +@example > +info keyboard > +@end example > +ETEXI > #ifdef HAS_AUDIO > { > .name = "wavcapture", > diff --git a/qerror.c b/qerror.c > index d0aba61..42a5cf7 100644 > --- a/qerror.c > +++ b/qerror.c > @@ -172,6 +172,14 @@ static const QErrorStringTable qerror_table[] = { > .error_fmt = QERR_VNC_SERVER_FAILED, > .desc = "Could not start VNC server on %(target)", > }, > + { > + .error_fmt = QERR_DEVICE_NOT_FOUND, > + .desc = "No keyboard device found", > + }, > + { > + .error_fmt = QERR_INVALID_PARAMETER, > + .desc = "Invalid index '%(name)' for keyboard device", > + }, > {} > }; I'm afraid you're breaking all other uses of these errors here.
On Fri, Mar 26, 2010 at 10:57:58AM +0100, Markus Armbruster wrote: > Date: Fri, 26 Mar 2010 10:57:58 +0100 > From: Markus Armbruster <armbru@redhat.com> > To: Shahar Havivi <shaharh@redhat.com> > Cc: qemu-devel@nongnu.org > Subject: Re: [Qemu-devel] [PATCH 2/2] Added monitor commands: > 'keyboard_set' and 'info keybaord' > > Shahar Havivi <shaharh@redhat.com> writes: > > > Two new monitor commands: adding ability to handle which keyboard qemu will > > use and to see which keyboard are currently available. > > > > $ info keyboard > > $ keyboard_set <index> > > > > Signed-off-by: Shahar Havivi <shaharh@redhat.com> > > --- > > console.h | 4 ++ > > input.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > monitor.c | 8 ++++ > > qemu-monitor.hx | 17 +++++++++ > > qerror.c | 8 ++++ > > 5 files changed, 141 insertions(+), 0 deletions(-) > > > > diff --git a/console.h b/console.h > > index 16c9c3d..7efa88e 100644 > > --- a/console.h > > +++ b/console.h > > @@ -85,6 +85,10 @@ void do_info_mice_print(Monitor *mon, const QObject *data); > > void do_info_mice(Monitor *mon, QObject **ret_data); > > void do_mouse_set(Monitor *mon, const QDict *qdict); > > > > +void do_info_keyboard_print(Monitor *mon, const QObject *data); > > +void do_info_keyboard(Monitor *mon, QObject **ret_data); > > +int do_keyboard_set(Monitor *mon, const QDict *qdict, QObject **ret_data); > > + > > /* keysym is a unicode code except for special keys (see QEMU_KEY_xxx > > constants) */ > > #define QEMU_KEY_ESC1(c) ((c) | 0xe100) > > diff --git a/input.c b/input.c > > index 563ecad..4daaeb0 100644 > > --- a/input.c > > +++ b/input.c > > @@ -368,3 +368,107 @@ void qemu_remove_mouse_mode_change_notifier(Notifier *notify) > > { > > notifier_list_remove(&mouse_mode_notifiers, notify); > > } > > + > > +static void info_keyboard_iter(QObject *data, void *opaque) > > +{ > > + QDict *kbd; > > + Monitor *mon = opaque; > > + > > + kbd = qobject_to_qdict(data); > > + monitor_printf(mon, "%c Keyboard #%" PRId64 ": %s\n", > > + (qdict_get_bool(kbd, "current") ? '*' : ' '), > > + qdict_get_int(kbd, "index"), qdict_get_str(kbd, "name")); > > +} > > + > > +void do_info_keyboard_print(Monitor *mon, const QObject *data) > > +{ > > + QList *kbd_list; > > + > > + kbd_list = qobject_to_qlist(data); > > + if (qlist_empty(kbd_list)) { > > + monitor_printf(mon, "No keyboard devices connected\n"); > > + return; > > + } > > + > > + qlist_iter(kbd_list, info_keyboard_iter, mon); > > +} > > + > > +/* > > + * do_info_keyboard(): Show VM keyboard information > > + * > > + * Each keyboard is represented by a QDict, the returned QObject is > > + * a QList of all keyboards. > > + * > > + * The keyboard QDict contains the following: > > + * > > + * - "name": keyboard's name > > + * - "index": keyboard's index > > + * - "current": true if this keyboard is receiving events, false otherwise > > + * > > + * Example: > > + * > > + * [ { "name": "QEMU USB Keyboard", "index": 0, "current": false }, > > + * { "name": "QEMU PS/2 Keyboard", "index": 1, "current": true } ] > > + */ > > +void do_info_keyboard(Monitor *mon, QObject **ret_data) > > +{ > > + QEMUPutKbdEntry *cursor; > > + QList *kbd_list; > > + int index = 0; > > + > > + kbd_list = qlist_new(); > > + > > + if (!qemu_put_kbd_event_head) { > > + goto out; > > + } > > + > > + cursor = qemu_put_kbd_event_head; > > + while (cursor != NULL) { > > + QObject *obj; > > + obj = qobject_from_jsonf("{ 'name': %s, 'index': %d, 'current': %i }", > > + cursor->qemu_put_kbd_name, > > + index, cursor == qemu_put_kbd_event_current); > > + qlist_append_obj(kbd_list, obj); > > + index++; > > + cursor = cursor->next; > > + } > > +out: > > + *ret_data = QOBJECT(kbd_list); > > +} > > Keyboard indexes change when keyboards other than the last one get > removed. Hmm. > > > + > > +/* > > + * do_keyboard_set(): Set active keyboard > > + * > > + * Argument qdict contains > > + * - "index": the keyboard index to set > > + * > > + * Example: > > + * > > + * { "index": "0" } > > + */ > > +int do_keyboard_set(Monitor *mon, const QDict *qdict, QObject **ret_data) > > +{ > > + QEMUPutKbdEntry *cursor; > > + int i = 0; > > + int index = qdict_get_int(qdict, "index"); > > + > > + if (!qemu_put_kbd_event_head) { > > + qerror_report(QERR_DEVICE_NOT_FOUND, "keyboard"); > > + return -1; > > + } > > + > > + cursor = qemu_put_kbd_event_head; > > + while (cursor != NULL && index != i) { > > + i++; > > + cursor = cursor->next; > > + } > > + > > + if (cursor != NULL) { > > + qemu_put_kbd_event_current = cursor; > > + } > > + else { > > + qerror_report(QERR_INVALID_PARAMETER, "index"); > > + return -1; > > + } > > + return 0; > > +} > > Since the index is not a stable identifier of keyboards, and we support > multiple monitors, you're prone to remove the wrong keyboard. > > Monitor#1 Monitor#2 > "info keyboard" to find the index > unplug a keyboard, invalidating > index > keyboard_set <index> > > I think it's best to use a stable ID, like we do in other places. We > commonly let the user specify it, e.g. as id=ID in -netdev, -device and > elswhere. What do we do when user not specify id for device? > > > diff --git a/monitor.c b/monitor.c > > index 0448a70..cc95b3d 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -2783,6 +2783,14 @@ static const mon_cmd_t info_cmds[] = { > > .mhandler.info_new = do_info_mice, > > }, > > { > > + .name = "keyboard", > > + .args_type = "", > > + .params = "", > > + .help = "show which guest keyboard is receiving events", > > + .user_print = do_info_keyboard_print, > > + .mhandler.info_new = do_info_keyboard, > > + }, > > + { > > .name = "vnc", > > .args_type = "", > > .params = "", > > diff --git a/qemu-monitor.hx b/qemu-monitor.hx > > index 5308f36..e9beb12 100644 > > --- a/qemu-monitor.hx > > +++ b/qemu-monitor.hx > > @@ -659,6 +659,23 @@ info mice > > @end example > > ETEXI > > > > + { > > + .name = "keyboard_set", > > + .args_type = "index:i", > > + .params = "index", > > + .help = "set which keyboard device receives events", > > + .mhandler.cmd_new = do_keyboard_set, > > + }, > > + > > +STEXI > > +@item keyboard_set @var{index} > > +@findex keyboard_set > > +Set which keyboard device receives events at given @var{index}, index > > +can be obtained with > > +@example > > +info keyboard > > +@end example > > +ETEXI > > #ifdef HAS_AUDIO > > { > > .name = "wavcapture", > > diff --git a/qerror.c b/qerror.c > > index d0aba61..42a5cf7 100644 > > --- a/qerror.c > > +++ b/qerror.c > > @@ -172,6 +172,14 @@ static const QErrorStringTable qerror_table[] = { > > .error_fmt = QERR_VNC_SERVER_FAILED, > > .desc = "Could not start VNC server on %(target)", > > }, > > + { > > + .error_fmt = QERR_DEVICE_NOT_FOUND, > > + .desc = "No keyboard device found", > > + }, > > + { > > + .error_fmt = QERR_INVALID_PARAMETER, > > + .desc = "Invalid index '%(name)' for keyboard device", > > + }, > > {} > > }; > > I'm afraid you're breaking all other uses of these errors here. you right, my bad - this errors where not meant to be in this patch.
Shahar Havivi <shaharh@redhat.com> writes: > On Fri, Mar 26, 2010 at 10:57:58AM +0100, Markus Armbruster wrote: [...] >> Since the index is not a stable identifier of keyboards, and we support >> multiple monitors, you're prone to remove the wrong keyboard. >> >> Monitor#1 Monitor#2 >> "info keyboard" to find the index >> unplug a keyboard, invalidating >> index >> keyboard_set <index> >> >> I think it's best to use a stable ID, like we do in other places. We >> commonly let the user specify it, e.g. as id=ID in -netdev, -device and >> elswhere. > What do we do when user not specify id for device? Two obvious options: make one up (problem: clashes), or stipulate "if you want to keyboard_set this keyboard, you must specify an id for it". [...]
On Wed, Mar 31, 2010 at 05:10:22PM +0200, Markus Armbruster wrote: > Date: Wed, 31 Mar 2010 17:10:22 +0200 > From: Markus Armbruster <armbru@redhat.com> > To: Shahar Havivi <shaharh@redhat.com> > Cc: qemu-devel@nongnu.org > Subject: Re: [Qemu-devel] [PATCH 2/2] Added monitor commands: > 'keyboard_set' and 'info keybaord' > > Shahar Havivi <shaharh@redhat.com> writes: > > > On Fri, Mar 26, 2010 at 10:57:58AM +0100, Markus Armbruster wrote: > [...] > >> Since the index is not a stable identifier of keyboards, and we support > >> multiple monitors, you're prone to remove the wrong keyboard. > >> > >> Monitor#1 Monitor#2 > >> "info keyboard" to find the index > >> unplug a keyboard, invalidating > >> index > >> keyboard_set <index> > >> > >> I think it's best to use a stable ID, like we do in other places. We > >> commonly let the user specify it, e.g. as id=ID in -netdev, -device and > >> elswhere. > > What do we do when user not specify id for device? > > Two obvious options: make one up (problem: clashes), or stipulate "if > you want to keyboard_set this keyboard, you must specify an id for it". > > [...] How about static auto incremented id like the mouse does?
Markus Armbruster <armbru@redhat.com> wrote: > Shahar Havivi <shaharh@redhat.com> writes: > >> On Fri, Mar 26, 2010 at 10:57:58AM +0100, Markus Armbruster wrote: > [...] >>> Since the index is not a stable identifier of keyboards, and we support >>> multiple monitors, you're prone to remove the wrong keyboard. >>> >>> Monitor#1 Monitor#2 >>> "info keyboard" to find the index >>> unplug a keyboard, invalidating >>> index >>> keyboard_set <index> >>> >>> I think it's best to use a stable ID, like we do in other places. We >>> commonly let the user specify it, e.g. as id=ID in -netdev, -device and >>> elswhere. >> What do we do when user not specify id for device? > > Two obvious options: make one up (problem: clashes), or stipulate "if > you want to keyboard_set this keyboard, you must specify an id for it". Second one is simpler, so ..... O:-) Later, Juan.
diff --git a/console.h b/console.h index 16c9c3d..7efa88e 100644 --- a/console.h +++ b/console.h @@ -85,6 +85,10 @@ void do_info_mice_print(Monitor *mon, const QObject *data); void do_info_mice(Monitor *mon, QObject **ret_data); void do_mouse_set(Monitor *mon, const QDict *qdict); +void do_info_keyboard_print(Monitor *mon, const QObject *data); +void do_info_keyboard(Monitor *mon, QObject **ret_data); +int do_keyboard_set(Monitor *mon, const QDict *qdict, QObject **ret_data); + /* keysym is a unicode code except for special keys (see QEMU_KEY_xxx constants) */ #define QEMU_KEY_ESC1(c) ((c) | 0xe100) diff --git a/input.c b/input.c index 563ecad..4daaeb0 100644 --- a/input.c +++ b/input.c @@ -368,3 +368,107 @@ void qemu_remove_mouse_mode_change_notifier(Notifier *notify) { notifier_list_remove(&mouse_mode_notifiers, notify); } + +static void info_keyboard_iter(QObject *data, void *opaque) +{ + QDict *kbd; + Monitor *mon = opaque; + + kbd = qobject_to_qdict(data); + monitor_printf(mon, "%c Keyboard #%" PRId64 ": %s\n", + (qdict_get_bool(kbd, "current") ? '*' : ' '), + qdict_get_int(kbd, "index"), qdict_get_str(kbd, "name")); +} + +void do_info_keyboard_print(Monitor *mon, const QObject *data) +{ + QList *kbd_list; + + kbd_list = qobject_to_qlist(data); + if (qlist_empty(kbd_list)) { + monitor_printf(mon, "No keyboard devices connected\n"); + return; + } + + qlist_iter(kbd_list, info_keyboard_iter, mon); +} + +/* + * do_info_keyboard(): Show VM keyboard information + * + * Each keyboard is represented by a QDict, the returned QObject is + * a QList of all keyboards. + * + * The keyboard QDict contains the following: + * + * - "name": keyboard's name + * - "index": keyboard's index + * - "current": true if this keyboard is receiving events, false otherwise + * + * Example: + * + * [ { "name": "QEMU USB Keyboard", "index": 0, "current": false }, + * { "name": "QEMU PS/2 Keyboard", "index": 1, "current": true } ] + */ +void do_info_keyboard(Monitor *mon, QObject **ret_data) +{ + QEMUPutKbdEntry *cursor; + QList *kbd_list; + int index = 0; + + kbd_list = qlist_new(); + + if (!qemu_put_kbd_event_head) { + goto out; + } + + cursor = qemu_put_kbd_event_head; + while (cursor != NULL) { + QObject *obj; + obj = qobject_from_jsonf("{ 'name': %s, 'index': %d, 'current': %i }", + cursor->qemu_put_kbd_name, + index, cursor == qemu_put_kbd_event_current); + qlist_append_obj(kbd_list, obj); + index++; + cursor = cursor->next; + } +out: + *ret_data = QOBJECT(kbd_list); +} + +/* + * do_keyboard_set(): Set active keyboard + * + * Argument qdict contains + * - "index": the keyboard index to set + * + * Example: + * + * { "index": "0" } + */ +int do_keyboard_set(Monitor *mon, const QDict *qdict, QObject **ret_data) +{ + QEMUPutKbdEntry *cursor; + int i = 0; + int index = qdict_get_int(qdict, "index"); + + if (!qemu_put_kbd_event_head) { + qerror_report(QERR_DEVICE_NOT_FOUND, "keyboard"); + return -1; + } + + cursor = qemu_put_kbd_event_head; + while (cursor != NULL && index != i) { + i++; + cursor = cursor->next; + } + + if (cursor != NULL) { + qemu_put_kbd_event_current = cursor; + } + else { + qerror_report(QERR_INVALID_PARAMETER, "index"); + return -1; + } + return 0; +} diff --git a/monitor.c b/monitor.c index 0448a70..cc95b3d 100644 --- a/monitor.c +++ b/monitor.c @@ -2783,6 +2783,14 @@ static const mon_cmd_t info_cmds[] = { .mhandler.info_new = do_info_mice, }, { + .name = "keyboard", + .args_type = "", + .params = "", + .help = "show which guest keyboard is receiving events", + .user_print = do_info_keyboard_print, + .mhandler.info_new = do_info_keyboard, + }, + { .name = "vnc", .args_type = "", .params = "", diff --git a/qemu-monitor.hx b/qemu-monitor.hx index 5308f36..e9beb12 100644 --- a/qemu-monitor.hx +++ b/qemu-monitor.hx @@ -659,6 +659,23 @@ info mice @end example ETEXI + { + .name = "keyboard_set", + .args_type = "index:i", + .params = "index", + .help = "set which keyboard device receives events", + .mhandler.cmd_new = do_keyboard_set, + }, + +STEXI +@item keyboard_set @var{index} +@findex keyboard_set +Set which keyboard device receives events at given @var{index}, index +can be obtained with +@example +info keyboard +@end example +ETEXI #ifdef HAS_AUDIO { .name = "wavcapture", diff --git a/qerror.c b/qerror.c index d0aba61..42a5cf7 100644 --- a/qerror.c +++ b/qerror.c @@ -172,6 +172,14 @@ static const QErrorStringTable qerror_table[] = { .error_fmt = QERR_VNC_SERVER_FAILED, .desc = "Could not start VNC server on %(target)", }, + { + .error_fmt = QERR_DEVICE_NOT_FOUND, + .desc = "No keyboard device found", + }, + { + .error_fmt = QERR_INVALID_PARAMETER, + .desc = "Invalid index '%(name)' for keyboard device", + }, {} };
Two new monitor commands: adding ability to handle which keyboard qemu will use and to see which keyboard are currently available. $ info keyboard $ keyboard_set <index> Signed-off-by: Shahar Havivi <shaharh@redhat.com> --- console.h | 4 ++ input.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ monitor.c | 8 ++++ qemu-monitor.hx | 17 +++++++++ qerror.c | 8 ++++ 5 files changed, 141 insertions(+), 0 deletions(-)