diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8d80f3b017..43872d1c73 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -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; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 88674eb830..e3a3679268 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -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; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3fd9803812..1ff2af4887 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -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); }