diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 49b0803c53..4a25480662 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2517,7 +2517,6 @@ struct _virDomainObj { virDomainDefPtr newDef; /* New definition to activate at shutdown */ virDomainSnapshotObjListPtr snapshots; - virDomainSnapshotObjPtr current_snapshot; bool hasManagedSave; diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 65094766f0..c300451e70 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -974,9 +974,9 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, return -1; } if (other) { - if (other == vm->current_snapshot) { + if (other == virDomainSnapshotGetCurrent(vm->snapshots)) { *update_current = true; - vm->current_snapshot = NULL; + virDomainSnapshotSetCurrent(vm->snapshots, NULL); } /* Drop and rebuild the parent relationship, but keep all diff --git a/src/conf/virdomainsnapshotobjlist.c b/src/conf/virdomainsnapshotobjlist.c index 1fc4049745..ed3acbc913 100644 --- a/src/conf/virdomainsnapshotobjlist.c +++ b/src/conf/virdomainsnapshotobjlist.c @@ -40,6 +40,7 @@ struct _virDomainSnapshotObjList { virHashTable *objs; virDomainSnapshotObj metaroot; /* Special parent of all root snapshots */ + virDomainSnapshotObjPtr current; /* The current snapshot, if any */ }; @@ -52,7 +53,6 @@ int virDomainSnapshotObjListParse(const char *xmlStr, const unsigned char *domain_uuid, virDomainSnapshotObjListPtr snapshots, - virDomainSnapshotObjPtr *current_snap, virCapsPtr caps, virDomainXMLOptionPtr xmlopt, unsigned int flags) @@ -64,6 +64,7 @@ virDomainSnapshotObjListParse(const char *xmlStr, int n; size_t i; int keepBlanksDefault = xmlKeepBlanksDefault(0); + virDomainSnapshotObjPtr snap; VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; VIR_AUTOFREE(char *) current = NULL; @@ -73,7 +74,7 @@ virDomainSnapshotObjListParse(const char *xmlStr, _("incorrect flags for bulk parse")); return -1; } - if (snapshots->metaroot.nchildren || *current_snap) { + if (snapshots->metaroot.nchildren || snapshots->current) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("bulk define of snapshots only possible with " "no existing snapshot")); @@ -103,7 +104,6 @@ virDomainSnapshotObjListParse(const char *xmlStr, for (i = 0; i < n; i++) { virDomainSnapshotDefPtr def; - virDomainSnapshotObjPtr snap; def = virDomainSnapshotDefParseNode(xml, nodes[i], caps, xmlopt, NULL, flags); @@ -126,12 +126,13 @@ virDomainSnapshotObjListParse(const char *xmlStr, } if (current) { - if (!(*current_snap = virDomainSnapshotFindByName(snapshots, - current))) { + snap = virDomainSnapshotFindByName(snapshots, current); + if (!snap) { virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, _("no snapshot matching current='%s'"), current); goto cleanup; } + virDomainSnapshotSetCurrent(snapshots, snap); } ret = n; @@ -181,7 +182,6 @@ int virDomainSnapshotObjListFormat(virBufferPtr buf, const char *uuidstr, virDomainSnapshotObjListPtr snapshots, - virDomainSnapshotObjPtr current_snapshot, virCapsPtr caps, virDomainXMLOptionPtr xmlopt, unsigned int flags) @@ -196,9 +196,8 @@ virDomainSnapshotObjListFormat(virBufferPtr buf, virCheckFlags(VIR_DOMAIN_SNAPSHOT_FORMAT_SECURE, -1); virBufferAddLit(buf, "def->name); + virBufferEscapeString(buf, " current='%s'", + virDomainSnapshotGetCurrentName(snapshots)); virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); if (virDomainSnapshotForEach(snapshots, virDomainSnapshotFormatOne, @@ -436,10 +435,53 @@ virDomainSnapshotFindByName(virDomainSnapshotObjListPtr snapshots, return name ? virHashLookup(snapshots->objs, name) : &snapshots->metaroot; } -void virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots, - virDomainSnapshotObjPtr snapshot) + +/* Return the current snapshot, or NULL */ +virDomainSnapshotObjPtr +virDomainSnapshotGetCurrent(virDomainSnapshotObjListPtr snapshots) { + return snapshots->current; +} + + +/* Return the current snapshot's name, or NULL */ +const char * +virDomainSnapshotGetCurrentName(virDomainSnapshotObjListPtr snapshots) +{ + if (snapshots->current) + return snapshots->current->def->name; + return NULL; +} + + +/* Return true if name matches the current snapshot */ +bool +virDomainSnapshotIsCurrentName(virDomainSnapshotObjListPtr snapshots, + const char *name) +{ + return snapshots->current && STREQ(snapshots->current->def->name, name); +} + + +/* Update the current snapshot, using NULL if no current remains */ +void +virDomainSnapshotSetCurrent(virDomainSnapshotObjListPtr snapshots, + virDomainSnapshotObjPtr snapshot) +{ + snapshots->current = snapshot; +} + + +/* Remove snapshot from the list; return true if it was current */ +bool +virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots, + virDomainSnapshotObjPtr snapshot) +{ + bool ret = snapshots->current == snapshot; virHashRemoveEntry(snapshots->objs, snapshot->def->name); + if (ret) + snapshots->current = NULL; + return ret; } int @@ -506,6 +548,8 @@ virDomainSnapshotUpdateRelations(virDomainSnapshotObjListPtr snapshots) snapshots->metaroot.nchildren = 0; snapshots->metaroot.first_child = NULL; virHashForEach(snapshots->objs, virDomainSnapshotSetRelations, &act); + if (act.err) + snapshots->current = NULL; return act.err; } diff --git a/src/conf/virdomainsnapshotobjlist.h b/src/conf/virdomainsnapshotobjlist.h index 2a1ee86586..e210849441 100644 --- a/src/conf/virdomainsnapshotobjlist.h +++ b/src/conf/virdomainsnapshotobjlist.h @@ -33,14 +33,12 @@ void virDomainSnapshotObjListFree(virDomainSnapshotObjListPtr snapshots); int virDomainSnapshotObjListParse(const char *xmlStr, const unsigned char *domain_uuid, virDomainSnapshotObjListPtr snapshots, - virDomainSnapshotObjPtr *current_snap, virCapsPtr caps, virDomainXMLOptionPtr xmlopt, unsigned int flags); int virDomainSnapshotObjListFormat(virBufferPtr buf, const char *uuidstr, virDomainSnapshotObjListPtr snapshots, - virDomainSnapshotObjPtr current_snapshot, virCapsPtr caps, virDomainXMLOptionPtr xmlopt, unsigned int flags); @@ -57,7 +55,13 @@ int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots, unsigned int flags); virDomainSnapshotObjPtr virDomainSnapshotFindByName(virDomainSnapshotObjListPtr snapshots, const char *name); -void virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots, +virDomainSnapshotObjPtr virDomainSnapshotGetCurrent(virDomainSnapshotObjListPtr snapshots); +const char *virDomainSnapshotGetCurrentName(virDomainSnapshotObjListPtr snapshots); +bool virDomainSnapshotIsCurrentName(virDomainSnapshotObjListPtr snapshots, + const char *name); +void virDomainSnapshotSetCurrent(virDomainSnapshotObjListPtr snapshots, + virDomainSnapshotObjPtr snapshot); +bool virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots, virDomainSnapshotObjPtr snapshot); int virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots, virHashIterator iter, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a33f9e61b1..41d17226f9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -990,6 +990,9 @@ virDomainListSnapshots; virDomainSnapshotAssignDef; virDomainSnapshotFindByName; virDomainSnapshotForEach; +virDomainSnapshotGetCurrent; +virDomainSnapshotGetCurrentName; +virDomainSnapshotIsCurrentName; virDomainSnapshotObjListFormat; virDomainSnapshotObjListFree; virDomainSnapshotObjListGetNames; @@ -997,6 +1000,7 @@ virDomainSnapshotObjListNew; virDomainSnapshotObjListNum; virDomainSnapshotObjListParse; virDomainSnapshotObjListRemove; +virDomainSnapshotSetCurrent; virDomainSnapshotUpdateRelations; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 76b013f58a..511b17cc83 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8461,7 +8461,7 @@ qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, unsigned int flags = VIR_DOMAIN_SNAPSHOT_FORMAT_SECURE | VIR_DOMAIN_SNAPSHOT_FORMAT_INTERNAL; - if (vm->current_snapshot == snapshot) + if (virDomainSnapshotGetCurrent(vm->snapshots) == snapshot) flags |= VIR_DOMAIN_SNAPSHOT_FORMAT_CURRENT; virUUIDFormat(vm->def->uuid, uuidstr); newxml = virDomainSnapshotDefFormat(uuidstr, snapshot->def, caps, xmlopt, @@ -8614,8 +8614,8 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver, vm->def->name, snap->def->name) < 0) goto cleanup; - if (snap == vm->current_snapshot) { - vm->current_snapshot = NULL; + if (snap == virDomainSnapshotGetCurrent(vm->snapshots)) { + virDomainSnapshotSetCurrent(vm->snapshots, NULL); if (update_parent && snap->def->parent) { parentsnap = virDomainSnapshotFindByName(vm->snapshots, snap->def->parent); @@ -8623,13 +8623,13 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver, VIR_WARN("missing parent snapshot matching name '%s'", snap->def->parent); } else { - vm->current_snapshot = parentsnap; + virDomainSnapshotSetCurrent(vm->snapshots, parentsnap); if (qemuDomainSnapshotWriteMetadata(vm, parentsnap, driver->caps, driver->xmlopt, cfg->snapshotDir) < 0) { VIR_WARN("failed to set parent snapshot '%s' as current", snap->def->parent); - vm->current_snapshot = NULL; + virDomainSnapshotSetCurrent(vm->snapshots, NULL); } } } @@ -8658,7 +8658,7 @@ int qemuDomainSnapshotDiscardAll(void *payload, virQEMUSnapRemovePtr curr = data; int err; - if (curr->vm->current_snapshot == snap) + if (virDomainSnapshotGetCurrent(curr->vm->snapshots) == snap) curr->current = true; err = qemuDomainSnapshotDiscard(curr->driver, curr->vm, snap, false, curr->metadata_only); @@ -8679,8 +8679,6 @@ qemuDomainSnapshotDiscardAllMetadata(virQEMUDriverPtr driver, rem.err = 0; virDomainSnapshotForEach(vm->snapshots, qemuDomainSnapshotDiscardAll, &rem); - if (rem.current) - vm->current_snapshot = NULL; if (virDomainSnapshotUpdateRelations(vm->snapshots) < 0 && !rem.err) rem.err = -1; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e49924ce1a..3479098e9c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -497,7 +497,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, _("Failed to fully read directory %s"), snapDir); - vm->current_snapshot = current; + virDomainSnapshotSetCurrent(vm->snapshots, current); if (virDomainSnapshotUpdateRelations(vm->snapshots) < 0) virReportError(VIR_ERR_INTERNAL_ERROR, _("Snapshots have inconsistent relations for domain %s"), @@ -15854,13 +15854,13 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, def = NULL; } - current = vm->current_snapshot; + current = virDomainSnapshotGetCurrent(vm->snapshots); if (current) { if (!redefine && VIR_STRDUP(snap->def->parent, current->def->name) < 0) goto endjob; if (update_current) { - vm->current_snapshot = NULL; + virDomainSnapshotSetCurrent(vm->snapshots, NULL); if (qemuDomainSnapshotWriteMetadata(vm, current, driver->caps, driver->xmlopt, cfg->snapshotDir) < 0) @@ -15911,7 +15911,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, endjob: if (snapshot && !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA)) { if (update_current) - vm->current_snapshot = snap; + virDomainSnapshotSetCurrent(vm->snapshots, snap); if (qemuDomainSnapshotWriteMetadata(vm, snap, driver->caps, driver->xmlopt, cfg->snapshotDir) < 0) { @@ -15923,7 +15923,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, _("unable to save metadata for snapshot %s"), snap->def->name); virDomainSnapshotObjListRemove(vm->snapshots, snap); - vm->current_snapshot = NULL; } else { other = virDomainSnapshotFindByName(vm->snapshots, snap->def->parent); @@ -16162,7 +16161,7 @@ qemuDomainHasCurrentSnapshot(virDomainPtr domain, if (virDomainHasCurrentSnapshotEnsureACL(domain->conn, vm->def) < 0) goto cleanup; - ret = (vm->current_snapshot != NULL); + ret = (virDomainSnapshotGetCurrent(vm->snapshots) != NULL); cleanup: virDomainObjEndAPI(&vm); @@ -16219,13 +16218,13 @@ qemuDomainSnapshotCurrent(virDomainPtr domain, if (virDomainSnapshotCurrentEnsureACL(domain->conn, vm->def) < 0) goto cleanup; - if (!vm->current_snapshot) { + if (!virDomainSnapshotGetCurrent(vm->snapshots)) { virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, "%s", _("the domain does not have a current snapshot")); goto cleanup; } - snapshot = virGetDomainSnapshot(domain, vm->current_snapshot->def->name); + snapshot = virGetDomainSnapshot(domain, virDomainSnapshotGetCurrentName(vm->snapshots)); cleanup: virDomainObjEndAPI(&vm); @@ -16285,8 +16284,7 @@ qemuDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot))) goto cleanup; - ret = (vm->current_snapshot && - STREQ(snapshot->name, vm->current_snapshot->def->name)); + ret = virDomainSnapshotIsCurrentName(vm->snapshots, snapshot->name); cleanup: virDomainObjEndAPI(&vm); @@ -16439,14 +16437,14 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } } - current = vm->current_snapshot; + current = virDomainSnapshotGetCurrent(vm->snapshots); if (current) { - vm->current_snapshot = NULL; + virDomainSnapshotSetCurrent(vm->snapshots, NULL); if (qemuDomainSnapshotWriteMetadata(vm, current, driver->caps, driver->xmlopt, cfg->snapshotDir) < 0) goto endjob; - /* XXX Should we restore vm->current_snapshot after this point + /* XXX Should we restore the current snapshot after this point * in the failure cases where we know there was no change? */ } @@ -16455,7 +16453,6 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, * * XXX Should domain snapshots track live xml rather * than inactive xml? */ - vm->current_snapshot = snap; if (snap->def->dom) { config = virDomainDefCopy(snap->def->dom, caps, driver->xmlopt, NULL, true); @@ -16726,15 +16723,15 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, cleanup: if (ret == 0) { - vm->current_snapshot = snap; + virDomainSnapshotSetCurrent(vm->snapshots, snap); if (qemuDomainSnapshotWriteMetadata(vm, snap, driver->caps, driver->xmlopt, cfg->snapshotDir) < 0) { - vm->current_snapshot = NULL; + virDomainSnapshotSetCurrent(vm->snapshots, NULL); ret = -1; } } else if (snap) { - vm->current_snapshot = NULL; + virDomainSnapshotSetCurrent(vm->snapshots, NULL); } if (ret == 0 && config && vm->persistent && !(ret = virDomainSaveConfig(cfg->configDir, driver->caps, @@ -16862,7 +16859,7 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, if (rem.err < 0) goto endjob; if (rem.current) { - vm->current_snapshot = snap; + virDomainSnapshotSetCurrent(vm->snapshots, snap); if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) { if (qemuDomainSnapshotWriteMetadata(vm, snap, driver->caps, driver->xmlopt, @@ -16870,7 +16867,7 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to set snapshot '%s' as current"), snap->def->name); - vm->current_snapshot = NULL; + virDomainSnapshotSetCurrent(vm->snapshots, NULL); goto endjob; } } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 25f1d78f01..f73648fbe1 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -845,13 +845,13 @@ testParseDomainSnapshots(testDriverPtr privconn, } if (cur) { - if (domobj->current_snapshot) { + if (virDomainSnapshotGetCurrent(domobj->snapshots)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("more than one snapshot claims to be active")); goto error; } - domobj->current_snapshot = snap; + virDomainSnapshotSetCurrent(domobj->snapshots, snap); } } @@ -6151,7 +6151,7 @@ testDomainHasCurrentSnapshot(virDomainPtr domain, if (!(vm = testDomObjFromDomain(domain))) return -1; - ret = (vm->current_snapshot != NULL); + ret = (virDomainSnapshotGetCurrent(vm->snapshots) != NULL); virDomainObjEndAPI(&vm); return ret; @@ -6193,19 +6193,21 @@ testDomainSnapshotCurrent(virDomainPtr domain, { virDomainObjPtr vm; virDomainSnapshotPtr snapshot = NULL; + virDomainSnapshotObjPtr current; virCheckFlags(0, NULL); if (!(vm = testDomObjFromDomain(domain))) return NULL; - if (!vm->current_snapshot) { + current = virDomainSnapshotGetCurrent(vm->snapshots); + if (!current) { virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, "%s", _("the domain does not have a current snapshot")); goto cleanup; } - snapshot = virGetDomainSnapshot(domain, vm->current_snapshot->def->name); + snapshot = virGetDomainSnapshot(domain, current->def->name); cleanup: virDomainObjEndAPI(&vm); @@ -6253,8 +6255,7 @@ testDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, if (!(vm = testDomObjFromSnapshot(snapshot))) return -1; - ret = (vm->current_snapshot && - STREQ(snapshot->name, vm->current_snapshot->def->name)); + ret = virDomainSnapshotIsCurrentName(vm->snapshots, snapshot->name); virDomainObjEndAPI(&vm); return ret; @@ -6393,9 +6394,8 @@ testDomainSnapshotCreateXML(virDomainPtr domain, } if (!redefine) { - if (vm->current_snapshot && - (VIR_STRDUP(snap->def->parent, - vm->current_snapshot->def->name) < 0)) + if (VIR_STRDUP(snap->def->parent, + virDomainSnapshotGetCurrentName(vm->snapshots)) < 0) goto cleanup; if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT) && @@ -6413,7 +6413,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain, if (snapshot) { virDomainSnapshotObjPtr other; if (update_current) - vm->current_snapshot = snap; + virDomainSnapshotSetCurrent(vm->snapshots, snap); other = virDomainSnapshotFindByName(vm->snapshots, snap->def->parent); snap->parent = other; @@ -6444,9 +6444,7 @@ testDomainSnapshotDiscardAll(void *payload, virDomainSnapshotObjPtr snap = payload; testSnapRemoveDataPtr curr = data; - if (curr->vm->current_snapshot == snap) - curr->current = true; - virDomainSnapshotObjListRemove(curr->vm->snapshots, snap); + curr->current |= virDomainSnapshotObjListRemove(curr->vm->snapshots, snap); return 0; } @@ -6511,7 +6509,7 @@ testDomainSnapshotDelete(virDomainSnapshotPtr snapshot, testDomainSnapshotDiscardAll, &rem); if (rem.current) - vm->current_snapshot = snap; + virDomainSnapshotSetCurrent(vm->snapshots, snap); } else if (snap->nchildren) { testSnapReparentData rep; rep.parent = snap->parent; @@ -6535,7 +6533,7 @@ testDomainSnapshotDelete(virDomainSnapshotPtr snapshot, snap->first_child = NULL; } else { virDomainSnapshotDropParent(snap); - if (snap == vm->current_snapshot) { + if (snap == virDomainSnapshotGetCurrent(vm->snapshots)) { if (snap->def->parent) { parentsnap = virDomainSnapshotFindByName(vm->snapshots, snap->def->parent); @@ -6543,7 +6541,7 @@ testDomainSnapshotDelete(virDomainSnapshotPtr snapshot, VIR_WARN("missing parent snapshot matching name '%s'", snap->def->parent); } - vm->current_snapshot = parentsnap; + virDomainSnapshotSetCurrent(vm->snapshots, parentsnap); } virDomainSnapshotObjListRemove(vm->snapshots, snap); } @@ -6619,9 +6617,7 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } } - - if (vm->current_snapshot) - vm->current_snapshot = NULL; + virDomainSnapshotSetCurrent(vm->snapshots, NULL); config = virDomainDefCopy(snap->def->dom, privconn->caps, privconn->xmlopt, NULL, true); @@ -6746,7 +6742,7 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } } - vm->current_snapshot = snap; + virDomainSnapshotSetCurrent(vm->snapshots, snap); ret = 0; cleanup: if (event) {