[libvirt] [PATCH] qemu: Reparent children when deleting a snapshot
Shorten qemuDomainSnapshotWriteSnapshotMetadata function name and make it take a snapshot pointer instead of dealing with the current snapshot. Update other functions accordingly. Add a qemuDomainSnapshotReparentChildren hash iterator to reparent the children of a snapshot that is being deleted. Use qemuDomainSnapshotWriteMetadata to write updated metadata to disk. This fixes a problem where outdated parent information breaks the snapshot tree and hinders the deletion of child snapshots. Reported by Philipp Hahn. --- src/qemu/qemu_driver.c | 92 +++++++++++++++++++++++++++++++++++++----------- 1 files changed, 71 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 924446f..9b538a3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2607,10 +2607,10 @@ qemuPrepareMonitorChr(struct qemud_driver *driver, return 0; } -static int qemuDomainSnapshotSetActive(virDomainObjPtr vm, - char *snapshotDir); -static int qemuDomainSnapshotSetInactive(virDomainObjPtr vm, - char *snapshotDir); +static int qemuDomainSnapshotSetCurrentActive(virDomainObjPtr vm, + char *snapshotDir); +static int qemuDomainSnapshotSetCurrentInactive(virDomainObjPtr vm, + char *snapshotDir); #define START_POSTFIX ": starting up\n" @@ -2807,7 +2807,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, vm->current_snapshot, vmop))) goto cleanup; - if (qemuDomainSnapshotSetInactive(vm, driver->snapshotDir) < 0) + if (qemuDomainSnapshotSetCurrentInactive(vm, driver->snapshotDir) < 0) goto cleanup; /* now that we know it is about to start call the hook if present */ @@ -9472,8 +9472,9 @@ static char *qemuFindQemuImgBinary(void) return ret; } -static int qemuDomainSnapshotWriteSnapshotMetadata(virDomainObjPtr vm, - char *snapshotDir) +static int qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, + virDomainSnapshotObjPtr snapshot, + char *snapshotDir) { int fd = -1; char *newxml = NULL; @@ -9484,7 +9485,7 @@ static int qemuDomainSnapshotWriteSnapshotMetadata(virDomainObjPtr vm, char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(vm->def->uuid, uuidstr); - newxml = virDomainSnapshotDefFormat(uuidstr, vm->current_snapshot->def, 1); + newxml = virDomainSnapshotDefFormat(uuidstr, snapshot->def, 1); if (newxml == NULL) { virReportOOMError(); return -1; @@ -9501,8 +9502,7 @@ static int qemuDomainSnapshotWriteSnapshotMetadata(virDomainObjPtr vm, goto cleanup; } - if (virAsprintf(&snapFile, "%s/%s.xml", snapDir, - vm->current_snapshot->def->name) < 0) { + if (virAsprintf(&snapFile, "%s/%s.xml", snapDir, snapshot->def->name) < 0) { virReportOOMError(); goto cleanup; } @@ -9528,25 +9528,27 @@ cleanup: return ret; } -static int qemuDomainSnapshotSetActive(virDomainObjPtr vm, - char *snapshotDir) +static int qemuDomainSnapshotSetCurrentActive(virDomainObjPtr vm, + char *snapshotDir) { if (vm->current_snapshot) { vm->current_snapshot->def->active = 1; - return qemuDomainSnapshotWriteSnapshotMetadata(vm, snapshotDir); + return qemuDomainSnapshotWriteMetadata(vm, vm->current_snapshot, + snapshotDir); } return 0; } -static int qemuDomainSnapshotSetInactive(virDomainObjPtr vm, - char *snapshotDir) +static int qemuDomainSnapshotSetCurrentInactive(virDomainObjPtr vm, + char *snapshotDir) { if (vm->current_snapshot) { vm->current_snapshot->def->active = 0; - return qemuDomainSnapshotWriteSnapshotMetadata(vm, snapshotDir); + return qemuDomainSnapshotWriteMetadata(vm, vm->current_snapshot, + snapshotDir); } return 0; @@ -9686,8 +9688,9 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain, /* Now we set the new current_snapshot for the domain */ vm->current_snapshot = snap; - if (qemuDomainSnapshotWriteSnapshotMetadata(vm, driver->snapshotDir) < 0) - /* qemuDomainSnapshotWriteSnapshotMetadata set the error */ + if (qemuDomainSnapshotWriteMetadata(vm, vm->current_snapshot, + driver->snapshotDir) < 0) + /* qemuDomainSnapshotWriteMetadata set the error */ goto cleanup; snapshot = virGetDomainSnapshot(domain, snap->def->name); @@ -9941,13 +9944,13 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, goto endjob; } else { - if (qemuDomainSnapshotSetActive(vm, driver->snapshotDir) < 0) + if (qemuDomainSnapshotSetCurrentActive(vm, driver->snapshotDir) < 0) goto endjob; rc = qemudStartVMDaemon(snapshot->domain->conn, driver, vm, NULL, false, -1, NULL, VIR_VM_OP_CREATE); qemuDomainStartAudit(vm, "from-snapshot", rc >= 0); - if (qemuDomainSnapshotSetInactive(vm, driver->snapshotDir) < 0) + if (qemuDomainSnapshotSetCurrentInactive(vm, driver->snapshotDir) < 0) goto endjob; if (rc < 0) goto endjob; @@ -9991,7 +9994,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } } - if (qemuDomainSnapshotSetActive(vm, driver->snapshotDir) < 0) + if (qemuDomainSnapshotSetCurrentActive(vm, driver->snapshotDir) < 0) goto endjob; } @@ -10128,6 +10131,43 @@ static void qemuDomainSnapshotDiscardChildren(void *payload, } } +struct snap_reparent { + struct qemud_driver *driver; + virDomainSnapshotObjPtr snap; + virDomainObjPtr vm; + int err; +}; + +static void +qemuDomainSnapshotReparentChildren(void *payload, + const char *name ATTRIBUTE_UNUSED, + void *data) +{ + virDomainSnapshotObjPtr snap = payload; + struct snap_reparent *rep = data; + + if (rep->err < 0) { + return; + } + + if (snap->def->parent && STREQ(snap->def->parent, rep->snap->def->name)) { + VIR_FREE(snap->def->parent); + + if (rep->snap->def->parent != NULL) { + snap->def->parent = strdup(rep->snap->def->parent); + + if (snap->def->parent == NULL) { + virReportOOMError(); + rep->err = -1; + return; + } + } + + rep->err = qemuDomainSnapshotWriteMetadata(rep->vm, snap, + rep->driver->snapshotDir); + } +} + static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags) { @@ -10137,6 +10177,7 @@ static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, virDomainSnapshotObjPtr snap = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; struct snap_remove rem; + struct snap_reparent rep; virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN, -1); @@ -10169,6 +10210,15 @@ static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, &rem); if (rem.err < 0) goto endjob; + } else { + rep.driver = driver; + rep.snap = snap; + rep.vm = vm; + rep.err = 0; + virHashForEach(vm->snapshots.objs, qemuDomainSnapshotReparentChildren, + &rep); + if (rep.err < 0) + goto endjob; } ret = qemuDomainSnapshotDiscard(driver, vm, snap); -- 1.7.0.4
On 12/22/2010 01:56 PM, Matthias Bolte wrote:
Shorten qemuDomainSnapshotWriteSnapshotMetadata function name and make it take a snapshot pointer instead of dealing with the current snapshot. Update other functions accordingly.
Add a qemuDomainSnapshotReparentChildren hash iterator to reparent the children of a snapshot that is being deleted. Use qemuDomainSnapshotWriteMetadata to write updated metadata to disk.
This fixes a problem where outdated parent information breaks the snapshot tree and hinders the deletion of child snapshots.
Reported by Philipp Hahn. --- src/qemu/qemu_driver.c | 92 +++++++++++++++++++++++++++++++++++++----------- 1 files changed, 71 insertions(+), 21 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 23/12/2010, at 8:13 AM, Eric Blake wrote:
On 12/22/2010 01:56 PM, Matthias Bolte wrote:
Shorten qemuDomainSnapshotWriteSnapshotMetadata function name and make it take a snapshot pointer instead of dealing with the current snapshot. Update other functions accordingly.
Add a qemuDomainSnapshotReparentChildren hash iterator to reparent the children of a snapshot that is being deleted. Use qemuDomainSnapshotWriteMetadata to write updated metadata to disk.
This fixes a problem where outdated parent information breaks the snapshot tree and hinders the deletion of child snapshots.
Reported by Philipp Hahn. --- src/qemu/qemu_driver.c | 92 +++++++++++++++++++++++++++++++++++++----------- 1 files changed, 71 insertions(+), 21 deletions(-)
ACK.
Cool. Sounds like this is the fix for BZ #663586: https://bugzilla.redhat.com/show_bug.cgi?id=663586
2010/12/22 Justin Clift <jclift@redhat.com>:
On 23/12/2010, at 8:13 AM, Eric Blake wrote:
On 12/22/2010 01:56 PM, Matthias Bolte wrote:
Shorten qemuDomainSnapshotWriteSnapshotMetadata function name and make it take a snapshot pointer instead of dealing with the current snapshot. Update other functions accordingly.
Add a qemuDomainSnapshotReparentChildren hash iterator to reparent the children of a snapshot that is being deleted. Use qemuDomainSnapshotWriteMetadata to write updated metadata to disk.
This fixes a problem where outdated parent information breaks the snapshot tree and hinders the deletion of child snapshots.
Reported by Philipp Hahn. --- src/qemu/qemu_driver.c | 92 +++++++++++++++++++++++++++++++++++++----------- 1 files changed, 71 insertions(+), 21 deletions(-)
ACK.
Cool. Sounds like this is the fix for BZ #663586:
I didn't know there is a BZ entry for it, but yes. Matthias
2010/12/22 Eric Blake <eblake@redhat.com>:
On 12/22/2010 01:56 PM, Matthias Bolte wrote:
Shorten qemuDomainSnapshotWriteSnapshotMetadata function name and make it take a snapshot pointer instead of dealing with the current snapshot. Update other functions accordingly.
Add a qemuDomainSnapshotReparentChildren hash iterator to reparent the children of a snapshot that is being deleted. Use qemuDomainSnapshotWriteMetadata to write updated metadata to disk.
This fixes a problem where outdated parent information breaks the snapshot tree and hinders the deletion of child snapshots.
Reported by Philipp Hahn. --- src/qemu/qemu_driver.c | 92 +++++++++++++++++++++++++++++++++++++----------- 1 files changed, 71 insertions(+), 21 deletions(-)
ACK.
Thanks, pushed. Matthias
participants (3)
-
Eric Blake -
Justin Clift -
Matthias Bolte