On Thu, Apr 12, 2018 at 03:11:53PM +0100, Elie Tournier wrote: > Hello, > > On Tue, Apr 10, 2018 at 03:33:35PM +0200, Gerd Hoffmann wrote: > > > # @off: Disable OpenGL (default). > > Just to be sure, I have to add @ in front of all parameter, right? > > > > > > > > + # '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 ... > > Is it possible to upstream this change before 2.12 release?
No need to hurry with this. QMP doesn't see the structs, so no compatibility problem here. gl=on cmd line option was present in 2.11 + older already, so that must continue to work no matter whenever this makes 2.12 or not. cheers, Gerd