Re: [Qemu-devel] [RESEND PATCH V3] qemu-img info: show nocow info

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1714 bytes --]

On 11/21/2014 09:25 AM, Kevin Wolf wrote:
> Am 30.07.2014 um 04:55 hat Chunyan Liu geschrieben:
>> Add nocow info in 'qemu-img info' output to show whether the file
>> currently has NOCOW flag set or not.
>>
>> Signed-off-by: Chunyan Liu <cyliu@suse.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> ---
>> Resend for QEMU 2.2. Change json version comment. Add Reviewed-by.
>>
>>  block/qapi.c         | 25 +++++++++++++++++++++++++
>>  qapi/block-core.json |  5 ++++-
>>  2 files changed, 29 insertions(+), 1 deletion(-)
> 
> Unfortunately, I have never been CCed on this patch and I saw the code
> it added only now. I think it doesn't work correctly and has the wrong
> design, so I would prefer to revert it for 2.2 rather than making a bad
> API stable.

Even though I reviewed the original, you make some valid points, and I
can agree to a revert to avoid baking in a mistake.

>> @@ -126,7 +128,8 @@
>>             '*backing-filename': 'str', '*full-backing-filename': 'str',
>>             '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'],
>>             '*backing-image': 'ImageInfo',
>> -           '*format-specific': 'ImageInfoSpecific' } }
>> +           '*format-specific': 'ImageInfoSpecific',
>> +           '*nocow': 'bool' } }
> 
> As this field makes only sense for raw-posix, it should be moved to
> ImageInfoSpecific. ("format-specific" is also an unfortunate misnomer,
> it should be "driver-specific", but it's too late to fix that.)

Yes, that alone is a compelling reason to back this out and get the
interface right.


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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]