| Message ID | 20190705152812.26438-1-eblake@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [for-4.1] qcow2: Allow -o compat=v3 during qemu-img amend | expand |
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.
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.
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
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
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.
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,
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(-)