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

2012-05-31 Thread Paolo Bonzini
Il 30/05/2012 15:26, Luiz Capitulino ha scritto:
  
  qapi-types.h:
  typedef enum KeyCodes
  {
   KEY_CODES_SHIFT = 0,
   KEY_CODES_SHIFT_R = 1,
   KEY_CODES_ALT = 2,
   
   KEY_CODES_ = ..
  
   ^^^ problem should exist here
 That's because you have something like '' in the enum list, right? I think
 we can call it 'less-than', no?

Let's keep some consistency and use less as in ui/vnc_keysym.h.

Since we're at it, let's add the missing ones: prtsc (b7), break (c6),
win (db), win_r (dc), menu (dd), power (de), sleep (df), wake (e3).
Especially win/win_r/menu.

Paolo



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

2012-05-31 Thread Amos Kong

On 31/05/12 19:00, Paolo Bonzini wrote:

Il 30/05/2012 15:26, Luiz Capitulino ha scritto:


qapi-types.h:
typedef enum KeyCodes
{
  KEY_CODES_SHIFT = 0,
  KEY_CODES_SHIFT_R = 1,
  KEY_CODES_ALT = 2,
  
  KEY_CODES_  = ..

  ^^^ problem should exist here

That's because you have something like '' in the enum list, right? I think
we can call it 'less-than', no?


Let's keep some consistency and use less as in ui/vnc_keysym.h.


Yeah! I also identified this.
https://github.com/kongove/QEMU/commit/23f657b0379ecfb151f97f34e34ec1bdfc9a04a2

I would send V2 later, 
https://github.com/kongove/QEMU/commits/qmp/sendkey/v2 (unfinished)



Since we're at it, let's add the missing ones: prtsc (b7), break (c6),
win (db), win_r (dc), menu (dd), power (de), sleep (df), wake (e3).
Especially win/win_r/menu.


Ok, I would do it.


Paolo


--
Amos.



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

2012-05-30 Thread Amos Kong

On 29/05/12 21:24, Luiz Capitulino wrote:

On Tue, 29 May 2012 20:17:53 +0800
Amos Kongak...@redhat.com  wrote:


On 05/29/2012 07:57 PM, Amos Kong wrote:

On 05/25/2012 09:14 PM, Anthony Liguori wrote:

On 05/24/2012 10:51 PM, 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 Kongak...@redhat.com



+## +# @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,

{ execute:sendky, data:{ keys:[ctrl, alt, del],
hold-time:200 } }


Actually, we should do:




{ 'enum': 'KeyCode', 'data': [ 'map', 'exclam', 'at', 'numbersign',
...] }

{ 'command': 'sendkey', 'data': { 'keys': [ 'KeyCode' ],
'*hold-time': 'int' } }


^^^

It doesn't work.  KeyCodeList could not be defined automatically. I
try to add a type definition to make it works, is it ok?


Looks like we don't support enum lists yet, so the right thing to do is to add 
it.
I can do it if you want, or you could give it a try.


I would like to try it.


{ 'enum': 'Keycode',
   'data': [ 'shift', 'shift_r', 'alt', 'alt_r', 'altgr', 'altgr_r',
 ..
 'lf', 'help', 'meta_l', 'meta_r', 'compose' ] }

{ 'type': 'KeyCodes',
   'data': { 'name', 'Keycode' } }

{ 'command': 'sendkey',
   'data': { 'keys': ['KeyCodes'], '*hold-time': 'int' } }


New problems: special character '' could not be added to enum, other
characters are fine.


Shouldn't the enum contain only symbolic names?


qapi-types.h:
typedef enum KeyCodes
{
KEY_CODES_SHIFT = 0,
KEY_CODES_SHIFT_R = 1,
KEY_CODES_ALT = 2,

KEY_CODES_ = ..

^^^ problem should exist here


--
Amos.



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

2012-05-30 Thread Amos Kong
- Original Message -
 On Wed, 30 May 2012 18:17:54 +0800
 Amos Kong ak...@redhat.com wrote:
 
  On 29/05/12 21:24, Luiz Capitulino wrote:
   On Tue, 29 May 2012 20:17:53 +0800
   Amos Kongak...@redhat.com  wrote:
  
   On 05/29/2012 07:57 PM, Amos Kong wrote:
   On 05/25/2012 09:14 PM, Anthony Liguori wrote:
   On 05/24/2012 10:51 PM, 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 Kongak...@redhat.com
  
   +## +# @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,
  
   { execute:sendky, data:{ keys:[ctrl, alt, del],
   hold-time:200 } }
  
   Actually, we should do:
  
  
   { 'enum': 'KeyCode', 'data': [ 'map', 'exclam', 'at',
   'numbersign',
   ...] }
  
   { 'command': 'sendkey', 'data': { 'keys': [ 'KeyCode' ],
   '*hold-time': 'int' } }
  
   ^^^
  
   It doesn't work.  KeyCodeList could not be defined
   automatically. I
   try to add a type definition to make it works, is it ok?
  
   Looks like we don't support enum lists yet, so the right thing to
   do is to add it.
   I can do it if you want, or you could give it a try.
  
  I would like to try it.
 
 You'll have to look in scripts/qapi-types.py and maybe in
 scripts/qapi-commands.py too.

Today I already started to read docs/qapi-code-gen.txt and
'hack' qapi-types.py and qapi-visit.py

Currently the KeycodesList can be generated, I'm trying to
make the change harmonious with 'type' functions.


 Please, don't hesitate to ping me if you have questions.

Ok. As you know, we live in different timezones, we almost could not
touch each other in the work time :)

I would send you email when I find other problem, thanks again!


   { 'enum': 'Keycode',
  'data': [ 'shift', 'shift_r', 'alt', 'alt_r', 'altgr',
  'altgr_r',
..
'lf', 'help', 'meta_l', 'meta_r', 'compose' ] }
  
   { 'type': 'KeyCodes',
  'data': { 'name', 'Keycode' } }
  
   { 'command': 'sendkey',
  'data': { 'keys': ['KeyCodes'], '*hold-time': 'int' } }
  
  
   New problems: special character '' could not be added to enum,
   other
   characters are fine.
  
   Shouldn't the enum contain only symbolic names?
  
  qapi-types.h:
  typedef enum KeyCodes
  {
   KEY_CODES_SHIFT = 0,
   KEY_CODES_SHIFT_R = 1,
   KEY_CODES_ALT = 2,
   
   KEY_CODES_ = ..
  
   ^^^ problem should exist here
 
 That's because you have something like '' in the enum list, right? I
 think
 we can call it 'less-than', no?

yes, it should be ok. Need to convert inputted '' to 'less-than'.
I will try tomorrow.


Amos.





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

2012-05-30 Thread Luiz Capitulino
On Wed, 30 May 2012 18:17:54 +0800
Amos Kong ak...@redhat.com wrote:

 On 29/05/12 21:24, Luiz Capitulino wrote:
  On Tue, 29 May 2012 20:17:53 +0800
  Amos Kongak...@redhat.com  wrote:
 
  On 05/29/2012 07:57 PM, Amos Kong wrote:
  On 05/25/2012 09:14 PM, Anthony Liguori wrote:
  On 05/24/2012 10:51 PM, 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 Kongak...@redhat.com
 
  +## +# @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,
 
  { execute:sendky, data:{ keys:[ctrl, alt, del],
  hold-time:200 } }
 
  Actually, we should do:
 
 
  { 'enum': 'KeyCode', 'data': [ 'map', 'exclam', 'at', 'numbersign',
  ...] }
 
  { 'command': 'sendkey', 'data': { 'keys': [ 'KeyCode' ],
  '*hold-time': 'int' } }
 
  ^^^
 
  It doesn't work.  KeyCodeList could not be defined automatically. I
  try to add a type definition to make it works, is it ok?
 
  Looks like we don't support enum lists yet, so the right thing to do is to 
  add it.
  I can do it if you want, or you could give it a try.
 
 I would like to try it.

You'll have to look in scripts/qapi-types.py and maybe in 
scripts/qapi-commands.py
too. Please, don't hesitate to ping me if you have questions.

  { 'enum': 'Keycode',
 'data': [ 'shift', 'shift_r', 'alt', 'alt_r', 'altgr', 'altgr_r',
   ..
   'lf', 'help', 'meta_l', 'meta_r', 'compose' ] }
 
  { 'type': 'KeyCodes',
 'data': { 'name', 'Keycode' } }
 
  { 'command': 'sendkey',
 'data': { 'keys': ['KeyCodes'], '*hold-time': 'int' } }
 
 
  New problems: special character '' could not be added to enum, other
  characters are fine.
 
  Shouldn't the enum contain only symbolic names?
 
 qapi-types.h:
 typedef enum KeyCodes
 {
  KEY_CODES_SHIFT = 0,
  KEY_CODES_SHIFT_R = 1,
  KEY_CODES_ALT = 2,
  
  KEY_CODES_ = ..
 
  ^^^ problem should exist here

That's because you have something like '' in the enum list, right? I think
we can call it 'less-than', no?



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

2012-05-29 Thread Amos Kong
On 05/25/2012 09:14 PM, Anthony Liguori wrote:
 On 05/24/2012 10:51 PM, 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 Kongak...@redhat.com
 
 +## +# @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,
 
 { execute:sendky, data:{ keys:[ctrl, alt, del], 
 hold-time:200 } }
 
 Actually, we should do:
 
 { 'enum': 'KeyCode', 'data': [ 'map', 'exclam', 'at', 'numbersign',
 ...] }
 
 { 'command': 'sendkey', 'data': { 'keys': [ 'KeyCode' ],
 '*hold-time': 'int' } }
 
 That also has the benefit of making it clear what symbolic names of
 the keycodes we support are.


Talked with Anthony in IRC, paste a summary.

There is a definition of supported keys in monitor.c (key_defs[]),
we need to add all the items to enum.

There is a macro in monitor.c (key_defs[]), just ignore it.
 #if defined(TARGET_SPARC)  !defined(TARGET_SPARC64)

If we use enum, we don't need to check the keycodes in qmp_sendkey(),
and key-names need to be converted to keycodes in hmp_sendkey().

The keycodes are not consecutive, enum values do not need to be the
keysym values, use a different table to map enum names to keysym values.


-- 
Amos.



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

2012-05-29 Thread Amos Kong
On 05/29/2012 07:57 PM, Amos Kong wrote:
 On 05/25/2012 09:14 PM, Anthony Liguori wrote:
 On 05/24/2012 10:51 PM, 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 Kongak...@redhat.com

 +## +# @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,

 { execute:sendky, data:{ keys:[ctrl, alt, del], 
 hold-time:200 } }

 Actually, we should do:


 { 'enum': 'KeyCode', 'data': [ 'map', 'exclam', 'at', 'numbersign',
 ...] }

 { 'command': 'sendkey', 'data': { 'keys': [ 'KeyCode' ],
 '*hold-time': 'int' } }

^^^

It doesn't work.  KeyCodeList could not be defined automatically. I
try to add a type definition to make it works, is it ok?

{ 'enum': 'Keycode',
  'data': [ 'shift', 'shift_r', 'alt', 'alt_r', 'altgr', 'altgr_r',
..
'lf', 'help', 'meta_l', 'meta_r', 'compose' ] }

{ 'type': 'KeyCodes',
  'data': { 'name', 'Keycode' } }

{ 'command': 'sendkey',
  'data': { 'keys': ['KeyCodes'], '*hold-time': 'int' } }


New problems: special character '' could not be added to enum, other
characters are fine.



 That also has the benefit of making it clear what symbolic names of
 the keycodes we support are.
 
 
 Talked with Anthony in IRC, paste a summary.
 
 There is a definition of supported keys in monitor.c (key_defs[]),
 we need to add all the items to enum.
 
 There is a macro in monitor.c (key_defs[]), just ignore it.
  #if defined(TARGET_SPARC)  !defined(TARGET_SPARC64)
 
 If we use enum, we don't need to check the keycodes in qmp_sendkey(),
 and key-names need to be converted to keycodes in hmp_sendkey().
 
 The keycodes are not consecutive, enum values do not need to be the
 keysym values, use a different table to map enum names to keysym values.
 
 


-- 
Amos.



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

2012-05-29 Thread Luiz Capitulino
On Tue, 29 May 2012 20:17:53 +0800
Amos Kong ak...@redhat.com wrote:

 On 05/29/2012 07:57 PM, Amos Kong wrote:
  On 05/25/2012 09:14 PM, Anthony Liguori wrote:
  On 05/24/2012 10:51 PM, 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 Kongak...@redhat.com
 
  +## +# @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,
 
  { execute:sendky, data:{ keys:[ctrl, alt, del], 
  hold-time:200 } }
 
  Actually, we should do:
 
 
  { 'enum': 'KeyCode', 'data': [ 'map', 'exclam', 'at', 'numbersign',
  ...] }
 
  { 'command': 'sendkey', 'data': { 'keys': [ 'KeyCode' ],
  '*hold-time': 'int' } }
 
 ^^^
 
 It doesn't work.  KeyCodeList could not be defined automatically. I
 try to add a type definition to make it works, is it ok?

Looks like we don't support enum lists yet, so the right thing to do is to add 
it.
I can do it if you want, or you could give it a try.

 
 { 'enum': 'Keycode',
   'data': [ 'shift', 'shift_r', 'alt', 'alt_r', 'altgr', 'altgr_r',
 ..
 'lf', 'help', 'meta_l', 'meta_r', 'compose' ] }
 
 { 'type': 'KeyCodes',
   'data': { 'name', 'Keycode' } }
 
 { 'command': 'sendkey',
   'data': { 'keys': ['KeyCodes'], '*hold-time': 'int' } }
 
 
 New problems: special character '' could not be added to enum, other
 characters are fine.

Shouldn't the enum contain only symbolic names?



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

2012-05-25 Thread Amos Kong

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 Kongak...@redhat.com



+##
+# @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?



{ execute:sendkey, data:{ keys:[ctrl, alt, del],
hold-time:200 } }


How to make it compatible with hum command? Still use 'ctrl-alt-delete'
for hum, separate keys and generate an array in hum_sendkey() before
calling qmp_sendkey()?


And I'm know clear about how to define command in qapi-schema.json,
I didn't find exist example, any clue?

  { 'command': 'sendkey', 'data': { 'keys': [ 'str'], '*hold-time': 
'int'} }

--
  { 'type': 'Key', 'data': {'name': 'str'} }
  { 'command': 'sendkey', 'data': { 'keys': [ 'Key' ], '*hold-time': 
'int'} }



--
Amos.



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

2012-05-25 Thread Daniel P. Berrange
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 Kongak...@redhat.com
 
 +##
 +# @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.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|



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

2012-05-25 Thread Markus Armbruster
Daniel P. Berrange berra...@redhat.com writes:

 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 Kongak...@redhat.com
 
 +##
 +# @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.

We've followed this rule in QMP so far.



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

2012-05-25 Thread Luiz Capitulino
On Fri, 25 May 2012 14:20:33 +0800
Amos Kong ak...@redhat.com 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 Kongak...@redhat.com
 
  +##
  +# @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?

I agree it's better.

  { execute:sendkey, data:{ keys:[ctrl, alt, del],
  hold-time:200 } }
 
 How to make it compatible with hum command? Still use 'ctrl-alt-delete'
 for hum, separate keys and generate an array in hum_sendkey() before
 calling qmp_sendkey()?

Yes. Basically, you'll move the parsing code to hmp_sendkey() and build
a QList to be passed to qmp_sendkey(). You can take a look at
tests/check-qlist.c for examples on how to work with qlists.

 And I'm know clear about how to define command in qapi-schema.json,
 I didn't find exist example, any clue?
 
{ 'command': 'sendkey', 'data': { 'keys': [ 'str'], '*hold-time': 
 'int'} }
 --
{ 'type': 'Key', 'data': {'name': 'str'} }
{ 'command': 'sendkey', 'data': { 'keys': [ 'Key' ], '*hold-time': 
 'int'} }

Jeff, didn't you implement this?



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

2012-05-25 Thread Luiz Capitulino
On Fri, 25 May 2012 08:34:54 +0100
Daniel P. Berrange berra...@redhat.com 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 Kongak...@redhat.com
  
  +##
  +# @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?



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

2012-05-25 Thread Daniel P. Berrange
On Fri, May 25, 2012 at 10:06:11AM -0300, Luiz Capitulino wrote:
 On Fri, 25 May 2012 08:34:54 +0100
 Daniel P. Berrange berra...@redhat.com 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 Kongak...@redhat.com
   
   +##
   +# @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?

We're not talking about changing the args of the existing HMP command,
and libvirt has no code for a QMP version of sendkey, so there's no
compatibility issue.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|



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

2012-05-25 Thread Anthony Liguori

On 05/24/2012 10:51 PM, 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 Kongak...@redhat.com



+##
+# @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,

{ execute:sendky, data:{ keys:[ctrl, alt, del],
hold-time:200 } }


Actually, we should do:

{ 'enum': 'KeyCode',
  'data': [ 'map', 'exclam', 'at', 'numbersign', ...] }

{ 'command': 'sendkey',
  'data': { 'keys': [ 'KeyCode' ], '*hold-time': 'int' } }

That also has the benefit of making it clear what symbolic names of the keycodes 
we support are.


Regards,

Anthony Liguori








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

2012-05-25 Thread Luiz Capitulino
On Fri, 25 May 2012 14:12:39 +0100
Daniel P. Berrange berra...@redhat.com wrote:

 On Fri, May 25, 2012 at 10:06:11AM -0300, Luiz Capitulino wrote:
  On Fri, 25 May 2012 08:34:54 +0100
  Daniel P. Berrange berra...@redhat.com 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 Kongak...@redhat.com

+##
+# @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?
 
 We're not talking about changing the args of the existing HMP command,
 and libvirt has no code for a QMP version of sendkey, so there's no
 compatibility issue.

Good.



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

2012-05-25 Thread Anthony Liguori

On 05/25/2012 08:06 AM, Luiz Capitulino wrote:

On Fri, 25 May 2012 08:34:54 +0100
Daniel P. Berrangeberra...@redhat.com  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 Kongak...@redhat.com



+##
+# @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.


I agree.  But obviously, changing the parameter syntax is straight forward 
enough.  I think we rat hole when we change the command semantics though.




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?


I don't think libvirt can possibly be assuming that a QMP command exists that 
it's never seen before...  So hopefully this isn't a blind check.  If it is, 
it's a libvirt bug.


Regards,

Anthony Liguori








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

2012-05-25 Thread Jeff Cody
On 05/25/2012 09:00 AM, Luiz Capitulino wrote:
 On Fri, 25 May 2012 14:20:33 +0800
 Amos Kong ak...@redhat.com 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 Kongak...@redhat.com

 +##
 +# @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?
 
 I agree it's better.
 
 { execute:sendkey, data:{ keys:[ctrl, alt, del],
 hold-time:200 } }

 How to make it compatible with hum command? Still use 'ctrl-alt-delete'
 for hum, separate keys and generate an array in hum_sendkey() before
 calling qmp_sendkey()?
 
 Yes. Basically, you'll move the parsing code to hmp_sendkey() and build
 a QList to be passed to qmp_sendkey(). You can take a look at
 tests/check-qlist.c for examples on how to work with qlists.
 
 And I'm know clear about how to define command in qapi-schema.json,
 I didn't find exist example, any clue?

{ 'command': 'sendkey', 'data': { 'keys': [ 'str'], '*hold-time': 
 'int'} }
 --
{ 'type': 'Key', 'data': {'name': 'str'} }
{ 'command': 'sendkey', 'data': { 'keys': [ 'Key' ], '*hold-time': 
 'int'} }
 
 Jeff, didn't you implement this?

Something similar with group snapshots - it had a command that took
an array input.

So I think the schema for what Eric had above would be something like:

{ 'type': 'Sendkey',
  'data': {'key': 'str' } }

{ 'command': 'sendkey',
  'data': { 'keylist': [ 'Sendkey' ], '*hold-time': 'int' } }

(But after typing this I see Anthony has proposed a schema using enums)




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

2012-05-25 Thread Luiz Capitulino
On Fri, 25 May 2012 11:32:01 +0800
Amos Kong ak...@redhat.com 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'

Splitting the args rename to a different patch would make review easier. Also,
please, include a cover letter as patch 0/3.

In general looks good, but apart the argument re-work others suggested I have
some comments below.

 Signed-off-by: Amos Kong ak...@redhat.com
 ---
  hmp-commands.hx  |4 ++--
  hmp.c|   11 +++
  hmp.h|1 +
  monitor.c|   21 ++---
  qapi-schema.json |   20 
  qmp-commands.hx  |   24 
  6 files changed, 68 insertions(+), 13 deletions(-)
 
 diff --git a/hmp-commands.hx b/hmp-commands.hx
 index af18156..afbfa61 100644
 --- a/hmp-commands.hx
 +++ b/hmp-commands.hx
 @@ -502,10 +502,10 @@ 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,
 +.mhandler.cmd = hmp_sendkey,
  },
  
  STEXI
 diff --git a/hmp.c b/hmp.c
 index bb0952e..abb7b59 100644
 --- a/hmp.c
 +++ b/hmp.c
 @@ -947,3 +947,14 @@ void hmp_device_del(Monitor *mon, const QDict *qdict)
  qmp_device_del(id, err);
  hmp_handle_error(mon, err);
  }
 +
 +void hmp_sendkey(Monitor *mon, const QDict *qdict)
 +{
 +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);
 +Error *err = NULL;
 +
 +qmp_sendkey(keys, has_hold_time, hold_time, err);
 +hmp_handle_error(mon, err);
 +}
 diff --git a/hmp.h b/hmp.h
 index 443b812..40de72c 100644
 --- a/hmp.h
 +++ b/hmp.h
 @@ -61,5 +61,6 @@ void hmp_block_job_set_speed(Monitor *mon, const QDict 
 *qdict);
  void hmp_block_job_cancel(Monitor *mon, const QDict *qdict);
  void hmp_migrate(Monitor *mon, const QDict *qdict);
  void hmp_device_del(Monitor *mon, const QDict *qdict);
 +void hmp_sendkey(Monitor *mon, const QDict *qdict);
  
  #endif
 diff --git a/monitor.c b/monitor.c
 index 12a6fe2..238f8da 100644
 --- a/monitor.c
 +++ b/monitor.c
 @@ -1377,14 +1377,12 @@ static void release_keys(void *opaque)
  }
  }
  
 -static void do_sendkey(Monitor *mon, const QDict *qdict)
 +void qmp_sendkey(const char *keys, bool has_hold_time, int64_t hold_time,
 + Error **err)
  {
  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);
  
  if (nb_pending_keycodes  0) {
  qemu_del_timer(key_timer);
 @@ -1394,29 +1392,30 @@ 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);
 +error_set(err, QERR_INVALID_PARAMETER_VALUE, keyname_buf,
 +  keyname_buf);

You should pass key name in the first string.

  return;
  }
  if (i == MAX_KEYCODES) {
 -monitor_printf(mon, too many keys\n);
 +error_set(err, QERR_TOO_MANY_PARAMETERS);
  return;
  }

I know I suggested TOO_MANY_PARAMETERS myself, but having QERR_OVERFLOW
would be better (and we should of course document that in the schema).

On the other hand I wonder if we should limit this, do we limit filenames
for example? Or maybe we should limit it to a bigger size, like 256 bytes.

Your choice.

  keyname_buf[keyname_len] = 0;
  keycode = get_keycode(keyname_buf);
  if (keycode  0) {
 -monitor_printf(mon, unknown key: '%s'\n, keyname_buf);
 +error_set(err, QERR_INVALID_PARAMETER, keyname_buf);
  return;
  }
  keycodes[i++] = keycode;
  }
  if (!separator)
  break;
 -string = separator + 1;
 +keys = separator + 1;
  }
  nb_pending_keycodes = i;
  /* key down events */
 diff --git a/qapi-schema.json 

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

2012-05-24 Thread Amos Kong
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 ak...@redhat.com
---
 hmp-commands.hx  |4 ++--
 hmp.c|   11 +++
 hmp.h|1 +
 monitor.c|   21 ++---
 qapi-schema.json |   20 
 qmp-commands.hx  |   24 
 6 files changed, 68 insertions(+), 13 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index af18156..afbfa61 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -502,10 +502,10 @@ 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,
+.mhandler.cmd = hmp_sendkey,
 },
 
 STEXI
diff --git a/hmp.c b/hmp.c
index bb0952e..abb7b59 100644
--- a/hmp.c
+++ b/hmp.c
@@ -947,3 +947,14 @@ void hmp_device_del(Monitor *mon, const QDict *qdict)
 qmp_device_del(id, err);
 hmp_handle_error(mon, err);
 }
+
+void hmp_sendkey(Monitor *mon, const QDict *qdict)
+{
+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);
+Error *err = NULL;
+
+qmp_sendkey(keys, has_hold_time, hold_time, err);
+hmp_handle_error(mon, err);
+}
diff --git a/hmp.h b/hmp.h
index 443b812..40de72c 100644
--- a/hmp.h
+++ b/hmp.h
@@ -61,5 +61,6 @@ void hmp_block_job_set_speed(Monitor *mon, const QDict 
*qdict);
 void hmp_block_job_cancel(Monitor *mon, const QDict *qdict);
 void hmp_migrate(Monitor *mon, const QDict *qdict);
 void hmp_device_del(Monitor *mon, const QDict *qdict);
+void hmp_sendkey(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/monitor.c b/monitor.c
index 12a6fe2..238f8da 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1377,14 +1377,12 @@ static void release_keys(void *opaque)
 }
 }
 
-static void do_sendkey(Monitor *mon, const QDict *qdict)
+void qmp_sendkey(const char *keys, bool has_hold_time, int64_t hold_time,
+ Error **err)
 {
 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);
 
 if (nb_pending_keycodes  0) {
 qemu_del_timer(key_timer);
@@ -1394,29 +1392,30 @@ 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);
+error_set(err, QERR_INVALID_PARAMETER_VALUE, keyname_buf,
+  keyname_buf);
 return;
 }
 if (i == MAX_KEYCODES) {
-monitor_printf(mon, too many keys\n);
+error_set(err, QERR_TOO_MANY_PARAMETERS);
 return;
 }
 keyname_buf[keyname_len] = 0;
 keycode = get_keycode(keyname_buf);
 if (keycode  0) {
-monitor_printf(mon, unknown key: '%s'\n, keyname_buf);
+error_set(err, QERR_INVALID_PARAMETER, keyname_buf);
 return;
 }
 keycodes[i++] = keycode;
 }
 if (!separator)
 break;
-string = separator + 1;
+keys = separator + 1;
 }
 nb_pending_keycodes = i;
 /* key down events */
diff --git a/qapi-schema.json b/qapi-schema.json
index 2ca7195..d1799bc 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1755,3 +1755,23 @@
 # Since: 0.14.0
 ##
 { 'command': 'device_del', 'data': {'id': 'str'} }
+
+##
+# @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'} }
diff --git 

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

2012-05-24 Thread Eric Blake
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 ak...@redhat.com

 +##
 +# @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,

{ execute:sendky, data:{ keys:[ctrl, alt, del],
hold-time:200 } }

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature