From: Eric Blake Date: Fri, 22 Mar 2019 05:46:57 +0000 (-0500) Subject: snapshot: Make virDomainSnapshotObjList use MomentObjList X-Git-Tag: v5.2.0-rc1~66 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ac0379043ac3a96856b4a1c83939adf12a85ee10;p=thirdparty%2Flibvirt.git snapshot: Make virDomainSnapshotObjList use MomentObjList Now that the generic moment code does pretty much everything that both snapshots and checkpoints will need, it's time to replace the now-duplicate code in virdomainsnapshotobjlist.c with simpler calls into the generic code. I considered using sub-classing (a 'virDomainMomentObjList parent;' member, but that requires making the opaque type visible in headers; so for now, I stuck with a container instead (a 'virDomainMomentObjListPtr base;' member). Signed-off-by: Eric Blake Reviewed-by: John Ferlan --- diff --git a/src/conf/virdomainsnapshotobjlist.c b/src/conf/virdomainsnapshotobjlist.c index d6cb2595bf..b1b218a745 100644 --- a/src/conf/virdomainsnapshotobjlist.c +++ b/src/conf/virdomainsnapshotobjlist.c @@ -35,12 +35,7 @@ VIR_LOG_INIT("conf.virdomainsnapshotobjlist"); struct _virDomainSnapshotObjList { - /* name string -> virDomainMomentObj mapping - * for O(1), lockless lookup-by-name */ - virHashTable *objs; - - virDomainMomentObj metaroot; /* Special parent of all root snapshots */ - virDomainMomentObjPtr current; /* The current snapshot, if any */ + virDomainMomentObjListPtr base; }; @@ -74,7 +69,7 @@ virDomainSnapshotObjListParse(const char *xmlStr, _("incorrect flags for bulk parse")); return -1; } - if (virDomainSnapshotObjListSize(snapshots) != 0) { + if (virDomainMomentObjListSize(snapshots->base) != 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("bulk define of snapshots only possible with " "no existing snapshot")); @@ -140,7 +135,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. */ - virDomainSnapshotObjListRemoveAll(snapshots); + virDomainMomentObjListRemoveAll(snapshots->base); } xmlXPathFreeContext(ctxt); xmlFreeDoc(xml); @@ -210,55 +205,14 @@ virDomainSnapshotObjListFormat(virBufferPtr buf, } -/* Snapshot Obj functions */ -static virDomainMomentObjPtr virDomainMomentObjNew(void) +virDomainMomentObjPtr +virDomainSnapshotAssignDef(virDomainSnapshotObjListPtr snapshots, + virDomainSnapshotDefPtr def) { - virDomainMomentObjPtr snapshot; - - if (VIR_ALLOC(snapshot) < 0) - return NULL; - - VIR_DEBUG("obj=%p", snapshot); - - return snapshot; + return virDomainMomentAssignDef(snapshots->base, &def->common); } -static void virDomainMomentObjFree(virDomainMomentObjPtr snapshot) -{ - if (!snapshot) - return; - - VIR_DEBUG("obj=%p", snapshot); - - virDomainSnapshotDefFree(virDomainSnapshotObjGetDef(snapshot)); - VIR_FREE(snapshot); -} - -virDomainMomentObjPtr virDomainSnapshotAssignDef(virDomainSnapshotObjListPtr snapshots, - virDomainSnapshotDefPtr def) -{ - virDomainMomentObjPtr snap; - - if (virHashLookup(snapshots->objs, def->common.name) != NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected domain snapshot %s already exists"), - def->common.name); - return NULL; - } - - if (!(snap = virDomainMomentObjNew())) - return NULL; - - if (virHashAddEntry(snapshots->objs, snap->def->name, snap) < 0) { - VIR_FREE(snap); - return NULL; - } - snap->def = &def->common; - return snap; -} - -/* Snapshot Obj List functions */ static bool virDomainSnapshotFilter(virDomainMomentObjPtr obj, unsigned int flags) @@ -291,76 +245,32 @@ virDomainSnapshotFilter(virDomainMomentObjPtr obj, } -static void -virDomainSnapshotObjListDataFree(void *payload, - const void *name ATTRIBUTE_UNUSED) -{ - virDomainMomentObjPtr obj = payload; - - virDomainMomentObjFree(obj); -} - virDomainSnapshotObjListPtr virDomainSnapshotObjListNew(void) { virDomainSnapshotObjListPtr snapshots; + if (VIR_ALLOC(snapshots) < 0) return NULL; - snapshots->objs = virHashCreate(50, virDomainSnapshotObjListDataFree); - if (!snapshots->objs) { + snapshots->base = virDomainMomentObjListNew(); + if (!snapshots->base) { VIR_FREE(snapshots); return NULL; } return snapshots; } + void virDomainSnapshotObjListFree(virDomainSnapshotObjListPtr snapshots) { if (!snapshots) return; - virHashFree(snapshots->objs); + virDomainMomentObjListFree(snapshots->base); VIR_FREE(snapshots); } -struct virDomainMomentNameData { - char **const names; - int maxnames; - unsigned int flags; - int count; - bool error; - virDomainMomentObjListFilter filter; -}; - -static int virDomainMomentObjListCopyNames(void *payload, - const void *name ATTRIBUTE_UNUSED, - void *opaque) -{ - virDomainMomentObjPtr obj = payload; - struct virDomainMomentNameData *data = opaque; - - if (data->error) - return 0; - /* Caller already sanitized flags. Filtering on DESCENDANTS was - * done by choice of iteration in the caller. */ - if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_LEAVES) && obj->nchildren) - return 0; - if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES) && !obj->nchildren) - return 0; - - if (data->filter(obj, data->flags)) - return 0; - - if (data->names && data->count < data->maxnames && - VIR_STRDUP(data->names[data->count], obj->def->name) < 0) { - data->error = true; - return 0; - } - data->count++; - return 0; -} - int virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots, virDomainMomentObjPtr from, @@ -368,75 +278,23 @@ virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots, int maxnames, unsigned int flags) { - struct virDomainMomentNameData data = { names, maxnames, flags, 0, - false, virDomainSnapshotFilter }; - size_t i; - - if (!from) { - /* 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; - from = &snapshots->metaroot; - } - - /* We handle LIST_ROOT/LIST_DESCENDANTS and LIST_TOPOLOGICAL directly, - * mask those bits out to determine when we must use the filter callback. */ - data.flags &= ~(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS | - VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL); - - /* If this common code is being used, we assume that all snapshots - * have metadata, and thus can handle METADATA up front as an - * all-or-none filter. XXX This might not always be true, if we - * add the ability to track qcow2 internal snapshots without the - * use of metadata. */ - if ((data.flags & VIR_DOMAIN_SNAPSHOT_FILTERS_METADATA) == - VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA) - return 0; - data.flags &= ~VIR_DOMAIN_SNAPSHOT_FILTERS_METADATA; - /* For ease of coding the visitor, it is easier to zero each group * where all of the bits are set. */ - if ((data.flags & VIR_DOMAIN_SNAPSHOT_FILTERS_LEAVES) == + if ((flags & VIR_DOMAIN_SNAPSHOT_FILTERS_LEAVES) == VIR_DOMAIN_SNAPSHOT_FILTERS_LEAVES) - data.flags &= ~VIR_DOMAIN_SNAPSHOT_FILTERS_LEAVES; - if ((data.flags & VIR_DOMAIN_SNAPSHOT_FILTERS_STATUS) == + flags &= ~VIR_DOMAIN_SNAPSHOT_FILTERS_LEAVES; + if ((flags & VIR_DOMAIN_SNAPSHOT_FILTERS_STATUS) == VIR_DOMAIN_SNAPSHOT_FILTERS_STATUS) - data.flags &= ~VIR_DOMAIN_SNAPSHOT_FILTERS_STATUS; - if ((data.flags & VIR_DOMAIN_SNAPSHOT_FILTERS_LOCATION) == + flags &= ~VIR_DOMAIN_SNAPSHOT_FILTERS_STATUS; + if ((flags & VIR_DOMAIN_SNAPSHOT_FILTERS_LOCATION) == VIR_DOMAIN_SNAPSHOT_FILTERS_LOCATION) - data.flags &= ~VIR_DOMAIN_SNAPSHOT_FILTERS_LOCATION; - - if (flags & VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS) { - /* We could just always do a topological visit; but it is - * possible to optimize for less stack usage and time when a - * simpler full hashtable visit or counter will do. */ - if (from->def || (names && - (flags & VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL))) - virDomainMomentForEachDescendant(from, - virDomainMomentObjListCopyNames, - &data); - else if (names || data.flags) - virHashForEach(snapshots->objs, virDomainMomentObjListCopyNames, - &data); - else - data.count = virHashSize(snapshots->objs); - } else if (names || data.flags) { - virDomainMomentForEachChild(from, - virDomainMomentObjListCopyNames, &data); - } else { - data.count = from->nchildren; - } - - if (data.error) { - for (i = 0; i < data.count; i++) - VIR_FREE(names[i]); - return -1; - } - - return data.count; + flags &= ~VIR_DOMAIN_SNAPSHOT_FILTERS_LOCATION; + return virDomainMomentObjListGetNames(snapshots->base, from, names, + maxnames, flags, + virDomainSnapshotFilter); } + int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots, virDomainMomentObjPtr from, @@ -445,19 +303,12 @@ virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots, return virDomainSnapshotObjListGetNames(snapshots, from, NULL, 0, flags); } + virDomainMomentObjPtr virDomainSnapshotFindByName(virDomainSnapshotObjListPtr snapshots, const char *name) { - return name ? virHashLookup(snapshots->objs, name) : &snapshots->metaroot; -} - - -/* Return the number of objects currently in the list */ -int -virDomainSnapshotObjListSize(virDomainSnapshotObjListPtr snapshots) -{ - return virHashSize(snapshots->objs); + return virDomainMomentFindByName(snapshots->base, name); } @@ -465,7 +316,7 @@ virDomainSnapshotObjListSize(virDomainSnapshotObjListPtr snapshots) virDomainMomentObjPtr virDomainSnapshotGetCurrent(virDomainSnapshotObjListPtr snapshots) { - return snapshots->current; + return virDomainMomentGetCurrent(snapshots->base); } @@ -473,9 +324,7 @@ virDomainSnapshotGetCurrent(virDomainSnapshotObjListPtr snapshots) const char * virDomainSnapshotGetCurrentName(virDomainSnapshotObjListPtr snapshots) { - if (snapshots->current) - return snapshots->current->def->name; - return NULL; + return virDomainMomentGetCurrentName(snapshots->base); } @@ -484,7 +333,7 @@ bool virDomainSnapshotIsCurrentName(virDomainSnapshotObjListPtr snapshots, const char *name) { - return snapshots->current && STREQ(snapshots->current->def->name, name); + return virDomainMomentIsCurrentName(snapshots->base, name); } @@ -493,7 +342,7 @@ void virDomainSnapshotSetCurrent(virDomainSnapshotObjListPtr snapshots, virDomainMomentObjPtr snapshot) { - snapshots->current = snapshot; + virDomainMomentSetCurrent(snapshots->base, snapshot); } @@ -502,19 +351,15 @@ bool virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots, virDomainMomentObjPtr snapshot) { - bool ret = snapshots->current == snapshot; - virHashRemoveEntry(snapshots->objs, snapshot->def->name); - if (ret) - snapshots->current = NULL; - return ret; + return virDomainMomentObjListRemove(snapshots->base, snapshot); } + /* Remove all snapshots tracked in the list */ void virDomainSnapshotObjListRemoveAll(virDomainSnapshotObjListPtr snapshots) { - virHashRemoveAll(snapshots->objs); - virDomainMomentDropChildren(&snapshots->metaroot); + return virDomainMomentObjListRemoveAll(snapshots->base); } @@ -523,51 +368,10 @@ virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots, virHashIterator iter, void *data) { - return virHashForEach(snapshots->objs, iter, data); + return virDomainMomentForEach(snapshots->base, iter, data); } -/* Struct and callback function used as a hash table callback; each call - * inspects the pre-existing snapshot->def->parent field, and adjusts - * the snapshot->parent field as well as the parent's child fields to - * wire up the hierarchical relations for the given snapshot. The error - * indicator gets set if a parent is missing or a requested parent would - * cause a circular parent chain. */ -struct moment_set_relation { - virDomainSnapshotObjListPtr snapshots; - int err; -}; -static int -virDomainMomentSetRelations(void *payload, - const void *name ATTRIBUTE_UNUSED, - void *data) -{ - virDomainMomentObjPtr obj = payload; - struct moment_set_relation *curr = data; - virDomainMomentObjPtr tmp; - virDomainMomentObjPtr parent; - - parent = virDomainSnapshotFindByName(curr->snapshots, obj->def->parent); - if (!parent) { - curr->err = -1; - parent = &curr->snapshots->metaroot; - VIR_WARN("snapshot %s lacks parent", obj->def->name); - } else { - tmp = parent; - while (tmp && tmp->def) { - if (tmp == obj) { - curr->err = -1; - parent = &curr->snapshots->metaroot; - VIR_WARN("snapshot %s in circular chain", obj->def->name); - break; - } - tmp = tmp->parent; - } - } - virDomainMomentSetParent(obj, parent); - return 0; -} - /* Populate parent link and child count of all snapshots, with all * assigned defs having relations starting as 0/NULL. Return 0 on * success, -1 if a parent is missing or if a circular relationship @@ -575,13 +379,7 @@ virDomainMomentSetRelations(void *payload, int virDomainSnapshotUpdateRelations(virDomainSnapshotObjListPtr snapshots) { - struct moment_set_relation act = { snapshots, 0 }; - - virDomainMomentDropChildren(&snapshots->metaroot); - virHashForEach(snapshots->objs, virDomainMomentSetRelations, &act); - if (act.err) - snapshots->current = NULL; - return act.err; + return virDomainMomentUpdateRelations(snapshots->base); } diff --git a/src/conf/virdomainsnapshotobjlist.h b/src/conf/virdomainsnapshotobjlist.h index 7398a5c331..3691662b4c 100644 --- a/src/conf/virdomainsnapshotobjlist.h +++ b/src/conf/virdomainsnapshotobjlist.h @@ -27,10 +27,6 @@ # include "virdomainmomentobjlist.h" # include "virbuffer.h" -/* Filter that returns true if a given moment matches the filter flags */ -typedef bool (*virDomainMomentObjListFilter)(virDomainMomentObjPtr obj, - unsigned int flags); - virDomainSnapshotObjListPtr virDomainSnapshotObjListNew(void); void virDomainSnapshotObjListFree(virDomainSnapshotObjListPtr snapshots); @@ -59,7 +55,6 @@ int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots, unsigned int flags); virDomainMomentObjPtr virDomainSnapshotFindByName(virDomainSnapshotObjListPtr snapshots, const char *name); -int virDomainSnapshotObjListSize(virDomainSnapshotObjListPtr snapshots); virDomainMomentObjPtr virDomainSnapshotGetCurrent(virDomainSnapshotObjListPtr snapshots); const char *virDomainSnapshotGetCurrentName(virDomainSnapshotObjListPtr snapshots); bool virDomainSnapshotIsCurrentName(virDomainSnapshotObjListPtr snapshots,