11:08 a.m.
See patch 6/6.
Peter Krempa (6):
util: bitmap: Make bitmaps const in virBitmapNewData and
virBitmapDataToString
virsh: domain: Fix broken indentation in virshCPUCountCollect
virsh: Fix xpath queries for retrieving vcpu count
virsh: Extract cpumap formatting in cmdVcpuinfo
virsh: Extract fallback handling in cmdVcpuinfo
virsh: vcpuinfo: Report proper vcpu numbers and data for offline VMs
src/util/virbitmap.c | 6 +-
src/util/virbitmap.h | 4 +-
tools/virsh-domain.c | 287 ++++++++++++++++++++++++++++++++++++++-------------
3 files changed, 219 insertions(+), 78 deletions(-)
--
2.10.0
11:08 a.m.
New subject: [libvirt] [PATCH 1/6] util: bitmap: Make bitmaps const in virBitmapNewData and virBitmapDataToString
The functions just read the passed pointer so it can be marked as const.
---
src/util/virbitmap.c | 6 +++---
src/util/virbitmap.h | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index 3b85c16..0c04f1a 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -689,12 +689,12 @@ virBitmapPtr virBitmapNewCopy(virBitmapPtr src)
* Returns a pointer to the allocated bitmap or NULL if
* memory cannot be allocated.
*/
-virBitmapPtr virBitmapNewData(void *data, int len)
+virBitmapPtr virBitmapNewData(const void *data, int len)
{
virBitmapPtr bitmap;
size_t i, j;
unsigned long *p;
- unsigned char *bytes = data;
+ const unsigned char *bytes = data;
bitmap = virBitmapNew(len * CHAR_BIT);
if (!bitmap)
@@ -1058,7 +1058,7 @@ virBitmapCountBits(virBitmapPtr bitmap)
* Returns: a string representation of the data, or NULL on error
*/
char *
-virBitmapDataToString(void *data,
+virBitmapDataToString(const void *data,
int len)
{
virBitmapPtr map = NULL;
diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h
index 5984b80..58e0ee6 100644
--- a/src/util/virbitmap.h
+++ b/src/util/virbitmap.h
@@ -101,7 +101,7 @@ virBitmapParseUnlimited(const char *str,
virBitmapPtr virBitmapNewCopy(virBitmapPtr src) ATTRIBUTE_NONNULL(1);
-virBitmapPtr virBitmapNewData(void *data, int len) ATTRIBUTE_NONNULL(1);
+virBitmapPtr virBitmapNewData(const void *data, int len) ATTRIBUTE_NONNULL(1);
int virBitmapToData(virBitmapPtr bitmap, unsigned char **data, int *dataLen)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
@@ -138,7 +138,7 @@ ssize_t virBitmapNextClearBit(virBitmapPtr bitmap, ssize_t pos)
size_t virBitmapCountBits(virBitmapPtr bitmap)
ATTRIBUTE_NONNULL(1);
-char *virBitmapDataToString(void *data,
+char *virBitmapDataToString(const void *data,
int len)
ATTRIBUTE_NONNULL(1);
bool virBitmapOverlaps(virBitmapPtr b1,
--
2.10.0
11:08 a.m.
New subject: [libvirt] [PATCH 2/6] virsh: domain: Fix broken indentation in virshCPUCountCollect
I managed to space most of the code by 5 spaces instead of 4 when
orignally implementing this function.
---
tools/virsh-domain.c | 68 ++++++++++++++++++++++++++--------------------------
1 file changed, 34 insertions(+), 34 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b19f499..03bf032 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6125,50 +6125,50 @@ virshCPUCountCollect(vshControl *ctl,
return count;
/* fallback code */
- if (!(last_error->code == VIR_ERR_NO_SUPPORT ||
- last_error->code == VIR_ERR_INVALID_ARG))
- goto cleanup;
+ if (!(last_error->code == VIR_ERR_NO_SUPPORT ||
+ last_error->code == VIR_ERR_INVALID_ARG))
+ goto cleanup;
- if (flags & VIR_DOMAIN_VCPU_GUEST) {
- vshError(ctl, "%s", _("Failed to retrieve vCPU count from the
guest"));
- goto cleanup;
- }
+ if (flags & VIR_DOMAIN_VCPU_GUEST) {
+ vshError(ctl, "%s", _("Failed to retrieve vCPU count from the
guest"));
+ goto cleanup;
+ }
- if (!(flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG)) &&
- virDomainIsActive(dom) == 1)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
+ if (!(flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG)) &&
+ virDomainIsActive(dom) == 1)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
- vshResetLibvirtError();
+ vshResetLibvirtError();
- if (flags & VIR_DOMAIN_AFFECT_LIVE) {
- if (flags & VIR_DOMAIN_VCPU_MAXIMUM) {
+ if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ if (flags & VIR_DOMAIN_VCPU_MAXIMUM) {
count = virDomainGetMaxVcpus(dom);
- } else {
- if (virDomainGetInfo(dom, &info) < 0)
- goto cleanup;
+ } else {
+ if (virDomainGetInfo(dom, &info) < 0)
+ goto cleanup;
- count = info.nrVirtCpu;
- }
- } else {
- if (!(def = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE)))
- goto cleanup;
+ count = info.nrVirtCpu;
+ }
+ } else {
+ if (!(def = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE)))
+ goto cleanup;
if (!(xml = virXMLParseStringCtxt(def, _("(domain_definition)"),
&ctxt)))
goto cleanup;
- if (flags & VIR_DOMAIN_VCPU_MAXIMUM) {
- if (virXPathInt("string(/domain/vcpus)", ctxt, &count) < 0)
{
- vshError(ctl, "%s", _("Failed to retrieve maximum vcpu
count"));
- goto cleanup;
- }
- } else {
- if (virXPathInt("string(/domain/vcpus/@current)",
- ctxt, &count) < 0) {
- vshError(ctl, "%s", _("Failed to retrieve current vcpu
count"));
- goto cleanup;
- }
- }
- }
+ if (flags & VIR_DOMAIN_VCPU_MAXIMUM) {
+ if (virXPathInt("string(/domain/vcpus)", ctxt, &count) < 0)
{
+ vshError(ctl, "%s", _("Failed to retrieve maximum vcpu
count"));
+ goto cleanup;
+ }
+ } else {
+ if (virXPathInt("string(/domain/vcpus/@current)",
+ ctxt, &count) < 0) {
+ vshError(ctl, "%s", _("Failed to retrieve current vcpu
count"));
+ goto cleanup;
+ }
+ }
+ }
ret = count;
cleanup:
--
2.10.0
11:08 a.m.
New subject: [libvirt] [PATCH 3/6] virsh: Fix xpath queries for retrieving vcpu count
The fallback code used if virDomainGetVcpusFlags is not supported used
wrong XPath queries and basically did not work at all. Fix them to point
to the <domain> <vcpu> element instead of <vcpus> which was not present
until lately.
---
tools/virsh-domain.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 03bf032..5fdad1b 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6157,13 +6157,12 @@ virshCPUCountCollect(vshControl *ctl,
goto cleanup;
if (flags & VIR_DOMAIN_VCPU_MAXIMUM) {
- if (virXPathInt("string(/domain/vcpus)", ctxt, &count) < 0)
{
+ if (virXPathInt("string(/domain/vcpu)", ctxt, &count) < 0)
{
vshError(ctl, "%s", _("Failed to retrieve maximum vcpu
count"));
goto cleanup;
}
} else {
- if (virXPathInt("string(/domain/vcpus/@current)",
- ctxt, &count) < 0) {
+ if (virXPathInt("string(/domain/vcpu/@current)", ctxt, &count)
< 0) {
vshError(ctl, "%s", _("Failed to retrieve current vcpu
count"));
goto cleanup;
}
--
2.10.0
11:08 a.m.
New subject: [libvirt] [PATCH 4/6] virsh: Extract cpumap formatting in cmdVcpuinfo
cmdVcpuinfo will be split in upcomming patches thus extract the common
code that formats pinning cpumaps for the vcpus.
---
tools/virsh-domain.c | 52 +++++++++++++++++++++++++++++++++++-----------------
1 file changed, 35 insertions(+), 17 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 5fdad1b..8e1b9ed 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6280,6 +6280,36 @@ static const vshCmdOptDef opts_vcpuinfo[] = {
{.name = NULL}
};
+
+static int
+virshVcpuinfoPrintAffinity(vshControl *ctl,
+ const unsigned char *cpumap,
+ int maxcpu,
+ bool pretty)
+{
+ char *str = NULL;
+ size_t i;
+ int ret = -1;
+
+ vshPrint(ctl, "%-15s ", _("CPU Affinity:"));
+ if (pretty) {
+ if (!(str = virBitmapDataToString(cpumap, VIR_CPU_MAPLEN(maxcpu))))
+ goto cleanup;
+ vshPrint(ctl, _("%s (out of %d)"), str, maxcpu);
+ } else {
+ for (i = 0; i < maxcpu; i++)
+ vshPrint(ctl, "%c", VIR_CPU_USED(cpumap, i) ? 'y' :
'-');
+ }
+ vshPrint(ctl, "\n");
+
+ ret = 0;
+
+ cleanup:
+ VIR_FREE(str);
+ return ret;
+}
+
+
static bool
cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd)
{
@@ -6291,7 +6321,7 @@ cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd)
size_t cpumaplen;
bool ret = false;
bool pretty = vshCommandOptBool(cmd, "pretty");
- int n, m;
+ int n;
virshControlPtr priv = ctl->privData;
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
@@ -6340,23 +6370,11 @@ cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, "%-15s %s\n", _("State:"),
_("N/A"));
vshPrint(ctl, "%-15s %s\n", _("CPU time"),
_("N/A"));
}
- vshPrint(ctl, "%-15s ", _("CPU Affinity:"));
- 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 (virshVcpuinfoPrintAffinity(ctl, VIR_GET_CPUMAP(cpumaps, cpumaplen, n),
+ maxcpu, pretty) < 0)
+ goto cleanup;
+
if (n < (ncpus - 1))
vshPrint(ctl, "\n");
}
--
2.10.0
11:08 a.m.
New subject: [libvirt] [PATCH 5/6] virsh: Extract fallback handling in cmdVcpuinfo
Put it into a separate function so that more fallback handling can be
added without making a mess.
---
tools/virsh-domain.c | 75 +++++++++++++++++++++++++++++++++++++---------------
1 file changed, 54 insertions(+), 21 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 8e1b9ed..84a4854 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6311,6 +6311,49 @@ virshVcpuinfoPrintAffinity(vshControl *ctl,
static bool
+virshVcpuinfoInactive(vshControl *ctl,
+ virDomainPtr dom,
+ int nvcpus,
+ int maxcpu,
+ bool pretty)
+{
+ unsigned char *cpumaps = NULL;
+ size_t cpumaplen;
+ int ncpus;
+ size_t i;
+ bool ret = false;
+
+ cpumaplen = VIR_CPU_MAPLEN(maxcpu);
+ cpumaps = vshMalloc(ctl, nvcpus * cpumaplen);
+
+ if ((ncpus = virDomainGetVcpuPinInfo(dom, nvcpus,
+ cpumaps, cpumaplen,
+ VIR_DOMAIN_AFFECT_CONFIG)) < 0)
+ goto cleanup;
+
+ for (i = 0; i < ncpus; i++) {
+ if (i != 0)
+ vshPrint(ctl, "\n");
+
+ vshPrint(ctl, "%-15s %zu\n", _("VCPU:"), i);
+ 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"));
+
+ if (virshVcpuinfoPrintAffinity(ctl, VIR_GET_CPUMAP(cpumaps, cpumaplen, i),
+ maxcpu, pretty) < 0)
+ goto cleanup;
+ }
+
+ ret = true;
+
+ cleanup:
+ VIR_FREE(cpumaps);
+ return ret;
+}
+
+
+static bool
cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd)
{
virDomainInfo info;
@@ -6343,32 +6386,22 @@ cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd)
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 offline VMs we can return pinning information */
+ ret = virshVcpuinfoInactive(ctl, dom, info.nrVirtCpu, maxcpu, pretty);
+ goto cleanup;
}
for (n = 0; n < ncpus; n++) {
- if (cpuinfo) {
- vshPrint(ctl, "%-15s %d\n", _("VCPU:"),
cpuinfo[n].number);
- vshPrint(ctl, "%-15s %d\n", _("CPU:"), cpuinfo[n].cpu);
- vshPrint(ctl, "%-15s %s\n", _("State:"),
- virshDomainVcpuStateToString(cpuinfo[n].state));
- if (cpuinfo[n].cpuTime != 0) {
- double cpuUsed = cpuinfo[n].cpuTime;
+ vshPrint(ctl, "%-15s %d\n", _("VCPU:"), cpuinfo[n].number);
+ vshPrint(ctl, "%-15s %d\n", _("CPU:"), cpuinfo[n].cpu);
+ vshPrint(ctl, "%-15s %s\n", _("State:"),
+ virshDomainVcpuStateToString(cpuinfo[n].state));
+ if (cpuinfo[n].cpuTime != 0) {
+ double cpuUsed = cpuinfo[n].cpuTime;
- cpuUsed /= 1000000000.0;
+ cpuUsed /= 1000000000.0;
- vshPrint(ctl, "%-15s %.1lfs\n", _("CPU time:"),
cpuUsed);
- }
- } else {
- 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 %.1lfs\n", _("CPU time:"),
cpuUsed);
}
if (virshVcpuinfoPrintAffinity(ctl, VIR_GET_CPUMAP(cpumaps, cpumaplen, n),
--
2.10.0
11:08 a.m.
New subject: [libvirt] [PATCH 6/6] virsh: vcpuinfo: Report proper vcpu numbers and data for offline VMs
If the VM is offline virsh attempted to at least report the pinning
information for the VM. This would not work properly now that the vcpus
can be sparse. Fix it by getting the vcpu states from the XML.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1375920
---
tools/virsh-domain.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 100 insertions(+), 9 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 84a4854..050e7fb 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6310,37 +6310,125 @@ virshVcpuinfoPrintAffinity(vshControl *ctl,
}
+static virBitmapPtr
+virshDomainGetVcpuBitmap(vshControl *ctl,
+ virDomainPtr dom,
+ bool inactive)
+{
+ unsigned int flags = 0;
+ char *def = NULL;
+ virBitmapPtr ret = NULL;
+ xmlDocPtr xml = NULL;
+ xmlXPathContextPtr ctxt = NULL;
+ xmlNodePtr *nodes = NULL;
+ xmlNodePtr old;
+ int nnodes;
+ size_t i;
+ unsigned int curvcpus = 0;
+ unsigned int maxvcpus = 0;
+ unsigned int vcpuid;
+ char *online = NULL;
+
+ if (inactive)
+ flags |= VIR_DOMAIN_XML_INACTIVE;
+
+ if (!(def = virDomainGetXMLDesc(dom, flags)))
+ goto cleanup;
+
+ if (!(xml = virXMLParseStringCtxt(def, _("(domain_definition)"),
&ctxt)))
+ goto cleanup;
+
+ if (virXPathUInt("string(/domain/vcpu)", ctxt, &maxvcpus) < 0) {
+ vshError(ctl, "%s", _("Failed to retrieve maximum vcpu
count"));
+ goto cleanup;
+ }
+
+ ignore_value(virXPathUInt("string(/domain/vcpu/@current)", ctxt,
&curvcpus));
+
+ if (curvcpus == 0)
+ curvcpus = maxvcpus;
+
+ if (!(ret = virBitmapNew(maxvcpus)))
+ goto cleanup;
+
+ if ((nnodes = virXPathNodeSet("/domain/vcpus/vcpu", ctxt, &nodes))
<= 0) {
+ /* if the specific vcpu state is missing provide a fallback */
+ for (i = 0; i < curvcpus; i++)
+ ignore_value(virBitmapSetBit(ret, i));
+
+ goto cleanup;
+ }
+
+ old = ctxt->node;
+
+ for (i = 0; i < nnodes; i++) {
+ ctxt->node = nodes[i];
+
+ if (virXPathUInt("string(@id)", ctxt, &vcpuid) < 0 ||
+ !(online = virXPathString("string(@enabled)", ctxt)))
+ continue;
+
+ if (STREQ(online, "yes"))
+ ignore_value(virBitmapSetBit(ret, vcpuid));
+
+ VIR_FREE(online);
+ }
+
+ ctxt->node = old;
+
+ if (virBitmapCountBits(ret) != curvcpus) {
+ vshError(ctl, "%s", _("Failed to retrieve vcpu state
bitmap"));
+ virBitmapFree(ret);
+ ret = NULL;
+ }
+
+ cleanup:
+ VIR_FREE(online);
+ VIR_FREE(nodes);
+ xmlXPathFreeContext(ctxt);
+ xmlFreeDoc(xml);
+ VIR_FREE(def);
+ return ret;
+}
+
+
static bool
virshVcpuinfoInactive(vshControl *ctl,
virDomainPtr dom,
- int nvcpus,
int maxcpu,
bool pretty)
{
unsigned char *cpumaps = NULL;
size_t cpumaplen;
int ncpus;
- size_t i;
+ virBitmapPtr vcpus = NULL;
+ ssize_t nextvcpu = -1;
bool ret = false;
+ bool first = true;
+
+ if (!(vcpus = virshDomainGetVcpuBitmap(ctl, dom, true)))
+ goto cleanup;
cpumaplen = VIR_CPU_MAPLEN(maxcpu);
- cpumaps = vshMalloc(ctl, nvcpus * cpumaplen);
+ cpumaps = vshMalloc(ctl, virBitmapSize(vcpus) * cpumaplen);
- if ((ncpus = virDomainGetVcpuPinInfo(dom, nvcpus,
+ if ((ncpus = virDomainGetVcpuPinInfo(dom, virBitmapSize(vcpus),
cpumaps, cpumaplen,
VIR_DOMAIN_AFFECT_CONFIG)) < 0)
goto cleanup;
- for (i = 0; i < ncpus; i++) {
- if (i != 0)
+ while ((nextvcpu = virBitmapNextSetBit(vcpus, nextvcpu)) >= 0) {
+ if (!first)
vshPrint(ctl, "\n");
+ first = false;
- vshPrint(ctl, "%-15s %zu\n", _("VCPU:"), i);
+ vshPrint(ctl, "%-15s %zd\n", _("VCPU:"), nextvcpu);
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"));
- if (virshVcpuinfoPrintAffinity(ctl, VIR_GET_CPUMAP(cpumaps, cpumaplen, i),
+ if (virshVcpuinfoPrintAffinity(ctl,
+ VIR_GET_CPUMAP(cpumaps, cpumaplen, nextvcpu),
maxcpu, pretty) < 0)
goto cleanup;
}
@@ -6348,6 +6436,7 @@ virshVcpuinfoInactive(vshControl *ctl,
ret = true;
cleanup:
+ virBitmapFree(vcpus);
VIR_FREE(cpumaps);
return ret;
}
@@ -6386,8 +6475,10 @@ cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd)
if (info.state != VIR_DOMAIN_SHUTOFF)
goto cleanup;
+ vshResetLibvirtError();
+
/* for offline VMs we can return pinning information */
- ret = virshVcpuinfoInactive(ctl, dom, info.nrVirtCpu, maxcpu, pretty);
+ ret = virshVcpuinfoInactive(ctl, dom, maxcpu, pretty);
goto cleanup;
}
--
2.10.0
10:20 a.m.
On 12.10.2016 00:08, Peter Krempa wrote:
See patch 6/6.
Peter Krempa (6):
util: bitmap: Make bitmaps const in virBitmapNewData and
virBitmapDataToString
virsh: domain: Fix broken indentation in virshCPUCountCollect
virsh: Fix xpath queries for retrieving vcpu count
virsh: Extract cpumap formatting in cmdVcpuinfo
virsh: Extract fallback handling in cmdVcpuinfo
virsh: vcpuinfo: Report proper vcpu numbers and data for offline VMs
src/util/virbitmap.c | 6 +-
src/util/virbitmap.h | 4 +-
tools/virsh-domain.c | 287 ++++++++++++++++++++++++++++++++++++++-------------
3 files changed, 219 insertions(+), 78 deletions(-)
ACK series.
Michal
2966
days inactive
2967
days old
7 comments
2 participants
participants (2)
-
Michal Privoznik -
Peter Krempa