After refactorings of this patch virCgroupGetPercpuStats has little to do with
cgroups. As already mentioned in commit message it mostly adapts raw cpu data
to storage used in API. So I want to move this function elsewhere together
with virCgroupCpuStatsPtr structure. I just can find proper header/source
to use and prefix too.
Actually I want to reuse this adapting function in vz driver in the process
of implementing virDomainGetCPUStats and don't want to include vircgroup.h
there.
Any ideas?
On 02.02.2017 14:09, Nikolay Shirokovskiy wrote:
> virCgroupGetPercpuStats do 2 things. First it extracts per cpu stats
> from cgroups, second it puts stats values into virTypedParameterPtr in
> accordance with virDomainGetCPUStats interface. As we need first
> function in order to prodive per cpus stats in virConnectGetAllDomainStats
> lets split these two functions.
> ---
> src/libvirt_private.syms | 2 +
> src/util/vircgroup.c | 140 +++++++++++++++++++++++++++++++++++------------
> src/util/vircgroup.h | 17 ++++++
> 3 files changed, 124 insertions(+), 35 deletions(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 8e994c7..e05335e 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1312,6 +1312,7 @@ virCgroupBindMount;
> virCgroupControllerAvailable;
> virCgroupControllerTypeFromString;
> virCgroupControllerTypeToString;
> +virCgroupCpuStatsFree;
> virCgroupDelThread;
> virCgroupDenyAllDevices;
> virCgroupDenyDevice;
> @@ -1335,6 +1336,7 @@ virCgroupGetCpusetCpus;
> virCgroupGetCpusetMemoryMigrate;
> virCgroupGetCpusetMems;
> virCgroupGetCpuShares;
> +virCgroupGetCpuStats;
> virCgroupGetDevicePermsString;
> virCgroupGetDomainTotalCpuStats;
> virCgroupGetFreezerState;
> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> index 5aa1db5..e4eaf32 100644
> --- a/src/util/vircgroup.c
> +++ b/src/util/vircgroup.c
> @@ -3170,6 +3170,105 @@ virCgroupGetPercpuVcpuSum(virCgroupPtr group,
> }
>
>
> +void virCgroupCpuStatsFree(virCgroupCpuStatsPtr stats)
> +{
> + VIR_FREE(stats->time);
> + VIR_FREE(stats->vtime);
> +}
> +
> +
> +/*
> + * Parses sparse per cpu usage cgroup output into continuos array.
> + * Sparse map is given by @cpumap.
> + */
> +static int
> +virCgroupGetPercpuTime(virCgroupPtr group,
> + virBitmapPtr cpumap,
> + unsigned long long *time)
> +{
> + char *pos;
> + char *buf = NULL;
> + size_t i;
> + int ret = -1;
> + int ncpus = virBitmapSize(cpumap);
> +
> + memset(time, 0, sizeof(*time) * ncpus);
> +
> + /* we get percpu cputime accounting info. */
> + if (virCgroupGetCpuacctPercpuUsage(group, &buf))
> + goto cleanup;
> + pos = buf;
> +
> + for (i = 0; i < ncpus; i++) {
> + if (!virBitmapIsBitSet(cpumap, i))
> + continue;
> +
> + if (virStrToLong_ull(pos, &pos, 10, &time[i]) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("cpuacct parse error"));
> + goto cleanup;
> + }
> + }
> +
> + ret = 0;
> +
> + cleanup:
> + VIR_FREE(buf);
> + return ret;
> +}
> +
> +
> +/*
> + * Get per cpu stats for given domain group.
> + * If @guestvcpus is not NULL per cpu stats for virtual cpu threads
> + * are provided as well.
> + */
> +int
> +virCgroupGetCpuStats(virCgroupPtr group,
> + virBitmapPtr guestvcpus,
> + virCgroupCpuStatsPtr stats)
> +{
> + int ret = -1;
> + int ncpus;
> + virBitmapPtr cpumap = NULL;
> +
> + /* To parse account file, we need to know how many cpus are present. */
> + if (!(cpumap = virHostCPUGetPresentBitmap()))
> + return -1;
> +
> + ncpus = virBitmapSize(cpumap);
> + if (ncpus == 0) {
> + ret = 0;
> + goto cleanup;
> + }
> +
> + memset(stats, 0, sizeof(*stats));
> +
> + if (VIR_ALLOC_N(stats->time, ncpus) < 0)
> + goto cleanup;
> +
> + if (virCgroupGetPercpuTime(group, cpumap, stats->time) < 0)
> + goto cleanup;
> +
> + if (guestvcpus) {
> + if (VIR_ALLOC_N(stats->vtime, ncpus) < 0)
> + goto cleanup;
> +
> + if (virCgroupGetPercpuVcpuSum(group, guestvcpus, stats->vtime,
> + ncpus, cpumap) < 0)
> + goto cleanup;
> + }
> +
> + ret = ncpus;
> +
> + cleanup:
> + if (ret < 0)
> + virCgroupCpuStatsFree(stats);
> + virBitmapFree(cpumap);
> + return ret;
> +}
> +
> +
> /**
> * virCgroupGetPercpuStats:
> * @cgroup: cgroup data structure
> @@ -3201,13 +3300,9 @@ virCgroupGetPercpuStats(virCgroupPtr group,
> int ret = -1;
> size_t i;
> int need_cpus, total_cpus;
> - char *pos;
> - char *buf = NULL;
> - unsigned long long *sum_cpu_time = NULL;
> virTypedParameterPtr ent;
> int param_idx;
> - unsigned long long cpu_time;
> - virBitmapPtr cpumap = NULL;
> + virCgroupCpuStats stats = {0};
>
> /* return the number of supported params */
> if (nparams == 0 && ncpus != 0) {
> @@ -3217,12 +3312,9 @@ virCgroupGetPercpuStats(virCgroupPtr group,
> return CGROUP_NB_PER_CPU_STAT_PARAM + 1;
> }
>
> - /* To parse account file, we need to know how many cpus are present. */
> - if (!(cpumap = virHostCPUGetPresentBitmap()))
> + if ((total_cpus = virCgroupGetCpuStats(group, guestvcpus, &stats)) < 0)
> return -1;
>
> - total_cpus = virBitmapSize(cpumap);
> -
> /* return total number of cpus */
> if (ncpus == 0) {
> ret = total_cpus;
> @@ -3236,30 +3328,16 @@ virCgroupGetPercpuStats(virCgroupPtr group,
> goto cleanup;
> }
>
> - /* we get percpu cputime accounting info. */
> - if (virCgroupGetCpuacctPercpuUsage(group, &buf))
> - goto cleanup;
> - pos = buf;
> -
> /* return percpu cputime in index 0 */
> param_idx = 0;
>
> /* number of cpus to compute */
> need_cpus = MIN(total_cpus, start_cpu + ncpus);
>
> - for (i = 0; i < need_cpus; i++) {
> - if (!virBitmapIsBitSet(cpumap, i)) {
> - cpu_time = 0;
> - } else if (virStrToLong_ull(pos, &pos, 10, &cpu_time) < 0) {
> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("cpuacct parse error"));
> - goto cleanup;
> - }
> - if (i < start_cpu)
> - continue;
> + for (i = start_cpu; i < need_cpus; i++) {
> ent = ¶ms[(i - start_cpu) * nparams + param_idx];
> if (virTypedParameterAssign(ent, VIR_DOMAIN_CPU_STATS_CPUTIME,
> - VIR_TYPED_PARAM_ULLONG, cpu_time) < 0)
> + VIR_TYPED_PARAM_ULLONG, stats.time[i]) < 0)
> goto cleanup;
> }
>
> @@ -3267,18 +3345,12 @@ virCgroupGetPercpuStats(virCgroupPtr group,
> param_idx = 1;
>
> if (guestvcpus && param_idx < nparams) {
> - if (VIR_ALLOC_N(sum_cpu_time, need_cpus) < 0)
> - goto cleanup;
> - if (virCgroupGetPercpuVcpuSum(group, guestvcpus, sum_cpu_time,
> - need_cpus, cpumap) < 0)
> - goto cleanup;
> -
> for (i = start_cpu; i < need_cpus; i++) {
> if (virTypedParameterAssign(¶ms[(i - start_cpu) * nparams +
> param_idx],
> VIR_DOMAIN_CPU_STATS_VCPUTIME,
> VIR_TYPED_PARAM_ULLONG,
> - sum_cpu_time[i]) < 0)
> + stats.vtime[i]) < 0)
> goto cleanup;
> }
>
> @@ -3288,9 +3360,7 @@ virCgroupGetPercpuStats(virCgroupPtr group,
> ret = param_idx;
>
> cleanup:
> - virBitmapFree(cpumap);
> - VIR_FREE(sum_cpu_time);
> - VIR_FREE(buf);
> + virCgroupCpuStatsFree(&stats);
> return ret;
> }
>
> diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
> index 2de1bf2..40b420d 100644
> --- a/src/util/vircgroup.h
> +++ b/src/util/vircgroup.h
> @@ -297,4 +297,21 @@ int virCgroupSetOwner(virCgroupPtr cgroup,
> int virCgroupHasEmptyTasks(virCgroupPtr cgroup, int controller);
>
> bool virCgroupControllerAvailable(int controller);
> +
> +struct virCgroupCpuStats_ {
> + /* time arrays are not sparsed, that is if cpu is offline
> + * it has value 0 for time instead of being skipped */
> + unsigned long long *time; /* per cpu time */
> + unsigned long long *vtime;/* per cpu time for virtual cpu threads */
> +};
> +
> +typedef struct virCgroupCpuStats_ virCgroupCpuStats;
> +typedef virCgroupCpuStats *virCgroupCpuStatsPtr;
> +
> +int virCgroupGetCpuStats(virCgroupPtr group,
> + virBitmapPtr guestvcpus,
> + virCgroupCpuStatsPtr stats);
> +void virCgroupCpuStatsFree(virCgroupCpuStatsPtr stats);
> +
> +
> #endif /* __VIR_CGROUP_H__ */
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
