Re: [libvirt] [PATCH] Recheck disk backing chains after snapshot
On 04/25/2014 09:13 AM, Jiri Denemark wrote:
When a snapshot operation finishes we have to recheck the backing
chain
of all disks involved in the snapshot. And we need to do that even if
the operation failed because some of the disks might have changed if
QEMU did not support transactions.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
- BlockRebase and BlockCommit already recheck the backing chain when we
get an event about a completed block job (in qemuProcessHandleBlockJob)
src/qemu/qemu_driver.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
FYI: This change fixes an issue that virt-test found over my
weekend run in the 'blockpull' test which was caused by 4/4 of
the addressing backing stores by index:
http://www.redhat.com/archives/libvir-list/2014-April/msg00823.html
Essentially the test sets things up via:
/usr/bin/virsh snapshot-create-as virt-tests-vm1 snapshot1 snap1-desc --disk-only --atomic
--no-metadata vda,snapshot=external,file=/home/virt-test/tmp/jeos-20-64.snap1
/usr/bin/virsh snapshot-create-as virt-tests-vm1 snapshot2 snap2-desc --disk-only --atomic
--no-metadata vda,snapshot=external,file=/home/virt-test/tmp/jeos-20-64.snap2
/usr/bin/virsh snapshot-create-as virt-tests-vm1 snapshot3 snap3-desc --disk-only --atomic
--no-metadata vda,snapshot=external,file=/home/virt-test/tmp/jeos-20-64.snap3
Then does (in different passes):
/usr/bin/virsh blockpull virt-tests-vm1 vda --wait --verbose
/usr/bin/virsh blockpull virt-tests-vm1 vda --wait --verbose --base
/home/virt-test/tmp/jeos-20-64.snap2
/usr/bin/virsh blockpull virt-tests-vm1 vda --wait --verbose --base
/home/virt-test/tmp/jeos-20-64.snap1
/usr/bin/virsh blockpull virt-tests-vm1 vda --wait --verbose --timeout 1
/usr/bin/virsh blockpull virt-tests-vm1 vda --wait --verbose --timeout 1 --base
/home/virt-test/tmp/jeos-20-64.snap2
/usr/bin/virsh blockpull virt-tests-vm1 vda --wait --verbose --timeout 1 --base
/home/virt-test/tmp/jeos-20-64.snap1
Without this patch applied, the blockpull's w/ --base supplied fail
with either:
could not find image '/home/virt-test/tmp/jeos-20-64.snap2' in chain for
'/home/virt-test/tmp/jeos-20-64.snap3'
or
could not find image '/home/virt-test/tmp/jeos-20-64.snap1' in chain for
'/home/virt-test/tmp/jeos-20-64.snap3'
John
FWIW: The weekend run I had w/ virt-test against recent upstream
changes was quite successful. There's still an issue or two to
resolve regarding how capacity is checked, but the recent storage
changes haven't caused regressions.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 67ba487..492fcc1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12942,6 +12942,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver,
bool persist = false;
bool reuse = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ virErrorPtr orig_err = NULL;
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID,
@@ -13033,6 +13034,17 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver,
}
cleanup:
+ /* recheck backing chains of all disks involved in the snapshot */
+ orig_err = virSaveLastError();
+ for (i = 0; i < snap->def->ndisks; i++) {
+ if (snap->def->disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE)
+ continue;
+ qemuDomainDetermineDiskChain(driver, vm, vm->def->disks[i], true);
+ }
+ if (orig_err) {
+ virSetError(orig_err);
+ virFreeError(orig_err);
+ }
if (ret == 0 || !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_TRANSACTION)) {
if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0 ||