Re: [Qemu-devel] [PULL 13/15] qmp: qmp_send_key(): accept key codes in hex

[Amos Kong <akong@redhat.com>]

On 27/09/12 21:28, Luiz Capitulino wrote:
>

Sorry the delay review.

> 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

'0xdc' is not a available keycode, I didn't find '0xdc' in old key_defs[]
(before my send-key series, commit: 
8db972cfa469b4e4afd9c65e54e796b83b5ce3a2)


(latest upstream code: 6f8fd2530e9a530f237240daf1c981fa5df7f978)
(qemu) sendkey 0x10
'q' will be inputted to guest
(qemu) sendkey 0x1d-0x38-0xd3
guest will be reset

hex isn't support when using qmp monitor.


> 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>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@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 2de3140..3306bcd 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1113,13 +1113,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, '-');
> @@ -1133,15 +1133,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;
> @@ -1151,17 +1144,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 c6a6767..28d8815 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2621,12 +2621,26 @@
>                'lf', 'help', 'meta_l', 'meta_r', 'compose' ] }
>
>   ##
> +# @KeyValue
> +#
> +# Represents a keyboard key.
> +#
> +# Since: 1.3.0
> +##
> +{ 'union': 'KeyValue',
> +  'data': {
> +    'number': 'int',

It's 'int' not hex format.

==> works with 'int'
{ "execute": "send-key", "arguments": { 'keys': [{'type':'hex', 'data': 
29}, {'type':'hex', 'data': 56}, {'type':'hex', 'data': 211}]}}
{
     "return": {
     }
}
{
     "timestamp": {
         "seconds": 1348826804,
         "microseconds": 777545
     },
     "event": "RESET"
}

==> doesn't work with 'hex'
{ "execute": "send-key", "arguments": { 'keys': [{'type':'hex', 'data': 
0x1d}, {'type':'hex', 'data': 0x38}, {'type':'hex', 'data': 0xd3}]}}
{
     "error": {
         "class": "GenericError",
         "desc": "Invalid JSON syntax"
     }
}


> +    '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
> @@ -2638,7 +2652,7 @@
>   #
>   ##
>   { 'command': 'send-key',
> -  'data': { 'keys': ['QKeyCode'], '*hold-time': 'int' } }
> +  'data': { 'keys': ['KeyValue'], '*hold-time': 'int' } }
>
>   ##
>   # @screendump:


qmp-commands.hx also needs to be updated with latest examples:


>

-- 
			Amos.