Re: [libvirt] [PATCHv2 03/13] 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. --- v2: fix bugs in virDomainSnapshotSetRelations, rebase earlier in series src/conf/domain_conf.c | 116 ++++++++++++------------------------------------ src/conf/domain_conf.h | 12 ++--- src/qemu/qemu_driver.c | 36 +++++---------- 3 files changed, 46 insertions(+), 118 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8d80f3b..c7437af 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14289,32 +14289,9 @@ 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; + flags ^= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;

+ return virDomainSnapshotObjListGetNamesFrom(&snapshots->metaroot, names, + maxnames, flags);

} int virDomainSnapshotObjListGetNamesFrom(virDomainSnapshotObjPtr snapshot, @@ -14369,23 +14346,8 @@ 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; + flags ^= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS; + return virDomainSnapshotObjListNumFrom(&snapshots->metaroot, flags);

} int