11:18 p.m.
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. Also, vshTreePrint
no longer has a generic callback struct; both clients now pass
something different according to their own needs.
* tools/virsh.c (cmdSnapshotList): Use previous patches.
(vshTreeArrayLookup): Rename...
(vshNodeListLookup): ...now that it only has one client.
(cmdNodeListDevices): Adjust caller.
---
v2: rename vshNodeListLookup
tools/virsh.c | 225 +++++++++++----------------------------------------------
1 file changed, 41 insertions(+), 184 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index ac748d9..4f841fd 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -13489,15 +13489,15 @@ vshTreePrint(vshControl *ctl, vshTreeLookup lookup, void
*opaque,
return ret;
}
-struct vshTreeArray {
+struct vshNodeList {
char **names;
char **parents;
};
static const char *
-vshTreeArrayLookup(int devid, bool parent, void *opaque)
+vshNodeListLookup(int devid, bool parent, void *opaque)
{
- struct vshTreeArray *arrays = opaque;
+ struct vshNodeList *arrays = opaque;
if (parent)
return arrays->parents[devid];
return arrays->names[devid];
@@ -13552,7 +13552,7 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd
ATTRIBUTE_UNUSED)
qsort(&devices[0], num_devices, sizeof(char*), namesorter);
if (tree) {
char **parents = vshMalloc(ctl, sizeof(char *) * num_devices);
- struct vshTreeArray arrays = { devices, parents };
+ struct vshNodeList arrays = { devices, parents };
for (i = 0; i < num_devices; i++) {
virNodeDevicePtr dev = virNodeDeviceLookupByName(ctl->conn, devices[i]);
@@ -13566,7 +13566,7 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd
ATTRIBUTE_UNUSED)
}
for (i = 0 ; i < num_devices ; i++) {
if (parents[i] == NULL &&
- vshTreePrint(ctl, vshTreeArrayLookup, &arrays, num_devices,
+ vshTreePrint(ctl, vshNodeListLookup, &arrays, num_devices,
i) < 0)
ret = false;
}
@@ -16992,7 +16992,7 @@ vshSnapSorter(const void *a, const void *b)
* list is limited to descendants of the given snapshot. If FLAGS is
* given, the list is filtered. If TREE is specified, then all but
* FROM or the roots will also have parent information. */
-static vshSnapshotListPtr ATTRIBUTE_UNUSED
+static vshSnapshotListPtr
vshSnapshotListCollect(vshControl *ctl, virDomainPtr dom,
virDomainSnapshotPtr from,
unsigned int flags, bool tree)
@@ -17221,6 +17221,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
*/
@@ -17251,11 +17260,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;
@@ -17271,8 +17276,7 @@ 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;
@@ -17297,7 +17301,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",
@@ -17324,50 +17328,21 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
flags |= VIR_DOMAIN_SNAPSHOT_LIST_METADATA;
}
- if (from) {
- descendants = vshCommandOptBool(cmd, "descendants");
- if (descendants || tree) {
- 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 (vshCommandOptBool(cmd, "descendants")) {
+ if (!from) {
+ vshError(ctl, "%s",
+ _("--descendants requires either --from or --current"));
+ goto cleanup;
}
+ flags |= VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
}
- 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"));
@@ -17378,142 +17353,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)
@@ -17523,12 +17391,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)
@@ -17547,31 +17412,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