For external snapshots to be useful on persistent domains, we must
alter the persistent definition alongside the running definition.
Thanks to the possibility of disk hotplug as well as of edits that
only affect the persistent xml, we can't assume that vm->def and
vm->newDef have the same disk at the same index, so we can only
update the persistent copy if the device destination matches up.
* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateDiskActive)
(qemuDomainSnapshotCreateSingleDiskActive): Also affect newDef, if
present.
---
This is worth including in 0.9.5 - without it, the new feature of
disk snapshots on a persistent domain are lost the moment the domain
stops running, which is likely to cause data corruption for guests.
I'm still trying to reproduce another reported snapshot failure:
https://bugzilla.redhat.com/show_bug.cgi?id=738676
although I don't think this patch will help that issue.
src/qemu/qemu_driver.c | 38 ++++++++++++++++++++++++++++++++------
1 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 32f376ec..cbe28d8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9107,12 +9107,15 @@ static int
qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainSnapshotDiskDefPtr snap,
- virDomainDiskDefPtr disk)
+ virDomainDiskDefPtr disk,
+ virDomainDiskDefPtr persistDisk)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
char *device = NULL;
char *source = NULL;
char *driverType = NULL;
+ char *persistSource = NULL;
+ char *persistDriverType = NULL;
int ret = -1;
int fd = -1;
char *origsrc = NULL;
@@ -9128,7 +9131,11 @@ qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver
*driver,
if (virAsprintf(&device, "drive-%s", disk->info.alias) < 0 ||
!(source = strdup(snap->file)) ||
(STRNEQ_NULLABLE(disk->driverType, "qcow2") &&
- !(driverType = strdup("qcow2")))) {
+ !(driverType = strdup("qcow2"))) ||
+ (persistDisk &&
+ (!(persistSource = strdup(source)) ||
+ (STRNEQ_NULLABLE(persistDisk->driverType, "qcow2") &&
+ !(persistDriverType = strdup("qcow2")))))) {
virReportOOMError();
goto cleanup;
}
@@ -9176,9 +9183,16 @@ qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver
*driver,
disk->driverType = driverType;
driverType = NULL;
}
-
- /* XXX Do we also need to update vm->newDef if there are pending
- * configuration changes awaiting the next boot? */
+ if (persistDisk) {
+ VIR_FREE(persistDisk->src);
+ persistDisk->src = persistSource;
+ persistSource = NULL;
+ if (persistDriverType) {
+ VIR_FREE(persistDisk->driverType);
+ persistDisk->driverType = persistDriverType;
+ persistDriverType = NULL;
+ }
+ }
cleanup:
if (origsrc) {
@@ -9190,6 +9204,8 @@ cleanup:
VIR_FREE(device);
VIR_FREE(source);
VIR_FREE(driverType);
+ VIR_FREE(persistSource);
+ VIR_FREE(persistDriverType);
return ret;
}
@@ -9235,12 +9251,22 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
* SNAPSHOT_EXTERNAL with a valid file name and qcow2 format. */
qemuDomainObjEnterMonitorWithDriver(driver, vm);
for (i = 0; i < snap->def->ndisks; i++) {
+ virDomainDiskDefPtr persistDisk = NULL;
+
if (snap->def->disks[i].snapshot == VIR_DOMAIN_DISK_SNAPSHOT_NO)
continue;
+ if (vm->newDef) {
+ int indx = virDomainDiskIndexByName(vm->newDef,
+ vm->def->disks[i]->dst,
+ false);
+ if (indx >= 0)
+ persistDisk = vm->newDef->disks[indx];
+ }
ret = qemuDomainSnapshotCreateSingleDiskActive(driver, vm,
&snap->def->disks[i],
- vm->def->disks[i]);
+ vm->def->disks[i],
+ persistDisk);
if (ret < 0)
break;
}
--
1.7.4.4