| Message ID | 1338591268-23962-7-git-send-email-akong@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 06/01/2012 04:54 PM, Amos Kong wrote: > Convert 'sendkey' to use QAPI. do_sendkey() depends on some > variables/functions in monitor.c, so reserve qmp_sendkey() > to monitor.c > > key_defs[] in monitor.c is the mapping of key name to keycode, > Keys' order in the enmu and key_defs[] is same. > > Signed-off-by: Amos Kong <akong@redhat.com> > --- > +++ b/qapi-schema.json > @@ -1755,3 +1755,50 @@ > # Since: 0.14.0 > ## > { 'command': 'device_del', 'data': {'id': 'str'} } > + > +## > +# @KeyCodes: > +# > +# An enumeration of key name. > +# > +# This is used by the sendkey command. > +# > +# Since: 0.14.0 Really? Or is this enum since 1.2? > + > +## > +# @sendkey: > +# > +# Send keys to VM. > +# > +# @keys: key sequence > +# @hold-time: time to delay key up events, milliseconds > +# > +# Returns: Nothing on success > +# If key is unknown or redundant, QERR_INVALID_PARAMETER > +# If keys number is over the limitation, QERR_OVERFLOW > +# > +# Notes: Send keys to the emulator. Keys could be the name of the > +# key or the raw value in either decimal or hexadecimal format. Use > +# "-" to press several keys simultaneously. These notes don't really correspond to the QMP interface of passing in a JSON array of simultaneous keys to press. > +# > +# Since: 0.14.0 Again, shouldn't this be 1.2? > +SQMP > +sendkey > +---------- > + > +Send keys to VM. > + > +Arguments: > + > +keys array: > + - "key": key sequence (json-string) > + > +- hold-time: time to delay key up events, miliseconds (josn-int, optional) s/miliseconds/milliseconds/
Hello Eric, Thanks for your comments. ----- Original Message ----- > On 06/01/2012 04:54 PM, Amos Kong wrote: > > Convert 'sendkey' to use QAPI. do_sendkey() depends on some > > variables/functions in monitor.c, so reserve qmp_sendkey() > > to monitor.c > > > > key_defs[] in monitor.c is the mapping of key name to keycode, > > Keys' order in the enmu and key_defs[] is same. > > > > Signed-off-by: Amos Kong <akong@redhat.com> > > --- > > > +++ b/qapi-schema.json > > @@ -1755,3 +1755,50 @@ > > # Since: 0.14.0 > > ## > > { 'command': 'device_del', 'data': {'id': 'str'} } > > + > > +## > > +# @KeyCodes: > > +# > > +# An enumeration of key name. > > +# > > +# This is used by the sendkey command. > > +# > > +# Since: 0.14.0 > > Really? Or is this enum since 1.2? Yeah, it should be 1.2 > > > + > > +## > > +# @sendkey: > > +# > > +# Send keys to VM. > > +# > > +# @keys: key sequence > > +# @hold-time: time to delay key up events, milliseconds > > +# > > +# Returns: Nothing on success > > +# If key is unknown or redundant, QERR_INVALID_PARAMETER > > +# If keys number is over the limitation, QERR_OVERFLOW > > +# > > +# Notes: Send keys to the emulator. Keys could be the name of the > > +# key or the raw value in either decimal or hexadecimal format. Use > > +# "-" to press several keys simultaneously. > > These notes don't really correspond to the QMP interface of passing > in a JSON array of simultaneous keys to press. # Notes: Send keys to the emulator. Keys could be the name of the # key or the raw value in either decimal or hexadecimal format. Use # a JSON array to press several keys simultaneously. Ho, I found another bug in my code, key in decimal or hexadecimal format is not supported. I will fix it. > > +# > > +# Since: 0.14.0 > > Again, shouldn't this be 1.2? yeah, 1.2 > > > +SQMP > > +sendkey > > +---------- > > + > > +Send keys to VM. > > + > > +Arguments: > > + > > +keys array: > > + - "key": key sequence (json-string) > > + > > +- hold-time: time to delay key up events, miliseconds (josn-int, > > optional) > > s/miliseconds/milliseconds/ > > -- > Eric Blake eblake@redhat.com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org > >
On 06/05/2012 08:55 AM, Amos Kong wrote: >>> +# @sendkey: >>> +# >>> +# Send keys to VM. >>> +# >>> +# @keys: key sequence >>> +# @hold-time: time to delay key up events, milliseconds >>> +# >>> +# Returns: Nothing on success >>> +# If key is unknown or redundant, QERR_INVALID_PARAMETER >>> +# If keys number is over the limitation, QERR_OVERFLOW >>> +# >>> +# Notes: Send keys to the emulator. Keys could be the name of the >>> +# key or the raw value in either decimal or hexadecimal format. Use >>> +# "-" to press several keys simultaneously. >> >> These notes don't really correspond to the QMP interface of passing >> in a JSON array of simultaneous keys to press. > > > # Notes: Send keys to the emulator. Keys could be the name of the > # key or the raw value in either decimal or hexadecimal format. Use > # a JSON array to press several keys simultaneously. > > > > Ho, I found another bug in my code, key in decimal or hexadecimal > format is not supported. I will fix it. How do you plan to support decimal in QMP? I don't think it's possible, at least not without still keeping a JSON array of keys to press. On the other hand, I'm not sure if we need to worry about that. Let me explain: Let's suppose the QMP interface is symbolic only. In the HMP interface, _you_ can use the lookup list to convert decimal to key name, so that the HMP interface is a wrapper around the QMP interface, but everything is symbolic by the time we get that far. In the libvirt case, where libvirt talks directly to QMP, libvirt also has the same table for looking up keysyms vs. values (in fact, libvirt already uses that table to convert between different keysets, so for an example, the libvirt user can specify a command using xt_kbd or usb or ... mappings, which libvirt then converts into the mapping desired by qemu). Since libvirt already consults a table, libvirt can ensure that the table has the proper QMP symbolic names in that table. Finally, I know there was a patch proposed by Dan Berrange a while back to let both libvirt and qemu share a common database of keyset names and corresponding integer mappings. I don't remember if it was applied to qemu; if not, it should be revived and used at this time.
On 05/06/12 23:05, Eric Blake wrote: > On 06/05/2012 08:55 AM, Amos Kong wrote: > >>>> +# @sendkey: >>>> +# >>>> +# Send keys to VM. >>>> +# >>>> +# @keys: key sequence >>>> +# @hold-time: time to delay key up events, milliseconds >>>> +# >>>> +# Returns: Nothing on success >>>> +# If key is unknown or redundant, QERR_INVALID_PARAMETER >>>> +# If keys number is over the limitation, QERR_OVERFLOW >>>> +# >>>> +# Notes: Send keys to the emulator. Keys could be the name of the >>>> +# key or the raw value in either decimal or hexadecimal format. Use >>>> +# "-" to press several keys simultaneously. >>> >>> These notes don't really correspond to the QMP interface of passing >>> in a JSON array of simultaneous keys to press. >> >> >> # Notes: Send keys to the emulator. Keys could be the name of the >> # key or the raw value in either decimal or hexadecimal format. Use >> # a JSON array to press several keys simultaneously. >> >> >> >> Ho, I found another bug in my code, key in decimal or hexadecimal >> format is not supported. I will fix it. > > How do you plan to support decimal in QMP? In old do_sendkey(), only key-name and hexadecimal were supported, the description needs to be fixed. > I don't think it's possible, > at least not without still keeping a JSON array of keys to press. On > the other hand, I'm not sure if we need to worry about that. Let me > explain: > > Let's suppose the QMP interface is symbolic only. > > In the HMP interface, _you_ can use the lookup list to convert decimal > to key name, so that the HMP interface is a wrapper around the QMP > interface, but everything is symbolic by the time we get that far. > > In the libvirt case, where libvirt talks directly to QMP, It seems we can only support key-name for QMP, and support key-name/hexadecimal for HMP. Is it acceptable? > libvirt also > has the same table for looking up keysyms vs. values (in fact, libvirt > already uses that table to convert between different keysets, so for an > example, the libvirt user can specify a command using xt_kbd or usb or > ... mappings, which libvirt then converts into the mapping desired by > qemu). Since libvirt already consults a table, libvirt can ensure that > the table has the proper QMP symbolic names in that table. > Finally, I know there was a patch proposed by Dan Berrange a while back > to let both libvirt and qemu share a common database of keyset names and > corresponding integer mappings. I don't remember if it was applied to > qemu; if not, it should be revived and used at this time. >
On 06/06/2012 01:13 AM, Amos Kong wrote: >>> Ho, I found another bug in my code, key in decimal or hexadecimal >>> format is not supported. I will fix it. >> >> How do you plan to support decimal in QMP? > > In old do_sendkey(), only key-name and hexadecimal were supported, > the description needs to be fixed. > > > It seems we can only support key-name for QMP, and support > key-name/hexadecimal > for HMP. Is it acceptable? Yes, that was my argument for bare minimum support - anyone using QMP (like libvirt) will have to be smart enough to use key-name only, while anyone using HMP will be able to use hex because HMP will convert hex to key-name at the appropriate time before calling into QMP. Of course, being able to support hex from QMP would be a nice feature, but I'm not sure how to add it in.
On 06/06/2012 07:58 PM, Eric Blake wrote: > On 06/06/2012 01:13 AM, Amos Kong wrote: > >>>> Ho, I found another bug in my code, key in decimal or hexadecimal >>>> format is not supported. I will fix it. >>> >>> How do you plan to support decimal in QMP? >> >> In old do_sendkey(), only key-name and hexadecimal were supported, >> the description needs to be fixed. >> > >> >> It seems we can only support key-name for QMP, and support >> key-name/hexadecimal >> for HMP. Is it acceptable? > > Yes, that was my argument for bare minimum support - anyone using QMP > (like libvirt) will have to be smart enough to use key-name only, while > anyone using HMP will be able to use hex because HMP will convert hex to > key-name at the appropriate time before calling into QMP. > > Of course, being able to support hex from QMP would be a nice feature, > but I'm not sure how to add it in. Why is it a nice feature? I don't quite understand why anyone would want to use hex actually... You would have to convert from whatever symbolic code you're using to QEMU's hex value so why not just stay in symbolic representation. Regards, Anthony Liguori >
On 06/06/2012 10:51 PM, Anthony Liguori wrote: >> Of course, being able to support hex from QMP would be a nice feature, >> but I'm not sure how to add it in. > > Why is it a nice feature? Because libvirt already allows users to pass in integers instead of symbolic names. Providing integer support in QMP would let the path be passthrough, instead of libvirt doing a reverse lookup to convert integer to symbol just to call QMP for qemu to do a lookup from symbol back to integer. > > I don't quite understand why anyone would want to use hex actually... > You would have to convert from whatever symbolic code you're using to > QEMU's hex value so why not just stay in symbolic representation. Staying just symbolic is fine - remember, I already stated that libvirt already has a lookup table to convert between several different keycode sets already, and Dan Berrange has already proposed sharing that table with qemu. And settling on symbolic as the only interface, even if it is less efficient than integer passthrough, is at least easier to maintain.
diff --git a/hmp-commands.hx b/hmp-commands.hx index 18b8e31..afbfa61 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -505,7 +505,7 @@ ETEXI .args_type = "keys:s,hold-time:i?", .params = "keys [hold_ms]", .help = "send keys to the VM (e.g. 'sendkey ctrl-alt-f1', default hold time=100 ms)", - .mhandler.cmd = do_sendkey, + .mhandler.cmd = hmp_sendkey, }, STEXI diff --git a/hmp.c b/hmp.c index bb0952e..06a6686 100644 --- a/hmp.c +++ b/hmp.c @@ -16,6 +16,7 @@ #include "hmp.h" #include "qemu-timer.h" #include "qmp-commands.h" +#include "qapi-types.h" static void hmp_handle_error(Monitor *mon, Error **errp) { @@ -947,3 +948,58 @@ void hmp_device_del(Monitor *mon, const QDict *qdict) qmp_device_del(id, &err); hmp_handle_error(mon, &err); } + +static int get_index(const char *key) +{ + int i; + + for (i = 0; KeyCodes_lookup[i] != NULL; i++) { + if (!strcmp(key, KeyCodes_lookup[i])) + return i; + } + return -1; +} + +void hmp_sendkey(Monitor *mon, const QDict *qdict) +{ + const char *keys = qdict_get_str(qdict, "keys"); + KeyCodesList *keylist, *head = NULL, *tmp = NULL; + int has_hold_time = qdict_haskey(qdict, "hold-time"); + int hold_time = qdict_get_try_int(qdict, "hold-time", -1); + Error *err = NULL; + char keyname_buf[16]; + + char *separator; + int keyname_len; + + while (1) { + separator = strchr(keys, '-'); + keyname_len = separator ? separator - keys : strlen(keys); + pstrcpy(keyname_buf, sizeof(keyname_buf), keys); + + /* Be compatible with old interface, convert user inputted "<" */ + if (!strcmp(keyname_buf, "<")) { + pstrcpy(keyname_buf, sizeof(keyname_buf), "less"); + keyname_len = 4; + } + keyname_buf[keyname_len] = 0; + + keylist = g_malloc0(sizeof(*keylist)); + keylist->value = get_index(keyname_buf); + keylist->next = NULL; + + if (tmp) + tmp->next = keylist; + tmp = keylist; + if (!head) + head = keylist; + + if (!separator) + break; + keys = separator + 1; + } + + qmp_sendkey(head, has_hold_time, hold_time, &err); + qapi_free_KeyCodesList(keylist); + hmp_handle_error(mon, &err); +} diff --git a/hmp.h b/hmp.h index 443b812..40de72c 100644 --- a/hmp.h +++ b/hmp.h @@ -61,5 +61,6 @@ void hmp_block_job_set_speed(Monitor *mon, const QDict *qdict); void hmp_block_job_cancel(Monitor *mon, const QDict *qdict); void hmp_migrate(Monitor *mon, const QDict *qdict); void hmp_device_del(Monitor *mon, const QDict *qdict); +void hmp_sendkey(Monitor *mon, const QDict *qdict); #endif diff --git a/monitor.c b/monitor.c index 536d9dd..2858ac0 100644 --- a/monitor.c +++ b/monitor.c @@ -1341,24 +1341,6 @@ static const KeyDef key_defs[] = { { 0, NULL }, }; -static int get_keycode(const char *key) -{ - const KeyDef *p; - char *endp; - int ret; - - for(p = key_defs; p->name != NULL; p++) { - if (!strcmp(key, p->name)) - return p->keycode; - } - if (strstart(key, "0x", NULL)) { - ret = strtoul(key, &endp, 0); - if (*endp == '\0' && ret >= 0x01 && ret <= 0xff) - return ret; - } - return -1; -} - #define MAX_KEYCODES 16 static uint8_t keycodes[MAX_KEYCODES]; static int nb_pending_keycodes; @@ -1377,14 +1359,12 @@ static void release_keys(void *opaque) } } -static void do_sendkey(Monitor *mon, const QDict *qdict) +void qmp_sendkey(KeyCodesList *keys, bool has_hold_time, int64_t hold_time, + Error **errp) { char keyname_buf[16]; - char *separator; - int keyname_len, keycode, i; - const char *keys = qdict_get_str(qdict, "keys"); - int has_hold_time = qdict_haskey(qdict, "hold-time"); - int hold_time = qdict_get_try_int(qdict, "hold-time", -1); + int keycode, i; + KeyCodesList *entry = keys; if (nb_pending_keycodes > 0) { qemu_del_timer(key_timer); @@ -1392,39 +1372,23 @@ static void do_sendkey(Monitor *mon, const QDict *qdict) } if (!has_hold_time) hold_time = 100; - i = 0; - while (1) { - separator = strchr(keys, '-'); - keyname_len = separator ? separator - keys : strlen(keys); - if (keyname_len > 0) { - pstrcpy(keyname_buf, sizeof(keyname_buf), keys); - if (keyname_len > sizeof(keyname_buf) - 1) { - monitor_printf(mon, "invalid key: '%s...'\n", keyname_buf); - return; - } - if (i == MAX_KEYCODES) { - monitor_printf(mon, "too many keys\n"); - return; - } - /* Be compatible with old interface, convert user inputted "<" */ - if (!strcmp(keyname_buf, "<")) { - pstrcpy(keyname_buf, sizeof(keyname_buf), "less"); - keyname_len = 4; - } + i = 0; + while (entry != NULL) { + if (entry->value > sizeof(key_defs) / sizeof(*(key_defs))) { + error_set(errp, QERR_INVALID_PARAMETER, keyname_buf); + return; + } - keyname_buf[keyname_len] = 0; - keycode = get_keycode(keyname_buf); - if (keycode < 0) { - monitor_printf(mon, "unknown key: '%s'\n", keyname_buf); - return; - } - keycodes[i++] = keycode; + if (i == MAX_KEYCODES) { + error_set(errp, QERR_OVERFLOW); + return; } - if (!separator) - break; - keys = separator + 1; + + keycodes[i++] = key_defs[entry->value].keycode; + entry = entry->next; } + nb_pending_keycodes = i; /* key down events */ for (i = 0; i < nb_pending_keycodes; i++) { diff --git a/qapi-schema.json b/qapi-schema.json index 2ca7195..8fa0cb7 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1755,3 +1755,50 @@ # Since: 0.14.0 ## { 'command': 'device_del', 'data': {'id': 'str'} } + +## +# @KeyCodes: +# +# An enumeration of key name. +# +# This is used by the sendkey command. +# +# Since: 0.14.0 +## +{ 'enum': 'KeyCodes', + 'data': [ 'shift', 'shift_r', 'alt', 'alt_r', 'altgr', 'altgr_r', 'ctrl', + 'ctrl_r', 'menu', 'esc', '1', '2', '3', '4', '5', '6', '7', '8', + '9', '0', 'minus', 'equal', 'backspace', 'tab', 'q', 'w', 'e', + 'r', 't', 'y', 'u', 'i', 'o', 'p', 'bracket_left', 'bracket_right', + 'ret', 'a', 's', 'd', 'f', 'g', 'h', 'j', 'k', 'l', 'semicolon', + 'apostrophe', 'grave_accent', 'backslash', 'z', 'x', 'c', 'v', 'b', + 'n', 'm', 'comma', 'dot', 'slash', 'asterisk', 'spc', 'caps_lock', + 'f1', 'f2', 'f3', 'f4', 'f5', 'f6', 'f7', 'f8', 'f9', 'f10', + 'num_lock', 'scroll_lock', 'kp_divide', 'kp_multiply', + 'kp_subtract', 'kp_add', 'kp_enter', 'kp_decimal', 'sysrq', 'kp_0', + 'kp_1', 'kp_2', 'kp_3', 'kp_4', 'kp_5', 'kp_6', 'kp_7', 'kp_8', + 'kp_9', 'less', 'f11', 'f12', 'print', 'home', 'pgup', 'pgdn', 'end', + 'left', 'up', 'down', 'right', 'insert', 'delete', 'stop', 'again', + 'props', 'undo', 'front', 'copy', 'open', 'paste', 'find', 'cut', + 'lf', 'help', 'meta_l', 'meta_r', 'compose' ] } + +## +# @sendkey: +# +# Send keys to VM. +# +# @keys: key sequence +# @hold-time: time to delay key up events, milliseconds +# +# Returns: Nothing on success +# If key is unknown or redundant, QERR_INVALID_PARAMETER +# If keys number is over the limitation, QERR_OVERFLOW +# +# Notes: Send keys to the emulator. Keys could be the name of the +# key or the raw value in either decimal or hexadecimal format. Use +# "-" to press several keys simultaneously. +# +# Since: 0.14.0 +## +{ 'command': 'sendkey', + 'data': { 'keys': ['KeyCodes'], '*hold-time': 'int' } } diff --git a/qmp-commands.hx b/qmp-commands.hx index db980fa..3692805 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -335,6 +335,33 @@ Example: EQMP { + .name = "sendkey", + .args_type = "keys:O,hold-time:i?", + .mhandler.cmd_new = qmp_marshal_input_sendkey, + }, + +SQMP +sendkey +---------- + +Send keys to VM. + +Arguments: + +keys array: + - "key": key sequence (json-string) + +- hold-time: time to delay key up events, miliseconds (josn-int, optional) + +Example: + +-> { "execute": "sendkey", + "arguments": { 'keys': [ 'ctrl', 'alt', 'delete' ] } } +<- { "return": {} } + +EQMP + + { .name = "cpu", .args_type = "index:i", .mhandler.cmd_new = qmp_marshal_input_cpu,
Convert 'sendkey' to use QAPI. do_sendkey() depends on some variables/functions in monitor.c, so reserve qmp_sendkey() to monitor.c key_defs[] in monitor.c is the mapping of key name to keycode, Keys' order in the enmu and key_defs[] is same. Signed-off-by: Amos Kong <akong@redhat.com> --- hmp-commands.hx | 2 +- hmp.c | 56 +++++++++++++++++++++++++++++++++++++++++++ hmp.h | 1 + monitor.c | 70 +++++++++++++---------------------------------------- qapi-schema.json | 47 ++++++++++++++++++++++++++++++++++++ qmp-commands.hx | 27 ++++++++++++++++++++ 6 files changed, 149 insertions(+), 54 deletions(-)