Bug 1203931
| Summary: | [RFE] libvirt should do better sanity checking when reusing external files as a destination | ||
|---|---|---|---|
| Product: | Red Hat Enterprise Linux 7 | Reporter: | Shanzhi Yu <shyu> |
| Component: | libvirt | Assignee: | Peter Krempa <pkrempa> |
| Status: | CLOSED ERRATA | QA Contact: | Virtualization Bugs <virt-bugs> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | 7.1 | CC: | dyuan, eblake, lmiksik, mzhan, pkrempa, rbalakri, xuzhang, yanyang |
| Target Milestone: | rc | Keywords: | FutureFeature, Reopened |
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | libvirt-1.2.15-1.el7 | Doc Type: | Enhancement |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2015-11-19 06:20:46 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: | |||
I don't think it's worth doing anything when reusing external files. The reuse option was added to allow relative paths, but the user has to set up the backing file properly. The documentation states: For block copy: "The destination will be created unless the VIR_DOMAIN_BLOCK_COPY_REUSE_EXT flag is present stating that the file was pre-created with the correct format and metadata and sufficient size to hold the copy." For snapshots: "However, if @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT, then the destination files must be pre-created manually with the correct image format and metadata including backing store path (this allows a management app to pre-create files with relative backing file names, rather than the default of creating with absolute backing file names). Note that setting incorrect metadata in the pre-created image may lead to the VM being unable to start." As this is basically a workaround for bug 1127226 and it's well documented that the users have to pass correct metadata in the backing chain I don't think there's anything to do for us. Using the external files should not be a common thing to do. (In reply to Peter Krempa from comment #1) > I don't think it's worth doing anything when reusing external files. The > reuse option was added to allow relative paths, but the user has to set up > the backing file properly. The documentation states: > > For block copy: > "The destination will be created unless the VIR_DOMAIN_BLOCK_COPY_REUSE_EXT > flag is present stating that the file was pre-created with the correct > format and metadata and sufficient size to hold the copy." > > For snapshots: > "However, if @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT, then the > destination files must be pre-created manually with the correct image format > and metadata including backing store path (this allows a management app to > pre-create files with relative backing file names, rather than the default > of creating with absolute backing file names). Note that setting incorrect > metadata in the pre-created image may lead to the VM being unable to start." > Users do create a backing chain with correct format and metadata in step 1(see comment 0), right? Problem is in step3. If users create external snapshot using the existing file with right sequence(here is a then b then c), then there will be no problem. If can not fix this, at least note it in our document(libvirt.org)? > As this is basically a workaround for bug 1127226 and it's well documented > that the users have to pass correct metadata in the backing chain I don't > think there's anything to do for us. Using the external files should not be > a common thing to do. (In reply to Shanzhi Yu from comment #2) > (In reply to Peter Krempa from comment #1) > > I don't think it's worth doing anything when reusing external files. The > > reuse option was added to allow relative paths, but the user has to set up > > the backing file properly. The documentation states: > > > > For block copy: > > "The destination will be created unless the VIR_DOMAIN_BLOCK_COPY_REUSE_EXT > > flag is present stating that the file was pre-created with the correct > > format and metadata and sufficient size to hold the copy." > > > > For snapshots: > > "However, if @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT, then the > > destination files must be pre-created manually with the correct image format > > and metadata including backing store path (this allows a management app to > > pre-create files with relative backing file names, rather than the default > > of creating with absolute backing file names). Note that setting incorrect > > metadata in the pre-created image may lead to the VM being unable to start." > > > > Users do create a backing chain with correct format and metadata in step > 1(see comment 0), right? Ah, now I see the problem. I had to carefully check the reproducer steps as I've misread them at first. > > Problem is in step3. > If users create external snapshot using the existing file with right > sequence(here is a then b then c), then there will be no problem. The problem is that you pre-create three (a/a, b/b, c/c) layers of images in your reproducer steps and use the last one (c/c) as the snapshot. Qemu in the live step uses just the image you passed to it thus it disregards the 'images/b/b' and 'images/a/a' from it's backing chain, while libvirt does re-detect the backing chain. In case libvirt would track the metadata properly this would not happen as the images in the backing chain that is displayed would be as it's tracked by qemu. > > If can not fix this, at least note it in our document(libvirt.org)? While this is still a user error, this particular problem is not documented. I'll try to come up with a wording that might explain that only the image passed to the API is added to the backing chain by the hypervisor and other are ignored. For now, I've documented that only one layer is inserted into the backing chain in the live definition:
commit d56707766e3ce7f505719f5adf753bfefd9f7c9a
Author: Peter Krempa <pkrempa>
Date: Fri Apr 10 10:48:34 2015 +0200
lib: snapshot: Explain that only one layer of images is inserted
When creating a snapshot with _REUSE_EXTERNAL when the pre-created image
does not directly link to the current active layer libvirt would
re-detect the backing chain incorrectly and it would not match with
qemu's view. Since the configuration is an operator mistake, document
that only the top layer image gets inserted.
diff --git a/src/libvirt-domain-snapshot.c b/src/libvirt-domain-snapshot.c
index ac858ba..55826fd 100644
--- a/src/libvirt-domain-snapshot.c
+++ b/src/libvirt-domain-snapshot.c
@@ -177,8 +177,9 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot)
* the correct image format and metadata including backing store path
* (this allows a management app to pre-create files with relative backing
* file names, rather than the default of creating with absolute backing
- * file names). Note that setting incorrect metadata in the pre-created
- * image may lead to the VM being unable to start.
+ * file names). Note that only the file specified in the snapshot XML is
+ * inserted as a snapshot thus setting incorrect metadata in the pre-created
+ * image may lead to the VM being unable to start or other block jobs may fail.
*
* Be aware that although libvirt prefers to report errors up front with
* no other effect, some hypervisors have certain types of failures where
v1.2.14-131-gd567077
Until we properly fix tracking of the backing chain, the steps above will break it but the user is advised not to do that.
Verify this bug with libvirt-1.2.15-2.el7.x86_64 Steps: # rpm -qf /usr/src/debug/libvirt-1.2.15/src/libvirt-domain-snapshot.c libvirt-debuginfo-1.2.15-2.el7.x86_64 # sed -n 177,183p /usr/src/debug/libvirt-1.2.15/src/libvirt-domain-snapshot.c * the correct image format and metadata including backing store path * (this allows a management app to pre-create files with relative backing * file names, rather than the default of creating with absolute backing * file names). Note that only the file specified in the snapshot XML is * inserted as a snapshot thus setting incorrect metadata in the pre-created * image may lead to the VM being unable to start or other block jobs may fail. * So according the comment, this bug has been fixed from doc sides. Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://rhn.redhat.com/errata/RHBA-2015-2202.html |
Description of problem: [RFE] libvirt should do better sanity checking when reusing external files as a destination. Version-Release number of selected component (if applicable): How reproducible: 100% Steps to Reproduce: 1. Prepare a backing chain with two or more backing file # cd /tmp || exit 1 # rm -rf images # mkdir -p images images/a images/b images/c # qemu-img create -f qcow2 images/a/a 10M -b gluster://10.66.5.38/gluster-vol1/r7-qcow2.img -o backing_fmt=qcow2|| exit 1 Formatting 'images/a/a', fmt=qcow2 size=10485760 backing_file='gluster://10.66.5.38/gluster-vol1/r7-qcow2.img' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off # (cd images/b && qemu-img create -f qcow2 -b ../a/a -o backing_fmt=qcow2 b) || exit 1 Formatting 'b', fmt=qcow2 size=10485760 backing_file='../a/a' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off # (cd images/c && qemu-img create -f qcow2 -b ../b/b -o backing_fmt=qcow2 c) || exit 1 Formatting 'c', fmt=qcow2 size=10485760 backing_file='../b/b' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off 2.Prepare guest # virsh create /dev/stdin <<EOF <domain type='kvm' id='18'> <name>vm</name> <memory unit='KiB'>262144</memory> <currentMemory unit='KiB'>262144</currentMemory> <vcpu placement='static'>1</vcpu> <os> <type arch='x86_64' machine='pc-i440fx-rhel7.1.0'>hvm</type> <boot dev='hd'/> </os> <devices> <emulator>/usr/libexec/qemu-kvm</emulator> <disk type='network' device='disk'> <driver name='qemu' type='qcow2'/> <source protocol='gluster' name='gluster-vol1/r7-qcow2.img'> <host name='10.66.5.38'/> </source> <backingStore/> <target dev='vda' bus='virtio'/> <alias name='virtio-disk0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </disk> </devices> </domain> EOF 3. Create external disk snapshot for guest with existing backing chain # virsh snapshot-create-as vm --no-metadata --disk-only --diskspec vda,file=/tmp/images/c/c --reuse-external Domain snapshot 1426749044 created # virsh dumpxml vm |grep disk -A 18 <disk type='file' device='disk'> <driver name='qemu' type='qcow2'/> <source file='/tmp/images/c/c'/> <backingStore type='file' index='1'> <format type='qcow2'/> <source file='/tmp/images/c/../b/b'/> <backingStore type='file' index='2'> <format type='qcow2'/> <source file='/tmp/images/c/../b/../a/a'/> <backingStore type='network' index='3'> <format type='qcow2'/> <source protocol='gluster' name='gluster-vol1/r7-qcow2.img'> <host name='10.66.5.38'/> </source> <backingStore/> </backingStore> </backingStore> </backingStore> 4. Do active blockcommit 4.1 with "--shallow" options # virsh blockcommit vm vda --pivot --verbose --wait --shallow error: internal error: qemu block name 'gluster://10.66.5.38/gluster-vol1/r7-qcow2.img' doesn't match expected '/tmp/images/c/../b/b' 4.2 without "--shallow" options # virsh blockcommit vm vda --pivot --verbose --wait error: internal error: unable to find backing name for device drive-virtio-disk0 Actual results: Expected results: Additional info: