| Message ID | 1279457035-28657-1-git-send-email-mgoldish@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Sun, 18 Jul 2010 15:43:55 +0300 Michael Goldish <mgoldish@redhat.com> wrote: > Signed-off-by: Michael Goldish <mgoldish@redhat.com> Do you need this for 0.13? I think the development window is already closed. > --- > monitor.c | 15 ++++++++------- > qemu-monitor.hx | 22 +++++++++++++++++++++- > qerror.c | 12 ++++++++++++ > qerror.h | 9 +++++++++ > 4 files changed, 50 insertions(+), 8 deletions(-) > > diff --git a/monitor.c b/monitor.c > index d5377de..082c29d 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -1614,7 +1614,7 @@ static void release_keys(void *opaque) > } > } > > -static void do_sendkey(Monitor *mon, const QDict *qdict) > +static int do_sendkey(Monitor *mon, const QDict *qdict, QObject **ret_data) > { > char keyname_buf[16]; > char *separator; > @@ -1636,18 +1636,18 @@ static void do_sendkey(Monitor *mon, const QDict *qdict) > if (keyname_len > 0) { > pstrcpy(keyname_buf, sizeof(keyname_buf), string); > if (keyname_len > sizeof(keyname_buf) - 1) { > - monitor_printf(mon, "invalid key: '%s...'\n", keyname_buf); > - return; > + qerror_report(QERR_INVALID_KEY, keyname_buf); > + return -1; > } > if (i == MAX_KEYCODES) { > - monitor_printf(mon, "too many keys\n"); > - return; > + qerror_report(QERR_TOO_MANY_KEYS); > + return -1; > } > keyname_buf[keyname_len] = 0; > keycode = get_keycode(keyname_buf); > if (keycode < 0) { > - monitor_printf(mon, "unknown key: '%s'\n", keyname_buf); > - return; > + qerror_report(QERR_UNKNOWN_KEY, keyname_buf); > + return -1; > } > keycodes[i++] = keycode; > } > @@ -1666,6 +1666,7 @@ static void do_sendkey(Monitor *mon, const QDict *qdict) > /* delayed key up events */ > qemu_mod_timer(key_timer, qemu_get_clock(vm_clock) + > muldiv64(get_ticks_per_sec(), hold_time, 1000)); > + return 0; > } > > static int mouse_button_state; > diff --git a/qemu-monitor.hx b/qemu-monitor.hx > index f7e46c4..8b6cf09 100644 > --- a/qemu-monitor.hx > +++ b/qemu-monitor.hx > @@ -532,7 +532,8 @@ ETEXI > .args_type = "string: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, > + .user_print = monitor_user_noop, > + .mhandler.cmd_new = do_sendkey, > }, > > STEXI > @@ -549,6 +550,25 @@ sendkey ctrl-alt-f1 > This command is useful to send keys that your graphical user interface > intercepts at low level, such as @code{ctrl-alt-f1} in X Window. > ETEXI > +SQMP > +sendkey > +------- > + > +Send keys to the emulator. > + > +Arguments: > + > +- "string": key names or key codes in hexadecimal format separated by dashes (json-string) This is leaking bad stuff from the user monitor into QMP. I think we should have a "keys" key, which accepts an array of keys. Strings are key names, integers are key codes. Unfortunately, key codes will have to be specified in decimal. Example: { "execute": "sendkey", "arguments": { "keys": ["foo", 10, "bar", 15] } } However, in order to maintain the current user monitor behavior, you will have to add a new args_type so that you can move the current parsing out of the handler to the user monitor parser. Then you'll have to change the handler to work with an array. A bit of work. Another related issue is that, this probably should an async handler. But as we don't have the proper infrastructure yet, I'm ok with having this in its current form. > +- "hold_time": duration in milliseconds to hold the keys down (json-int, optional, default=100) > + > +Example: > + > +-> { "execute": "sendkey", "arguments": { "string": "ctrl-alt-f1" } } > +<- { "return": {} } > + > +Note: if several keys are given they are pressed simultaneously. > + > +EQMP > > { > .name = "system_reset", > diff --git a/qerror.c b/qerror.c > index 44d0bf8..34a698e 100644 > --- a/qerror.c > +++ b/qerror.c Please, split this into two patches: the errors first, then the conversion. That helps reviewing and reverting. > @@ -117,6 +117,10 @@ static const QErrorStringTable qerror_table[] = { > .desc = "Invalid block format '%(name)'", > }, > { > + .error_fmt = QERR_INVALID_KEY, > + .desc = "Invalid key '%(key)...'", > + }, > + { > .error_fmt = QERR_INVALID_PARAMETER, > .desc = "Invalid parameter '%(name)'", > }, > @@ -185,10 +189,18 @@ static const QErrorStringTable qerror_table[] = { > .desc = "Too many open files", > }, > { > + .error_fmt = QERR_TOO_MANY_KEYS, > + .desc = "Too many keys", > + }, > + { > .error_fmt = QERR_UNDEFINED_ERROR, > .desc = "An undefined error has ocurred", > }, > { > + .error_fmt = QERR_UNKNOWN_KEY, > + .desc = "Unknown key '%(key)'", > + }, > + { > .error_fmt = QERR_VNC_SERVER_FAILED, > .desc = "Could not start VNC server on %(target)", > }, > diff --git a/qerror.h b/qerror.h > index 77ae574..a23630c 100644 > --- a/qerror.h > +++ b/qerror.h > @@ -103,6 +103,9 @@ QError *qobject_to_qerror(const QObject *obj); > #define QERR_INVALID_BLOCK_FORMAT \ > "{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }" > > +#define QERR_INVALID_KEY \ > + "{ 'class': 'InvalidKey', 'data': { 'key': %s } }" > + > #define QERR_INVALID_PARAMETER \ > "{ 'class': 'InvalidParameter', 'data': { 'name': %s } }" > > @@ -154,9 +157,15 @@ QError *qobject_to_qerror(const QObject *obj); > #define QERR_TOO_MANY_FILES \ > "{ 'class': 'TooManyFiles', 'data': {} }" > > +#define QERR_TOO_MANY_KEYS \ > + "{ 'class': 'TooManyKeys', 'data': {} }" > + > #define QERR_UNDEFINED_ERROR \ > "{ 'class': 'UndefinedError', 'data': {} }" > > +#define QERR_UNKNOWN_KEY \ > + "{ 'class': 'UnknownKey', 'data': { 'key': %s } }" > + > #define QERR_VNC_SERVER_FAILED \ > "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }" >
On Wed, Jul 21, 2010 at 03:44:14PM -0300, Luiz Capitulino wrote: > On Sun, 18 Jul 2010 15:43:55 +0300 > Michael Goldish <mgoldish@redhat.com> wrote: > > > Signed-off-by: Michael Goldish <mgoldish@redhat.com> > > Do you need this for 0.13? I think the development window is already closed. > > > --- > > monitor.c | 15 ++++++++------- > > qemu-monitor.hx | 22 +++++++++++++++++++++- > > qerror.c | 12 ++++++++++++ > > qerror.h | 9 +++++++++ > > 4 files changed, 50 insertions(+), 8 deletions(-) > > > > diff --git a/monitor.c b/monitor.c > > index d5377de..082c29d 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -1614,7 +1614,7 @@ static void release_keys(void *opaque) > > } > > } > > > > -static void do_sendkey(Monitor *mon, const QDict *qdict) > > +static int do_sendkey(Monitor *mon, const QDict *qdict, QObject **ret_data) > > { > > char keyname_buf[16]; > > char *separator; > > @@ -1636,18 +1636,18 @@ static void do_sendkey(Monitor *mon, const QDict *qdict) > > if (keyname_len > 0) { > > pstrcpy(keyname_buf, sizeof(keyname_buf), string); > > if (keyname_len > sizeof(keyname_buf) - 1) { > > - monitor_printf(mon, "invalid key: '%s...'\n", keyname_buf); > > - return; > > + qerror_report(QERR_INVALID_KEY, keyname_buf); > > + return -1; > > } > > if (i == MAX_KEYCODES) { > > - monitor_printf(mon, "too many keys\n"); > > - return; > > + qerror_report(QERR_TOO_MANY_KEYS); > > + return -1; > > } > > keyname_buf[keyname_len] = 0; > > keycode = get_keycode(keyname_buf); > > if (keycode < 0) { > > - monitor_printf(mon, "unknown key: '%s'\n", keyname_buf); > > - return; > > + qerror_report(QERR_UNKNOWN_KEY, keyname_buf); > > + return -1; > > } > > keycodes[i++] = keycode; > > } > > @@ -1666,6 +1666,7 @@ static void do_sendkey(Monitor *mon, const QDict *qdict) > > /* delayed key up events */ > > qemu_mod_timer(key_timer, qemu_get_clock(vm_clock) + > > muldiv64(get_ticks_per_sec(), hold_time, 1000)); > > + return 0; > > } > > > > static int mouse_button_state; > > diff --git a/qemu-monitor.hx b/qemu-monitor.hx > > index f7e46c4..8b6cf09 100644 > > --- a/qemu-monitor.hx > > +++ b/qemu-monitor.hx > > @@ -532,7 +532,8 @@ ETEXI > > .args_type = "string: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, > > + .user_print = monitor_user_noop, > > + .mhandler.cmd_new = do_sendkey, > > }, > > > > STEXI > > @@ -549,6 +550,25 @@ sendkey ctrl-alt-f1 > > This command is useful to send keys that your graphical user interface > > intercepts at low level, such as @code{ctrl-alt-f1} in X Window. > > ETEXI > > +SQMP > > +sendkey > > +------- > > + > > +Send keys to the emulator. > > + > > +Arguments: > > + > > +- "string": key names or key codes in hexadecimal format separated by dashes (json-string) > > This is leaking bad stuff from the user monitor into QMP. > > I think we should have a "keys" key, which accepts an array of keys. Strings > are key names, integers are key codes. Unfortunately, key codes will have to > be specified in decimal. I can see why keynames are nice in the text monitor, but is there a need for JSON mode to accept keynames too ? If we're focusing on providing a machine protocol, then keycodes seem like they sufficient to me. Apps can using the command can provide symbol constants for the keycodes, or string if actually needed by their end users > > Example: > > { "execute": "sendkey", "arguments": { "keys": ["foo", 10, "bar", 15] } } > > However, in order to maintain the current user monitor behavior, you will > have to add a new args_type so that you can move the current parsing out > of the handler to the user monitor parser. Then you'll have to change the > handler to work with an array. > > A bit of work. > > Another related issue is that, this probably should an async handler. But > as we don't have the proper infrastructure yet, I'm ok with having this in > its current form. > > > +- "hold_time": duration in milliseconds to hold the keys down (json-int, optional, default=100) Having 'hold-time' which applies to the full list of keys is limiting the flexibility of apps. eg, it means you can only do down ctrl down alt down f1 wait 100ms up ctrl up alt up f1 Again I can see why the impl works this way currently, because it is clearly a nicer option for humans. For a machine protocol though it seems sub-optimal. What if app needed more flexibility over ordering of press+release events eg to release in a different order down ctrl down alt down f1 wait 100ms up f1 up ctrl up alt Should we just follow VNC and explicitly have a up/down flag in the protocol & let press & release events be sent separately. { "execute": "sendkey", "arguments": { "keycode": 0x31, "down": true } } We could allow multiple keycodes in one message { "execute": "sendkey", "arguments": { "keycodes": [ 0x31, 0x32 ], "down": true } } but its not really adding critical functionality that can't be got by sending a sequence of sendkey commands in a row. Daniel
On 07/21/2010 09:44 PM, Luiz Capitulino wrote: > On Sun, 18 Jul 2010 15:43:55 +0300 > Michael Goldish <mgoldish@redhat.com> wrote: > >> Signed-off-by: Michael Goldish <mgoldish@redhat.com> > > Do you need this for 0.13? I think the development window is already closed. No, it's not urgent. >> --- >> monitor.c | 15 ++++++++------- >> qemu-monitor.hx | 22 +++++++++++++++++++++- >> qerror.c | 12 ++++++++++++ >> qerror.h | 9 +++++++++ >> 4 files changed, 50 insertions(+), 8 deletions(-) >> >> diff --git a/monitor.c b/monitor.c >> index d5377de..082c29d 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -1614,7 +1614,7 @@ static void release_keys(void *opaque) >> } >> } >> >> -static void do_sendkey(Monitor *mon, const QDict *qdict) >> +static int do_sendkey(Monitor *mon, const QDict *qdict, QObject **ret_data) >> { >> char keyname_buf[16]; >> char *separator; >> @@ -1636,18 +1636,18 @@ static void do_sendkey(Monitor *mon, const QDict *qdict) >> if (keyname_len > 0) { >> pstrcpy(keyname_buf, sizeof(keyname_buf), string); >> if (keyname_len > sizeof(keyname_buf) - 1) { >> - monitor_printf(mon, "invalid key: '%s...'\n", keyname_buf); >> - return; >> + qerror_report(QERR_INVALID_KEY, keyname_buf); >> + return -1; >> } >> if (i == MAX_KEYCODES) { >> - monitor_printf(mon, "too many keys\n"); >> - return; >> + qerror_report(QERR_TOO_MANY_KEYS); >> + return -1; >> } >> keyname_buf[keyname_len] = 0; >> keycode = get_keycode(keyname_buf); >> if (keycode < 0) { >> - monitor_printf(mon, "unknown key: '%s'\n", keyname_buf); >> - return; >> + qerror_report(QERR_UNKNOWN_KEY, keyname_buf); >> + return -1; >> } >> keycodes[i++] = keycode; >> } >> @@ -1666,6 +1666,7 @@ static void do_sendkey(Monitor *mon, const QDict *qdict) >> /* delayed key up events */ >> qemu_mod_timer(key_timer, qemu_get_clock(vm_clock) + >> muldiv64(get_ticks_per_sec(), hold_time, 1000)); >> + return 0; >> } >> >> static int mouse_button_state; >> diff --git a/qemu-monitor.hx b/qemu-monitor.hx >> index f7e46c4..8b6cf09 100644 >> --- a/qemu-monitor.hx >> +++ b/qemu-monitor.hx >> @@ -532,7 +532,8 @@ ETEXI >> .args_type = "string: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, >> + .user_print = monitor_user_noop, >> + .mhandler.cmd_new = do_sendkey, >> }, >> >> STEXI >> @@ -549,6 +550,25 @@ sendkey ctrl-alt-f1 >> This command is useful to send keys that your graphical user interface >> intercepts at low level, such as @code{ctrl-alt-f1} in X Window. >> ETEXI >> +SQMP >> +sendkey >> +------- >> + >> +Send keys to the emulator. >> + >> +Arguments: >> + >> +- "string": key names or key codes in hexadecimal format separated by dashes (json-string) > > This is leaking bad stuff from the user monitor into QMP. > > I think we should have a "keys" key, which accepts an array of keys. Strings > are key names, integers are key codes. Unfortunately, key codes will have to > be specified in decimal. > > Example: > > { "execute": "sendkey", "arguments": { "keys": ["foo", 10, "bar", 15] } } > > However, in order to maintain the current user monitor behavior, you will > have to add a new args_type so that you can move the current parsing out > of the handler to the user monitor parser. Then you'll have to change the > handler to work with an array. What do you mean by "add a new args_type"? Do you mean I should add a new type 'a', similar to 's' and 'i', that represents a dash separated string/int array? If so, I suppose obtaining the array in do_sendkey() would involve something like QList *keys = qdict_get_qlist(qdict, "keys"); Is this correct? I'm just asking to be sure. > A bit of work. > > Another related issue is that, this probably should an async handler. But > as we don't have the proper infrastructure yet, I'm ok with having this in > its current form. What's an async handler? Feel free to point me to some documentation if there is any. >> +- "hold_time": duration in milliseconds to hold the keys down (json-int, optional, default=100) >> + >> +Example: >> + >> +-> { "execute": "sendkey", "arguments": { "string": "ctrl-alt-f1" } } >> +<- { "return": {} } >> + >> +Note: if several keys are given they are pressed simultaneously. >> + >> +EQMP >> >> { >> .name = "system_reset", >> diff --git a/qerror.c b/qerror.c >> index 44d0bf8..34a698e 100644 >> --- a/qerror.c >> +++ b/qerror.c > > Please, split this into two patches: the errors first, then the conversion. > That helps reviewing and reverting. OK. Thanks for the comments. >> @@ -117,6 +117,10 @@ static const QErrorStringTable qerror_table[] = { >> .desc = "Invalid block format '%(name)'", >> }, >> { >> + .error_fmt = QERR_INVALID_KEY, >> + .desc = "Invalid key '%(key)...'", >> + }, >> + { >> .error_fmt = QERR_INVALID_PARAMETER, >> .desc = "Invalid parameter '%(name)'", >> }, >> @@ -185,10 +189,18 @@ static const QErrorStringTable qerror_table[] = { >> .desc = "Too many open files", >> }, >> { >> + .error_fmt = QERR_TOO_MANY_KEYS, >> + .desc = "Too many keys", >> + }, >> + { >> .error_fmt = QERR_UNDEFINED_ERROR, >> .desc = "An undefined error has ocurred", >> }, >> { >> + .error_fmt = QERR_UNKNOWN_KEY, >> + .desc = "Unknown key '%(key)'", >> + }, >> + { >> .error_fmt = QERR_VNC_SERVER_FAILED, >> .desc = "Could not start VNC server on %(target)", >> }, >> diff --git a/qerror.h b/qerror.h >> index 77ae574..a23630c 100644 >> --- a/qerror.h >> +++ b/qerror.h >> @@ -103,6 +103,9 @@ QError *qobject_to_qerror(const QObject *obj); >> #define QERR_INVALID_BLOCK_FORMAT \ >> "{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }" >> >> +#define QERR_INVALID_KEY \ >> + "{ 'class': 'InvalidKey', 'data': { 'key': %s } }" >> + >> #define QERR_INVALID_PARAMETER \ >> "{ 'class': 'InvalidParameter', 'data': { 'name': %s } }" >> >> @@ -154,9 +157,15 @@ QError *qobject_to_qerror(const QObject *obj); >> #define QERR_TOO_MANY_FILES \ >> "{ 'class': 'TooManyFiles', 'data': {} }" >> >> +#define QERR_TOO_MANY_KEYS \ >> + "{ 'class': 'TooManyKeys', 'data': {} }" >> + >> #define QERR_UNDEFINED_ERROR \ >> "{ 'class': 'UndefinedError', 'data': {} }" >> >> +#define QERR_UNKNOWN_KEY \ >> + "{ 'class': 'UnknownKey', 'data': { 'key': %s } }" >> + >> #define QERR_VNC_SERVER_FAILED \ >> "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }" >> > >
On Wed, 21 Jul 2010 20:06:56 +0100 "Daniel P. Berrange" <berrange@redhat.com> wrote: > On Wed, Jul 21, 2010 at 03:44:14PM -0300, Luiz Capitulino wrote: > > On Sun, 18 Jul 2010 15:43:55 +0300 > > Michael Goldish <mgoldish@redhat.com> wrote: > > > > > Signed-off-by: Michael Goldish <mgoldish@redhat.com> > > > > Do you need this for 0.13? I think the development window is already closed. > > > > > --- > > > monitor.c | 15 ++++++++------- > > > qemu-monitor.hx | 22 +++++++++++++++++++++- > > > qerror.c | 12 ++++++++++++ > > > qerror.h | 9 +++++++++ > > > 4 files changed, 50 insertions(+), 8 deletions(-) > > > > > > diff --git a/monitor.c b/monitor.c > > > index d5377de..082c29d 100644 > > > --- a/monitor.c > > > +++ b/monitor.c > > > @@ -1614,7 +1614,7 @@ static void release_keys(void *opaque) > > > } > > > } > > > > > > -static void do_sendkey(Monitor *mon, const QDict *qdict) > > > +static int do_sendkey(Monitor *mon, const QDict *qdict, QObject **ret_data) > > > { > > > char keyname_buf[16]; > > > char *separator; > > > @@ -1636,18 +1636,18 @@ static void do_sendkey(Monitor *mon, const QDict *qdict) > > > if (keyname_len > 0) { > > > pstrcpy(keyname_buf, sizeof(keyname_buf), string); > > > if (keyname_len > sizeof(keyname_buf) - 1) { > > > - monitor_printf(mon, "invalid key: '%s...'\n", keyname_buf); > > > - return; > > > + qerror_report(QERR_INVALID_KEY, keyname_buf); > > > + return -1; > > > } > > > if (i == MAX_KEYCODES) { > > > - monitor_printf(mon, "too many keys\n"); > > > - return; > > > + qerror_report(QERR_TOO_MANY_KEYS); > > > + return -1; > > > } > > > keyname_buf[keyname_len] = 0; > > > keycode = get_keycode(keyname_buf); > > > if (keycode < 0) { > > > - monitor_printf(mon, "unknown key: '%s'\n", keyname_buf); > > > - return; > > > + qerror_report(QERR_UNKNOWN_KEY, keyname_buf); > > > + return -1; > > > } > > > keycodes[i++] = keycode; > > > } > > > @@ -1666,6 +1666,7 @@ static void do_sendkey(Monitor *mon, const QDict *qdict) > > > /* delayed key up events */ > > > qemu_mod_timer(key_timer, qemu_get_clock(vm_clock) + > > > muldiv64(get_ticks_per_sec(), hold_time, 1000)); > > > + return 0; > > > } > > > > > > static int mouse_button_state; > > > diff --git a/qemu-monitor.hx b/qemu-monitor.hx > > > index f7e46c4..8b6cf09 100644 > > > --- a/qemu-monitor.hx > > > +++ b/qemu-monitor.hx > > > @@ -532,7 +532,8 @@ ETEXI > > > .args_type = "string: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, > > > + .user_print = monitor_user_noop, > > > + .mhandler.cmd_new = do_sendkey, > > > }, > > > > > > STEXI > > > @@ -549,6 +550,25 @@ sendkey ctrl-alt-f1 > > > This command is useful to send keys that your graphical user interface > > > intercepts at low level, such as @code{ctrl-alt-f1} in X Window. > > > ETEXI > > > +SQMP > > > +sendkey > > > +------- > > > + > > > +Send keys to the emulator. > > > + > > > +Arguments: > > > + > > > +- "string": key names or key codes in hexadecimal format separated by dashes (json-string) > > > > This is leaking bad stuff from the user monitor into QMP. > > > > I think we should have a "keys" key, which accepts an array of keys. Strings > > are key names, integers are key codes. Unfortunately, key codes will have to > > be specified in decimal. > > I can see why keynames are nice in the text monitor, but is there a need > for JSON mode to accept keynames too ? If we're focusing on providing a > machine protocol, then keycodes seem like they sufficient to me. Apps can > using the command can provide symbol constants for the keycodes, or string > if actually needed by their end users Right. That should be a user monitor's feature, not QMP's. > > Example: > > > > { "execute": "sendkey", "arguments": { "keys": ["foo", 10, "bar", 15] } } > > > > However, in order to maintain the current user monitor behavior, you will > > have to add a new args_type so that you can move the current parsing out > > of the handler to the user monitor parser. Then you'll have to change the > > handler to work with an array. > > > > A bit of work. > > > > Another related issue is that, this probably should an async handler. But > > as we don't have the proper infrastructure yet, I'm ok with having this in > > its current form. > > > > > +- "hold_time": duration in milliseconds to hold the keys down (json-int, optional, default=100) > > Having 'hold-time' which applies to the full list of keys is limiting > the flexibility of apps. eg, it means you can only do > > down ctrl > down alt > down f1 > wait 100ms > up ctrl > up alt > up f1 > > Again I can see why the impl works this way currently, because it is > clearly a nicer option for humans. For a machine protocol though it > seems sub-optimal. What if app needed more flexibility over ordering > of press+release events eg to release in a different order > > down ctrl > down alt > down f1 > wait 100ms > up f1 > up ctrl > up alt > > Should we just follow VNC and explicitly have a up/down flag in > the protocol & let press & release events be sent separately. > > { "execute": "sendkey", "arguments": { "keycode": 0x31, "down": true } } > > We could allow multiple keycodes in one message > > { "execute": "sendkey", "arguments": { "keycodes": [ 0x31, 0x32 ], "down": true } } > > but its not really adding critical functionality that can't be got by > sending a sequence of sendkey commands in a row. Hm, looks good to me, but then the hold time would be the time period between the down/up commands. This won't be reliable in case the client wants to exactly wait 100ms, as we can have network latency, for example. Isn't this a problem? I believe VNC doesn't have this feature, right?
On Wed, 21 Jul 2010 23:30:56 +0300 Michael Goldish <mgoldish@redhat.com> wrote: > On 07/21/2010 09:44 PM, Luiz Capitulino wrote: > > On Sun, 18 Jul 2010 15:43:55 +0300 > > Michael Goldish <mgoldish@redhat.com> wrote: > > > >> Signed-off-by: Michael Goldish <mgoldish@redhat.com> > > > > Do you need this for 0.13? I think the development window is already closed. > > No, it's not urgent. > > >> --- > >> monitor.c | 15 ++++++++------- > >> qemu-monitor.hx | 22 +++++++++++++++++++++- > >> qerror.c | 12 ++++++++++++ > >> qerror.h | 9 +++++++++ > >> 4 files changed, 50 insertions(+), 8 deletions(-) > >> > >> diff --git a/monitor.c b/monitor.c > >> index d5377de..082c29d 100644 > >> --- a/monitor.c > >> +++ b/monitor.c > >> @@ -1614,7 +1614,7 @@ static void release_keys(void *opaque) > >> } > >> } > >> > >> -static void do_sendkey(Monitor *mon, const QDict *qdict) > >> +static int do_sendkey(Monitor *mon, const QDict *qdict, QObject **ret_data) > >> { > >> char keyname_buf[16]; > >> char *separator; > >> @@ -1636,18 +1636,18 @@ static void do_sendkey(Monitor *mon, const QDict *qdict) > >> if (keyname_len > 0) { > >> pstrcpy(keyname_buf, sizeof(keyname_buf), string); > >> if (keyname_len > sizeof(keyname_buf) - 1) { > >> - monitor_printf(mon, "invalid key: '%s...'\n", keyname_buf); > >> - return; > >> + qerror_report(QERR_INVALID_KEY, keyname_buf); > >> + return -1; > >> } > >> if (i == MAX_KEYCODES) { > >> - monitor_printf(mon, "too many keys\n"); > >> - return; > >> + qerror_report(QERR_TOO_MANY_KEYS); > >> + return -1; > >> } > >> keyname_buf[keyname_len] = 0; > >> keycode = get_keycode(keyname_buf); > >> if (keycode < 0) { > >> - monitor_printf(mon, "unknown key: '%s'\n", keyname_buf); > >> - return; > >> + qerror_report(QERR_UNKNOWN_KEY, keyname_buf); > >> + return -1; > >> } > >> keycodes[i++] = keycode; > >> } > >> @@ -1666,6 +1666,7 @@ static void do_sendkey(Monitor *mon, const QDict *qdict) > >> /* delayed key up events */ > >> qemu_mod_timer(key_timer, qemu_get_clock(vm_clock) + > >> muldiv64(get_ticks_per_sec(), hold_time, 1000)); > >> + return 0; > >> } > >> > >> static int mouse_button_state; > >> diff --git a/qemu-monitor.hx b/qemu-monitor.hx > >> index f7e46c4..8b6cf09 100644 > >> --- a/qemu-monitor.hx > >> +++ b/qemu-monitor.hx > >> @@ -532,7 +532,8 @@ ETEXI > >> .args_type = "string: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, > >> + .user_print = monitor_user_noop, > >> + .mhandler.cmd_new = do_sendkey, > >> }, > >> > >> STEXI > >> @@ -549,6 +550,25 @@ sendkey ctrl-alt-f1 > >> This command is useful to send keys that your graphical user interface > >> intercepts at low level, such as @code{ctrl-alt-f1} in X Window. > >> ETEXI > >> +SQMP > >> +sendkey > >> +------- > >> + > >> +Send keys to the emulator. > >> + > >> +Arguments: > >> + > >> +- "string": key names or key codes in hexadecimal format separated by dashes (json-string) > > > > This is leaking bad stuff from the user monitor into QMP. > > > > I think we should have a "keys" key, which accepts an array of keys. Strings > > are key names, integers are key codes. Unfortunately, key codes will have to > > be specified in decimal. > > > > Example: > > > > { "execute": "sendkey", "arguments": { "keys": ["foo", 10, "bar", 15] } } > > > > However, in order to maintain the current user monitor behavior, you will > > have to add a new args_type so that you can move the current parsing out > > of the handler to the user monitor parser. Then you'll have to change the > > handler to work with an array. > > What do you mean by "add a new args_type"? Do you mean I should add a > new type 'a', similar to 's' and 'i', that represents a dash separated > string/int array? If so, I suppose obtaining the array in do_sendkey() > would involve something like > > QList *keys = qdict_get_qlist(qdict, "keys"); > > Is this correct? I'm just asking to be sure. It's exactly that. The user monitor does the fancy user parsing, handlers handle QMP data only. But note that the recommended QMP development process is to send the documentation patch first, as an RFC. This way we can discuss and stabilize the interface before touching the code (which is easier and avoids the code dictating the interface). So, it's a good idea to rewrite the documentation based on my discussion with Daniel, and re-send it. This process is not documented yet, will send a patch shortly. > > A bit of work. > > > > Another related issue is that, this probably should an async handler. But > > as we don't have the proper infrastructure yet, I'm ok with having this in > > its current form. > > What's an async handler? Feel free to point me to some documentation if > there is any. http://wiki.qemu.org/Features/QMP_0.14 It's quite beyond this conversion, you don't have to worry about that. > > >> +- "hold_time": duration in milliseconds to hold the keys down (json-int, optional, default=100) > >> + > >> +Example: > >> + > >> +-> { "execute": "sendkey", "arguments": { "string": "ctrl-alt-f1" } } > >> +<- { "return": {} } > >> + > >> +Note: if several keys are given they are pressed simultaneously. > >> + > >> +EQMP > >> > >> { > >> .name = "system_reset", > >> diff --git a/qerror.c b/qerror.c > >> index 44d0bf8..34a698e 100644 > >> --- a/qerror.c > >> +++ b/qerror.c > > > > Please, split this into two patches: the errors first, then the conversion. > > That helps reviewing and reverting. > > OK. Thanks for the comments. > > >> @@ -117,6 +117,10 @@ static const QErrorStringTable qerror_table[] = { > >> .desc = "Invalid block format '%(name)'", > >> }, > >> { > >> + .error_fmt = QERR_INVALID_KEY, > >> + .desc = "Invalid key '%(key)...'", > >> + }, > >> + { > >> .error_fmt = QERR_INVALID_PARAMETER, > >> .desc = "Invalid parameter '%(name)'", > >> }, > >> @@ -185,10 +189,18 @@ static const QErrorStringTable qerror_table[] = { > >> .desc = "Too many open files", > >> }, > >> { > >> + .error_fmt = QERR_TOO_MANY_KEYS, > >> + .desc = "Too many keys", > >> + }, > >> + { > >> .error_fmt = QERR_UNDEFINED_ERROR, > >> .desc = "An undefined error has ocurred", > >> }, > >> { > >> + .error_fmt = QERR_UNKNOWN_KEY, > >> + .desc = "Unknown key '%(key)'", > >> + }, > >> + { > >> .error_fmt = QERR_VNC_SERVER_FAILED, > >> .desc = "Could not start VNC server on %(target)", > >> }, > >> diff --git a/qerror.h b/qerror.h > >> index 77ae574..a23630c 100644 > >> --- a/qerror.h > >> +++ b/qerror.h > >> @@ -103,6 +103,9 @@ QError *qobject_to_qerror(const QObject *obj); > >> #define QERR_INVALID_BLOCK_FORMAT \ > >> "{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }" > >> > >> +#define QERR_INVALID_KEY \ > >> + "{ 'class': 'InvalidKey', 'data': { 'key': %s } }" > >> + > >> #define QERR_INVALID_PARAMETER \ > >> "{ 'class': 'InvalidParameter', 'data': { 'name': %s } }" > >> > >> @@ -154,9 +157,15 @@ QError *qobject_to_qerror(const QObject *obj); > >> #define QERR_TOO_MANY_FILES \ > >> "{ 'class': 'TooManyFiles', 'data': {} }" > >> > >> +#define QERR_TOO_MANY_KEYS \ > >> + "{ 'class': 'TooManyKeys', 'data': {} }" > >> + > >> #define QERR_UNDEFINED_ERROR \ > >> "{ 'class': 'UndefinedError', 'data': {} }" > >> > >> +#define QERR_UNKNOWN_KEY \ > >> + "{ 'class': 'UnknownKey', 'data': { 'key': %s } }" > >> + > >> #define QERR_VNC_SERVER_FAILED \ > >> "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }" > >> > > > > >
On Thu, Jul 22, 2010 at 10:28:39AM -0300, Luiz Capitulino wrote: > On Wed, 21 Jul 2010 20:06:56 +0100 > "Daniel P. Berrange" <berrange@redhat.com> wrote: > > > On Wed, Jul 21, 2010 at 03:44:14PM -0300, Luiz Capitulino wrote: > > > Another related issue is that, this probably should an async handler. But > > > as we don't have the proper infrastructure yet, I'm ok with having this in > > > its current form. > > > > > > > +- "hold_time": duration in milliseconds to hold the keys down (json-int, optional, default=100) > > > > Having 'hold-time' which applies to the full list of keys is limiting > > the flexibility of apps. eg, it means you can only do > > > > down ctrl > > down alt > > down f1 > > wait 100ms > > up ctrl > > up alt > > up f1 > > > > Again I can see why the impl works this way currently, because it is > > clearly a nicer option for humans. For a machine protocol though it > > seems sub-optimal. What if app needed more flexibility over ordering > > of press+release events eg to release in a different order > > > > down ctrl > > down alt > > down f1 > > wait 100ms > > up f1 > > up ctrl > > up alt > > > > Should we just follow VNC and explicitly have a up/down flag in > > the protocol & let press & release events be sent separately. > > > > { "execute": "sendkey", "arguments": { "keycode": 0x31, "down": true } } > > > > We could allow multiple keycodes in one message > > > > { "execute": "sendkey", "arguments": { "keycodes": [ 0x31, 0x32 ], "down": true } } > > > > but its not really adding critical functionality that can't be got by > > sending a sequence of sendkey commands in a row. > > Hm, looks good to me, but then the hold time would be the time period > between the down/up commands. This won't be reliable in case the client > wants to exactly wait 100ms, as we can have network latency, for example. > > Isn't this a problem? I believe VNC doesn't have this feature, right? Correct, VNC just sends each individual press / release event as a separate message, so you can have network delay effects there too. If we needed to support precise delays safe from network delay for some simulation needs, then you'd probably need a more complex structure where you can provide a whole sequence of operations. And why stop at keys, including mouse movement & buttons to. { "execute": "sendinput", "arguments": { "sequence" : [ { "event": "keypress", "keycode": 0x31 }, { "event": "keypress", "keycode": 0x75 }, { "event": "wait", "delay": 100 }, { "event": "mousepress", "button": 1 }, { "event": "mousemove", "xdelta": 1, "ydelta": 1 }, { "event": "keyrelease", "keycode": 0x31 }, { "event": "wait", "delay": 100 }, { "event": "keyrelease", "keycode": 0x75 }, { "event": "mousepos", "x": 102, "y": 102 }, ] } } This is getting kind of advanced now. Whether we need this vs the simpler sendkey, mouse_move, etc command comes down to whether we need ability to set precise delays between events. We could stick with the individual simple commands & add a advanced one alter Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Thu, 22 Jul 2010 14:45:35 +0100 "Daniel P. Berrange" <berrange@redhat.com> wrote: > On Thu, Jul 22, 2010 at 10:28:39AM -0300, Luiz Capitulino wrote: > > On Wed, 21 Jul 2010 20:06:56 +0100 > > "Daniel P. Berrange" <berrange@redhat.com> wrote: > > > > > On Wed, Jul 21, 2010 at 03:44:14PM -0300, Luiz Capitulino wrote: > > > > Another related issue is that, this probably should an async handler. But > > > > as we don't have the proper infrastructure yet, I'm ok with having this in > > > > its current form. > > > > > > > > > +- "hold_time": duration in milliseconds to hold the keys down (json-int, optional, default=100) > > > > > > Having 'hold-time' which applies to the full list of keys is limiting > > > the flexibility of apps. eg, it means you can only do > > > > > > down ctrl > > > down alt > > > down f1 > > > wait 100ms > > > up ctrl > > > up alt > > > up f1 > > > > > > Again I can see why the impl works this way currently, because it is > > > clearly a nicer option for humans. For a machine protocol though it > > > seems sub-optimal. What if app needed more flexibility over ordering > > > of press+release events eg to release in a different order > > > > > > down ctrl > > > down alt > > > down f1 > > > wait 100ms > > > up f1 > > > up ctrl > > > up alt > > > > > > Should we just follow VNC and explicitly have a up/down flag in > > > the protocol & let press & release events be sent separately. > > > > > > { "execute": "sendkey", "arguments": { "keycode": 0x31, "down": true } } > > > > > > We could allow multiple keycodes in one message > > > > > > { "execute": "sendkey", "arguments": { "keycodes": [ 0x31, 0x32 ], "down": true } } > > > > > > but its not really adding critical functionality that can't be got by > > > sending a sequence of sendkey commands in a row. > > > > Hm, looks good to me, but then the hold time would be the time period > > between the down/up commands. This won't be reliable in case the client > > wants to exactly wait 100ms, as we can have network latency, for example. > > > > Isn't this a problem? I believe VNC doesn't have this feature, right? > > Correct, VNC just sends each individual press / release event as a separate > message, so you can have network delay effects there too. > > If we needed to support precise delays safe from network delay for some > simulation needs, then you'd probably need a more complex structure > where you can provide a whole sequence of operations. And why stop at > keys, including mouse movement & buttons to. > > { "execute": "sendinput", "arguments": { > "sequence" : [ > { "event": "keypress", "keycode": 0x31 }, > { "event": "keypress", "keycode": 0x75 }, > { "event": "wait", "delay": 100 }, > { "event": "mousepress", "button": 1 }, > { "event": "mousemove", "xdelta": 1, "ydelta": 1 }, > { "event": "keyrelease", "keycode": 0x31 }, > { "event": "wait", "delay": 100 }, > { "event": "keyrelease", "keycode": 0x75 }, > { "event": "mousepos", "x": 102, "y": 102 }, > ] > } } > > This is getting kind of advanced now. Whether we need this vs the simpler > sendkey, mouse_move, etc command comes down to whether we need ability to > set precise delays between events. We could stick with the individual > simple commands & add a advanced one alter Yeah, agreed and I think the current design (ie. hold_time) is simpler.
On Thu, Jul 22, 2010 at 10:50:00AM -0300, Luiz Capitulino wrote: > On Thu, 22 Jul 2010 14:45:35 +0100 > "Daniel P. Berrange" <berrange@redhat.com> wrote: > > > On Thu, Jul 22, 2010 at 10:28:39AM -0300, Luiz Capitulino wrote: > > > On Wed, 21 Jul 2010 20:06:56 +0100 > > > "Daniel P. Berrange" <berrange@redhat.com> wrote: > > > > > > > On Wed, Jul 21, 2010 at 03:44:14PM -0300, Luiz Capitulino wrote: > > > > > Another related issue is that, this probably should an async handler. But > > > > > as we don't have the proper infrastructure yet, I'm ok with having this in > > > > > its current form. > > > > > > > > > > > +- "hold_time": duration in milliseconds to hold the keys down (json-int, optional, default=100) > > > > > > > > Having 'hold-time' which applies to the full list of keys is limiting > > > > the flexibility of apps. eg, it means you can only do > > > > > > > > down ctrl > > > > down alt > > > > down f1 > > > > wait 100ms > > > > up ctrl > > > > up alt > > > > up f1 > > > > > > > > Again I can see why the impl works this way currently, because it is > > > > clearly a nicer option for humans. For a machine protocol though it > > > > seems sub-optimal. What if app needed more flexibility over ordering > > > > of press+release events eg to release in a different order > > > > > > > > down ctrl > > > > down alt > > > > down f1 > > > > wait 100ms > > > > up f1 > > > > up ctrl > > > > up alt > > > > > > > > Should we just follow VNC and explicitly have a up/down flag in > > > > the protocol & let press & release events be sent separately. > > > > > > > > { "execute": "sendkey", "arguments": { "keycode": 0x31, "down": true } } > > > > > > > > We could allow multiple keycodes in one message > > > > > > > > { "execute": "sendkey", "arguments": { "keycodes": [ 0x31, 0x32 ], "down": true } } > > > > > > > > but its not really adding critical functionality that can't be got by > > > > sending a sequence of sendkey commands in a row. > > > > > > Hm, looks good to me, but then the hold time would be the time period > > > between the down/up commands. This won't be reliable in case the client > > > wants to exactly wait 100ms, as we can have network latency, for example. > > > > > > Isn't this a problem? I believe VNC doesn't have this feature, right? > > > > Correct, VNC just sends each individual press / release event as a separate > > message, so you can have network delay effects there too. > > > > If we needed to support precise delays safe from network delay for some > > simulation needs, then you'd probably need a more complex structure > > where you can provide a whole sequence of operations. And why stop at > > keys, including mouse movement & buttons to. > > > > { "execute": "sendinput", "arguments": { > > "sequence" : [ > > { "event": "keypress", "keycode": 0x31 }, > > { "event": "keypress", "keycode": 0x75 }, > > { "event": "wait", "delay": 100 }, > > { "event": "mousepress", "button": 1 }, > > { "event": "mousemove", "xdelta": 1, "ydelta": 1 }, > > { "event": "keyrelease", "keycode": 0x31 }, > > { "event": "wait", "delay": 100 }, > > { "event": "keyrelease", "keycode": 0x75 }, > > { "event": "mousepos", "x": 102, "y": 102 }, > > ] > > } } > > > > This is getting kind of advanced now. Whether we need this vs the simpler > > sendkey, mouse_move, etc command comes down to whether we need ability to > > set precise delays between events. We could stick with the individual > > simple commands & add a advanced one alter > > Yeah, agreed and I think the current design (ie. hold_time) is simpler. When I said stick with simpler sendkey, I mean the style I outlined without any hold time at all > > > > { "execute": "sendkey", "arguments": { "keycode": 0x31, "down": true } } If its good enough for VNC & SPICE, it should be good enough for most monitor users, even more so since QMP communication is usually over a low latency UNIX domain socket. I think hold time is a flawed concept as currently provide since it prevents up/down interleaving by sending a sequence of QMP commands. Daniel
On Thu, 22 Jul 2010 15:03:27 +0100 "Daniel P. Berrange" <berrange@redhat.com> wrote: > On Thu, Jul 22, 2010 at 10:50:00AM -0300, Luiz Capitulino wrote: > > On Thu, 22 Jul 2010 14:45:35 +0100 > > "Daniel P. Berrange" <berrange@redhat.com> wrote: > > > > > On Thu, Jul 22, 2010 at 10:28:39AM -0300, Luiz Capitulino wrote: > > > > On Wed, 21 Jul 2010 20:06:56 +0100 > > > > "Daniel P. Berrange" <berrange@redhat.com> wrote: > > > > > > > > > On Wed, Jul 21, 2010 at 03:44:14PM -0300, Luiz Capitulino wrote: > > > > > > Another related issue is that, this probably should an async handler. But > > > > > > as we don't have the proper infrastructure yet, I'm ok with having this in > > > > > > its current form. > > > > > > > > > > > > > +- "hold_time": duration in milliseconds to hold the keys down (json-int, optional, default=100) > > > > > > > > > > Having 'hold-time' which applies to the full list of keys is limiting > > > > > the flexibility of apps. eg, it means you can only do > > > > > > > > > > down ctrl > > > > > down alt > > > > > down f1 > > > > > wait 100ms > > > > > up ctrl > > > > > up alt > > > > > up f1 > > > > > > > > > > Again I can see why the impl works this way currently, because it is > > > > > clearly a nicer option for humans. For a machine protocol though it > > > > > seems sub-optimal. What if app needed more flexibility over ordering > > > > > of press+release events eg to release in a different order > > > > > > > > > > down ctrl > > > > > down alt > > > > > down f1 > > > > > wait 100ms > > > > > up f1 > > > > > up ctrl > > > > > up alt > > > > > > > > > > Should we just follow VNC and explicitly have a up/down flag in > > > > > the protocol & let press & release events be sent separately. > > > > > > > > > > { "execute": "sendkey", "arguments": { "keycode": 0x31, "down": true } } > > > > > > > > > > We could allow multiple keycodes in one message > > > > > > > > > > { "execute": "sendkey", "arguments": { "keycodes": [ 0x31, 0x32 ], "down": true } } > > > > > > > > > > but its not really adding critical functionality that can't be got by > > > > > sending a sequence of sendkey commands in a row. > > > > > > > > Hm, looks good to me, but then the hold time would be the time period > > > > between the down/up commands. This won't be reliable in case the client > > > > wants to exactly wait 100ms, as we can have network latency, for example. > > > > > > > > Isn't this a problem? I believe VNC doesn't have this feature, right? > > > > > > Correct, VNC just sends each individual press / release event as a separate > > > message, so you can have network delay effects there too. > > > > > > If we needed to support precise delays safe from network delay for some > > > simulation needs, then you'd probably need a more complex structure > > > where you can provide a whole sequence of operations. And why stop at > > > keys, including mouse movement & buttons to. > > > > > > { "execute": "sendinput", "arguments": { > > > "sequence" : [ > > > { "event": "keypress", "keycode": 0x31 }, > > > { "event": "keypress", "keycode": 0x75 }, > > > { "event": "wait", "delay": 100 }, > > > { "event": "mousepress", "button": 1 }, > > > { "event": "mousemove", "xdelta": 1, "ydelta": 1 }, > > > { "event": "keyrelease", "keycode": 0x31 }, > > > { "event": "wait", "delay": 100 }, > > > { "event": "keyrelease", "keycode": 0x75 }, > > > { "event": "mousepos", "x": 102, "y": 102 }, > > > ] > > > } } > > > > > > This is getting kind of advanced now. Whether we need this vs the simpler > > > sendkey, mouse_move, etc command comes down to whether we need ability to > > > set precise delays between events. We could stick with the individual > > > simple commands & add a advanced one alter > > > > Yeah, agreed and I think the current design (ie. hold_time) is simpler. > > When I said stick with simpler sendkey, I mean the style I outlined > without any hold time at all > > > > > > { "execute": "sendkey", "arguments": { "keycode": 0x31, "down": true } } > > If its good enough for VNC & SPICE, it should be good enough for most > monitor users, even more so since QMP communication is usually over a > low latency UNIX domain socket. I think hold time is a flawed concept > as currently provide since it prevents up/down interleaving by sending > a sequence of QMP commands. I don't want to suck the QMP interface because of that, but the problem here is how to maintain the user monitor's current behavior. The best option is to wait for the user monitor/QMP split. This way the hold time could be a feature of the user monitor command only. Or, we could add a new command. But this would just be a workaround..
2010/7/21 Luiz Capitulino <lcapitulino@redhat.com>:
> Do you need this for 0.13? I think the development window is already closed.
Was it announced somewhere?
On Thu, 22 Jul 2010 23:17:39 +0200 Artyom Tarasenko <atar4qemu@googlemail.com> wrote: > 2010/7/21 Luiz Capitulino <lcapitulino@redhat.com>: > > > Do you need this for 0.13? I think the development window is already closed. > > Was it announced somewhere? I heard it on the conf call and iirc it was said on some thread too, but I think it will only be official when we get the first -rc.
diff --git a/monitor.c b/monitor.c index d5377de..082c29d 100644 --- a/monitor.c +++ b/monitor.c @@ -1614,7 +1614,7 @@ static void release_keys(void *opaque) } } -static void do_sendkey(Monitor *mon, const QDict *qdict) +static int do_sendkey(Monitor *mon, const QDict *qdict, QObject **ret_data) { char keyname_buf[16]; char *separator; @@ -1636,18 +1636,18 @@ static void do_sendkey(Monitor *mon, const QDict *qdict) if (keyname_len > 0) { pstrcpy(keyname_buf, sizeof(keyname_buf), string); if (keyname_len > sizeof(keyname_buf) - 1) { - monitor_printf(mon, "invalid key: '%s...'\n", keyname_buf); - return; + qerror_report(QERR_INVALID_KEY, keyname_buf); + return -1; } if (i == MAX_KEYCODES) { - monitor_printf(mon, "too many keys\n"); - return; + qerror_report(QERR_TOO_MANY_KEYS); + return -1; } keyname_buf[keyname_len] = 0; keycode = get_keycode(keyname_buf); if (keycode < 0) { - monitor_printf(mon, "unknown key: '%s'\n", keyname_buf); - return; + qerror_report(QERR_UNKNOWN_KEY, keyname_buf); + return -1; } keycodes[i++] = keycode; } @@ -1666,6 +1666,7 @@ static void do_sendkey(Monitor *mon, const QDict *qdict) /* delayed key up events */ qemu_mod_timer(key_timer, qemu_get_clock(vm_clock) + muldiv64(get_ticks_per_sec(), hold_time, 1000)); + return 0; } static int mouse_button_state; diff --git a/qemu-monitor.hx b/qemu-monitor.hx index f7e46c4..8b6cf09 100644 --- a/qemu-monitor.hx +++ b/qemu-monitor.hx @@ -532,7 +532,8 @@ ETEXI .args_type = "string: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, + .user_print = monitor_user_noop, + .mhandler.cmd_new = do_sendkey, }, STEXI @@ -549,6 +550,25 @@ sendkey ctrl-alt-f1 This command is useful to send keys that your graphical user interface intercepts at low level, such as @code{ctrl-alt-f1} in X Window. ETEXI +SQMP +sendkey +------- + +Send keys to the emulator. + +Arguments: + +- "string": key names or key codes in hexadecimal format separated by dashes (json-string) +- "hold_time": duration in milliseconds to hold the keys down (json-int, optional, default=100) + +Example: + +-> { "execute": "sendkey", "arguments": { "string": "ctrl-alt-f1" } } +<- { "return": {} } + +Note: if several keys are given they are pressed simultaneously. + +EQMP { .name = "system_reset", diff --git a/qerror.c b/qerror.c index 44d0bf8..34a698e 100644 --- a/qerror.c +++ b/qerror.c @@ -117,6 +117,10 @@ static const QErrorStringTable qerror_table[] = { .desc = "Invalid block format '%(name)'", }, { + .error_fmt = QERR_INVALID_KEY, + .desc = "Invalid key '%(key)...'", + }, + { .error_fmt = QERR_INVALID_PARAMETER, .desc = "Invalid parameter '%(name)'", }, @@ -185,10 +189,18 @@ static const QErrorStringTable qerror_table[] = { .desc = "Too many open files", }, { + .error_fmt = QERR_TOO_MANY_KEYS, + .desc = "Too many keys", + }, + { .error_fmt = QERR_UNDEFINED_ERROR, .desc = "An undefined error has ocurred", }, { + .error_fmt = QERR_UNKNOWN_KEY, + .desc = "Unknown key '%(key)'", + }, + { .error_fmt = QERR_VNC_SERVER_FAILED, .desc = "Could not start VNC server on %(target)", }, diff --git a/qerror.h b/qerror.h index 77ae574..a23630c 100644 --- a/qerror.h +++ b/qerror.h @@ -103,6 +103,9 @@ QError *qobject_to_qerror(const QObject *obj); #define QERR_INVALID_BLOCK_FORMAT \ "{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }" +#define QERR_INVALID_KEY \ + "{ 'class': 'InvalidKey', 'data': { 'key': %s } }" + #define QERR_INVALID_PARAMETER \ "{ 'class': 'InvalidParameter', 'data': { 'name': %s } }" @@ -154,9 +157,15 @@ QError *qobject_to_qerror(const QObject *obj); #define QERR_TOO_MANY_FILES \ "{ 'class': 'TooManyFiles', 'data': {} }" +#define QERR_TOO_MANY_KEYS \ + "{ 'class': 'TooManyKeys', 'data': {} }" + #define QERR_UNDEFINED_ERROR \ "{ 'class': 'UndefinedError', 'data': {} }" +#define QERR_UNKNOWN_KEY \ + "{ 'class': 'UnknownKey', 'data': { 'key': %s } }" + #define QERR_VNC_SERVER_FAILED \ "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
Signed-off-by: Michael Goldish <mgoldish@redhat.com> --- monitor.c | 15 ++++++++------- qemu-monitor.hx | 22 +++++++++++++++++++++- qerror.c | 12 ++++++++++++ qerror.h | 9 +++++++++ 4 files changed, 50 insertions(+), 8 deletions(-)