[libvirt] [PATCH v2 00/13] Implement vsh-table API to virsh and virt-admin
API is implemented in virsh and virt-admin. It fixes problems with table-alignment, makes tables more readable and deals with unicode. Table however could not be implemented in places, which do use simple lists, or table implementation would change content of output, which users might rely on. If you know about other place where it could be implemented, let me know. https://bugzilla.redhat.com/show_bug.cgi?id=1574624 https://bugzilla.redhat.com/show_bug.cgi?id=1584630 Changes in v2: - Implemented gettext function - Fixed few possible leaks caused by inserting "goto cleanup" statements - Implemented VIR_AUTOFREE where it's appropriate Simon Kobyda (13): virsh: Implement vsh-table to iface-list virsh: Implement vshTable API to net-list and net-dhcp-leases virsh: Implement vshTable API to secret-list virsh: Implement vshTable API to nwfilter-list and nwfilterbinding-list virsh: Implement vshTable API to snapshot-list. virsh: Set up cmdDomblkinfo() and cmdDomblkinfoPrint() for vshTable API implementation virsh: Implement vshTable API to domblkinfo virsh: Implement vshTable API to domblklist virsh: Implement vshTable API to domiflist virsh: Implement vshTable API to vcpupin, iothreadinfo, domfsinfo virsh: Implement vshTable API to pool-list virsh: Implement vshTable API to vol-list virt-admin: Implement vshTable API to server-list and client-list tools/virsh-domain-monitor.c | 147 ++++++++++++++++--------------- tools/virsh-domain.c | 98 +++++++++++++++------ tools/virsh-interface.c | 27 ++++-- tools/virsh-network.c | 59 ++++++++----- tools/virsh-nwfilter.c | 47 +++++++--- tools/virsh-pool.c | 162 +++++++---------------------------- tools/virsh-secret.c | 28 ++++-- tools/virsh-snapshot.c | 33 ++++--- tools/virsh-volume.c | 129 ++++++---------------------- tools/virt-admin.c | 47 +++++++--- 10 files changed, 371 insertions(+), 406 deletions(-) -- 2.17.1
Signed-off-by: Simon Kobyda <skobyda@redhat.com> --- tools/virsh-interface.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c index 50518c667b..1eb1a27ac7 100644 --- a/tools/virsh-interface.c +++ b/tools/virsh-interface.c @@ -48,6 +48,7 @@ #include "virutil.h" #include "virxml.h" #include "virstring.h" +#include "vsh-table.h" virInterfacePtr virshCommandOptInterfaceBy(vshControl *ctl, const vshCmd *cmd, @@ -356,6 +357,8 @@ cmdInterfaceList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) unsigned int flags = VIR_CONNECT_LIST_INTERFACES_ACTIVE; virshInterfaceListPtr list = NULL; size_t i; + bool ret = false; + vshTablePtr table = NULL; if (inactive) flags = VIR_CONNECT_LIST_INTERFACES_INACTIVE; @@ -366,21 +369,29 @@ cmdInterfaceList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) if (!(list = virshInterfaceListCollect(ctl, flags))) return false; - vshPrintExtra(ctl, " %-20s %-10s %s\n", _("Name"), _("State"), - _("MAC Address")); - vshPrintExtra(ctl, "---------------------------------------------------\n"); + table = vshTableNew(_("Name"), _("State"), _("MAC Address"), NULL); + if (!table) + goto cleanup; for (i = 0; i < list->nifaces; i++) { virInterfacePtr iface = list->ifaces[i]; - vshPrint(ctl, " %-20s %-10s %s\n", - virInterfaceGetName(iface), - virInterfaceIsActive(iface) ? _("active") : _("inactive"), - virInterfaceGetMACString(iface)); + if (vshTableRowAppend(table, + virInterfaceGetName(iface), + virInterfaceIsActive(iface) ? _("active") + : _("inactive"), + virInterfaceGetMACString(iface), + NULL) < 0) + goto cleanup; } + vshTablePrintToStdout(table, ctl); + + ret = true; + cleanup: + vshTableFree(table); virshInterfaceListFree(list); - return true; + return ret; } /* -- 2.17.1
Signed-off-by: Simon Kobyda <skobyda@redhat.com> --- tools/virsh-network.c | 59 ++++++++++++++++++++++++++++--------------- 1 file changed, 38 insertions(+), 21 deletions(-) diff --git a/tools/virsh-network.c b/tools/virsh-network.c index ca07fb568f..440b23d8a8 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -33,6 +33,7 @@ #include "virstring.h" #include "virtime.h" #include "conf/network_conf.h" +#include "vsh-table.h" #define VIRSH_COMMON_OPT_NETWORK(_helpstr, cflags) \ {.name = "network", \ @@ -677,6 +678,7 @@ cmdNetworkList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) bool optUUID = vshCommandOptBool(cmd, "uuid"); char uuid[VIR_UUID_STRING_BUFLEN]; unsigned int flags = VIR_CONNECT_LIST_NETWORKS_ACTIVE; + vshTablePtr table = NULL; if (vshCommandOptBool(cmd, "inactive")) flags = VIR_CONNECT_LIST_NETWORKS_INACTIVE; @@ -705,10 +707,10 @@ cmdNetworkList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) return false; if (optTable) { - vshPrintExtra(ctl, " %-20s %-10s %-13s %s\n", _("Name"), _("State"), - _("Autostart"), _("Persistent")); - vshPrintExtra(ctl, - "----------------------------------------------------------\n"); + table = vshTableNew(_("Name"), _("State"), _("Autostart"), + _("Persistent"), NULL); + if (!table) + goto cleanup; } for (i = 0; i < list->nnets; i++) { @@ -722,11 +724,15 @@ cmdNetworkList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) else autostartStr = is_autostart ? _("yes") : _("no"); - vshPrint(ctl, " %-20s %-10s %-13s %s\n", - virNetworkGetName(network), - virNetworkIsActive(network) ? _("active") : _("inactive"), - autostartStr, - virNetworkIsPersistent(network) ? _("yes") : _("no")); + if (vshTableRowAppend(table, + virNetworkGetName(network), + virNetworkIsActive(network) ? + _("active") : _("inactive"), + autostartStr, + virNetworkIsPersistent(network) ? + _("yes") : _("no"), + NULL) < 0) + goto cleanup; } else if (optUUID) { if (virNetworkGetUUIDString(network, uuid) < 0) { vshError(ctl, "%s", _("Failed to get network's UUID")); @@ -738,8 +744,12 @@ cmdNetworkList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) } } + if (optTable) + vshTablePrintToStdout(table, ctl); + ret = true; cleanup: + vshTableFree(table); virshNetworkListFree(list); return ret; } @@ -1351,6 +1361,7 @@ cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd) size_t i; unsigned int flags = 0; virNetworkPtr network = NULL; + vshTablePtr table = NULL; if (vshCommandOptStringReq(ctl, cmd, "mac", &mac) < 0) return false; @@ -1366,15 +1377,15 @@ cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd) /* Sort the list according to MAC Address/IAID */ qsort(leases, nleases, sizeof(*leases), virshNetworkDHCPLeaseSorter); - vshPrintExtra(ctl, " %-20s %-18s %-9s %-25s %-15s %s\n%s%s\n", - _("Expiry Time"), _("MAC address"), _("Protocol"), - _("IP address"), _("Hostname"), _("Client ID or DUID"), - "----------------------------------------------------------", - "---------------------------------------------------------"); + table = vshTableNew(_("Expiry Time"), _("MAC address"), _("Protocol"), + _("IP address"), _("Hostname"), _("Client ID or DUID"), + NULL); + if (!table) + goto cleanup; for (i = 0; i < nleases; i++) { const char *typestr = NULL; - char *cidr_format = NULL; + VIR_AUTOFREE(char *) cidr_format = NULL; virNetworkDHCPLeasePtr lease = leases[i]; time_t expirytime_tmp = lease->expirytime; struct tm ts; @@ -1390,17 +1401,23 @@ cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd) ignore_value(virAsprintf(&cidr_format, "%s/%d", lease->ipaddr, lease->prefix)); - vshPrint(ctl, " %-20s %-18s %-9s %-25s %-15s %s\n", - expirytime, EMPTYSTR(lease->mac), - EMPTYSTR(typestr), cidr_format, - EMPTYSTR(lease->hostname), EMPTYSTR(lease->clientid)); - - VIR_FREE(cidr_format); + if (vshTableRowAppend(table, + expirytime, + EMPTYSTR(lease->mac), + EMPTYSTR(typestr), + EMPTYSTR(cidr_format), + EMPTYSTR(lease->hostname), + EMPTYSTR(lease->clientid), + NULL) < 0) + goto cleanup; } + vshTablePrintToStdout(table, ctl); + ret = true; cleanup: + vshTableFree(table); if (leases) { for (i = 0; i < nleases; i++) virNetworkDHCPLeaseFree(leases[i]); -- 2.17.1
Signed-off-by: Simon Kobyda <skobyda@redhat.com> --- tools/virsh-secret.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c index 670beea706..87239ff60b 100644 --- a/tools/virsh-secret.c +++ b/tools/virsh-secret.c @@ -35,6 +35,7 @@ #include "virsecret.h" #include "virstring.h" #include "virtime.h" +#include "vsh-table.h" static virSecretPtr virshCommandOptSecret(vshControl *ctl, const vshCmd *cmd, const char **name) @@ -507,6 +508,7 @@ cmdSecretList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) virshSecretListPtr list = NULL; bool ret = false; unsigned int flags = 0; + vshTablePtr table = NULL; if (vshCommandOptBool(cmd, "ephemeral")) flags |= VIR_CONNECT_LIST_SECRETS_EPHEMERAL; @@ -523,15 +525,17 @@ cmdSecretList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) if (!(list = virshSecretListCollect(ctl, flags))) return false; - vshPrintExtra(ctl, " %-36s %s\n", _("UUID"), _("Usage")); - vshPrintExtra(ctl, "----------------------------------------" - "----------------------------------------\n"); + table = vshTableNew(_("UUID"), _("Usage"), NULL); + if (!table) + goto cleanup; for (i = 0; i < list->nsecrets; i++) { virSecretPtr sec = list->secrets[i]; int usageType = virSecretGetUsageType(sec); const char *usageStr = virSecretUsageTypeToString(usageType); char uuid[VIR_UUID_STRING_BUFLEN]; + virBuffer buf = VIR_BUFFER_INITIALIZER; + VIR_AUTOFREE(char *) usage = NULL; if (virSecretGetUUIDString(sec, uuid) < 0) { vshError(ctl, "%s", _("Failed to get uuid of secret")); @@ -539,18 +543,26 @@ cmdSecretList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) } if (usageType) { - vshPrint(ctl, " %-36s %s %s\n", - uuid, usageStr, - virSecretGetUsageID(sec)); + virBufferStrcat(&buf, usageStr, " ", + virSecretGetUsageID(sec), NULL); + usage = virBufferContentAndReset(&buf); + if (!usage) + goto cleanup; + + if (vshTableRowAppend(table, uuid, usage, NULL) < 0) + goto cleanup; } else { - vshPrint(ctl, " %-36s %s\n", - uuid, _("Unused")); + if (vshTableRowAppend(table, uuid, _("Unused"), NULL) < 0) + goto cleanup; } } + vshTablePrintToStdout(table, ctl); + ret = true; cleanup: + vshTableFree(table); virshSecretListFree(list); return ret; } -- 2.17.1
Signed-off-by: Simon Kobyda <skobyda@redhat.com> --- tools/virsh-nwfilter.c | 47 +++++++++++++++++++++++++++++------------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/tools/virsh-nwfilter.c b/tools/virsh-nwfilter.c index 1cdbe5053a..b680ea082c 100644 --- a/tools/virsh-nwfilter.c +++ b/tools/virsh-nwfilter.c @@ -31,6 +31,7 @@ #include "viralloc.h" #include "virfile.h" #include "virutil.h" +#include "vsh-table.h" virNWFilterPtr virshCommandOptNWFilterBy(vshControl *ctl, const vshCmd *cmd, @@ -359,26 +360,35 @@ cmdNWFilterList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { size_t i; char uuid[VIR_UUID_STRING_BUFLEN]; + bool ret = false; virshNWFilterListPtr list = NULL; + vshTablePtr table = NULL; if (!(list = virshNWFilterListCollect(ctl, 0))) return false; - vshPrintExtra(ctl, " %-36s %-20s \n", _("UUID"), _("Name")); - vshPrintExtra(ctl, "---------------------------------" - "---------------------------------\n"); + table = vshTableNew(_("UUID"), _("Name"), NULL); + if (!table) + goto cleanup; for (i = 0; i < list->nfilters; i++) { virNWFilterPtr nwfilter = list->filters[i]; virNWFilterGetUUIDString(nwfilter, uuid); - vshPrint(ctl, " %-36s %-20s\n", - uuid, - virNWFilterGetName(nwfilter)); + if (vshTableRowAppend(table, + uuid, + virNWFilterGetName(nwfilter), + NULL) < 0) + goto cleanup; } + vshTablePrintToStdout(table, ctl); + + ret = true; + cleanup: + vshTableFree(table); virshNWFilterListFree(list); - return true; + return ret; } /* @@ -714,25 +724,34 @@ static bool cmdNWFilterBindingList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { size_t i; + bool ret = false; virshNWFilterBindingListPtr list = NULL; + vshTablePtr table = NULL; if (!(list = virshNWFilterBindingListCollect(ctl, 0))) return false; - vshPrintExtra(ctl, " %-20s %-20s \n", _("Port Dev"), _("Filter")); - vshPrintExtra(ctl, "---------------------------------" - "---------------------------------\n"); + table = vshTableNew(_("Port Dev"), _("Filter"), NULL); + if (!table) + goto cleanup; for (i = 0; i < list->nbindings; i++) { virNWFilterBindingPtr binding = list->bindings[i]; - vshPrint(ctl, " %-20s %-20s\n", - virNWFilterBindingGetPortDev(binding), - virNWFilterBindingGetFilterName(binding)); + if (vshTableRowAppend(table, + virNWFilterBindingGetPortDev(binding), + virNWFilterBindingGetFilterName(binding), + NULL) < 0) + goto cleanup; } + vshTablePrintToStdout(table, ctl); + + ret = true; + cleanup: + vshTableFree(table); virshNWFilterBindingListFree(list); - return true; + return ret; } -- 2.17.1
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..73861957ba 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) + goto cleanup; + } else { + if (vshTableRowAppend(table, snap_name, timestr, state, + NULL) < 0) + goto cleanup; + } } + 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; } -- 2.17.1
I've moved all the printing from cmdDomblkinfoPrint() to cmdDomblkinfo(), and renamed the cmdDomblkinfoPrint() to cmdDomblkinfoGet(), since nature of that function changed from gathering and printing informations only to gathering information. This I believe simplifies the functions and makes the implementation of vshTable API simpler. Signed-off-by: Simon Kobyda <skobyda@redhat.com> --- tools/virsh-domain-monitor.c | 78 +++++++++++++++++------------------- 1 file changed, 36 insertions(+), 42 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index adc5bb1a7a..cb48f9a7be 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -406,33 +406,23 @@ static const vshCmdOptDef opts_domblkinfo[] = { {.name = NULL} }; -static void -cmdDomblkinfoPrint(vshControl *ctl, +static bool +cmdDomblkinfoGet(vshControl *ctl, const virDomainBlockInfo *info, - const char *device, - bool human, bool title) + char **cap, + char **alloc, + char **phy, + bool human) { - char *cap = NULL; - char *alloc = NULL; - char *phy = NULL; - - if (title) { - vshPrintExtra(ctl, "%-10s %-15s %-15s %-15s\n", _("Target"), - _("Capacity"), _("Allocation"), _("Physical")); - vshPrintExtra(ctl, "-----------------------------" - "------------------------\n"); - return; - } - if (info->capacity == 0 && info->allocation == 0 && info->physical == 0) { - cap = vshStrdup(ctl, "-"); - alloc = vshStrdup(ctl, "-"); - phy = vshStrdup(ctl, "-"); + *cap = vshStrdup(ctl, "-"); + *alloc = vshStrdup(ctl, "-"); + *phy = vshStrdup(ctl, "-"); } else if (!human) { - if (virAsprintf(&cap, "%llu", info->capacity) < 0 || - virAsprintf(&alloc, "%llu", info->allocation) < 0 || - virAsprintf(&phy, "%llu", info->physical) < 0) - goto cleanup; + if (virAsprintf(cap, "%llu", info->capacity) < 0 || + virAsprintf(alloc, "%llu", info->allocation) < 0 || + virAsprintf(phy, "%llu", info->physical) < 0) + return false; } else { double val_cap, val_alloc, val_phy; const char *unit_cap, *unit_alloc, *unit_phy; @@ -441,24 +431,13 @@ cmdDomblkinfoPrint(vshControl *ctl, val_alloc = vshPrettyCapacity(info->allocation, &unit_alloc); val_phy = vshPrettyCapacity(info->physical, &unit_phy); - if (virAsprintf(&cap, "%.3lf %s", val_cap, unit_cap) < 0 || - virAsprintf(&alloc, "%.3lf %s", val_alloc, unit_alloc) < 0 || - virAsprintf(&phy, "%.3lf %s", val_phy, unit_phy) < 0) - goto cleanup; - } - - if (device) { - vshPrint(ctl, "%-10s %-15s %-15s %-15s\n", device, cap, alloc, phy); - } else { - vshPrint(ctl, "%-15s %s\n", _("Capacity:"), cap); - vshPrint(ctl, "%-15s %s\n", _("Allocation:"), alloc); - vshPrint(ctl, "%-15s %s\n", _("Physical:"), phy); + if (virAsprintf(cap, "%.3lf %s", val_cap, unit_cap) < 0 || + virAsprintf(alloc, "%.3lf %s", val_alloc, unit_alloc) < 0 || + virAsprintf(phy, "%.3lf %s", val_phy, unit_phy) < 0) + return false; } - cleanup: - VIR_FREE(cap); - VIR_FREE(alloc); - VIR_FREE(phy); + return true; } @@ -478,6 +457,9 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd) xmlNodePtr *disks = NULL; char *target = NULL; char *protocol = NULL; + char *cap = NULL; + char *alloc = NULL; + char *phy = NULL; if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; @@ -502,7 +484,10 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd) goto cleanup; /* print the title */ - cmdDomblkinfoPrint(ctl, NULL, NULL, false, true); + vshPrintExtra(ctl, "%-10s %-15s %-15s %-15s\n", _("Target"), + _("Capacity"), _("Allocation"), _("Physical")); + vshPrintExtra(ctl, "-----------------------------" + "------------------------\n"); for (i = 0; i < ndisks; i++) { ctxt->node = disks[i]; @@ -525,7 +510,9 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd) } } - cmdDomblkinfoPrint(ctl, &info, target, human, false); + if (!cmdDomblkinfoGet(ctl, &info, &cap, &alloc, &phy, human)) + goto cleanup; + vshPrint(ctl, "%-10s %-15s %-15s %-15s\n", target, cap, alloc, phy); VIR_FREE(target); VIR_FREE(protocol); @@ -534,12 +521,19 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd) if (virDomainGetBlockInfo(dom, device, &info, 0) < 0) goto cleanup; - cmdDomblkinfoPrint(ctl, &info, NULL, human, false); + if (!cmdDomblkinfoGet(ctl, &info, &cap, &alloc, &phy, human)) + goto cleanup; + vshPrint(ctl, "%-15s %s\n", _("Capacity:"), cap); + vshPrint(ctl, "%-15s %s\n", _("Allocation:"), alloc); + vshPrint(ctl, "%-15s %s\n", _("Physical:"), phy); } ret = true; cleanup: + VIR_FREE(cap); + VIR_FREE(alloc); + VIR_FREE(phy); virshDomainFree(dom); VIR_FREE(target); VIR_FREE(protocol); -- 2.17.1
Signed-off-by: Simon Kobyda <skobyda@redhat.com> --- tools/virsh-domain-monitor.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index cb48f9a7be..80c7e8a99e 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -460,6 +460,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd) char *cap = NULL; char *alloc = NULL; char *phy = NULL; + vshTablePtr table = NULL; if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; @@ -483,11 +484,10 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd) if (ndisks < 0) goto cleanup; - /* print the title */ - vshPrintExtra(ctl, "%-10s %-15s %-15s %-15s\n", _("Target"), - _("Capacity"), _("Allocation"), _("Physical")); - vshPrintExtra(ctl, "-----------------------------" - "------------------------\n"); + /* title */ + table = vshTableNew(_("Target"), _("Capacity"), _("Allocation"), _("Physical"), NULL); + if (!table) + goto cleanup; for (i = 0; i < ndisks; i++) { ctxt->node = disks[i]; @@ -512,11 +512,15 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd) if (!cmdDomblkinfoGet(ctl, &info, &cap, &alloc, &phy, human)) goto cleanup; - vshPrint(ctl, "%-10s %-15s %-15s %-15s\n", target, cap, alloc, phy); + if (vshTableRowAppend(table, target, cap, alloc, phy, NULL) < 0) + goto cleanup; VIR_FREE(target); VIR_FREE(protocol); } + + vshTablePrintToStdout(table, ctl); + } else { if (virDomainGetBlockInfo(dom, device, &info, 0) < 0) goto cleanup; @@ -531,6 +535,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd) ret = true; cleanup: + vshTableFree(table); VIR_FREE(cap); VIR_FREE(alloc); VIR_FREE(phy); -- 2.17.1
Signed-off-by: Simon Kobyda <skobyda@redhat.com> --- tools/virsh-domain-monitor.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 80c7e8a99e..b887bb48d9 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -589,6 +589,7 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd) char *device = NULL; char *target = NULL; char *source = NULL; + vshTablePtr table = NULL; if (vshCommandOptBool(cmd, "inactive")) flags |= VIR_DOMAIN_XML_INACTIVE; @@ -603,12 +604,12 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd) goto cleanup; if (details) - vshPrintExtra(ctl, "%-10s %-10s %-10s %s\n", _("Type"), - _("Device"), _("Target"), _("Source")); + table = vshTableNew(_("Type"), _("Device"), _("Target"), _("Source"), NULL); else - vshPrintExtra(ctl, "%-10s %s\n", _("Target"), _("Source")); + table = vshTableNew(_("Target"), _("Source"), NULL); - vshPrintExtra(ctl, "------------------------------------------------\n"); + if (!table) + goto cleanup; for (i = 0; i < ndisks; i++) { ctxt->node = disks[i]; @@ -633,10 +634,13 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd) "|./source/@name" "|./source/@volume)", ctxt); if (details) { - vshPrint(ctl, "%-10s %-10s %-10s %s\n", type, device, - target, source ? source : "-"); + if (vshTableRowAppend(table, type, device, target, + source ? source : "-", NULL) < 0) + goto cleanup; } else { - vshPrint(ctl, "%-10s %s\n", target, source ? source : "-"); + if (vshTableRowAppend(table, target, + source ? source : "-", NULL) < 0) + goto cleanup; } VIR_FREE(source); @@ -645,9 +649,12 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd) VIR_FREE(type); } + vshTablePrintToStdout(table, ctl); + ret = true; cleanup: + vshTableFree(table); VIR_FREE(source); VIR_FREE(target); VIR_FREE(device); -- 2.17.1
Signed-off-by: Simon Kobyda <skobyda@redhat.com> --- tools/virsh-domain-monitor.c | 41 ++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index b887bb48d9..4bba8438af 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -693,6 +693,7 @@ cmdDomiflist(vshControl *ctl, const vshCmd *cmd) int ninterfaces; xmlNodePtr *interfaces = NULL; size_t i; + vshTablePtr table = NULL; if (vshCommandOptBool(cmd, "inactive")) flags |= VIR_DOMAIN_XML_INACTIVE; @@ -704,16 +705,17 @@ cmdDomiflist(vshControl *ctl, const vshCmd *cmd) if (ninterfaces < 0) goto cleanup; - vshPrintExtra(ctl, "%-10s %-10s %-10s %-11s %s\n", _("Interface"), - _("Type"), _("Source"), _("Model"), _("MAC")); - vshPrintExtra(ctl, "-------------------------------------------------------\n"); + table = vshTableNew(_("Interface"), _("Type"), + _("Source"), _("Model"), _("MAC"), NULL); + if (!table) + goto cleanup; for (i = 0; i < ninterfaces; i++) { - char *type = NULL; - char *source = NULL; - char *target = NULL; - char *model = NULL; - char *mac = NULL; + VIR_AUTOFREE(char *) type = NULL; + VIR_AUTOFREE(char *) source = NULL; + VIR_AUTOFREE(char *) target = NULL; + VIR_AUTOFREE(char *) model = NULL; + VIR_AUTOFREE(char *) mac = NULL; ctxt->node = interfaces[i]; type = virXPathString("string(./@type)", ctxt); @@ -727,23 +729,22 @@ cmdDomiflist(vshControl *ctl, const vshCmd *cmd) model = virXPathString("string(./model/@type)", ctxt); mac = virXPathString("string(./mac/@address)", ctxt); - vshPrint(ctl, "%-10s %-10s %-10s %-11s %-10s\n", - target ? target : "-", - type, - source ? source : "-", - model ? model : "-", - mac ? mac : "-"); - - VIR_FREE(type); - VIR_FREE(source); - VIR_FREE(target); - VIR_FREE(model); - VIR_FREE(mac); + if (vshTableRowAppend(table, + target ? target : "-", + type, + source ? source : "-", + model ? model : "-", + mac ? mac : "-", + NULL) < 0) + goto cleanup; } + vshTablePrintToStdout(table, ctl); + ret = true; cleanup: + vshTableFree(table); VIR_FREE(interfaces); xmlFreeDoc(xmldoc); xmlXPathFreeContext(ctxt); -- 2.17.1
Signed-off-by: Simon Kobyda <skobyda@redhat.com> --- tools/virsh-domain.c | 98 +++++++++++++++++++++++++++++++------------- 1 file changed, 70 insertions(+), 28 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index c1cff9fe2d..2a416b919a 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -59,6 +59,7 @@ #include "virxml.h" #include "virsh-nodedev.h" #include "viruri.h" +#include "vsh-table.h" /* Gnulib doesn't guarantee SA_SIGINFO support. */ #ifndef SA_SIGINFO @@ -6905,6 +6906,7 @@ virshVcpuPinQuery(vshControl *ctl, size_t i; int ncpus; bool ret = false; + vshTablePtr table = NULL; if ((ncpus = virshCPUCountCollect(ctl, dom, countFlags, true)) < 0) { if (ncpus == -1) { @@ -6913,7 +6915,7 @@ virshVcpuPinQuery(vshControl *ctl, else vshError(ctl, "%s", _("cannot get vcpupin for transient domain")); } - return false; + goto cleanup; } if (got_vcpu && vcpu >= ncpus) { @@ -6927,28 +6929,39 @@ virshVcpuPinQuery(vshControl *ctl, vshError(ctl, _("vcpu %d is out of range of persistent cpu count %d"), vcpu, ncpus); - return false; + goto cleanup; } cpumaplen = VIR_CPU_MAPLEN(maxcpu); cpumap = vshMalloc(ctl, ncpus * cpumaplen); if ((ncpus = virDomainGetVcpuPinInfo(dom, ncpus, cpumap, cpumaplen, flags)) >= 0) { - vshPrintExtra(ctl, "%s %s\n", _("VCPU:"), _("CPU Affinity")); - vshPrintExtra(ctl, "----------------------------------\n"); + table = vshTableNew(_("VCPU"), _("CPU Affinity"), NULL); + if (!table) + goto cleanup; + for (i = 0; i < ncpus; i++) { + VIR_AUTOFREE(char *) pinInfo = NULL; + VIR_AUTOFREE(char *) vcpuStr = NULL; if (got_vcpu && i != vcpu) continue; - vshPrint(ctl, "%4zu: ", i); - ret = virshPrintPinInfo(ctl, VIR_GET_CPUMAP(cpumap, cpumaplen, i), - cpumaplen); - vshPrint(ctl, "\n"); - if (!ret) - break; + if (!(pinInfo = virBitmapDataFormat(cpumap, cpumaplen))) + goto cleanup; + + if (virAsprintf(&vcpuStr, "%lu", i) < 0) + goto cleanup; + + if (vshTableRowAppend(table, vcpuStr, pinInfo, NULL) < 0) + goto cleanup; } + + vshTablePrintToStdout(table, ctl); } + ret = true; + cleanup: + vshTableFree(table); VIR_FREE(cpumap); return ret; } @@ -7520,6 +7533,7 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) int maxcpu; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; virshControlPtr priv = ctl->privData; + vshTablePtr table = NULL; VSH_EXCLUSIVE_OPTIONS_VAR(current, live); VSH_EXCLUSIVE_OPTIONS_VAR(current, config); @@ -7545,19 +7559,30 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - vshPrintExtra(ctl, " %-15s %-15s\n", - _("IOThread ID"), _("CPU Affinity")); - vshPrintExtra(ctl, "---------------------------------------------------\n"); + table = vshTableNew(_("IOThread ID"), _("CPU Affinity"), NULL); + if (!table) + goto cleanup; + for (i = 0; i < niothreads; i++) { + VIR_AUTOFREE(char *) pinInfo = NULL; + VIR_AUTOFREE(char *) iothreadIdStr = NULL; - vshPrint(ctl, " %-15u ", info[i]->iothread_id); - ignore_value(virshPrintPinInfo(ctl, info[i]->cpumap, info[i]->cpumaplen)); - vshPrint(ctl, "\n"); - virDomainIOThreadInfoFree(info[i]); + if (virAsprintf(&iothreadIdStr, "%u", info[i]->iothread_id) < 0) + goto cleanup; + + ignore_value(pinInfo = virBitmapDataFormat(info[i]->cpumap, info[i]->cpumaplen)); + + if (vshTableRowAppend(table, iothreadIdStr, pinInfo ? pinInfo : "", NULL) < 0) + goto cleanup; } - VIR_FREE(info); + + vshTablePrintToStdout(table, ctl); cleanup: + for (i = 0; i < niothreads; i++) + virDomainIOThreadInfoFree(info[i]); + VIR_FREE(info); + vshTableFree(table); virshDomainFree(dom); return niothreads >= 0; } @@ -13778,6 +13803,7 @@ cmdDomFSInfo(vshControl *ctl, const vshCmd *cmd) int ret = -1; size_t i, j; virDomainFSInfoPtr *info; + vshTablePtr table = NULL; if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; @@ -13793,25 +13819,41 @@ cmdDomFSInfo(vshControl *ctl, const vshCmd *cmd) } if (info) { - vshPrintExtra(ctl, "%-36s %-8s %-8s %s\n", - _("Mountpoint"), _("Name"), _("Type"), _("Target")); - vshPrintExtra(ctl, "-------------------------------------------------------------------\n"); + table = vshTableNew(_("Mountpoint"), _("Name"), _("Type"), _("Target"), NULL); + if (!table) + goto cleanup; + for (i = 0; i < ret; i++) { - vshPrint(ctl, "%-36s %-8s %-8s ", - info[i]->mountpoint, info[i]->name, info[i]->fstype); + virBuffer targetsBuff = VIR_BUFFER_INITIALIZER; + VIR_AUTOFREE(char *) targets = NULL; + for (j = 0; j < info[i]->ndevAlias; j++) { - vshPrint(ctl, "%s", info[i]->devAlias[j]); + virBufferAdd(&targetsBuff, info[i]->devAlias[j], -1); if (j != info[i]->ndevAlias - 1) - vshPrint(ctl, ","); + virBufferAddChar(&targetsBuff, ','); } - vshPrint(ctl, "\n"); - virDomainFSInfoFree(info[i]); + targets = virBufferContentAndReset(&targetsBuff); + + if (vshTableRowAppend(table, + info[i]->mountpoint, + info[i]->name, + info[i]->fstype, + targets, + NULL) < 0) + goto cleanup; } - VIR_FREE(info); + + vshTablePrintToStdout(table, ctl); } cleanup: + if (info) { + for (i = 0; i < ret; i++) + virDomainFSInfoFree(info[i]); + VIR_FREE(info); + } + vshTableFree(table); virshDomainFree(dom); return ret >= 0; } -- 2.17.1
Local lengthy unicode-unreliable table formatting was replaced by new API. Great example of how new API saves space and time. Removed a lot of string lenght canculation used by the local table. Signed-off-by: Simon Kobyda <skobyda@redhat.com> --- tools/virsh-pool.c | 162 +++++++++------------------------------------ 1 file changed, 31 insertions(+), 131 deletions(-) diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 89206a48f5..75ec572af2 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -33,6 +33,7 @@ #include "conf/storage_conf.h" #include "virstring.h" #include "virtime.h" +#include "vsh-table.h" #define VIRSH_COMMON_OPT_POOL_FULL(cflags) \ VIRSH_COMMON_OPT_POOL(N_("pool name or uuid"), cflags) @@ -1113,10 +1114,6 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) virStoragePoolInfo info; size_t i; bool ret = false; - size_t stringLength = 0, nameStrLength = 0; - size_t autostartStrLength = 0, persistStrLength = 0; - size_t stateStrLength = 0, capStrLength = 0; - size_t allocStrLength = 0, availStrLength = 0; struct poolInfoText { char *state; char *autostart; @@ -1133,7 +1130,7 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) bool inactive, all; bool uuid = false; bool name = false; - char *outputStr = NULL; + vshTablePtr table = NULL; inactive = vshCommandOptBool(cmd, "inactive"); all = vshCommandOptBool(cmd, "all"); @@ -1260,11 +1257,6 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) else poolInfoTexts[i].persistent = vshStrdup(ctl, persistent ? _("yes") : _("no")); - - /* Keep the length of persistent string if longest so far */ - stringLength = strlen(poolInfoTexts[i].persistent); - if (stringLength > persistStrLength) - persistStrLength = stringLength; } /* Collect further extended information about the pool */ @@ -1310,21 +1302,6 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) poolInfoTexts[i].allocation = vshStrdup(ctl, _("-")); poolInfoTexts[i].available = vshStrdup(ctl, _("-")); } - - /* Keep the length of capacity string if longest so far */ - stringLength = strlen(poolInfoTexts[i].capacity); - if (stringLength > capStrLength) - capStrLength = stringLength; - - /* Keep the length of allocation string if longest so far */ - stringLength = strlen(poolInfoTexts[i].allocation); - if (stringLength > allocStrLength) - allocStrLength = stringLength; - - /* Keep the length of available string if longest so far */ - stringLength = strlen(poolInfoTexts[i].available); - if (stringLength > availStrLength) - availStrLength = stringLength; } else { /* --details option was not specified, only active/inactive * state strings are used */ @@ -1334,21 +1311,6 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) poolInfoTexts[i].state = vshStrdup(ctl, _("inactive")); } } - - /* Keep the length of name string if longest so far */ - stringLength = strlen(virStoragePoolGetName(list->pools[i])); - if (stringLength > nameStrLength) - nameStrLength = stringLength; - - /* Keep the length of state string if longest so far */ - stringLength = strlen(poolInfoTexts[i].state); - if (stringLength > stateStrLength) - stateStrLength = stringLength; - - /* Keep the length of autostart string if longest so far */ - stringLength = strlen(poolInfoTexts[i].autostart); - if (stringLength > autostartStrLength) - autostartStrLength = stringLength; } /* If the --details option wasn't selected, we output the pool @@ -1376,19 +1338,23 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) } /* Output old style header */ - vshPrintExtra(ctl, " %-20s %-10s %-10s\n", _("Name"), _("State"), - _("Autostart")); - vshPrintExtra(ctl, "-------------------------------------------\n"); + table = vshTableNew(_("Name"), _("State"), _("Autostart"), NULL); + if (!table) + goto cleanup; /* Output old style pool info */ for (i = 0; i < list->npools; i++) { const char *name_str = virStoragePoolGetName(list->pools[i]); - vshPrint(ctl, " %-20s %-10s %-10s\n", - name_str, - poolInfoTexts[i].state, - poolInfoTexts[i].autostart); + if (vshTableRowAppend(table, + name_str, + poolInfoTexts[i].state, + poolInfoTexts[i].autostart, + NULL) < 0) + goto cleanup; } + vshTablePrintToStdout(table, ctl); + /* Cleanup and return */ ret = true; goto cleanup; @@ -1396,99 +1362,33 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) /* We only get here if the --details option was selected. */ - /* Use the length of name header string if it's longest */ - stringLength = strlen(_("Name")); - if (stringLength > nameStrLength) - nameStrLength = stringLength; - - /* Use the length of state header string if it's longest */ - stringLength = strlen(_("State")); - if (stringLength > stateStrLength) - stateStrLength = stringLength; - - /* Use the length of autostart header string if it's longest */ - stringLength = strlen(_("Autostart")); - if (stringLength > autostartStrLength) - autostartStrLength = stringLength; - - /* Use the length of persistent header string if it's longest */ - stringLength = strlen(_("Persistent")); - if (stringLength > persistStrLength) - persistStrLength = stringLength; - - /* Use the length of capacity header string if it's longest */ - stringLength = strlen(_("Capacity")); - if (stringLength > capStrLength) - capStrLength = stringLength; - - /* Use the length of allocation header string if it's longest */ - stringLength = strlen(_("Allocation")); - if (stringLength > allocStrLength) - allocStrLength = stringLength; - - /* Use the length of available header string if it's longest */ - stringLength = strlen(_("Available")); - if (stringLength > availStrLength) - availStrLength = stringLength; - - /* Display the string lengths for debugging. */ - vshDebug(ctl, VSH_ERR_DEBUG, "Longest name string = %lu chars\n", - (unsigned long) nameStrLength); - vshDebug(ctl, VSH_ERR_DEBUG, "Longest state string = %lu chars\n", - (unsigned long) stateStrLength); - vshDebug(ctl, VSH_ERR_DEBUG, "Longest autostart string = %lu chars\n", - (unsigned long) autostartStrLength); - vshDebug(ctl, VSH_ERR_DEBUG, "Longest persistent string = %lu chars\n", - (unsigned long) persistStrLength); - vshDebug(ctl, VSH_ERR_DEBUG, "Longest capacity string = %lu chars\n", - (unsigned long) capStrLength); - vshDebug(ctl, VSH_ERR_DEBUG, "Longest allocation string = %lu chars\n", - (unsigned long) allocStrLength); - vshDebug(ctl, VSH_ERR_DEBUG, "Longest available string = %lu chars\n", - (unsigned long) availStrLength); - - /* Create the output template. Each column is sized according to - * the longest string. - */ - if (virAsprintf(&outputStr, - " %%-%lus %%-%lus %%-%lus %%-%lus %%%lus %%%lus %%%lus\n", - (unsigned long) nameStrLength, - (unsigned long) stateStrLength, - (unsigned long) autostartStrLength, - (unsigned long) persistStrLength, - (unsigned long) capStrLength, - (unsigned long) allocStrLength, - (unsigned long) availStrLength) < 0) + /* Insert the header into table */ + table = vshTableNew(_("Name"), _("State"), _("Autostart"), _("Persistent"), + _("Capacity"), _("Allocation"), _("Available"), NULL); + if (!table) goto cleanup; - /* Display the header */ - vshPrintExtra(ctl, outputStr, _("Name"), _("State"), _("Autostart"), - _("Persistent"), _("Capacity"), _("Allocation"), - _("Available")); - for (i = nameStrLength + stateStrLength + autostartStrLength - + persistStrLength + capStrLength - + allocStrLength + availStrLength - + 14; i > 0; i--) - vshPrintExtra(ctl, "-"); - vshPrintExtra(ctl, "\n"); - - /* Display the pool info rows */ + /* Insert the pool info rows into table*/ for (i = 0; i < list->npools; i++) { - vshPrint(ctl, outputStr, - virStoragePoolGetName(list->pools[i]), - poolInfoTexts[i].state, - poolInfoTexts[i].autostart, - poolInfoTexts[i].persistent, - poolInfoTexts[i].capacity, - poolInfoTexts[i].allocation, - poolInfoTexts[i].available); + if (vshTableRowAppend(table, + virStoragePoolGetName(list->pools[i]), + poolInfoTexts[i].state, + poolInfoTexts[i].autostart, + poolInfoTexts[i].persistent, + poolInfoTexts[i].capacity, + poolInfoTexts[i].allocation, + poolInfoTexts[i].available, + NULL) < 0) + goto cleanup; } + vshTablePrintToStdout(table, ctl); + /* Cleanup and return */ ret = true; cleanup: - VIR_FREE(outputStr); + vshTableFree(table); if (list && list->npools) { for (i = 0; i < list->npools; i++) { VIR_FREE(poolInfoTexts[i].state); -- 2.17.1
Local lengthy unicode-unreliable table formatting was replaced by new API. Great example of how new API saves space and time. Removed a lot of string lenght calculation used by the local table. Signed-off-by: Simon Kobyda <skobyda@redhat.com> --- tools/virsh-volume.c | 129 ++++++++++--------------------------------- 1 file changed, 28 insertions(+), 101 deletions(-) diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 42d11701ec..6cd989446e 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -42,6 +42,7 @@ #include "virsh-pool.h" #include "virxml.h" #include "virstring.h" +#include "vsh-table.h" #define VIRSH_COMMON_OPT_POOL_FULL \ VIRSH_COMMON_OPT_POOL(N_("pool name or uuid"), \ @@ -1382,16 +1383,11 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { virStorageVolInfo volumeInfo; virStoragePoolPtr pool; - char *outputStr = NULL; const char *unit; double val; bool details = vshCommandOptBool(cmd, "details"); size_t i; bool ret = false; - int stringLength = 0; - size_t allocStrLength = 0, capStrLength = 0; - size_t nameStrLength = 0, pathStrLength = 0; - size_t typeStrLength = 0; struct volInfoText { char *allocation; char *capacity; @@ -1400,6 +1396,7 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) }; struct volInfoText *volInfoTexts = NULL; virshStorageVolListPtr list = NULL; + vshTablePtr table = NULL; /* Look up the pool information given to us by the user */ if (!(pool = virshCommandOptPool(ctl, cmd, "pool", NULL))) @@ -1446,36 +1443,6 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) "%.2lf %s", val, unit) < 0) goto cleanup; } - - /* Remember the largest length for each output string. - * This lets us displaying header and volume information rows - * using a single, properly sized, printf style output string. - */ - - /* Keep the length of name string if longest so far */ - stringLength = strlen(virStorageVolGetName(list->vols[i])); - if (stringLength > nameStrLength) - nameStrLength = stringLength; - - /* Keep the length of path string if longest so far */ - stringLength = strlen(volInfoTexts[i].path); - if (stringLength > pathStrLength) - pathStrLength = stringLength; - - /* Keep the length of type string if longest so far */ - stringLength = strlen(volInfoTexts[i].type); - if (stringLength > typeStrLength) - typeStrLength = stringLength; - - /* Keep the length of capacity string if longest so far */ - stringLength = strlen(volInfoTexts[i].capacity); - if (stringLength > capStrLength) - capStrLength = stringLength; - - /* Keep the length of allocation string if longest so far */ - stringLength = strlen(volInfoTexts[i].allocation); - if (stringLength > allocStrLength) - allocStrLength = stringLength; } } @@ -1487,14 +1454,20 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) /* Output basic info then return if --details option not selected */ if (!details) { /* The old output format */ - vshPrintExtra(ctl, " %-20s %-40s\n", _("Name"), _("Path")); - vshPrintExtra(ctl, "---------------------------------------" - "---------------------------------------\n"); + table = vshTableNew(_("Name"), _("Path"), NULL); + if (!table) + goto cleanup; + for (i = 0; i < list->nvols; i++) { - vshPrint(ctl, " %-20s %-40s\n", virStorageVolGetName(list->vols[i]), - volInfoTexts[i].path); + if (vshTableRowAppend(table, + virStorageVolGetName(list->vols[i]), + volInfoTexts[i].path, + NULL) < 0) + goto cleanup; } + vshTablePrintToStdout(table, ctl); + /* Cleanup and return */ ret = true; goto cleanup; @@ -1502,75 +1475,30 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) /* We only get here if the --details option was selected. */ - /* Use the length of name header string if it's longest */ - stringLength = strlen(_("Name")); - if (stringLength > nameStrLength) - nameStrLength = stringLength; - - /* Use the length of path header string if it's longest */ - stringLength = strlen(_("Path")); - if (stringLength > pathStrLength) - pathStrLength = stringLength; - - /* Use the length of type header string if it's longest */ - stringLength = strlen(_("Type")); - if (stringLength > typeStrLength) - typeStrLength = stringLength; - - /* Use the length of capacity header string if it's longest */ - stringLength = strlen(_("Capacity")); - if (stringLength > capStrLength) - capStrLength = stringLength; - - /* Use the length of allocation header string if it's longest */ - stringLength = strlen(_("Allocation")); - if (stringLength > allocStrLength) - allocStrLength = stringLength; - - /* Display the string lengths for debugging */ - vshDebug(ctl, VSH_ERR_DEBUG, - "Longest name string = %zu chars\n", nameStrLength); - vshDebug(ctl, VSH_ERR_DEBUG, - "Longest path string = %zu chars\n", pathStrLength); - vshDebug(ctl, VSH_ERR_DEBUG, - "Longest type string = %zu chars\n", typeStrLength); - vshDebug(ctl, VSH_ERR_DEBUG, - "Longest capacity string = %zu chars\n", capStrLength); - vshDebug(ctl, VSH_ERR_DEBUG, - "Longest allocation string = %zu chars\n", allocStrLength); - - if (virAsprintf(&outputStr, - " %%-%lus %%-%lus %%-%lus %%%lus %%%lus\n", - (unsigned long) nameStrLength, - (unsigned long) pathStrLength, - (unsigned long) typeStrLength, - (unsigned long) capStrLength, - (unsigned long) allocStrLength) < 0) + /* Insert the header into table */ + table = vshTableNew(_("Name"), _("Path"), _("Type"), _("Capacity"), _("Allocation"), NULL); + if (!table) goto cleanup; - /* Display the header */ - vshPrintExtra(ctl, outputStr, _("Name"), _("Path"), _("Type"), - _("Capacity"), _("Allocation")); - for (i = nameStrLength + pathStrLength + typeStrLength - + capStrLength + allocStrLength - + 10; i > 0; i--) - vshPrintExtra(ctl, "-"); - vshPrintExtra(ctl, "\n"); - - /* Display the volume info rows */ + /* Insert the volume info rows into table */ for (i = 0; i < list->nvols; i++) { - vshPrint(ctl, outputStr, - virStorageVolGetName(list->vols[i]), - volInfoTexts[i].path, - volInfoTexts[i].type, - volInfoTexts[i].capacity, - volInfoTexts[i].allocation); + if (vshTableRowAppend(table, + virStorageVolGetName(list->vols[i]), + volInfoTexts[i].path, + volInfoTexts[i].type, + volInfoTexts[i].capacity, + volInfoTexts[i].allocation, + NULL) < 0) + goto cleanup; } + vshTablePrintToStdout(table, ctl); + /* Cleanup and return */ ret = true; cleanup: + vshTableFree(table); /* Safely free the memory allocated in this function */ if (list && list->nvols) { @@ -1584,7 +1512,6 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) } /* Cleanup remaining memory */ - VIR_FREE(outputStr); VIR_FREE(volInfoTexts); virStoragePoolFree(pool); virshStorageVolListFree(list); -- 2.17.1
Signed-off-by: Simon Kobyda <skobyda@redhat.com> --- tools/virt-admin.c | 47 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 35 insertions(+), 12 deletions(-) diff --git a/tools/virt-admin.c b/tools/virt-admin.c index 63822bc13e..d119e4d960 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -40,6 +40,7 @@ #include "virgettext.h" #include "virtime.h" #include "virt-admin-completer.h" +#include "vsh-table.h" /* Gnulib doesn't guarantee SA_SIGINFO support. */ #ifndef SA_SIGINFO @@ -382,6 +383,7 @@ cmdSrvList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) char *uri = NULL; virAdmServerPtr *srvs = NULL; vshAdmControlPtr priv = ctl->privData; + vshTablePtr table = NULL; /* Obtain a list of available servers on the daemon */ if ((nsrvs = virAdmConnectListServers(priv->conn, &srvs, 0)) < 0) { @@ -391,13 +393,27 @@ cmdSrvList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) goto cleanup; } - vshPrintExtra(ctl, " %-5s %-15s\n", "Id", "Name"); - vshPrintExtra(ctl, "---------------\n"); - for (i = 0; i < nsrvs; i++) - vshPrint(ctl, " %-5zu %-15s\n", i, virAdmServerGetName(srvs[i])); + table = vshTableNew(_("Id"), _("Name"), NULL); + if (!table) + goto cleanup; + + for (i = 0; i < nsrvs; i++) { + VIR_AUTOFREE(char *) idStr = NULL; + if (virAsprintf(&idStr, "%lu", i) < 0) + goto cleanup; + + if (vshTableRowAppend(table, + idStr, + virAdmServerGetName(srvs[i]), + NULL) < 0) + goto cleanup; + } + + vshTablePrintToStdout(table, ctl); ret = true; cleanup: + vshTableFree(table); if (srvs) { for (i = 0; i < nsrvs; i++) virAdmServerFree(srvs[i]); @@ -613,10 +629,10 @@ cmdSrvClientsList(vshControl *ctl, const vshCmd *cmd) const char *srvname = NULL; unsigned long long id; virClientTransport transport; - char *timestr = NULL; virAdmServerPtr srv = NULL; virAdmClientPtr *clts = NULL; vshAdmControlPtr priv = ctl->privData; + vshTablePtr table = NULL; if (vshCommandOptStringReq(ctl, cmd, "server", &srvname) < 0) return false; @@ -631,12 +647,13 @@ cmdSrvClientsList(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - vshPrintExtra(ctl, " %-5s %-15s %-15s\n%s\n", _("Id"), _("Transport"), - _("Connected since"), - "-------------------------" - "-------------------------"); + table = vshTableNew(_("Id"), _("Transport"), _("Connected sice"), NULL); + if (!table) + goto cleanup; for (i = 0; i < nclts; i++) { + VIR_AUTOFREE(char *) timestr = NULL; + VIR_AUTOFREE(char *) idStr = NULL; virAdmClientPtr client = clts[i]; id = virAdmClientGetID(client); transport = virAdmClientGetTransport(client); @@ -644,14 +661,20 @@ cmdSrvClientsList(vshControl *ctl, const vshCmd *cmd) ×tr) < 0) goto cleanup; - vshPrint(ctl, " %-5llu %-15s %-15s\n", - id, vshAdmClientTransportToString(transport), timestr); - VIR_FREE(timestr); + if (virAsprintf(&idStr, "%llu", id) < 0) + goto cleanup; + if (vshTableRowAppend(table, idStr, + vshAdmClientTransportToString(transport), + timestr, NULL) < 0) + goto cleanup; } + vshTablePrintToStdout(table, ctl); + ret = true; cleanup: + vshTableFree(table); if (clts) { for (i = 0; i < nclts; i++) virAdmClientFree(clts[i]); -- 2.17.1
On 09/21/2018 04:17 PM, Simon Kobyda wrote:
API is implemented in virsh and virt-admin. It fixes problems with table-alignment, makes tables more readable and deals with unicode. Table however could not be implemented in places, which do use simple lists, or table implementation would change content of output, which users might rely on. If you know about other place where it could be implemented, let me know.
https://bugzilla.redhat.com/show_bug.cgi?id=1574624 https://bugzilla.redhat.com/show_bug.cgi?id=1584630
Changes in v2: - Implemented gettext function - Fixed few possible leaks caused by inserting "goto cleanup" statements - Implemented VIR_AUTOFREE where it's appropriate
Simon Kobyda (13): virsh: Implement vsh-table to iface-list virsh: Implement vshTable API to net-list and net-dhcp-leases virsh: Implement vshTable API to secret-list virsh: Implement vshTable API to nwfilter-list and nwfilterbinding-list virsh: Implement vshTable API to snapshot-list. virsh: Set up cmdDomblkinfo() and cmdDomblkinfoPrint() for vshTable API implementation virsh: Implement vshTable API to domblkinfo virsh: Implement vshTable API to domblklist virsh: Implement vshTable API to domiflist virsh: Implement vshTable API to vcpupin, iothreadinfo, domfsinfo virsh: Implement vshTable API to pool-list virsh: Implement vshTable API to vol-list virt-admin: Implement vshTable API to server-list and client-list
tools/virsh-domain-monitor.c | 147 ++++++++++++++++--------------- tools/virsh-domain.c | 98 +++++++++++++++------ tools/virsh-interface.c | 27 ++++-- tools/virsh-network.c | 59 ++++++++----- tools/virsh-nwfilter.c | 47 +++++++--- tools/virsh-pool.c | 162 +++++++---------------------------- tools/virsh-secret.c | 28 ++++-- tools/virsh-snapshot.c | 33 ++++--- tools/virsh-volume.c | 129 ++++++---------------------- tools/virt-admin.c | 47 +++++++--- 10 files changed, 371 insertions(+), 406 deletions(-)
ACKed and pushed. Michal
participants (2)
-
Michal Privoznik -
Simon Kobyda