* [Qemu-devel] [PATCH v3 0/3] qmp: send-key: accept key codes in hex @ 2012-09-26 20:25 Luiz Capitulino 2012-09-26 20:25 ` [Qemu-devel] [PATCH 1/3] input: qmp_send_key(): simplify Luiz Capitulino ` (5 more replies) 0 siblings, 6 replies; 13+ messages in thread From: Luiz Capitulino @ 2012-09-26 20:25 UTC (permalink / raw) To: qemu-devel; +Cc: akong, eblake, avi, armbru o v3 - doc & log fixups - rename KeyCode.hex to KeyCode.number This actually fixes a regression introduced by the qapi conversion, please refer to patch 2/3 for details. It's also important to note that this series changes the QMP interface for the send-key command, but this shouldn't be a problem as we're still in development phase. Luiz Capitulino (3): input: qmp_send_key(): simplify qmp: qmp_send_key(): accept key codes in hex input: index_from_key(): drop unused code hmp.c | 43 +++++++++++++++++++++----------- input.c | 75 ++++++++++++++++++++++++++++---------------------------- qapi-schema.json | 20 ++++++++++++--- 3 files changed, 84 insertions(+), 54 deletions(-) -- 1.7.12.315.g682ce8b ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 1/3] input: qmp_send_key(): simplify 2012-09-26 20:25 [Qemu-devel] [PATCH v3 0/3] qmp: send-key: accept key codes in hex Luiz Capitulino @ 2012-09-26 20:25 ` Luiz Capitulino 2012-09-27 9:50 ` Markus Armbruster 2012-09-26 20:25 ` [Qemu-devel] [PATCH 2/3] qmp: qmp_send_key(): accept key codes in hex Luiz Capitulino ` (4 subsequent siblings) 5 siblings, 1 reply; 13+ messages in thread From: Luiz Capitulino @ 2012-09-26 20:25 UTC (permalink / raw) To: qemu-devel; +Cc: akong, eblake, avi, armbru The current code duplicates the QKeyCodeList keys in order to store the key values for release_keys() late run. This is a bit complicated though, as we have to care about correct ordering and then release_keys() will have to index key_defs[] over again. Switch to an array of integers, which is dynamically allocated and stores the already converted key value. This simplifies the current code and the next commit. Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> --- input.c | 36 ++++++++++++++---------------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/input.c b/input.c index c4b0619..32c6057 100644 --- a/input.c +++ b/input.c @@ -224,30 +224,31 @@ int index_from_keycode(int code) return i; } -static QKeyCodeList *keycodes; +static int *keycodes; +static int keycodes_size; static QEMUTimer *key_timer; static void release_keys(void *opaque) { - int keycode; - QKeyCodeList *p; + int i; - for (p = keycodes; p != NULL; p = p->next) { - keycode = key_defs[p->value]; - if (keycode & 0x80) { + for (i = 0; i < keycodes_size; i++) { + if (keycodes[i] & 0x80) { kbd_put_keycode(0xe0); } - kbd_put_keycode(keycode | 0x80); + kbd_put_keycode(keycodes[i]| 0x80); } - qapi_free_QKeyCodeList(keycodes); + + g_free(keycodes); keycodes = NULL; + keycodes_size = 0; } void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time, Error **errp) { int keycode; - QKeyCodeList *p, *keylist, *head = NULL, *tmp = NULL; + QKeyCodeList *p; if (!key_timer) { key_timer = qemu_new_timer_ns(vm_clock, release_keys, NULL); @@ -257,31 +258,22 @@ void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time, qemu_del_timer(key_timer); release_keys(NULL); } + if (!has_hold_time) { hold_time = 100; } for (p = keys; p != NULL; p = p->next) { - keylist = g_malloc0(sizeof(*keylist)); - keylist->value = p->value; - keylist->next = NULL; - - if (!head) { - head = keylist; - } - if (tmp) { - tmp->next = keylist; - } - tmp = keylist; - /* key down events */ keycode = key_defs[p->value]; if (keycode & 0x80) { kbd_put_keycode(0xe0); } kbd_put_keycode(keycode & 0x7f); + + keycodes = g_realloc(keycodes, sizeof(int) * (keycodes_size + 1)); + keycodes[keycodes_size++] = keycode; } - keycodes = head; /* delayed key up events */ qemu_mod_timer(key_timer, qemu_get_clock_ns(vm_clock) + -- 1.7.12.315.g682ce8b ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] input: qmp_send_key(): simplify 2012-09-26 20:25 ` [Qemu-devel] [PATCH 1/3] input: qmp_send_key(): simplify Luiz Capitulino @ 2012-09-27 9:50 ` Markus Armbruster 0 siblings, 0 replies; 13+ messages in thread From: Markus Armbruster @ 2012-09-27 9:50 UTC (permalink / raw) To: Luiz Capitulino; +Cc: akong, eblake, qemu-devel, avi Luiz Capitulino <lcapitulino@redhat.com> writes: > The current code duplicates the QKeyCodeList keys in order to store > the key values for release_keys() late run. This is a bit complicated > though, as we have to care about correct ordering and then release_keys() > will have to index key_defs[] over again. > > Switch to an array of integers, which is dynamically allocated and stores > the already converted key value. > > This simplifies the current code and the next commit. > > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > --- > input.c | 36 ++++++++++++++---------------------- > 1 file changed, 14 insertions(+), 22 deletions(-) > > diff --git a/input.c b/input.c > index c4b0619..32c6057 100644 > --- a/input.c > +++ b/input.c > @@ -224,30 +224,31 @@ int index_from_keycode(int code) > return i; > } > > -static QKeyCodeList *keycodes; > +static int *keycodes; > +static int keycodes_size; > static QEMUTimer *key_timer; > > static void release_keys(void *opaque) > { > - int keycode; > - QKeyCodeList *p; > + int i; > > - for (p = keycodes; p != NULL; p = p->next) { > - keycode = key_defs[p->value]; > - if (keycode & 0x80) { > + for (i = 0; i < keycodes_size; i++) { > + if (keycodes[i] & 0x80) { > kbd_put_keycode(0xe0); > } > - kbd_put_keycode(keycode | 0x80); > + kbd_put_keycode(keycodes[i]| 0x80); > } > - qapi_free_QKeyCodeList(keycodes); > + > + g_free(keycodes); > keycodes = NULL; > + keycodes_size = 0; > } > > void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time, > Error **errp) > { > int keycode; > - QKeyCodeList *p, *keylist, *head = NULL, *tmp = NULL; > + QKeyCodeList *p; > > if (!key_timer) { > key_timer = qemu_new_timer_ns(vm_clock, release_keys, NULL); > @@ -257,31 +258,22 @@ void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time, > qemu_del_timer(key_timer); > release_keys(NULL); > } > + > if (!has_hold_time) { > hold_time = 100; > } > > for (p = keys; p != NULL; p = p->next) { > - keylist = g_malloc0(sizeof(*keylist)); > - keylist->value = p->value; > - keylist->next = NULL; > - > - if (!head) { > - head = keylist; > - } > - if (tmp) { > - tmp->next = keylist; > - } > - tmp = keylist; > - > /* key down events */ > keycode = key_defs[p->value]; > if (keycode & 0x80) { > kbd_put_keycode(0xe0); > } > kbd_put_keycode(keycode & 0x7f); > + > + keycodes = g_realloc(keycodes, sizeof(int) * (keycodes_size + 1)); > + keycodes[keycodes_size++] = keycode; One realloc per key is a bit wasteful (the common efficient way to grow a flexible array is to double it), but it takes a mighty fast typist to make that matter. > } > - keycodes = head; > > /* delayed key up events */ > qemu_mod_timer(key_timer, qemu_get_clock_ns(vm_clock) + ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 2/3] qmp: qmp_send_key(): accept key codes in hex 2012-09-26 20:25 [Qemu-devel] [PATCH v3 0/3] qmp: send-key: accept key codes in hex Luiz Capitulino 2012-09-26 20:25 ` [Qemu-devel] [PATCH 1/3] input: qmp_send_key(): simplify Luiz Capitulino @ 2012-09-26 20:25 ` Luiz Capitulino 2012-09-27 11:42 ` Markus Armbruster 2012-09-26 20:25 ` [Qemu-devel] [PATCH 3/3] input: index_from_key(): drop unused code Luiz Capitulino ` (3 subsequent siblings) 5 siblings, 1 reply; 13+ messages in thread From: Luiz Capitulino @ 2012-09-26 20:25 UTC (permalink / raw) To: qemu-devel; +Cc: akong, eblake, avi, armbru Before the qapi conversion, the sendkey command could be used to send key codes in hex directly to the guest. In HMP, this would be like: (qemu) sendkey 0xdc However, the qapi conversion broke this, as it only supports sending QKeyCode values to the guest. That's a regression. This commit fixes the problem by adding hex value support down the QMP interface, qmp_send_key(). In more detail, this commit: 1. Adds the KeyValue union. This can represent an hex value or a QKeyCode value 2. *Changes* the QMP send-key command to take an KeyValue argument instead of a QKeyCode one 3. Adapt hmp_send_key() to the QMP interface changes Item 2 is an incompatible change, but as we're in development phase (and this command has been merged a few weeks ago) this shouldn't be a problem. Finally, it's not possible to split this commit without breaking the build. Reported-by: Avi Kivity <avi@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> --- hmp.c | 43 +++++++++++++++++++++++++++++-------------- input.c | 33 +++++++++++++++++++++++++++------ qapi-schema.json | 20 +++++++++++++++++--- 3 files changed, 73 insertions(+), 23 deletions(-) diff --git a/hmp.c b/hmp.c index ba6fbd3..8738dc5 100644 --- a/hmp.c +++ b/hmp.c @@ -1109,13 +1109,13 @@ void hmp_closefd(Monitor *mon, const QDict *qdict) void hmp_send_key(Monitor *mon, const QDict *qdict) { const char *keys = qdict_get_str(qdict, "keys"); - QKeyCodeList *keylist, *head = NULL, *tmp = NULL; + KeyValueList *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, idx; + int keyname_len; while (1) { separator = strchr(keys, '-'); @@ -1129,15 +1129,8 @@ void hmp_send_key(Monitor *mon, const QDict *qdict) } keyname_buf[keyname_len] = 0; - idx = index_from_key(keyname_buf); - if (idx == Q_KEY_CODE_MAX) { - monitor_printf(mon, "invalid parameter: %s\n", keyname_buf); - break; - } - keylist = g_malloc0(sizeof(*keylist)); - keylist->value = idx; - keylist->next = NULL; + keylist->value = g_malloc0(sizeof(*keylist->value)); if (!head) { head = keylist; @@ -1147,17 +1140,39 @@ void hmp_send_key(Monitor *mon, const QDict *qdict) } tmp = keylist; + if (strstart(keyname_buf, "0x", NULL)) { + char *endp; + int value = strtoul(keyname_buf, &endp, 0); + if (*endp != '\0') { + goto err_out; + } + keylist->value->kind = KEY_VALUE_KIND_NUMBER; + keylist->value->number = value; + } else { + int idx = index_from_key(keyname_buf); + if (idx == Q_KEY_CODE_MAX) { + goto err_out; + } + keylist->value->kind = KEY_VALUE_KIND_QCODE; + keylist->value->qcode = idx; + } + if (!separator) { break; } keys = separator + 1; } - if (idx != Q_KEY_CODE_MAX) { - qmp_send_key(head, has_hold_time, hold_time, &err); - } + qmp_send_key(head, has_hold_time, hold_time, &err); hmp_handle_error(mon, &err); - qapi_free_QKeyCodeList(head); + +out: + qapi_free_KeyValueList(head); + return; + +err_out: + monitor_printf(mon, "invalid parameter: %s\n", keyname_buf); + goto out; } void hmp_screen_dump(Monitor *mon, const QDict *qdict) diff --git a/input.c b/input.c index 32c6057..76ade64 100644 --- a/input.c +++ b/input.c @@ -228,6 +228,23 @@ static int *keycodes; static int keycodes_size; static QEMUTimer *key_timer; +static int keycode_from_keyvalue(const KeyValue *value) +{ + if (value->kind == KEY_VALUE_KIND_QCODE) { + return key_defs[value->qcode]; + } else { + assert(value->kind == KEY_VALUE_KIND_NUMBER); + return value->number; + } +} + +static void free_keycodes(void) +{ + g_free(keycodes); + keycodes = NULL; + keycodes_size = 0; +} + static void release_keys(void *opaque) { int i; @@ -239,16 +256,14 @@ static void release_keys(void *opaque) kbd_put_keycode(keycodes[i]| 0x80); } - g_free(keycodes); - keycodes = NULL; - keycodes_size = 0; + free_keycodes(); } -void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time, +void qmp_send_key(KeyValueList *keys, bool has_hold_time, int64_t hold_time, Error **errp) { int keycode; - QKeyCodeList *p; + KeyValueList *p; if (!key_timer) { key_timer = qemu_new_timer_ns(vm_clock, release_keys, NULL); @@ -265,7 +280,13 @@ void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time, for (p = keys; p != NULL; p = p->next) { /* key down events */ - keycode = key_defs[p->value]; + keycode = keycode_from_keyvalue(p->value); + if (keycode < 0x01 || keycode > 0xff) { + error_setg(errp, "invalid hex keycode 0x%x\n", keycode); + free_keycodes(); + return; + } + if (keycode & 0x80) { kbd_put_keycode(0xe0); } diff --git a/qapi-schema.json b/qapi-schema.json index 14e4419..02135cc 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -2588,12 +2588,26 @@ 'lf', 'help', 'meta_l', 'meta_r', 'compose' ] } ## +# @KeyValue +# +# Represents a keyboard key. +# +# Since: 1.3.0 +## +{ 'union': 'KeyValue', + 'data': { + 'number': 'int', + 'qcode': 'QKeyCode' } } + +## # @send-key: # # Send keys to guest. # -# @keys: key sequence. 'keys' is the name of the key. Use a JSON array to -# press several keys simultaneously. +# @keys: An array of @KeyValue elements. All @KeyValues in this array are +# simultaneously sent to the guest. A @KeyValue.number value is sent +# directly to the guest, while @KeyValue.qcode must be a valid +# @QKeyCode value # # @hold-time: #optional time to delay key up events, milliseconds. Defaults # to 100 @@ -2605,7 +2619,7 @@ # ## { 'command': 'send-key', - 'data': { 'keys': ['QKeyCode'], '*hold-time': 'int' } } + 'data': { 'keys': ['KeyValue'], '*hold-time': 'int' } } ## # @screendump: -- 1.7.12.315.g682ce8b ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] qmp: qmp_send_key(): accept key codes in hex 2012-09-26 20:25 ` [Qemu-devel] [PATCH 2/3] qmp: qmp_send_key(): accept key codes in hex Luiz Capitulino @ 2012-09-27 11:42 ` Markus Armbruster 2012-09-27 12:57 ` Luiz Capitulino 0 siblings, 1 reply; 13+ messages in thread From: Markus Armbruster @ 2012-09-27 11:42 UTC (permalink / raw) To: Luiz Capitulino; +Cc: akong, eblake, qemu-devel, avi Luiz Capitulino <lcapitulino@redhat.com> writes: > Before the qapi conversion, the sendkey command could be used to > send key codes in hex directly to the guest. In HMP, this would > be like: > > (qemu) sendkey 0xdc > > However, the qapi conversion broke this, as it only supports sending > QKeyCode values to the guest. That's a regression. Let me elaborate, to make sure I got it. Before the QAPI conversion, sendkey used get_keycode() to convert key substrings to integer key codes. Source code: 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; } If @key is a recognized key name, return its (positive) code. Else, if @key is a hexadecimal number between 1 and 0xff, return the number. Else, return -1. Since the QAPI conversion, it uses index_from_key(): int index_from_key(const char *key) { int i, keycode; char *endp; for (i = 0; QKeyCode_lookup[i] != NULL; i++) { if (!strcmp(key, QKeyCode_lookup[i])) { break; } } if (strstart(key, "0x", NULL)) { keycode = strtoul(key, &endp, 0); if (*endp == '\0' && keycode >= 0x01 && keycode <= 0xff) { for (i = 0; i < Q_KEY_CODE_MAX; i++) { if (keycode == key_defs[i]) { break; } } } } /* Return Q_KEY_CODE_MAX if the key is invalid */ return i; } If @key is a recognized key name, return its (positive) code. Else, if @key is a hexadecimal number between 1 and 0xff, and a recognized key with that code exists, return the number. Else, return -1. The regression is *not* that the hexadecimal format isn't recognized anymore, it's that numbers that don't have a key definition are now rejected. > This commit fixes the problem by adding hex value support down > the QMP interface, qmp_send_key(). > > In more detail, this commit: > > 1. Adds the KeyValue union. This can represent an hex value or > a QKeyCode value > > 2. *Changes* the QMP send-key command to take an KeyValue argument > instead of a QKeyCode one > > 3. Adapt hmp_send_key() to the QMP interface changes > > Item 2 is an incompatible change, but as we're in development phase > (and this command has been merged a few weeks ago) this shouldn't be > a problem. > > Finally, it's not possible to split this commit without breaking the > build. > > Reported-by: Avi Kivity <avi@redhat.com> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > --- > hmp.c | 43 +++++++++++++++++++++++++++++-------------- > input.c | 33 +++++++++++++++++++++++++++------ > qapi-schema.json | 20 +++++++++++++++++--- > 3 files changed, 73 insertions(+), 23 deletions(-) > > diff --git a/hmp.c b/hmp.c > index ba6fbd3..8738dc5 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -1109,13 +1109,13 @@ void hmp_closefd(Monitor *mon, const QDict *qdict) > void hmp_send_key(Monitor *mon, const QDict *qdict) > { > const char *keys = qdict_get_str(qdict, "keys"); > - QKeyCodeList *keylist, *head = NULL, *tmp = NULL; > + KeyValueList *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, idx; > + int keyname_len; > > while (1) { > separator = strchr(keys, '-'); > @@ -1129,15 +1129,8 @@ void hmp_send_key(Monitor *mon, const QDict *qdict) > } > keyname_buf[keyname_len] = 0; > > - idx = index_from_key(keyname_buf); > - if (idx == Q_KEY_CODE_MAX) { > - monitor_printf(mon, "invalid parameter: %s\n", keyname_buf); > - break; > - } > - > keylist = g_malloc0(sizeof(*keylist)); > - keylist->value = idx; > - keylist->next = NULL; > + keylist->value = g_malloc0(sizeof(*keylist->value)); > > if (!head) { > head = keylist; > @@ -1147,17 +1140,39 @@ void hmp_send_key(Monitor *mon, const QDict *qdict) > } > tmp = keylist; > > + if (strstart(keyname_buf, "0x", NULL)) { > + char *endp; > + int value = strtoul(keyname_buf, &endp, 0); > + if (*endp != '\0') { > + goto err_out; > + } > + keylist->value->kind = KEY_VALUE_KIND_NUMBER; > + keylist->value->number = value; > + } else { > + int idx = index_from_key(keyname_buf); > + if (idx == Q_KEY_CODE_MAX) { > + goto err_out; > + } > + keylist->value->kind = KEY_VALUE_KIND_QCODE; > + keylist->value->qcode = idx; > + } > + TL;DR: this differs unnecessarily from the pre-QAPI code, which makes the fix less obvious than it could be. But I think it should work, and unbreaking autotest quickly is more important than making this patch perfect, so I'm *not* asking for a respin because of that. Strings starting with "0x" are interpreted differently than before the QAPI conversion. Before: If @key is a recognized key name, use its (positive) code. Else, if @key is a hexadecimal number between 1 and 0xff, use the number. Else, error. After: If @key starts with "0x" If @key is a number, use it (range check is done later) Else, error Else If it's a recognized key name, use its (positive) code Else, error. Makes a difference only if key names starting with "0x" exist. None exist now, and such names aren't likely to appear in the future. Aside: codes starting with a digit do exist, and that's why we can do only hexadecimal numeric keys. The use of strtoul() here is sloppy, but it's no worse as before, in index_from_key(): * Numbers overflowing int are silently truncated, as before. * Numbers outside 0x01..0xff (possibly after truncation) are still flagged as error, in qmp_send_key() below. > if (!separator) { > break; > } > keys = separator + 1; > } > > - if (idx != Q_KEY_CODE_MAX) { > - qmp_send_key(head, has_hold_time, hold_time, &err); > - } > + qmp_send_key(head, has_hold_time, hold_time, &err); > hmp_handle_error(mon, &err); > - qapi_free_QKeyCodeList(head); > + > +out: > + qapi_free_KeyValueList(head); > + return; > + > +err_out: > + monitor_printf(mon, "invalid parameter: %s\n", keyname_buf); > + goto out; Jumping forward and backward like that is a bit ugly, but let's unbreak autotest. > } > > void hmp_screen_dump(Monitor *mon, const QDict *qdict) > diff --git a/input.c b/input.c > index 32c6057..76ade64 100644 > --- a/input.c > +++ b/input.c > @@ -228,6 +228,23 @@ static int *keycodes; > static int keycodes_size; > static QEMUTimer *key_timer; > > +static int keycode_from_keyvalue(const KeyValue *value) > +{ > + if (value->kind == KEY_VALUE_KIND_QCODE) { > + return key_defs[value->qcode]; > + } else { > + assert(value->kind == KEY_VALUE_KIND_NUMBER); > + return value->number; > + } > +} > + > +static void free_keycodes(void) > +{ > + g_free(keycodes); > + keycodes = NULL; > + keycodes_size = 0; > +} > + > static void release_keys(void *opaque) > { > int i; > @@ -239,16 +256,14 @@ static void release_keys(void *opaque) > kbd_put_keycode(keycodes[i]| 0x80); > } > > - g_free(keycodes); > - keycodes = NULL; > - keycodes_size = 0; > + free_keycodes(); > } > > -void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time, > +void qmp_send_key(KeyValueList *keys, bool has_hold_time, int64_t hold_time, > Error **errp) > { > int keycode; > - QKeyCodeList *p; > + KeyValueList *p; > > if (!key_timer) { > key_timer = qemu_new_timer_ns(vm_clock, release_keys, NULL); > @@ -265,7 +280,13 @@ void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time, > > for (p = keys; p != NULL; p = p->next) { > /* key down events */ > - keycode = key_defs[p->value]; > + keycode = keycode_from_keyvalue(p->value); > + if (keycode < 0x01 || keycode > 0xff) { > + error_setg(errp, "invalid hex keycode 0x%x\n", keycode); > + free_keycodes(); > + return; > + } > + > if (keycode & 0x80) { > kbd_put_keycode(0xe0); > } > diff --git a/qapi-schema.json b/qapi-schema.json > index 14e4419..02135cc 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -2588,12 +2588,26 @@ > 'lf', 'help', 'meta_l', 'meta_r', 'compose' ] } > > ## > +# @KeyValue > +# > +# Represents a keyboard key. > +# Trailing whitespace, could be fixed up on merge if we care. > +# Since: 1.3.0 > +## > +{ 'union': 'KeyValue', > + 'data': { > + 'number': 'int', > + 'qcode': 'QKeyCode' } } > + > +## > # @send-key: > # > # Send keys to guest. > # > -# @keys: key sequence. 'keys' is the name of the key. Use a JSON array to > -# press several keys simultaneously. > +# @keys: An array of @KeyValue elements. All @KeyValues in this array are > +# simultaneously sent to the guest. A @KeyValue.number value is sent > +# directly to the guest, while @KeyValue.qcode must be a valid > +# @QKeyCode value > # > # @hold-time: #optional time to delay key up events, milliseconds. Defaults > # to 100 > @@ -2605,7 +2619,7 @@ > # > ## > { 'command': 'send-key', > - 'data': { 'keys': ['QKeyCode'], '*hold-time': 'int' } } > + 'data': { 'keys': ['KeyValue'], '*hold-time': 'int' } } > > ## > # @screendump: ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] qmp: qmp_send_key(): accept key codes in hex 2012-09-27 11:42 ` Markus Armbruster @ 2012-09-27 12:57 ` Luiz Capitulino 0 siblings, 0 replies; 13+ messages in thread From: Luiz Capitulino @ 2012-09-27 12:57 UTC (permalink / raw) To: Markus Armbruster; +Cc: akong, eblake, qemu-devel, avi On Thu, 27 Sep 2012 13:42:57 +0200 Markus Armbruster <armbru@redhat.com> wrote: > Luiz Capitulino <lcapitulino@redhat.com> writes: > > > Before the qapi conversion, the sendkey command could be used to > > send key codes in hex directly to the guest. In HMP, this would > > be like: > > > > (qemu) sendkey 0xdc > > > > However, the qapi conversion broke this, as it only supports sending > > QKeyCode values to the guest. That's a regression. > > Let me elaborate, to make sure I got it. > > Before the QAPI conversion, sendkey used get_keycode() to convert key > substrings to integer key codes. Source code: > > 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; > } > > If @key is a recognized key name, return its (positive) code. > Else, if @key is a hexadecimal number between 1 and 0xff, return the > number. > Else, return -1. > > Since the QAPI conversion, it uses index_from_key(): > > int index_from_key(const char *key) > { > int i, keycode; > char *endp; > > for (i = 0; QKeyCode_lookup[i] != NULL; i++) { > if (!strcmp(key, QKeyCode_lookup[i])) { > break; > } > } > > if (strstart(key, "0x", NULL)) { > keycode = strtoul(key, &endp, 0); > if (*endp == '\0' && keycode >= 0x01 && keycode <= 0xff) { > for (i = 0; i < Q_KEY_CODE_MAX; i++) { > if (keycode == key_defs[i]) { > break; > } > } > } > } > > /* Return Q_KEY_CODE_MAX if the key is invalid */ > return i; > } > > If @key is a recognized key name, return its (positive) code. > Else, if @key is a hexadecimal number between 1 and 0xff, and a > recognized key with that code exists, return the number. > Else, return -1. > > The regression is *not* that the hexadecimal format isn't recognized > anymore, it's that numbers that don't have a key definition are now > rejected. Yes. > > This commit fixes the problem by adding hex value support down > > the QMP interface, qmp_send_key(). > > > > In more detail, this commit: > > > > 1. Adds the KeyValue union. This can represent an hex value or > > a QKeyCode value > > > > 2. *Changes* the QMP send-key command to take an KeyValue argument > > instead of a QKeyCode one > > > > 3. Adapt hmp_send_key() to the QMP interface changes > > > > Item 2 is an incompatible change, but as we're in development phase > > (and this command has been merged a few weeks ago) this shouldn't be > > a problem. > > > > Finally, it's not possible to split this commit without breaking the > > build. > > > > Reported-by: Avi Kivity <avi@redhat.com> > > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > > --- > > hmp.c | 43 +++++++++++++++++++++++++++++-------------- > > input.c | 33 +++++++++++++++++++++++++++------ > > qapi-schema.json | 20 +++++++++++++++++--- > > 3 files changed, 73 insertions(+), 23 deletions(-) > > > > diff --git a/hmp.c b/hmp.c > > index ba6fbd3..8738dc5 100644 > > --- a/hmp.c > > +++ b/hmp.c > > @@ -1109,13 +1109,13 @@ void hmp_closefd(Monitor *mon, const QDict *qdict) > > void hmp_send_key(Monitor *mon, const QDict *qdict) > > { > > const char *keys = qdict_get_str(qdict, "keys"); > > - QKeyCodeList *keylist, *head = NULL, *tmp = NULL; > > + KeyValueList *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, idx; > > + int keyname_len; > > > > while (1) { > > separator = strchr(keys, '-'); > > @@ -1129,15 +1129,8 @@ void hmp_send_key(Monitor *mon, const QDict *qdict) > > } > > keyname_buf[keyname_len] = 0; > > > > - idx = index_from_key(keyname_buf); > > - if (idx == Q_KEY_CODE_MAX) { > > - monitor_printf(mon, "invalid parameter: %s\n", keyname_buf); > > - break; > > - } > > - > > keylist = g_malloc0(sizeof(*keylist)); > > - keylist->value = idx; > > - keylist->next = NULL; > > + keylist->value = g_malloc0(sizeof(*keylist->value)); > > > > if (!head) { > > head = keylist; > > @@ -1147,17 +1140,39 @@ void hmp_send_key(Monitor *mon, const QDict *qdict) > > } > > tmp = keylist; > > > > + if (strstart(keyname_buf, "0x", NULL)) { > > + char *endp; > > + int value = strtoul(keyname_buf, &endp, 0); > > + if (*endp != '\0') { > > + goto err_out; > > + } > > + keylist->value->kind = KEY_VALUE_KIND_NUMBER; > > + keylist->value->number = value; > > + } else { > > + int idx = index_from_key(keyname_buf); > > + if (idx == Q_KEY_CODE_MAX) { > > + goto err_out; > > + } > > + keylist->value->kind = KEY_VALUE_KIND_QCODE; > > + keylist->value->qcode = idx; > > + } > > + > > TL;DR: this differs unnecessarily from the pre-QAPI code, which makes > the fix less obvious than it could be. But I think it should work, and > unbreaking autotest quickly is more important than making this patch > perfect, so I'm *not* asking for a respin because of that. > > Strings starting with "0x" are interpreted differently than before the > QAPI conversion. > > Before: > > If @key is a recognized key name, use its (positive) code. > Else, if @key is a hexadecimal number between 1 and 0xff, use the > number. > Else, error. > > After: > > If @key starts with "0x" > If @key is a number, use it (range check is done later) > Else, error > Else > If it's a recognized key name, use its (positive) code > Else, error. > > Makes a difference only if key names starting with "0x" exist. None > exist now, and such names aren't likely to appear in the future. > > Aside: codes starting with a digit do exist, and that's why we can do > only hexadecimal numeric keys. > > The use of strtoul() here is sloppy, but it's no worse as before, in > index_from_key(): > > * Numbers overflowing int are silently truncated, as before. > > * Numbers outside 0x01..0xff (possibly after truncation) are still > flagged as error, in qmp_send_key() below. > > > if (!separator) { > > break; > > } > > keys = separator + 1; > > } > > > > - if (idx != Q_KEY_CODE_MAX) { > > - qmp_send_key(head, has_hold_time, hold_time, &err); > > - } > > + qmp_send_key(head, has_hold_time, hold_time, &err); > > hmp_handle_error(mon, &err); > > - qapi_free_QKeyCodeList(head); > > + > > +out: > > + qapi_free_KeyValueList(head); > > + return; > > + > > +err_out: > > + monitor_printf(mon, "invalid parameter: %s\n", keyname_buf); > > + goto out; > > Jumping forward and backward like that is a bit ugly, but let's unbreak > autotest. > > > } > > > > void hmp_screen_dump(Monitor *mon, const QDict *qdict) > > diff --git a/input.c b/input.c > > index 32c6057..76ade64 100644 > > --- a/input.c > > +++ b/input.c > > @@ -228,6 +228,23 @@ static int *keycodes; > > static int keycodes_size; > > static QEMUTimer *key_timer; > > > > +static int keycode_from_keyvalue(const KeyValue *value) > > +{ > > + if (value->kind == KEY_VALUE_KIND_QCODE) { > > + return key_defs[value->qcode]; > > + } else { > > + assert(value->kind == KEY_VALUE_KIND_NUMBER); > > + return value->number; > > + } > > +} > > + > > +static void free_keycodes(void) > > +{ > > + g_free(keycodes); > > + keycodes = NULL; > > + keycodes_size = 0; > > +} > > + > > static void release_keys(void *opaque) > > { > > int i; > > @@ -239,16 +256,14 @@ static void release_keys(void *opaque) > > kbd_put_keycode(keycodes[i]| 0x80); > > } > > > > - g_free(keycodes); > > - keycodes = NULL; > > - keycodes_size = 0; > > + free_keycodes(); > > } > > > > -void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time, > > +void qmp_send_key(KeyValueList *keys, bool has_hold_time, int64_t hold_time, > > Error **errp) > > { > > int keycode; > > - QKeyCodeList *p; > > + KeyValueList *p; > > > > if (!key_timer) { > > key_timer = qemu_new_timer_ns(vm_clock, release_keys, NULL); > > @@ -265,7 +280,13 @@ void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time, > > > > for (p = keys; p != NULL; p = p->next) { > > /* key down events */ > > - keycode = key_defs[p->value]; > > + keycode = keycode_from_keyvalue(p->value); > > + if (keycode < 0x01 || keycode > 0xff) { > > + error_setg(errp, "invalid hex keycode 0x%x\n", keycode); > > + free_keycodes(); > > + return; > > + } > > + > > if (keycode & 0x80) { > > kbd_put_keycode(0xe0); > > } > > diff --git a/qapi-schema.json b/qapi-schema.json > > index 14e4419..02135cc 100644 > > --- a/qapi-schema.json > > +++ b/qapi-schema.json > > @@ -2588,12 +2588,26 @@ > > 'lf', 'help', 'meta_l', 'meta_r', 'compose' ] } > > > > ## > > +# @KeyValue > > +# > > +# Represents a keyboard key. > > +# > > Trailing whitespace, could be fixed up on merge if we care. Fixed in my queue. > > > +# Since: 1.3.0 > > +## > > +{ 'union': 'KeyValue', > > + 'data': { > > + 'number': 'int', > > + 'qcode': 'QKeyCode' } } > > + > > +## > > # @send-key: > > # > > # Send keys to guest. > > # > > -# @keys: key sequence. 'keys' is the name of the key. Use a JSON array to > > -# press several keys simultaneously. > > +# @keys: An array of @KeyValue elements. All @KeyValues in this array are > > +# simultaneously sent to the guest. A @KeyValue.number value is sent > > +# directly to the guest, while @KeyValue.qcode must be a valid > > +# @QKeyCode value > > # > > # @hold-time: #optional time to delay key up events, milliseconds. Defaults > > # to 100 > > @@ -2605,7 +2619,7 @@ > > # > > ## > > { 'command': 'send-key', > > - 'data': { 'keys': ['QKeyCode'], '*hold-time': 'int' } } > > + 'data': { 'keys': ['KeyValue'], '*hold-time': 'int' } } > > > > ## > > # @screendump: > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 3/3] input: index_from_key(): drop unused code 2012-09-26 20:25 [Qemu-devel] [PATCH v3 0/3] qmp: send-key: accept key codes in hex Luiz Capitulino 2012-09-26 20:25 ` [Qemu-devel] [PATCH 1/3] input: qmp_send_key(): simplify Luiz Capitulino 2012-09-26 20:25 ` [Qemu-devel] [PATCH 2/3] qmp: qmp_send_key(): accept key codes in hex Luiz Capitulino @ 2012-09-26 20:25 ` Luiz Capitulino 2012-09-26 21:23 ` [Qemu-devel] [PATCH v3 0/3] qmp: send-key: accept key codes in hex Eric Blake ` (2 subsequent siblings) 5 siblings, 0 replies; 13+ messages in thread From: Luiz Capitulino @ 2012-09-26 20:25 UTC (permalink / raw) To: qemu-devel; +Cc: akong, eblake, avi, armbru The hex key conversion is unused since last commit. Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> --- input.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/input.c b/input.c index 76ade64..25d3973 100644 --- a/input.c +++ b/input.c @@ -186,8 +186,7 @@ static const int key_defs[] = { int index_from_key(const char *key) { - int i, keycode; - char *endp; + int i; for (i = 0; QKeyCode_lookup[i] != NULL; i++) { if (!strcmp(key, QKeyCode_lookup[i])) { @@ -195,17 +194,6 @@ int index_from_key(const char *key) } } - if (strstart(key, "0x", NULL)) { - keycode = strtoul(key, &endp, 0); - if (*endp == '\0' && keycode >= 0x01 && keycode <= 0xff) { - for (i = 0; i < Q_KEY_CODE_MAX; i++) { - if (keycode == key_defs[i]) { - break; - } - } - } - } - /* Return Q_KEY_CODE_MAX if the key is invalid */ return i; } -- 1.7.12.315.g682ce8b ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/3] qmp: send-key: accept key codes in hex 2012-09-26 20:25 [Qemu-devel] [PATCH v3 0/3] qmp: send-key: accept key codes in hex Luiz Capitulino ` (2 preceding siblings ...) 2012-09-26 20:25 ` [Qemu-devel] [PATCH 3/3] input: index_from_key(): drop unused code Luiz Capitulino @ 2012-09-26 21:23 ` Eric Blake 2012-09-27 9:19 ` Avi Kivity 2012-09-27 11:43 ` Markus Armbruster 5 siblings, 0 replies; 13+ messages in thread From: Eric Blake @ 2012-09-26 21:23 UTC (permalink / raw) To: Luiz Capitulino; +Cc: armbru, akong, qemu-devel, avi [-- Attachment #1: Type: text/plain, Size: 998 bytes --] On 09/26/2012 02:25 PM, Luiz Capitulino wrote: > o v3 > > - doc & log fixups > - rename KeyCode.hex to KeyCode.number > > This actually fixes a regression introduced by the qapi conversion, > please refer to patch 2/3 for details. > > It's also important to note that this series changes the QMP > interface for the send-key command, but this shouldn't be a problem > as we're still in development phase. > > Luiz Capitulino (3): > input: qmp_send_key(): simplify > qmp: qmp_send_key(): accept key codes in hex > input: index_from_key(): drop unused code > > hmp.c | 43 +++++++++++++++++++++----------- > input.c | 75 ++++++++++++++++++++++++++++---------------------------- > qapi-schema.json | 20 ++++++++++++--- > 3 files changed, 84 insertions(+), 54 deletions(-) > Series: Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 617 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/3] qmp: send-key: accept key codes in hex 2012-09-26 20:25 [Qemu-devel] [PATCH v3 0/3] qmp: send-key: accept key codes in hex Luiz Capitulino ` (3 preceding siblings ...) 2012-09-26 21:23 ` [Qemu-devel] [PATCH v3 0/3] qmp: send-key: accept key codes in hex Eric Blake @ 2012-09-27 9:19 ` Avi Kivity 2012-09-27 12:14 ` Luiz Capitulino 2012-09-27 11:43 ` Markus Armbruster 5 siblings, 1 reply; 13+ messages in thread From: Avi Kivity @ 2012-09-27 9:19 UTC (permalink / raw) To: Luiz Capitulino; +Cc: akong, eblake, qemu-devel, Anthony Liguori, armbru On 09/26/2012 10:25 PM, Luiz Capitulino wrote: > o v3 > > - doc & log fixups > - rename KeyCode.hex to KeyCode.number > > This actually fixes a regression introduced by the qapi conversion, > please refer to patch 2/3 for details. > > It's also important to note that this series changes the QMP > interface for the send-key command, but this shouldn't be a problem > as we're still in development phase. > Anthony, this regression breaks autotest, so please give this series higher priority if possible. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/3] qmp: send-key: accept key codes in hex 2012-09-27 9:19 ` Avi Kivity @ 2012-09-27 12:14 ` Luiz Capitulino 0 siblings, 0 replies; 13+ messages in thread From: Luiz Capitulino @ 2012-09-27 12:14 UTC (permalink / raw) To: Avi Kivity; +Cc: akong, eblake, qemu-devel, Anthony Liguori, armbru On Thu, 27 Sep 2012 11:19:58 +0200 Avi Kivity <avi@redhat.com> wrote: > On 09/26/2012 10:25 PM, Luiz Capitulino wrote: > > o v3 > > > > - doc & log fixups > > - rename KeyCode.hex to KeyCode.number > > > > This actually fixes a regression introduced by the qapi conversion, > > please refer to patch 2/3 for details. > > > > It's also important to note that this series changes the QMP > > interface for the send-key command, but this shouldn't be a problem > > as we're still in development phase. > > > > Anthony, this regression breaks autotest, so please give this series > higher priority if possible. As my other in-flight series have already been reviewed, I'll send a pull request today with all queued qmp patches I have. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/3] qmp: send-key: accept key codes in hex 2012-09-26 20:25 [Qemu-devel] [PATCH v3 0/3] qmp: send-key: accept key codes in hex Luiz Capitulino ` (4 preceding siblings ...) 2012-09-27 9:19 ` Avi Kivity @ 2012-09-27 11:43 ` Markus Armbruster 5 siblings, 0 replies; 13+ messages in thread From: Markus Armbruster @ 2012-09-27 11:43 UTC (permalink / raw) To: Luiz Capitulino; +Cc: akong, eblake, qemu-devel, avi Luiz Capitulino <lcapitulino@redhat.com> writes: > o v3 > > - doc & log fixups > - rename KeyCode.hex to KeyCode.number > > This actually fixes a regression introduced by the qapi conversion, > please refer to patch 2/3 for details. > > It's also important to note that this series changes the QMP > interface for the send-key command, but this shouldn't be a problem > as we're still in development phase. Reviewed-by: Markus Armbruster <armbru@redhat.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 0/3]: qmp: send-key: accept key codes in hex @ 2012-09-21 14:55 Luiz Capitulino 2012-09-21 14:55 ` [Qemu-devel] [PATCH 1/3] input: qmp_send_key(): simplify Luiz Capitulino 0 siblings, 1 reply; 13+ messages in thread From: Luiz Capitulino @ 2012-09-21 14:55 UTC (permalink / raw) To: qemu-devel; +Cc: akong, eblake, avi o v2 - re-write some commit logs - fix a possible issue with error handling in patch 2/3 This actually fixes a regression introduced by the qapi conversion, please refer to patch 2/3 for details. It's also important to note that this series changes the QMP interface for the send-key command, but this shouldn't be a problem as we're still in development phase. hmp.c | 43 +++++++++++++++++++++----------- input.c | 75 ++++++++++++++++++++++++++++---------------------------- qapi-schema.json | 19 +++++++++++--- 3 files changed, 83 insertions(+), 54 deletions(-) ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 1/3] input: qmp_send_key(): simplify 2012-09-21 14:55 [Qemu-devel] [PATCH v2 0/3]: " Luiz Capitulino @ 2012-09-21 14:55 ` Luiz Capitulino 0 siblings, 0 replies; 13+ messages in thread From: Luiz Capitulino @ 2012-09-21 14:55 UTC (permalink / raw) To: qemu-devel; +Cc: akong, eblake, avi The current code duplicates the QKeyCodeList keys in order to store the key values for release_keys() late run. This is a bit complicated though, as we have to care about correct ordering and then release_keys() will have to index key_defs[] over again. Switch to an array of integers, which is dynamically allocated and stores the already converted key value. This simplifies the current code and the next commit. Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> --- input.c | 36 ++++++++++++++---------------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/input.c b/input.c index c4b0619..32c6057 100644 --- a/input.c +++ b/input.c @@ -224,30 +224,31 @@ int index_from_keycode(int code) return i; } -static QKeyCodeList *keycodes; +static int *keycodes; +static int keycodes_size; static QEMUTimer *key_timer; static void release_keys(void *opaque) { - int keycode; - QKeyCodeList *p; + int i; - for (p = keycodes; p != NULL; p = p->next) { - keycode = key_defs[p->value]; - if (keycode & 0x80) { + for (i = 0; i < keycodes_size; i++) { + if (keycodes[i] & 0x80) { kbd_put_keycode(0xe0); } - kbd_put_keycode(keycode | 0x80); + kbd_put_keycode(keycodes[i]| 0x80); } - qapi_free_QKeyCodeList(keycodes); + + g_free(keycodes); keycodes = NULL; + keycodes_size = 0; } void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time, Error **errp) { int keycode; - QKeyCodeList *p, *keylist, *head = NULL, *tmp = NULL; + QKeyCodeList *p; if (!key_timer) { key_timer = qemu_new_timer_ns(vm_clock, release_keys, NULL); @@ -257,31 +258,22 @@ void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time, qemu_del_timer(key_timer); release_keys(NULL); } + if (!has_hold_time) { hold_time = 100; } for (p = keys; p != NULL; p = p->next) { - keylist = g_malloc0(sizeof(*keylist)); - keylist->value = p->value; - keylist->next = NULL; - - if (!head) { - head = keylist; - } - if (tmp) { - tmp->next = keylist; - } - tmp = keylist; - /* key down events */ keycode = key_defs[p->value]; if (keycode & 0x80) { kbd_put_keycode(0xe0); } kbd_put_keycode(keycode & 0x7f); + + keycodes = g_realloc(keycodes, sizeof(int) * (keycodes_size + 1)); + keycodes[keycodes_size++] = keycode; } - keycodes = head; /* delayed key up events */ qemu_mod_timer(key_timer, qemu_get_clock_ns(vm_clock) + -- 1.7.12.315.g682ce8b ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 0/3]: qmp: send-key: accept key codes in hex @ 2012-09-20 18:17 Luiz Capitulino 2012-09-20 18:17 ` [Qemu-devel] [PATCH 1/3] input: qmp_send_key(): simplify Luiz Capitulino 0 siblings, 1 reply; 13+ messages in thread From: Luiz Capitulino @ 2012-09-20 18:17 UTC (permalink / raw) To: qemu-devel; +Cc: akong, eblake, avi This actually fixes a regression introduced by the qapi conversion, please refer to patch 2/3 for details. It's also important to note that this series changes the QMP interface for the send-key command, but this shouldn't be a problem as we're still in development phase. hmp.c | 43 +++++++++++++++++++++++------------ input.c | 69 +++++++++++++++++++++++++++----------------------------- qapi-schema.json | 19 +++++++++++++--- 3 files changed, 78 insertions(+), 53 deletions(-) ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 1/3] input: qmp_send_key(): simplify 2012-09-20 18:17 [Qemu-devel] [PATCH 0/3]: qmp: send-key: accept key codes in hex Luiz Capitulino @ 2012-09-20 18:17 ` Luiz Capitulino 0 siblings, 0 replies; 13+ messages in thread From: Luiz Capitulino @ 2012-09-20 18:17 UTC (permalink / raw) To: qemu-devel; +Cc: akong, eblake, avi The current code duplicates the QKeyCodeList keys in order to store the key values for release_keys() late run. This is a bit complicated though, as we have to care about correct ordering and then release_keys() will have to index key_defs[] over again. Switch to an array of integers, which is dynamically allocated and stores the already converted key value. This simplies the current code and the next commit. Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> --- input.c | 36 ++++++++++++++---------------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/input.c b/input.c index c4b0619..32c6057 100644 --- a/input.c +++ b/input.c @@ -224,30 +224,31 @@ int index_from_keycode(int code) return i; } -static QKeyCodeList *keycodes; +static int *keycodes; +static int keycodes_size; static QEMUTimer *key_timer; static void release_keys(void *opaque) { - int keycode; - QKeyCodeList *p; + int i; - for (p = keycodes; p != NULL; p = p->next) { - keycode = key_defs[p->value]; - if (keycode & 0x80) { + for (i = 0; i < keycodes_size; i++) { + if (keycodes[i] & 0x80) { kbd_put_keycode(0xe0); } - kbd_put_keycode(keycode | 0x80); + kbd_put_keycode(keycodes[i]| 0x80); } - qapi_free_QKeyCodeList(keycodes); + + g_free(keycodes); keycodes = NULL; + keycodes_size = 0; } void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time, Error **errp) { int keycode; - QKeyCodeList *p, *keylist, *head = NULL, *tmp = NULL; + QKeyCodeList *p; if (!key_timer) { key_timer = qemu_new_timer_ns(vm_clock, release_keys, NULL); @@ -257,31 +258,22 @@ void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time, qemu_del_timer(key_timer); release_keys(NULL); } + if (!has_hold_time) { hold_time = 100; } for (p = keys; p != NULL; p = p->next) { - keylist = g_malloc0(sizeof(*keylist)); - keylist->value = p->value; - keylist->next = NULL; - - if (!head) { - head = keylist; - } - if (tmp) { - tmp->next = keylist; - } - tmp = keylist; - /* key down events */ keycode = key_defs[p->value]; if (keycode & 0x80) { kbd_put_keycode(0xe0); } kbd_put_keycode(keycode & 0x7f); + + keycodes = g_realloc(keycodes, sizeof(int) * (keycodes_size + 1)); + keycodes[keycodes_size++] = keycode; } - keycodes = head; /* delayed key up events */ qemu_mod_timer(key_timer, qemu_get_clock_ns(vm_clock) + -- 1.7.12.315.g682ce8b ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2012-09-27 12:56 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-09-26 20:25 [Qemu-devel] [PATCH v3 0/3] qmp: send-key: accept key codes in hex Luiz Capitulino 2012-09-26 20:25 ` [Qemu-devel] [PATCH 1/3] input: qmp_send_key(): simplify Luiz Capitulino 2012-09-27 9:50 ` Markus Armbruster 2012-09-26 20:25 ` [Qemu-devel] [PATCH 2/3] qmp: qmp_send_key(): accept key codes in hex Luiz Capitulino 2012-09-27 11:42 ` Markus Armbruster 2012-09-27 12:57 ` Luiz Capitulino 2012-09-26 20:25 ` [Qemu-devel] [PATCH 3/3] input: index_from_key(): drop unused code Luiz Capitulino 2012-09-26 21:23 ` [Qemu-devel] [PATCH v3 0/3] qmp: send-key: accept key codes in hex Eric Blake 2012-09-27 9:19 ` Avi Kivity 2012-09-27 12:14 ` Luiz Capitulino 2012-09-27 11:43 ` Markus Armbruster -- strict thread matches above, loose matches on Subject: below -- 2012-09-21 14:55 [Qemu-devel] [PATCH v2 0/3]: " Luiz Capitulino 2012-09-21 14:55 ` [Qemu-devel] [PATCH 1/3] input: qmp_send_key(): simplify Luiz Capitulino 2012-09-20 18:17 [Qemu-devel] [PATCH 0/3]: qmp: send-key: accept key codes in hex Luiz Capitulino 2012-09-20 18:17 ` [Qemu-devel] [PATCH 1/3] input: qmp_send_key(): simplify Luiz Capitulino
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.