[libvirt] [PATCH 0/6] Print cpu maps in human readable form
Consistently report errors in virBitmapFormat and introduce a flag for vcpuinfo and nodecpumap for more readable cpu map outputs. Ján Tomko (6): Invert logic in cmdVcpuinfo Separate API calls and result printing in cmdVcpuinfo Format NULL bitmap as an empty string Always report an error if virBitmapFormat fails Introudce virBitmapDataToString Implement pretty flag for vcpuinfo and nodecpumap src/conf/capabilities.c | 4 +- src/conf/domain_conf.c | 19 ++-------- src/conf/network_conf.c | 2 +- src/libvirt_private.syms | 1 + src/libxl/libxl_driver.c | 3 +- src/lxc/lxc_cgroup.c | 11 +----- src/qemu/qemu_cgroup.c | 16 ++------ src/qemu/qemu_driver.c | 12 ++---- src/util/virbitmap.c | 32 +++++++++++++--- src/util/virbitmap.h | 7 +++- src/util/virstring.h | 1 - tests/virbitmaptest.c | 14 ++++++- tools/virsh-domain.c | 97 +++++++++++++++++++++++++----------------------- tools/virsh-host.c | 25 +++++++++++-- 14 files changed, 134 insertions(+), 110 deletions(-) -- 1.8.3.2
Initialize 'ret' to false and introduce a cleanup label. --- tools/virsh-domain.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 84a6706..4d21704 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5536,25 +5536,21 @@ cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd) { virDomainInfo info; virDomainPtr dom; - virVcpuInfoPtr cpuinfo; - unsigned char *cpumaps; + virVcpuInfoPtr cpuinfo = NULL; + unsigned char *cpumaps = NULL; int ncpus, maxcpu; size_t cpumaplen; - bool ret = true; + bool ret = false; int n, m; if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if ((maxcpu = vshNodeGetCPUCount(ctl->conn)) < 0) { - virDomainFree(dom); - return false; - } + if ((maxcpu = vshNodeGetCPUCount(ctl->conn)) < 0) + goto cleanup; - if (virDomainGetInfo(dom, &info) != 0) { - virDomainFree(dom); - return false; - } + if (virDomainGetInfo(dom, &info) != 0) + goto cleanup; cpuinfo = vshMalloc(ctl, sizeof(virVcpuInfo)*info.nrVirtCpu); cpumaplen = VIR_CPU_MAPLEN(maxcpu); @@ -5608,10 +5604,13 @@ cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd) } } } else { - ret = false; + goto cleanup; } } + ret = true; + + cleanup: VIR_FREE(cpumaps); VIR_FREE(cpuinfo); virDomainFree(dom); -- 1.8.3.2
In subject: "virsh:" prefix is missing. On 06/05/14 16:39, Peter Krempa wrote:
On 06/05/14 13:25, Ján Tomko wrote:
Initialize 'ret' to false and introduce a cleanup label. --- tools/virsh-domain.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)
ACK,
Peter
This allows reuse of the result printing code. --- tools/virsh-domain.c | 61 +++++++++++++++++++++------------------------------- 1 file changed, 25 insertions(+), 36 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 4d21704..e85c906 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5558,9 +5558,21 @@ cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd) if ((ncpus = virDomainGetVcpus(dom, cpuinfo, info.nrVirtCpu, - cpumaps, cpumaplen)) >= 0) { - for (n = 0; n < ncpus; n++) { - vshPrint(ctl, "%-15s %d\n", _("VCPU:"), n); + cpumaps, cpumaplen)) < 0) { + if (info.state != VIR_DOMAIN_SHUTOFF) + goto cleanup; + + /* fall back to virDomainGetVcpuPinInfo and free cpuinfo to mark this */ + VIR_FREE(cpuinfo); + if ((ncpus = virDomainGetVcpuPinInfo(dom, info.nrVirtCpu, + cpumaps, cpumaplen, + VIR_DOMAIN_AFFECT_CONFIG)) < 0) + goto cleanup; + } + + for (n = 0; n < ncpus; n++) { + vshPrint(ctl, "%-15s %d\n", _("VCPU:"), n); + if (cpuinfo) { vshPrint(ctl, "%-15s %d\n", _("CPU:"), cpuinfo[n].cpu); vshPrint(ctl, "%-15s %s\n", _("State:"), vshDomainVcpuStateToString(cpuinfo[n].state)); @@ -5571,41 +5583,18 @@ cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, "%-15s %.1lfs\n", _("CPU time:"), cpuUsed); } - vshPrint(ctl, "%-15s ", _("CPU Affinity:")); - for (m = 0; m < maxcpu; m++) { - vshPrint(ctl, "%c", VIR_CPU_USABLE(cpumaps, cpumaplen, n, m) ? 'y' : '-'); - } - vshPrint(ctl, "\n"); - if (n < (ncpus - 1)) { - vshPrint(ctl, "\n"); - } - } - } else { - if (info.state == VIR_DOMAIN_SHUTOFF && - (ncpus = virDomainGetVcpuPinInfo(dom, info.nrVirtCpu, - cpumaps, cpumaplen, - VIR_DOMAIN_AFFECT_CONFIG)) >= 0) { - - /* fallback plan to use virDomainGetVcpuPinInfo */ - - for (n = 0; n < ncpus; n++) { - vshPrint(ctl, "%-15s %d\n", _("VCPU:"), n); - vshPrint(ctl, "%-15s %s\n", _("CPU:"), _("N/A")); - vshPrint(ctl, "%-15s %s\n", _("State:"), _("N/A")); - vshPrint(ctl, "%-15s %s\n", _("CPU time"), _("N/A")); - vshPrint(ctl, "%-15s ", _("CPU Affinity:")); - for (m = 0; m < maxcpu; m++) { - vshPrint(ctl, "%c", - VIR_CPU_USABLE(cpumaps, cpumaplen, n, m) ? 'y' : '-'); - } - vshPrint(ctl, "\n"); - if (n < (ncpus - 1)) { - vshPrint(ctl, "\n"); - } - } } else { - goto cleanup; + vshPrint(ctl, "%-15s %s\n", _("CPU:"), _("N/A")); + vshPrint(ctl, "%-15s %s\n", _("State:"), _("N/A")); + vshPrint(ctl, "%-15s %s\n", _("CPU time"), _("N/A")); } + vshPrint(ctl, "%-15s ", _("CPU Affinity:")); + for (m = 0; m < maxcpu; m++) { + vshPrint(ctl, "%c", VIR_CPU_USABLE(cpumaps, cpumaplen, n, m) ? 'y' : '-'); + } + vshPrint(ctl, "\n"); + if (n < (ncpus - 1)) + vshPrint(ctl, "\n"); } ret = true; -- 1.8.3.2
"virsh: " tag missing in subject On 06/05/14 13:25, Ján Tomko wrote:
This allows reuse of the result printing code. --- tools/virsh-domain.c | 61 +++++++++++++++++++++------------------------------- 1 file changed, 25 insertions(+), 36 deletions(-)
ACK, Peter
This simplifies the usage in {libxl,qemu}DomainGetNumaParameters and it's needed for consistent error reporting in virBitmapFormat. Also remove the forgotten ATTRIBUTE_NONNULL marker. --- src/libxl/libxl_driver.c | 3 +-- src/qemu/qemu_driver.c | 2 +- src/util/virbitmap.c | 6 +----- src/util/virbitmap.h | 3 +-- 4 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 9feacb1..c62c55e 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -4002,8 +4002,7 @@ libxlDomainGetNumaParameters(virDomainPtr dom, } } - nodeset = virBitmapFormat(nodes); - if (!nodeset && VIR_STRDUP(nodeset, "") < 0) + if (!(nodeset = virBitmapFormat(nodes))) goto cleanup; if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_NODESET, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3a7622a..578c223 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8914,7 +8914,7 @@ qemuDomainGetNumaParameters(virDomainPtr dom, case 1: /* fill numa nodeset here */ if (flags & VIR_DOMAIN_AFFECT_CONFIG) { nodeset = virBitmapFormat(persistentDef->numatune.memory.nodemask); - if (!nodeset && VIR_STRDUP(nodeset, "") < 0) + if (!nodeset) goto cleanup; } else { if (virCgroupGetCpusetMems(priv->cgroup, &nodeset) < 0) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index e44dce6..1dc74cb 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -227,11 +227,7 @@ char *virBitmapFormat(virBitmapPtr bitmap) bool first = true; int start, cur, prev; - if (!bitmap) - return NULL; - - cur = virBitmapNextSetBit(bitmap, -1); - if (cur < 0) { + if (!bitmap || (cur = virBitmapNextSetBit(bitmap, -1)) < 0) { char *ret; ignore_value(VIR_STRDUP(ret, "")); return ret; diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index b682523..6573aa2 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -69,8 +69,7 @@ int virBitmapGetBit(virBitmapPtr bitmap, size_t b, bool *result) char *virBitmapString(virBitmapPtr bitmap) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; -char *virBitmapFormat(virBitmapPtr bitmap) - ATTRIBUTE_NONNULL(1); +char *virBitmapFormat(virBitmapPtr bitmap); int virBitmapParse(const char *str, char sep, -- 1.8.3.2
On 06/05/14 13:25, Ján Tomko wrote:
This simplifies the usage in {libxl,qemu}DomainGetNumaParameters and it's needed for consistent error reporting in virBitmapFormat.
Also remove the forgotten ATTRIBUTE_NONNULL marker. --- src/libxl/libxl_driver.c | 3 +-- src/qemu/qemu_driver.c | 2 +- src/util/virbitmap.c | 6 +----- src/util/virbitmap.h | 3 +-- 4 files changed, 4 insertions(+), 10 deletions(-)
You should mention the new semantics when NULL is passed in the function comment. ACK with that added. Peter
It already reports an error if STRDUP fails. --- src/conf/capabilities.c | 4 +--- src/conf/domain_conf.c | 19 ++++--------------- src/conf/network_conf.c | 2 +- src/lxc/lxc_cgroup.c | 11 ++--------- src/qemu/qemu_cgroup.c | 16 +++------------- src/qemu/qemu_driver.c | 10 ++-------- src/util/virbitmap.c | 1 + 7 files changed, 14 insertions(+), 49 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 408fab3..954456b 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -791,10 +791,8 @@ virCapabilitiesFormatNUMATopology(virBufferPtr buf, virBufferAsprintf(buf, "<cpu id='%d'", cells[i]->cpus[j].id); if (cells[i]->cpus[j].siblings) { - if (!(siblings = virBitmapFormat(cells[i]->cpus[j].siblings))) { - virReportOOMError(); + if (!(siblings = virBitmapFormat(cells[i]->cpus[j].siblings))) return -1; - } virBufferAsprintf(buf, " socket_id='%d' core_id='%d' siblings='%s'", diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fe06921..ee59312 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17363,11 +17363,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAsprintf(buf, "<vcpupin vcpu='%u' ", def->cputune.vcpupin[i]->vcpuid); - if (!(cpumask = virBitmapFormat(def->cputune.vcpupin[i]->cpumask))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("failed to format cpuset for vcpupin")); + if (!(cpumask = virBitmapFormat(def->cputune.vcpupin[i]->cpumask))) goto error; - } virBufferAsprintf(buf, "cpuset='%s'/>\n", cpumask); VIR_FREE(cpumask); @@ -17377,11 +17374,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, char *cpumask; virBufferAddLit(buf, "<emulatorpin "); - if (!(cpumask = virBitmapFormat(def->cputune.emulatorpin->cpumask))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("failed to format cpuset for emulator")); - goto error; - } + if (!(cpumask = virBitmapFormat(def->cputune.emulatorpin->cpumask))) + goto error; virBufferAsprintf(buf, "cpuset='%s'/>\n", cpumask); VIR_FREE(cpumask); @@ -17407,13 +17401,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->numatune.memory.placement_mode == VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_STATIC) { - nodemask = virBitmapFormat(def->numatune.memory.nodemask); - if (nodemask == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to format nodeset for " - "NUMA memory tuning")); + if (!(nodemask = virBitmapFormat(def->numatune.memory.nodemask))) goto error; - } virBufferAsprintf(buf, "nodeset='%s'/>\n", nodemask); VIR_FREE(nodemask); } else if (def->numatune.memory.placement_mode) { diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 847bf11..f391056 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2851,7 +2851,7 @@ virNetworkObjFormat(virNetworkObjPtr net, size_t i; if (!class_id) - goto no_memory; + goto error; virBufferAddLit(&buf, "<networkstatus>\n"); virBufferAdjustIndent(&buf, 2); diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index c641132..8dfdc60 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -72,12 +72,8 @@ static int virLXCCgroupSetupCpusetTune(virDomainDefPtr def, if (def->placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO && def->cpumask) { - mask = virBitmapFormat(def->cpumask); - if (!mask) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to convert cpumask")); + if (!(mask = virBitmapFormat(def->cpumask))) return -1; - } if (virCgroupSetCpusetCpus(cgroup, mask) < 0) goto cleanup; @@ -93,11 +89,8 @@ static int virLXCCgroupSetupCpusetTune(virDomainDefPtr def, else mask = virBitmapFormat(def->numatune.memory.nodemask); - if (!mask) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to convert memory nodemask")); + if (!mask) return -1; - } if (virCgroupSetCpusetMems(cgroup, mask) < 0) goto cleanup; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index b1bfb5a..a31558f 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -599,11 +599,8 @@ qemuSetupCpusetCgroup(virDomainObjPtr vm, else mem_mask = virBitmapFormat(vm->def->numatune.memory.nodemask); - if (!mem_mask) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to convert memory nodemask")); + if (!mem_mask) goto cleanup; - } if (virCgroupSetCpusetMems(priv->cgroup, mem_mask) < 0) goto cleanup; @@ -622,11 +619,8 @@ qemuSetupCpusetCgroup(virDomainObjPtr vm, cpu_mask = virBitmapFormat(vm->def->cpumask); } - if (!cpu_mask) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to convert cpu mask")); + if (!cpu_mask) goto cleanup; - } if (virCgroupSetCpusetCpus(priv->cgroup, cpu_mask) < 0) goto cleanup; @@ -870,12 +864,8 @@ qemuSetupCgroupEmulatorPin(virCgroupPtr cgroup, int ret = -1; char *new_cpus = NULL; - new_cpus = virBitmapFormat(cpumask); - if (!new_cpus) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to convert cpu mask")); + if (!(new_cpus = virBitmapFormat(cpumask))) goto cleanup; - } if (virCgroupSetCpusetCpus(cgroup, new_cpus) < 0) goto cleanup; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 578c223..f9a243f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8679,22 +8679,16 @@ qemuDomainSetNumaParamsLive(virDomainObjPtr vm, } } - if (!(nodeset_str = virBitmapFormat(temp_nodeset))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed to format nodeset")); + if (!(nodeset_str = virBitmapFormat(temp_nodeset))) goto cleanup; - } if (virCgroupSetCpusetMems(priv->cgroup, nodeset_str) < 0) goto cleanup; VIR_FREE(nodeset_str); /* Ensure the cpuset string is formatted before passing to cgroup */ - if (!(nodeset_str = virBitmapFormat(nodeset))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed to format nodeset")); + if (!(nodeset_str = virBitmapFormat(nodeset))) goto cleanup; - } for (i = 0; i < priv->nvcpupids; i++) { if (virCgroupNewVcpu(priv->cgroup, i, false, &cgroup_temp) < 0 || diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 1dc74cb..81aaa80 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -259,6 +259,7 @@ char *virBitmapFormat(virBitmapPtr bitmap) if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); + virReportOOMError(); return NULL; } -- 1.8.3.2
On 06/05/14 13:25, Ján Tomko wrote:
It already reports an error if STRDUP fails. --- src/conf/capabilities.c | 4 +--- src/conf/domain_conf.c | 19 ++++--------------- src/conf/network_conf.c | 2 +- src/lxc/lxc_cgroup.c | 11 ++--------- src/qemu/qemu_cgroup.c | 16 +++------------- src/qemu/qemu_driver.c | 10 ++-------- src/util/virbitmap.c | 1 + 7 files changed, 14 insertions(+), 49 deletions(-)
ACK, nice cleanup. Peter
For converting bitmap data to human-readable strings. --- src/libvirt_private.syms | 1 + src/util/virbitmap.c | 25 +++++++++++++++++++++++++ src/util/virbitmap.h | 4 ++++ src/util/virstring.h | 1 - tests/virbitmaptest.c | 14 +++++++++++++- 5 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d73a9f5..f3dc39b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -970,6 +970,7 @@ virBitmapClearAll; virBitmapClearBit; virBitmapCopy; virBitmapCountBits; +virBitmapDataToString; virBitmapEqual; virBitmapFormat; virBitmapFree; diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 81aaa80..fb0df81 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -705,3 +705,28 @@ virBitmapCountBits(virBitmapPtr bitmap) return ret; } + +/** + * virBitmapDataToString: + * @data: the data + * @len: length of @data in bytes + * + * Convert a chunk of data containing bits information to a human + * readable string, e.g.: 0-1,4 + * + * Returns: a string representation of the data, or NULL on error + */ +char * +virBitmapDataToString(void *data, + int len) +{ + virBitmapPtr map = NULL; + char *ret = NULL; + + if (!(map = virBitmapNewData(data, len))) + return NULL; + + ret = virBitmapFormat(map); + virBitmapFree(map); + return ret; +} diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index 6573aa2..142a218 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -111,4 +111,8 @@ ssize_t virBitmapNextClearBit(virBitmapPtr bitmap, ssize_t pos) size_t virBitmapCountBits(virBitmapPtr bitmap) ATTRIBUTE_NONNULL(1); +char *virBitmapDataToString(void *data, + int len) + ATTRIBUTE_NONNULL(1); + #endif diff --git a/src/util/virstring.h b/src/util/virstring.h index 0ab9d96..6ddcff5 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -255,5 +255,4 @@ char *virStringReplace(const char *haystack, const char *newneedle) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); - #endif /* __VIR_STRING_H__ */ diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index cc34216..139b8e2 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -266,7 +266,7 @@ test4(const void *data ATTRIBUTE_UNUSED) return -1; } -/* test for virBitmapNewData/ToData */ +/* test for virBitmapNewData/ToData/DataToString */ static int test5(const void *v ATTRIBUTE_UNUSED) { @@ -278,6 +278,7 @@ test5(const void *v ATTRIBUTE_UNUSED) size_t i; ssize_t j; int ret = -1; + char *str = NULL; bitmap = virBitmapNewData(data, sizeof(data)); if (!bitmap) @@ -307,8 +308,19 @@ test5(const void *v ATTRIBUTE_UNUSED) data2[4] != 0x04) goto error; + if (!(str = virBitmapDataToString(data, sizeof(data)))) + goto error; + if (STRNEQ(str, "0,9,34")) + goto error; + VIR_FREE(str); + if (!(str = virBitmapDataToString(data2, sizeof(data2)))) + goto error; + if (STRNEQ(str, "0,2,9,15,34")) + goto error; + ret = 0; error: + VIR_FREE(str); virBitmapFree(bitmap); VIR_FREE(data2); return ret; -- 1.8.3.2
On 06/05/14 13:25, Ján Tomko wrote:
For converting bitmap data to human-readable strings. --- src/libvirt_private.syms | 1 + src/util/virbitmap.c | 25 +++++++++++++++++++++++++ src/util/virbitmap.h | 4 ++++ src/util/virstring.h | 1 - tests/virbitmaptest.c | 14 +++++++++++++- 5 files changed, 43 insertions(+), 2 deletions(-)
ACK, Peter
Report CPU affinities / online CPUs in human-readable form (0-2,4) instead of yy-y when this flag is present. https://bugzilla.redhat.com/show_bug.cgi?id=985980 --- tools/virsh-domain.c | 21 +++++++++++++++++++-- tools/virsh-host.c | 25 ++++++++++++++++++++++--- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index e85c906..d27886d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5528,6 +5528,10 @@ static const vshCmdOptDef opts_vcpuinfo[] = { .flags = VSH_OFLAG_REQ, .help = N_("domain name, id or uuid") }, + {.name = "pretty", + .type = VSH_OT_BOOL, + .help = N_("return human readable output") + }, {.name = NULL} }; @@ -5541,6 +5545,7 @@ cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd) int ncpus, maxcpu; size_t cpumaplen; bool ret = false; + bool pretty = vshCommandOptBool(cmd, "pretty"); int n, m; if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) @@ -5589,8 +5594,20 @@ cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, "%-15s %s\n", _("CPU time"), _("N/A")); } vshPrint(ctl, "%-15s ", _("CPU Affinity:")); - for (m = 0; m < maxcpu; m++) { - vshPrint(ctl, "%c", VIR_CPU_USABLE(cpumaps, cpumaplen, n, m) ? 'y' : '-'); + if (pretty) { + char *str; + + str = virBitmapDataToString(VIR_GET_CPUMAP(cpumaps, cpumaplen, n), + cpumaplen); + if (!str) + goto cleanup; + vshPrint(ctl, "%s", str); + VIR_FREE(str); + } else { + for (m = 0; m < maxcpu; m++) { + vshPrint(ctl, "%c", + VIR_CPU_USABLE(cpumaps, cpumaplen, n, m) ? 'y' : '-'); + } } vshPrint(ctl, "\n"); if (n < (ncpus - 1)) diff --git a/tools/virsh-host.c b/tools/virsh-host.c index cac6086..8091437 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -32,6 +32,7 @@ #include <libxml/xmlsave.h> #include "internal.h" +#include "virbitmap.h" #include "virbuffer.h" #include "viralloc.h" #include "virsh-domain.h" @@ -278,12 +279,21 @@ static const vshCmdInfo info_node_cpumap[] = { {.name = NULL} }; +static const vshCmdOptDef opts_node_cpumap[] = { + {.name = "pretty", + .type = VSH_OT_BOOL, + .help = N_("return human readable output") + }, + {.name = NULL} +}; + static bool cmdNodeCpuMap(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { int cpu, cpunum; unsigned char *cpumap = NULL; unsigned int online; + bool pretty = vshCommandOptBool(cmd, "pretty"); bool ret = false; cpunum = virNodeGetCPUMap(ctl->conn, &cpumap, &online, 0); @@ -296,8 +306,17 @@ cmdNodeCpuMap(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) vshPrint(ctl, "%-15s %d\n", _("CPUs online:"), online); vshPrint(ctl, "%-15s ", _("CPU map:")); - for (cpu = 0; cpu < cpunum; cpu++) - vshPrint(ctl, "%c", VIR_CPU_USED(cpumap, cpu) ? 'y' : '-'); + if (pretty) { + char *str = virBitmapDataToString(cpumap, cpunum); + + if (!str) + goto cleanup; + vshPrint(ctl, "%s", str); + VIR_FREE(str); + } else { + for (cpu = 0; cpu < cpunum; cpu++) + vshPrint(ctl, "%c", VIR_CPU_USED(cpumap, cpu) ? 'y' : '-'); + } vshPrint(ctl, "\n"); ret = true; @@ -978,7 +997,7 @@ const vshCmdDef hostAndHypervisorCmds[] = { }, {.name = "nodecpumap", .handler = cmdNodeCpuMap, - .opts = NULL, + .opts = opts_node_cpumap, .info = info_node_cpumap, .flags = 0 }, -- 1.8.3.2
On 06/05/14 13:25, Ján Tomko wrote:
Report CPU affinities / online CPUs in human-readable form (0-2,4) instead of yy-y when this flag is present.
https://bugzilla.redhat.com/show_bug.cgi?id=985980 --- tools/virsh-domain.c | 21 +++++++++++++++++++-- tools/virsh-host.c | 25 ++++++++++++++++++++++--- 2 files changed, 41 insertions(+), 5 deletions(-)
Missing documentation for the new flags in virsh.pod Otherwise looks good. Peter
On 06/05/2014 05:25 AM, Ján Tomko wrote:
Report CPU affinities / online CPUs in human-readable form (0-2,4) instead of yy-y when this flag is present.
Nit on the commit message: Wouldn't 0-2,4 map to yyy-y ? And it's unusual to pick 5 vcpus, 4 or 8 is more common... -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 06/05/2014 05:09 PM, Eric Blake wrote:
On 06/05/2014 05:25 AM, Ján Tomko wrote:
Report CPU affinities / online CPUs in human-readable form (0-2,4) instead of yy-y when this flag is present.
Nit on the commit message: Wouldn't 0-2,4 map to yyy-y ? And it's unusual to pick 5 vcpus, 4 or 8 is more common...
These are host CPUs, so it would probably be: 'yyy-y---' (if it has 8 CPUs). I have (over?)simplified the proposal from the bz: CPU Affinity: y: 0-3 -: 4-127 and only included the CPUs the vcpu is pinned to, which means the total CPU count can't be determined from the output. Jan
On 06/05/2014 09:32 AM, Ján Tomko wrote:
On 06/05/2014 05:09 PM, Eric Blake wrote:
On 06/05/2014 05:25 AM, Ján Tomko wrote:
Report CPU affinities / online CPUs in human-readable form (0-2,4) instead of yy-y when this flag is present.
Nit on the commit message: Wouldn't 0-2,4 map to yyy-y ? And it's unusual to pick 5 vcpus, 4 or 8 is more common...
These are host CPUs, so it would probably be: 'yyy-y---' (if it has 8 CPUs).
I have (over?)simplified the proposal from the bz: CPU Affinity: y: 0-3 -: 4-127
and only included the CPUs the vcpu is pinned to, which means the total CPU count can't be determined from the output.
Maybe: 0-2,4 (out of 8) so that we aren't losing the total CPU count? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Report CPU affinities / online CPUs in human-readable form when this flag is present: Before: CPU Affinity: y-yy After: CPU Affinity: 0,2-3 (out of 4) https://bugzilla.redhat.com/show_bug.cgi?id=985980 --- v2: man page additions, added total cpu count to vcpuinfo output fixed example in the commit message tools/virsh-domain.c | 21 +++++++++++++++++++-- tools/virsh-host.c | 25 ++++++++++++++++++++++--- tools/virsh.pod | 8 ++++++-- 3 files changed, 47 insertions(+), 7 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index e85c906..4f45ed1 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5528,6 +5528,10 @@ static const vshCmdOptDef opts_vcpuinfo[] = { .flags = VSH_OFLAG_REQ, .help = N_("domain name, id or uuid") }, + {.name = "pretty", + .type = VSH_OT_BOOL, + .help = N_("return human readable output") + }, {.name = NULL} }; @@ -5541,6 +5545,7 @@ cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd) int ncpus, maxcpu; size_t cpumaplen; bool ret = false; + bool pretty = vshCommandOptBool(cmd, "pretty"); int n, m; if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) @@ -5589,8 +5594,20 @@ cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, "%-15s %s\n", _("CPU time"), _("N/A")); } vshPrint(ctl, "%-15s ", _("CPU Affinity:")); - for (m = 0; m < maxcpu; m++) { - vshPrint(ctl, "%c", VIR_CPU_USABLE(cpumaps, cpumaplen, n, m) ? 'y' : '-'); + if (pretty) { + char *str; + + str = virBitmapDataToString(VIR_GET_CPUMAP(cpumaps, cpumaplen, n), + cpumaplen); + if (!str) + goto cleanup; + vshPrint(ctl, _("%s (out of %d)"), str, maxcpu); + VIR_FREE(str); + } else { + for (m = 0; m < maxcpu; m++) { + vshPrint(ctl, "%c", + VIR_CPU_USABLE(cpumaps, cpumaplen, n, m) ? 'y' : '-'); + } } vshPrint(ctl, "\n"); if (n < (ncpus - 1)) diff --git a/tools/virsh-host.c b/tools/virsh-host.c index cac6086..8091437 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -32,6 +32,7 @@ #include <libxml/xmlsave.h> #include "internal.h" +#include "virbitmap.h" #include "virbuffer.h" #include "viralloc.h" #include "virsh-domain.h" @@ -278,12 +279,21 @@ static const vshCmdInfo info_node_cpumap[] = { {.name = NULL} }; +static const vshCmdOptDef opts_node_cpumap[] = { + {.name = "pretty", + .type = VSH_OT_BOOL, + .help = N_("return human readable output") + }, + {.name = NULL} +}; + static bool cmdNodeCpuMap(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { int cpu, cpunum; unsigned char *cpumap = NULL; unsigned int online; + bool pretty = vshCommandOptBool(cmd, "pretty"); bool ret = false; cpunum = virNodeGetCPUMap(ctl->conn, &cpumap, &online, 0); @@ -296,8 +306,17 @@ cmdNodeCpuMap(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) vshPrint(ctl, "%-15s %d\n", _("CPUs online:"), online); vshPrint(ctl, "%-15s ", _("CPU map:")); - for (cpu = 0; cpu < cpunum; cpu++) - vshPrint(ctl, "%c", VIR_CPU_USED(cpumap, cpu) ? 'y' : '-'); + if (pretty) { + char *str = virBitmapDataToString(cpumap, cpunum); + + if (!str) + goto cleanup; + vshPrint(ctl, "%s", str); + VIR_FREE(str); + } else { + for (cpu = 0; cpu < cpunum; cpu++) + vshPrint(ctl, "%c", VIR_CPU_USED(cpumap, cpu) ? 'y' : '-'); + } vshPrint(ctl, "\n"); ret = true; @@ -978,7 +997,7 @@ const vshCmdDef hostAndHypervisorCmds[] = { }, {.name = "nodecpumap", .handler = cmdNodeCpuMap, - .opts = NULL, + .opts = opts_node_cpumap, .info = info_node_cpumap, .flags = 0 }, diff --git a/tools/virsh.pod b/tools/virsh.pod index 02671b4..80501f9 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -290,11 +290,13 @@ and size of the physical memory. The output corresponds to virNodeInfo structure. Specifically, the "CPU socket(s)" field means number of CPU sockets per NUMA cell. -=item B<nodecpumap> +=item B<nodecpumap> [I<--pretty>] Displays the node's total number of CPUs, the number of online CPUs and the list of online CPUs. +With I<--pretty> the online CPUs are printed as a range instead of a list. + =item B<nodecpustats> [I<cpu>] [I<--percent>] Returns cpu stats of the node. @@ -1954,11 +1956,13 @@ If I<--guest> is specified, then the count of cpus is reported from the perspective of the guest. This flag is usable only for live domains and may require guest agent to be configured in the guest. -=item B<vcpuinfo> I<domain> +=item B<vcpuinfo> I<domain> [I<--pretty>] Returns basic information about the domain virtual CPUs, like the number of vCPUs, the running time, the affinity to physical processors. +With I<--pretty>, cpu affinities are shown as ranges. + =item B<vcpupin> I<domain> [I<vcpu>] [I<cpulist>] [[I<--live>] [I<--config>] | [I<--current>]] -- 1.8.3.2
On 06/05/2014 11:28 AM, Ján Tomko wrote:
Report CPU affinities / online CPUs in human-readable form when this flag is present:
Before: CPU Affinity: y-yy
After: CPU Affinity: 0,2-3 (out of 4)
https://bugzilla.redhat.com/show_bug.cgi?id=985980 --- v2: man page additions, added total cpu count to vcpuinfo output fixed example in the commit message
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 06/06/2014 02:27 PM, Eric Blake wrote:
On 06/05/2014 11:28 AM, Ján Tomko wrote:
Report CPU affinities / online CPUs in human-readable form when this flag is present:
Before: CPU Affinity: y-yy
After: CPU Affinity: 0,2-3 (out of 4)
https://bugzilla.redhat.com/show_bug.cgi?id=985980 --- v2: man page additions, added total cpu count to vcpuinfo output fixed example in the commit message
ACK.
I fixed the nits pointed out by Peter and pushed the series. Thank you for the reviews! Jan
participants (3)
-
Eric Blake -
Ján Tomko -
Peter Krempa