Re: [Qemu-devel] [PATCH v2 1/2 for-2.12?] qapi: Parameter gl of DisplayType now accept an enum

[Eric Blake <eblake@redhat.com>]

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

On 04/10/2018 07:02 AM, Elie Tournier wrote:
> Signed-off-by: Elie Tournier <elie.tournier@collabora.com>
> ---
>  qapi/ui.json | 21 ++++++++++++++++++++-
>  vl.c         | 10 +++++-----
>  2 files changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 5d01ad4304..c8005867e5 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1019,6 +1019,25 @@
>  { 'struct'  : 'DisplayGTK',
>    'data'    : { '*grab-on-hover' : 'bool' } }
>  
> + ##
> + # @DisplayGLMode:
> + #
> + # Display OpenGL mode.
> + #
> + # 'off'   Disable OpenGL (default).

Wrong format; this should be:

# @off: Disable OpenGL (default).

> + # 'on'    Use OpenGL, pick context type automatically.
> + #         Would better be named 'auto' but is called 'on' for backward
> + #         compatibility with bool type.

See below...

> + # 'core'  Use OpenGL with Core (desktop) Context.
> + # 'es'    Use OpenGL with ES (embedded systems) Context.
> + #
> + # Since: 2.13
> + #
> + ##
> + { 'enum'    : 'DisplayGLMode',
> +   'data'    : [ 'off', 'on', 'core', 'es' ] }
> +
> +
>  ##
>  # @DisplayType:
>  #
> @@ -1048,7 +1067,7 @@
>    'base'    : { 'type'           : 'DisplayType',
>                  '*full-screen'   : 'bool',
>                  '*window-close'  : 'bool',
> -                '*gl'            : 'bool' },
> +                '*gl'            : 'DisplayGLMode' },

DisplayOptions was added in 2.12.  This is a backwards-incompatible
change in QMP (you CANNOT change 'bool' to 'DisplayGLMode' across
releases, because the on-the-wire representation differs; pre-patch it
would be "gl":true, post-patch it is "gl":"on").  So if it affects QMP,
and we want it, this patch either HAS to go in 2.12, or we have to have
more finesse (perhaps by using an 'alternate' type in the QAPI) so that
it remains backwards-compatible.

/me goes and looks at introspection output...

You may be in luck - there is no instance of 'window-close' in the
introspection output, which means 'DisplayType' exists only for ease of
command-line parsing and is not currently used by QMP, so tweaks here
are not affecting the command line.

That said, you can STILL name the enum value something smarter than 'on'
IF you make the change now, for 2.12, given that the QAPI type was only
introduced in 2.12 (you only have to worry about backwards compatibility
if 2.11 already parsed gl=on).

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


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