Re: [libvirt] [PATCH 1/3 V7] add nodeGetCPUmap() for getting available CPU IDs in a cpumap.
On Tue, Mar 06, 2012 at 10:02:15PM -0700, Eric Blake wrote: On 03/06/2012 05:10 AM, Richard W.M. Jones wrote: This all appears to work. I built my own libvirt with the three remaining patches and was able to read pCPU information for a running domain. The libvirt side is now pushed. Rich: The documentation in libvirt.c correctly says that calling virDomainGetCPUStats(dom, NULL, 0, 0, 0, 0) must tell you part of the information needed to compute the size of an array to allocate; but in v7, the return was off by one too small. But it was generally masked by the v7 virsh code overallocating to a fixed 128 slots rather than paying attention to the return value. I fixed both those problems before pushing the libvirt patches, but since you tested with v7 instead of my fixed version, you may need to double check that virt-top isn't making the same mistakes, as it might be a boundary case that only strikes on machines with 128 physical CPUs. TBH I found the documentation for virDomainGetCPUStats to be very confusing indeed. I couldn't really tell if virt-top is calling the API correctly or not, so I simply used Fujitsu's code directly. Do you have any comments on whether this is correct or not? http://git.annexia.org/?p=ocaml-libvirt.git;a=blob;f=libvirt/libvirt_c_oneoffs.c;h=f827707a77e6478129370fce67e46ae745b9be9a;hb=HEAD#l534 Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones New in Fedora 11: Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 70 libraries supprt'd http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3 V7] add nodeGetCPUmap() for getting available CPU IDs in a cpumap.
On 03/07/2012 03:36 AM, Richard W.M. Jones wrote:
TBH I found the documentation for virDomainGetCPUStats to be very
confusing indeed. I couldn't really tell if virt-top is calling the
API correctly or not, so I simply used Fujitsu's code directly.
That's a shame about the documentation not being clear; anything we can
do to improve it?
There are basically 5 calling modes:
* Typical use sequence is below.
*
* getting total stats: set start_cpu as -1, ncpus 1
* virDomainGetCPUStats(dom, NULL, 0, -1, 1, 0) = nparams
* params = calloc(nparams, sizeof (virTypedParameter))
* virDomainGetCPUStats(dom, params, nparams, -1, 1, 0) = total stats.
*
* getting per-cpu stats:
* virDomainGetCPUStats(dom, NULL, 0, 0, 0, 0) = ncpus
* virDomainGetCPUStats(dom, NULL, 0, 0, 1, 0) = nparams
* params = calloc(ncpus * nparams, sizeof (virTypedParameter))
* virDomainGetCPUStats(dom, params, nparams, 0, ncpus, 0) = per-cpu stats
*
3 of the calling modes (when params is NULL) are there to let you
determine how large to size your arrays; the remaining two modes exist
to query the total stats, and to query a slice of up to 128 per-cpu stats.
The number of total stats can be different from the number of per-cpu
stats (right now, it's 1 each for qemu, but I have a pending patch that
I will post later today that adds two new total stats with no per-cpu
counterpart).
When querying total stats, start_cpu is -1 and ncpus is 1 (this is a
hard-coded requirement), so the return value is the number of stats
populated.
When querying per-cpu stats, the single 'params' pointer is actually
representing a 2D array. So if you allocate params with 3x4 slots, and
call virDomainGetCPUStats(dom, params, 3, 0, 4, 0), but there are only 3
online CPUs and the result of the call is 2, then the result will be
laid out as:
params[0] = CPU0 stat 0
params[1] = CPU0 stat 1
params[2] = NULL
params[3] = CPU1 stat 0
params[4] = CPU1 stat 1
params[5] = NULL
params[6] = CPU2 stat 0
params[7] = CPU2 stat 1
params[8] = NULL
params[9] = NULL
params[10] = NULL
params[11] = NULL
Furthermore, if you have a beefy system with more than 128 cpus, you
have to break the call into chunks of 128 at a time, using start_cpu at
0, 128, and so forth.
Do you have any comments on whether this is correct or not?
http://git.annexia.org/?p=ocaml-libvirt.git;a=blob;f=libvirt/libvirt_c_oneoffs.c;h=f827707a77e6478129370fce67e46ae745b9be9a;hb=HEAD#l534
546
547 /* get percpu information */
548 NONBLOCKING (nparams = virDomainGetCPUStats(dom, NULL, 0, -1, 1, 0));
This gets the number of total params, but this might be different than
the number of per-cpu params. I think you want this to use
virDomainGetCPUStats(dom, NULL, 0, 0, 1, 0)
or even the maximum of the two, if you plan on combining total and
per-cpu usage into the same call.
549 CHECK_ERROR (nparams 0, conn, virDomainGetCPUStats);
550
551 if ((params = malloc(sizeof(*params) * nparams * 128)) == NULL)
Here, you are blindly sizing params based on the maximum supported by
the API. It might be more efficient to s/128/nr_pcpus/ or even MIN(128,
nr_pcpus). It would also be possible to use virDomainGetCPUStats(dom,
NULL, 0, 0, 0, 0), which should be the same or less than nr_pcpus, if
I'm correctly understanding how nr_pcpus was computed.
552 caml_failwith (virDomainGetCPUStats: malloc);
553
554 cpustats = caml_alloc (nr_pcpus, 0); /* cpustats: array of params(list
of typed_param) */
555 cpu = 0;
556 while (cpu nr_pcpus) {
557 ncpus = nr_pcpus - cpu 128 ? 128 : nr_pcpus - cpu;
Good, this looks like the correct way to divide things into slices of
128 cpus per read.
558
559 NONBLOCKING (r = virDomainGetCPUStats(dom, params, nparams, cpu,
ncpus, 0));
Here, nparams should be at least as large as the value from
virDomainGetCPUStats(dom, NULL, 0, 0, 1, 0), since there might be more
per-cpu stats than there are total stats. If nparams is too small, you
will be silently losing out on those extra stats.
560 CHECK_ERROR (r 0, conn, virDomainGetCPUStats);
561
562 for (i = 0; i ncpus; i++) {
563 /* list of typed_param: single linked list of param_nodes */
564 param_head = Val_emptylist; /* param_head: the head param_node of
list of typed_param */
565
566 if (params[i * nparams].type == 0) {
567 Store_field(cpustats, cpu + i, param_head);
568 continue;
569 }
570
571 for (j = nparams - 1; j = 0; j--) {
Here, I'd start the iteration on j = r - 1, since we already know r =
nparams, and that any slots beyond r are NULL.
572 pos = i * nparams + j;
573 if (params[pos].type == 0)
574 continue;
575
576 param_node = caml_alloc(2, 0); /* param_node: typed_param, next
param_node */
577 Store_field(param_node, 1, param_head);
578 param_head = param_node;
579
580 typed_param =
Re: [libvirt] [PATCH 1/3 V7] add nodeGetCPUmap() for getting available CPU IDs in a cpumap.
On Fri, Mar 02, 2012 at 10:54:22AM +0800, Lai Jiangshan wrote:
+/*
+ * Linux maintains cpu bit map. For example, if cpuid=5's flag is not set
+ * and max cpu is 7. The map file shows 0-4,6-7. This function parses
+ * it and returns cpumap.
+ */
+static const char *
+linuxParseCPUmap(int *max_cpuid, const char *path)
+{
+char *map = NULL;
+char *str = NULL;
+int max_id, i;
+
+if (virFileReadAll(path, 5 * VIR_DOMAIN_CPUMASK_LEN, str) 0) {
+virReportOOMError();
+goto error;
+}
+
+if (VIR_ALLOC_N(map, VIR_DOMAIN_CPUMASK_LEN) 0) {
+virReportOOMError();
+goto error;
+}
+if (virDomainCpuSetParse(str, 0, map,
+ VIR_DOMAIN_CPUMASK_LEN) 0) {
+goto error;
+}
+
+for (i = 0; i VIR_DOMAIN_CPUMASK_LEN; i++) {
+if (map[i]) {
+max_id = i;
+}
+}
+*max_cpuid = max_id;
Please compile with './configure --enable-compile-warnings=error'.
The compiler spots that max_id could be used uninitialized here.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3 V7] add nodeGetCPUmap() for getting available CPU IDs in a cpumap.
This all appears to work. I built my own libvirt with the three remaining patches and was able to read pCPU information for a running domain. I have pushed the required bits upstream in ocaml-libvirt and virt-top: http://git.annexia.org/?p=ocaml-libvirt.git;a=summary http://git.annexia.org/?p=virt-top.git;a=summary (ocaml-libvirt = 0.6.1.1 and virt-top = 1.0.7) Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3 V7] add nodeGetCPUmap() for getting available CPU IDs in a cpumap.
On 03/06/2012 05:10 AM, Richard W.M. Jones wrote: This all appears to work. I built my own libvirt with the three remaining patches and was able to read pCPU information for a running domain. The libvirt side is now pushed. Rich: The documentation in libvirt.c correctly says that calling virDomainGetCPUStats(dom, NULL, 0, 0, 0, 0) must tell you part of the information needed to compute the size of an array to allocate; but in v7, the return was off by one too small. But it was generally masked by the v7 virsh code overallocating to a fixed 128 slots rather than paying attention to the return value. I fixed both those problems before pushing the libvirt patches, but since you tested with v7 instead of my fixed version, you may need to double check that virt-top isn't making the same mistakes, as it might be a boundary case that only strikes on machines with 128 physical CPUs. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3 V7] add nodeGetCPUmap() for getting available CPU IDs in a cpumap.
From: KAMEZAWA Hiroyuki kamezawa.hir...@jp.fujitsu.com
Changelog:
- fixed typos.
- fixed string scan routine.
Signed-off-by: KAMEZAWA Hiroyuki kamezawa.hir...@jp.fujitsu.com
Signed-off-by: Lai Jiangshan la...@cn.fujitsu.com
---
src/libvirt_private.syms |1 +
src/nodeinfo.c | 66 ++
src/nodeinfo.h |3 ++
3 files changed, 70 insertions(+), 0 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a104e70..0f4e64c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -793,6 +793,7 @@ virNodeDeviceObjUnlock;
# nodeinfo.h
nodeCapsInitNUMA;
+nodeGetCPUmap;
nodeGetCPUStats;
nodeGetCellsFreeMemory;
nodeGetFreeMemory;
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index e0b66f7..2950306 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -31,6 +31,7 @@
#include dirent.h
#include sys/utsname.h
#include sched.h
+#include conf/domain_conf.h
#if HAVE_NUMACTL
# define NUMA_VERSION1_COMPATIBILITY 1
@@ -569,6 +570,47 @@ int linuxNodeGetMemoryStats(FILE *meminfo,
cleanup:
return ret;
}
+
+/*
+ * Linux maintains cpu bit map. For example, if cpuid=5's flag is not set
+ * and max cpu is 7. The map file shows 0-4,6-7. This function parses
+ * it and returns cpumap.
+ */
+static const char *
+linuxParseCPUmap(int *max_cpuid, const char *path)
+{
+char *map = NULL;
+char *str = NULL;
+int max_id, i;
+
+if (virFileReadAll(path, 5 * VIR_DOMAIN_CPUMASK_LEN, str) 0) {
+virReportOOMError();
+goto error;
+}
+
+if (VIR_ALLOC_N(map, VIR_DOMAIN_CPUMASK_LEN) 0) {
+virReportOOMError();
+goto error;
+}
+if (virDomainCpuSetParse(str, 0, map,
+ VIR_DOMAIN_CPUMASK_LEN) 0) {
+goto error;
+}
+
+for (i = 0; i VIR_DOMAIN_CPUMASK_LEN; i++) {
+if (map[i]) {
+max_id = i;
+}
+}
+*max_cpuid = max_id;
+
+return map;
+
+error:
+VIR_FREE(str);
+VIR_FREE(map);
+return NULL;
+}
#endif
int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo) {
@@ -712,6 +754,30 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED,
#endif
}
+const char *
+nodeGetCPUmap(virConnectPtr conn ATTRIBUTE_UNUSED,
+ int *max_id ATTRIBUTE_UNUSED,
+ const char *mapname ATTRIBUTE_UNUSED)
+{
+#ifdef __linux__
+char *path;
+const char *cpumap;
+
+if (virAsprintf(path, CPU_SYS_PATH /%s, mapname) 0) {
+virReportOOMError();
+return NULL;
+}
+
+cpumap = linuxParseCPUmap(max_id, path);
+VIR_FREE(path);
+return cpumap;
+#else
+ nodeReportError(VIR_ERR_NO_SUPPORT, %s,
+ _(node cpumap not implemented on this platform));
+ return NULL;
+#endif
+}
+
#if HAVE_NUMACTL
# if LIBNUMA_API_VERSION = 1
# define NUMA_MAX_N_CPUS 4096
diff --git a/src/nodeinfo.h b/src/nodeinfo.h
index 4766152..852e19d 100644
--- a/src/nodeinfo.h
+++ b/src/nodeinfo.h
@@ -46,4 +46,7 @@ int nodeGetCellsFreeMemory(virConnectPtr conn,
int maxCells);
unsigned long long nodeGetFreeMemory(virConnectPtr conn);
+const char *nodeGetCPUmap(virConnectPtr conn,
+ int *max_id,
+ const char *mapname);
#endif /* __VIR_NODEINFO_H__*/
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
