snapshot: Add accessors for updating snapshot list relations

Rather than allowing a leaky abstraction where multiple drivers have
to open-code operations that update the relations in a
virDomainSnapshotObjList, it is better to add accessor functions so
that updates to relations are maintained closer to the internals.
This patch finishes the job started in the previous patch, by getting
rid of all direct access to nchildren, first_child, or sibling outside
of the lowest level functions, making it easier to refactor later on.

The lone new caller to virDomainSnapshotObjListSize() checks for a
return != 0, because it wants to handles errors (-1, only possible if
the hash table wasn't allocated) and existing snapshots (> 0) in the
same manner; we can drop the check for a current snapshot on the
grounds that there shouldn't be one if there are no snapshots.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This commit is contained in:
Eric Blake 2019-03-16 22:38:33 -05:00
parent ced0898f86
commit 02c4e24db7
8 changed files with 61 additions and 29 deletions

View File

@ -123,6 +123,27 @@ virDomainSnapshotDropParent(virDomainSnapshotObjPtr snapshot)
}
/* Update @snapshot to no longer have children. */
void
virDomainSnapshotDropChildren(virDomainSnapshotObjPtr snapshot)
{
snapshot->nchildren = 0;
snapshot->first_child = NULL;
}
/* Add @snapshot to @parent's list of children. */
void
virDomainSnapshotSetParent(virDomainSnapshotObjPtr snapshot,
virDomainSnapshotObjPtr parent)
{
snapshot->parent = parent;
parent->nchildren++;
snapshot->sibling = parent->first_child;
parent->first_child = snapshot;
}
/* Take all children of @from and convert them into children of @to. */
void
virDomainSnapshotMoveChildren(virDomainSnapshotObjPtr from,

View File

@ -46,7 +46,10 @@ int virDomainSnapshotForEachDescendant(virDomainSnapshotObjPtr snapshot,
virHashIterator iter,
void *data);
void virDomainSnapshotDropParent(virDomainSnapshotObjPtr snapshot);
void virDomainSnapshotDropChildren(virDomainSnapshotObjPtr snapshot);
void virDomainSnapshotMoveChildren(virDomainSnapshotObjPtr from,
virDomainSnapshotObjPtr to);
void virDomainSnapshotSetParent(virDomainSnapshotObjPtr snapshot,
virDomainSnapshotObjPtr parent);
#endif /* LIBVIRT_VIRDOMAINSNAPSHOTOBJ_H */

View File

@ -74,7 +74,7 @@ virDomainSnapshotObjListParse(const char *xmlStr,
_("incorrect flags for bulk parse"));
return -1;
}
if (snapshots->metaroot.nchildren || snapshots->current) {
if (virDomainSnapshotObjListSize(snapshots) != 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("bulk define of snapshots only possible with "
"no existing snapshot"));
@ -140,9 +140,7 @@ virDomainSnapshotObjListParse(const char *xmlStr,
if (ret < 0) {
/* There were no snapshots before this call; so on error, just
* blindly delete anything created before the failure. */
virHashRemoveAll(snapshots->objs);
snapshots->metaroot.nchildren = 0;
snapshots->metaroot.first_child = NULL;
virDomainSnapshotObjListRemoveAll(snapshots);
}
xmlXPathFreeContext(ctxt);
xmlFreeDoc(xml);
@ -436,6 +434,14 @@ virDomainSnapshotFindByName(virDomainSnapshotObjListPtr snapshots,
}
/* Return the number of objects currently in the list */
int
virDomainSnapshotObjListSize(virDomainSnapshotObjListPtr snapshots)
{
return virHashSize(snapshots->objs);
}
/* Return the current snapshot, or NULL */
virDomainSnapshotObjPtr
virDomainSnapshotGetCurrent(virDomainSnapshotObjListPtr snapshots)
@ -484,6 +490,15 @@ virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots,
return ret;
}
/* Remove all snapshots tracked in the list */
void
virDomainSnapshotObjListRemoveAll(virDomainSnapshotObjListPtr snapshots)
{
virHashRemoveAll(snapshots->objs);
virDomainSnapshotDropChildren(&snapshots->metaroot);
}
int
virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots,
virHashIterator iter,
@ -511,28 +526,26 @@ virDomainSnapshotSetRelations(void *payload,
virDomainSnapshotObjPtr obj = payload;
struct snapshot_set_relation *curr = data;
virDomainSnapshotObjPtr tmp;
virDomainSnapshotObjPtr parent;
obj->parent = virDomainSnapshotFindByName(curr->snapshots,
obj->def->parent);
if (!obj->parent) {
parent = virDomainSnapshotFindByName(curr->snapshots, obj->def->parent);
if (!parent) {
curr->err = -1;
obj->parent = &curr->snapshots->metaroot;
parent = &curr->snapshots->metaroot;
VIR_WARN("snapshot %s lacks parent", obj->def->name);
} else {
tmp = obj->parent;
tmp = parent;
while (tmp && tmp->def) {
if (tmp == obj) {
curr->err = -1;
obj->parent = &curr->snapshots->metaroot;
parent = &curr->snapshots->metaroot;
VIR_WARN("snapshot %s in circular chain", obj->def->name);
break;
}
tmp = tmp->parent;
}
}
obj->parent->nchildren++;
obj->sibling = obj->parent->first_child;
obj->parent->first_child = obj;
virDomainSnapshotSetParent(obj, parent);
return 0;
}
@ -545,8 +558,7 @@ virDomainSnapshotUpdateRelations(virDomainSnapshotObjListPtr snapshots)
{
struct snapshot_set_relation act = { snapshots, 0 };
snapshots->metaroot.nchildren = 0;
snapshots->metaroot.first_child = NULL;
virDomainSnapshotDropChildren(&snapshots->metaroot);
virHashForEach(snapshots->objs, virDomainSnapshotSetRelations, &act);
if (act.err)
snapshots->current = NULL;

View File

@ -55,6 +55,7 @@ int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots,
unsigned int flags);
virDomainSnapshotObjPtr virDomainSnapshotFindByName(virDomainSnapshotObjListPtr snapshots,
const char *name);
int virDomainSnapshotObjListSize(virDomainSnapshotObjListPtr snapshots);
virDomainSnapshotObjPtr virDomainSnapshotGetCurrent(virDomainSnapshotObjListPtr snapshots);
const char *virDomainSnapshotGetCurrentName(virDomainSnapshotObjListPtr snapshots);
bool virDomainSnapshotIsCurrentName(virDomainSnapshotObjListPtr snapshots,
@ -63,6 +64,7 @@ void virDomainSnapshotSetCurrent(virDomainSnapshotObjListPtr snapshots,
virDomainSnapshotObjPtr snapshot);
bool virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots,
virDomainSnapshotObjPtr snapshot);
void virDomainSnapshotObjListRemoveAll(virDomainSnapshotObjListPtr snapshots);
int virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots,
virHashIterator iter,
void *data);

View File

@ -980,10 +980,12 @@ virDomainObjListRename;
# conf/virdomainsnapshotobj.h
virDomainSnapshotDropChildren;
virDomainSnapshotDropParent;
virDomainSnapshotForEachChild;
virDomainSnapshotForEachDescendant;
virDomainSnapshotMoveChildren;
virDomainSnapshotSetParent;
# conf/virdomainsnapshotobjlist.h
@ -1001,6 +1003,7 @@ virDomainSnapshotObjListNew;
virDomainSnapshotObjListNum;
virDomainSnapshotObjListParse;
virDomainSnapshotObjListRemove;
virDomainSnapshotObjListRemoveAll;
virDomainSnapshotSetCurrent;
virDomainSnapshotUpdateRelations;

View File

@ -8679,8 +8679,7 @@ qemuDomainSnapshotDiscardAllMetadata(virQEMUDriverPtr driver,
rem.err = 0;
virDomainSnapshotForEach(vm->snapshots, qemuDomainSnapshotDiscardAll,
&rem);
if (virDomainSnapshotUpdateRelations(vm->snapshots) < 0 && !rem.err)
rem.err = -1;
virDomainSnapshotObjListRemoveAll(vm->snapshots);
return rem.err;
}

View File

@ -15926,10 +15926,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
} else {
other = virDomainSnapshotFindByName(vm->snapshots,
snap->def->parent);
snap->parent = other;
other->nchildren++;
snap->sibling = other->first_child;
other->first_child = snap;
virDomainSnapshotSetParent(snap, other);
}
} else if (snap) {
virDomainSnapshotObjListRemove(vm->snapshots, snap);
@ -16883,8 +16880,7 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
}
if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) {
snap->nchildren = 0;
snap->first_child = NULL;
virDomainSnapshotDropChildren(snap);
ret = 0;
} else {
ret = qemuDomainSnapshotDiscard(driver, vm, snap, true, metadata_only);

View File

@ -6416,10 +6416,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain,
virDomainSnapshotSetCurrent(vm->snapshots, snap);
other = virDomainSnapshotFindByName(vm->snapshots,
snap->def->parent);
snap->parent = other;
other->nchildren++;
snap->sibling = other->first_child;
other->first_child = snap;
virDomainSnapshotSetParent(snap, other);
}
virDomainObjEndAPI(&vm);
}
@ -6521,8 +6518,7 @@ testDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
}
if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) {
snap->nchildren = 0;
snap->first_child = NULL;
virDomainSnapshotDropChildren(snap);
} else {
virDomainSnapshotDropParent(snap);
if (snap == virDomainSnapshotGetCurrent(vm->snapshots)) {