Re: [Qemu-devel] [PATCH 3/3] qapi: convert sendkey

[Luiz Capitulino]

On Fri, 25 May 2012 08:34:54 +0100
"Daniel P. Berrange" <address@hidden> wrote:

> On Fri, May 25, 2012 at 02:20:33PM +0800, Amos Kong wrote:
> > On 25/05/12 11:51, Eric Blake wrote:
> > >On 05/24/2012 09:32 PM, Amos Kong wrote:
> > >>Convert 'sendkey' to use. do_sendkey() depends on some variables
> > >>in monitor.c, so reserve qmp_sendkey() to monitor.c
> > >>Rename 'string' to 'keys', rename 'hold_time' to 'hold-time'
> > >>
> > >>Signed-off-by: Amos Kong<address@hidden>
> > >
> > >>+##
> > >>+# @sendkey:
> > >>+#
> > >>+# Send keys to VM.
> > >>+#
> > >>+# @keys: key sequence
> > >>+# @hold-time: time to delay key up events
> > >>+#
> > >>+# Returns: Nothing on success
> > >>+#          If key is unknown or redundant, QERR_INVALID_PARAMETER
> > >>+#          If key is invalid, QERR_INVALID_PARAMETER_VALUE
> > >>+#
> > >>+# Notes: Send @var{keys} to the emulator. @var{keys} could be the name 
> > >>of the
> > >>+#        key or the raw value in either decimal or hexadecimal  format. 
> > >>Use
> > >>+#        @code{-} to press several keys simultaneously.
> > >>+#
> > >>+# Since: 0.14.0
> > >>+##
> > >>+{ 'command': 'sendkey', 'data': {'keys': 'str', '*hold-time': 'int'} }
> > >
> > >Rather than making 'keys' a free-form string where qemu then has to
> > >parse '-' to separate keys, should we instead make it a JSON array?  For
> > >example,
> > 
> > 
> > Anthony, Luiz, Daniel,  what's your opinion?
> 
> Using a JSON array for the key names does seem like the most
> natural way to model this. A good rule of thumb is that the
> implementation of a command should not need to further
> parse the individual parameter values. Using a magic string
> encoding instead of the JSON array requires such extra special
> case parsing.

That's true, and I agree this is better.

Just would like to point out that we can't go too far on improving
HMP-inherited commands, as our goal here is to convert most (or every single)
HMP commands to QMP. If we go far on improving commands, we'll get stuck as we
did some time ago.

Btw, I remember someone saying that when libvirt wants to use a HMP command it
first checks if the command exists in QMP before using passthrough. In that 
case,
how will libvirt behave if we change the arguments?