[Qemu-devel] [PATCH for-4.1] qcow2: Allow -o compat=v3 during qemu-img amend

[Qemu-devel] [PATCH for-4.1] qcow2: Allow -o compat=v3 during qemu-img amend

[Eric Blake]

Commit b76b4f60 allowed '-o compat=v3' as an alias for the
less-appealing '-o compat=1.1' for 'qemu-img create' since we want to
use the QMP form as much as possible, but forgot to do likewise for
qemu-img amend.  Also, it doesn't help that '-o help' doesn't list our
new preferred spellings.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

I'm arguing that the lack of consistency is a bug, even though the bug
has been present since 2.12.

 block/qcow2.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 2a59eb27febb..039bdc2f7e79 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4823,9 +4823,9 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
             compat = qemu_opt_get(opts, BLOCK_OPT_COMPAT_LEVEL);
             if (!compat) {
                 /* preserve default */
-            } else if (!strcmp(compat, "0.10")) {
+            } else if (!strcmp(compat, "0.10") || !strcmp(compat, "v2")) {
                 new_version = 2;
-            } else if (!strcmp(compat, "1.1")) {
+            } else if (!strcmp(compat, "1.1") || !strcmp(compat, "v3")) {
                 new_version = 3;
             } else {
                 error_setg(errp, "Unknown compatibility level %s", compat);
@@ -5098,7 +5098,7 @@ static QemuOptsList qcow2_create_opts = {
         {
             .name = BLOCK_OPT_COMPAT_LEVEL,
             .type = QEMU_OPT_STRING,
-            .help = "Compatibility level (0.10 or 1.1)"
+            .help = "Compatibility level (v2 [0.10] or v3 [1.1])"
         },
         {
             .name = BLOCK_OPT_BACKING_FILE,
-- 
2.20.1

Re: [Qemu-devel] [PATCH for-4.1] qcow2: Allow -o compat=v3 during qemu-img amend

[Eric Blake]

[-- Attachment #1.1: Type: text/plain, Size: 1898 bytes --]

On 7/5/19 10:28 AM, Eric Blake wrote:
> Commit b76b4f60 allowed '-o compat=v3' as an alias for the
> less-appealing '-o compat=1.1' for 'qemu-img create' since we want to
> use the QMP form as much as possible, but forgot to do likewise for
> qemu-img amend.  Also, it doesn't help that '-o help' doesn't list our
> new preferred spellings.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> 
> I'm arguing that the lack of consistency is a bug, even though the bug
> has been present since 2.12.

I found this bug while chasing down another one: trying to see if we can
now lift our restriction against 'qemu-img resize' on an image with
internal snapshots.  For v3 images, the limitation is artificial (the
spec says every snapshot is required to have an associated size, so you
know what size to change back to when reverting to that snapshot); but
for v2 the limitation is real (the spec did not require tracking image
size, and therefore changing the size meant that you might not be able
to safely revert).  Except that we ALSO have a bug in qemu-img amend:

1. Create a v2 file with internal snapshot. On CentOS 6:
$ qemu-img create -f qcow2 file 1m
$ qemu-img snapshot -c s1 file
2. Check that the internal snapshot header uses the smaller size:
$ od -Ax -j64 -N8 -tx1 file  # Learn the offset for the next command
$ offset=$((0x50000+36))
$ od -Ax -j$offset -N 4 -tx1 file
 => extra field is 0
3. Upgrade it to v3. Using qemu.git master:
$ qemu-img amend -o compat=1.1 file
4. Check the internal snapshot header size:
$ od -Ax -j64 -N8 -tx1 file  # Learn the offset for the next command
$ offset=$((0x50000+36))
$ od -Ax -j$offset -N 4 -tx1 file
 => oops - extra field is still 0, but should now be at least 16.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH for-4.1] qcow2: Allow -o compat=v3 during qemu-img amend

[Eric Blake]

[-- Attachment #1.1: Type: text/plain, Size: 2098 bytes --]

On 7/5/19 10:53 AM, Eric Blake wrote:
> On 7/5/19 10:28 AM, Eric Blake wrote:
>> Commit b76b4f60 allowed '-o compat=v3' as an alias for the
>> less-appealing '-o compat=1.1' for 'qemu-img create' since we want to
>> use the QMP form as much as possible, but forgot to do likewise for
>> qemu-img amend.  Also, it doesn't help that '-o help' doesn't list our
>> new preferred spellings.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>
>> I'm arguing that the lack of consistency is a bug, even though the bug
>> has been present since 2.12.
> 
> I found this bug while chasing down another one: trying to see if we can
> now lift our restriction against 'qemu-img resize' on an image with
> internal snapshots.  For v3 images, the limitation is artificial (the
> spec says every snapshot is required to have an associated size, so you
> know what size to change back to when reverting to that snapshot); but
> for v2 the limitation is real (the spec did not require tracking image
> size, and therefore changing the size meant that you might not be able
> to safely revert).  Except that we ALSO have a bug in qemu-img amend:
> 
> 1. Create a v2 file with internal snapshot. On CentOS 6:
> $ qemu-img create -f qcow2 file 1m
> $ qemu-img snapshot -c s1 file
> 2. Check that the internal snapshot header uses the smaller size:
> $ od -Ax -j64 -N8 -tx1 file  # Learn the offset for the next command
> $ offset=$((0x50000+36))
> $ od -Ax -j$offset -N 4 -tx1 file
>  => extra field is 0
> 3. Upgrade it to v3. Using qemu.git master:
> $ qemu-img amend -o compat=1.1 file
> 4. Check the internal snapshot header size:
> $ od -Ax -j64 -N8 -tx1 file  # Learn the offset for the next command
> $ offset=$((0x50000+36))
> $ od -Ax -j$offset -N 4 -tx1 file
>  => oops - extra field is still 0, but should now be at least 16.
> 


Oh, and 'qemu-img check file' fails to diagnose the v3 image as
violating the qcow2 spec.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH for-4.1] qcow2: Allow -o compat=v3 during qemu-img amend

[Kevin Wolf]

Am 05.07.2019 um 17:28 hat Eric Blake geschrieben:
> Commit b76b4f60 allowed '-o compat=v3' as an alias for the
> less-appealing '-o compat=1.1' for 'qemu-img create' since we want to
> use the QMP form as much as possible, but forgot to do likewise for
> qemu-img amend.  Also, it doesn't help that '-o help' doesn't list our
> new preferred spellings.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>

Thanks, applied to the block branch.

Kevin

Re: [Qemu-devel] [PATCH for-4.1] qcow2: Allow -o compat=v3 during qemu-img amend

[Kevin Wolf]

Am 05.07.2019 um 17:28 hat Eric Blake geschrieben:
> Commit b76b4f60 allowed '-o compat=v3' as an alias for the
> less-appealing '-o compat=1.1' for 'qemu-img create' since we want to
> use the QMP form as much as possible, but forgot to do likewise for
> qemu-img amend.  Also, it doesn't help that '-o help' doesn't list our
> new preferred spellings.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>

This broke qemu-iotests 082. Please send a follow-up to update the
reference output.

Kevin

Re: [Qemu-devel] [PATCH for-4.1] qcow2: Allow -o compat=v3 during qemu-img amend

[Eric Blake]

[-- Attachment #1.1: Type: text/plain, Size: 1130 bytes --]

On 7/8/19 11:51 AM, Kevin Wolf wrote:
> Am 05.07.2019 um 17:28 hat Eric Blake geschrieben:
>> Commit b76b4f60 allowed '-o compat=v3' as an alias for the
>> less-appealing '-o compat=1.1' for 'qemu-img create' since we want to
>> use the QMP form as much as possible, but forgot to do likewise for
>> qemu-img amend.  Also, it doesn't help that '-o help' doesn't list our
>> new preferred spellings.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> This broke qemu-iotests 082. Please send a follow-up to update the
> reference output.

I know that I did not specifically run iotests explicitly for this one
patch, but thought I had at least ran 'make check', since I know we've
been improving lately to run at least some of the iotests.

/me goes and rechecks...

well, 82 is definitely marked 'auto', and group claims that auto tests
will be run by 'make check', but that didn't trigger the failure for me.
 I wonder why?  At least I've posted the fix, as requested.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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