Operating on a list of snapshot objects looks so much simpler.
In particular, since the helper function already trimmed out
irrelevant entries, we no longer have quite so many special cases
on finding the first snapshot to operate on.
* tools/virsh.c (cmdSnapshotList): Use previous patches.
---
tools/virsh.c | 207 +++++++++------------------------------------------------
1 file changed, 31 insertions(+), 176 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 62546b2..dc098f5 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -16621,7 +16621,7 @@ vshSnapSorter(const void *a, const void *b)
* parents array of SNAPLIST will also be set in a manner usable by
* tree listings (note that parents may also be set when getting
* descendants using older servers). */
-static vshSnapshotListPtr ATTRIBUTE_UNUSED
+static vshSnapshotListPtr
vshSnapshotListCollect(vshControl *ctl, virDomainPtr dom,
virDomainSnapshotPtr from,
unsigned int flags, bool tree)
@@ -16840,6 +16840,15 @@ cleanup:
return ret;
}
+static const char *
+vshSnapshotListLookup(int id, bool parent, void *opaque)
+{
+ vshSnapshotListPtr snaplist = opaque;
+ if (parent)
+ return snaplist->snaps[id].parent;
+ return virDomainSnapshotGetName(snaplist->snaps[id].snap);
+}
+
/*
* "snapshot-list" command
*/
@@ -16870,11 +16879,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
virDomainPtr dom = NULL;
bool ret = false;
unsigned int flags = 0;
- int parent_filter = 0; /* -1 for roots filtering, 0 for no parent
- information needed, 1 for parent column */
- char **names = NULL;
- char **parents = NULL;
- int count = 0;
+ bool show_parent = false;
int i;
xmlDocPtr xml = NULL;
xmlXPathContextPtr ctxt = NULL;
@@ -16890,8 +16895,8 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
bool leaves = vshCommandOptBool(cmd, "leaves");
const char *from = NULL;
virDomainSnapshotPtr start = NULL;
- int start_index = -1;
bool descendants = false;
+ vshSnapshotListPtr snaplist = NULL;
if (!vshConnectionUsability(ctl, ctl->conn))
goto cleanup;
@@ -16916,7 +16921,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
_("--parent and --tree are mutually exclusive"));
goto cleanup;
}
- parent_filter = 1;
+ show_parent = true;
} else if (vshCommandOptBool(cmd, "roots")) {
if (tree) {
vshError(ctl, "%s",
@@ -16945,48 +16950,16 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (from) {
descendants = vshCommandOptBool(cmd, "descendants");
- if (descendants || tree) {
+ if (descendants)
flags |= VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
- }
- count = ctl->useSnapshotOld ? -1 :
- virDomainSnapshotNumChildren(start, flags);
- if (count < 0) {
- if (ctl->useSnapshotOld ||
- last_error->code == VIR_ERR_NO_SUPPORT) {
- /* We can emulate --from. */
- /* XXX can we also emulate --leaves? */
- virFreeError(last_error);
- last_error = NULL;
- ctl->useSnapshotOld = true;
- flags &= ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
- count = virDomainSnapshotNum(dom, flags);
- }
- } else if (tree) {
- count++;
- }
- } else {
- count = virDomainSnapshotNum(dom, flags);
-
- /* Fall back to simulation if --roots was unsupported. */
- /* XXX can we also emulate --leaves? */
- if (count < 0 && last_error->code == VIR_ERR_INVALID_ARG &&
- (flags & VIR_DOMAIN_SNAPSHOT_LIST_ROOTS)) {
- virFreeError(last_error);
- last_error = NULL;
- parent_filter = -1;
- flags &= ~VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
- count = virDomainSnapshotNum(dom, flags);
- }
}
- if (count < 0) {
- if (!last_error)
- vshError(ctl, _("missing support"));
+ if ((snaplist = vshSnapshotListCollect(ctl, dom, start, flags,
+ tree)) == NULL)
goto cleanup;
- }
if (!tree) {
- if (parent_filter > 0)
+ if (show_parent)
vshPrintExtra(ctl, " %-20s %-25s %-15s %s",
_("Name"), _("Creation Time"), _("State"),
_("Parent"));
@@ -16997,142 +16970,35 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
"------------------------------------------------------------\n");
}
- if (!count) {
+ if (!snaplist->nsnaps) {
ret = true;
goto cleanup;
}
- if (VIR_ALLOC_N(names, count) < 0)
- goto cleanup;
-
- if (from && !ctl->useSnapshotOld) {
- /* When mixing --from and --tree, we want to start the tree at the
- * given snapshot. Without --tree, only list the children. */
- if (tree) {
- if (count)
- count = virDomainSnapshotListChildrenNames(start, names + 1,
- count - 1, flags);
- if (count >= 0) {
- count++;
- names[0] = vshStrdup(ctl, from);
- }
- } else {
- count = virDomainSnapshotListChildrenNames(start, names,
- count, flags);
- }
- } else {
- count = virDomainSnapshotListNames(dom, names, count, flags);
- }
- if (count < 0)
- goto cleanup;
-
- qsort(&names[0], count, sizeof(char*), namesorter);
-
- if (tree || (from && ctl->useSnapshotOld)) {
- parents = vshCalloc(ctl, sizeof(char *), count);
- for (i = (from && !ctl->useSnapshotOld); i < count; i++) {
- if (from && ctl->useSnapshotOld && STREQ(names[i], from)) {
- start_index = i;
- continue;
- }
-
- /* free up memory from previous iterations of the loop */
- if (snapshot)
- virDomainSnapshotFree(snapshot);
- snapshot = virDomainSnapshotLookupByName(dom, names[i], 0);
- if (!snapshot ||
- vshGetSnapshotParent(ctl, snapshot, &parents[i]) < 0) {
- while (--i >= 0)
- VIR_FREE(parents[i]);
- VIR_FREE(parents);
- goto cleanup;
- }
- }
- }
if (tree) {
- struct vshTreeArray arrays = { names, parents };
-
- for (i = 0 ; i < count ; i++) {
- if ((from && ctl->useSnapshotOld) ? STREQ(names[i], from) :
- !parents[i] &&
- vshTreePrint(ctl, vshTreeArrayLookup, &arrays, count, i) < 0)
+ for (i = 0 ; i < snaplist->nsnaps ; i++) {
+ if (!snaplist->snaps[i].parent &&
+ vshTreePrint(ctl, vshSnapshotListLookup, snaplist,
+ snaplist->nsnaps, i) < 0)
goto cleanup;
}
-
ret = true;
goto cleanup;
}
- if (ctl->useSnapshotOld && descendants) {
- bool changed = false;
-
- /* Make multiple passes over the list - first pass NULLs out
- * all roots except start, remaining passes NULL out any entry
- * whose parent is not still in list. Also, we NULL out
- * parent when name is known to be in list. Sorry, this is
- * O(n^3) - hope your hierarchy isn't huge. */
- if (start_index < 0) {
- vshError(ctl, _("snapshot %s disappeared from list"), from);
- goto cleanup;
- }
- for (i = 0; i < count; i++) {
- if (i == start_index)
- continue;
- if (!parents[i]) {
- VIR_FREE(names[i]);
- } else if (STREQ(parents[i], from)) {
- VIR_FREE(parents[i]);
- changed = true;
- }
- }
- if (!changed) {
- ret = true;
- goto cleanup;
- }
- while (changed) {
- changed = false;
- for (i = 0; i < count; i++) {
- bool found = false;
- int j;
-
- if (!names[i] || !parents[i])
- continue;
- for (j = 0; j < count; j++) {
- if (!names[j] || i == j)
- continue;
- if (STREQ(parents[i], names[j])) {
- found = true;
- if (!parents[j])
- VIR_FREE(parents[i]);
- break;
- }
- }
- if (!found) {
- changed = true;
- VIR_FREE(names[i]);
- }
- }
- }
- VIR_FREE(names[start_index]);
- }
-
- for (i = 0; i < count; i++) {
- if (from && ctl->useSnapshotOld &&
- (descendants ? !names[i] : STRNEQ_NULLABLE(parents[i], from)))
- continue;
+ for (i = 0; i < snaplist->nsnaps; i++) {
+ const char *name;
/* free up memory from previous iterations of the loop */
VIR_FREE(parent);
VIR_FREE(state);
- if (snapshot)
- virDomainSnapshotFree(snapshot);
xmlXPathFreeContext(ctxt);
xmlFreeDoc(xml);
VIR_FREE(doc);
- snapshot = virDomainSnapshotLookupByName(dom, names[i], 0);
- if (snapshot == NULL)
- continue;
+ snapshot = snaplist->snaps[i].snap;
+ name = virDomainSnapshotGetName(snapshot);
+ assert(name);
doc = virDomainSnapshotGetXMLDesc(snapshot, 0);
if (!doc)
@@ -17142,12 +17008,9 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (!xml)
continue;
- if (parent_filter) {
+ if (show_parent)
parent = virXPathString("string(/domainsnapshot/parent/name)",
ctxt);
- if (!parent && parent_filter < 0)
- continue;
- }
state = virXPathString("string(/domainsnapshot/state)", ctxt);
if (state == NULL)
@@ -17166,31 +17029,23 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (parent)
vshPrint(ctl, " %-20s %-25s %-15s %s\n",
- names[i], timestr, state, parent);
+ name, timestr, state, parent);
else
- vshPrint(ctl, " %-20s %-25s %s\n", names[i], timestr, state);
+ vshPrint(ctl, " %-20s %-25s %s\n", name, timestr, state);
}
ret = true;
cleanup:
/* this frees up memory from the last iteration of the loop */
+ vshSnapshotListFree(snaplist);
VIR_FREE(parent);
VIR_FREE(state);
- if (snapshot)
- virDomainSnapshotFree(snapshot);
if (start)
virDomainSnapshotFree(start);
xmlXPathFreeContext(ctxt);
xmlFreeDoc(xml);
VIR_FREE(doc);
- for (i = 0; i < count; i++) {
- VIR_FREE(names[i]);
- if (parents)
- VIR_FREE(parents[i]);
- }
- VIR_FREE(names);
- VIR_FREE(parents);
if (dom)
virDomainFree(dom);
--
1.7.10.2
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list