diff mbox series

[v2,17/31] qapi/qom: Add ObjectOptions for input-*

Message ID 20210224135255.253837-18-kwolf@redhat.com (mailing list archive)
State New, archived
Headers show
Series qapi/qom: QAPIfy --object and object-add | expand

Commit Message

Kevin Wolf Feb. 24, 2021, 1:52 p.m. UTC
This adds a QAPI schema for the properties of the input-* objects.

ui.json cannot be included in qom.json because the storage daemon can't
use it, so move GrabToggleKeys to common.json.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qapi/common.json | 12 ++++++++++
 qapi/qom.json    | 58 ++++++++++++++++++++++++++++++++++++++++++++++++
 qapi/ui.json     | 13 +----------
 3 files changed, 71 insertions(+), 12 deletions(-)

Comments

Eric Blake Feb. 26, 2021, 8:55 p.m. UTC | #1
On 2/24/21 7:52 AM, Kevin Wolf wrote:
> This adds a QAPI schema for the properties of the input-* objects.
> 
> ui.json cannot be included in qom.json because the storage daemon can't
> use it, so move GrabToggleKeys to common.json.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qapi/common.json | 12 ++++++++++
>  qapi/qom.json    | 58 ++++++++++++++++++++++++++++++++++++++++++++++++
>  qapi/ui.json     | 13 +----------
>  3 files changed, 71 insertions(+), 12 deletions(-)
> 

> +##
> +# @InputBarrierProperties:
> +#
> +# Properties for input-barrier objects.
> +#
> +# @name: the screen name as declared in the screens section of barrier.conf
> +#
> +# @server: hostname of the Barrier server (default: "localhost")
> +#
> +# @port: TCP port of the Barrier server (default: "24800")

I can understand this being a string (if non-numeric, it can be treated
as a well-known service name instead), but...

> +#
> +# @x-origin: x coordinate of the leftmost pixel on the guest screen
> +#            (default: "0")

...why are these other fields a string instead of an integer?  But you
are just doing faithful translation of what we already have.

Bummer - our naming for this member implies that it is experimental,
which is a misnomer (it is quite stable, when viewed in tandem with
y-origin).  Not your fault.  Would 'origin-x' and 'origin-y' be any
better as new aliases in a followup patch?

> +#
> +# @y-origin: y coordinate of he topmost pixel on the guest screen (default: "0")

"the", long line

> +#
> +# @width: the width of secondary screen in pixels (default: "1920")
> +#
> +# @height: the height of secondary screen in pixels (default: "1080")
> +#
> +# Since: 4.2
> +##
> +{ 'struct': 'InputBarrierProperties',
> +  'data': { 'name': 'str',
> +            '*server': 'str',
> +            '*port': 'str',
> +            '*x-origin': 'str',
> +            '*y-origin': 'str',
> +            '*width': 'str',
> +            '*height': 'str' } }

Matches ui/input-barrier.c:input_barrier_class_init().

> +
> +##
> +# @InputLinuxProperties:
> +#
> +# Properties for input-linux objects.
> +#
> +# @evdev: the path of the host evdev device to use
> +#
> +# @grab_all: if true, grab is toggled for all devices (e.g. both keyboard and
> +#            mouse) instead of just one device (default: false)

We have inconsistent naming within this object (see grab-toggle); a good
followup would be an alias for 'grab-all'.

> +#
> +# @repeat: enables auto-repeat events (default: false)
> +#
> +# @grab-toggle: the key or key combination that toggles device grab
> +#               (default: ctrl-ctrl)
> +#
> +# Since: 2.6
> +##
> +{ 'struct': 'InputLinuxProperties',
> +  'data': { 'evdev': 'str',
> +            '*grab_all': 'bool',
> +            '*repeat': 'bool',
> +            '*grab-toggle': 'GrabToggleKeys' } }

matches ui/input-linux.c.

> +
>  ##
>  # @IothreadProperties:
>  #
> @@ -689,6 +743,8 @@
>      'filter-redirector',
>      'filter-replay',
>      'filter-rewriter',
> +    'input-barrier',
> +    'input-linux',
>      'iothread',
>      'memory-backend-file',
>      'memory-backend-memfd',
> @@ -741,6 +797,8 @@
>        'filter-redirector':          'FilterRedirectorProperties',
>        'filter-replay':              'NetfilterProperties',
>        'filter-rewriter':            'FilterRewriterProperties',
> +      'input-barrier':              'InputBarrierProperties',
> +      'input-linux':                'InputLinuxProperties',
>        'iothread':                   'IothreadProperties',
>        'memory-backend-file':        'MemoryBackendFileProperties',
>        'memory-backend-memfd':       'MemoryBackendMemfdProperties',

Reviewed-by: Eric Blake <eblake@redhat.com>
Kevin Wolf March 2, 2021, 6:42 p.m. UTC | #2
Am 26.02.2021 um 21:55 hat Eric Blake geschrieben:
> On 2/24/21 7:52 AM, Kevin Wolf wrote:
> > This adds a QAPI schema for the properties of the input-* objects.
> > 
> > ui.json cannot be included in qom.json because the storage daemon can't
> > use it, so move GrabToggleKeys to common.json.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  qapi/common.json | 12 ++++++++++
> >  qapi/qom.json    | 58 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  qapi/ui.json     | 13 +----------
> >  3 files changed, 71 insertions(+), 12 deletions(-)
> > 
> 
> > +##
> > +# @InputBarrierProperties:
> > +#
> > +# Properties for input-barrier objects.
> > +#
> > +# @name: the screen name as declared in the screens section of barrier.conf
> > +#
> > +# @server: hostname of the Barrier server (default: "localhost")
> > +#
> > +# @port: TCP port of the Barrier server (default: "24800")
> 
> I can understand this being a string (if non-numeric, it can be treated
> as a well-known service name instead), but...
> 
> > +#
> > +# @x-origin: x coordinate of the leftmost pixel on the guest screen
> > +#            (default: "0")
> 
> ...why are these other fields a string instead of an integer?  But you
> are just doing faithful translation of what we already have.

I wondered the same. Most properties of the user creatable objects make
sense, but for some, I can't imagine why we thought this was a good
idea.

Well, moving descriptions to the QAPI schema can hopefully help to avoid
introducing new cases in the future because they become more obvious.

> Bummer - our naming for this member implies that it is experimental,
> which is a misnomer (it is quite stable, when viewed in tandem with
> y-origin).  Not your fault.  Would 'origin-x' and 'origin-y' be any
> better as new aliases in a followup patch?

Oh, good point. Makes sense, once the alias series is in.

Kevin
diff mbox series

Patch

diff --git a/qapi/common.json b/qapi/common.json
index b87e7f9039..7c976296f0 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -185,3 +185,15 @@ 
 ##
 { 'enum': 'NetFilterDirection',
   'data': [ 'all', 'rx', 'tx' ] }
+
+##
+# @GrabToggleKeys:
+#
+# Keys to toggle input-linux between host and guest.
+#
+# Since: 4.0
+#
+##
+{ 'enum': 'GrabToggleKeys',
+  'data': [ 'ctrl-ctrl', 'alt-alt', 'shift-shift','meta-meta', 'scrolllock',
+            'ctrl-scrolllock' ] }
diff --git a/qapi/qom.json b/qapi/qom.json
index d5f68b5c89..f8ff322df0 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -444,6 +444,60 @@ 
   'base': 'NetfilterProperties',
   'data': { '*vnet_hdr_support': 'bool' } }
 
+##
+# @InputBarrierProperties:
+#
+# Properties for input-barrier objects.
+#
+# @name: the screen name as declared in the screens section of barrier.conf
+#
+# @server: hostname of the Barrier server (default: "localhost")
+#
+# @port: TCP port of the Barrier server (default: "24800")
+#
+# @x-origin: x coordinate of the leftmost pixel on the guest screen
+#            (default: "0")
+#
+# @y-origin: y coordinate of he topmost pixel on the guest screen (default: "0")
+#
+# @width: the width of secondary screen in pixels (default: "1920")
+#
+# @height: the height of secondary screen in pixels (default: "1080")
+#
+# Since: 4.2
+##
+{ 'struct': 'InputBarrierProperties',
+  'data': { 'name': 'str',
+            '*server': 'str',
+            '*port': 'str',
+            '*x-origin': 'str',
+            '*y-origin': 'str',
+            '*width': 'str',
+            '*height': 'str' } }
+
+##
+# @InputLinuxProperties:
+#
+# Properties for input-linux objects.
+#
+# @evdev: the path of the host evdev device to use
+#
+# @grab_all: if true, grab is toggled for all devices (e.g. both keyboard and
+#            mouse) instead of just one device (default: false)
+#
+# @repeat: enables auto-repeat events (default: false)
+#
+# @grab-toggle: the key or key combination that toggles device grab
+#               (default: ctrl-ctrl)
+#
+# Since: 2.6
+##
+{ 'struct': 'InputLinuxProperties',
+  'data': { 'evdev': 'str',
+            '*grab_all': 'bool',
+            '*repeat': 'bool',
+            '*grab-toggle': 'GrabToggleKeys' } }
+
 ##
 # @IothreadProperties:
 #
@@ -689,6 +743,8 @@ 
     'filter-redirector',
     'filter-replay',
     'filter-rewriter',
+    'input-barrier',
+    'input-linux',
     'iothread',
     'memory-backend-file',
     'memory-backend-memfd',
@@ -741,6 +797,8 @@ 
       'filter-redirector':          'FilterRedirectorProperties',
       'filter-replay':              'NetfilterProperties',
       'filter-rewriter':            'FilterRewriterProperties',
+      'input-barrier':              'InputBarrierProperties',
+      'input-linux':                'InputLinuxProperties',
       'iothread':                   'IothreadProperties',
       'memory-backend-file':        'MemoryBackendFileProperties',
       'memory-backend-memfd':       'MemoryBackendMemfdProperties',
diff --git a/qapi/ui.json b/qapi/ui.json
index d08d72b439..cc1882108b 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -6,6 +6,7 @@ 
 # = Remote desktop
 ##
 
+{ 'include': 'common.json' }
 { 'include': 'sockets.json' }
 
 ##
@@ -1021,18 +1022,6 @@ 
             '*head'  : 'int',
             'events' : [ 'InputEvent' ] } }
 
-##
-# @GrabToggleKeys:
-#
-# Keys to toggle input-linux between host and guest.
-#
-# Since: 4.0
-#
-##
-{ 'enum': 'GrabToggleKeys',
-  'data': [ 'ctrl-ctrl', 'alt-alt', 'shift-shift','meta-meta', 'scrolllock',
-            'ctrl-scrolllock' ] }
-
 ##
 # @DisplayGTK:
 #