From 6766ff10dd87752d7008f3218ad537371fec4b8d Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 25 Aug 2011 15:11:03 -0600 Subject: [PATCH] snapshot: track current snapshot across restarts Audit all changes to the qemu vm->current_snapshot, and make them update the saved xml file for both the previous and the new snapshot, so that there is always at most one snapshot with 1 in the xml, and that snapshot is used as the current snapshot even across libvirtd restarts. This patch does not fix the case of virDomainSnapshotDelete(,CHILDREN) where one of the children is the current snapshot; that will be later. * src/conf/domain_conf.h (_virDomainSnapshotDef): Alter member type and name. * src/conf/domain_conf.c (virDomainSnapshotDefParseString) (virDomainSnapshotDefFormat): Update clients. * docs/schemas/domainsnapshot.rng: Tighten rng. * src/qemu/qemu_driver.c (qemuDomainSnapshotLoad): Reload current snapshot. (qemuDomainSnapshotCreateXML, qemuDomainRevertToSnapshot) (qemuDomainSnapshotDiscard): Track current snapshot. --- docs/schemas/domainsnapshot.rng | 5 +- src/conf/domain_conf.c | 6 ++- src/conf/domain_conf.h | 4 +- src/qemu/qemu_driver.c | 81 +++++++++++++++++++++++++-------- 4 files changed, 72 insertions(+), 24 deletions(-) diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng index 86bab0b969..410833f670 100644 --- a/docs/schemas/domainsnapshot.rng +++ b/docs/schemas/domainsnapshot.rng @@ -29,7 +29,10 @@ - + + 0 + 1 + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0a2c9ebafb..44212cf2d0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10960,6 +10960,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, virDomainSnapshotDefPtr ret = NULL; char *creation = NULL, *state = NULL; struct timeval tv; + int active; xml = virXMLParseCtxt(NULL, xmlStr, "domainsnapshot.xml", &ctxt); if (!xml) { @@ -11016,11 +11017,12 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, goto cleanup; } - if (virXPathLong("string(./active)", ctxt, &def->active) < 0) { + if (virXPathInt("string(./active)", ctxt, &active) < 0) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not find 'active' element")); goto cleanup; } + def->current = active != 0; } else def->creationTime = tv.tv_sec; @@ -11062,7 +11064,7 @@ char *virDomainSnapshotDefFormat(char *domain_uuid, virBufferAsprintf(&buf, " %s\n", domain_uuid); virBufferAddLit(&buf, " \n"); if (internal) - virBufferAsprintf(&buf, " %ld\n", def->active); + virBufferAsprintf(&buf, " %d\n", def->current); virBufferAddLit(&buf, "\n"); if (virBufferError(&buf)) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2cc9b0653f..8382d2839f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1297,13 +1297,15 @@ enum virDomainTaintFlags { typedef struct _virDomainSnapshotDef virDomainSnapshotDef; typedef virDomainSnapshotDef *virDomainSnapshotDefPtr; struct _virDomainSnapshotDef { + /* Public XML. */ char *name; char *description; char *parent; long long creationTime; /* in seconds */ int state; - long active; + /* Internal use. */ + bool current; /* At most one snapshot in the list should have this set */ }; typedef struct _virDomainSnapshotObj virDomainSnapshotObj; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b06a4dc350..b5268e4330 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -294,6 +294,7 @@ static void qemuDomainSnapshotLoad(void *payload, char *fullpath; virDomainSnapshotDefPtr def = NULL; virDomainSnapshotObjPtr snap = NULL; + virDomainSnapshotObjPtr current = NULL; char ebuf[1024]; virDomainObjLock(vm); @@ -339,7 +340,8 @@ static void qemuDomainSnapshotLoad(void *payload, def = virDomainSnapshotDefParseString(xmlStr, 0); if (def == NULL) { /* Nothing we can do here, skip this one */ - VIR_ERROR(_("Failed to parse snapshot XML from file '%s'"), fullpath); + VIR_ERROR(_("Failed to parse snapshot XML from file '%s'"), + fullpath); VIR_FREE(fullpath); VIR_FREE(xmlStr); continue; @@ -348,12 +350,22 @@ static void qemuDomainSnapshotLoad(void *payload, snap = virDomainSnapshotAssignDef(&vm->snapshots, def); if (snap == NULL) { virDomainSnapshotDefFree(def); + } else if (snap->def->current) { + current = snap; + if (!vm->current_snapshot) + vm->current_snapshot = snap; } VIR_FREE(fullpath); VIR_FREE(xmlStr); } + if (vm->current_snapshot != current) { + VIR_ERROR(_("Too many snapshots claiming to be current for domain %s"), + vm->def->name); + vm->current_snapshot = NULL; + } + /* FIXME: qemu keeps internal track of snapshots. We can get access * to this info via the "info snapshots" monitor command for running * domains, or via "qemu-img snapshot -l" for shutoff domains. It would @@ -8477,12 +8489,17 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain, def = NULL; snap->def->state = virDomainObjGetState(vm, NULL); + snap->def->current = true; if (vm->current_snapshot) { snap->def->parent = strdup(vm->current_snapshot->def->name); if (snap->def->parent == NULL) { virReportOOMError(); goto cleanup; } + vm->current_snapshot->def->current = false; + if (qemuDomainSnapshotWriteMetadata(vm, vm->current_snapshot, + driver->snapshotDir) < 0) + goto cleanup; vm->current_snapshot = NULL; } @@ -8502,6 +8519,7 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain, */ if (qemuDomainSnapshotWriteMetadata(vm, snap, driver->snapshotDir) < 0) goto cleanup; + vm->current_snapshot = snap; snapshot = virGetDomainSnapshot(domain, snap->def->name); @@ -8742,7 +8760,17 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, goto cleanup; } - vm->current_snapshot = snap; + if (vm->current_snapshot) { + vm->current_snapshot->def->current = false; + if (qemuDomainSnapshotWriteMetadata(vm, vm->current_snapshot, + driver->snapshotDir) < 0) + goto cleanup; + vm->current_snapshot = NULL; + /* XXX Should we restore vm->current_snapshot after this point + * in the failure cases where we know there was no change? */ + } + + snap->def->current = true; if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -8759,7 +8787,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, goto endjob; } else { rc = qemuProcessStart(snapshot->domain->conn, driver, vm, NULL, - false, false, -1, NULL, vm->current_snapshot, + false, false, -1, NULL, snap, VIR_VM_OP_CREATE); virDomainAuditStart(vm, "from-snapshot", rc >= 0); if (rc < 0) @@ -8817,6 +8845,15 @@ endjob: vm = NULL; cleanup: + if (vm && ret == 0) { + if (qemuDomainSnapshotWriteMetadata(vm, snap, + driver->snapshotDir) < 0) + ret = -1; + else + vm->current_snapshot = snap; + } else if (snap) { + snap->def->current = false; + } if (event) qemuDomainEventQueue(driver, event); if (vm) @@ -8835,7 +8872,7 @@ static int qemuDomainSnapshotDiscard(struct qemud_driver *driver, int ret = -1; int i; qemuDomainObjPrivatePtr priv; - virDomainSnapshotObjPtr parentsnap; + virDomainSnapshotObjPtr parentsnap = NULL; if (!virDomainObjIsActive(vm)) { qemuimgarg[0] = qemuFindQemuImgBinary(); @@ -8874,31 +8911,35 @@ static int qemuDomainSnapshotDiscard(struct qemud_driver *driver, qemuDomainObjExitMonitorWithDriver(driver, vm); } + if (virAsprintf(&snapFile, "%s/%s/%s.xml", driver->snapshotDir, + vm->def->name, snap->def->name) < 0) { + virReportOOMError(); + goto cleanup; + } + if (snap == vm->current_snapshot) { if (snap->def->parent) { parentsnap = virDomainSnapshotFindByName(&vm->snapshots, snap->def->parent); if (!parentsnap) { - qemuReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, - _("no domain snapshot parent with matching name '%s'"), - snap->def->parent); - goto cleanup; + VIR_WARN("missing parent snapshot matching name '%s'", + snap->def->parent); + } else { + parentsnap->def->current = true; + if (qemuDomainSnapshotWriteMetadata(vm, parentsnap, + driver->snapshotDir) < 0) { + VIR_WARN("failed to set parent snapshot '%s' as current", + snap->def->parent); + parentsnap->def->current = false; + parentsnap = NULL; + } } - - /* Now we set the new current_snapshot for the domain */ - vm->current_snapshot = parentsnap; - } else { - vm->current_snapshot = NULL; } + vm->current_snapshot = parentsnap; } - if (virAsprintf(&snapFile, "%s/%s/%s.xml", driver->snapshotDir, - vm->def->name, snap->def->name) < 0) { - virReportOOMError(); - goto cleanup; - } - unlink(snapFile); - + if (unlink(snapFile) < 0) + VIR_WARN("Failed to unlink %s", snapFile); virDomainSnapshotObjListRemove(&vm->snapshots, snap); ret = 0; -- GitLab