[Qemu-devel] [PATCH for-2.5 0/2] Promote 'input-send-event' to supported

[Qemu-devel] [PATCH for-2.5 0/2] Promote 'input-send-event' to supported

[Eric Blake]

Now that we have introspection, we should consider what we
are exposing to the end user, and try to avoid spurious changes
in future versions.  There has been debate about switching the
qapi code generator to produce different output for 'CamelCase'
enum values; the easiest way to ensure this doesn't bite us in
later releases is to rename the QMP spellings now in a way that
doesn't impact the C spelling.  Promoting the experimental
command to supported will be the easiest way to learn about the
switch in spelling.

This isn't quite a bug fix, and it won't hurt us if we delay to
2.6 (the command remains expermental for one more release).  But
delaying will let the churn show up in introspection results,
so I'm arguing that it would be nice to get this series into 2.5;
even though any proposed changes to the generator won't go into
effect until 2.6.

Eric Blake (2):
  input: Avoid CamelCase in InputEvent enums
  input: Promote 'input-send-event' to stable API

 qapi-schema.json | 14 ++++++--------
 qmp-commands.hx  | 24 +++++++++++-------------
 ui/input.c       |  4 ++--
 3 files changed, 19 insertions(+), 23 deletions(-)

-- 
2.4.3

[Qemu-devel] [PATCH for-2.5 1/2] input: Avoid CamelCase in InputEvent enums

[Eric Blake]

Our documentation states that we prefer 'lower-case', rather than
'CamelCase', for qapi enum values.  The InputButton and InputAxis
enums violated this convention.  However, they are currently used
primarily for generating code that is used internally; their only
exposure through QMP is via the experimental 'x-input-send-event'
command.  Since this is experimental, changing the QMP wire format
for that command is acceptable.

The existing c_enum_const() code in the generator for turning the
enum names into C constants happens to munge both pre- and
post-patch spellings to the same C code, which means making the
change now touches very few files.  But we are considering a
future patch which would change c_enum_const() to use
c_name(V).upper() rather than camel_to_upper(), which would render
'WheelUp' as INPUT_BUTTON_WHEELUP instead of its current
INPUT_BUTTON_WHEEL_UP.  Making the change to the enum values now
will isolate these enums from any impact if the generator munging
algorithm is changed.

Note that SDL code uses the spelling WHEELUP rather than WHEEL_UP
in its constants, but that shouldn't drive our decision.

Fix a typo in the qapi docs for InputAxis while at it.

CC: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 qapi-schema.json | 6 +++---
 qmp-commands.hx  | 8 ++++----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index c3f95ab..ecefb17 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3531,17 +3531,17 @@
 # Since: 2.0
 ##
 { 'enum'  : 'InputButton',
-  'data'  : [ 'Left', 'Middle', 'Right', 'WheelUp', 'WheelDown' ] }
+  'data'  : [ 'left', 'middle', 'right', 'wheel-up', 'wheel-down' ] }

 ##
-# @InputButton
+# @InputAxis
 #
 # Position axis of a pointer input device (mouse, tablet).
 #
 # Since: 2.0
 ##
 { 'enum'  : 'InputAxis',
-  'data'  : [ 'X', 'Y' ] }
+  'data'  : [ 'x', 'y' ] }

 ##
 # @InputKeyEvent
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 02c0c5b..8f25fe0 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4504,13 +4504,13 @@ Press left mouse button.
 -> { "execute": "x-input-send-event",
     "arguments": { "console": 0,
                    "events": [ { "type": "btn",
-                    "data" : { "down": true, "button": "Left" } } ] } }
+                    "data" : { "down": true, "button": "left" } } ] } }
 <- { "return": {} }

 -> { "execute": "x-input-send-event",
     "arguments": { "console": 0,
                    "events": [ { "type": "btn",
-                    "data" : { "down": false, "button": "Left" } } ] } }
+                    "data" : { "down": false, "button": "left" } } ] } }
 <- { "return": {} }

 Example (2):
@@ -4533,8 +4533,8 @@ Move mouse pointer to absolute coordinates (20000, 400).

 -> { "execute": "x-input-send-event" ,
   "arguments": { "console": 0, "events": [
-               { "type": "abs", "data" : { "axis": "X", "value" : 20000 } },
-               { "type": "abs", "data" : { "axis": "Y", "value" : 400 } } ] } }
+               { "type": "abs", "data" : { "axis": "x", "value" : 20000 } },
+               { "type": "abs", "data" : { "axis": "y", "value" : 400 } } ] } }
 <- { "return": {} }

 EQMP
-- 
2.4.3

Re: [Qemu-devel] [PATCH for-2.5 1/2] input: Avoid CamelCase in InputEvent enums

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> Our documentation states that we prefer 'lower-case', rather than
> 'CamelCase', for qapi enum values.  The InputButton and InputAxis
> enums violated this convention.  However, they are currently used
> primarily for generating code that is used internally; their only
> exposure through QMP is via the experimental 'x-input-send-event'
> command.  Since this is experimental, changing the QMP wire format
> for that command is acceptable.
>
> The existing c_enum_const() code in the generator for turning the
> enum names into C constants happens to munge both pre- and
> post-patch spellings to the same C code, which means making the
> change now touches very few files.  But we are considering a
> future patch which would change c_enum_const() to use
> c_name(V).upper() rather than camel_to_upper(), which would render
> 'WheelUp' as INPUT_BUTTON_WHEELUP instead of its current
> INPUT_BUTTON_WHEEL_UP.  Making the change to the enum values now
> will isolate these enums from any impact if the generator munging
> algorithm is changed.
>
> Note that SDL code uses the spelling WHEELUP rather than WHEEL_UP
> in its constants, but that shouldn't drive our decision.
>
> Fix a typo in the qapi docs for InputAxis while at it.
>
> CC: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>

I can take this through my tree if Gerd doesn't object.

Re: [Qemu-devel] [PATCH for-2.5 1/2] input: Avoid CamelCase in InputEvent enums

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1114 bytes --]

On 11/12/2015 01:23 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> Our documentation states that we prefer 'lower-case', rather than
>> 'CamelCase', for qapi enum values.  The InputButton and InputAxis
>> enums violated this convention.  However, they are currently used
>> primarily for generating code that is used internally; their only
>> exposure through QMP is via the experimental 'x-input-send-event'
>> command.  Since this is experimental, changing the QMP wire format
>> for that command is acceptable.

> 
> I can take this through my tree if Gerd doesn't object.

If we aren't changing the spelling of x-, then we shouldn't change the
spelling of the arguments to x-.

How about this instead: add a FIXME comment to qapi-schema.json that
documents that the names of the enum values will likely be changed at
the (later) point where x- is removed.  In the meantime, if we add any
namespace enforcing, we'll just have to whitelist these two.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

[Qemu-devel] [PATCH for-2.5 2/2] input: Promote 'input-send-event' to stable API

[Eric Blake]

We've had 'x-input-send-event' since 2.3, with no further
changes to the interface other than tweaks in the previous patch
to the spelling of the enum constants ('X' and 'WheelUp' changed
to 'x' and 'wheel-up').

What's more, changing the spelling of enum constants is not easy
to introspect prior to 2.5; so a client that was relying on the
experimental command can't easily tell which spelling is expected.
But 'query-commands' works in all qemu versions that supported
the command, so renaming the command now makes it an easy thing
to determine which spelling of the enum values to use.

Thus, it's time to promote this interface to stable.

CC: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 qapi-schema.json |  8 +++-----
 qmp-commands.hx  | 16 +++++++---------
 ui/input.c       |  4 ++--
 3 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index ecefb17..f99d413 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3605,7 +3605,7 @@
               'abs'     : 'InputMoveEvent' } }

 ##
-# @x-input-send-event
+# @input-send-event
 #
 # Send input event(s) to guest.
 #
@@ -3627,12 +3627,10 @@
 #
 # Returns: Nothing on success.
 #
-# Since: 2.2
-#
-# Note: this command is experimental, and not a stable API.
+# Since: 2.5
 #
 ##
-{ 'command': 'x-input-send-event',
+{ 'command': 'input-send-event',
   'data': { '*console':'int', 'events': [ 'InputEvent' ] } }

 ##
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 8f25fe0..cde7505 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4475,13 +4475,13 @@ Example:
 EQMP

     {
-        .name       = "x-input-send-event",
+        .name       = "input-send-event",
         .args_type  = "console:i?,events:q",
-        .mhandler.cmd_new = qmp_marshal_x_input_send_event,
+        .mhandler.cmd_new = qmp_marshal_input_send_event,
     },

 SQMP
-@x-input-send-event
+@input-send-event
 -----------------

 Send input event to guest.
@@ -4495,19 +4495,17 @@ The consoles are visible in the qom tree, under
 /backend/console[$index]. They have a device link and head property, so
 it is possible to map which console belongs to which device and display.

-Note: this command is experimental, and not a stable API.
-
 Example (1):

 Press left mouse button.

--> { "execute": "x-input-send-event",
+-> { "execute": "input-send-event",
     "arguments": { "console": 0,
                    "events": [ { "type": "btn",
                     "data" : { "down": true, "button": "left" } } ] } }
 <- { "return": {} }

--> { "execute": "x-input-send-event",
+-> { "execute": "input-send-event",
     "arguments": { "console": 0,
                    "events": [ { "type": "btn",
                     "data" : { "down": false, "button": "left" } } ] } }
@@ -4517,7 +4515,7 @@ Example (2):

 Press ctrl-alt-del.

--> { "execute": "x-input-send-event",
+-> { "execute": "input-send-event",
      "arguments": { "console": 0, "events": [
         { "type": "key", "data" : { "down": true,
           "key": {"type": "qcode", "data": "ctrl" } } },
@@ -4531,7 +4529,7 @@ Example (3):

 Move mouse pointer to absolute coordinates (20000, 400).

--> { "execute": "x-input-send-event" ,
+-> { "execute": "input-send-event" ,
   "arguments": { "console": 0, "events": [
                { "type": "abs", "data" : { "axis": "x", "value" : 20000 } },
                { "type": "abs", "data" : { "axis": "y", "value" : 400 } } ] } }
diff --git a/ui/input.c b/ui/input.c
index a0f9873..59560f0 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -125,8 +125,8 @@ qemu_input_find_handler(uint32_t mask, QemuConsole *con)
     return NULL;
 }

-void qmp_x_input_send_event(bool has_console, int64_t console,
-                            InputEventList *events, Error **errp)
+void qmp_input_send_event(bool has_console, int64_t console,
+                          InputEventList *events, Error **errp)
 {
     InputEventList *e;
     QemuConsole *con;
-- 
2.4.3

Re: [Qemu-devel] [PATCH for-2.5 2/2] input: Promote 'input-send-event' to stable API

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> We've had 'x-input-send-event' since 2.3, with no further
> changes to the interface other than tweaks in the previous patch
> to the spelling of the enum constants ('X' and 'WheelUp' changed
> to 'x' and 'wheel-up').
>
> What's more, changing the spelling of enum constants is not easy
> to introspect prior to 2.5; so a client that was relying on the
> experimental command can't easily tell which spelling is expected.
> But 'query-commands' works in all qemu versions that supported
> the command, so renaming the command now makes it an easy thing
> to determine which spelling of the enum values to use.
>
> Thus, it's time to promote this interface to stable.

The x- goes back to commit df5b2ad:

    input: move input-send-event into experimental namespace
    
    Ongoing discussions on how we are going to specify the console,
    so tag the command as experiental so we can refine things in
    the 2.3 development cycle.
    
Have we settled "how we are going to specify the console"?  If yes,
commit, please.  If no, I'm afraid the command should stay experimental.

Re: [Qemu-devel] [PATCH for-2.5 2/2] input: Promote 'input-send-event' to stable API

[Gerd Hoffmann]

On Do, 2015-11-12 at 09:23 +0100, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
> > We've had 'x-input-send-event' since 2.3, with no further
> > changes to the interface other than tweaks in the previous patch
> > to the spelling of the enum constants ('X' and 'WheelUp' changed
> > to 'x' and 'wheel-up').
> >
> > What's more, changing the spelling of enum constants is not easy
> > to introspect prior to 2.5; so a client that was relying on the
> > experimental command can't easily tell which spelling is expected.
> > But 'query-commands' works in all qemu versions that supported
> > the command, so renaming the command now makes it an easy thing
> > to determine which spelling of the enum values to use.
> >
> > Thus, it's time to promote this interface to stable.
> 
> The x- goes back to commit df5b2ad:
> 
>     input: move input-send-event into experimental namespace
>     
>     Ongoing discussions on how we are going to specify the console,
>     so tag the command as experiental so we can refine things in
>     the 2.3 development cycle.
>     
> Have we settled "how we are going to specify the console"?  If yes,
> commit, please.  If no, I'm afraid the command should stay experimental.

Good question.  I don't think so.

IIRC the question was whenever we'll leave it as-is (console=<index>),
or whenever we'll do something like display=<id>,head=<nr> instead.

The latter would be consistent with how we are doing input routing, i.e.
grouping display and input devices to a seat for multiseat setups (see
docs/multiseat.txt for more details).

The consoles are already present in the qom tree
as /backend/console[<index>] nodes, and they have device + head
children.  So qom users can map console=<index> to
display=<id>,head=<nr> and visa versa already.  So from a functionality
point of view it doesn't really matter, it is largely a matter of
taste ...

cheers,
  Gerd

Re: [Qemu-devel] [PATCH for-2.5 2/2] input: Promote 'input-send-event' to stable API

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

> On Do, 2015-11-12 at 09:23 +0100, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>> 
>> > We've had 'x-input-send-event' since 2.3, with no further
>> > changes to the interface other than tweaks in the previous patch
>> > to the spelling of the enum constants ('X' and 'WheelUp' changed
>> > to 'x' and 'wheel-up').
>> >
>> > What's more, changing the spelling of enum constants is not easy
>> > to introspect prior to 2.5; so a client that was relying on the
>> > experimental command can't easily tell which spelling is expected.
>> > But 'query-commands' works in all qemu versions that supported
>> > the command, so renaming the command now makes it an easy thing
>> > to determine which spelling of the enum values to use.
>> >
>> > Thus, it's time to promote this interface to stable.
>> 
>> The x- goes back to commit df5b2ad:
>> 
>>     input: move input-send-event into experimental namespace
>>     
>>     Ongoing discussions on how we are going to specify the console,
>>     so tag the command as experiental so we can refine things in
>>     the 2.3 development cycle.
>>     
>> Have we settled "how we are going to specify the console"?  If yes,
>> commit, please.  If no, I'm afraid the command should stay experimental.
>
> Good question.  I don't think so.
>
> IIRC the question was whenever we'll leave it as-is (console=<index>),
> or whenever we'll do something like display=<id>,head=<nr> instead.
>
> The latter would be consistent with how we are doing input routing, i.e.
> grouping display and input devices to a seat for multiseat setups (see
> docs/multiseat.txt for more details).
>
> The consoles are already present in the qom tree
> as /backend/console[<index>] nodes, and they have device + head
> children.  So qom users can map console=<index> to
> display=<id>,head=<nr> and visa versa already.  So from a functionality
> point of view it doesn't really matter, it is largely a matter of
> taste ...

The thread leading to the x-:
http://lists.nongnu.org/archive/html/qemu-devel/2014-11/msg03197.html

I'm not sure the console numbers were visible in QOM back then.

We also discussed use of qdev ID.

Let's keep the x- until we've figured this out.