| Message ID | 689f24e27d483de0e3201a9386c772dc241d4178.1270022701.git.shaharh@redhat.com |
|---|---|
| State | New |
| Headers | show |
Shahar Havivi <shaharh@redhat.com> wrote: > Two new monitor commands: adding ability to handle which keyboard qemu will > use and to see which keyboard are currently available. > +int do_keyboard_set(Monitor *mon, const QDict *qdict, QObject **ret_data) > +{ > + QEMUPutKbdEntry *cursor; > + int index = qdict_get_int(qdict, "index"); > + int found = 0; found variable is not used. > + > + if (QTAILQ_EMPTY(&kbd_handlers)) { > + qerror_report(QERR_DEVICE_NOT_FOUND, "keyboard"); > + return -1; > + } > + > + QTAILQ_FOREACH(cursor, &kbd_handlers, node) { > + if (cursor->index == index) { > + QTAILQ_REMOVE(&kbd_handlers, cursor, node); > + QTAILQ_INSERT_HEAD(&kbd_handlers, cursor, node); > + found = 1; well it is set :) > + break; > + } > + } > + > + return 0; I guess you want to return one error if the index don't exist. > +} I still think that adding an "id" property as in markus proposal would be neat. Otherwise I don't know how you are going to distinguish between two keyboards with the same name. Later, Juan.
Juan Quintela <quintela@redhat.com> writes: > Shahar Havivi <shaharh@redhat.com> wrote: >> Two new monitor commands: adding ability to handle which keyboard qemu will >> use and to see which keyboard are currently available. > >> +int do_keyboard_set(Monitor *mon, const QDict *qdict, QObject **ret_data) >> +{ >> + QEMUPutKbdEntry *cursor; >> + int index = qdict_get_int(qdict, "index"); >> + int found = 0; > > found variable is not used. > >> + >> + if (QTAILQ_EMPTY(&kbd_handlers)) { >> + qerror_report(QERR_DEVICE_NOT_FOUND, "keyboard"); >> + return -1; >> + } >> + >> + QTAILQ_FOREACH(cursor, &kbd_handlers, node) { >> + if (cursor->index == index) { >> + QTAILQ_REMOVE(&kbd_handlers, cursor, node); >> + QTAILQ_INSERT_HEAD(&kbd_handlers, cursor, node); >> + found = 1; > > well it is set :) > >> + break; >> + } >> + } >> + >> + return 0; > > I guess you want to return one error if the index don't exist. > >> +} > > I still think that adding an "id" property as in markus proposal would > be neat. Otherwise I don't know how you are going to distinguish > between two keyboards with the same name. If I understand the patch correctly (only time for a quick skim today), the keyboard receives a numeric ID when it is created, and keyboard_set identifies it by that ID. Yes, a user-defined ID would be nicer, and consistent with how similar things work. But the numeric ID isn't *wrong*, as far as I can see.
Markus Armbruster <armbru@redhat.com> wrote: > Juan Quintela <quintela@redhat.com> writes: >> I still think that adding an "id" property as in markus proposal would >> be neat. Otherwise I don't know how you are going to distinguish >> between two keyboards with the same name. > > If I understand the patch correctly (only time for a quick skim today), > the keyboard receives a numeric ID when it is created, and keyboard_set > identifies it by that ID. Yes, a user-defined ID would be nicer, and > consistent with how similar things work. But the numeric ID isn't > *wrong*, as far as I can see. my problem is that if you add two keyboards of the same type, they will receive random index (different ones), now you do info keyboard, and you see two keyboard with the same names and different numbers, how do you know which of the two given you want to choose? And no, I don't have magic bullet to make multimonitor/keyboard/mouse behave as expected out of given them right id's. Later, Juan.
On 03/31/2010 05:20 AM, Juan Quintela wrote: > Shahar Havivi<shaharh@redhat.com> wrote: > >> Two new monitor commands: adding ability to handle which keyboard qemu will >> use and to see which keyboard are currently available. >> > >> +int do_keyboard_set(Monitor *mon, const QDict *qdict, QObject **ret_data) >> +{ >> + QEMUPutKbdEntry *cursor; >> + int index = qdict_get_int(qdict, "index"); >> + int found = 0; >> > found variable is not used. > > >> + >> + if (QTAILQ_EMPTY(&kbd_handlers)) { >> + qerror_report(QERR_DEVICE_NOT_FOUND, "keyboard"); >> + return -1; >> + } >> + >> + QTAILQ_FOREACH(cursor,&kbd_handlers, node) { >> + if (cursor->index == index) { >> + QTAILQ_REMOVE(&kbd_handlers, cursor, node); >> + QTAILQ_INSERT_HEAD(&kbd_handlers, cursor, node); >> + found = 1; >> > well it is set :) > Please split out this bit into a qemu_activate_keyboard_handler() and plumb it with the usb device such that it's only used when a guest loads a driver for it. It should be very symmetric to how the mouse driver works. Regards, Anthony Liguori >> + break; >> + } >> + } >> + >> + return 0; >> > I guess you want to return one error if the index don't exist. > > >> +} >> > I still think that adding an "id" property as in markus proposal would > be neat. Otherwise I don't know how you are going to distinguish > between two keyboards with the same name. > > Later, Juan. > > >
On Wed, Mar 31, 2010 at 06:58:14PM +0200, Juan Quintela wrote: > Date: Wed, 31 Mar 2010 18:58:14 +0200 > From: Juan Quintela <quintela@redhat.com> > To: Markus Armbruster <armbru@redhat.com> > Cc: Shahar Havivi <shaharh@redhat.com>, qemu-devel@nongnu.org > Subject: [Qemu-devel] Re: [PATCH 2/2] Added monitor commands: 'keyboard_set' > and 'info keybaord' > > Markus Armbruster <armbru@redhat.com> wrote: > > Juan Quintela <quintela@redhat.com> writes: > >> I still think that adding an "id" property as in markus proposal would > >> be neat. Otherwise I don't know how you are going to distinguish > >> between two keyboards with the same name. > > > > If I understand the patch correctly (only time for a quick skim today), > > the keyboard receives a numeric ID when it is created, and keyboard_set > > identifies it by that ID. Yes, a user-defined ID would be nicer, and > > consistent with how similar things work. But the numeric ID isn't > > *wrong*, as far as I can see. > > my problem is that if you add two keyboards of the same type, they will > receive random index (different ones), now you do info keyboard, and you > see two keyboard with the same names and different numbers, how do you > know which of the two given you want to choose? > > And no, I don't have magic bullet to make multimonitor/keyboard/mouse > behave as expected out of given them right id's. > > Later, Juan. You right, this is a problem. As I see it id cannot be force to receive by the user right? Thanks, Shahar.
On 03/31/2010 02:37 PM, Shahar Havivi wrote: > On Wed, Mar 31, 2010 at 06:58:14PM +0200, Juan Quintela wrote: > >> Date: Wed, 31 Mar 2010 18:58:14 +0200 >> From: Juan Quintela<quintela@redhat.com> >> To: Markus Armbruster<armbru@redhat.com> >> Cc: Shahar Havivi<shaharh@redhat.com>, qemu-devel@nongnu.org >> Subject: [Qemu-devel] Re: [PATCH 2/2] Added monitor commands: 'keyboard_set' >> and 'info keybaord' >> >> Markus Armbruster<armbru@redhat.com> wrote: >> >>> Juan Quintela<quintela@redhat.com> writes: >>> >>>> I still think that adding an "id" property as in markus proposal would >>>> be neat. Otherwise I don't know how you are going to distinguish >>>> between two keyboards with the same name. >>>> >>> If I understand the patch correctly (only time for a quick skim today), >>> the keyboard receives a numeric ID when it is created, and keyboard_set >>> identifies it by that ID. Yes, a user-defined ID would be nicer, and >>> consistent with how similar things work. But the numeric ID isn't >>> *wrong*, as far as I can see. >>> >> my problem is that if you add two keyboards of the same type, they will >> receive random index (different ones), now you do info keyboard, and you >> see two keyboard with the same names and different numbers, how do you >> know which of the two given you want to choose? >> >> And no, I don't have magic bullet to make multimonitor/keyboard/mouse >> behave as expected out of given them right id's. >> >> Later, Juan. >> > You right, this is a problem. > As I see it id cannot be force to receive by the user right? > This is already the case with the mouse. Ultimately, I don't think it's a useful problem to attempt to solve. Why would there every be two keyboards of the same type (or two mice)? Regards, Anthony Liguori > Thanks, Shahar. > > >
Anthony Liguori <anthony@codemonkey.ws> wrote: > On 03/31/2010 02:37 PM, Shahar Havivi wrote: >> On Wed, Mar 31, 2010 at 06:58:14PM +0200, Juan Quintela wrote: >> >>> Date: Wed, 31 Mar 2010 18:58:14 +0200 >>> From: Juan Quintela<quintela@redhat.com> >>> To: Markus Armbruster<armbru@redhat.com> >>> Cc: Shahar Havivi<shaharh@redhat.com>, qemu-devel@nongnu.org >>> Subject: [Qemu-devel] Re: [PATCH 2/2] Added monitor commands: 'keyboard_set' >>> and 'info keybaord' >>> >>> Markus Armbruster<armbru@redhat.com> wrote: >>> >>>> Juan Quintela<quintela@redhat.com> writes: >>>> >>>>> I still think that adding an "id" property as in markus proposal would >>>>> be neat. Otherwise I don't know how you are going to distinguish >>>>> between two keyboards with the same name. >>>>> >>>> If I understand the patch correctly (only time for a quick skim today), >>>> the keyboard receives a numeric ID when it is created, and keyboard_set >>>> identifies it by that ID. Yes, a user-defined ID would be nicer, and >>>> consistent with how similar things work. But the numeric ID isn't >>>> *wrong*, as far as I can see. >>>> >>> my problem is that if you add two keyboards of the same type, they will >>> receive random index (different ones), now you do info keyboard, and you >>> see two keyboard with the same names and different numbers, how do you >>> know which of the two given you want to choose? >>> >>> And no, I don't have magic bullet to make multimonitor/keyboard/mouse >>> behave as expected out of given them right id's. >>> >>> Later, Juan. >>> >> You right, this is a problem. >> As I see it id cannot be force to receive by the user right? >> > > This is already the case with the mouse. > > Ultimately, I don't think it's a useful problem to attempt to solve. > Why would there every be two keyboards of the same type (or two mice)? I don't understand why somebody will want multiscreen+mouse+keyboard. Once told that, if having more than one of any of them, then it is better that keyboard_set /* * do_keyboard_set(): Set active keyboard * * Argument qdict contains * - "index": the keyboard index to set should take as a keyboard the string identifier and be done with that? Instead of all this bussines of numbering/renumbering during hot [un]plug? Later, Juan.
diff --git a/console.h b/console.h index 91b66ea..889f5d5 100644 --- a/console.h +++ b/console.h @@ -87,6 +87,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 e6dda25..3cc33f0 100644 --- a/input.c +++ b/input.c @@ -329,3 +329,104 @@ 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 current; + + kbd_list = qlist_new(); + + if (QTAILQ_EMPTY(&kbd_handlers)) { + goto out; + } + + current = QTAILQ_FIRST(&kbd_handlers)->index; + QTAILQ_FOREACH(cursor, &kbd_handlers, node) { + QObject *obj; + obj = qobject_from_jsonf("{ 'name': %s," + " 'index': %d," + " 'current': %i }", + cursor->qemu_put_kbd_name, + cursor->index, + current == cursor->index); + qlist_append_obj(kbd_list, obj); + } +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 index = qdict_get_int(qdict, "index"); + int found = 0; + + if (QTAILQ_EMPTY(&kbd_handlers)) { + qerror_report(QERR_DEVICE_NOT_FOUND, "keyboard"); + return -1; + } + + QTAILQ_FOREACH(cursor, &kbd_handlers, node) { + if (cursor->index == index) { + QTAILQ_REMOVE(&kbd_handlers, cursor, node); + QTAILQ_INSERT_HEAD(&kbd_handlers, cursor, node); + found = 1; + break; + } + } + + return 0; +} diff --git a/monitor.c b/monitor.c index 822dc27..f5474b1 100644 --- a/monitor.c +++ b/monitor.c @@ -2781,6 +2781,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",
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 | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ monitor.c | 8 ++++ qemu-monitor.hx | 17 +++++++++ 4 files changed, 130 insertions(+), 0 deletions(-)