Signed-off-by: Lai Jiangshan <laijs(a)cn.fujitsu.com&gt; --- daemon/remote.c | 68 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 51 +++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 17 ++++++++++- src/remote_protocol-structs | 13 ++++++++ 4 files changed, 148 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index a28a754..9b0206f 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2084,6 +2084,74 @@ cleanup: return rv; } +static int +remoteDispatchDomainGetPcpusUsage(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr hdr ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_pcpus_usage_args *args, + remote_domain_get_pcpus_usage_ret *ret) +{ + int i; + virDomainPtr dom = NULL; + int rv = -1; + unsigned long long *usages; + int nr_usages = args->nr_usages; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (nr_usages > REMOTE_DOMAIN_PCPUS_USAGE_PARAMETERS_MAX) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("nr_usages too large")); + goto cleanup; + } + + if (VIR_ALLOC_N(usages, nr_usages) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + + if (virDomainGetPcpusUsage(dom, usages, &nr_usages, args->flags) < 0) + goto cleanup; + + ret->nr_usages = nr_usages; + + /* + * In this case, we need to send back the number of parameters + * supported + */ + if (args->nr_usages == 0) { + goto success; + } + + ret->usages.usages_len = MIN(args->nr_usages, nr_usages); + if (VIR_ALLOC_N(ret->usages.usages_val, ret->usages.usages_len) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0; i < ret->usages.usages_len; i++) { + ret->usages.usages_val[i] = usages[i]; + } + +success: + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + VIR_FREE(usages); + if (dom) + virDomainFree(dom); + return rv; +} #ifdef HAVE_SASL /* diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e28840b..9e2f931 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2305,6 +2305,56 @@ done: return rv; } +static int remoteDomainGetPcpusUsage(virDomainPtr domain, + unsigned long long *usages, + int *nr_usages, + unsigned int flags) +{ + int i; + int rv = -1; + remote_domain_get_pcpus_usage_args args; + remote_domain_get_pcpus_usage_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, domain); + args.nr_usages = *nr_usages; + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + + if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_PCPUS_USAGE, + (xdrproc_t) xdr_remote_domain_get_pcpus_usage_args, + (char *) &args, + (xdrproc_t) xdr_remote_domain_get_pcpus_usage_ret, + (char *) &ret) == -1) { + goto done; + } + + /* Handle the case when the caller does not know the number of parameters + * and is asking for the number of parameters supported + */ + if (*nr_usages == 0) { + *nr_usages = ret.nr_usages; + goto cleanup; + } + + for (i = 0; i < MIN(*nr_usages, ret.usages.usages_len); i++) { + usages[i] = ret.usages.usages_val[i]; + } + *nr_usages = ret.nr_usages; + + rv = 0; + +cleanup: + xdr_free ((xdrproc_t) xdr_remote_domain_get_pcpus_usage_ret, + (char *) &ret); +done: + remoteDriverUnlock(priv); + return rv; +} + /*----------------------------------------------------------------------*/ static virDrvOpenStatus ATTRIBUTE_NONNULL (1) @@ -4750,6 +4800,7 @@ static virDriver remote_driver = { .domainGetBlockIoTune = remoteDomainGetBlockIoTune, /* 0.9.8 */ .domainSetNumaParameters = remoteDomainSetNumaParameters, /* 0.9.9 */ .domainGetNumaParameters = remoteDomainGetNumaParameters, /* 0.9.9 */ + .domainGetPcpusUsage = remoteDomainGetPcpusUsage, /* 0.9.10 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index ca739ff..3c1ab41 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -131,6 +131,9 @@ const REMOTE_DOMAIN_BLOCK_IO_TUNE_PARAMETERS_MAX = 16; /* Upper limit on list of numa parameters. */ const REMOTE_DOMAIN_NUMA_PARAMETERS_MAX = 16; +/* Upper limit on list of physical cpu parameters. */ +const REMOTE_DOMAIN_PCPUS_USAGE_PARAMETERS_MAX = 32; + /* Upper limit on list of node cpu stats. */ const REMOTE_NODE_CPU_STATS_MAX = 16; @@ -1148,6 +1151,17 @@ struct remote_domain_get_block_io_tune_ret { int nparams; }; +struct remote_domain_get_pcpus_usage_args { + remote_nonnull_domain dom; + int nr_usages; + unsigned int flags; +}; + +struct remote_domain_get_pcpus_usage_ret { + uint64_t usages<REMOTE_DOMAIN_PCPUS_USAGE_PARAMETERS_MAX>; + int nr_usages; +}; + /* Network calls: */ struct remote_num_of_networks_ret { @@ -2653,7 +2667,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SET_NUMA_PARAMETERS = 254, /* autogen autogen */ REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, /* autogen autogen */ - REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_GET_PCPUS_USAGE = 258 /* skipgen skipgen */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 2758315..4fd7b6d 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1832,6 +1832,18 @@ struct remote_node_suspend_for_duration_args { uint64_t duration; u_int flags; }; +struct remote_domain_get_pcpus_usage_args { + remote_nonnull_domain dom; + int nr_usages; + u_int flags; +}; +struct remote_domain_get_pcpus_usage_ret { + struct { + u_int usages_len; + unsigned long long *usages_val; + } usages; + int nr_usages; +}; enum remote_procedure { REMOTE_PROC_OPEN = 1, REMOTE_PROC_CLOSE = 2, @@ -2090,4 +2102,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255, REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, + REMOTE_PROC_DOMAIN_GET_PCPU_USAGE = 258, }; -- 1.7.4.4

This command gets information about the physical CPUs. Example: # virsh pcpuinfo rhel6 CPU: 0 Curr VCPU: - Usage: 47.3 CPU: 1 Curr VCPU: 1 Usage: 46.8 CPU: 2 Curr VCPU: 0 Usage: 52.7 CPU: 3 Curr VCPU: - Usage: 44.1 Signed-off-by: Lai Jiangshan <laijs(a)cn.fujitsu.com&gt; --- tools/virsh.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 5 +++ 2 files changed, 98 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index c511e2a..e8b9221 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -4702,6 +4702,98 @@ cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd) } /* + * "pcpuinfo" command + */ +static const vshCmdInfo info_pcpuinfo[] = { + {"help", N_("detailed domain pcpu information")}, + {"desc", N_("Returns basic information about the domain's physical CPUs.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_pcpuinfo[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdPcpuinfo(vshControl *ctl, const vshCmd *cmd) +{ + virDomainInfo info; + virDomainPtr dom; + virNodeInfo nodeinfo; + virVcpuInfoPtr cpuinfo; + unsigned char *cpumaps; + int ncpus, maxcpu; + size_t cpumaplen; + bool ret = true; + int n, m; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (virNodeGetInfo(ctl->conn, &nodeinfo) != 0) { + virDomainFree(dom); + return false; + } + + if (virDomainGetInfo(dom, &info) != 0) { + virDomainFree(dom); + return false; + } + + cpuinfo = vshMalloc(ctl, sizeof(virVcpuInfo)*info.nrVirtCpu); + maxcpu = VIR_NODEINFO_MAXCPUS(nodeinfo); + cpumaplen = VIR_CPU_MAPLEN(maxcpu); + cpumaps = vshMalloc(ctl, info.nrVirtCpu * cpumaplen); + + if ((ncpus = virDomainGetVcpus(dom, + cpuinfo, info.nrVirtCpu, + cpumaps, cpumaplen)) >= 0) { + unsigned long long *usages; + int nr_usages = maxcpu; + + if (VIR_ALLOC_N(usages, nr_usages) < 0) { + virReportOOMError(); + goto fail; + } + + if (virDomainGetPcpusUsage(dom, usages, &nr_usages, 0) < 0) { + VIR_FREE(usages); + goto fail; + } + + for (n = 0; n < MIN(maxcpu, nr_usages); n++) { + vshPrint(ctl, "%-15s %d\n", _("CPU:"), n); + for (m = 0; m < ncpus; m++) { + if (cpuinfo[m].cpu == n) { + vshPrint(ctl, "%-15s %d\n", _("Curr VCPU:"), m); + break; + } + } + if (m == ncpus) { + vshPrint(ctl, "%-15s %s\n", _("Curr VCPU:"), _("-")); + } + vshPrint(ctl, "%-15s %.1lf\n\n", _("Usage:"), + usages[n] / 1000000000.0); + } + VIR_FREE(usages); + goto cleanup; + } + +fail: + ret = false; + +cleanup: + VIR_FREE(cpumaps); + VIR_FREE(cpuinfo); + virDomainFree(dom); + return ret; +} + +/* * "vcpupin" command */ static const vshCmdInfo info_vcpupin[] = { @@ -15952,6 +16044,7 @@ static const vshCmdDef domManagementCmds[] = { {"migrate-getspeed", cmdMigrateGetMaxSpeed, opts_migrate_getspeed, info_migrate_getspeed, 0}, {"numatune", cmdNumatune, opts_numatune, info_numatune, 0}, + {"pcpuinfo", cmdPcpuinfo, opts_pcpuinfo, info_pcpuinfo, 0}, {"reboot", cmdReboot, opts_reboot, info_reboot, 0}, {"reset", cmdReset, opts_reset, info_reset, 0}, {"restore", cmdRestore, opts_restore, info_restore, 0}, diff --git a/tools/virsh.pod b/tools/virsh.pod index c88395b..8d9cceb 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1296,6 +1296,11 @@ Thus, this command always takes exactly zero or two flags. Returns basic information about the domain virtual CPUs, like the number of vCPUs, the running time, the affinity to physical processors. +=item B<pcpuinfo> I<domain-id> + +Returns information about the physical CPUs of the domain, like the usage of +CPUs, the current attached vCPUs. + =item B<vcpupin> I<domain-id> [I<vcpu>] [I<cpulist>] [[I<--live>] [I<--config>] | [I<--current>]] -- 1.7.4.4

Acked-by: "Richard W.M. Jones" <rjones(a)redhat.com&gt; Signed-off-by: Lai Jiangshan <laijs(a)cn.fujitsu.com&gt; --- libvirt/libvirt.ml | 1 + libvirt/libvirt.mli | 4 ++++ libvirt/libvirt_c_oneoffs.c | 25 +++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 0 deletions(-) diff --git a/libvirt/libvirt.ml b/libvirt/libvirt.ml index fc29264..a8720a9 100644 --- a/libvirt/libvirt.ml +++ b/libvirt/libvirt.ml @@ -419,6 +419,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_pcpu_usages : [>`R] t -> int -> int64 array = "ocaml_libvirt_domain_get_pcpu_usages" 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 7bda889..63bf830 100644 --- a/libvirt/libvirt.mli +++ b/libvirt/libvirt.mli @@ -586,6 +586,10 @@ sig for a domain. See the libvirt documentation for details of the array and bitmap returned from this function. *) + val get_pcpu_usages : [>`R] t -> int -> int64 array + (** [get_pcpu_usages dom nr_pcpu] returns the physical CPU usages + for a domain. See the libvirt documentation for details. + *) val get_max_vcpus : [>`R] t -> int (** Returns the maximum number of vCPUs supported for this domain. *) val attach_device : [>`W] t -> xml -> unit diff --git a/libvirt/libvirt_c_oneoffs.c b/libvirt/libvirt_c_oneoffs.c index d87dd21..68d5ecc 100644 --- a/libvirt/libvirt_c_oneoffs.c +++ b/libvirt/libvirt_c_oneoffs.c @@ -604,6 +604,31 @@ ocaml_libvirt_domain_get_vcpus (value domv, value maxinfov, value maplenv) CAMLreturn (rv); } +CAMLprim value +ocaml_libvirt_domain_get_pcpu_usages (value domv, value nr_pcpusv) +{ + CAMLparam2 (domv, nr_pcpusv); + CAMLlocal1 (usagev); + virDomainPtr dom = Domain_val (domv); + virConnectPtr conn = Connect_domv (domv); + int nr_pcpus = Int_val (nr_pcpusv); + unsigned long long pcpu_usages[nr_pcpus]; + int r, i; + + memset (pcpu_usages, 0, sizeof(pcpu_usages[0]) * nr_pcpus); + + NONBLOCKING (r = virDomainGetPcpusUsage (dom, pcpu_usages, &nr_pcpus, 0)); + CHECK_ERROR (r == -1, conn, "virDomainGetPcpusUsage"); + + /* Copy the pcpu_usages. */ + usagev = caml_alloc (nr_pcpus, 0); + for (i = 0; i < nr_pcpus; ++i) { + Store_field (usagev, i, caml_copy_int64 ((int64_t)pcpu_usages[i])); + } + + CAMLreturn (usagev); +} + #ifdef HAVE_WEAK_SYMBOLS #ifdef HAVE_VIRDOMAINMIGRATE extern virDomainPtr virDomainMigrate (virDomainPtr domain, virConnectPtr dconn, -- 1.7.4.4

On 01/27/2012 11:20 PM, KAMEZAWA Hiroyuki wrote: s/morified/modified/ Nice extension to my proposal. And you made it - I'm going to push this today, so your API is definitely in 0.9.10, even if we need a few touchups discovered during testing during the freeze. For example, it's fine if you don't have the Python implementation done before the freeze; that's something I don't mind taking during the freeze week on the grounds that it is rounding out a feature that we have committed to, and that it won't be altering the API itself. But of course, sooner means more review time and testing :) I'm hoisting the hunk from 4/5 on libvirt.h.in, where you define the first stat, "cpu_time". This should use the common text we have elsewhere. I'd combine these limits, and mention that they mainly apply to remote protocols. s/ncparam/nparams/ Yes, this is useful. but it means this information is not quite right. The return paragraph should be last. This repeats part of the return paragraph. This should be documented by the various macros for well-defined stat names. Example code is written for apps that aren't using libvirt internals, therefore it must use calloc instead of VIR_ALLOC_N. s/parasm/params/ This should be moved up right after the documentation of special casing. Missing params. We can further check that start_cpu is -1 or non-negative. We can further check that nparams is non-zero if params is non-NULL. We can further check that ncpus is non-zero unless params is NULL. Since we don't distinguish the error message, we could join these conditionals. This restriction should only be forced by the remote protocol. See virDomainMemoryPeek for an example of a documented RPC limitation, but which is only enforced in the RPC code. I like to keep this sorted. Overall, ACK - you picked up on the review suggestions, and the API looks good enough now to commit to. Here's what I'm squashing before pushing, which means we now have the API in place before the freeze! I'm not sure if I will finish reviewing the rest of the series today, seeing as it is my weekend, but we'll certainly get it all in before the release. diff --git i/include/libvirt/libvirt.h.in w/include/libvirt/libvirt.h.in index 10a8862..f55fac3 100644 --- i/include/libvirt/libvirt.h.in +++ w/include/libvirt/libvirt.h.in @@ -3811,6 +3811,14 @@ int virConnectSetKeepAlive(virConnectPtr conn, int interval, unsigned int count); +/* Collecting CPU statistics */ + +/** + * VIR_DOMAIN_CPU_STATS_CPUTIME: + * cpu usage in nanoseconds, as a ullong + */ +#define VIR_DOMAIN_CPU_STATS_CPUTIME "cpu_time" + int virDomainGetCPUStats(virDomainPtr domain, virTypedParameterPtr params, unsigned int nparams, diff --git i/src/libvirt.c w/src/libvirt.c index 6ff93cc..7efea67 100644 --- i/src/libvirt.c +++ w/src/libvirt.c @@ -18159,7 +18159,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: unused for now + * @flags: extra flags; not used yet, so callers should always pass 0 * * Get statistics relating to CPU usage attributable to a single * domain (in contrast to the statistics returned by @@ -18168,13 +18168,6 @@ error: * usage). On input, @params must contain at least @nparams * @ncpus * entries, allocated by the caller. * - * Now, @ncpus is limited to be <= 128. If you want to get - * values in a host with more cpus, you need to call multiple times. - * - * @nparams are limited to be <= 16 but maximum params per cpu - * provided by will be smaller than this. - * - * * If @start_cpu is -1, then @ncpus must be 1, and the returned * results reflect the statistics attributable to the entire * domain (such as user and system time for the process as a @@ -18183,57 +18176,52 @@ error: * query, with statistics attributable per processor (such as * per-cpu usage). * - * As a special case, if @params is NULL and @nparams is 0 and + * The remote driver imposes a limit of 128 @ncpus and 16 @nparams; + * the number of parameters per cpu should not exceed 16, but if you + * have a host with more than 128 CPUs, your program should split + * the request into multiple calls. + * + * As special cases, if @params is NULL and @nparams is 0 and * @ncpus is 1, and the return value will be how many * statistics are available for the given @start_cpu. This number * may be different for @start_cpu of -1 than for any non-negative * value, but will be the same for all non-negative @start_cpu. - * - * And, if @param is NULL and @ncparam is 0 and @ncpus is 0, - * Max cpu id of the node is returned. (considering cpu hotplug, - * max cpu id may be different from the number of cpu on a host.) + * Likewise, if @params is NULL and @nparams is 0 and @ncpus is 0, + * the number of cpus available to query is returned. From the + * 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 - * usage from the host perspective; the number of cpus available to - * query can be determined by the cpus member of the result of - * virNodeGetInfo(), even if the domain has had vcpus pinned to only - * use a subset of overall host cpus. It is possible that a future + * 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, using the number of vcpus available to the - * guest, found by virDomainGetVcpusFlags(). An individual guest - * vcpu cannot be reliably mapped back to a specific host cpu unless - * a single-processor vcpu pinning was used, but when @start_cpu is -1, - * any difference in usage between a host and guest perspective would - * serve as a measure of hypervisor overhead. - * - * Returns -1 on failure, or the number of statistics that were - * populated per cpu on success (this will be less than the total - * number of populated @params, unless @ncpus was 1; and may be - * less than @nparams). The populated parameters start at each - * stride of @nparams; any unpopulated parameters will be zeroed - * on success. The caller is responsible for freeing any returned - * string parameters. - * - * Note: - * Because cpu ids may be discontig, retuned @param array may contain - * zero-filled entry in the middle. - * All time related values are represented in ns, using UULONG. + * guest's perspective, where the maximum cpu to query would be + * related to virDomainGetVcpusFlags() rather than virNodeGetInfo(). + * An individual guest vcpu cannot be reliably mapped back to a + * specific host cpu unless a single-processor vcpu pinning was used, + * but when @start_cpu is -1, any difference in usage between a host + * and guest perspective would serve as a measure of hypervisor overhead. * * Typical use sequence is below. * * getting total stats: set start_cpu as -1, ncpus 1 * virDomainGetCPUStats(dom, NULL, 0, -1, 1, 0) => nparams - * VIR_ALLOC_N(params, nparams) - * virDomainGetCPUStats(dom, parasm, nparams, -1, 1, 0) => total stats. + * params = calloc(nparams, sizeof (virTypedParameter)) + * virDomainGetCPUStats(dom, params, nparams, -1, 1, 0) => total stats. * - * getting percpu stats: - * virDomainGetCPUStats(dom, NULL, 0, 0, 0, 0) => max_cpu_id + * getting per-cpu stats: + * virDomainGetCPUStats(dom, NULL, 0, 0, 0, 0) => ncpus * virDomainGetCPUStats(dom, NULL, 0, 0, 1, 0) => nparams - * VIR_ALLOC_N(params, max_cpu_id * nparams) - * virDomainGetCPUStats(dom, params, nparams, 0, max_cpu_id, 0) => percpu stats + * params = calloc(ncpus * nparams, sizeof (virTypedParameter)) + * virDomainGetCPUStats(dom, params, nparams, 0, ncpus, 0) => per-cpu stats * + * Returns -1 on failure, or the number of statistics that were + * populated per cpu on success (this will be less than the total + * number of populated @params, unless @ncpus was 1; and may be + * less than @nparams). The populated parameters start at each + * stride of @nparams, which means the results may be discontiguous; + * any unpopulated parameters will be zeroed on success. The caller + * is responsible for freeing any returned string parameters. */ - int virDomainGetCPUStats(virDomainPtr domain, virTypedParameterPtr params, unsigned int nparams, @@ -18243,8 +18231,9 @@ int virDomainGetCPUStats(virDomainPtr domain, { virConnectPtr conn; - VIR_DOMAIN_DEBUG(domain, "nparams=%d, start_cpu=%d, ncpu=%d, flags=%x", - nparams, start_cpu, ncpus, flags); + VIR_DOMAIN_DEBUG(domain, + "params=%p, nparams=%d, start_cpu=%d, ncpus=%u, flags=%x", + params, nparams, start_cpu, ncpus, flags); virResetLastError(); if (!VIR_IS_CONNECTED_DOMAIN(domain)) { @@ -18254,13 +18243,16 @@ int virDomainGetCPUStats(virDomainPtr domain, } conn = domain->conn; - /* start_cpu == -1 is a special case. ncpus must be 1 */ - if ((start_cpu == -1) && (ncpus != 1)) { - virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); - goto error; - } - /* if params == NULL, nparams must be 0 */ - if ((params == NULL) && ((nparams != 0))) { + /* Special cases: + * start_cpu must be non-negative, or else -1 + * if start_cpu is -1, ncpus must be 1 + * params == NULL must match nparams == 0 + * ncpus must be non-zero unless params == NULL + */ + if (start_cpu < -1 || + (start_cpu == -1 && ncpus != 1) || + ((params == NULL) != (nparams == 0)) || + (ncpus == 0 && params != NULL)) { virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); goto error; } -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

[I see you cc'd Eric(a)redhat.com; unfortunately, that doesn't forward to me - at least I saw it on-list :) ] On 01/27/2012 11:20 PM, KAMEZAWA Hiroyuki wrote: Phooey - I didn't notice this earlier, but we like new API to occur in context (all the virDomain calls in sequence), and not slammed at the bottom of the file (where we sunk the deprecated interfaces). I'm pushing this followup as trivial (it is purely code motion and doc comments).diff --git i/include/libvirt/libvirt.h.in w/include/libvirt/libvirt.h.in index f55fac3..0a7b324 100644 --- i/include/libvirt/libvirt.h.in +++ w/include/libvirt/libvirt.h.in @@ -1127,6 +1127,10 @@ char * virConnectGetURI (virConnectPtr conn); char * virConnectGetSysinfo (virConnectPtr conn, unsigned int flags); +int virConnectSetKeepAlive(virConnectPtr conn, + int interval, + unsigned int count); + /* * Capabilities of the connection / driver. @@ -1299,14 +1303,29 @@ char * virDomainScreenshot (virDomainPtr domain, unsigned int flags); /* - * Domain runtime information + * Domain runtime information, and collecting CPU statistics */ + int virDomainGetInfo (virDomainPtr domain, virDomainInfoPtr info); int virDomainGetState (virDomainPtr domain, int *state, int *reason, unsigned int flags); + +/** + * VIR_DOMAIN_CPU_STATS_CPUTIME: + * cpu usage in nanoseconds, as a ullong + */ +#define VIR_DOMAIN_CPU_STATS_CPUTIME "cpu_time" + +int virDomainGetCPUStats(virDomainPtr domain, + virTypedParameterPtr params, + unsigned int nparams, + int start_cpu, + unsigned int ncpus, + unsigned int flags); + int virDomainGetControlInfo (virDomainPtr domain, virDomainControlInfoPtr info, unsigned int flags); @@ -3807,24 +3826,6 @@ typedef struct _virTypedParameter virMemoryParameter; */ typedef virMemoryParameter *virMemoryParameterPtr; -int virConnectSetKeepAlive(virConnectPtr conn, - int interval, - unsigned int count); - -/* Collecting CPU statistics */ - -/** - * VIR_DOMAIN_CPU_STATS_CPUTIME: - * cpu usage in nanoseconds, as a ullong - */ -#define VIR_DOMAIN_CPU_STATS_CPUTIME "cpu_time" - -int virDomainGetCPUStats(virDomainPtr domain, - virTypedParameterPtr params, - unsigned int nparams, - int start_cpu, - unsigned int ncpus, - unsigned int flags); #ifdef __cplusplus } #endif -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Changed from V3: use new virDomainGetCPUStats() libvirt-API. use C code to construct the typed_param list array Acked-by: "Richard W.M. Jones" <rjones(a)redhat.com&gt; Signed-off-by: Lai Jiangshan <laijs(a)cn.fujitsu.com&gt; --- libvirt/libvirt.ml | 8 ++++ libvirt/libvirt.mli | 11 +++++ libvirt/libvirt_c_oneoffs.c | 91 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 0 deletions(-) diff --git a/libvirt/libvirt.ml b/libvirt/libvirt.ml index fc29264..08c501b 100644 --- a/libvirt/libvirt.ml +++ b/libvirt/libvirt.ml @@ -343,6 +343,13 @@ struct | SchedFieldInt64 of int64 | SchedFieldUInt64 of int64 | SchedFieldFloat of float | SchedFieldBool of bool + type typed_param = string * typed_param_value + and typed_param_value = + | TypedFieldInt32 of int32 | TypedFieldUInt32 of int32 + | TypedFieldInt64 of int64 | TypedFieldUInt64 of int64 + | TypedFieldFloat of float | TypedFieldBool of bool + | TypedFieldString of string + type migrate_flag = Live type memory_flag = Virtual @@ -419,6 +426,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_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 7bda889..31f2ca2 100644 --- a/libvirt/libvirt.mli +++ b/libvirt/libvirt.mli @@ -439,6 +439,13 @@ sig | SchedFieldInt64 of int64 | SchedFieldUInt64 of int64 | SchedFieldFloat of float | SchedFieldBool of bool + type typed_param = string * typed_param_value + and typed_param_value = + | TypedFieldInt32 of int32 | TypedFieldUInt32 of int32 + | TypedFieldInt64 of int64 | TypedFieldUInt64 of int64 + | TypedFieldFloat of float | TypedFieldBool of bool + | TypedFieldString of string + type migrate_flag = Live type memory_flag = Virtual @@ -586,6 +593,10 @@ 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 + (** [get_pcpu_stats dom nr_pcpu] returns the physic CPU stats + for a domain. See the libvirt documentation for details. + *) val get_max_vcpus : [>`R] t -> int (** Returns the maximum number of vCPUs supported for this domain. *) val attach_device : [>`W] t -> xml -> unit diff --git a/libvirt/libvirt_c_oneoffs.c b/libvirt/libvirt_c_oneoffs.c index d87dd21..a0c74be 100644 --- a/libvirt/libvirt_c_oneoffs.c +++ b/libvirt/libvirt_c_oneoffs.c @@ -604,6 +604,97 @@ ocaml_libvirt_domain_get_vcpus (value domv, value maxinfov, value maplenv) CAMLreturn (rv); } +CAMLprim value +ocaml_libvirt_domain_get_cpu_stats (value domv, value nr_pcpusv) +{ + CAMLparam2 (domv, nr_pcpusv); + CAMLlocal5 (cpustats, param_head, param_node, typed_param, typed_param_value); + virDomainPtr dom = Domain_val (domv); + virConnectPtr conn = Connect_domv (domv); + int nr_pcpus = Int_val (nr_pcpusv); + virTypedParameterPtr params; + int r, cpu, ncpus, nparams, i, j, pos; + + /* get percpu information */ + NONBLOCKING (nparams = virDomainGetCPUStats(dom, NULL, 0, -1, 1, 0)); + CHECK_ERROR (nparams < 0, conn, "virDomainGetCPUStats"); + + if ((params = malloc(sizeof(*params) * nparams * 128)) == NULL) + caml_failwith ((char *)__FUNCTION__); + + cpustats = caml_alloc (nr_pcpus, 0); /* cpustats: array of params(list of typed_param) */ + cpu = 0; + while (cpu < nr_pcpus) { + ncpus = nr_pcpus - cpu > 128 ? 128 : nr_pcpus - cpu; + + NONBLOCKING (r = virDomainGetCPUStats(dom, params, nparams, cpu, ncpus, 0)); + CHECK_ERROR (r < 0, conn, "virDomainGetCPUStats"); + + for (i = 0; i < ncpus; i++) { + /* list of typed_param: single linked list of param_nodes */ + param_head = Val_emptylist; /* param_head: the head param_node of list of typed_param */ + + if (params[i * nparams].type == 0) { + Store_field(cpustats, cpu + i, param_head); + continue; + } + + for (j = nparams - 1; j >= 0; j--) { + pos = i * nparams + j; + if (params[pos].type == 0) + continue; + + 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); + Store_field(typed_param_value, 0, caml_copy_int32(params[pos].value.i)); + break; + case VIR_TYPED_PARAM_UINT: + typed_param_value = caml_alloc (1, 1); + Store_field(typed_param_value, 0, caml_copy_int32(params[pos].value.ui)); + break; + case VIR_TYPED_PARAM_LLONG: + typed_param_value = caml_alloc (1, 2); + Store_field(typed_param_value, 0, caml_copy_int64(params[pos].value.l)); + break; + case VIR_TYPED_PARAM_ULLONG: + typed_param_value = caml_alloc (1, 3); + Store_field(typed_param_value, 0, caml_copy_int64(params[pos].value.ul)); + break; + case VIR_TYPED_PARAM_DOUBLE: + typed_param_value = caml_alloc (1, 4); + Store_field(typed_param_value, 0, caml_copy_double(params[pos].value.d)); + break; + case VIR_TYPED_PARAM_BOOLEAN: + typed_param_value = caml_alloc (1, 5); + Store_field(typed_param_value, 0, Int_val(!!(params[pos].value.b))); + break; + case VIR_TYPED_PARAM_STRING: + typed_param_value = caml_alloc (1, 6); + Store_field(typed_param_value, 0, caml_copy_string(params[pos].value.s)); + break; + default: + free(params); + caml_failwith ((char *)__FUNCTION__); + } + Store_field(typed_param, 1, typed_param_value); + } + Store_field(cpustats, cpu + i, param_head); + } + cpu += ncpus; + } + free(params); + CAMLreturn (cpustats); +} #ifdef HAVE_WEAK_SYMBOLS #ifdef HAVE_VIRDOMAINMIGRATE extern virDomainPtr virDomainMigrate (virDomainPtr domain, virConnectPtr dconn,

Hi, Richard virt-top and ocaml site patches can work and are completed weeks ago since public API of libvirt is merged. Could you merged them? virt-top and ocaml site patches only depend on the public API of libvirt. but the testing of them may depend on the qemu-driver-site patch of libvirt, which is sent today. Thanks, Lai On 02/08/2012 04:59 PM, Lai Jiangshan wrote:

From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; add virBitmapParseCommaSeparetedFormat() for parsing bitmap in comma separeted ascii format. This format of bitmap is used in Linux sysfs and cpuset. Changelog: - fixed typos. - fixed string scan routine. Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; Signed-off-by: Lai Jiangshan <laijs(a)cn.fujitsu.com&gt; --- src/libvirt_private.syms | 2 + src/nodeinfo.c | 51 ++++++++++++++++++++++++++++++++++++ src/nodeinfo.h | 4 +++ src/util/bitmap.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/bitmap.h | 6 +++- 5 files changed, 127 insertions(+), 1 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d6ad36c..14b144f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -17,6 +17,7 @@ virBitmapFree; virBitmapGetBit; virBitmapSetBit; virBitmapString; +virBitmapParseCommaSeparatedFormat; # buf.h @@ -797,6 +798,7 @@ nodeGetCellsFreeMemory; nodeGetFreeMemory; nodeGetInfo; nodeGetMemoryStats; +nodeGetCPUmap; # nwfilter_conf.h diff --git a/src/nodeinfo.c b/src/nodeinfo.c index e0b66f7..b0ebb88 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -47,6 +47,7 @@ #include "count-one-bits.h" #include "intprops.h" #include "virfile.h" +#include "bitmap.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -569,6 +570,33 @@ 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 functin parses + * it and returns bitmap. + */ +static virBitmapPtr linuxParseCPUmap(int *max_cpuid, const char *path) +{ + char str[1024]; + FILE *fp; + virBitmapPtr map = NULL; + + fp = fopen(path, "r"); + if (!fp) { + virReportSystemError(errno, _("cannot open %s"), path); + goto cleanup; + } + if (fgets(str, sizeof(str), fp) == NULL) { + virReportSystemError(errno, _("cannot read from %s"), path); + goto cleanup; + } + map = virBitmapParseCommaSeparatedFormat(str, max_cpuid); + +cleanup: + VIR_FORCE_FCLOSE(fp); + return map; +} #endif int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo) { @@ -712,6 +740,29 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED, #endif } +virBitmapPtr nodeGetCPUmap(virConnectPtr conn ATTRIBUTE_UNUSED, + int *max_id, + const char *mapname) +{ +#ifdef __linux__ + char *path; + virBitmapPtr map; + + if (virAsprintf(&path, CPU_SYS_PATH "/%s", mapname) < 0) { + virReportOOMError(); + return NULL; + } + + map = linuxParseCPUmap(max_id, path); + VIR_FREE(path); + return map; +#else + nodeReportError(VIR_ERR_NO_SUPPORT, "%s", + _("node cpumap not implemented on this platform")); + return -1; +#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..7f26b77 100644 --- a/src/nodeinfo.h +++ b/src/nodeinfo.h @@ -26,6 +26,7 @@ # include "libvirt/libvirt.h" # include "capabilities.h" +# include "bitmap.h" int nodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo); int nodeCapsInitNUMA(virCapsPtr caps); @@ -46,4 +47,7 @@ int nodeGetCellsFreeMemory(virConnectPtr conn, int maxCells); unsigned long long nodeGetFreeMemory(virConnectPtr conn); +virBitmapPtr nodeGetCPUmap(virConnectPtr conn, + int *max_id, + const char *mapname); #endif /* __VIR_NODEINFO_H__*/ diff --git a/src/util/bitmap.c b/src/util/bitmap.c index 8c326c4..6dead3b 100644 --- a/src/util/bitmap.c +++ b/src/util/bitmap.c @@ -33,6 +33,11 @@ #include "bitmap.h" #include "memory.h" #include "buf.h" +#include "c-ctype.h" +#include "util.h" +#include "virterror_internal.h" + +#define VIR_FROM_THIS VIR_FROM_NONE struct _virBitmap { @@ -180,3 +185,63 @@ char *virBitmapString(virBitmapPtr bitmap) return virBufferContentAndReset(&buf); } + +/** + * virBitmapParseCommaSeparatedFormat: + * + * When bitmap is printed in ascii format, especially in Linux, + * comma-separated format is sometimes used. For example, a bitmap 11111011 is + * represetned as 0-4,6-7. This function parses comma-separated format + * and returns virBitmap. Found max bit is returned, too. + * + * This functions stops if characters other than digits, ',', '-' are + * found. This function will be useful for parsing linux's bitmap. + */ +virBitmapPtr virBitmapParseCommaSeparatedFormat(char *buf, int *max_bit) +{ + char *pos = buf; + virBitmapPtr map = NULL; + int val, x; + + /* at first, find the highest number */ + val = 0; + while ((*pos != '\n') && (*pos != 0)) { + if (c_isdigit(*pos)) { + virStrToLong_i(pos, &pos, 10, &val); + } else if ((*pos == ',') || (*pos == '-')) { + ++pos; + } else + break; + } + *max_bit = val; + + map = virBitmapAlloc(val + 1); + if (map == NULL) { /* we may return NULL at failure of parsing */ + virReportOOMError(); + return NULL; + } + + pos = buf; + while ((*pos != '\n') && (*pos != 0)) { + if (c_isdigit(*pos)) { + virStrToLong_i(pos, &pos, 10, &val); + if (virBitmapSetBit(map, val) < 0) + goto failed; + } else if (*pos == ',') { + ++pos; + } else if (*pos == '-') { + x = val; + pos++; + virStrToLong_i(pos, &pos, 10, &val); + for (;x <= val; ++x) { + if (virBitmapSetBit(map, x) < 0) + goto failed; + } + } else + break; + } + return map; +failed: + virBitmapFree(map); + return NULL; +} diff --git a/src/util/bitmap.h b/src/util/bitmap.h index ef62cca..e3d3e2b 100644 --- a/src/util/bitmap.h +++ b/src/util/bitmap.h @@ -61,5 +61,9 @@ int virBitmapGetBit(virBitmapPtr bitmap, size_t b, bool *result) char *virBitmapString(virBitmapPtr bitmap) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; - +/* + * parese comma-separeted bitmap format and allocate virBitmap. + */ +virBitmapPtr virBitmapParseCommaSeparatedFormat(char *buf, int *max_bit) + ATTRIBUTE_RETURN_CHECK; #endif -- 1.7.4.4

On 02/09/2012 03:43 AM, Lai Jiangshan wrote: Needs consistent quoting with lines above. Formatting nit: these days, we are preferring to start new function names on column 1, with the return type on the previous line, as in: static int qemuDomainGetTotalCpuStats(...) Why are you passing an unused parameter through to a static function? The only time that should happen is in a callback interface, but you are directly calling this function, so I'd prune the parameter instead. No need to re-check flags; we already checked it up front in the primary entry point. At which point, you probably don't need the flags parameter in this function. s/supprted/supported/ Why are you using the 'param_idx' variable, which is always 0? I'd simplify this to just 'return 1;', and worry about things if we ever add additional parameters later. Again, why check flags here? It should have already been done in the caller. I'm wondering if you can just use virDomainCpuSetParse to get a cpumap, and then use macros like VIR_CPU_USABLE in parsing that map, rather than going through virBitmap. s/accoutning/accounting/ This part looks right, even if we aren't incrementing param_idx until some future date when we add a second parameter. Formatting nit: s/ ,/, / We've now missed 0.9.10, so this should be 0.9.11. -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 02/09/2012 03:43 AM, Lai Jiangshan wrote: We should align the output so all '.' are in the same column. Given that the kernel gives us more than just tenths of a second, should we be exposing more granularity to the user? Wrong comment, since you installed it under "cpu-accts". But I still don't like that name, either; since we're wrapping virDomainGetCPUStats, I think a better name would be "cpu-stats". Here, I'd use "show domain cpu usage statistics", and for the longer text, maybe "Display information about the domain's CPU usage, including per-CPU accounting" Should we expose the ability to pass in '--all' or an optional '--start=n --count=n' to let the user limit how much input they want? Given your above example, virsh cpu-stats dom => Stats for all CPUs, and a total virsh cpu-stats dom --all => Only the total stats virsh cpu-stats dom --start=2 => Only the per-cpu stats, for CPU 2 to the end virsh cpu-stats dom --start=2 --count=1 => Only the per-cpu stats of just CPU 2 If max_id is less than 0, then it will be -1 and the command failed; we should print out the libvirt error at that point. That may not be true in the future. I'd rather see this specifically do a STREQ(params[i].field, VIR_DOMAIN_CPU_STATS_CPUTIME) before reformatting the output... ...and the default, rather than erroring out, should instead use vshGetTypedParamValue and output the name/value pair for all statistics that were unknown by this version of virsh but which may have been added in the server. Memory leak if you don't use virTypedParameterArrayClear first. Just because we don't pass strings back now doesn't mean we won't do it in the future. Why are we re-allocating this each time through the loop? It should be enough to allocate it once before the loop, and reuse it thereafter. This shouldn't be possible (if it is true, we have a bug in our server). This shouldn't be possible (if it is true, we have a bug in our server). Same story about formatting - we want to format all name/value pairs, even those that were added in the server after this version of virsh was compiled; and we can only do the conversion from nanoseconds to fractional seconds if the field name is known. Your comment was right - you are missing a call to virTypedParameterArrayClear. And even if you hoist the array allocation outside of the loop, you still need an array clear in the loop between successive iterations. -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; * Now, only "cpu_time" is supported. * cpuacct cgroup is used for providing percpu cputime information. * include/libvirt/libvirt.h.in - defines VIR_DOMAIN_CPU_STATS_CPUTIME * src/qemu/qemu.conf - take care of cpuacct cgroup. * src/qemu/qemu_conf.c - take care of cpuacct cgroup. * src/qemu/qemu_driver.c - added an interface * src/util/cgroup.c/h - added interface for getting percpu cputime Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; Signed-off-by: Lai Jiangshan <laijs(a)cn.fujitsu.com&gt; --- src/qemu/qemu.conf | 3 +- src/qemu/qemu_conf.c | 3 +- src/qemu/qemu_driver.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/cgroup.c | 6 ++ src/util/cgroup.h | 1 + 5 files changed, 158 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 95428c1..cb87728 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -166,6 +166,7 @@ # - 'memory' - use for memory tunables # - 'blkio' - use for block devices I/O tunables # - 'cpuset' - use for CPUs and memory nodes +# - 'cpuacct' - use for CPUs statistics. # # NB, even if configured here, they won't be used unless # the administrator has mounted cgroups, e.g.: @@ -177,7 +178,7 @@ # can be mounted in different locations. libvirt will detect # where they are located. # -# cgroup_controllers = [ "cpu", "devices", "memory", "blkio", "cpuset" ] +# cgroup_controllers = [ "cpu", "devices", "memory", "blkio", "cpuset", "cpuacct" ] # This is the basic set of devices allowed / required by # all virtual machines. diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index e95c7a5..a709cbf 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -318,7 +318,8 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, (1 << VIR_CGROUP_CONTROLLER_DEVICES) | (1 << VIR_CGROUP_CONTROLLER_MEMORY) | (1 << VIR_CGROUP_CONTROLLER_BLKIO) | - (1 << VIR_CGROUP_CONTROLLER_CPUSET); + (1 << VIR_CGROUP_CONTROLLER_CPUSET) | + (1 << VIR_CGROUP_CONTROLLER_CPUACCT); } for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { if (driver->cgroupControllers & (1 << i)) { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e2e73b7..e7871ab 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12078,6 +12078,152 @@ cleanup: return ret; } + +/* qemuDomainGetCPUStats() with start_cpu == -1 */ +static int +qemuDomainGetTotalcpuStats(virCgroupPtr group, + virTypedParameterPtr params, + int nparams) +{ + unsigned long long cpu_time; + int param_idx = 0; + int ret; + + if (nparams == 0) /* return supported number of params */ + return 1; + /* entry 0 is cputime */ + ret = virCgroupGetCpuacctUsage(group, &cpu_time); + if (ret < 0) { + virReportSystemError(-ret, "%s", _("unable to get cpu account")); + return -1; + } + + virTypedParameterAssign(&params[param_idx], VIR_DOMAIN_CPU_STATS_CPUTIME, + VIR_TYPED_PARAM_ULLONG, cpu_time); + return 1; +} + +static int qemuDomainGetPercpuStats(virDomainPtr domain, + virCgroupPtr group, + virTypedParameterPtr params, + unsigned int nparams, + int start_cpu, + unsigned int ncpus) +{ + const char *map = NULL; + int rv = -1; + int i, max_id; + char *pos; + char *buf = NULL; + virTypedParameterPtr ent; + int param_idx; + + /* return the number of supported params ? */ + if (nparams == 0 && ncpus != 0) + return 1; /* only cpu_time is supported */ + + /* return percpu cputime in index 0 */ + param_idx = 0; + /* to parse account file, we need "present" cpu map */ + map = nodeGetCPUmap(domain->conn, &max_id, "present"); + if (!map) + return rv; + + if (ncpus == 0) { /* returns max cpu ID */ + rv = max_id; + goto cleanup; + } + /* we get percpu cputime accounting info. */ + if (virCgroupGetCpuacctPercpuUsage(group, &buf)) + goto cleanup; + pos = buf; + + if (max_id > start_cpu + ncpus - 1) + max_id = start_cpu + ncpus - 1; + + for (i = 0; i <= max_id; i++) { + unsigned long long cpu_time; + + if (!map[i]) + continue; + 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 = &params[ (i - start_cpu) * nparams + param_idx]; + virTypedParameterAssign(ent, VIR_DOMAIN_CPU_STATS_CPUTIME, + VIR_TYPED_PARAM_ULLONG, cpu_time); + } + rv = param_idx + 1; +cleanup: + VIR_FREE(buf); + VIR_FREE(map); + return rv; +} + + +static int +qemuDomainGetCPUStats(virDomainPtr domain, + virTypedParameterPtr params, + unsigned int nparams, + int start_cpu, + unsigned int ncpus, + unsigned int flags) +{ + struct qemud_driver *driver = domain->conn->privateData; + virCgroupPtr group = NULL; + virDomainObjPtr vm = NULL; + int ret = -1; + bool isActive; + + virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1); + + qemuDriverLock(driver); + + vm = virDomainFindByUUID(&driver->domains, domain->uuid); + if (vm == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("No such domain %s"), domain->uuid); + goto cleanup; + } + + isActive = virDomainObjIsActive(vm); + if (!isActive) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain is not running")); + goto cleanup; + } + + if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUACCT)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cgroup CPUACCT controller is not mounted")); + goto cleanup; + } + + if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find cgroup for domain %s"), vm->def->name); + goto cleanup; + } + + if (nparams == 0 && ncpus == 0) /* returns max cpu id */ + ret = qemuDomainGetPercpuStats(domain, group, NULL, 0, 0, 0); + else if (start_cpu == -1) /* get total */ + ret = qemuDomainGetTotalcpuStats(group, params, nparams); + else + ret = qemuDomainGetPercpuStats(domain, group, params, nparams, + start_cpu, ncpus); +cleanup: + virCgroupFree(&group); + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} + static virDriver qemuDriver = { .no = VIR_DRV_QEMU, .name = "QEMU", @@ -12235,6 +12381,7 @@ static virDriver qemuDriver = { .domainGetDiskErrors = qemuDomainGetDiskErrors, /* 0.9.10 */ .domainSetMetadata = qemuDomainSetMetadata, /* 0.9.10 */ .domainGetMetadata = qemuDomainGetMetadata, /* 0.9.10 */ + .domainGetCPUStats = qemuDomainGetCPUStats, /* 0.9.11 */ }; diff --git a/src/util/cgroup.c b/src/util/cgroup.c index 15b870d..2d34c50 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -1555,6 +1555,12 @@ int virCgroupGetCpuacctUsage(virCgroupPtr group, unsigned long long *usage) "cpuacct.usage", usage); } +int virCgroupGetCpuacctPercpuUsage(virCgroupPtr group, char **usage) +{ + return virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_CPUACCT, + "cpuacct.usage_percpu", usage); +} + int virCgroupSetFreezerState(virCgroupPtr group, const char *state) { return virCgroupSetValueStr(group, diff --git a/src/util/cgroup.h b/src/util/cgroup.h index 8d75735..b4e0f37 100644 --- a/src/util/cgroup.h +++ b/src/util/cgroup.h @@ -115,6 +115,7 @@ int virCgroupSetCpuCfsQuota(virCgroupPtr group, long long cfs_quota); int virCgroupGetCpuCfsQuota(virCgroupPtr group, long long *cfs_quota); int virCgroupGetCpuacctUsage(virCgroupPtr group, unsigned long long *usage); +int virCgroupGetCpuacctPercpuUsage(virCgroupPtr group, char **usage); int virCgroupSetFreezerState(virCgroupPtr group, const char *state); int virCgroupGetFreezerState(virCgroupPtr group, char **state); -- 1.7.4.4

Hi, Eric Did you received these new patches? Thanks, Lai On 02/16/2012 03:15 PM, Lai Jiangshan wrote:

From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; * Now, only "cpu_time" is supported. * cpuacct cgroup is used for providing percpu cputime information. * include/libvirt/libvirt.h.in - defines VIR_DOMAIN_CPU_STATS_CPUTIME * src/qemu/qemu.conf - take care of cpuacct cgroup. * src/qemu/qemu_conf.c - take care of cpuacct cgroup. * src/qemu/qemu_driver.c - added an interface * src/util/cgroup.c/h - added interface for getting percpu cputime Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; Signed-off-by: Lai Jiangshan <laijs(a)cn.fujitsu.com&gt; --- src/qemu/qemu.conf | 3 +- src/qemu/qemu_conf.c | 3 +- src/qemu/qemu_driver.c | 146 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/cgroup.c | 6 ++ src/util/cgroup.h | 1 + 5 files changed, 157 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 95428c1..cb87728 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -166,6 +166,7 @@ # - 'memory' - use for memory tunables # - 'blkio' - use for block devices I/O tunables # - 'cpuset' - use for CPUs and memory nodes +# - 'cpuacct' - use for CPUs statistics. # # NB, even if configured here, they won't be used unless # the administrator has mounted cgroups, e.g.: @@ -177,7 +178,7 @@ # can be mounted in different locations. libvirt will detect # where they are located. # -# cgroup_controllers = [ "cpu", "devices", "memory", "blkio", "cpuset" ] +# cgroup_controllers = [ "cpu", "devices", "memory", "blkio", "cpuset", "cpuacct" ] # This is the basic set of devices allowed / required by # all virtual machines. diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index e95c7a5..a709cbf 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -318,7 +318,8 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, (1 << VIR_CGROUP_CONTROLLER_DEVICES) | (1 << VIR_CGROUP_CONTROLLER_MEMORY) | (1 << VIR_CGROUP_CONTROLLER_BLKIO) | - (1 << VIR_CGROUP_CONTROLLER_CPUSET); + (1 << VIR_CGROUP_CONTROLLER_CPUSET) | + (1 << VIR_CGROUP_CONTROLLER_CPUACCT); } for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { if (driver->cgroupControllers & (1 << i)) { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c6bdd29..d6b56ae 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12087,6 +12087,151 @@ cleanup: return ret; } +/* qemuDomainGetCPUStats() with start_cpu == -1 */ +static int +qemuDomainGetTotalcpuStats(virCgroupPtr group, + virTypedParameterPtr params, + int nparams) +{ + unsigned long long cpu_time; + int param_idx = 0; + int ret; + + if (nparams == 0) /* return supported number of params */ + return 1; + /* entry 0 is cputime */ + ret = virCgroupGetCpuacctUsage(group, &cpu_time); + if (ret < 0) { + virReportSystemError(-ret, "%s", _("unable to get cpu account")); + return -1; + } + + virTypedParameterAssign(&params[param_idx], VIR_DOMAIN_CPU_STATS_CPUTIME, + VIR_TYPED_PARAM_ULLONG, cpu_time); + return 1; +} + +static int qemuDomainGetPercpuStats(virDomainPtr domain, + virCgroupPtr group, + virTypedParameterPtr params, + unsigned int nparams, + int start_cpu, + unsigned int ncpus) +{ + const char *map = NULL; + int rv = -1; + int i, max_id; + char *pos; + char *buf = NULL; + virTypedParameterPtr ent; + int param_idx; + + /* return the number of supported params ? */ + if (nparams == 0 && ncpus != 0) + return 1; /* only cpu_time is supported */ + + /* return percpu cputime in index 0 */ + param_idx = 0; + /* to parse account file, we need "present" cpu map */ + map = nodeGetCPUmap(domain->conn, &max_id, "present"); + if (!map) + return rv; + + if (ncpus == 0) { /* returns max cpu ID */ + rv = max_id; + goto cleanup; + } + /* we get percpu cputime accounting info. */ + if (virCgroupGetCpuacctPercpuUsage(group, &buf)) + goto cleanup; + pos = buf; + + if (max_id > start_cpu + ncpus - 1) + max_id = start_cpu + ncpus - 1; + + for (i = 0; i <= max_id; i++) { + unsigned long long cpu_time; + + if (!map[i]) + continue; + 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 = &params[ (i - start_cpu) * nparams + param_idx]; + virTypedParameterAssign(ent, VIR_DOMAIN_CPU_STATS_CPUTIME, + VIR_TYPED_PARAM_ULLONG, cpu_time); + } + rv = param_idx + 1; +cleanup: + VIR_FREE(buf); + VIR_FREE(map); + return rv; +} + + +static int +qemuDomainGetCPUStats(virDomainPtr domain, + virTypedParameterPtr params, + unsigned int nparams, + int start_cpu, + unsigned int ncpus, + unsigned int flags) +{ + struct qemud_driver *driver = domain->conn->privateData; + virCgroupPtr group = NULL; + virDomainObjPtr vm = NULL; + int ret = -1; + bool isActive; + + virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1); + + qemuDriverLock(driver); + + vm = virDomainFindByUUID(&driver->domains, domain->uuid); + if (vm == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("No such domain %s"), domain->uuid); + goto cleanup; + } + + isActive = virDomainObjIsActive(vm); + if (!isActive) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain is not running")); + goto cleanup; + } + + if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUACCT)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cgroup CPUACCT controller is not mounted")); + goto cleanup; + } + + if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find cgroup for domain %s"), vm->def->name); + goto cleanup; + } + + if (nparams == 0 && ncpus == 0) /* returns max cpu id */ + ret = qemuDomainGetPercpuStats(domain, group, NULL, 0, 0, 0); + else if (start_cpu == -1) /* get total */ + ret = qemuDomainGetTotalcpuStats(group, params, nparams); + else + ret = qemuDomainGetPercpuStats(domain, group, params, nparams, + start_cpu, ncpus); +cleanup: + virCgroupFree(&group); + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} + static int qemuDomainPMSuspendForDuration(virDomainPtr dom, unsigned int target, @@ -12387,6 +12532,7 @@ static virDriver qemuDriver = { .domainGetMetadata = qemuDomainGetMetadata, /* 0.9.10 */ .domainPMSuspendForDuration = qemuDomainPMSuspendForDuration, /* 0.9.11 */ .domainPMWakeup = qemuDomainPMWakeup, /* 0.9.11 */ + .domainGetCPUStats = qemuDomainGetCPUStats, /* 0.9.11 */ }; diff --git a/src/util/cgroup.c b/src/util/cgroup.c index 15b870d..2d34c50 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -1555,6 +1555,12 @@ int virCgroupGetCpuacctUsage(virCgroupPtr group, unsigned long long *usage) "cpuacct.usage", usage); } +int virCgroupGetCpuacctPercpuUsage(virCgroupPtr group, char **usage) +{ + return virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_CPUACCT, + "cpuacct.usage_percpu", usage); +} + int virCgroupSetFreezerState(virCgroupPtr group, const char *state) { return virCgroupSetValueStr(group, diff --git a/src/util/cgroup.h b/src/util/cgroup.h index 8d75735..b4e0f37 100644 --- a/src/util/cgroup.h +++ b/src/util/cgroup.h @@ -115,6 +115,7 @@ int virCgroupSetCpuCfsQuota(virCgroupPtr group, long long cfs_quota); int virCgroupGetCpuCfsQuota(virCgroupPtr group, long long *cfs_quota); int virCgroupGetCpuacctUsage(virCgroupPtr group, unsigned long long *usage); +int virCgroupGetCpuacctPercpuUsage(virCgroupPtr group, char **usage); int virCgroupSetFreezerState(virCgroupPtr group, const char *state); int virCgroupGetFreezerState(virCgroupPtr group, char **state); -- 1.7.4.4

From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; Total: cpu_time 14.312 CPU0: cpu_time 3.253 CPU1: cpu_time 1.923 CPU2: cpu_time 7.424 CPU3: cpu_time 1.712 Changed from V5: add --all, --start, --count option Changed from V: rebase Signed-off-by: Lai Jiangshan <laijs(a)cn.fujitsu.com&gt; --- tools/virsh.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 8 +++ 2 files changed, 162 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index dc362dc..a431826 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -5537,6 +5537,159 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd) } /* + * "cpu-stats" command + */ +static const vshCmdInfo info_cpu_stats[] = { + {"help", N_("show domain cpu statistics")}, + {"desc", N_("Display statistics about the domain's CPUs, including per-CPU statistics.")}, + {NULL, NULL}, +}; + +static const vshCmdOptDef opts_cpu_stats[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"all", 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")}, + {NULL, 0, 0, NULL}, +}; + +static bool +cmdCPUStats(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + virTypedParameterPtr params = NULL; + int i, j, pos, max_id, cpu = -1, show_count = -1, nparams; + bool show_all = false, show_per_cpu = false; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + show_all = vshCommandOptBool(cmd, "all"); + if (vshCommandOptInt(cmd, "start", &cpu) > 0) + show_per_cpu = true; + if (vshCommandOptInt(cmd, "count", &show_count) > 0) + show_per_cpu = true; + + /* default show per_cpu and total */ + if (!show_all && !show_per_cpu) { + show_all = true; + show_per_cpu = true; + } + + if (!show_all) + goto do_show_per_cpu; + + /* get supported num of parameter for total statistics */ + if ((nparams = virDomainGetCPUStats(dom, NULL, 0, -1, 1, 0)) < 0) + goto failed_stats; + if (VIR_ALLOC_N(params, nparams)) + goto failed_params; + + /* passing start_cpu == -1 gives us domain's total status */ + if ((nparams = virDomainGetCPUStats(dom, params, nparams, -1, 1, 0)) < 0) + goto failed_stats; + + vshPrint(ctl, "Total:\n"); + for (i = 0; i < nparams; i++) { + vshPrint(ctl, "\t%-10s ", params[i].field); + if (STREQ(params[i].field, VIR_DOMAIN_CPU_STATS_CPUTIME)) { + if (params[i].type == VIR_TYPED_PARAM_ULLONG) { + vshPrint(ctl, "%12.3lf\n", + params[i].value.ul / 1000000000.0); + } else { + const char *s = vshGetTypedParamValue(ctl, &params[i]); + vshPrint(ctl, _("%s(ABI changed? ullong is expected)\n"), s); + VIR_FREE(s); + } + } else { + const char *s = vshGetTypedParamValue(ctl, &params[i]); + vshPrint(ctl, _("%s\n"), s); + VIR_FREE(s); + } + } + virTypedParameterArrayClear(params, nparams); + VIR_FREE(params); + + if (!show_per_cpu) /* show all stats only */ + goto cleanup; + +do_show_per_cpu: + /* check cpu, show_count, and ignore wrong argument */ + if (cpu < 0) + cpu = 0; + if (show_count < 0) + show_count = INT_MAX; + + /* get max cpu id on the node */ + if ((max_id = virDomainGetCPUStats(dom, NULL, 0, 0, 0, 0)) < 0) + goto failed_stats; + /* get percpu information */ + if ((nparams = virDomainGetCPUStats(dom, NULL, 0, -1, 1, 0)) < 0) + goto failed_stats; + + if (VIR_ALLOC_N(params, nparams * 128)) + goto failed_params; + + while (cpu <= max_id) { + int ncpus = 128; + + if (cpu + ncpus - 1 > max_id) /* id starts from 0. */ + ncpus = max_id + 1 - cpu; + + if (virDomainGetCPUStats(dom, params, nparams, cpu, ncpus, 0) < 0) + goto failed_stats; + + for (i = 0; i < ncpus; i++) { + if (params[i * nparams].type == 0) /* this cpu is not in the map */ + continue; + vshPrint(ctl, "CPU%d:\n", cpu + i); + + for (j = 0; j < nparams; j++) { + pos = i * nparams + j; + vshPrint(ctl, "\t%-10s ", params[pos].field); + if (STREQ(params[pos].field, VIR_DOMAIN_CPU_STATS_CPUTIME)) { + if (params[j].type == VIR_TYPED_PARAM_ULLONG) { + vshPrint(ctl, "%12.3lf\n", + params[pos].value.ul / 1000000000.0); + } else { + const char *s = vshGetTypedParamValue(ctl, &params[pos]); + vshPrint(ctl, _("%s(ABI changed? ullong is expected)\n"), s); + VIR_FREE(s); + } + } else { + const char *s = vshGetTypedParamValue(ctl, &params[pos]); + vshPrint(ctl, _("%s\n"), s); + VIR_FREE(s); + } + } + + if (--show_count <= 0) /* mark done to exit the outmost loop */ + cpu = max_id; + } + cpu += ncpus; + virTypedParameterArrayClear(params, nparams * ncpus); + } + VIR_FREE(params); +cleanup: + virDomainFree(dom); + return true; + +failed_params: + virReportOOMError(); + virDomainFree(dom); + return false; + +failed_stats: + vshError(ctl, _("Failed to virDomainGetCPUStats()\n")); + VIR_FREE(params); + virDomainFree(dom); + return false; +} + +/* * "inject-nmi" command */ static const vshCmdInfo info_inject_nmi[] = { @@ -16910,6 +17063,7 @@ static const vshCmdDef domManagementCmds[] = { #endif {"cpu-baseline", cmdCPUBaseline, opts_cpu_baseline, info_cpu_baseline, 0}, {"cpu-compare", cmdCPUCompare, opts_cpu_compare, info_cpu_compare, 0}, + {"cpu-stats", cmdCPUStats, opts_cpu_stats, info_cpu_stats, 0}, {"create", cmdCreate, opts_create, info_create, 0}, {"define", cmdDefine, opts_define, info_define, 0}, {"desc", cmdDesc, opts_desc, info_desc, 0}, diff --git a/tools/virsh.pod b/tools/virsh.pod index 8f6a2d6..b23868d 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -790,6 +790,14 @@ Provide the maximum number of virtual CPUs supported for a guest VM on this connection. If provided, the I<type> parameter must be a valid type attribute for the <domain> element of XML. +=item B<cpu-stats> I<domain> [I<--all>] [I<start>] [I<count>] + +Provide cpu statistics information of a domain. The domain should +be running. Default it shows stats for all CPUs, and a total. Use +I<--all> for only the total stats, I<start> for only the per-cpu +stats of the CPUs from I<start>, I<count> for only I<count> CPUs' +stats. + =item B<migrate> [I<--live>] [I<--direct>] [I<--p2p> [I<--tunnelled>]] [I<--persistent>] [I<--undefinesource>] [I<--suspend>] [I<--copy-storage-all>] [I<--copy-storage-inc>] [I<--change-protection>] [I<--unsafe>] [I<--verbose>] -- 1.7.4.4

Signed-off-by: Lai Jiangshan <laijs(a)cn.fujitsu.com&gt; --- src/libvirt.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 8035add..a55d823 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18541,7 +18541,7 @@ int virDomainGetCPUStats(virDomainPtr domain, (start_cpu == -1 && ncpus != 1) || ((params == NULL) != (nparams == 0)) || (ncpus == 0 && params != NULL) || - ncpus < UINT_MAX / nparams) { + (nparams && ncpus > UINT_MAX / nparams)) { virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); goto error; } -- 1.7.4.4

On 02/09/2012 03:43 AM, Lai Jiangshan wrote: s/Separeted/Separated/ and again Sorting. s/functin/function/ How is this function different from virDomainCpuSetParse in domain_conf.c? That function parses out a cpuset rather than a bitmap, but the syntax string being parsed looks to be a superset of what you are parsing here. Maybe it's more efficient to write a generic conversion from cpumap to bitset, and reuse the existing parser. I'll have to see how you actually use the parsed bitset later in the series to see which approach might be best. Are we sure that is long enough? Suppose I have a machine with 512 cores - I can come up with a cpumap that exceeds 1024 bytes. max_id has to be marked unused, to avoid gcc warnings on non-Linux compilation. same for mapname. s/represetned/represented/ const char *buf, since we aren't modifying it. const char *pos, since buf is const. Yeah, I'm definitely starting to think that reusing our existing cpumap parser, then a loop that converts bytes of the cpumap into longs of a virBitmap, might be a more reusable prospect. s/parese/parse/; s/separeted/separated/ ATTRIBUTE_NONNULL(1), and decide whether max_bit can be NULL (if so, fix the code to allow it; if not, add the attribute) If we go with my reuse suggestion, this would be: virBitmapPtr virBitmapCreateFromCpumap(char *cpumap, int maplen, int *max_bit) -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 01/27/2012 11:21 PM, KAMEZAWA Hiroyuki wrote: s/dimentional/dimensional/ Odd that the diffstat listed twice. I'm going to reorder my review, to start with the .x file. We should enforce the two limits independently, which means we also need two smaller constants (it's a shame that rpcgen doesn't allow products when computing a constant, but we can document how we arrive at various numbers). We can reuse VIR_NODE_CPU_STATS_MAX for one of the two limits. Looks good. autogen */ skipgen */ Trivial conflict resolution. Use the constants from the .x file, rather than open-coding things. Oops, I just realized that I mentioned that libvirt.c should not have a length check, but then I forgot to delete it. Meanwhile, even if libvirt.c does not have a length check, it still needs to have a multiplication overflow check. s/callar/caller/ We can still use remoteSerializeTypedParameters; we just have to teach it to skip zero entries like it already can skip string entries. Oh, I just realized that to support string entries, we have to support a flags of VIR_TYPED_PARAM_STRING_OKAY. Cleanup is a lot simpler if we let the existing serialization skip zero entries. This side needs to validate incoming parameters before sending over the wire. Hmm, remoteNodeGetCPUStats needs to do likewise. I prefer sizeof(expr) over sizeof(type), in case expr ever changes types. Again, open coding things is not nice. If we write the server to never send zero entries (which is a good thing - less data over the wire), then the client needs to re-create zero entries by calling deserialization in a loop - deserialize ret.nparams entries at every nparams slot. Potential free of bogus memory if anything can reach cleanup with rv < 0 but before we sanitized the contents of params. Here's what I'm squashing in: diff --git i/daemon/remote.c w/daemon/remote.c index 15857a8..cb8423a 100644 --- i/daemon/remote.c +++ w/daemon/remote.c @@ -704,8 +704,11 @@ remoteSerializeTypedParameters(virTypedParameterPtr params, } for (i = 0, j = 0; i < nparams; ++i) { - if (!(flags & VIR_TYPED_PARAM_STRING_OKAY) && - params[i].type == VIR_TYPED_PARAM_STRING) { + /* virDomainGetCPUStats can return a sparse array; also, we + * can't pass back strings to older clients. */ + if (!params[i].type || + (!(flags & VIR_TYPED_PARAM_STRING_OKAY) && + params[i].type == VIR_TYPED_PARAM_STRING)) { --*ret_params_len; continue; } @@ -3534,10 +3537,7 @@ remoteDispatchDomainGetCPUStats(virNetServerPtr server ATTRIBUTE_UNUSED, virDomainPtr dom = NULL; struct daemonClientPrivate *priv; virTypedParameterPtr params = NULL; - remote_typed_param *info = NULL; - int cpu, idx, src, dest; int rv = -1; - unsigned int return_size = 0; int percpu_len = 0; priv = virNetServerClientGetPrivateData(client); @@ -3546,128 +3546,53 @@ remoteDispatchDomainGetCPUStats(virNetServerPtr server ATTRIBUTE_UNUSED, goto cleanup; } - /* - * nparams * ncpus should be smaller than - * REMOTE_DOMAIN_GET_CPU_STATS_MAX but nparams - * and ncpus are limited by API. check it. - */ - if (args->nparams > 16) { + if (args->nparams > REMOTE_NODE_CPU_STATS_MAX) { virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); goto cleanup; } - if (args->ncpus > 128) { + if (args->ncpus > REMOTE_DOMAIN_GET_CPU_STATS_NCPUS_MAX) { virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("ncpus too large")); goto cleanup; } - if (args->nparams > 0) { - if (VIR_ALLOC_N(params, args->ncpus * args->nparams) < 0) - goto no_memory; + if (args->nparams > 0 && + VIR_ALLOC_N(params, args->ncpus * args->nparams) < 0) { + virReportOOMError(); + goto cleanup; } if (!(dom = get_nonnull_domain(priv->conn, args->dom))) goto cleanup; percpu_len = virDomainGetCPUStats(dom, params, args->nparams, - args->start_cpu, args->ncpus, args->flags); + args->start_cpu, args->ncpus, + args->flags); if (percpu_len < 0) goto cleanup; - /* If nparams == 0, the function returns a sigle value */ + /* If nparams == 0, the function returns a single value */ if (args->nparams == 0) goto success; - /* - * Here, we return values based on the real param size returned by - * a driver rather than the passed one. RPC Client stub should decode - * it to fit callar's expectation. - * - * If cpu ID is sparse, The whole array for some cpu IDs will be - * zero-cleared. This doesn't meet to remoteSerializeTypedParameters() - * and we do serialization by ourselves. - */ - return_size = percpu_len * args->ncpus; - if (VIR_ALLOC_N(info, return_size) < 0) - goto no_memory; + if (remoteSerializeTypedParameters(params, args->nparams * args->ncpus, + &ret->params.params_val, + &ret->params.params_len, + args->flags) < 0) + goto cleanup; + + percpu_len = ret->params.params_len / args->ncpus; - for (cpu = 0; cpu < args->ncpus; ++cpu) { - for (idx = 0; idx < percpu_len; ++idx) { - src = cpu * args->nparams + idx; - dest = cpu * percpu_len + idx; - - /* If CPU ID is discontiguous, this can happen */ - if (params[src].type == 0) - continue; - - info[dest].field = strdup(params[src].field); - if (info[dest].field == NULL) - goto no_memory; - - info[dest].value.type = params[src].type; - - switch (params[src].type) { - case VIR_TYPED_PARAM_INT: - info[dest].value.remote_typed_param_value_u.i = - params[src].value.i; - break; - case VIR_TYPED_PARAM_UINT: - info[dest].value.remote_typed_param_value_u.ui = - params[src].value.ui; - break; - case VIR_TYPED_PARAM_LLONG: - info[dest].value.remote_typed_param_value_u.l = - params[src].value.l; - break; - case VIR_TYPED_PARAM_ULLONG: - info[dest].value.remote_typed_param_value_u.ul = - params[src].value.ul; - break; - case VIR_TYPED_PARAM_DOUBLE: - info[dest].value.remote_typed_param_value_u.d = - params[src].value.d; - break; - case VIR_TYPED_PARAM_BOOLEAN: - info[dest].value.remote_typed_param_value_u.b = - params[src].value.b; - break; - case VIR_TYPED_PARAM_STRING: - info[dest].value.remote_typed_param_value_u.s = - strdup(params[src].value.s); - if (info[dest].value.remote_typed_param_value_u.s == NULL) - goto no_memory; - break; - default: - virNetError(VIR_ERR_RPC, _("unknown parameter type: %d"), - params[src].type); - goto cleanup; - } - } - } success: rv = 0; - ret->params.params_len = return_size; - ret->params.params_val = info; ret->nparams = percpu_len; + cleanup: - if (rv < 0) { + if (rv < 0) virNetMessageSaveError(rerr); - if (info) { - int i; - for (i = 0; i < return_size; i++) { - VIR_FREE(info[i].field); - if (info[i].value.type == VIR_TYPED_PARAM_STRING) - VIR_FREE(info[i].value.remote_typed_param_value_u.s); - } - VIR_FREE(info); - } - } virTypedParameterArrayClear(params, args->ncpus * args->nparams); VIR_FREE(params); if (dom) virDomainFree(dom); return rv; -no_memory: - virReportOOMError(); - goto cleanup; } /*----- Helpers. -----*/ diff --git i/src/libvirt.c w/src/libvirt.c index 7060f5a..e702a34 100644 --- i/src/libvirt.c +++ w/src/libvirt.c @@ -18162,7 +18162,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: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of virTypedParameterFlags * * Get statistics relating to CPU usage attributable to a single * domain (in contrast to the statistics returned by @@ -18251,20 +18251,19 @@ int virDomainGetCPUStats(virDomainPtr domain, * if start_cpu is -1, ncpus must be 1 * params == NULL must match nparams == 0 * ncpus must be non-zero unless params == NULL + * nparams * ncpus must not overflow (RPC may restrict it even more) */ if (start_cpu < -1 || (start_cpu == -1 && ncpus != 1) || ((params == NULL) != (nparams == 0)) || - (ncpus == 0 && params != NULL)) { - virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); - goto error; - } - - /* remote protocol doesn't welcome big args in one shot */ - if ((nparams > 16) || (ncpus > 128)) { + (ncpus == 0 && params != NULL) || + ncpus < UINT_MAX / nparams) { virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); goto error; } + if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, + VIR_DRV_FEATURE_TYPED_PARAM_STRING)) + flags |= VIR_TYPED_PARAM_STRING_OKAY; if (conn->driver->domainGetCPUStats) { int ret; diff --git i/src/remote/remote_driver.c w/src/remote/remote_driver.c index a3ca63e..2312cf9 100644 --- i/src/remote/remote_driver.c +++ w/src/remote/remote_driver.c @@ -2,7 +2,7 @@ * remote_driver.c: driver to provide access to libvirtd running * on a remote machine * - * Copyright (C) 2007-2011 Red Hat, Inc. + * Copyright (C) 2007-2012 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -2315,12 +2315,24 @@ static int remoteDomainGetCPUStats(virDomainPtr domain, struct private_data *priv = domain->conn->privateData; remote_domain_get_cpu_stats_args args; remote_domain_get_cpu_stats_ret ret; - remote_typed_param *info; int rv = -1; - int cpu, idx, src, dest; + int cpu; remoteDriverLock(priv); + if (nparams > REMOTE_NODE_CPU_STATS_MAX) { + remoteError(VIR_ERR_RPC, + _("nparams count exceeds maximum: %u > %u"), + nparams, REMOTE_NODE_CPU_STATS_MAX); + goto done; + } + if (ncpus > REMOTE_DOMAIN_GET_CPU_STATS_NCPUS_MAX) { + remoteError(VIR_ERR_RPC, + _("ncpus count exceeds maximum: %u > %u"), + ncpus, REMOTE_DOMAIN_GET_CPU_STATS_NCPUS_MAX); + goto done; + } + make_nonnull_domain(&args.dom, domain); args.nparams = nparams; args.start_cpu = start_cpu; @@ -2336,67 +2348,38 @@ static int remoteDomainGetCPUStats(virDomainPtr domain, (char *) &ret) == -1) goto done; + /* Check the length of the returned list carefully. */ + if (ret.params.params_len > nparams * ncpus || + (ret.params.params_len && + ret.nparams * ncpus != ret.params.params_len)) { + remoteError(VIR_ERR_RPC, "%s", + _("remoteDomainGetCPUStats: " + "returned number of stats exceeds limit")); + memset(params, 0, sizeof(*params) * nparams * ncpus); + goto cleanup; + } + + /* Handle the case when the caller does not know the number of stats + * and is asking for the number of stats supported + */ if (nparams == 0) { rv = ret.nparams; goto cleanup; } - /* - * the returned arrray's size is not same to nparams * ncpus. And - * if cpu ID is not contiguous, all-zero entries can be found. + + /* The remote side did not send back any zero entries, so we have + * to expand things back into a possibly sparse array. */ - memset(params, 0, sizeof(virTypedParameter) * nparams * ncpus); - - /* Here, ret.nparams is always smaller than nparams */ - info = ret.params.params_val; - - for (cpu = 0; cpu < ncpus; ++cpu) { - for (idx = 0; idx < ret.nparams; ++idx) { - src = cpu * ret.nparams + idx; - dest = cpu * nparams + idx; - - if (info[src].value.type == 0) /* skip zeroed ones */ - continue; - - params[dest].type = info[src].value.type; - strcpy(params[dest].field, info[src].field); - - switch (params[dest].type) { - case VIR_TYPED_PARAM_INT: - params[dest].value.i = - info[src].value.remote_typed_param_value_u.i; - break; - case VIR_TYPED_PARAM_UINT: - params[dest].value.ui = - info[src].value.remote_typed_param_value_u.ui; - break; - case VIR_TYPED_PARAM_LLONG: - params[dest].value.l = - info[src].value.remote_typed_param_value_u.l; - break; - case VIR_TYPED_PARAM_ULLONG: - params[dest].value.ul = - info[src].value.remote_typed_param_value_u.ul; - break; - case VIR_TYPED_PARAM_DOUBLE: - params[dest].value.d = - info[src].value.remote_typed_param_value_u.d; - break; - case VIR_TYPED_PARAM_BOOLEAN: - params[dest].value.b = - info[src].value.remote_typed_param_value_u.b; - break; - case VIR_TYPED_PARAM_STRING: - params[dest].value.s = - strdup(info[src].value.remote_typed_param_value_u.s); - if (params[dest].value.s == NULL) - goto out_of_memory; - break; - default: - remoteError(VIR_ERR_RPC, _("unknown parameter type: %d"), - params[dest].type); - goto cleanup; - } - } + memset(params, 0, sizeof(*params) * nparams * ncpus); + for (cpu = 0; cpu < ncpus; cpu++) { + int tmp = nparams; + remote_typed_param *stride = &ret.params.params_val[cpu * ret.nparams]; + + if (remoteDeserializeTypedParameters(stride, ret.nparams, + REMOTE_NODE_CPU_STATS_MAX, + &params[cpu * nparams], + &tmp) < 0) + goto cleanup; } rv = ret.nparams; @@ -2415,9 +2398,6 @@ cleanup: done: remoteDriverUnlock(priv); return rv; -out_of_memory: - virReportOOMError(); - goto cleanup; } diff --git i/src/remote/remote_protocol.x w/src/remote/remote_protocol.x index 0bd399c..b58925a 100644 --- i/src/remote/remote_protocol.x +++ w/src/remote/remote_protocol.x @@ -209,8 +209,13 @@ const REMOTE_DOMAIN_SEND_KEY_MAX = 16; const REMOTE_DOMAIN_INTERFACE_PARAMETERS_MAX = 16; /* - * Upper limit on list of (real) cpu parameters - * nparams(<=16) * ncpus(<=128) <= 2048. + * Upper limit on cpus involved in per-cpu stats + */ +const REMOTE_DOMAIN_GET_CPU_STATS_NCPUS_MAX = 128; + +/* + * Upper limit on list of per-cpu stats: + * REMOTE_NODE_CPU_STATS_MAX * REMOTE_DOMAIN_GET_CPU_STATS_MAX */ const REMOTE_DOMAIN_GET_CPU_STATS_MAX = 2048; -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

add nodeGetCPUmap() for getting available CPU IDs in a bitmap. add virBitmapParseCommaSeparetedFormat() for parsing bitmap in comma separeted ascii format. This format of bitmap is used in Linux sysfs and cpuset. * cpuacct's percpu usage information is provided based on /sys/devices/system/cpu/present cpu map. So, this kind of call is required. This routine itself may be useful for other purpose. libvirt_private.syms | 2 + nodeinfo.c | 51 ++++++++++++++++++++++++++++++++++++++++ nodeinfo.h | 4 +++ util/bitmap.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++ util/bitmap.h | 6 +++- 5 files changed, 127 insertions(+), 1 deletion(-) --- src/libvirt_private.syms | 2 + src/nodeinfo.c | 51 ++++++++++++++++++++++++++++++++++++ src/nodeinfo.h | 4 +++ src/util/bitmap.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/bitmap.h | 6 +++- 5 files changed, 127 insertions(+), 1 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 915a43f..fd44322 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -17,6 +17,7 @@ virBitmapFree; virBitmapGetBit; virBitmapSetBit; virBitmapString; +virBitmapParseCommaSeparatedFormat; # buf.h @@ -800,6 +801,7 @@ nodeGetCellsFreeMemory; nodeGetFreeMemory; nodeGetInfo; nodeGetMemoryStats; +nodeGetCPUmap; # nwfilter_conf.h diff --git a/src/nodeinfo.c b/src/nodeinfo.c index e0b66f7..92bada7 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -47,6 +47,7 @@ #include "count-one-bits.h" #include "intprops.h" #include "virfile.h" +#include "bitmap.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -569,6 +570,33 @@ 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 functin parse + * it and returns bitmap. + */ +static virBitmapPtr linuxParseCPUmap(int *max_cpuid, const char *path) +{ + char str[1024]; + FILE *fp; + virBitmapPtr map = NULL; + + fp = fopen(path, "r"); + if (!fp) { + virReportSystemError(errno, _("cannot open %s"), path); + goto cleanup; + } + if (fgets(str, sizeof(str), fp) == NULL) { + virReportSystemError(errno, _("cannot read from %s"), path); + goto cleanup; + } + map = virBitmapParseCommaSeparatedFormat(str, max_cpuid); + +cleanup: + VIR_FORCE_FCLOSE(fp); + return map; +} #endif int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo) { @@ -712,6 +740,29 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED, #endif } +virBitmapPtr nodeGetCPUmap(virConnectPtr conn ATTRIBUTE_UNUSED, + int *max_id, + const char *mapname) +{ +#ifdef __linux__ + char *path; + virBitmapPtr map; + + if (virAsprintf(&path, CPU_SYS_PATH "/%s", mapname) < 0) { + virReportOOMError(); + return NULL; + } + + map = linuxParseCPUmap(max_id, path); + VIR_FREE(path); + return map; +#else + nodeReportError(VIR_ERR_NO_SUPPORT, "%s", + _("node cpumap not implemented on this platform")); + return -1; +#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..7f26b77 100644 --- a/src/nodeinfo.h +++ b/src/nodeinfo.h @@ -26,6 +26,7 @@ # include "libvirt/libvirt.h" # include "capabilities.h" +# include "bitmap.h" int nodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo); int nodeCapsInitNUMA(virCapsPtr caps); @@ -46,4 +47,7 @@ int nodeGetCellsFreeMemory(virConnectPtr conn, int maxCells); unsigned long long nodeGetFreeMemory(virConnectPtr conn); +virBitmapPtr nodeGetCPUmap(virConnectPtr conn, + int *max_id, + const char *mapname); #endif /* __VIR_NODEINFO_H__*/ diff --git a/src/util/bitmap.c b/src/util/bitmap.c index 8c326c4..c7e1a81 100644 --- a/src/util/bitmap.c +++ b/src/util/bitmap.c @@ -33,6 +33,11 @@ #include "bitmap.h" #include "memory.h" #include "buf.h" +#include "c-ctype.h" +#include "util.h" +#include "virterror_internal.h" + +#define VIR_FROM_THIS VIR_FROM_NONE struct _virBitmap { @@ -180,3 +185,63 @@ char *virBitmapString(virBitmapPtr bitmap) return virBufferContentAndReset(&buf); } + +/** + * virBitmapParseCommaSeparatedFormat: + * + * When bitmap is printed in ascii format, expecially in Linux, + * comma-separated format is sometimes used. For example, a bitmap 11111011 is + * represetned as 0-4,6-7. This function parse comma-separated format + * and returns virBitmap. Found max bit is returend, too. + * + * This functions stops if characters other than digits, ',', '-' are + * found. This function will be useful for parsing linux's bitmap. + */ +virBitmapPtr virBitmapParseCommaSeparatedFormat(char *buf, int *max_bit) +{ + char *pos = buf; + virBitmapPtr map = NULL; + int val, x; + + /* at first, find the highest number */ + val = 0; + while ((*pos != '\n') || (*pos != 0)) { + if (c_isdigit(*pos)) { + virStrToLong_i(pos, &pos, 10, &val); + } else if ((*pos == ',') || (*pos == '-')) { + ++pos; + } else + break; + } + *max_bit = val; + + map = virBitmapAlloc(val + 1); + if (map == NULL) { /* we may return NULL at failure of parsing */ + virReportOOMError(); + return NULL; + } + + pos = buf; + while ((*pos != '\n') || (*pos != 0)) { + if (c_isdigit(*pos)) { + virStrToLong_i(pos, &pos, 10, &val); + if (virBitmapSetBit(map, val) < 0) + goto failed; + } else if (*pos == ',') { + ++pos; + } else if (*pos == '-') { + x = val; + pos++; + virStrToLong_i(pos, &pos, 10, &val); + for (;x <= val; ++x) { + if (virBitmapSetBit(map, x) < 0) + goto failed; + } + } else + break; + } + return map; +failed: + virBitmapFree(map); + return NULL; +} diff --git a/src/util/bitmap.h b/src/util/bitmap.h index ef62cca..e3d3e2b 100644 --- a/src/util/bitmap.h +++ b/src/util/bitmap.h @@ -61,5 +61,9 @@ int virBitmapGetBit(virBitmapPtr bitmap, size_t b, bool *result) char *virBitmapString(virBitmapPtr bitmap) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; - +/* + * parese comma-separeted bitmap format and allocate virBitmap. + */ +virBitmapPtr virBitmapParseCommaSeparatedFormat(char *buf, int *max_bit) + ATTRIBUTE_RETURN_CHECK; #endif -- 1.7.4.1

On Tue, 31 Jan 2012 17:58:17 +0800 Hu Tao <hutao(a)cn.fujitsu.com&gt; wrote: will fix. Ah, yes. Fixed one here. == From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; Date: Fri, 27 Jan 2012 19:38:24 +0900 Subject: [PATCH] add nodeGetCPUmap() for getting available CPU IDs in a bitmap. add virBitmapParseCommaSeparetedFormat() for parsing bitmap in comma separeted ascii format. This format of bitmap is used in Linux sysfs and cpuset. Changelog: - fixed typos. - fixed string scan routine. Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; libvirt_private.syms | 2 + nodeinfo.c | 51 ++++++++++++++++++++++++++++++++++++++++ nodeinfo.h | 4 +++ util/bitmap.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++ util/bitmap.h | 6 +++- 5 files changed, 127 insertions(+), 1 deletion(-) --- src/libvirt_private.syms | 2 + src/nodeinfo.c | 51 ++++++++++++++++++++++++++++++++++++ src/nodeinfo.h | 4 +++ src/util/bitmap.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/bitmap.h | 6 +++- 5 files changed, 127 insertions(+), 1 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e300f06..f6fc5d2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -17,6 +17,7 @@ virBitmapFree; virBitmapGetBit; virBitmapSetBit; virBitmapString; +virBitmapParseCommaSeparatedFormat; # buf.h @@ -801,6 +802,7 @@ nodeGetCellsFreeMemory; nodeGetFreeMemory; nodeGetInfo; nodeGetMemoryStats; +nodeGetCPUmap; # nwfilter_conf.h diff --git a/src/nodeinfo.c b/src/nodeinfo.c index e0b66f7..b0ebb88 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -47,6 +47,7 @@ #include "count-one-bits.h" #include "intprops.h" #include "virfile.h" +#include "bitmap.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -569,6 +570,33 @@ 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 functin parses + * it and returns bitmap. + */ +static virBitmapPtr linuxParseCPUmap(int *max_cpuid, const char *path) +{ + char str[1024]; + FILE *fp; + virBitmapPtr map = NULL; + + fp = fopen(path, "r"); + if (!fp) { + virReportSystemError(errno, _("cannot open %s"), path); + goto cleanup; + } + if (fgets(str, sizeof(str), fp) == NULL) { + virReportSystemError(errno, _("cannot read from %s"), path); + goto cleanup; + } + map = virBitmapParseCommaSeparatedFormat(str, max_cpuid); + +cleanup: + VIR_FORCE_FCLOSE(fp); + return map; +} #endif int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo) { @@ -712,6 +740,29 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED, #endif } +virBitmapPtr nodeGetCPUmap(virConnectPtr conn ATTRIBUTE_UNUSED, + int *max_id, + const char *mapname) +{ +#ifdef __linux__ + char *path; + virBitmapPtr map; + + if (virAsprintf(&path, CPU_SYS_PATH "/%s", mapname) < 0) { + virReportOOMError(); + return NULL; + } + + map = linuxParseCPUmap(max_id, path); + VIR_FREE(path); + return map; +#else + nodeReportError(VIR_ERR_NO_SUPPORT, "%s", + _("node cpumap not implemented on this platform")); + return -1; +#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..7f26b77 100644 --- a/src/nodeinfo.h +++ b/src/nodeinfo.h @@ -26,6 +26,7 @@ # include "libvirt/libvirt.h" # include "capabilities.h" +# include "bitmap.h" int nodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo); int nodeCapsInitNUMA(virCapsPtr caps); @@ -46,4 +47,7 @@ int nodeGetCellsFreeMemory(virConnectPtr conn, int maxCells); unsigned long long nodeGetFreeMemory(virConnectPtr conn); +virBitmapPtr nodeGetCPUmap(virConnectPtr conn, + int *max_id, + const char *mapname); #endif /* __VIR_NODEINFO_H__*/ diff --git a/src/util/bitmap.c b/src/util/bitmap.c index 8c326c4..6dead3b 100644 --- a/src/util/bitmap.c +++ b/src/util/bitmap.c @@ -33,6 +33,11 @@ #include "bitmap.h" #include "memory.h" #include "buf.h" +#include "c-ctype.h" +#include "util.h" +#include "virterror_internal.h" + +#define VIR_FROM_THIS VIR_FROM_NONE struct _virBitmap { @@ -180,3 +185,63 @@ char *virBitmapString(virBitmapPtr bitmap) return virBufferContentAndReset(&buf); } + +/** + * virBitmapParseCommaSeparatedFormat: + * + * When bitmap is printed in ascii format, especially in Linux, + * comma-separated format is sometimes used. For example, a bitmap 11111011 is + * represetned as 0-4,6-7. This function parses comma-separated format + * and returns virBitmap. Found max bit is returned, too. + * + * This functions stops if characters other than digits, ',', '-' are + * found. This function will be useful for parsing linux's bitmap. + */ +virBitmapPtr virBitmapParseCommaSeparatedFormat(char *buf, int *max_bit) +{ + char *pos = buf; + virBitmapPtr map = NULL; + int val, x; + + /* at first, find the highest number */ + val = 0; + while ((*pos != '\n') && (*pos != 0)) { + if (c_isdigit(*pos)) { + virStrToLong_i(pos, &pos, 10, &val); + } else if ((*pos == ',') || (*pos == '-')) { + ++pos; + } else + break; + } + *max_bit = val; + + map = virBitmapAlloc(val + 1); + if (map == NULL) { /* we may return NULL at failure of parsing */ + virReportOOMError(); + return NULL; + } + + pos = buf; + while ((*pos != '\n') && (*pos != 0)) { + if (c_isdigit(*pos)) { + virStrToLong_i(pos, &pos, 10, &val); + if (virBitmapSetBit(map, val) < 0) + goto failed; + } else if (*pos == ',') { + ++pos; + } else if (*pos == '-') { + x = val; + pos++; + virStrToLong_i(pos, &pos, 10, &val); + for (;x <= val; ++x) { + if (virBitmapSetBit(map, x) < 0) + goto failed; + } + } else + break; + } + return map; +failed: + virBitmapFree(map); + return NULL; +} diff --git a/src/util/bitmap.h b/src/util/bitmap.h index ef62cca..e3d3e2b 100644 --- a/src/util/bitmap.h +++ b/src/util/bitmap.h @@ -61,5 +61,9 @@ int virBitmapGetBit(virBitmapPtr bitmap, size_t b, bool *result) char *virBitmapString(virBitmapPtr bitmap) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; - +/* + * parese comma-separeted bitmap format and allocate virBitmap. + */ +virBitmapPtr virBitmapParseCommaSeparatedFormat(char *buf, int *max_bit) + ATTRIBUTE_RETURN_CHECK; #endif -- 1.7.4.1

cpu-accts command shows cpu accounting information of a domain. [root@bluextal ~]# virsh cpu-accts GuestX1 Total: cpu_time 165.4 CPU0: cpu_time 0.0 CPU1: cpu_time 0.0 CPU2: cpu_time 0.0 CPU3: cpu_time 0.0 CPU4: cpu_time 78.9 CPU5: cpu_time 0.0 CPU6: cpu_time 0.0 CPU7: cpu_time 86.5 This guest using 165secs of cpu in total. CPU4/7 is used mainly. --- tools/virsh.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 5 ++ 2 files changed, 121 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 74655c2..9746500 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -5106,6 +5106,121 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd) } /* + * "cputime" command + */ +static const vshCmdInfo info_cpu_accts[] = { + {"help", N_("show domain cpu accounting information")}, + {"desc", N_("Returns information about the domain's cpu accounting")}, + {NULL, NULL}, +}; + +static const vshCmdOptDef opts_cpu_accts[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {NULL, 0, 0, NULL}, +}; + +static bool +cmdCPUAccts(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + virTypedParameterPtr params = NULL; + bool ret = true; + int i, j, pos, cpu, nparams; + int max_id; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + /* get max cpu id on the node */ + max_id = virDomainGetCPUStats(dom, NULL, 0, 0, 0, 0); + if (max_id < 0) { + vshPrint(ctl, "max id %d\n", max_id); + goto cleanup; + } + /* get supported num of parameter for total statistics */ + nparams = virDomainGetCPUStats(dom, NULL, 0, -1, 1, 0); + if (nparams < 0) + goto cleanup; + + if (VIR_ALLOC_N(params, nparams)) { + virReportOOMError(); + goto cleanup; + } + /* passing start_cpu == -1 gives us domain's total status */ + nparams = virDomainGetCPUStats(dom, params, nparams, -1, 1, 0); + if (nparams < 0) + goto cleanup; + + vshPrint(ctl, "Total:\n"); + for (i = 0; i < nparams; i++) { + vshPrint(ctl, "\t%-15s ", params[i].field); + switch (params[i].type) { + case VIR_TYPED_PARAM_ULLONG: + /* Now all UULONG param is used for time accounting in ns */ + vshPrint(ctl, "%.1lf\n", + params[i].value.ul / 1000000000.0); + break; + default: + vshError(ctl, "%s", _("API mismatch?")); + goto cleanup; + } + } + VIR_FREE(params); + /* get percpu information */ + nparams = virDomainGetCPUStats(dom, NULL, 0, -1, 1, 0); + if (nparams < 0) + goto cleanup; + + cpu = 0; + while (cpu <= max_id) { + int ncpus = 128; + + if (cpu + ncpus - 1 > max_id) /* id starts from 0. */ + ncpus = max_id + 1 - cpu; + + if (VIR_ALLOC_N(params, nparams * ncpus)) { + virReportOOMError(); + goto cleanup; + } + + if (virDomainGetCPUStats(dom, params, nparams, cpu, ncpus, 0) < 0) + goto cleanup; + + for (i = 0; i < ncpus; i++) { + if (params[i * nparams].type == 0) + continue; + vshPrint(ctl, "CPU%d:\n", cpu + i); + + for (j = 0; j < nparams; j++) { + pos = i * nparams + j; + if (params[pos].type == 0) + continue; + vshPrint(ctl, "\t%-15s ", params[pos].field); + switch(params[j].type) { + case VIR_TYPED_PARAM_ULLONG: + vshPrint(ctl, "%.1lf\n", + params[pos].value.ul / 1000000000.0); + break; + default: + vshError(ctl, "%s", _("API mismatch?")); + goto cleanup; + } + } + } + cpu += ncpus; + /* Note: If we handle string type, it should be freed */ + VIR_FREE(params); + } +cleanup: + VIR_FREE(params); + virDomainFree(dom); + return ret; +} + +/* * "inject-nmi" command */ static const vshCmdInfo info_inject_nmi[] = { @@ -16014,6 +16129,7 @@ static const vshCmdDef domManagementCmds[] = { #endif {"cpu-baseline", cmdCPUBaseline, opts_cpu_baseline, info_cpu_baseline, 0}, {"cpu-compare", cmdCPUCompare, opts_cpu_compare, info_cpu_compare, 0}, + {"cpu-accts", cmdCPUAccts, opts_cpu_accts, info_cpu_accts, 0}, {"create", cmdCreate, opts_create, info_create, 0}, {"define", cmdDefine, opts_define, info_define, 0}, {"destroy", cmdDestroy, opts_destroy, info_destroy, 0}, diff --git a/tools/virsh.pod b/tools/virsh.pod index 8599f66..df65935 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -721,6 +721,11 @@ Provide the maximum number of virtual CPUs supported for a guest VM on this connection. If provided, the I<type> parameter must be a valid type attribute for the <domain> element of XML. +=item B<cpu-accts> I<domain> + +Provide cpu accounting information of a domain. The domain should +be running. + =item B<migrate> [I<--live>] [I<--direct>] [I<--p2p> [I<--tunnelled>]] [I<--persistent>] [I<--undefinesource>] [I<--suspend>] [I<--copy-storage-all>] [I<--copy-storage-inc>] [I<--change-protection>] [I<--verbose>] -- 1.7.4.1