[libvirt] [PATCHv2 0/2] a new flag VIR_DOMAIN_CPU_STATS_VCPU
From: root <root@KERNEL-128.(none)> This series adds a new flag VIR_DOMAIN_CPU_STATS_VCPU to virDomainGetCPUStats, which is for getting statistics of physical cpu time consumed by virtual cpus. Changes: v2: - added detailed documentation - added some error checks and fix some memleaks - the two patches rearranged slightly Hu Tao (2): Add a new flag VIR_DOMAIN_CPU_STATS_VCPU to virDomainGetCPUStats Adds support to VIR_DOMAIN_CPU_STATS_VCPU in qemu_driver. include/libvirt/libvirt.h.in | 8 ++ src/libvirt.c | 9 ++- src/qemu/qemu_driver.c | 186 ++++++++++++++++++++++++++++++++++++++---- src/util/cgroup.c | 4 +- tools/virsh.c | 16 +++- 5 files changed, 199 insertions(+), 24 deletions(-)
Currently virDomainGetCPUStats gets total cpu usage, which consists of: 1. vcpu usage: the physical cpu time consumed by virtual cpu(s) of domain 2. hypervisor: `total cpu usage' - `vcpu usage' The flag VIR_DOMAIN_CPU_STATS_VCPU is for getting vcpu usages. --- include/libvirt/libvirt.h.in | 8 ++++++++ src/libvirt.c | 9 +++++++-- tools/virsh.c | 16 +++++++++++----- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index ac5df95..2ce8876 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1355,6 +1355,14 @@ int virDomainGetState (virDomainPtr domain, */ #define VIR_DOMAIN_CPU_STATS_SYSTEMTIME "system_time" +typedef enum { + /* virTypedParameterFlags goes here. */ + + /* Additionally, these flags may be bitwise-OR'd in. These + flags should not override those of virTypedParameterFlags */ + VIR_DOMAIN_CPU_STATS_VCPU = 1 << 3, /* get vcpu stats */ +} virDomainGetCPUStatsFlags; + int virDomainGetCPUStats(virDomainPtr domain, virTypedParameterPtr params, unsigned int nparams, diff --git a/src/libvirt.c b/src/libvirt.c index af42d3b..3702bd4 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18670,7 +18670,7 @@ error: * @nparams: number of parameters per cpu * @start_cpu: which cpu to start with, or -1 for summary * @ncpus: how many cpus to query - * @flags: bitwise-OR of virTypedParameterFlags + * @flags: bitwise-OR of virDomainGetCPUStatsFlags * * Get statistics relating to CPU usage attributable to a single * domain (in contrast to the statistics returned by @@ -18704,7 +18704,12 @@ error: * host perspective, this would typically match the cpus member * of virNodeGetInfo(), but might be less due to host cpu hotplug. * - * For now, @flags is unused, and the statistics all relate to the + * If no flags specified, then statistics of total physical cpu time + * consumed by a domain are returned. If flag VIR_DOMAIN_CPU_STATS_VCPU + * is set, then statistics of physical cpu time consumed by virtual cpus + * of domain are returned. The difference is: + * total cpu usage = vcpu usage + hypervisor usage + * For now, the statistics all relate to the * usage from the host perspective. It is possible that a future * version will support a flag that queries the cpu usage from the * guest's perspective, where the maximum cpu to query would be diff --git a/tools/virsh.c b/tools/virsh.c index e177684..194dae7 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -5562,6 +5562,7 @@ static const vshCmdOptDef opts_cpu_stats[] = { {"total", VSH_OT_BOOL, 0, N_("Show total statistics only")}, {"start", VSH_OT_INT, 0, N_("Show statistics from this CPU")}, {"count", VSH_OT_INT, 0, N_("Number of shown CPUs at most")}, + {"vcpu", VSH_OT_BOOL, 0, N_("Show vcpu statistics")}, {NULL, 0, 0, NULL}, }; @@ -5572,6 +5573,7 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd) virTypedParameterPtr params = NULL; int i, j, pos, max_id, cpu = -1, show_count = -1, nparams; bool show_total = false, show_per_cpu = false; + unsigned int flags = 0; if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -5579,6 +5581,10 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; + if (vshCommandOptBool(cmd, "vcpu")) { + flags |= VIR_DOMAIN_CPU_STATS_VCPU; + } + show_total = vshCommandOptBool(cmd, "total"); if (vshCommandOptInt(cmd, "start", &cpu) > 0) show_per_cpu = true; @@ -5599,13 +5605,13 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd) cpu = 0; /* get number of cpus on the node */ - if ((max_id = virDomainGetCPUStats(dom, NULL, 0, 0, 0, 0)) < 0) + if ((max_id = virDomainGetCPUStats(dom, NULL, 0, 0, 0, flags)) < 0) goto failed_stats; if (show_count < 0 || show_count > max_id) show_count = max_id; /* get percpu information */ - if ((nparams = virDomainGetCPUStats(dom, NULL, 0, 0, 1, 0)) < 0) + if ((nparams = virDomainGetCPUStats(dom, NULL, 0, 0, 1, flags)) < 0) goto failed_stats; if (!nparams) { @@ -5619,7 +5625,7 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd) while (show_count) { int ncpus = MIN(show_count, 128); - if (virDomainGetCPUStats(dom, params, nparams, cpu, ncpus, 0) < 0) + if (virDomainGetCPUStats(dom, params, nparams, cpu, ncpus, flags) < 0) goto failed_stats; for (i = 0; i < ncpus; i++) { @@ -5653,7 +5659,7 @@ do_show_total: goto cleanup; /* get supported num of parameter for total statistics */ - if ((nparams = virDomainGetCPUStats(dom, NULL, 0, -1, 1, 0)) < 0) + if ((nparams = virDomainGetCPUStats(dom, NULL, 0, -1, 1, flags)) < 0) goto failed_stats; if (!nparams) { @@ -5665,7 +5671,7 @@ do_show_total: goto failed_params; /* passing start_cpu == -1 gives us domain's total status */ - if ((nparams = virDomainGetCPUStats(dom, params, nparams, -1, 1, 0)) < 0) + if ((nparams = virDomainGetCPUStats(dom, params, nparams, -1, 1, flags)) < 0) goto failed_stats; vshPrint(ctl, _("Total:\n")); -- 1.7.1
On 04/24/2012 03:20 AM, Hu Tao wrote:
Currently virDomainGetCPUStats gets total cpu usage, which consists of:
1. vcpu usage: the physical cpu time consumed by virtual cpu(s) of domain 2. hypervisor: `total cpu usage' - `vcpu usage'
The flag VIR_DOMAIN_CPU_STATS_VCPU is for getting vcpu usages. --- include/libvirt/libvirt.h.in | 8 ++++++++ src/libvirt.c | 9 +++++++-- tools/virsh.c | 16 +++++++++++----- 3 files changed, 26 insertions(+), 7 deletions(-)
Question: is it better to make the user call virDomainGetCPUStats twice (once for total with flags==0, once for vcpu with flags=VCPU), where there is a race between the two calls? Or should we instead return both counts in the same call, by having two separate virTypedParameter stat names and no flag? In other words, I'm thinking it might be better to keep "cpu_time" (VIR_DOMAIN_CPU_STATS_CPUTIME) as the first stat, and add a second stat "vcpu_time" (VIR_DOMAIN_CPU_STATS_VCPUTIME) as a second stat, all within the same call, with no need to add a flag.
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index ac5df95..2ce8876 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1355,6 +1355,14 @@ int virDomainGetState (virDomainPtr domain, */ #define VIR_DOMAIN_CPU_STATS_SYSTEMTIME "system_time"
So that would be a new #define here...
+typedef enum { + /* virTypedParameterFlags goes here. */ + + /* Additionally, these flags may be bitwise-OR'd in. These + flags should not override those of virTypedParameterFlags */ + VIR_DOMAIN_CPU_STATS_VCPU = 1 << 3, /* get vcpu stats */ +} virDomainGetCPUStatsFlags;
drop this enum...
@@ -18704,7 +18704,12 @@ error: * host perspective, this would typically match the cpus member * of virNodeGetInfo(), but might be less due to host cpu hotplug. * - * For now, @flags is unused, and the statistics all relate to the + * If no flags specified, then statistics of total physical cpu time + * consumed by a domain are returned. If flag VIR_DOMAIN_CPU_STATS_VCPU + * is set, then statistics of physical cpu time consumed by virtual cpus + * of domain are returned. The difference is: + * total cpu usage = vcpu usage + hypervisor usage + * For now, the statistics all relate to the
and tweak this documentation.
+++ b/tools/virsh.c @@ -5562,6 +5562,7 @@ static const vshCmdOptDef opts_cpu_stats[] = { {"total", VSH_OT_BOOL, 0, N_("Show total statistics only")}, {"start", VSH_OT_INT, 0, N_("Show statistics from this CPU")}, {"count", VSH_OT_INT, 0, N_("Number of shown CPUs at most")}, + {"vcpu", VSH_OT_BOOL, 0, N_("Show vcpu statistics")}, {NULL, 0, 0, NULL}, };
virsh would then automatically output both stats in one pass, and wouldn't need a --vcpu flag. The qemu_driver implementation would be mostly the same, but instead of basing things off a flag, it would always grab both stats (total and vcpu) at once. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Wed, May 02, 2012 at 01:43:03PM -0600, Eric Blake wrote:
On 04/24/2012 03:20 AM, Hu Tao wrote:
Currently virDomainGetCPUStats gets total cpu usage, which consists of:
1. vcpu usage: the physical cpu time consumed by virtual cpu(s) of domain 2. hypervisor: `total cpu usage' - `vcpu usage'
The flag VIR_DOMAIN_CPU_STATS_VCPU is for getting vcpu usages. --- include/libvirt/libvirt.h.in | 8 ++++++++ src/libvirt.c | 9 +++++++-- tools/virsh.c | 16 +++++++++++----- 3 files changed, 26 insertions(+), 7 deletions(-)
Question: is it better to make the user call virDomainGetCPUStats twice (once for total with flags==0, once for vcpu with flags=VCPU), where there is a race between the two calls? Or should we instead return both counts in the same call, by having two separate virTypedParameter stat names and no flag?
Slightly off-topic ... My opinion on this -- and it applies to many libvirt calls -- is that APIs where you have to call them twice, once to size the result, and a second time to get the result, are obtuse and hard to use. In libguestfs I took the approach that all calls return malloc'd strings, arrays, etc of the right size. The caller has to free the data, but that's the lesser of two evils. 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
On Wed, May 02, 2012 at 01:43:03PM -0600, Eric Blake wrote:
On 04/24/2012 03:20 AM, Hu Tao wrote:
Currently virDomainGetCPUStats gets total cpu usage, which consists of:
1. vcpu usage: the physical cpu time consumed by virtual cpu(s) of domain 2. hypervisor: `total cpu usage' - `vcpu usage'
The flag VIR_DOMAIN_CPU_STATS_VCPU is for getting vcpu usages. --- include/libvirt/libvirt.h.in | 8 ++++++++ src/libvirt.c | 9 +++++++-- tools/virsh.c | 16 +++++++++++----- 3 files changed, 26 insertions(+), 7 deletions(-)
Question: is it better to make the user call virDomainGetCPUStats twice (once for total with flags==0, once for vcpu with flags=VCPU), where there is a race between the two calls? Or should we instead return both
As Richard said, we already have had many two-stage-call APIs: one for the size, another for the result. Do we also face the same race there? Anyway, this is another story.
counts in the same call, by having two separate virTypedParameter stat names and no flag?
In other words, I'm thinking it might be better to keep "cpu_time" (VIR_DOMAIN_CPU_STATS_CPUTIME) as the first stat, and add a second stat "vcpu_time" (VIR_DOMAIN_CPU_STATS_VCPUTIME) as a second stat, all within the same call, with no need to add a flag.
But I like this way because it seems more natural. See v4. -- Thanks, Hu Tao
On 05/09/2012 02:38 AM, Hu Tao wrote:
Question: is it better to make the user call virDomainGetCPUStats twice (once for total with flags==0, once for vcpu with flags=VCPU), where there is a race between the two calls? Or should we instead return both
As Richard said, we already have had many two-stage-call APIs: one for the size, another for the result. Do we also face the same race there?
The two-stage-call API is an optimization - so that you can minimally size your array. You can always oversize your API to begin with, and libvirt should make it obvious (from return value, NULL entries at the end of the array, or some other means) if it did not fill in all the oversized entries you passed in. If you go with this approach (oversized arrays), then you need only call the API once. With oversized arrays, you risk running out of memory or exceeding RPC limits if you size too big, as well as extra overhead in determining what you got back. Also, if you are ever worried about a race (what happens if some other thread manages to change things so that between the time you get the size hint, then use that size), the solution is to always oversize by 1 more than the hint, then verify after you use your new size that you were indeed oversized. That is, if the hint says you should size with 2 elements, then you pass in 3 elements, you will either see 2 or fewer elements (no race occurred) or 3 elements (the race occurred, someone else was able to inject a new element between you grabbing the hint and actually using it, so try again with 4 elements). Some of our interfaces are not very race-prone - for example, the number of statistics returned by virDomainGetCPUStats is hard-coded in the driver returning stats (you have to recompile libvirtd to change from returning 1 stat into returning 2 stats); while others really are run-time variables (the number of online domains for virConnectListDomains can easily change between the hint and usage). We probably should do a better job of documenting when a hint will be unreliable and the caller should use the oversize technique to ensure that all values are returned. Also, I really want to add a new API that returns a list of virDomainPtr rather than ids or names, with a proper flags argument (so that you _don't_ have the race currently mandated by our poor design of virConnectListDomains/virConnectListDefinedDomains). When we add that API, we can do it properly (have the return list allocated by libvirt rather than pre-allocated by the caller).
Anyway, this is another story.
Agreed.
counts in the same call, by having two separate virTypedParameter stat names and no flag?
In other words, I'm thinking it might be better to keep "cpu_time" (VIR_DOMAIN_CPU_STATS_CPUTIME) as the first stat, and add a second stat "vcpu_time" (VIR_DOMAIN_CPU_STATS_VCPUTIME) as a second stat, all within the same call, with no need to add a flag.
But I like this way because it seems more natural. See v4.
At first glance, I already like v4 better, but I think it's worth waiting until after the 0.9.12 release to actually include it. I'll review it soon. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
--- src/qemu/qemu_driver.c | 186 +++++++++++++++++++++++++++++++++++++++++++---- src/util/cgroup.c | 4 +- 2 files changed, 173 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c3555ca..8ba3806 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12463,19 +12463,142 @@ qemuDomainGetTotalcpuStats(virCgroupPtr group, return nparams; } +/* get the cpu time from cpuacct cgroup group, saving + cpu time value in cpu_time. caller is responsible + for freeing memory allocated for cpu_time. + return 0 on success, -1 otherwise */ +static int getVcpuPercpuStats(virCgroupPtr group, + unsigned long long **cpu_time, + unsigned int *num) +{ + int ret = -1; + unsigned long long *ptime = NULL; + char *buf = NULL; + char *pos; + unsigned long long tmp; + + if (virCgroupGetCpuacctPercpuUsage(group, &buf)) + goto error; + + pos = buf; + *num = 0; + while (virStrToLong_ull(pos, &pos, 10, &tmp) == 0) + (*num)++; + + if (*num > 0) { + int i; + + if (VIR_ALLOC_N(ptime, *num) < 0) { + virReportOOMError(); + goto error; + } + + pos = buf; + for (i = 0; i < *num; i++) + if (virStrToLong_ull(pos, &pos, 10, ptime + i) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cpuacct parse error")); + } + *cpu_time = ptime; + ret = 0; + } +error: + VIR_FREE(buf); + return ret; +} + +/* This function gets the sums of cpu time consumed by all vcpus. + * For example, if there are 4 physical cpus, and 2 vcpus in a domain, + * then for each vcpu, the cpuacct.usage_percpu looks like this: + * t0 t1 t2 t3 + * and we have 2 groups of such data: + * v\p 0 1 2 3 + * 0 t00 t01 t02 t03 + * 1 t10 t11 t12 t13 + * for each pcpu, the sum is cpu time consumed by all vcpus. + * s0 = t00 + t10 + * s1 = t01 + t11 + * s2 = t02 + t12 + * s3 = t03 + t13 + */ +static int getSumVcpuPercpuStats(virCgroupPtr group, + unsigned int nvcpu, + unsigned long long **sum_cpu_time, + unsigned int *num) +{ + unsigned long long **cpu_time; + unsigned int *ncpu_time; + unsigned int max = 0; + unsigned long long *tmp = NULL; + int ret = -1; + int i, j; + + if ((VIR_ALLOC_N(cpu_time, nvcpu) < 0) || + (VIR_ALLOC_N(ncpu_time, nvcpu) < 0)) { + virReportOOMError(); + goto error; + } + + for (i = 0; i < nvcpu; i++) { + virCgroupPtr group_vcpu = NULL; + ret = virCgroupForVcpu(group, i, &group_vcpu, 0); + if (ret < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("error on creating cgroup cpuacct for vcpu")); + goto error; + } + ret = getVcpuPercpuStats(group_vcpu, + &cpu_time[i], + &ncpu_time[i]); + virCgroupFree(&group_vcpu); + + if (ret < 0) + goto error; + + if (max < ncpu_time[i]) + max = ncpu_time[i]; + } + + if (max > 0) { + if (VIR_ALLOC_N(tmp, max) < 0) + goto error; + + for (i = 0; i < nvcpu; i++) { + for (j = 0; j < ncpu_time[i]; j++) + tmp[j] += cpu_time[i][j]; + } + *sum_cpu_time = tmp; + *num = max; + ret = 0; + } + +error: + if (cpu_time) { + for (i = 0; i < nvcpu; i++) + VIR_FREE(cpu_time[i]); + } + + VIR_FREE(cpu_time); + VIR_FREE(ncpu_time); + return ret; +} + static int qemuDomainGetPercpuStats(virDomainPtr domain, + virDomainObjPtr vm, virCgroupPtr group, virTypedParameterPtr params, unsigned int nparams, int start_cpu, - unsigned int ncpus) + unsigned int ncpus, + unsigned int flags) { char *map = NULL; int rv = -1; int i, max_id; char *pos; char *buf = NULL; + qemuDomainObjPrivatePtr priv = vm->privateData; virTypedParameterPtr ent; int param_idx; @@ -12511,22 +12634,52 @@ qemuDomainGetPercpuStats(virDomainPtr domain, if (max_id - start_cpu > ncpus - 1) max_id = start_cpu + ncpus - 1; - for (i = 0; i <= max_id; i++) { + if (flags & VIR_DOMAIN_CPU_STATS_VCPU) { + unsigned long long *sum_cpu_time = NULL; + unsigned int n = 0; unsigned long long cpu_time; - if (!map[i]) { - cpu_time = 0; - } else if (virStrToLong_ull(pos, &pos, 10, &cpu_time) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("cpuacct parse error")); + if (getSumVcpuPercpuStats(group, + priv->nvcpupids, + &sum_cpu_time, + &n) < 0) goto cleanup; + + for (i = 0; i <= max_id && i < n; i++) { + if (i < start_cpu) + continue; + + if (!map[i]) + cpu_time = 0; + else + cpu_time = sum_cpu_time[i]; + if (virTypedParameterAssign(¶ms[(i - start_cpu) * nparams + param_idx], + VIR_DOMAIN_CPU_STATS_CPUTIME, + VIR_TYPED_PARAM_ULLONG, + cpu_time) < 0) { + VIR_FREE(sum_cpu_time); + goto cleanup; + } + } + VIR_FREE(sum_cpu_time); + } else { + for (i = 0; i <= max_id; i++) { + unsigned long long cpu_time; + + if (!map[i]) { + cpu_time = 0; + } else if (virStrToLong_ull(pos, &pos, 10, &cpu_time) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cpuacct parse error")); + goto cleanup; + } + if (i < start_cpu) + continue; + ent = ¶ms[ (i - start_cpu) * nparams + param_idx]; + if (virTypedParameterAssign(ent, VIR_DOMAIN_CPU_STATS_CPUTIME, + VIR_TYPED_PARAM_ULLONG, cpu_time) < 0) + goto cleanup; } - if (i < start_cpu) - continue; - ent = ¶ms[ (i - start_cpu) * nparams + param_idx]; - if (virTypedParameterAssign(ent, VIR_DOMAIN_CPU_STATS_CPUTIME, - VIR_TYPED_PARAM_ULLONG, cpu_time) < 0) - goto cleanup; } rv = param_idx + 1; cleanup: @@ -12550,7 +12703,8 @@ qemuDomainGetCPUStats(virDomainPtr domain, int ret = -1; bool isActive; - virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1); + virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY | + VIR_DOMAIN_CPU_STATS_VCPU, -1); qemuDriverLock(driver); @@ -12583,8 +12737,8 @@ qemuDomainGetCPUStats(virDomainPtr domain, if (start_cpu == -1) ret = qemuDomainGetTotalcpuStats(group, params, nparams); else - ret = qemuDomainGetPercpuStats(domain, group, params, nparams, - start_cpu, ncpus); + ret = qemuDomainGetPercpuStats(domain, vm, group, params, nparams, + start_cpu, ncpus, flags); cleanup: virCgroupFree(&group); if (vm) diff --git a/src/util/cgroup.c b/src/util/cgroup.c index ad49bc2..5b32881 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -530,7 +530,9 @@ static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group, continue; /* We need to control cpu bandwidth for each vcpu now */ - if ((flags & VIR_CGROUP_VCPU) && (i != VIR_CGROUP_CONTROLLER_CPU)) { + if ((flags & VIR_CGROUP_VCPU) && + (i != VIR_CGROUP_CONTROLLER_CPU && + i != VIR_CGROUP_CONTROLLER_CPUACCT)) { /* treat it as unmounted and we can use virCgroupAddTask */ VIR_FREE(group->controllers[i].mountPoint); continue; -- 1.7.1
--- examples/get_cpu_stats.ml | 2 +- libvirt/libvirt.ml | 2 +- libvirt/libvirt.mli | 2 +- libvirt/libvirt_c_oneoffs.c | 9 +++++---- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/examples/get_cpu_stats.ml b/examples/get_cpu_stats.ml index 79d5c3c..3541720 100644 --- a/examples/get_cpu_stats.ml +++ b/examples/get_cpu_stats.ml @@ -25,7 +25,7 @@ let () = let stats = let dom = D.lookup_by_name conn domname in - D.get_cpu_stats dom nr_pcpus in + D.get_cpu_stats dom nr_pcpus 0 in Array.iteri ( fun n params -> diff --git a/libvirt/libvirt.ml b/libvirt/libvirt.ml index 53c5bb4..7a32071 100644 --- a/libvirt/libvirt.ml +++ b/libvirt/libvirt.ml @@ -417,7 +417,7 @@ struct external set_vcpus : [>`W] t -> int -> unit = "ocaml_libvirt_domain_set_vcpus" external pin_vcpu : [>`W] t -> int -> string -> unit = "ocaml_libvirt_domain_pin_vcpu" external get_vcpus : [>`R] t -> int -> int -> int * vcpu_info array * string = "ocaml_libvirt_domain_get_vcpus" - external get_cpu_stats : [>`R] t -> int -> typed_param list array = "ocaml_libvirt_domain_get_cpu_stats" + external get_cpu_stats : [>`R] t -> int -> int -> typed_param list array = "ocaml_libvirt_domain_get_cpu_stats" external get_max_vcpus : [>`R] t -> int = "ocaml_libvirt_domain_get_max_vcpus" external attach_device : [>`W] t -> xml -> unit = "ocaml_libvirt_domain_attach_device" external detach_device : [>`W] t -> xml -> unit = "ocaml_libvirt_domain_detach_device" diff --git a/libvirt/libvirt.mli b/libvirt/libvirt.mli index 0913a63..9782406 100644 --- a/libvirt/libvirt.mli +++ b/libvirt/libvirt.mli @@ -559,7 +559,7 @@ sig for a domain. See the libvirt documentation for details of the array and bitmap returned from this function. *) - val get_cpu_stats : [>`R] t -> int -> typed_param list array + val get_cpu_stats : [>`R] t -> int -> int -> typed_param list array (** [get_pcpu_stats dom nr_pcpu] returns the physical CPU stats for a domain. See the libvirt documentation for details. *) diff --git a/libvirt/libvirt_c_oneoffs.c b/libvirt/libvirt_c_oneoffs.c index 3d42b73..135b934 100644 --- a/libvirt/libvirt_c_oneoffs.c +++ b/libvirt/libvirt_c_oneoffs.c @@ -532,20 +532,21 @@ extern int virDomainGetCPUStats (virDomainPtr domain, #endif CAMLprim value -ocaml_libvirt_domain_get_cpu_stats (value domv, value nr_pcpusv) +ocaml_libvirt_domain_get_cpu_stats (value domv, value nr_pcpusv, value flagsv) { #ifdef HAVE_VIRDOMAINGETCPUSTATS - CAMLparam2 (domv, nr_pcpusv); + CAMLparam3 (domv, nr_pcpusv, flagsv); CAMLlocal5 (cpustats, param_head, param_node, typed_param, typed_param_value); CAMLlocal1 (v); virDomainPtr dom = Domain_val (domv); virConnectPtr conn = Connect_domv (domv); int nr_pcpus = Int_val (nr_pcpusv); + int flags = Int_val (flagsv); virTypedParameterPtr params; int r, cpu, ncpus, nparams, i, j, pos; /* get percpu information */ - NONBLOCKING (nparams = virDomainGetCPUStats(dom, NULL, 0, 0, 1, 0)); + NONBLOCKING (nparams = virDomainGetCPUStats(dom, NULL, 0, 0, 1, flags)); CHECK_ERROR (nparams < 0, conn, "virDomainGetCPUStats"); if ((params = malloc(sizeof(*params) * nparams * 128)) == NULL) @@ -556,7 +557,7 @@ ocaml_libvirt_domain_get_cpu_stats (value domv, value nr_pcpusv) while (cpu < nr_pcpus) { ncpus = nr_pcpus - cpu > 128 ? 128 : nr_pcpus - cpu; - NONBLOCKING (r = virDomainGetCPUStats(dom, params, nparams, cpu, ncpus, 0)); + NONBLOCKING (r = virDomainGetCPUStats(dom, params, nparams, cpu, ncpus, flags)); CHECK_ERROR (r < 0, conn, "virDomainGetCPUStats"); for (i = 0; i < ncpus; i++) { -- 1.7.1
On Thu, Apr 26, 2012 at 04:09:05PM +0800, Hu Tao wrote:
--- examples/get_cpu_stats.ml | 2 +- libvirt/libvirt.ml | 2 +- libvirt/libvirt.mli | 2 +- libvirt/libvirt_c_oneoffs.c | 9 +++++---- 4 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/examples/get_cpu_stats.ml b/examples/get_cpu_stats.ml index 79d5c3c..3541720 100644 --- a/examples/get_cpu_stats.ml +++ b/examples/get_cpu_stats.ml @@ -25,7 +25,7 @@ let () =
let stats = let dom = D.lookup_by_name conn domname in - D.get_cpu_stats dom nr_pcpus in + D.get_cpu_stats dom nr_pcpus 0 in
This needs to be a list of (the OCaml equivalent of) virTypedParameterFlags. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
Remove parameter nr_pcpus. Add another parameter total of type bool to indicate wheter to get total cpu statistic or per_cpu statistics. --- examples/get_cpu_stats.ml | 50 ++++++++++++++---------- libvirt/libvirt.ml | 2 +- libvirt/libvirt.mli | 2 +- libvirt/libvirt_c_oneoffs.c | 89 +++++++++++++++++++++++++++++++++++++++++- 4 files changed, 117 insertions(+), 26 deletions(-) diff --git a/examples/get_cpu_stats.ml b/examples/get_cpu_stats.ml index 3541720..6355071 100644 --- a/examples/get_cpu_stats.ml +++ b/examples/get_cpu_stats.ml @@ -18,32 +18,40 @@ let () = let domname = Sys.argv.(1) in let conn = C.connect_readonly () in - - let nr_pcpus = - let info = C.get_node_info conn in - C.maxcpus_of_node_info info in - - let stats = - let dom = D.lookup_by_name conn domname in - D.get_cpu_stats dom nr_pcpus 0 in + let dom = D.lookup_by_name conn domname in + + let stats = D.get_cpu_stats dom false 0 in + let total_stats = D.get_cpu_stats dom true 0 in + + let print_params n params = + List.iter ( + fun (name, value) -> + printf " %s=" name; + match value with + | D.TypedFieldInt32 i -> printf "%ld" i + | D.TypedFieldUInt32 i -> printf "%ld" i + | D.TypedFieldInt64 i -> printf "%Ld" i + | D.TypedFieldUInt64 i -> printf "%Ld" i + | D.TypedFieldFloat f -> printf "%g" f + | D.TypedFieldBool b -> printf "%b" b + | D.TypedFieldString s -> printf "%S" s + ) params in Array.iteri ( fun n params -> printf "pCPU %d:" n; - List.iter ( - fun (name, value) -> - printf " %s=" name; - match value with - | D.TypedFieldInt32 i -> printf "%ld" i - | D.TypedFieldUInt32 i -> printf "%ld" i - | D.TypedFieldInt64 i -> printf "%Ld" i - | D.TypedFieldUInt64 i -> printf "%Ld" i - | D.TypedFieldFloat f -> printf "%g" f - | D.TypedFieldBool b -> printf "%b" b - | D.TypedFieldString s -> printf "%S" s - ) params; + print_params n params; + printf "\n" + ) stats; + + Array.iteri ( + fun n params -> + printf "total:"; + print_params n params; printf "\n" - ) stats + ) total_stats + + with Libvirt.Virterror err -> eprintf "error: %s\n" (Libvirt.Virterror.to_string err) diff --git a/libvirt/libvirt.ml b/libvirt/libvirt.ml index 7a32071..102aec7 100644 --- a/libvirt/libvirt.ml +++ b/libvirt/libvirt.ml @@ -417,7 +417,7 @@ struct external set_vcpus : [>`W] t -> int -> unit = "ocaml_libvirt_domain_set_vcpus" external pin_vcpu : [>`W] t -> int -> string -> unit = "ocaml_libvirt_domain_pin_vcpu" external get_vcpus : [>`R] t -> int -> int -> int * vcpu_info array * string = "ocaml_libvirt_domain_get_vcpus" - external get_cpu_stats : [>`R] t -> int -> int -> typed_param list array = "ocaml_libvirt_domain_get_cpu_stats" + external get_cpu_stats : [>`R] t -> bool -> int -> typed_param list array = "ocaml_libvirt_domain_get_cpu_stats" external get_max_vcpus : [>`R] t -> int = "ocaml_libvirt_domain_get_max_vcpus" external attach_device : [>`W] t -> xml -> unit = "ocaml_libvirt_domain_attach_device" external detach_device : [>`W] t -> xml -> unit = "ocaml_libvirt_domain_detach_device" diff --git a/libvirt/libvirt.mli b/libvirt/libvirt.mli index 9782406..17892a1 100644 --- a/libvirt/libvirt.mli +++ b/libvirt/libvirt.mli @@ -559,7 +559,7 @@ sig for a domain. See the libvirt documentation for details of the array and bitmap returned from this function. *) - val get_cpu_stats : [>`R] t -> int -> int -> typed_param list array + val get_cpu_stats : [>`R] t -> bool -> int -> typed_param list array (** [get_pcpu_stats dom nr_pcpu] returns the physical CPU stats for a domain. See the libvirt documentation for details. *) diff --git a/libvirt/libvirt_c_oneoffs.c b/libvirt/libvirt_c_oneoffs.c index 135b934..12ad634 100644 --- a/libvirt/libvirt_c_oneoffs.c +++ b/libvirt/libvirt_c_oneoffs.c @@ -532,18 +532,26 @@ extern int virDomainGetCPUStats (virDomainPtr domain, #endif CAMLprim value -ocaml_libvirt_domain_get_cpu_stats (value domv, value nr_pcpusv, value flagsv) +ocaml_libvirt_domain_get_cpu_stats (value domv, value totalv, value flagsv) { #ifdef HAVE_VIRDOMAINGETCPUSTATS - CAMLparam3 (domv, nr_pcpusv, flagsv); + CAMLparam3 (domv, totalv, flagsv); CAMLlocal5 (cpustats, param_head, param_node, typed_param, typed_param_value); CAMLlocal1 (v); virDomainPtr dom = Domain_val (domv); virConnectPtr conn = Connect_domv (domv); - int nr_pcpus = Int_val (nr_pcpusv); + int get_total = totalv == Val_true ? 1 : 0; int flags = Int_val (flagsv); virTypedParameterPtr params; int r, cpu, ncpus, nparams, i, j, pos; + int nr_pcpus; + + if (get_total) + goto do_total; + + /* get number of pcpus */ + NONBLOCKING (nr_pcpus = virDomainGetCPUStats(dom, NULL, 0, 0, 0, flags)); + CHECK_ERROR (nr_pcpus < 0, conn, "virDomainGetCPUStats"); /* get percpu information */ NONBLOCKING (nparams = virDomainGetCPUStats(dom, NULL, 0, 0, 1, flags)); @@ -630,6 +638,81 @@ ocaml_libvirt_domain_get_cpu_stats (value domv, value nr_pcpusv, value flagsv) } free(params); CAMLreturn (cpustats); + +do_total: + + /* get total information */ + NONBLOCKING (nparams = virDomainGetCPUStats(dom, NULL, 0, -1, 1, flags)); + CHECK_ERROR (nparams < 0, conn, "virDomainGetCPUStats"); + + if ((params = malloc(sizeof(*params) * nparams)) == NULL) + caml_failwith ("virDomainGetCPUStats: malloc"); + + cpustats = caml_alloc (1, 0); /* cpustats: array of params(list of typed_param) */ + + NONBLOCKING (r = virDomainGetCPUStats(dom, params, nparams, -1, 1, flags)); + CHECK_ERROR (r < 0, conn, "virDomainGetCPUStats"); + + param_head = Val_emptylist; + if (params[nparams].type != 0) { + for (j = r - 1; j >= 0; j--) { + pos = j; + + param_node = caml_alloc(2, 0); /* param_node: typed_param, next param_node */ + Store_field(param_node, 1, param_head); + param_head = param_node; + + typed_param = caml_alloc(2, 0); /* typed_param: field name(string), typed_param_value */ + Store_field(param_node, 0, typed_param); + Store_field(typed_param, 0, caml_copy_string(params[pos].field)); + + /* typed_param_value: value with the corresponding type tag */ + switch(params[pos].type) { + case VIR_TYPED_PARAM_INT: + typed_param_value = caml_alloc (1, 0); + v = caml_copy_int32 (params[pos].value.i); + break; + case VIR_TYPED_PARAM_UINT: + typed_param_value = caml_alloc (1, 1); + v = caml_copy_int32 (params[pos].value.ui); + break; + case VIR_TYPED_PARAM_LLONG: + typed_param_value = caml_alloc (1, 2); + v = caml_copy_int64 (params[pos].value.l); + break; + case VIR_TYPED_PARAM_ULLONG: + typed_param_value = caml_alloc (1, 3); + v = caml_copy_int64 (params[pos].value.ul); + break; + case VIR_TYPED_PARAM_DOUBLE: + typed_param_value = caml_alloc (1, 4); + v = caml_copy_double (params[pos].value.d); + break; + case VIR_TYPED_PARAM_BOOLEAN: + typed_param_value = caml_alloc (1, 5); + v = Val_bool (params[pos].value.b); + break; + case VIR_TYPED_PARAM_STRING: + typed_param_value = caml_alloc (1, 6); + v = caml_copy_string (params[pos].value.s); + free (params[pos].value.s); + break; + default: + /* XXX Memory leak on this path, if there are more + * VIR_TYPED_PARAM_STRING past this point in the array. + */ + free (params); + caml_failwith ("virDomainGetCPUStats: " + "unknown parameter type returned"); + } + Store_field (typed_param_value, 0, v); + Store_field (typed_param, 1, typed_param_value); + } + } + Store_field (cpustats, 0, param_head); + + free(params); + CAMLreturn (cpustats); #else not_supported ("virDomainGetCPUStats"); #endif -- 1.7.1
participants (3)
-
Eric Blake -
Hu Tao -
Richard W.M. Jones