Bug 1783173
| Summary: | The JSON format of backing files is not flexible | ||
|---|---|---|---|
| Product: | Red Hat Enterprise Linux Advanced Virtualization | Reporter: | Tingting Mao <timao> |
| Component: | qemu-kvm | Assignee: | Hanna Czenczek <hreitz> |
| Status: | CLOSED WONTFIX | QA Contact: | Tingting Mao <timao> |
| Severity: | unspecified | Docs Contact: | |
| Priority: | medium | ||
| Version: | 8.2 | CC: | coli, hreitz, jferlan, jinzhao, juzhang, kwolf, mlevitsk, qzhang, 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: | Environment: | ||
| Last Closed: | 2019-12-16 16:41:27 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: | |||
Hi, I’m not sure whether the failure in step 4 has anything to do with the fact that both objects aren’t compared as dicts. I rather think it stems somewhere from the fact that qemu modifies the internal backing filename when the backing file is opened, and so the user has little chance of guessing what that internal name is. (bdrv_find_backing_image() assumes the filename is still unmodified and thus makes things even worse.) This is already tracked as BZ 1599277. The advantage of comparing the objects as dicts would be that you could specify options in any order. But I don’t know whether that’s really worth the effort. I think we can expect users to specify the commit base filename exactly as it was specified when the overlay was created. If they change the dict order, they might as well omit optional keys with default values, or add optional keys with their default values. If we were to allow those cases, too, then the comparison would need to be schema-sensitive and thus would really get complicated. It boils down to the fact that filenames are not a good way of specifying layers. In the qemu system emulator, this has been addressed with node names. Maybe it is worth trying to add something similar to qemu-img, e.g. a menu where the user is presented with all backing files in the chain and can then choose. But I don’t know whether even that is necessary. I think it should be enough if the string given as a backing filename when creating the overlay works (and that’s BZ 1599277). Max (In reply to Max Reitz from comment #1) > Hi, > > I’m not sure whether the failure in step 4 has anything to do with the fact > that both objects aren’t compared as dicts. I rather think it stems > somewhere from the fact that qemu modifies the internal backing filename > when the backing file is opened, and so the user has little chance of > guessing what that internal name is. (bdrv_find_backing_image() assumes the > filename is still unmodified and thus makes things even worse.) > > This is already tracked as BZ 1599277. > > The advantage of comparing the objects as dicts would be that you could > specify options in any order. But I don’t know whether that’s really worth > the effort. I think we can expect users to specify the commit base filename > exactly as it was specified when the overlay was created. If they change > the dict order, they might as well omit optional keys with default values, > or add optional keys with their default values. If we were to allow those > cases, too, then the comparison would need to be schema-sensitive and thus > would really get complicated. > > It boils down to the fact that filenames are not a good way of specifying > layers. In the qemu system emulator, this has been addressed with node > names. Maybe it is worth trying to add something similar to qemu-img, e.g. > a menu where the user is presented with all backing files in the chain and > can then choose. But I don’t know whether even that is necessary. I think > it should be enough if the string given as a backing filename when creating > the overlay works (and that’s BZ 1599277). > Agree, closing as WONTFIX given it's a low-priority corner-case, let's fix BZ 1599277 instead. |
Description of problem: As subject. Version-Release number of selected component (if applicable): qemu-kvm-4.2.0-2.module+el8.2.0+5135+ed3b2489 How reproducible: 100% Steps to Reproduce: 1. Create a base luks image. # qemu-img create -f luks --object secret,id=image1_encrypt0,data=redhat -o key-secret=image1_encrypt0 base.luks 1G 2. Create snapshot based on the luks image. # qemu-img create --object secret,id=image1_encrypt0,data=redhat -f qcow2 -b 'json:{"file": {"driver": "file", "filename": "base.luks"}, "driver": "luks", "key-secret": "image1_encrypt0"}' sn1.qcow2 3. Create another snapshot based on last snapshot. # qemu-img create -f qcow2 --object secret,id=image1_encrypt0,data=redhat -b sn1.qcow2 sn2.qcow2 4. Rebase the commit sn2.qcow2 to base. # qemu-img commit -f qcow2 --object secret,id=image1_encrypt0,data=redhat -b 'json:{"file": {"driver": "file", "filename": "base.luks"}, "driver": "luks", "key-secret": "image1_encrypt0"}' sn2.qcow2 -p ------------------- The string for base image(I.E. The value of '-b' option) is copied from step2. Actual results: After step4, failed as below. # qemu-img commit -f qcow2 --object secret,id=image1_encrypt0,data=redhat -b 'json:{"file": {"driver": "file", "filename": "base.luks"}, "driver": "luks", "key-secret": "image1_encrypt0"}' sn2.qcow2 -p (0.00/100%) qemu-img: Did not find 'json:{"file": {"driver": "file", "filename": "base.luks"}, "driver": "luks", "key-secret": "image1_encrypt0"}' in the backing chain of 'sn2.qcow2' Expected results: Commit successfully. Qemu-img commit in case when -b is a json to convert it to dict,and compare with the dict of the backing file. Additional info: It only works when specifying the string for new base with the result from backing-chain info. 1. Check the backing chain image info. # qemu-img info sn2.qcow2 --backing-chain image: sn2.qcow2 file format: qcow2 virtual size: 1 GiB (1073741824 bytes) disk size: 196 KiB cluster_size: 65536 backing file: sn1.qcow2 Format specific information: compat: 1.1 lazy refcounts: false refcount bits: 16 corrupt: false image: sn1.qcow2 file format: qcow2 virtual size: 1 GiB (1073741824 bytes) disk size: 196 KiB cluster_size: 65536 backing file: json:{"file": {"driver": "file", "filename": "base.luks"}, "driver": "luks", "key-secret": "image1_encrypt0"} Format specific information: compat: 1.1 lazy refcounts: false refcount bits: 16 corrupt: false image: json:{"driver": "luks", "file": {"driver": "file", "filename": "base.luks"}, "key-secret": "image1_encrypt0"} file format: luks virtual size: 1 GiB (1073741824 bytes) disk size: 256 KiB encrypted: yes Format specific information: ivgen alg: plain64 hash alg: sha256 cipher alg: aes-256 uuid: 624f7e2f-48db-449e-9ebf-9a4a68c4a69c cipher mode: xts slots: [0]: active: true iters: 1030440 key offset: 4096 stripes: 4000 [1]: active: false key offset: 262144 [2]: active: false key offset: 520192 [3]: active: false key offset: 778240 [4]: active: false key offset: 1036288 [5]: active: false key offset: 1294336 [6]: active: false key offset: 1552384 [7]: active: false key offset: 1810432 payload offset: 2068480 master key iters: 213596 2. Commit the changes to the base. # qemu-img commit -f qcow2 --object secret,id=image1_encrypt0,data=redhat -b 'json:{"driver": "luks", "file": {"driver": "file", "filename": "base.luks"}, "key-secret": "image1_encrypt0"}' sn2.qcow2 -p --------------------------------- The string of new base is copied from the value of "image" of the result from last step. (100.00/100%) Image committed.