qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 1/2 for-2.12?] qapi: Parameter gl of DisplayT


From: Gerd Hoffmann
Subject: Re: [Qemu-devel] [PATCH v2 1/2 for-2.12?] qapi: Parameter gl of DisplayType now accept an enum
Date: Tue, 10 Apr 2018 15:33:35 +0200
User-agent: NeoMutt/20180323

> # @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...

> 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.

Yes, right now the struct is only used to store the parsed command line
opts, so no effect on QMP.

Plan for the future is to also parse command line options with generic
qapi/json code instead of the home-grown parser, but that switch didn't
happen yet.

> 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).

gl=on is older, so that must continue to work.  Making both "on" and
"auto" work isn't a problem for our home-grown parser (aka
parse_display() in vl.c).  But having quirks like this makes the switch
to generic parser code more difficuilt, so I'd prefer to avoid that ...

cheers,
  Gerd




reply via email to

[Prev in Thread] Current Thread [Next in Thread]