On Tue, Apr 10, 2018 at 08:13:00AM -0500, Eric Blake wrote:
> On 04/10/2018 07:02 AM, Elie Tournier wrote:
> > Signed-off-by: Elie Tournier <elie.tourn...@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).
Fix locally
>
> > + # '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).
It sounds like making the change now is the best option.
So, is 'auto' a better choice than 'on' in the enum?
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3266
> Virtualization: qemu.org | libvirt.org
>
