Re: [libvirt] [PATCH 2/2] snapshot: wire up qemu transaction command

The hardest part about adding transactions is not using the new monitor command, but undoing the partial changes we made prior to a failed transaction. * src/qemu/qemu_driver.c (qemuDomainSnapshotCreateDiskActive): Use transaction when available. (qemuDomainSnapshotUndoSingleDiskActive): New function. (qemuDomainSnapshotCreateSingleDiskActive): Pass through actions. (qemuDomainSnapshotCreateXML): Adjust caller. --- src/qemu/qemu_driver.c | 112 +++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 107 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c3bbc3f..2bc8d5d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9822,7 +9822,8 @@ qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver, virDomainObjPtr vm, virDomainSnapshotDiskDefPtr snap, virDomainDiskDefPtr disk, - virDomainDiskDefPtr persistDisk) + virDomainDiskDefPtr persistDisk, + virJSONValuePtr actions) { qemuDomainObjPrivatePtr priv = vm->privateData; char *device = NULL; @@ -9882,7 +9883,7 @@ qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver, origdriver = NULL; /* create the actual snapshot */ - ret = qemuMonitorDiskSnapshot(priv->mon, NULL, device, source); + ret = qemuMonitorDiskSnapshot(priv->mon, actions, device, source); virDomainAuditDisk(vm, disk->src, source, "snapshot", ret>= 0); if (ret< 0) goto cleanup; @@ -9923,6 +9924,69 @@ cleanup: return ret; } +/* The domain is expected to hold monitor lock. This is the + * counterpart to qemuDomainSnapshotCreateSingleDiskActive, called + * only on a failed transaction. */ +static void +qemuDomainSnapshotUndoSingleDiskActive(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainDiskDefPtr origdisk, + virDomainDiskDefPtr disk, + virDomainDiskDefPtr persistDisk, + bool need_unlink) +{ + char *source = NULL; + char *driverType = NULL; + char *persistSource = NULL; + char *persistDriverType = NULL; + struct stat st; + + if (!(source = strdup(origdisk->src)) || + (origdisk->driverType&& + !(driverType = strdup(origdisk->driverType))) || + (persistDisk&& + (!(persistSource = strdup(source)) || + (driverType&& !(persistDriverType = strdup(driverType)))))) { + virReportOOMError(); + goto cleanup; + } + + if (virSecurityManagerRestoreImageLabel(driver->securityManager, + vm->def, disk)< 0) + VIR_WARN("Unable to restore security label on %s", disk->src); + if (virDomainLockDiskDetach(driver->lockManager, vm, disk)< 0) + VIR_WARN("Unable to release lock on %s", source); + if (need_unlink&& stat(disk->src,&st) == 0 +&& st.st_size == 0&& S_ISREG(st.st_mode)&& unlink(disk->src)< 0) + VIR_WARN("Unable to release lock on %s", disk->src);

+ + /* Update vm in place to match changes. */ + VIR_FREE(disk->src); + disk->src = source; + source = NULL; + VIR_FREE(disk->driverType); + if (driverType) { + disk->driverType = driverType; + driverType = NULL; + } + if (persistDisk) { + VIR_FREE(persistDisk->src); + persistDisk->src = persistSource; + persistSource = NULL; + VIR_FREE(persistDisk->driverType); + if (persistDriverType) { + persistDisk->driverType = persistDriverType; + persistDriverType = NULL; + } + } + +cleanup: + VIR_FREE(source); + VIR_FREE(driverType); + VIR_FREE(persistSource); + VIR_FREE(persistDriverType); +} +