On 09/18/2018 04:21 PM, Simon Kobyda wrote:
Signed-off-by: Simon Kobyda <skobyda@redhat.com> --- tools/virsh-snapshot.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index a4ea959230..5a02d2c786 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -41,6 +41,7 @@ #include "virstring.h" #include "virxml.h" #include "conf/snapshot_conf.h" +#include "vsh-table.h"
/* Helper for snapshot-create and snapshot-create-as */ static bool @@ -1487,6 +1488,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) char *parent_snap = NULL; virDomainSnapshotPtr start = NULL; virshSnapshotListPtr snaplist = NULL; + vshTablePtr table = NULL;
VSH_EXCLUSIVE_OPTIONS_VAR(tree, name); VSH_EXCLUSIVE_OPTIONS_VAR(parent, roots); @@ -1547,15 +1549,12 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (!tree && !name) { if (parent) - vshPrintExtra(ctl, " %-20s %-25s %-15s %s", - _("Name"), _("Creation Time"), _("State"), - _("Parent")); + table = vshTableNew("Name", "Creation Time", "State", "Parent", NULL); else - vshPrintExtra(ctl, " %-20s %-25s %s", - _("Name"), _("Creation Time"), _("State")); - vshPrintExtra(ctl, "\n" - "------------------------------" - "------------------------------\n"); + table = vshTableNew("Name", "Creation Time", "State", NULL); + + if (!table) + goto cleanup; }
if (tree) { @@ -1614,13 +1613,20 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) strftime(timestr, sizeof(timestr), "%Y-%m-%d %H:%M:%S %z", &time_info);
- if (parent) - vshPrint(ctl, " %-20s %-25s %-15s %s\n", - snap_name, timestr, state, parent_snap); - else - vshPrint(ctl, " %-20s %-25s %s\n", snap_name, timestr, state); + if (parent) { + if (vshTableRowAppend(table, snap_name, timestr, state, parent_snap, + NULL) < 0) + continue; + } else { + if (vshTableRowAppend(table, snap_name, timestr, state, + NULL) < 0) + continue;
What is the point of these 'continue'? Shouldn't we jump to cleanup instead?
+ } }
+ if (!tree && !name)
Or simply: if (table)
+ vshTablePrintToStdout(table, ctl); + ret = true;
cleanup: @@ -1633,6 +1639,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) xmlFreeDoc(xml); VIR_FREE(doc); virshDomainFree(dom); + vshTableFree(table);
return ret; }
This is where I'm going to stop my review. You get the idea what changes you need to do for v2. Michal