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@xxxxxxxxxx> wrote:
>
> > Signed-off-by: Michael Goldish <mgoldish@xxxxxxxxxx>
>
> 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
--
|: 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 :|
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
