snapshot: use metaroot node to simplify management

This idea was first suggested by Daniel Veillard here:
https://www.redhat.com/archives/libvir-list/2011-October/msg00353.html

Now that I am about to add more complexity to snapshot listing, it
makes sense to avoid code duplication and special casing for domain
listing (all snapshots) vs. snapshot listing (descendants); adding
a metaroot reduces the number of code lines by having the domain
listing turn into a descendant listing of the metaroot.

Note that this has one minor pessimization - if we are going to list
ALL snapshots without filtering, then virHashForeach is more efficient
than recursing through the child relationships; restoring that minor
optimization will occur in the next patch.

* src/conf/domain_conf.h (_virDomainSnapshotObj)
(_virDomainSnapshotObjList): Repurpose some fields.
(virDomainSnapshotDropParent): Drop unused parameter.
* src/conf/domain_conf.c (virDomainSnapshotObjListGetNames)
(virDomainSnapshotObjListCount): Simplify.
(virDomainSnapshotFindByName, virDomainSnapshotSetRelations)
(virDomainSnapshotDropParent): Match new field semantics.
* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML)
(qemuDomainSnapshotReparentChildren, qemuDomainSnapshotDelete):
Adjust clients.
This commit is contained in:
Eric Blake 2012-06-13 13:13:02 -06:00
parent 7dcee3f956
commit 06d4a1e429
3 changed files with 59 additions and 123 deletions

View File

@ -14272,8 +14272,9 @@ static void virDomainSnapshotObjListCopyNames(void *payload,
if (data->oom)
return;
/* LIST_ROOTS/LIST_DESCENDANTS was handled by caller,
* LIST_METADATA is a no-op if we get this far. */
/* LIST_ROOTS/LIST_DESCENDANTS was handled by the choice of
* iteration made in the caller, and LIST_METADATA is a no-op if
* we get this far. */
if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_LEAVES) && obj->nchildren)
return;
@ -14289,32 +14290,12 @@ int virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots,
char **const names, int maxnames,
unsigned int flags)
{
struct virDomainSnapshotNameData data = { 0, 0, maxnames, names, 0 };
int i;
data.flags = flags & ~VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
if (!(flags & VIR_DOMAIN_SNAPSHOT_LIST_ROOTS)) {
virHashForEach(snapshots->objs, virDomainSnapshotObjListCopyNames,
&data);
} else {
virDomainSnapshotObjPtr root = snapshots->first_root;
while (root) {
virDomainSnapshotObjListCopyNames(root, root->def->name, &data);
root = root->sibling;
}
}
if (data.oom) {
virReportOOMError();
goto cleanup;
}
return data.numnames;
cleanup:
for (i = 0; i < data.numnames; i++)
VIR_FREE(data.names[i]);
return -1;
/* LIST_ROOTS and LIST_DESCENDANTS have the same bit value, but
* opposite semantics. Toggle here to get the correct traversal
* on the metaroot. */
flags ^= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
return virDomainSnapshotObjListGetNamesFrom(&snapshots->metaroot, names,
maxnames, flags);
}
int virDomainSnapshotObjListGetNamesFrom(virDomainSnapshotObjPtr snapshot,
@ -14359,8 +14340,9 @@ static void virDomainSnapshotObjListCount(void *payload,
virDomainSnapshotObjPtr obj = payload;
struct virDomainSnapshotNumData *data = opaque;
/* LIST_ROOTS/LIST_DESCENDANTS was handled by caller,
* LIST_METADATA is a no-op if we get this far. */
/* LIST_ROOTS/LIST_DESCENDANTS was handled by the choice of
* iteration made in the caller, and LIST_METADATA is a no-op if
* we get this far. */
if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_LEAVES) && obj->nchildren)
return;
data->count++;
@ -14369,23 +14351,11 @@ static void virDomainSnapshotObjListCount(void *payload,
int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots,
unsigned int flags)
{
struct virDomainSnapshotNumData data = { 0, 0 };
data.flags = flags & ~VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
if (!(flags & VIR_DOMAIN_SNAPSHOT_LIST_ROOTS)) {
virHashForEach(snapshots->objs, virDomainSnapshotObjListCount, &data);
} else if (data.flags) {
virDomainSnapshotObjPtr root = snapshots->first_root;
while (root) {
virDomainSnapshotObjListCount(root, root->def->name, &data);
root = root->sibling;
}
} else {
data.count = snapshots->nroots;
}
return data.count;
/* LIST_ROOTS and LIST_DESCENDANTS have the same bit value, but
* opposite semantics. Toggle here to get the correct traversal
* on the metaroot. */
flags ^= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
return virDomainSnapshotObjListNumFrom(&snapshots->metaroot, flags);
}
int
@ -14413,7 +14383,7 @@ virDomainSnapshotObjPtr
virDomainSnapshotFindByName(const virDomainSnapshotObjListPtr snapshots,
const char *name)
{
return virHashLookup(snapshots->objs, name);
return name ? virHashLookup(snapshots->objs, name) : &snapshots->metaroot;
}
void virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots,
@ -14499,34 +14469,27 @@ virDomainSnapshotSetRelations(void *payload,
struct snapshot_set_relation *curr = data;
virDomainSnapshotObjPtr tmp;
if (obj->def->parent) {
obj->parent = virDomainSnapshotFindByName(curr->snapshots,
obj->def->parent);
if (!obj->parent) {
curr->err = -1;
VIR_WARN("snapshot %s lacks parent", obj->def->name);
} else {
tmp = obj->parent;
while (tmp) {
if (tmp == obj) {
curr->err = -1;
obj->parent = NULL;
VIR_WARN("snapshot %s in circular chain", obj->def->name);
break;
}
tmp = tmp->parent;
}
if (!tmp) {
obj->parent->nchildren++;
obj->sibling = obj->parent->first_child;
obj->parent->first_child = obj;
}
}
obj->parent = virDomainSnapshotFindByName(curr->snapshots,
obj->def->parent);
if (!obj->parent) {
curr->err = -1;
obj->parent = &curr->snapshots->metaroot;
VIR_WARN("snapshot %s lacks parent", obj->def->name);
} else {
curr->snapshots->nroots++;
obj->sibling = curr->snapshots->first_root;
curr->snapshots->first_root = obj;
tmp = obj->parent;
while (tmp && tmp->def) {
if (tmp == obj) {
curr->err = -1;
obj->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;
}
/* Populate parent link and child count of all snapshots, with all
@ -14546,28 +14509,13 @@ virDomainSnapshotUpdateRelations(virDomainSnapshotObjListPtr snapshots)
* of a parent, it is faster to just 0 the count rather than calling
* this function on each child. */
void
virDomainSnapshotDropParent(virDomainSnapshotObjListPtr snapshots,
virDomainSnapshotObjPtr snapshot)
virDomainSnapshotDropParent(virDomainSnapshotObjPtr snapshot)
{
virDomainSnapshotObjPtr prev = NULL;
virDomainSnapshotObjPtr curr = NULL;
size_t *count;
virDomainSnapshotObjPtr *first;
if (snapshot->parent) {
count = &snapshot->parent->nchildren;
first = &snapshot->parent->first_child;
} else {
count = &snapshots->nroots;
first = &snapshots->first_root;
}
if (!*count || !*first) {
VIR_WARN("inconsistent snapshot relations");
return;
}
(*count)--;
curr = *first;
snapshot->parent->nchildren--;
curr = snapshot->parent->first_child;
while (curr != snapshot) {
if (!curr) {
VIR_WARN("inconsistent snapshot relations");
@ -14579,7 +14527,7 @@ virDomainSnapshotDropParent(virDomainSnapshotObjListPtr snapshots,
if (prev)
prev->sibling = snapshot->sibling;
else
*first = snapshot->sibling;
snapshot->parent->first_child = snapshot->sibling;
snapshot->parent = NULL;
snapshot->sibling = NULL;
}

View File

@ -1726,9 +1726,11 @@ struct _virDomainSnapshotDef {
typedef struct _virDomainSnapshotObj virDomainSnapshotObj;
typedef virDomainSnapshotObj *virDomainSnapshotObjPtr;
struct _virDomainSnapshotObj {
virDomainSnapshotDefPtr def;
virDomainSnapshotDefPtr def; /* non-NULL except for metaroot */
virDomainSnapshotObjPtr parent; /* NULL if root */
virDomainSnapshotObjPtr parent; /* non-NULL except for metaroot, before
virDomainSnapshotUpdateRelations, or
after virDomainSnapshotDropParent */
virDomainSnapshotObjPtr sibling; /* NULL if last child of parent */
size_t nchildren;
virDomainSnapshotObjPtr first_child; /* NULL if no children */
@ -1741,8 +1743,7 @@ struct _virDomainSnapshotObjList {
* for O(1), lockless lookup-by-name */
virHashTable *objs;
size_t nroots;
virDomainSnapshotObjPtr first_root;
virDomainSnapshotObj metaroot; /* Special parent of all root snapshots */
};
typedef enum {
@ -1788,8 +1789,7 @@ int virDomainSnapshotForEachDescendant(virDomainSnapshotObjPtr snapshot,
virHashIterator iter,
void *data);
int virDomainSnapshotUpdateRelations(virDomainSnapshotObjListPtr snapshots);
void virDomainSnapshotDropParent(virDomainSnapshotObjListPtr snapshots,
virDomainSnapshotObjPtr snapshot);
void virDomainSnapshotDropParent(virDomainSnapshotObjPtr snapshot);
/* Guest VM runtime state */
typedef struct _virDomainStateReason virDomainStateReason;

View File

@ -10514,7 +10514,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
}
/* Drop and rebuild the parent relationship, but keep all
* child relations by reusing snap. */
virDomainSnapshotDropParent(&vm->snapshots, other);
virDomainSnapshotDropParent(other);
virDomainSnapshotDefFree(other->def);
other->def = NULL;
snap = other;
@ -10623,18 +10623,12 @@ cleanup:
} else {
if (update_current)
vm->current_snapshot = snap;
if (snap->def->parent) {
other = virDomainSnapshotFindByName(&vm->snapshots,
snap->def->parent);
snap->parent = other;
other->nchildren++;
snap->sibling = other->first_child;
other->first_child = snap;
} else {
vm->snapshots.nroots++;
snap->sibling = vm->snapshots.first_root;
vm->snapshots.first_root = snap;
}
other = virDomainSnapshotFindByName(&vm->snapshots,
snap->def->parent);
snap->parent = other;
other->nchildren++;
snap->sibling = other->first_child;
other->first_child = snap;
}
} else if (snap) {
virDomainSnapshotObjListRemove(&vm->snapshots, snap);
@ -11439,7 +11433,7 @@ qemuDomainSnapshotReparentChildren(void *payload,
VIR_FREE(snap->def->parent);
snap->parent = rep->parent;
if (rep->parent) {
if (rep->parent->def) {
snap->def->parent = strdup(rep->parent->def->name);
if (snap->def->parent == NULL) {
@ -11547,15 +11541,9 @@ static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
if (rep.err < 0)
goto endjob;
/* Can't modify siblings during ForEachChild, so do it now. */
if (snap->parent) {
snap->parent->nchildren += snap->nchildren;
rep.last->sibling = snap->parent->first_child;
snap->parent->first_child = snap->first_child;
} else {
vm->snapshots.nroots += snap->nchildren;
rep.last->sibling = vm->snapshots.first_root;
vm->snapshots.first_root = snap->first_child;
}
snap->parent->nchildren += snap->nchildren;
rep.last->sibling = snap->parent->first_child;
snap->parent->first_child = snap->first_child;
}
if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) {
@ -11563,7 +11551,7 @@ static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
snap->first_child = NULL;
ret = 0;
} else {
virDomainSnapshotDropParent(&vm->snapshots, snap);
virDomainSnapshotDropParent(snap);
ret = qemuDomainSnapshotDiscard(driver, vm, snap, true, metadata_only);
}