On Sat, Sep 24, 2011 at 06:30:05PM -0600, Eric Blake wrote:
Reuse the tree listing of nodedev-list, coupled with the new helper function to efficiently grab snapshot parent names, to produce tree output for a snapshot hierarchy. For example:
$ virsh snapshot-list dom --tree root1 | +- sibling1 +- sibling2 | | | +- grandchild | +- sibling3
root2 | +- child
* tools/virsh.c (cmdSnapshotList): Add --tree. * tools/virsh.pod (snapshot-list): Document it. --- tools/virsh.c | 83 +++++++++++++++++++++++++++++++++++++++++++++--------- tools/virsh.pod | 14 +++++---- 2 files changed, 77 insertions(+), 20 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 035b209..7e7b65d 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -12987,7 +12987,7 @@ vshGetSnapshotParent(vshControl *ctl, virDomainSnapshotPtr snapshot) parent = virDomainSnapshotGetParent(snapshot, 0); if (parent) { /* API works, and virDomainSnapshotGetName will succeed */ - parent_name = vshStrdup(ctl, virDomainSnapshotGetName(snapshot)); + parent_name = vshStrdup(ctl, virDomainSnapshotGetName(parent)); goto cleanup; } if (last_error->code == VIR_ERR_NO_DOMAIN_SNAPSHOT) {
That looks like a bug fix squashed in here instead of previous patch, what am I missing ?
@@ -13032,6 +13032,7 @@ static const vshCmdOptDef opts_snapshot_list[] = { {"roots", VSH_OT_BOOL, 0, N_("list only snapshots without parents")}, {"metadata", VSH_OT_BOOL, 0, N_("list only snapshots that have metadata that would prevent undefine")}, + {"tree", VSH_OT_BOOL, 0, N_("list snapshots in a tree")}, {NULL, 0, 0, NULL} };
@@ -13057,6 +13058,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) time_t creation_time_t; char timestr[100]; struct tm time_info; + bool tree = vshCommandOptBool(cmd, "tree");
if (vshCommandOptBool(cmd, "parent")) { if (vshCommandOptBool(cmd, "roots")) { @@ -13064,8 +13066,18 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) _("--parent and --roots are mutually exlusive")); return false; } + if (tree) { + vshError(ctl, "%s", + _("--parent and --tree are mutually exlusive")); + return false; + } parent_filter = 1; } else if (vshCommandOptBool(cmd, "roots")) { + if (tree) { + vshError(ctl, "%s", + _("--roots and --tree are mutually exlusive")); + return false; + } flags |= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS; }
@@ -13095,23 +13107,66 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) if (numsnaps < 0) goto cleanup;
- if (parent_filter > 0) - vshPrintExtra(ctl, " %-20s %-25s %-15s %s", - _("Name"), _("Creation Time"), _("State"), _("Parent")); - else - vshPrintExtra(ctl, " %-20s %-25s %s", - _("Name"), _("Creation Time"), _("State")); - vshPrintExtra(ctl, "\n\ + if (!tree) { + if (parent_filter > 0) + vshPrintExtra(ctl, " %-20s %-25s %-15s %s", + _("Name"), _("Creation Time"), _("State"), + _("Parent")); + else + vshPrintExtra(ctl, " %-20s %-25s %s", + _("Name"), _("Creation Time"), _("State")); + vshPrintExtra(ctl, "\n\ ------------------------------------------------------------\n"); + }
- if (numsnaps) { - if (VIR_ALLOC_N(names, numsnaps) < 0) - goto cleanup; + if (!numsnaps) { + ret = true; + goto cleanup; + }
- actual = virDomainSnapshotListNames(dom, names, numsnaps, flags); - if (actual < 0) - goto cleanup; + if (VIR_ALLOC_N(names, numsnaps) < 0) + goto cleanup; + + actual = virDomainSnapshotListNames(dom, names, numsnaps, flags); + if (actual < 0) + goto cleanup;
+ if (tree) { + char indentBuf[INDENT_BUFLEN]; + char **parents = vshMalloc(ctl, sizeof(char *) * actual); + for (i = 0; i < actual; i++) { + /* free up memory from previous iterations of the loop */ + if (snapshot) + virDomainSnapshotFree(snapshot); + snapshot = virDomainSnapshotLookupByName(dom, names[i], 0); + if (!snapshot) { + while (--i >= 0) + VIR_FREE(parents[i]); + VIR_FREE(parents); + goto cleanup; + } + parents[i] = vshGetSnapshotParent(ctl, snapshot); + } + for (i = 0 ; i < actual ; i++) { + memset(indentBuf, '\0', sizeof indentBuf); + if (parents[i] == NULL) + cmdNodeListDevicesPrint(ctl, + names, + parents, + actual, + i, + i, + 0, + 0, + indentBuf); + } + for (i = 0 ; i < actual ; i++) + VIR_FREE(parents[i]); + VIR_FREE(parents); + + ret = true; + goto cleanup; + } else { qsort(&names[0], actual, sizeof(char*), namesorter);
for (i = 0; i < actual; i++) { diff --git a/tools/virsh.pod b/tools/virsh.pod index ddf7578..49aa42a 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1934,15 +1934,17 @@ except that it does some error checking. The editor used can be supplied by the C<$VISUAL> or C<$EDITOR> environment variables, and defaults to C<vi>.
-=item B<snapshot-list> I<domain> [{I<--parent> | I<--roots>}] [I<--metadata>] +=item B<snapshot-list> I<domain> [{I<--parent> | I<--roots> | I<--tree>}] +[I<--metadata>]
-List all of the available snapshots for the given domain. +List all of the available snapshots for the given domain, defaulting +to show columns for the snapshot name, creation time, and domain state.
If I<--parent> is specified, add a column to the output table giving -the name of the parent of each snapshot. - -If I<--roots> is specified, the list will be filtered to just snapshots -that have no parents; this option is not compatible with I<--parent>. +the name of the parent of each snapshot. If I<--roots> is specified, +the list will be filtered to just snapshots that have no parents. +If I<--tree> is specified, the output will be in a tree format, listing +just snapshot names. These three options are mutually exclusive.
If I<--metadata> is specified, the list will be filtered to just snapshots that involve libvirt metadata, and thus would prevent
ACK, great improvement, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/