[Qemu-devel] [PATCH v4 0/6] convert sendkey to qapi

[Qemu-devel] [PATCH v4 0/6] convert sendkey to qapi

[Amos Kong]

This series converted 'sendkey' command to qapi. The raw value
in hexadecimal format is not supported by 'send-key' of qmp.

Amos Kong (6):
  fix doc of using raw values with sendkey
  monitor: rename keyname '<' to 'less'
  hmp: rename arguments
  qapi: generate list struct and visit_list for enum
  qapi: convert sendkey
  ps2: output warning when event queue full

 console.h             |  152 +++++++++++++++++++++++++++++++
 hmp-commands.hx       |   10 +-
 hmp.c                 |   64 +++++++++++++
 hmp.h                 |    1 +
 hw/ps2.c              |    4 +-
 monitor.c             |  236 ++++++-------------------------------------------
 qapi-schema.json      |   46 ++++++++++
 qmp-commands.hx       |   28 ++++++
 scripts/qapi-types.py |   16 +++-
 scripts/qapi-visit.py |   14 +++-
 10 files changed, 354 insertions(+), 217 deletions(-)

---
Changes from v1:
- using a JSON array for the key names
- rename new error to 'QERR_OVERFLOW'
- fix command descriptions 
- qapi: generate list struct for enum
- add '<' fixing

Changes from v2:
- fix support of raw value in hexadecimal format
- fix bug in processing of '<-x'
- don't generate useless cleanup functions for enum
- introduced two functions for enum in qapi scripts
- fix command description 
- drop keys number limitation in sendkey
- drop patch: qerror: add QERR_OVERFLOW

Changes from v3:
- move key_defs[] to console.h
- link mapping tables by enum values
- rename 'sendkey' to 'send-key' for qmp

[Qemu-devel] [PATCH v4 1/6] fix doc of using raw values with sendkey

[Amos Kong]

(qemu) sendkey a
(qemu) sendkey 0x1e
(qemu) sendkey #0x1e
 unknown key: '#0x1e'

The last command doesn't work, '#' is not requested before
raw values. And the raw value in decimal format is also not
supported.

Signed-off-by: Amos Kong <akong@redhat.com>
---
 hmp-commands.hx |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index f5d9d91..30243b6 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -512,9 +512,9 @@ STEXI
 @item sendkey @var{keys}
 @findex sendkey
 
-Send @var{keys} to the emulator. @var{keys} could be the name of the
-key or @code{#} followed by the raw value in either decimal or hexadecimal
-format. Use @code{-} to press several keys simultaneously. Example:
+Send @var{keys} to the guest. @var{keys} could be the name of the
+key or the raw value in hexadecimal format. Use @code{-} to press
+several keys simultaneously. Example:
 @example
 sendkey ctrl-alt-f1
 @end example
-- 
1.7.1

[Qemu-devel] [PATCH v4 2/6] monitor: rename keyname '<' to 'less'

[Amos Kong]

There are many maps of keycode 0x56 in pc-bios/keymaps/*
  pc-bios/keymaps/common:less 0x56
  pc-bios/keymaps/common:greater 0x56 shift
  pc-bios/keymaps/common:bar 0x56 altgr
  pc-bios/keymaps/common:brokenbar 0x56 shift altgr

This patch just renames '<' to 'less', QAPI would add new
variable by adding a prefix to keyname, '$PREFIX_<' is not
available, '$PREFIX_less' is ok.

For compatibility, convert user inputted '<' to 'less'.

Signed-off-by: Amos Kong <akong@redhat.com>
---
 monitor.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index f6107ba..e87677d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1431,7 +1431,7 @@ static const KeyDef key_defs[] = {
     { 0x48, "kp_8" },
     { 0x49, "kp_9" },
 
-    { 0x56, "<" },
+    { 0x56, "less" },
 
     { 0x57, "f11" },
     { 0x58, "f12" },
@@ -1535,6 +1535,13 @@ static void do_sendkey(Monitor *mon, const QDict *qdict)
                 monitor_printf(mon, "too many keys\n");
                 return;
             }
+
+            /* Be compatible with old interface, convert user inputted "<" */
+            if (!strncmp(keyname_buf, "<", 1) && keyname_len == 1) {
+                pstrcpy(keyname_buf, sizeof(keyname_buf), "less");
+                keyname_len = 4;
+            }
+
             keyname_buf[keyname_len] = 0;
             keycode = get_keycode(keyname_buf);
             if (keycode < 0) {
-- 
1.7.1

[Qemu-devel] [PATCH v4 3/6] hmp: rename arguments

[Amos Kong]

Rename 'string' to 'keys', rename 'hold_time' to 'hold-time'.

Signed-off-by: Amos Kong <akong@redhat.com>
---
 hmp-commands.hx |    2 +-
 monitor.c       |   14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 30243b6..e336251 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -502,7 +502,7 @@ ETEXI
 
     {
         .name       = "sendkey",
-        .args_type  = "string:s,hold_time:i?",
+        .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,
diff --git a/monitor.c b/monitor.c
index e87677d..6fa0104 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1511,9 +1511,9 @@ static void do_sendkey(Monitor *mon, const QDict *qdict)
     char keyname_buf[16];
     char *separator;
     int keyname_len, keycode, i;
-    const char *string = qdict_get_str(qdict, "string");
-    int has_hold_time = qdict_haskey(qdict, "hold_time");
-    int hold_time = qdict_get_try_int(qdict, "hold_time", -1);
+    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);
 
     if (nb_pending_keycodes > 0) {
         qemu_del_timer(key_timer);
@@ -1523,10 +1523,10 @@ static void do_sendkey(Monitor *mon, const QDict *qdict)
         hold_time = 100;
     i = 0;
     while (1) {
-        separator = strchr(string, '-');
-        keyname_len = separator ? separator - string : strlen(string);
+        separator = strchr(keys, '-');
+        keyname_len = separator ? separator - keys : strlen(keys);
         if (keyname_len > 0) {
-            pstrcpy(keyname_buf, sizeof(keyname_buf), string);
+            pstrcpy(keyname_buf, sizeof(keyname_buf), keys);
             if (keyname_len > sizeof(keyname_buf) - 1) {
                 monitor_printf(mon, "invalid key: '%s...'\n", keyname_buf);
                 return;
@@ -1552,7 +1552,7 @@ static void do_sendkey(Monitor *mon, const QDict *qdict)
         }
         if (!separator)
             break;
-        string = separator + 1;
+        keys = separator + 1;
     }
     nb_pending_keycodes = i;
     /* key down events */
-- 
1.7.1

[Qemu-devel] [PATCH v4 4/6] qapi: generate list struct and visit_list for enum

[Amos Kong]

Currently, if define an 'enum' and use it in one command's data,
List struct for enum could not be generated, but it's used in
qmp function.

For example: KeyCodesList could not be generated.
>>> qapi-schema.json:
{ 'enum': 'KeyCodes',
  'data': [ 'shift', 'alt' ... ] }
{ 'command': 'sendkey',
  'data': { 'keys': ['KeyCodes'], '*hold-time': 'int' } }

>>> qmp-command.h:
void qmp_sendkey(KeyCodesList * keys, bool has_hold_time, int64_t
hold_time, Error **errp);

This patch makes qapi can generate List struct for enum.

Signed-off-by: Amos Kong <akong@redhat.com>
---
 scripts/qapi-types.py |   16 +++++++++++++++-
 scripts/qapi-visit.py |   14 +++++++++++++-
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 4a734f5..648eac7 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -28,6 +28,16 @@ typedef struct %(name)sList
 ''',
                  name=name)
 
+def generate_fwd_enum_struct(name, members):
+    return mcgen('''
+typedef struct %(name)sList
+{
+    %(name)s value;
+    struct %(name)sList *next;
+} %(name)sList;
+''',
+                 name=name)
+
 def generate_struct(structname, fieldname, members):
     ret = mcgen('''
 struct %(name)s
@@ -265,7 +275,8 @@ for expr in exprs:
     if expr.has_key('type'):
         ret += generate_fwd_struct(expr['type'], expr['data'])
     elif expr.has_key('enum'):
-        ret += generate_enum(expr['enum'], expr['data'])
+        ret += generate_enum(expr['enum'], expr['data']) + "\n"
+        ret += generate_fwd_enum_struct(expr['enum'], expr['data'])
         fdef.write(generate_enum_lookup(expr['enum'], expr['data']))
     elif expr.has_key('union'):
         ret += generate_fwd_struct(expr['union'], expr['data']) + "\n"
@@ -289,6 +300,9 @@ for expr in exprs:
         fdef.write(generate_type_cleanup(expr['union'] + "List") + "\n")
         ret += generate_type_cleanup_decl(expr['union'])
         fdef.write(generate_type_cleanup(expr['union']) + "\n")
+    elif expr.has_key('enum'):
+        ret += generate_type_cleanup_decl(expr['enum'] + "List")
+        fdef.write(generate_type_cleanup(expr['enum'] + "List") + "\n")
     else:
         continue
     fdecl.write(ret)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 8d4e94a..4d282d5 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -175,6 +175,16 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name,
 
     return ret
 
+def generate_enum_declaration(name, members, genlist=True):
+    ret = ""
+    if genlist:
+        ret += mcgen('''
+void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name, Error **errp);
+''',
+                     name=name)
+
+    return ret
+
 def generate_decl_enum(name, members, genlist=True):
     return mcgen('''
 
@@ -293,10 +303,12 @@ for expr in exprs:
         ret += generate_declaration(expr['union'], expr['data'])
         fdecl.write(ret)
     elif expr.has_key('enum'):
-        ret = generate_visit_enum(expr['enum'], expr['data'])
+        ret = generate_visit_list(expr['enum'], expr['data'])
+        ret += generate_visit_enum(expr['enum'], expr['data'])
         fdef.write(ret)
 
         ret = generate_decl_enum(expr['enum'], expr['data'])
+        ret += generate_enum_declaration(expr['enum'], expr['data'])
         fdecl.write(ret)
 
 fdecl.write('''
-- 
1.7.1

Re: [Qemu-devel] [PATCH v4 4/6] qapi: generate list struct and visit_list for enum

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 769 bytes --]

On 07/05/2012 06:48 AM, Amos Kong wrote:
> Currently, if define an 'enum' and use it in one command's data,
> List struct for enum could not be generated, but it's used in
> qmp function.
> 
> For example: KeyCodesList could not be generated.
>>>> qapi-schema.json:
> { 'enum': 'KeyCodes',
>   'data': [ 'shift', 'alt' ... ] }
> { 'command': 'sendkey',
>   'data': { 'keys': ['KeyCodes'], '*hold-time': 'int' } }
> 
>>>> qmp-command.h:
> void qmp_sendkey(KeyCodesList * keys, bool has_hold_time, int64_t
> hold_time, Error **errp);
> 
> This patch makes qapi can generate List struct for enum.

Grammar nit:

s/makes qapi can/lets/qapi/

-- 
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: 620 bytes --]

[Qemu-devel] [PATCH v4 5/6] qapi: convert sendkey

[Amos Kong]

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 console.h is the mapping of key name to keycode,
Keys' index in the enmu and key_defs[] is same.

'send-key' of QMP doesn't support key in hexadecimal format.

Signed-off-by: Amos Kong <akong@redhat.com>
---
 console.h        |  152 ++++++++++++++++++++++++++++++++++
 hmp-commands.hx  |    2 +-
 hmp.c            |   64 +++++++++++++++
 hmp.h            |    1 +
 monitor.c        |  239 ++++++------------------------------------------------
 qapi-schema.json |   46 +++++++++++
 qmp-commands.hx  |   28 +++++++
 7 files changed, 317 insertions(+), 215 deletions(-)

diff --git a/console.h b/console.h
index 4334db5..e1b0c45 100644
--- a/console.h
+++ b/console.h
@@ -6,6 +6,7 @@
 #include "notify.h"
 #include "monitor.h"
 #include "trace.h"
+#include "qapi-types.h"
 
 /* keyboard/mouse support */
 
@@ -397,4 +398,155 @@ static inline int vnc_display_pw_expire(DisplayState *ds, time_t expires)
 /* curses.c */
 void curses_display_init(DisplayState *ds, int full_screen);
 
+typedef struct {
+    int keycode;
+    const char *name;
+} KeyDef;
+
+static const KeyDef key_defs[] = {
+    [KEY_CODES_SHIFT] = { 0x2a, "shift" },
+    [KEY_CODES_SHIFT_R] = { 0x36, "shift_r" },
+
+    [KEY_CODES_ALT] = { 0x38, "alt" },
+    [KEY_CODES_ALT_R] = { 0xb8, "alt_r" },
+    [KEY_CODES_ALTGR] = { 0x64, "altgr" },
+    [KEY_CODES_ALTGR_R] = { 0xe4, "altgr_r" },
+    [KEY_CODES_CTRL] = { 0x1d, "ctrl" },
+    [KEY_CODES_CTRL_R] = { 0x9d, "ctrl_r" },
+
+    [KEY_CODES_MENU] = { 0xdd, "menu" },
+
+    [KEY_CODES_ESC] = { 0x01, "esc" },
+
+    [KEY_CODES_1] = { 0x02, "1" },
+    [KEY_CODES_2] = { 0x03, "2" },
+    [KEY_CODES_3] = { 0x04, "3" },
+    [KEY_CODES_4] = { 0x05, "4" },
+    [KEY_CODES_5] = { 0x06, "5" },
+    [KEY_CODES_6] = { 0x07, "6" },
+    [KEY_CODES_7] = { 0x08, "7" },
+    [KEY_CODES_8] = { 0x09, "8" },
+    [KEY_CODES_9] = { 0x0a, "9" },
+    [KEY_CODES_0] = { 0x0b, "0" },
+    [KEY_CODES_MINUS] = { 0x0c, "minus" },
+    [KEY_CODES_EQUAL] = { 0x0d, "equal" },
+    [KEY_CODES_BACKSPACE] = { 0x0e, "backspace" },
+
+    [KEY_CODES_TAB] = { 0x0f, "tab" },
+    [KEY_CODES_Q] = { 0x10, "q" },
+    [KEY_CODES_W] = { 0x11, "w" },
+    [KEY_CODES_E] = { 0x12, "e" },
+    [KEY_CODES_R] = { 0x13, "r" },
+    [KEY_CODES_T] = { 0x14, "t" },
+    [KEY_CODES_Y] = { 0x15, "y" },
+    [KEY_CODES_U] = { 0x16, "u" },
+    [KEY_CODES_I] = { 0x17, "i" },
+    [KEY_CODES_O] = { 0x18, "o" },
+    [KEY_CODES_P] = { 0x19, "p" },
+    [KEY_CODES_BRACKET_LEFT] = { 0x1a, "bracket_left" },
+    [KEY_CODES_BRACKET_RIGHT] = { 0x1b, "bracket_right" },
+    [KEY_CODES_RET] = { 0x1c, "ret" },
+
+    [KEY_CODES_A] = { 0x1e, "a" },
+    [KEY_CODES_S] = { 0x1f, "s" },
+    [KEY_CODES_D] = { 0x20, "d" },
+    [KEY_CODES_F] = { 0x21, "f" },
+    [KEY_CODES_G] = { 0x22, "g" },
+    [KEY_CODES_H] = { 0x23, "h" },
+    [KEY_CODES_J] = { 0x24, "j" },
+    [KEY_CODES_K] = { 0x25, "k" },
+    [KEY_CODES_L] = { 0x26, "l" },
+    [KEY_CODES_SEMICOLON] = { 0x27, "semicolon" },
+    [KEY_CODES_APOSTROPHE] = { 0x28, "apostrophe" },
+    [KEY_CODES_GRAVE_ACCENT] = { 0x29, "grave_accent" },
+
+    [KEY_CODES_BACKSLASH] = { 0x2b, "backslash" },
+    [KEY_CODES_Z] = { 0x2c, "z" },
+    [KEY_CODES_X] = { 0x2d, "x" },
+    [KEY_CODES_C] = { 0x2e, "c" },
+    [KEY_CODES_V] = { 0x2f, "v" },
+    [KEY_CODES_B] = { 0x30, "b" },
+    [KEY_CODES_N] = { 0x31, "n" },
+    [KEY_CODES_M] = { 0x32, "m" },
+    [KEY_CODES_COMMA] = { 0x33, "comma" },
+    [KEY_CODES_DOT] = { 0x34, "dot" },
+    [KEY_CODES_SLASH] = { 0x35, "slash" },
+
+    [KEY_CODES_ASTERISK] = { 0x37, "asterisk" },
+
+    [KEY_CODES_SPC] = { 0x39, "spc" },
+    [KEY_CODES_CAPS_LOCK] = { 0x3a, "caps_lock" },
+    [KEY_CODES_F1] = { 0x3b, "f1" },
+    [KEY_CODES_F2] = { 0x3c, "f2" },
+    [KEY_CODES_F3] = { 0x3d, "f3" },
+    [KEY_CODES_F4] = { 0x3e, "f4" },
+    [KEY_CODES_F5] = { 0x3f, "f5" },
+    [KEY_CODES_F6] = { 0x40, "f6" },
+    [KEY_CODES_F7] = { 0x41, "f7" },
+    [KEY_CODES_F8] = { 0x42, "f8" },
+    [KEY_CODES_F9] = { 0x43, "f9" },
+    [KEY_CODES_F10] = { 0x44, "f10" },
+    [KEY_CODES_NUM_LOCK] = { 0x45, "num_lock" },
+    [KEY_CODES_SCROLL_LOCK] = { 0x46, "scroll_lock" },
+
+    [KEY_CODES_KP_DIVIDE] = { 0xb5, "kp_divide" },
+    [KEY_CODES_KP_MULTIPLY] = { 0x37, "kp_multiply" },
+    [KEY_CODES_KP_SUBTRACT] = { 0x4a, "kp_subtract" },
+    [KEY_CODES_KP_ADD] = { 0x4e, "kp_add" },
+    [KEY_CODES_KP_ENTER] = { 0x9c, "kp_enter" },
+    [KEY_CODES_KP_DECIMAL] = { 0x53, "kp_decimal" },
+    [KEY_CODES_SYSRQ] = { 0x54, "sysrq" },
+
+    [KEY_CODES_KP_0] = { 0x52, "kp_0" },
+    [KEY_CODES_KP_1] = { 0x4f, "kp_1" },
+    [KEY_CODES_KP_2] = { 0x50, "kp_2" },
+    [KEY_CODES_KP_3] = { 0x51, "kp_3" },
+    [KEY_CODES_KP_4] = { 0x4b, "kp_4" },
+    [KEY_CODES_KP_5] = { 0x4c, "kp_5" },
+    [KEY_CODES_KP_6] = { 0x4d, "kp_6" },
+    [KEY_CODES_KP_7] = { 0x47, "kp_7" },
+    [KEY_CODES_KP_8] = { 0x48, "kp_8" },
+    [KEY_CODES_KP_9] = { 0x49, "kp_9" },
+
+    [KEY_CODES_LESS] = { 0x56, "less" },
+
+    [KEY_CODES_F11] = { 0x57, "f11" },
+    [KEY_CODES_F12] = { 0x58, "f12" },
+
+    [KEY_CODES_PRINT] = { 0xb7, "print" },
+
+    [KEY_CODES_HOME] = { 0xc7, "home" },
+    [KEY_CODES_PGUP] = { 0xc9, "pgup" },
+    [KEY_CODES_PGDN] = { 0xd1, "pgdn" },
+    [KEY_CODES_END] = { 0xcf, "end" },
+
+    [KEY_CODES_LEFT] = { 0xcb, "left" },
+    [KEY_CODES_UP] = { 0xc8, "up" },
+    [KEY_CODES_DOWN] = { 0xd0, "down" },
+    [KEY_CODES_RIGHT] = { 0xcd, "right" },
+
+    [KEY_CODES_INSERT] = { 0xd2, "insert" },
+    [KEY_CODES_DELETE] = { 0xd3, "delete" },
+#ifdef NEED_CPU_H
+#if defined(TARGET_SPARC) && !defined(TARGET_SPARC64)
+    [KEY_CODES_STOP] = { 0xf0, "stop" },
+    [KEY_CODES_AGAIN] = { 0xf1, "again" },
+    [KEY_CODES_PROPS] = { 0xf2, "props" },
+    [KEY_CODES_UNDO] = { 0xf3, "undo" },
+    [KEY_CODES_FRONT] = { 0xf4, "front" },
+    [KEY_CODES_COPY] = { 0xf5, "copy" },
+    [KEY_CODES_OPEN] = { 0xf6, "open" },
+    [KEY_CODES_PASTE] = { 0xf7, "paste" },
+    [KEY_CODES_FIND] = { 0xf8, "find" },
+    [KEY_CODES_CUT] = { 0xf9, "cut" },
+    [KEY_CODES_LF] = { 0xfa, "lf" },
+    [KEY_CODES_HELP] = { 0xfb, "help" },
+    [KEY_CODES_META_L] = { 0xfc, "meta_l" },
+    [KEY_CODES_META_R] = { 0xfd, "meta_r" },
+    [KEY_CODES_COMPOSE] = { 0xfe, "compose" },
+#endif
+#endif
+    [KEY_CODES_MAX] = { 0, NULL },
+};
+
 #endif
diff --git a/hmp-commands.hx b/hmp-commands.hx
index e336251..865eea9 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_send_key,
     },
 
 STEXI
diff --git a/hmp.c b/hmp.c
index b9cec1d..cfdc106 100644
--- a/hmp.c
+++ b/hmp.c
@@ -19,6 +19,7 @@
 #include "qemu-timer.h"
 #include "qmp-commands.h"
 #include "monitor.h"
+#include "console.h"
 
 static void hmp_handle_error(Monitor *mon, Error **errp)
 {
@@ -1000,3 +1001,66 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
     qmp_netdev_del(id, &err);
     hmp_handle_error(mon, &err);
 }
+
+static int get_key_index(const char *key)
+{
+    int i, keycode;
+    char *endp;
+
+    for (i = 0; i < KEY_CODES_MAX; i++)
+        if (key_defs[i].keycode && !strcmp(key, key_defs[i].name))
+            return i;
+
+    if (strstart(key, "0x", NULL)) {
+        keycode = strtoul(key, &endp, 0);
+        if (*endp == '\0' && keycode >= 0x01 && keycode <= 0xff)
+            for (i = 0; i < KEY_CODES_MAX; i++)
+                if (keycode == key_defs[i].keycode)
+                    return i;
+    }
+
+    return -1;
+}
+
+void hmp_send_key(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 (!strncmp(keyname_buf, "<", 1) && keyname_len == 1) {
+            pstrcpy(keyname_buf, sizeof(keyname_buf), "less");
+            keyname_len = 4;
+        }
+        keyname_buf[keyname_len] = 0;
+
+        keylist = g_malloc0(sizeof(*keylist));
+        keylist->value = get_key_index(keyname_buf);
+        keylist->next = NULL;
+
+        if (tmp)
+            tmp->next = keylist;
+        tmp = keylist;
+        if (!head)
+            head = keylist;
+
+        if (!separator)
+            break;
+        keys = separator + 1;
+    }
+
+    qmp_send_key(head, has_hold_time, hold_time, &err);
+    hmp_handle_error(mon, &err);
+    qapi_free_KeyCodesList(head);
+}
diff --git a/hmp.h b/hmp.h
index 79d138d..c17769f 100644
--- a/hmp.h
+++ b/hmp.h
@@ -64,5 +64,6 @@ void hmp_device_del(Monitor *mon, const QDict *qdict);
 void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
 void hmp_netdev_add(Monitor *mon, const QDict *qdict);
 void hmp_netdev_del(Monitor *mon, const QDict *qdict);
+void hmp_send_key(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/monitor.c b/monitor.c
index 6fa0104..994b85c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1321,247 +1321,58 @@ static void do_sum(Monitor *mon, const QDict *qdict)
     monitor_printf(mon, "%05d\n", sum);
 }
 
-typedef struct {
-    int keycode;
-    const char *name;
-} KeyDef;
-
-static const KeyDef key_defs[] = {
-    { 0x2a, "shift" },
-    { 0x36, "shift_r" },
-
-    { 0x38, "alt" },
-    { 0xb8, "alt_r" },
-    { 0x64, "altgr" },
-    { 0xe4, "altgr_r" },
-    { 0x1d, "ctrl" },
-    { 0x9d, "ctrl_r" },
-
-    { 0xdd, "menu" },
-
-    { 0x01, "esc" },
-
-    { 0x02, "1" },
-    { 0x03, "2" },
-    { 0x04, "3" },
-    { 0x05, "4" },
-    { 0x06, "5" },
-    { 0x07, "6" },
-    { 0x08, "7" },
-    { 0x09, "8" },
-    { 0x0a, "9" },
-    { 0x0b, "0" },
-    { 0x0c, "minus" },
-    { 0x0d, "equal" },
-    { 0x0e, "backspace" },
-
-    { 0x0f, "tab" },
-    { 0x10, "q" },
-    { 0x11, "w" },
-    { 0x12, "e" },
-    { 0x13, "r" },
-    { 0x14, "t" },
-    { 0x15, "y" },
-    { 0x16, "u" },
-    { 0x17, "i" },
-    { 0x18, "o" },
-    { 0x19, "p" },
-    { 0x1a, "bracket_left" },
-    { 0x1b, "bracket_right" },
-    { 0x1c, "ret" },
-
-    { 0x1e, "a" },
-    { 0x1f, "s" },
-    { 0x20, "d" },
-    { 0x21, "f" },
-    { 0x22, "g" },
-    { 0x23, "h" },
-    { 0x24, "j" },
-    { 0x25, "k" },
-    { 0x26, "l" },
-    { 0x27, "semicolon" },
-    { 0x28, "apostrophe" },
-    { 0x29, "grave_accent" },
-
-    { 0x2b, "backslash" },
-    { 0x2c, "z" },
-    { 0x2d, "x" },
-    { 0x2e, "c" },
-    { 0x2f, "v" },
-    { 0x30, "b" },
-    { 0x31, "n" },
-    { 0x32, "m" },
-    { 0x33, "comma" },
-    { 0x34, "dot" },
-    { 0x35, "slash" },
-
-    { 0x37, "asterisk" },
-
-    { 0x39, "spc" },
-    { 0x3a, "caps_lock" },
-    { 0x3b, "f1" },
-    { 0x3c, "f2" },
-    { 0x3d, "f3" },
-    { 0x3e, "f4" },
-    { 0x3f, "f5" },
-    { 0x40, "f6" },
-    { 0x41, "f7" },
-    { 0x42, "f8" },
-    { 0x43, "f9" },
-    { 0x44, "f10" },
-    { 0x45, "num_lock" },
-    { 0x46, "scroll_lock" },
-
-    { 0xb5, "kp_divide" },
-    { 0x37, "kp_multiply" },
-    { 0x4a, "kp_subtract" },
-    { 0x4e, "kp_add" },
-    { 0x9c, "kp_enter" },
-    { 0x53, "kp_decimal" },
-    { 0x54, "sysrq" },
-
-    { 0x52, "kp_0" },
-    { 0x4f, "kp_1" },
-    { 0x50, "kp_2" },
-    { 0x51, "kp_3" },
-    { 0x4b, "kp_4" },
-    { 0x4c, "kp_5" },
-    { 0x4d, "kp_6" },
-    { 0x47, "kp_7" },
-    { 0x48, "kp_8" },
-    { 0x49, "kp_9" },
-
-    { 0x56, "less" },
-
-    { 0x57, "f11" },
-    { 0x58, "f12" },
-
-    { 0xb7, "print" },
-
-    { 0xc7, "home" },
-    { 0xc9, "pgup" },
-    { 0xd1, "pgdn" },
-    { 0xcf, "end" },
-
-    { 0xcb, "left" },
-    { 0xc8, "up" },
-    { 0xd0, "down" },
-    { 0xcd, "right" },
-
-    { 0xd2, "insert" },
-    { 0xd3, "delete" },
-#if defined(TARGET_SPARC) && !defined(TARGET_SPARC64)
-    { 0xf0, "stop" },
-    { 0xf1, "again" },
-    { 0xf2, "props" },
-    { 0xf3, "undo" },
-    { 0xf4, "front" },
-    { 0xf5, "copy" },
-    { 0xf6, "open" },
-    { 0xf7, "paste" },
-    { 0xf8, "find" },
-    { 0xf9, "cut" },
-    { 0xfa, "lf" },
-    { 0xfb, "help" },
-    { 0xfc, "meta_l" },
-    { 0xfd, "meta_r" },
-    { 0xfe, "compose" },
-#endif
-    { 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;
+static KeyCodesList *keycodes;
 static QEMUTimer *key_timer;
 
 static void release_keys(void *opaque)
 {
     int keycode;
+    KeyCodesList *p;
+
+    for (p = keycodes; p != NULL; p = p->next) {
+        if (p->value > KEY_CODES_MAX) {
+            keycodes = NULL;
+            break;
+        }
 
-    while (nb_pending_keycodes > 0) {
-        nb_pending_keycodes--;
-        keycode = keycodes[nb_pending_keycodes];
+        keycode = key_defs[p->value].keycode;
         if (keycode & 0x80)
             kbd_put_keycode(0xe0);
         kbd_put_keycode(keycode | 0x80);
     }
 }
 
-static void do_sendkey(Monitor *mon, const QDict *qdict)
+void qmp_send_key(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;
+    char value[5];
+    KeyCodesList *p;
 
-    if (nb_pending_keycodes > 0) {
+    if (keycodes != NULL) {
         qemu_del_timer(key_timer);
         release_keys(NULL);
     }
     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 (!strncmp(keyname_buf, "<", 1) && keyname_len == 1) {
-                pstrcpy(keyname_buf, sizeof(keyname_buf), "less");
-                keyname_len = 4;
-            }
+    keycodes = keys;
 
-            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;
+    for (p = keycodes; p != NULL; p = p->next) {
+        if (p->value > KEY_CODES_MAX) {
+            sprintf(value, "%d", p->value);
+            error_set(errp, QERR_INVALID_PARAMETER, value);
+            return;
         }
-        if (!separator)
-            break;
-        keys = separator + 1;
-    }
-    nb_pending_keycodes = i;
-    /* key down events */
-    for (i = 0; i < nb_pending_keycodes; i++) {
-        keycode = keycodes[i];
+
+        /* key down events */
+        keycode = key_defs[p->value].keycode;
         if (keycode & 0x80)
             kbd_put_keycode(0xe0);
         kbd_put_keycode(keycode & 0x7f);
     }
+
     /* delayed key up events */
     qemu_mod_timer(key_timer, qemu_get_clock_ns(vm_clock) +
                    muldiv64(get_ticks_per_sec(), hold_time, 1000));
diff --git a/qapi-schema.json b/qapi-schema.json
index 3b6e346..08e51c6 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1862,3 +1862,49 @@
 # Since: 0.14.0
 ##
 { 'command': 'netdev_del', 'data': {'id': 'str'} }
+
+##
+# @KeyCodes:
+#
+# An enumeration of key name.
+#
+# This is used by the send-key command.
+#
+# Since: 1.2
+##
+{ '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' ] }
+
+##
+# @send-key:
+#
+# Send keys to guest.
+#
+# @keys: key sequence. 'keys' could be the name of the key. Use a JSON array to
+#        press several keys simultaneously.
+#
+# @hold-time: #optional time to delay key up events, milliseconds. Defaults
+#             to 200
+#
+# Returns: Nothing on success
+#          If key is unknown or redundant, InvalidParameter
+#
+# Since: 1.2
+#
+##
+{ 'command': 'send-key',
+  'data': { 'keys': ['KeyCodes'], '*hold-time': 'int' } }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 2e1a38e..12f6b76 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -335,6 +335,34 @@ Example:
 EQMP
 
     {
+        .name       = "send-key",
+        .args_type  = "keys:O,hold-time:i?",
+        .mhandler.cmd_new = qmp_marshal_input_send_key,
+    },
+
+SQMP
+send-key
+----------
+
+Send keys to VM.
+
+Arguments:
+
+keys array:
+    - "key": key sequence (a json-array of key enum values)
+
+- hold-time: time to delay key up events, milliseconds. Defaults to 200
+             (json-int, optional)
+
+Example:
+
+-> { "execute": "send-key",
+     "arguments": { 'keys': [ 'ctrl', 'alt', 'delete' ] } }
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "cpu",
         .args_type  = "index:i",
         .mhandler.cmd_new = qmp_marshal_input_cpu,
-- 
1.7.1

Re: [Qemu-devel] [PATCH v4 5/6] qapi: convert sendkey

[Luiz Capitulino]

On Thu,  5 Jul 2012 20:48:44 +0800
Amos Kong <akong@redhat.com> 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 console.h is the mapping of key name to keycode,
> Keys' index in the enmu and key_defs[] is same.
> 
> 'send-key' of QMP doesn't support key in hexadecimal format.
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
>  console.h        |  152 ++++++++++++++++++++++++++++++++++
>  hmp-commands.hx  |    2 +-
>  hmp.c            |   64 +++++++++++++++
>  hmp.h            |    1 +
>  monitor.c        |  239 ++++++------------------------------------------------
>  qapi-schema.json |   46 +++++++++++
>  qmp-commands.hx  |   28 +++++++
>  7 files changed, 317 insertions(+), 215 deletions(-)
> 
> diff --git a/console.h b/console.h
> index 4334db5..e1b0c45 100644
> --- a/console.h
> +++ b/console.h
> @@ -6,6 +6,7 @@
>  #include "notify.h"
>  #include "monitor.h"
>  #include "trace.h"
> +#include "qapi-types.h"
>  
>  /* keyboard/mouse support */
>  
> @@ -397,4 +398,155 @@ static inline int vnc_display_pw_expire(DisplayState *ds, time_t expires)
>  /* curses.c */
>  void curses_display_init(DisplayState *ds, int full_screen);
>  
> +typedef struct {
> +    int keycode;
> +    const char *name;

I don't think we need 'name', as key names are also provided by KeyCodes_lookup[].
See more on this below.

> +} KeyDef;
> +
> +static const KeyDef key_defs[] = {

We can't have an array defined in a header file because it will be defined in
each .c file that includes it.

Please, define it in input.c (along with qmp_send_key()) and write the following
public functions:

 o KeyCode keycode_from_key(const char *key);
 o KeyCode keycode_from_code(int code);

and then use these functions where using key_defs would be necessary. Also,
note that keycode_from_key() can use KeyCodes_lookup[] instead of key_defs (this
way we can drop 'name' from KeyDef).

> +    [KEY_CODES_SHIFT] = { 0x2a, "shift" },
> +    [KEY_CODES_SHIFT_R] = { 0x36, "shift_r" },
> +
> +    [KEY_CODES_ALT] = { 0x38, "alt" },
> +    [KEY_CODES_ALT_R] = { 0xb8, "alt_r" },
> +    [KEY_CODES_ALTGR] = { 0x64, "altgr" },
> +    [KEY_CODES_ALTGR_R] = { 0xe4, "altgr_r" },
> +    [KEY_CODES_CTRL] = { 0x1d, "ctrl" },
> +    [KEY_CODES_CTRL_R] = { 0x9d, "ctrl_r" },
> +
> +    [KEY_CODES_MENU] = { 0xdd, "menu" },
> +
> +    [KEY_CODES_ESC] = { 0x01, "esc" },
> +
> +    [KEY_CODES_1] = { 0x02, "1" },
> +    [KEY_CODES_2] = { 0x03, "2" },
> +    [KEY_CODES_3] = { 0x04, "3" },
> +    [KEY_CODES_4] = { 0x05, "4" },
> +    [KEY_CODES_5] = { 0x06, "5" },
> +    [KEY_CODES_6] = { 0x07, "6" },
> +    [KEY_CODES_7] = { 0x08, "7" },
> +    [KEY_CODES_8] = { 0x09, "8" },
> +    [KEY_CODES_9] = { 0x0a, "9" },
> +    [KEY_CODES_0] = { 0x0b, "0" },
> +    [KEY_CODES_MINUS] = { 0x0c, "minus" },
> +    [KEY_CODES_EQUAL] = { 0x0d, "equal" },
> +    [KEY_CODES_BACKSPACE] = { 0x0e, "backspace" },
> +
> +    [KEY_CODES_TAB] = { 0x0f, "tab" },
> +    [KEY_CODES_Q] = { 0x10, "q" },
> +    [KEY_CODES_W] = { 0x11, "w" },
> +    [KEY_CODES_E] = { 0x12, "e" },
> +    [KEY_CODES_R] = { 0x13, "r" },
> +    [KEY_CODES_T] = { 0x14, "t" },
> +    [KEY_CODES_Y] = { 0x15, "y" },
> +    [KEY_CODES_U] = { 0x16, "u" },
> +    [KEY_CODES_I] = { 0x17, "i" },
> +    [KEY_CODES_O] = { 0x18, "o" },
> +    [KEY_CODES_P] = { 0x19, "p" },
> +    [KEY_CODES_BRACKET_LEFT] = { 0x1a, "bracket_left" },
> +    [KEY_CODES_BRACKET_RIGHT] = { 0x1b, "bracket_right" },
> +    [KEY_CODES_RET] = { 0x1c, "ret" },
> +
> +    [KEY_CODES_A] = { 0x1e, "a" },
> +    [KEY_CODES_S] = { 0x1f, "s" },
> +    [KEY_CODES_D] = { 0x20, "d" },
> +    [KEY_CODES_F] = { 0x21, "f" },
> +    [KEY_CODES_G] = { 0x22, "g" },
> +    [KEY_CODES_H] = { 0x23, "h" },
> +    [KEY_CODES_J] = { 0x24, "j" },
> +    [KEY_CODES_K] = { 0x25, "k" },
> +    [KEY_CODES_L] = { 0x26, "l" },
> +    [KEY_CODES_SEMICOLON] = { 0x27, "semicolon" },
> +    [KEY_CODES_APOSTROPHE] = { 0x28, "apostrophe" },
> +    [KEY_CODES_GRAVE_ACCENT] = { 0x29, "grave_accent" },
> +
> +    [KEY_CODES_BACKSLASH] = { 0x2b, "backslash" },
> +    [KEY_CODES_Z] = { 0x2c, "z" },
> +    [KEY_CODES_X] = { 0x2d, "x" },
> +    [KEY_CODES_C] = { 0x2e, "c" },
> +    [KEY_CODES_V] = { 0x2f, "v" },
> +    [KEY_CODES_B] = { 0x30, "b" },
> +    [KEY_CODES_N] = { 0x31, "n" },
> +    [KEY_CODES_M] = { 0x32, "m" },
> +    [KEY_CODES_COMMA] = { 0x33, "comma" },
> +    [KEY_CODES_DOT] = { 0x34, "dot" },
> +    [KEY_CODES_SLASH] = { 0x35, "slash" },
> +
> +    [KEY_CODES_ASTERISK] = { 0x37, "asterisk" },
> +
> +    [KEY_CODES_SPC] = { 0x39, "spc" },
> +    [KEY_CODES_CAPS_LOCK] = { 0x3a, "caps_lock" },
> +    [KEY_CODES_F1] = { 0x3b, "f1" },
> +    [KEY_CODES_F2] = { 0x3c, "f2" },
> +    [KEY_CODES_F3] = { 0x3d, "f3" },
> +    [KEY_CODES_F4] = { 0x3e, "f4" },
> +    [KEY_CODES_F5] = { 0x3f, "f5" },
> +    [KEY_CODES_F6] = { 0x40, "f6" },
> +    [KEY_CODES_F7] = { 0x41, "f7" },
> +    [KEY_CODES_F8] = { 0x42, "f8" },
> +    [KEY_CODES_F9] = { 0x43, "f9" },
> +    [KEY_CODES_F10] = { 0x44, "f10" },
> +    [KEY_CODES_NUM_LOCK] = { 0x45, "num_lock" },
> +    [KEY_CODES_SCROLL_LOCK] = { 0x46, "scroll_lock" },
> +
> +    [KEY_CODES_KP_DIVIDE] = { 0xb5, "kp_divide" },
> +    [KEY_CODES_KP_MULTIPLY] = { 0x37, "kp_multiply" },
> +    [KEY_CODES_KP_SUBTRACT] = { 0x4a, "kp_subtract" },
> +    [KEY_CODES_KP_ADD] = { 0x4e, "kp_add" },
> +    [KEY_CODES_KP_ENTER] = { 0x9c, "kp_enter" },
> +    [KEY_CODES_KP_DECIMAL] = { 0x53, "kp_decimal" },
> +    [KEY_CODES_SYSRQ] = { 0x54, "sysrq" },
> +
> +    [KEY_CODES_KP_0] = { 0x52, "kp_0" },
> +    [KEY_CODES_KP_1] = { 0x4f, "kp_1" },
> +    [KEY_CODES_KP_2] = { 0x50, "kp_2" },
> +    [KEY_CODES_KP_3] = { 0x51, "kp_3" },
> +    [KEY_CODES_KP_4] = { 0x4b, "kp_4" },
> +    [KEY_CODES_KP_5] = { 0x4c, "kp_5" },
> +    [KEY_CODES_KP_6] = { 0x4d, "kp_6" },
> +    [KEY_CODES_KP_7] = { 0x47, "kp_7" },
> +    [KEY_CODES_KP_8] = { 0x48, "kp_8" },
> +    [KEY_CODES_KP_9] = { 0x49, "kp_9" },
> +
> +    [KEY_CODES_LESS] = { 0x56, "less" },
> +
> +    [KEY_CODES_F11] = { 0x57, "f11" },
> +    [KEY_CODES_F12] = { 0x58, "f12" },
> +
> +    [KEY_CODES_PRINT] = { 0xb7, "print" },
> +
> +    [KEY_CODES_HOME] = { 0xc7, "home" },
> +    [KEY_CODES_PGUP] = { 0xc9, "pgup" },
> +    [KEY_CODES_PGDN] = { 0xd1, "pgdn" },
> +    [KEY_CODES_END] = { 0xcf, "end" },
> +
> +    [KEY_CODES_LEFT] = { 0xcb, "left" },
> +    [KEY_CODES_UP] = { 0xc8, "up" },
> +    [KEY_CODES_DOWN] = { 0xd0, "down" },
> +    [KEY_CODES_RIGHT] = { 0xcd, "right" },
> +
> +    [KEY_CODES_INSERT] = { 0xd2, "insert" },
> +    [KEY_CODES_DELETE] = { 0xd3, "delete" },
> +#ifdef NEED_CPU_H
> +#if defined(TARGET_SPARC) && !defined(TARGET_SPARC64)
> +    [KEY_CODES_STOP] = { 0xf0, "stop" },
> +    [KEY_CODES_AGAIN] = { 0xf1, "again" },
> +    [KEY_CODES_PROPS] = { 0xf2, "props" },
> +    [KEY_CODES_UNDO] = { 0xf3, "undo" },
> +    [KEY_CODES_FRONT] = { 0xf4, "front" },
> +    [KEY_CODES_COPY] = { 0xf5, "copy" },
> +    [KEY_CODES_OPEN] = { 0xf6, "open" },
> +    [KEY_CODES_PASTE] = { 0xf7, "paste" },
> +    [KEY_CODES_FIND] = { 0xf8, "find" },
> +    [KEY_CODES_CUT] = { 0xf9, "cut" },
> +    [KEY_CODES_LF] = { 0xfa, "lf" },
> +    [KEY_CODES_HELP] = { 0xfb, "help" },
> +    [KEY_CODES_META_L] = { 0xfc, "meta_l" },
> +    [KEY_CODES_META_R] = { 0xfd, "meta_r" },
> +    [KEY_CODES_COMPOSE] = { 0xfe, "compose" },
> +#endif
> +#endif
> +    [KEY_CODES_MAX] = { 0, NULL },
> +};
> +
>  #endif
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e336251..865eea9 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_send_key,
>      },
>  
>  STEXI
> diff --git a/hmp.c b/hmp.c
> index b9cec1d..cfdc106 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -19,6 +19,7 @@
>  #include "qemu-timer.h"
>  #include "qmp-commands.h"
>  #include "monitor.h"
> +#include "console.h"
>  
>  static void hmp_handle_error(Monitor *mon, Error **errp)
>  {
> @@ -1000,3 +1001,66 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
>      qmp_netdev_del(id, &err);
>      hmp_handle_error(mon, &err);
>  }
> +
> +static int get_key_index(const char *key)
> +{
> +    int i, keycode;
> +    char *endp;
> +
> +    for (i = 0; i < KEY_CODES_MAX; i++)
> +        if (key_defs[i].keycode && !strcmp(key, key_defs[i].name))
> +            return i;

Here you can call do:

  keycode = keycode_from_key(key);
  if (keycode != KEY_CODES_MAX) {
  	return keycode;
  }

> +
> +    if (strstart(key, "0x", NULL)) {
> +        keycode = strtoul(key, &endp, 0);
> +        if (*endp == '\0' && keycode >= 0x01 && keycode <= 0xff)
> +            for (i = 0; i < KEY_CODES_MAX; i++)
> +                if (keycode == key_defs[i].keycode)
> +                    return i;

You can drop that for loop and do instead:

  keycode = keycode_from_code(keycode);


> +    }
> +
> +    return -1;
> +}
> +
> +void hmp_send_key(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 (!strncmp(keyname_buf, "<", 1) && keyname_len == 1) {
> +            pstrcpy(keyname_buf, sizeof(keyname_buf), "less");
> +            keyname_len = 4;
> +        }
> +        keyname_buf[keyname_len] = 0;
> +
> +        keylist = g_malloc0(sizeof(*keylist));
> +        keylist->value = get_key_index(keyname_buf);

get_key_index() can fail.

> +        keylist->next = NULL;
> +
> +        if (tmp)
> +            tmp->next = keylist;
> +        tmp = keylist;
> +        if (!head)
> +            head = keylist;
> +
> +        if (!separator)
> +            break;
> +        keys = separator + 1;
> +    }
> +
> +    qmp_send_key(head, has_hold_time, hold_time, &err);
> +    hmp_handle_error(mon, &err);
> +    qapi_free_KeyCodesList(head);
> +}
> diff --git a/hmp.h b/hmp.h
> index 79d138d..c17769f 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -64,5 +64,6 @@ void hmp_device_del(Monitor *mon, const QDict *qdict);
>  void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
>  void hmp_netdev_add(Monitor *mon, const QDict *qdict);
>  void hmp_netdev_del(Monitor *mon, const QDict *qdict);
> +void hmp_send_key(Monitor *mon, const QDict *qdict);
>  
>  #endif
> diff --git a/monitor.c b/monitor.c
> index 6fa0104..994b85c 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1321,247 +1321,58 @@ static void do_sum(Monitor *mon, const QDict *qdict)
>      monitor_printf(mon, "%05d\n", sum);
>  }
>  
> -typedef struct {
> -    int keycode;
> -    const char *name;
> -} KeyDef;
> -
> -static const KeyDef key_defs[] = {
> -    { 0x2a, "shift" },
> -    { 0x36, "shift_r" },
> -
> -    { 0x38, "alt" },
> -    { 0xb8, "alt_r" },
> -    { 0x64, "altgr" },
> -    { 0xe4, "altgr_r" },
> -    { 0x1d, "ctrl" },
> -    { 0x9d, "ctrl_r" },
> -
> -    { 0xdd, "menu" },
> -
> -    { 0x01, "esc" },
> -
> -    { 0x02, "1" },
> -    { 0x03, "2" },
> -    { 0x04, "3" },
> -    { 0x05, "4" },
> -    { 0x06, "5" },
> -    { 0x07, "6" },
> -    { 0x08, "7" },
> -    { 0x09, "8" },
> -    { 0x0a, "9" },
> -    { 0x0b, "0" },
> -    { 0x0c, "minus" },
> -    { 0x0d, "equal" },
> -    { 0x0e, "backspace" },
> -
> -    { 0x0f, "tab" },
> -    { 0x10, "q" },
> -    { 0x11, "w" },
> -    { 0x12, "e" },
> -    { 0x13, "r" },
> -    { 0x14, "t" },
> -    { 0x15, "y" },
> -    { 0x16, "u" },
> -    { 0x17, "i" },
> -    { 0x18, "o" },
> -    { 0x19, "p" },
> -    { 0x1a, "bracket_left" },
> -    { 0x1b, "bracket_right" },
> -    { 0x1c, "ret" },
> -
> -    { 0x1e, "a" },
> -    { 0x1f, "s" },
> -    { 0x20, "d" },
> -    { 0x21, "f" },
> -    { 0x22, "g" },
> -    { 0x23, "h" },
> -    { 0x24, "j" },
> -    { 0x25, "k" },
> -    { 0x26, "l" },
> -    { 0x27, "semicolon" },
> -    { 0x28, "apostrophe" },
> -    { 0x29, "grave_accent" },
> -
> -    { 0x2b, "backslash" },
> -    { 0x2c, "z" },
> -    { 0x2d, "x" },
> -    { 0x2e, "c" },
> -    { 0x2f, "v" },
> -    { 0x30, "b" },
> -    { 0x31, "n" },
> -    { 0x32, "m" },
> -    { 0x33, "comma" },
> -    { 0x34, "dot" },
> -    { 0x35, "slash" },
> -
> -    { 0x37, "asterisk" },
> -
> -    { 0x39, "spc" },
> -    { 0x3a, "caps_lock" },
> -    { 0x3b, "f1" },
> -    { 0x3c, "f2" },
> -    { 0x3d, "f3" },
> -    { 0x3e, "f4" },
> -    { 0x3f, "f5" },
> -    { 0x40, "f6" },
> -    { 0x41, "f7" },
> -    { 0x42, "f8" },
> -    { 0x43, "f9" },
> -    { 0x44, "f10" },
> -    { 0x45, "num_lock" },
> -    { 0x46, "scroll_lock" },
> -
> -    { 0xb5, "kp_divide" },
> -    { 0x37, "kp_multiply" },
> -    { 0x4a, "kp_subtract" },
> -    { 0x4e, "kp_add" },
> -    { 0x9c, "kp_enter" },
> -    { 0x53, "kp_decimal" },
> -    { 0x54, "sysrq" },
> -
> -    { 0x52, "kp_0" },
> -    { 0x4f, "kp_1" },
> -    { 0x50, "kp_2" },
> -    { 0x51, "kp_3" },
> -    { 0x4b, "kp_4" },
> -    { 0x4c, "kp_5" },
> -    { 0x4d, "kp_6" },
> -    { 0x47, "kp_7" },
> -    { 0x48, "kp_8" },
> -    { 0x49, "kp_9" },
> -
> -    { 0x56, "less" },
> -
> -    { 0x57, "f11" },
> -    { 0x58, "f12" },
> -
> -    { 0xb7, "print" },
> -
> -    { 0xc7, "home" },
> -    { 0xc9, "pgup" },
> -    { 0xd1, "pgdn" },
> -    { 0xcf, "end" },
> -
> -    { 0xcb, "left" },
> -    { 0xc8, "up" },
> -    { 0xd0, "down" },
> -    { 0xcd, "right" },
> -
> -    { 0xd2, "insert" },
> -    { 0xd3, "delete" },
> -#if defined(TARGET_SPARC) && !defined(TARGET_SPARC64)
> -    { 0xf0, "stop" },
> -    { 0xf1, "again" },
> -    { 0xf2, "props" },
> -    { 0xf3, "undo" },
> -    { 0xf4, "front" },
> -    { 0xf5, "copy" },
> -    { 0xf6, "open" },
> -    { 0xf7, "paste" },
> -    { 0xf8, "find" },
> -    { 0xf9, "cut" },
> -    { 0xfa, "lf" },
> -    { 0xfb, "help" },
> -    { 0xfc, "meta_l" },
> -    { 0xfd, "meta_r" },
> -    { 0xfe, "compose" },
> -#endif
> -    { 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;
> +static KeyCodesList *keycodes;
>  static QEMUTimer *key_timer;
>  
>  static void release_keys(void *opaque)
>  {
>      int keycode;
> +    KeyCodesList *p;
> +
> +    for (p = keycodes; p != NULL; p = p->next) {
> +        if (p->value > KEY_CODES_MAX) {

This check is not needed, as far as I can understand only qmp_send_key() can put
keys in the keycodes list and qmp_send_key() does this check already.

> +            keycodes = NULL;
> +            break;
> +        }
>  
> -    while (nb_pending_keycodes > 0) {
> -        nb_pending_keycodes--;
> -        keycode = keycodes[nb_pending_keycodes];
> +        keycode = key_defs[p->value].keycode;
>          if (keycode & 0x80)
>              kbd_put_keycode(0xe0);
>          kbd_put_keycode(keycode | 0x80);
>      }

Please set keycodes to NULL here. Actually, you'll have to free it first,
because keycodes has to be duplicated (see below).

>  }
>  
> -static void do_sendkey(Monitor *mon, const QDict *qdict)
> +void qmp_send_key(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;
> +    char value[5];
> +    KeyCodesList *p;

Let's initialize key_timer here, like this:

if (!key_timer) {
    key_timer = qemu_new_timer_ns(vm_clock, release_keys, NULL);
}

Then drop the initialization done in monitor_init(). This way we untangle
qmp_send_key() from the monitor.

Also, please, move qmp_send_key(), release_keys, etc to input.c as I said
above.

>  
> -    if (nb_pending_keycodes > 0) {
> +    if (keycodes != NULL) {
>          qemu_del_timer(key_timer);
>          release_keys(NULL);
>      }
>      if (!has_hold_time)
>          hold_time = 100;

Please, add { } around the if body above.

> -    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 (!strncmp(keyname_buf, "<", 1) && keyname_len == 1) {
> -                pstrcpy(keyname_buf, sizeof(keyname_buf), "less");
> -                keyname_len = 4;
> -            }
> +    keycodes = keys;

It's better to this assignment after the for loop below. Actually, you have to
duplicate the key list because the qapi code will free it as soon as
qmp_send_key() returns, but it will be used later in the timer handler.

>  
> -            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;
> +    for (p = keycodes; p != NULL; p = p->next) {
> +        if (p->value > KEY_CODES_MAX) {

You should test against >=.

> +            sprintf(value, "%d", p->value);
> +            error_set(errp, QERR_INVALID_PARAMETER, value);
> +            return;
>          }
> -        if (!separator)
> -            break;
> -        keys = separator + 1;
> -    }
> -    nb_pending_keycodes = i;
> -    /* key down events */
> -    for (i = 0; i < nb_pending_keycodes; i++) {
> -        keycode = keycodes[i];
> +
> +        /* key down events */
> +        keycode = key_defs[p->value].keycode;
>          if (keycode & 0x80)
>              kbd_put_keycode(0xe0);
>          kbd_put_keycode(keycode & 0x7f);
>      }
> +
>      /* delayed key up events */
>      qemu_mod_timer(key_timer, qemu_get_clock_ns(vm_clock) +
>                     muldiv64(get_ticks_per_sec(), hold_time, 1000));
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 3b6e346..08e51c6 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1862,3 +1862,49 @@
>  # Since: 0.14.0
>  ##
>  { 'command': 'netdev_del', 'data': {'id': 'str'} }
> +
> +##
> +# @KeyCodes:

s/KeyCodes/KeyCode

> +#
> +# An enumeration of key name.
> +#
> +# This is used by the send-key command.
> +#
> +# Since: 1.2
> +##
> +{ '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' ] }
> +
> +##
> +# @send-key:
> +#
> +# Send keys to guest.
> +#
> +# @keys: key sequence. 'keys' could be the name of the key. Use a JSON array to
> +#        press several keys simultaneously.

s/could be/is

> +#
> +# @hold-time: #optional time to delay key up events, milliseconds. Defaults
> +#             to 200

The default is 100.

> +#
> +# Returns: Nothing on success
> +#          If key is unknown or redundant, InvalidParameter
> +#
> +# Since: 1.2
> +#
> +##
> +{ 'command': 'send-key',
> +  'data': { 'keys': ['KeyCodes'], '*hold-time': 'int' } }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 2e1a38e..12f6b76 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -335,6 +335,34 @@ Example:
>  EQMP
>  
>      {
> +        .name       = "send-key",
> +        .args_type  = "keys:O,hold-time:i?",
> +        .mhandler.cmd_new = qmp_marshal_input_send_key,
> +    },
> +
> +SQMP
> +send-key
> +----------
> +
> +Send keys to VM.
> +
> +Arguments:
> +
> +keys array:
> +    - "key": key sequence (a json-array of key enum values)
> +
> +- hold-time: time to delay key up events, milliseconds. Defaults to 200
> +             (json-int, optional)
> +
> +Example:
> +
> +-> { "execute": "send-key",
> +     "arguments": { 'keys': [ 'ctrl', 'alt', 'delete' ] } }
> +<- { "return": {} }
> +
> +EQMP
> +
> +    {
>          .name       = "cpu",
>          .args_type  = "index:i",
>          .mhandler.cmd_new = qmp_marshal_input_cpu,

Re: [Qemu-devel] [PATCH v4 5/6] qapi: convert sendkey

[Amos Kong]

On 12/07/12 23:09, Luiz Capitulino wrote:
>

Hi Luiz,

> On Thu,  5 Jul 2012 20:48:44 +0800
> Amos Kong<akong@redhat.com>  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 console.h is the mapping of key name to keycode,
>> Keys' index in the enmu and key_defs[] is same.
>>
>> 'send-key' of QMP doesn't support key in hexadecimal format.
>>
>> Signed-off-by: Amos Kong <akong@redhat.com>
>> ---
>>   console.h        |  152 ++++++++++++++++++++++++++++++++++
>>   hmp-commands.hx  |    2 +-
>>   hmp.c            |   64 +++++++++++++++
>>   hmp.h            |    1 +
>>   monitor.c        |  239 ++++++------------------------------------------------
>>   qapi-schema.json |   46 +++++++++++
>>   qmp-commands.hx  |   28 +++++++
>>   7 files changed, 317 insertions(+), 215 deletions(-)
>>
>> diff --git a/console.h b/console.h
>> index 4334db5..e1b0c45 100644
>> --- a/console.h
>> +++ b/console.h
>> @@ -6,6 +6,7 @@
>>   #include "notify.h"
>>   #include "monitor.h"
>>   #include "trace.h"
>> +#include "qapi-types.h"
>>
>>   /* keyboard/mouse support */
>>
>> @@ -397,4 +398,155 @@ static inline int vnc_display_pw_expire(DisplayState *ds, time_t expires)
>>   /* curses.c */
>>   void curses_display_init(DisplayState *ds, int full_screen);
>>
>> +typedef struct {
>> +    int keycode;
>> +    const char *name;
>
> I don't think we need 'name', as key names are also provided by KeyCodes_lookup[].
> See more on this below.


Yes, I tried to define key_defs[] to a int array, and get keyname from 
KeyCodes_loopup[],
it works.

const int key_defs[] = {
     [KEY_CODES_SHIFT] = 0x2a,
     ....


>> +} KeyDef;
>> +
>> +static const KeyDef key_defs[] = {
>
> We can't have an array defined in a header file because it will be defined in
> each .c file that includes it.
>
> Please, define it in input.c (along with qmp_send_key())

Ok.

> and write the following public functions:
>
>   o KeyCode keycode_from_key(const char *key);
>   o KeyCode keycode_from_code(int code);


void qmp_send_key(KeyCodesList *keys, bool has_hold_time, int64_t 
hold_time, ...)
                     ^
                     \_ when we use qmp, a key list will be passed, the 
values are the index
                        in enum KeyCodes. not the real KeyCode.

                     { 'enum': 'KeyCodes',
                       'data': [ 'shift', 'shift_r', 'al...

So we need to get this kind of 'index' in hmp_send_key() and pass to 
qmp_send_key().
then convert this 'index' to keycode in qmp_send_key()

I didn't find a way to define a non-serial enum.

eg: (then int qmp_marshal_input_send_key() would pass real keycode to 
qmp_send_key())
{ 'enum': 'KeyCodes',
   'data': [ 'shift' = 0x2a, 'shift_r' = 0x36, 'alt' = 0x38, ...


If we still pass 'index' to qmp_send_key as patch V4.

extern int index_from_key(const char *key);   -> it's used in hmp_send_key()
extern int index_from_keycode(int code);      -> it's used in hmp_send_key()
extern char *key_from_keycode(int idx);       -> it's used in 
monitor_find_completion()
extern int keycode_from_key(const char *key); -> it's used in qmp_send_key()


> and then use these functions where using key_defs would be necessary. Also,
> note that keycode_from_key() can use KeyCodes_lookup[] instead of key_defs (this
> way we can drop 'name' from KeyDef).

....

>> +#endif
>> +#endif
>> +    [KEY_CODES_MAX] = { 0, NULL },
>> +};
>> +
>>   #endif
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index e336251..865eea9 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_send_key,
>>       },
>>
>>   STEXI
>> diff --git a/hmp.c b/hmp.c
>> index b9cec1d..cfdc106 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -19,6 +19,7 @@
>>   #include "qemu-timer.h"
>>   #include "qmp-commands.h"
>>   #include "monitor.h"
>> +#include "console.h"
>>
>>   static void hmp_handle_error(Monitor *mon, Error **errp)
>>   {
>> @@ -1000,3 +1001,66 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
>>       qmp_netdev_del(id,&err);
>>       hmp_handle_error(mon,&err);
>>   }
>> +
>> +static int get_key_index(const char *key)
>> +{
>> +    int i, keycode;
>> +    char *endp;
>> +
>> +    for (i = 0; i<  KEY_CODES_MAX; i++)
>> +        if (key_defs[i].keycode&&  !strcmp(key, key_defs[i].name))
>> +            return i;
>
> Here you can call do:
>
>    keycode = keycode_from_key(key);
>    if (keycode != KEY_CODES_MAX) {
>    	return keycode;
>    }
>
>> +
>> +    if (strstart(key, "0x", NULL)) {
>> +        keycode = strtoul(key,&endp, 0);
>> +        if (*endp == '\0'&&  keycode>= 0x01&&  keycode<= 0xff)
>> +            for (i = 0; i<  KEY_CODES_MAX; i++)
>> +                if (keycode == key_defs[i].keycode)
>> +                    return i;
>
> You can drop that for loop and do instead:
>
>    keycode = keycode_from_code(keycode);
>
>
>> +    }
>> +
>> +    return -1;
>> +}
>> +
>> +void hmp_send_key(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 (!strncmp(keyname_buf, "<", 1)&&  keyname_len == 1) {
>> +            pstrcpy(keyname_buf, sizeof(keyname_buf), "less");
>> +            keyname_len = 4;
>> +        }
>> +        keyname_buf[keyname_len] = 0;
>> +
>> +        keylist = g_malloc0(sizeof(*keylist));
>> +        keylist->value = get_key_index(keyname_buf);
>
> get_key_index() can fail.

Ok, I would check it.

>> +        keylist->next = NULL;
>> +
>> +        if (tmp)
>> +            tmp->next = keylist;
>> +        tmp = keylist;
>> +        if (!head)
>> +            head = keylist;
>> +
>> +        if (!separator)
>> +            break;
>> +        keys = separator + 1;
>> +    }

...

>> -    { 0xfe, "compose" },
>> -#endif
>> -    { 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;
>> +static KeyCodesList *keycodes;
>>   static QEMUTimer *key_timer;
>>
>>   static void release_keys(void *opaque)
>>   {
>>       int keycode;
>> +    KeyCodesList *p;
>> +
>> +    for (p = keycodes; p != NULL; p = p->next) {
>> +        if (p->value>  KEY_CODES_MAX) {
>
> This check is not needed, as far as I can understand only qmp_send_key() can put
> keys in the keycodes list and qmp_send_key() does this check already.

If we find one invalid 'value', other keys in the list will be ignored.
so we don't need to release them here.


>> +            keycodes = NULL;
>> +            break;
>> +        }
>>
>> -    while (nb_pending_keycodes>  0) {
>> -        nb_pending_keycodes--;
>> -        keycode = keycodes[nb_pending_keycodes];
>> +        keycode = key_defs[p->value].keycode;
>>           if (keycode&  0x80)
>>               kbd_put_keycode(0xe0);
>>           kbd_put_keycode(keycode | 0x80);
>>       }
>
> Please set keycodes to NULL here. Actually, you'll have to free it first,
> because keycodes has to be duplicated (see below).
>
>>   }
>>
>> -static void do_sendkey(Monitor *mon, const QDict *qdict)
>> +void qmp_send_key(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;
>> +    char value[5];
>> +    KeyCodesList *p;
>
> Let's initialize key_timer here, like this:
>
> if (!key_timer) {
>      key_timer = qemu_new_timer_ns(vm_clock, release_keys, NULL);
> }
>
> Then drop the initialization done in monitor_init(). This way we untangle
> qmp_send_key() from the monitor.
>
> Also, please, move qmp_send_key(), release_keys, etc to input.c as I said
> above.

Ok.

>> -    if (nb_pending_keycodes>  0) {
>> +    if (keycodes != NULL) {
>>           qemu_del_timer(key_timer);
>>           release_keys(NULL);
>>       }
>>       if (!has_hold_time)
>>           hold_time = 100;
>
> Please, add { } around the if body above.
>
>> -    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 (!strncmp(keyname_buf, "<", 1)&&  keyname_len == 1) {
>> -                pstrcpy(keyname_buf, sizeof(keyname_buf), "less");
>> -                keyname_len = 4;
>> -            }
>> +    keycodes = keys;
>
> It's better to this assignment after the for loop below. Actually, you have to
> duplicate the key list because the qapi code will free it as soon as
> qmp_send_key() returns, but it will be used later in the timer handler.
>
>>
>> -            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;
>> +    for (p = keycodes; p != NULL; p = p->next) {
>> +        if (p->value>  KEY_CODES_MAX) {
>
> You should test against>=.
>
>> +            sprintf(value, "%d", p->value);
>> +            error_set(errp, QERR_INVALID_PARAMETER, value);

^^ If an invalid key is found, the other keys will be ignored.


Will fix other issues you mentioned, thanks for your review.

-- 
			Amos.

Re: [Qemu-devel] [PATCH v4 5/6] qapi: convert sendkey

[Luiz Capitulino]

On Wed, 18 Jul 2012 20:56:54 +0800
Amos Kong <akong@redhat.com> wrote:

> >> +} KeyDef;
> >> +
> >> +static const KeyDef key_defs[] = {
> >
> > We can't have an array defined in a header file because it will be defined in
> > each .c file that includes it.
> >
> > Please, define it in input.c (along with qmp_send_key())
> 
> Ok.
> 
> > and write the following public functions:
> >
> >   o KeyCode keycode_from_key(const char *key);
> >   o KeyCode keycode_from_code(int code);
> 
> 
> void qmp_send_key(KeyCodesList *keys, bool has_hold_time, int64_t 
> hold_time, ...)
>                      ^
>                      \_ when we use qmp, a key list will be passed, the 
> values are the index
>                         in enum KeyCodes. not the real KeyCode.

Right.

> 
>                      { 'enum': 'KeyCodes',
>                        'data': [ 'shift', 'shift_r', 'al...
> 
> So we need to get this kind of 'index' in hmp_send_key() and pass to 
> qmp_send_key().

Yes, that's what keycode_from_key() would do, something like this:

KeyCode keycode_from_key(const char *key)
{
    int i;

    for (i = 0; i < KEY_CODES_MAX; i++) {
        if (!strcmp(key, KeyCode_lookup[i])) {
            return i;
        }
    }

    return KEY_CODE_MAX;
}

Note that it returns the KeyCode index, and should be defined in input.c.

> then convert this 'index' to keycode in qmp_send_key()

Exactly, qmp_send_key() can access key_defs[] to get the keycode from the
index.

> 
> I didn't find a way to define a non-serial enum.

I'm not sure I follow you here, I think that what I suggested above will work.

> 
> eg: (then int qmp_marshal_input_send_key() would pass real keycode to 
> qmp_send_key())
> { 'enum': 'KeyCodes',
>    'data': [ 'shift' = 0x2a, 'shift_r' = 0x36, 'alt' = 0x38, ...
> 
> 
> If we still pass 'index' to qmp_send_key as patch V4.
> 
> extern int index_from_key(const char *key);   -> it's used in hmp_send_key()
> extern int index_from_keycode(int code);      -> it's used in hmp_send_key()
> extern char *key_from_keycode(int idx);       -> it's used in 
> monitor_find_completion()
> extern int keycode_from_key(const char *key); -> it's used in qmp_send_key()
> 
> 
> > and then use these functions where using key_defs would be necessary. Also,
> > note that keycode_from_key() can use KeyCodes_lookup[] instead of key_defs (this
> > way we can drop 'name' from KeyDef).
> 
> ....
> 
> >> +#endif
> >> +#endif
> >> +    [KEY_CODES_MAX] = { 0, NULL },
> >> +};
> >> +
> >>   #endif
> >> diff --git a/hmp-commands.hx b/hmp-commands.hx
> >> index e336251..865eea9 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_send_key,
> >>       },
> >>
> >>   STEXI
> >> diff --git a/hmp.c b/hmp.c
> >> index b9cec1d..cfdc106 100644
> >> --- a/hmp.c
> >> +++ b/hmp.c
> >> @@ -19,6 +19,7 @@
> >>   #include "qemu-timer.h"
> >>   #include "qmp-commands.h"
> >>   #include "monitor.h"
> >> +#include "console.h"
> >>
> >>   static void hmp_handle_error(Monitor *mon, Error **errp)
> >>   {
> >> @@ -1000,3 +1001,66 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
> >>       qmp_netdev_del(id,&err);
> >>       hmp_handle_error(mon,&err);
> >>   }
> >> +
> >> +static int get_key_index(const char *key)
> >> +{
> >> +    int i, keycode;
> >> +    char *endp;
> >> +
> >> +    for (i = 0; i<  KEY_CODES_MAX; i++)
> >> +        if (key_defs[i].keycode&&  !strcmp(key, key_defs[i].name))
> >> +            return i;
> >
> > Here you can call do:
> >
> >    keycode = keycode_from_key(key);
> >    if (keycode != KEY_CODES_MAX) {
> >    	return keycode;
> >    }
> >
> >> +
> >> +    if (strstart(key, "0x", NULL)) {
> >> +        keycode = strtoul(key,&endp, 0);
> >> +        if (*endp == '\0'&&  keycode>= 0x01&&  keycode<= 0xff)
> >> +            for (i = 0; i<  KEY_CODES_MAX; i++)
> >> +                if (keycode == key_defs[i].keycode)
> >> +                    return i;
> >
> > You can drop that for loop and do instead:
> >
> >    keycode = keycode_from_code(keycode);
> >
> >
> >> +    }
> >> +
> >> +    return -1;
> >> +}
> >> +
> >> +void hmp_send_key(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 (!strncmp(keyname_buf, "<", 1)&&  keyname_len == 1) {
> >> +            pstrcpy(keyname_buf, sizeof(keyname_buf), "less");
> >> +            keyname_len = 4;
> >> +        }
> >> +        keyname_buf[keyname_len] = 0;
> >> +
> >> +        keylist = g_malloc0(sizeof(*keylist));
> >> +        keylist->value = get_key_index(keyname_buf);
> >
> > get_key_index() can fail.
> 
> Ok, I would check it.
> 
> >> +        keylist->next = NULL;
> >> +
> >> +        if (tmp)
> >> +            tmp->next = keylist;
> >> +        tmp = keylist;
> >> +        if (!head)
> >> +            head = keylist;
> >> +
> >> +        if (!separator)
> >> +            break;
> >> +        keys = separator + 1;
> >> +    }
> 
> ...
> 
> >> -    { 0xfe, "compose" },
> >> -#endif
> >> -    { 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;
> >> +static KeyCodesList *keycodes;
> >>   static QEMUTimer *key_timer;
> >>
> >>   static void release_keys(void *opaque)
> >>   {
> >>       int keycode;
> >> +    KeyCodesList *p;
> >> +
> >> +    for (p = keycodes; p != NULL; p = p->next) {
> >> +        if (p->value>  KEY_CODES_MAX) {
> >
> > This check is not needed, as far as I can understand only qmp_send_key() can put
> > keys in the keycodes list and qmp_send_key() does this check already.
> 
> If we find one invalid 'value', other keys in the list will be ignored.
> so we don't need to release them here.

That should be done in qmp_send_key(), only valid keys should be passed to
release_keys().

> 
> 
> >> +            keycodes = NULL;
> >> +            break;
> >> +        }
> >>
> >> -    while (nb_pending_keycodes>  0) {
> >> -        nb_pending_keycodes--;
> >> -        keycode = keycodes[nb_pending_keycodes];
> >> +        keycode = key_defs[p->value].keycode;
> >>           if (keycode&  0x80)
> >>               kbd_put_keycode(0xe0);
> >>           kbd_put_keycode(keycode | 0x80);
> >>       }
> >
> > Please set keycodes to NULL here. Actually, you'll have to free it first,
> > because keycodes has to be duplicated (see below).
> >
> >>   }
> >>
> >> -static void do_sendkey(Monitor *mon, const QDict *qdict)
> >> +void qmp_send_key(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;
> >> +    char value[5];
> >> +    KeyCodesList *p;
> >
> > Let's initialize key_timer here, like this:
> >
> > if (!key_timer) {
> >      key_timer = qemu_new_timer_ns(vm_clock, release_keys, NULL);
> > }
> >
> > Then drop the initialization done in monitor_init(). This way we untangle
> > qmp_send_key() from the monitor.
> >
> > Also, please, move qmp_send_key(), release_keys, etc to input.c as I said
> > above.
> 
> Ok.
> 
> >> -    if (nb_pending_keycodes>  0) {
> >> +    if (keycodes != NULL) {
> >>           qemu_del_timer(key_timer);
> >>           release_keys(NULL);
> >>       }
> >>       if (!has_hold_time)
> >>           hold_time = 100;
> >
> > Please, add { } around the if body above.
> >
> >> -    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 (!strncmp(keyname_buf, "<", 1)&&  keyname_len == 1) {
> >> -                pstrcpy(keyname_buf, sizeof(keyname_buf), "less");
> >> -                keyname_len = 4;
> >> -            }
> >> +    keycodes = keys;
> >
> > It's better to this assignment after the for loop below. Actually, you have to
> > duplicate the key list because the qapi code will free it as soon as
> > qmp_send_key() returns, but it will be used later in the timer handler.
> >
> >>
> >> -            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;
> >> +    for (p = keycodes; p != NULL; p = p->next) {
> >> +        if (p->value>  KEY_CODES_MAX) {
> >
> > You should test against>=.
> >
> >> +            sprintf(value, "%d", p->value);
> >> +            error_set(errp, QERR_INVALID_PARAMETER, value);
> 
> ^^ If an invalid key is found, the other keys will be ignored.

Well, that's the behavior I'd expect. Maybe we should loop twice. First
we check everything is ok and then we trigger the key down events.

Now, the current code doesn't do this and I'm not sure if we would
break compatibility... I'll let you choose what you think is best.

> 
> 
> Will fix other issues you mentioned, thanks for your review.
> 

Re: [Qemu-devel] [PATCH v4 5/6] qapi: convert sendkey

[Amos Kong]

----- Original Message -----
> On Wed, 18 Jul 2012 20:56:54 +0800
> Amos Kong <akong@redhat.com> wrote:
> 
> > >> +} KeyDef;
> > >> +
> > >> +static const KeyDef key_defs[] = {
> > >
> > > We can't have an array defined in a header file because it will
> > > be defined in
> > > each .c file that includes it.
> > >
> > > Please, define it in input.c (along with qmp_send_key())
> > 
> > Ok.
> > 
> > > and write the following public functions:
> > >
> > >   o KeyCode keycode_from_key(const char *key);
> > >   o KeyCode keycode_from_code(int code);
> > 
> > 
> > void qmp_send_key(KeyCodesList *keys, bool has_hold_time, int64_t
> > hold_time, ...)
> >                      ^
> >                      \_ when we use qmp, a key list will be passed,
> >                      the
> > values are the index
> >                         in enum KeyCodes. not the real KeyCode.
> 
> Right.
> 
> > 
> >                      { 'enum': 'KeyCodes',
> >                        'data': [ 'shift', 'shift_r', 'al...
> > 
> > So we need to get this kind of 'index' in hmp_send_key() and pass
> > to
> > qmp_send_key().
> 
> Yes, that's what keycode_from_key() would do, something like this:
> 
> KeyCode keycode_from_key(const char *key)
> {
>     int i;
> 
>     for (i = 0; i < KEY_CODES_MAX; i++) {
>         if (!strcmp(key, KeyCode_lookup[i])) {
>             return i;
>         }
>     }
> 
>     return KEY_CODE_MAX;
> }
> 
> Note that it returns the KeyCode index, and should be defined in
> input.c.

Sure :)
 
> > then convert this 'index' to keycode in qmp_send_key()
> 
> Exactly, qmp_send_key() can access key_defs[] to get the keycode from
> the
> index.
> 
> > 
> > I didn't find a way to define a non-serial enum.
> 
> I'm not sure I follow you here, I think that what I suggested above
> will work.

So I would continually pass 'index' to qmp_send_key().
I already implemented those in localhost, they all works.
Will fix other issues and post v5 later, thanks.


> > eg: (then int qmp_marshal_input_send_key() would pass real keycode
> > to
> > qmp_send_key())
> > { 'enum': 'KeyCodes',
> >    'data': [ 'shift' = 0x2a, 'shift_r' = 0x36, 'alt' = 0x38, ...
> > 
> > 
> > If we still pass 'index' to qmp_send_key as patch V4.
> > 
> > extern int index_from_key(const char *key);   -> it's used in
> > hmp_send_key()
> > extern int index_from_keycode(int code);      -> it's used in
> > hmp_send_key()
> > extern char *key_from_keycode(int idx);       -> it's used in
> > monitor_find_completion()
> > extern int keycode_from_key(const char *key); -> it's used in
> > qmp_send_key()
> > 
> > 
> > > and then use these functions where using key_defs would be
> > > necessary. Also,
> > > note that keycode_from_key() can use KeyCodes_lookup[] instead of
> > > key_defs (this
> > > way we can drop 'name' from KeyDef).
> > 
> > ....
> > 
> > >> +#endif
> > >> +#endif
> > >> +    [KEY_CODES_MAX] = { 0, NULL },
> > >> +};
> > >> +
> > >>   #endif
> > >> diff --git a/hmp-commands.hx b/hmp-commands.hx
> > >> index e336251..865eea9 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_send_key,
> > >>       },
> > >>
> > >>   STEXI
> > >> diff --git a/hmp.c b/hmp.c
> > >> index b9cec1d..cfdc106 100644
> > >> --- a/hmp.c
> > >> +++ b/hmp.c
> > >> @@ -19,6 +19,7 @@
> > >>   #include "qemu-timer.h"
> > >>   #include "qmp-commands.h"
> > >>   #include "monitor.h"
> > >> +#include "console.h"
> > >>
> > >>   static void hmp_handle_error(Monitor *mon, Error **errp)
> > >>   {
> > >> @@ -1000,3 +1001,66 @@ void hmp_netdev_del(Monitor *mon, const
> > >> QDict *qdict)
> > >>       qmp_netdev_del(id,&err);
> > >>       hmp_handle_error(mon,&err);
> > >>   }
> > >> +
> > >> +static int get_key_index(const char *key)
> > >> +{
> > >> +    int i, keycode;
> > >> +    char *endp;
> > >> +
> > >> +    for (i = 0; i<  KEY_CODES_MAX; i++)
> > >> +        if (key_defs[i].keycode&&  !strcmp(key,
> > >> key_defs[i].name))
> > >> +            return i;
> > >
> > > Here you can call do:
> > >
> > >    keycode = keycode_from_key(key);
> > >    if (keycode != KEY_CODES_MAX) {
> > >    	return keycode;
> > >    }
> > >
> > >> +
> > >> +    if (strstart(key, "0x", NULL)) {
> > >> +        keycode = strtoul(key,&endp, 0);
> > >> +        if (*endp == '\0'&&  keycode>= 0x01&&  keycode<= 0xff)
> > >> +            for (i = 0; i<  KEY_CODES_MAX; i++)
> > >> +                if (keycode == key_defs[i].keycode)
> > >> +                    return i;
> > >
> > > You can drop that for loop and do instead:
> > >
> > >    keycode = keycode_from_code(keycode);
> > >
> > >
> > >> +    }
> > >> +
> > >> +    return -1;
> > >> +}
> > >> +
> > >> +void hmp_send_key(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 (!strncmp(keyname_buf, "<", 1)&&  keyname_len == 1)
> > >> {
> > >> +            pstrcpy(keyname_buf, sizeof(keyname_buf), "less");
> > >> +            keyname_len = 4;
> > >> +        }
> > >> +        keyname_buf[keyname_len] = 0;
> > >> +
> > >> +        keylist = g_malloc0(sizeof(*keylist));
> > >> +        keylist->value = get_key_index(keyname_buf);
> > >
> > > get_key_index() can fail.
> > 
> > Ok, I would check it.
> > 
> > >> +        keylist->next = NULL;
> > >> +
> > >> +        if (tmp)
> > >> +            tmp->next = keylist;
> > >> +        tmp = keylist;
> > >> +        if (!head)
> > >> +            head = keylist;
> > >> +
> > >> +        if (!separator)
> > >> +            break;
> > >> +        keys = separator + 1;
> > >> +    }
> > 
> > ...
> > 
> > >> -    { 0xfe, "compose" },
> > >> -#endif
> > >> -    { 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;
> > >> +static KeyCodesList *keycodes;
> > >>   static QEMUTimer *key_timer;
> > >>
> > >>   static void release_keys(void *opaque)
> > >>   {
> > >>       int keycode;
> > >> +    KeyCodesList *p;
> > >> +
> > >> +    for (p = keycodes; p != NULL; p = p->next) {
> > >> +        if (p->value>  KEY_CODES_MAX) {
> > >
> > > This check is not needed, as far as I can understand only
> > > qmp_send_key() can put
> > > keys in the keycodes list and qmp_send_key() does this check
> > > already.
> > 
> > If we find one invalid 'value', other keys in the list will be
> > ignored.
> > so we don't need to release them here.
> 
> That should be done in qmp_send_key(), only valid keys should be
> passed to
> release_keys().

Ok. will use two loops.
 
> > 
> > 
> > >> +            keycodes = NULL;
> > >> +            break;
> > >> +        }
> > >>
> > >> -    while (nb_pending_keycodes>  0) {
> > >> -        nb_pending_keycodes--;
> > >> -        keycode = keycodes[nb_pending_keycodes];
> > >> +        keycode = key_defs[p->value].keycode;
> > >>           if (keycode&  0x80)
> > >>               kbd_put_keycode(0xe0);
> > >>           kbd_put_keycode(keycode | 0x80);
> > >>       }
> > >
> > > Please set keycodes to NULL here. Actually, you'll have to free
> > > it first,
> > > because keycodes has to be duplicated (see below).
> > >
> > >>   }
> > >>
> > >> -static void do_sendkey(Monitor *mon, const QDict *qdict)
> > >> +void qmp_send_key(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;
> > >> +    char value[5];
> > >> +    KeyCodesList *p;
> > >
> > > Let's initialize key_timer here, like this:
> > >
> > > if (!key_timer) {
> > >      key_timer = qemu_new_timer_ns(vm_clock, release_keys, NULL);
> > > }
> > >
> > > Then drop the initialization done in monitor_init(). This way we
> > > untangle
> > > qmp_send_key() from the monitor.
> > >
> > > Also, please, move qmp_send_key(), release_keys, etc to input.c
> > > as I said
> > > above.
> > 
> > Ok.
> > 
> > >> -    if (nb_pending_keycodes>  0) {
> > >> +    if (keycodes != NULL) {
> > >>           qemu_del_timer(key_timer);
> > >>           release_keys(NULL);
> > >>       }
> > >>       if (!has_hold_time)
> > >>           hold_time = 100;
> > >
> > > Please, add { } around the if body above.
> > >
> > >> -    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 (!strncmp(keyname_buf, "<", 1)&&  keyname_len ==
> > >> 1) {
> > >> -                pstrcpy(keyname_buf, sizeof(keyname_buf),
> > >> "less");
> > >> -                keyname_len = 4;
> > >> -            }
> > >> +    keycodes = keys;
> > >
> > > It's better to this assignment after the for loop below.
> > > Actually, you have to
> > > duplicate the key list because the qapi code will free it as soon
> > > as
> > > qmp_send_key() returns, but it will be used later in the timer
> > > handler.
> > >
> > >>
> > >> -            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;
> > >> +    for (p = keycodes; p != NULL; p = p->next) {
> > >> +        if (p->value>  KEY_CODES_MAX) {
> > >
> > > You should test against>=.
> > >
> > >> +            sprintf(value, "%d", p->value);
> > >> +            error_set(errp, QERR_INVALID_PARAMETER, value);
> > 
> > ^^ If an invalid key is found, the other keys will be ignored.
> 
> Well, that's the behavior I'd expect. Maybe we should loop twice.
> First
> we check everything is ok and then we trigger the key down events.
> 
> Now, the current code doesn't do this and I'm not sure if we would
> break compatibility... I'll let you choose what you think is best.
> 
> > 
> > 
> > Will fix other issues you mentioned, thanks for your review.
> > 
> 
> 
> 

Re: [Qemu-devel] [PATCH v4 5/6] qapi: convert sendkey

[Amos Kong]

----- Original Message -----
> On Thu,  5 Jul 2012 20:48:44 +0800
> Amos Kong <akong@redhat.com> 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 console.h is the mapping of key name to keycode,
> > Keys' index in the enmu and key_defs[] is same.
> > 
> > 'send-key' of QMP doesn't support key in hexadecimal format.
> > 
> > Signed-off-by: Amos Kong <akong@redhat.com>
> > ---
> >  console.h        |  152 ++++++++++++++++++++++++++++++++++
> >  hmp-commands.hx  |    2 +-
> >  hmp.c            |   64 +++++++++++++++
> >  hmp.h            |    1 +
> >  monitor.c        |  239
> >  ++++++------------------------------------------------
> >  qapi-schema.json |   46 +++++++++++
> >  qmp-commands.hx  |   28 +++++++
> >  7 files changed, 317 insertions(+), 215 deletions(-)

...

> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 3b6e346..08e51c6 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -1862,3 +1862,49 @@
> >  # Since: 0.14.0
> >  ##
> >  { 'command': 'netdev_del', 'data': {'id': 'str'} }
> > +
> > +##
> > +# @KeyCodes:
> 
> s/KeyCodes/KeyCode


'KeyCode' is not an available variable name.

| ./qapi-types.h:471: error: conflicting types for ‘KeyCode’
| /usr/include/X11/X.h:108: note: previous declaration of ‘KeyCode’ was here

How about 'CodeOfKey'?

> > +#
> > +# An enumeration of key name.
> > +#
> > +# This is used by the send-key command.
> > +#
> > +# Since: 1.2
> > +##
> > +{ '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' ] }

Re: [Qemu-devel] [PATCH v4 5/6] qapi: convert sendkey

[Luiz Capitulino]

On Wed, 25 Jul 2012 01:55:14 -0400 (EDT)
Amos Kong <akong@redhat.com> wrote:

> 
> 
> ----- Original Message -----
> > On Thu,  5 Jul 2012 20:48:44 +0800
> > Amos Kong <akong@redhat.com> 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 console.h is the mapping of key name to keycode,
> > > Keys' index in the enmu and key_defs[] is same.
> > > 
> > > 'send-key' of QMP doesn't support key in hexadecimal format.
> > > 
> > > Signed-off-by: Amos Kong <akong@redhat.com>
> > > ---
> > >  console.h        |  152 ++++++++++++++++++++++++++++++++++
> > >  hmp-commands.hx  |    2 +-
> > >  hmp.c            |   64 +++++++++++++++
> > >  hmp.h            |    1 +
> > >  monitor.c        |  239
> > >  ++++++------------------------------------------------
> > >  qapi-schema.json |   46 +++++++++++
> > >  qmp-commands.hx  |   28 +++++++
> > >  7 files changed, 317 insertions(+), 215 deletions(-)
> 
> ...
> 
> > > diff --git a/qapi-schema.json b/qapi-schema.json
> > > index 3b6e346..08e51c6 100644
> > > --- a/qapi-schema.json
> > > +++ b/qapi-schema.json
> > > @@ -1862,3 +1862,49 @@
> > >  # Since: 0.14.0
> > >  ##
> > >  { 'command': 'netdev_del', 'data': {'id': 'str'} }
> > > +
> > > +##
> > > +# @KeyCodes:
> > 
> > s/KeyCodes/KeyCode
> 
> 
> 'KeyCode' is not an available variable name.
> 
> | ./qapi-types.h:471: error: conflicting types for ‘KeyCode’
> | /usr/include/X11/X.h:108: note: previous declaration of ‘KeyCode’ was here
> 
> How about 'CodeOfKey'?

QKeyCode, maybe?

Can you please paste the full error message?

> 
> > > +#
> > > +# An enumeration of key name.
> > > +#
> > > +# This is used by the send-key command.
> > > +#
> > > +# Since: 1.2
> > > +##
> > > +{ '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' ] }
> 

Re: [Qemu-devel] [PATCH v4 5/6] qapi: convert sendkey

[Amos Kong]

----- Original Message -----
> On Wed, 25 Jul 2012 01:55:14 -0400 (EDT)
> Amos Kong <akong@redhat.com> wrote:
> 
> > 
> > 
> > ----- Original Message -----
> > > On Thu,  5 Jul 2012 20:48:44 +0800
> > > Amos Kong <akong@redhat.com> 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 console.h is the mapping of key name to keycode,
> > > > Keys' index in the enmu and key_defs[] is same.
> > > > 
> > > > 'send-key' of QMP doesn't support key in hexadecimal format.
> > > > 
> > > > Signed-off-by: Amos Kong <akong@redhat.com>
> > > > ---
> > > >  console.h        |  152 ++++++++++++++++++++++++++++++++++
> > > >  hmp-commands.hx  |    2 +-
> > > >  hmp.c            |   64 +++++++++++++++
> > > >  hmp.h            |    1 +
> > > >  monitor.c        |  239
> > > >  ++++++------------------------------------------------
> > > >  qapi-schema.json |   46 +++++++++++
> > > >  qmp-commands.hx  |   28 +++++++
> > > >  7 files changed, 317 insertions(+), 215 deletions(-)
> > 
> > ...
> > 
> > > > diff --git a/qapi-schema.json b/qapi-schema.json
> > > > index 3b6e346..08e51c6 100644
> > > > --- a/qapi-schema.json
> > > > +++ b/qapi-schema.json
> > > > @@ -1862,3 +1862,49 @@
> > > >  # Since: 0.14.0
> > > >  ##
> > > >  { 'command': 'netdev_del', 'data': {'id': 'str'} }
> > > > +
> > > > +##
> > > > +# @KeyCodes:
> > > 
> > > s/KeyCodes/KeyCode
> > 
> > 
> > 'KeyCode' is not an available variable name.
> > 
> > | ./qapi-types.h:471: error: conflicting types for ‘KeyCode’
> > | /usr/include/X11/X.h:108: note: previous declaration of ‘KeyCode’
> > | was here
> > 
> > How about 'CodeOfKey'?
> 
> QKeyCode, maybe?

Looks good.

> Can you please paste the full error message?

[root@dhcp-8-167 qemu]# make 
  ....
  CC    slirp/arp_table.o
  CC    ui/keymaps.o
  CC    ui/spice-core.o
  CC    ui/spice-input.o
  CC    ui/spice-display.o
  CC    ui/sdl.o
In file included from ./console.h:9,
                 from ui/sdl.c:32:
./qapi-types.h:471: error: conflicting types for ‘KeyCode’
/usr/include/X11/X.h:108: note: previous declaration of ‘KeyCode’ was here
make: *** [ui/sdl.o] Error 1
[root@dhcp-8-167 qemu]# 

[Qemu-devel] [PATCH v4 6/6] ps2: output warning when event queue full

[Amos Kong]

Event would be ignored if ps2 queue is full, this patch
added a warning in ignore path.

Signed-off-by: Amos Kong <akong@redhat.com>
---
 hw/ps2.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/hw/ps2.c b/hw/ps2.c
index f93cd24..799c36b 100644
--- a/hw/ps2.c
+++ b/hw/ps2.c
@@ -137,8 +137,10 @@ void ps2_queue(void *opaque, int b)
     PS2State *s = (PS2State *)opaque;
     PS2Queue *q = &s->queue;
 
-    if (q->count >= PS2_QUEUE_SIZE)
+    if (q->count >= PS2_QUEUE_SIZE) {
+        fprintf(stderr, "ps2: warning: event queue full\n");
         return;
+    }
     q->data[q->wptr] = b;
     if (++q->wptr == PS2_QUEUE_SIZE)
         q->wptr = 0;
-- 
1.7.1