6:25 a.m.
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
6:25 a.m.
New subject: [libvirt] [PATCH 1/6] Invert logic in cmdVcpuinfo
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
9:42 a.m.
New subject: [libvirt] [PATCH 1/6] Invert logic in cmdVcpuinfo
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
6:25 a.m.
New subject: [libvirt] [PATCH 2/6] Separate API calls and result printing in cmdVcpuinfo
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
9:48 a.m.
New subject: [libvirt] [PATCH 2/6] Separate API calls and result printing in cmdVcpuinfo
"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
6:25 a.m.
New subject: [libvirt] [PATCH 3/6] Format NULL bitmap as an empty string
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
10:11 a.m.
New subject: [libvirt] [PATCH 3/6] Format NULL bitmap as an empty string
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
6:25 a.m.
New subject: [libvirt] [PATCH 4/6] Always report an error if virBitmapFormat fails
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
10:09 a.m.
New subject: [libvirt] [PATCH 4/6] Always report an error if virBitmapFormat fails
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
6:25 a.m.
New subject: [libvirt] [PATCH 5/6] Introudce virBitmapDataToString
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
10:01 a.m.
New subject: [libvirt] [PATCH 5/6] Introudce virBitmapDataToString
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
6:25 a.m.
New subject: [libvirt] [PATCH 6/6] Implement pretty flag for vcpuinfo and nodecpumap
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
10:05 a.m.
New subject: [libvirt] [PATCH 6/6] Implement pretty flag for vcpuinfo and nodecpumap
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
10:09 a.m.
New subject: [libvirt] [PATCH 6/6] Implement pretty flag for vcpuinfo and nodecpumap
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
10:32 a.m.
New subject: [libvirt] [PATCH 6/6] Implement pretty flag for vcpuinfo and nodecpumap
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
10:35 a.m.
New subject: [libvirt] [PATCH 6/6] Implement pretty flag for vcpuinfo and nodecpumap
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
12:28 p.m.
New subject: [libvirt] [PATCH v2] Implement pretty flag for vcpuinfo and nodecpumap
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
7:27 a.m.
New subject: [libvirt] [PATCH v2] Implement pretty flag for vcpuinfo and nodecpumap
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
7:41 a.m.
New subject: [libvirt] [PATCH v2] Implement pretty flag for vcpuinfo and nodecpumap
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
3822
days inactive
3823
days old
19 comments
3 participants
participants (3)
-
Eric Blake -
Ján Tomko -
Peter Krempa