Bug 1727347

Summary:	starting with v2 image with internal snapshot, qemu-img amend -o compat=1.1 creates invalid image
Product:	Red Hat Enterprise Linux 8	Reporter:	Eric Blake <eblake>
Component:	qemu-kvm	Assignee:	Hanna Czenczek <hreitz>
Status:	CLOSED WONTFIX	QA Contact:	Tingting Mao <timao>
Severity:	unspecified	Docs Contact:
Priority:	medium
Version:	8.1	CC:	coli, eblake, jferlan, jinzhao, juzhang, rbalakri, virt-maint
Target Milestone:	rc
Target Release:	---
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:		Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:
Clones:	1736842 (view as bug list)		Environment:
Last Closed:	2019-11-19 14:32:18 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:
Bug Blocks:	1736842

Description Eric Blake 2019-07-05 15:46:58 UTC

Description of problem:
The qcow2 spec requires that any v3 file with internal snapshots must use at least 16 bytes of extra data per snapshot entry, in order to include the virtual disk size of the internal snapshot. However, when converting a v2 file created by older qemu (such as RHEL 6) into a v3 file, the existing snapshot headers are not expanded into the correct v3 format.

True, RHEL tends to frown on internal snapshots (and prefers the use of external), but creating an image that violates the spec, and one that 'qemu-img check' fails to diagnose as violating the spec, is not good for interoperability.

Version-Release number of selected component (if applicable):

How reproducible:
100%

Steps to Reproduce:
1. Create a v2 file with internal snapshot. On RHEL6:
$ 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
3. Upgrade it to v3. On RHEL8:
$ 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
5. Contrast with an image created natively in v3. On RHEL8:
$ qemu-img create -f qcow2 file2 1m
$ qemu-img snapshot -c s1 file2
$ od -Ax -j64 -N8 -tx1 file2 # Learn the offset for the next command
$ offset=$((0x50000+36))
$ od -Ax -j$offset -N 4 -tx1 file2

Actual results:
2. extra size is 0
4. extra size is still 0
5. extra size is 16

Expected results:
4. extra size should change from 0 to 16

Additional info:
qemu-img amend should be taught to rewrite the internal snapshots table to ensure that things get padded out. Having the disk size at the time of each snapshot is a prerequisite to being able to resize an image that contains internal snapshots (we forbid resize of v2 images with internal snapshots because otherwise you may lack the size to switch back to on reverting to that snapshot, but this is a pointless restriction for v3 images).

Comment 1 Eric Blake 2019-07-05 15:54:00 UTC

See also the upstream thread at: https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg01663.html

Comment 2 Eric Blake 2019-07-07 00:54:17 UTC

Note, however, that it is sufficient to do the following to bring the image back into v3 compliance on RHEL 8:

$ qemu-img snapshot -c tmp file
$ qemu-img snapshot -d tmp file

The act of adding a new (temporary) internal checkpoint causes qemu to allocate a new snapshot table at a different offset, and in the process of copying all old entries plus the new entry to the new location, the old entries are expanded out to a compliant size.

Comment 4 Tingting Mao 2019-08-02 07:07:55 UTC

Reproduce this bug as below:


Steps:
1. Create a v2 file with internal snapshot. On RHEL6: 
# qemu-img create -f qcow2 test_v2.qcow2 1m
# qemu-img snapshot -c s1 test_v2.qcow2

2. Check that the internal snapshot header uses the smaller size:
# od -Ax -j64 -N8 -tx1 test_v2.qcow2 
000040 00 00 00 00 00 05 00 00
000048
# offset=$((0x50000+36))
# od -Ax -j$offset -N 4 -tx1 test_v2.qcow2 
050024 00 00 00 00
050028

3. Scp the image to RHEL8 host, and upgrade it to v3.
# qemu-img amend -o compat=1.1 test_v2.qcow2

4. Check the internal snapshot header size:
# od -Ax -j64 -N8 -tx1 test_v2.qcow2
000040 00 00 00 00 00 05 00 00
000048
# offset=$((0x50000+36))
# od -Ax -j$offset -N 4 -tx1 test_v2.qcow2 
050024 00 00 00 00  ------------------------------ Still 0!
050028

5. Contrast with an image created natively in v3. On RHEL8:
# qemu-img create -f qcow2 test_v3.qcow2 1m
# qemu-img snapshot -c s1 test_v3.qcow2
# od -Ax -j64 -N8 -tx1 test_v3.qcow2 
000040 00 00 00 00 00 05 00 00
000048
# offset=$((0x50000+36))
# od -Ax -j$offset -N 4 -tx1 test_v3.qcow2 
050024 00 00 00 10 --------------------- The extra size is 16!
050028


Additional info:
qemu version in RHEL6:
# rpm -qa | grep qemu-kvm
qemu-kvm-0.12.1.2-2.503.el6_9.6.x86_64

qemu version in RHEL8:
qemu-kvm-2.12.0-83.module+el8.1.0+3852+0ba8aef0

Comment 6 Hanna Czenczek 2019-11-19 11:15:53 UTC

Is this really something we need to fix in RHEL 8 (non-AV)?  I don’t know whether interoperability is actually a concern in practice, and as comment 2 notes, creating a new snapshot fixes the issue.  As far as I can see, this also applies to v2 images operated on by RHEL-7+ qemu tools.  So this only applies to images that you have created snapshots on with RHEL 6 tools, and that you have not yet created a snapshot on with RHEL 7+ tools.  That seems unlikely to me.

Therefore, I don’t think this is actually practically relevant.

Max

Comment 7 Eric Blake 2019-11-19 14:24:40 UTC

(In reply to Max Reitz from comment #6)
> Is this really something we need to fix in RHEL 8 (non-AV)?  I don’t know
> whether interoperability is actually a concern in practice, and as comment 2
> notes, creating a new snapshot fixes the issue.  As far as I can see, this
> also applies to v2 images operated on by RHEL-7+ qemu tools.  So this only
> applies to images that you have created snapshots on with RHEL 6 tools, and
> that you have not yet created a snapshot on with RHEL 7+ tools.  That seems
> unlikely to me.
> 
> Therefore, I don’t think this is actually practically relevant.

We already discourage internal snapshots as unsupported downstream, favoring external snapshots instead (even in RHEL 6). Interoperability with qcow2 images created from other sources is less important that correct operation on images created by RHEL, when it comes to level of support.  Thus, I agree with the argument that bending over backwards to backport this fix is not worth the effort; we can wait to pick it up via rebase in the AV release.

Comment 8 Hanna Czenczek 2019-11-19 14:32:18 UTC

OK, thanks!  I’ll close this BZ then.  (I’ve moved the AV BZ 1736842 to POST earlier today.)

Max