diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng index 86bab0b969df9e8ff5e3895d89fa0b10a5ef5159..410833f670234e1eb6ec2d68f4e8a1b0e2030fb3 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 0a2c9ebafb8d4d96ef8d2218d9360fc5dd1875da..44212cf2d009699192216da526c8f938c9f0b981 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 2cc9b0653fdba38209ca996e8cccc9280812d19b..8382d2839ff75940aea0cbd5d669dba274771575 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 b06a4dc350ef2597602b777b362f85649f3dda9c..b5268e4330f128d9acced3720da85848ee13f114 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;