Am 06.12.2016 um 16:43 hat Eric Blake geschrieben: > On 12/06/2016 09:11 AM, Eric Blake wrote: > > >> BlockdevOptionsGluster.debug(-level) does not have "Added in 2.8" so I > >> had to dig through git-blame(1) to verify that it was indeed added in > >> the current release cycle. > > > > Then that implies we should add yet one more patch that adds the > > appropriate versioning information to all the gluster fields added for > > 2.8. My reviewed-by was given on the assumption that debug was in 2.7 > > and that this was a break from 2.7 behavior, but that we already KNOW > > we're breaking blockdev-add between 2.7 and 2.8; while your argument is > > that there is no backwards incompatibility because it was not in 2.7 to > > begin with. I think both reasons are indeed acceptable, but it also > > means that my reason was flawed because of the incomplete documentation. > > I checked again. 'git diff v2.7.0..origin qapi/*.json' shows: > > ## > -# @BlockdevOptionsGluster > +# @BlockdevOptionsGluster: > # > # Driver specific block device options for Gluster > # > @@ -2140,7 +2195,9 @@ > # > # @server: gluster servers description > # > -# @debug-level: #optional libgfapi log level (default '4' which is Error) > +# @debug: #optional libgfapi log level (default '4' which is Error) > +# > +# @logfile: #optional libgfapi log file (default /dev/stderr) > (Since 2.8) > # > # Since: 2.7 > ## > @@ -2148,25 +2205,163 @@ > 'data': { 'volume': 'str', > 'path': 'str', > 'server': ['GlusterServer'], > - '*debug-level': 'int' } } > + '*debug': 'int', > + '*logfile': 'str' } } > > So I was right after all - this IS a backwards-incompatible change (and > NOT something that was introduced only in 2.8) - but I still argue that > the change is appropriate NOW (but not later) because blockdev-add is > the only client of this type in QMP, and that command changed > backwards-incompatibly at the same time.
I don't completely understand the change anyway. 'debug-level' is the better name, and if the command line doesn't agree, change that one or grudgingly accept both. But I think we can just change the command line, it's only a debug option. Kevin
pgplJvUzC4l4A.pgp
Description: PGP signature