Re: [Qemu-devel] [PATCH v3 04/12] gtk: add and use DisplayOptions + DisplayGTK

[Eric Blake]

On 02/02/2018 05:10 AM, Gerd Hoffmann wrote:
> Add QAPI DisplayType enum, DisplayOptions union and DisplayGTK struct.
> Switch gtk configuration to use the qapi type.
> 
> Some bookkeeping (fullscreen for example) is done twice now, this is
> temporary until more/all UIs are switched over to qapi configuration.
> 
> Signed-off-by: Gerd Hoffmann <address@hidden>
> ---
>  include/ui/console.h |  9 ++++----
>  ui/gtk.c             | 32 ++++++++++++++++-------------
>  vl.c                 | 23 ++++++++++++++++-----
>  qapi/ui.json         | 58 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 98 insertions(+), 24 deletions(-)
> 

> +++ b/vl.c

>              if (strstart(opts, ",grab_on_hover=", &nextopt)) {
>                  opts = nextopt;
> +                dpy.u.gtk.has_grab_on_hover = true;
>                  if (strstart(opts, "on", &nextopt)) {

Pre-existing: this doesn't flag stuff like "grab_on_hover=only" as
bogus.  But doing further cleanups can be later patches; for example,
Markus' work to unify command-line parsing through QAPI visits.

> +++ b/qapi/ui.json
> @@ -982,3 +982,61 @@
>    'data': { '*device': 'str',
>              '*head'  : 'int',
>              'events' : [ 'InputEvent' ] } }
> +
> +
> +##

Is two blank lines typical here, or just one?

> +# @DisplayNoOpts:
> +#
> +# Empty struct for displays without config options.
> +#
> +# Since: 2.12
> +#
> +##
> +{ 'struct'  : 'DisplayNoOpts',
> +  'data'    : { } }
> +
> +##
> +# @DisplayGTK:
> +#
> +# GTK display options.
> +#
> +# @grab-on-hover: Grab keyboard input on mouse hover.
> +#
> +# Since: 2.12
> +#
> +##
> +{ 'struct'  : 'DisplayGTK',
> +  'data'    : { '*grab-on-hover' : 'bool' } }
> +
> +##
> +# @DisplayType:
> +#
> +# Display (user interface) type.
> +#
> +# Since: 2.12
> +#
> +##
> +{ 'enum'    : 'DisplayType',
> +  'data'    : [ 'none', 'gtk' ] }
> +
> +##
> +# @DisplayOptions:
> +#
> +# Display (user interface) options.
> +#
> +# @type:          Which DisplayType qemu should use.
> +# @full-screen:   Start user interface in fullscreen mode (default: off).
> +# @window-close:  Allow to quit qemu with window close button (default: on).

s/Allow to quit/Allow quitting/

> +# @gl:            Enable OpenGL support (default: off).
> +#
> +# Since: 2.12
> +#
> +##
> +{ 'union'   : 'DisplayOptions',
> +  'base'    : { 'type'           : 'DisplayType',
> +                '*full-screen'   : 'bool',
> +                '*window-close'  : 'bool',
> +                '*gl'            : 'bool' },
> +  'discriminator' : 'type',
> +  'data'    : { 'none'           : 'DisplayNoOpts',
> +                'gtk'            : 'DisplayGTK' } }
> 

Struct looks okay, and grammar tweak is minor, so you can add
Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature