On 12/07/12 23:09, Luiz Capitulino wrote:
Hi Luiz,
On Thu, 5 Jul 2012 20:48:44 +0800
Amos Kong<ak...@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 <ak...@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.
