Re: [Qemu-block] [Qemu-devel] [PULL for-2.8 0/3] Block patches for -rc3

[Eric Blake]

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.

> 
>>
>> In the future please make sure all QAPI changes are marked by version.
> 
> Indeed, and I try to flag it in my reviews as often as I notice it.
> 
>> If there tricky changes you can include a statement showing you are
>> aware of QAPI backwards compatibility ("These new options were added in
>> the 2.8 release cycle and can therefore still be changed without
>> breaking backward compatibility").  This will make me confident that
>> you've checked the QAPI changes.
>>
>> Thanks, applied to my staging tree:
>> https://github.com/stefanha/qemu/commits/staging

In my audit of all differences between 2.7 and current state of qapi
.json files, I only found one addition that was lacking versioning
information:

event DEVICE_TRAY_MOVED gained 'id' parameter

I'll submit a patch for that shortly.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature