[libvirt] [PATCH 06/34] cgroup: Clean up virCgroupGetPercpuStats

[John Ferlan]

On 01/14/2016 11:26 AM, Peter Krempa wrote:
> Use 'ret' for return variable name, clarify use of 'param_idx' and avoid
> unnecessary 'success' label. No functional changes.
> ---
>  src/util/vircgroup.c | 42 +++++++++++++++++++++---------------------
>  1 file changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> index 78f519c..2fc0276 100644
> --- a/src/util/vircgroup.c
> +++ b/src/util/vircgroup.c

Existing, but sure would be nice to have a description of what this does
in the comments...  Especially w/r/t input/output expectations.

I agree the whole param_idx usage/setting is a bit odd.  Realistically
it's [0] for CPU and [1] for the vCPU...  Thus if 'nparams == 1', then
fill in CPU data and if 'nparams == 2', then fill in vCPU data...

John
> @@ -3175,7 +3175,7 @@ virCgroupGetPercpuStats(virCgroupPtr group,
>                          unsigned int ncpus,
>                          unsigned int nvcpupids)
>  {
> -    int rv = -1;
> +    int ret = -1;
>      size_t i;
>      int need_cpus, total_cpus;
>      char *pos;
> @@ -3196,12 +3196,13 @@ virCgroupGetPercpuStats(virCgroupPtr group,
> 
>      /* To parse account file, we need to know how many cpus are present.  */
>      if (!(cpumap = nodeGetPresentCPUBitmap(NULL)))
> -        return rv;
> +        return -1;
> 
>      total_cpus = virBitmapSize(cpumap);
> 
> +    /* return total number of cpus */
>      if (ncpus == 0) {
> -        rv = total_cpus;
> +        ret = total_cpus;
>          goto cleanup;
>      }
> 
> @@ -3239,34 +3240,33 @@ virCgroupGetPercpuStats(virCgroupPtr group,
>              goto cleanup;
>      }
> 
> -    if (nvcpupids == 0 || param_idx + 1 >= nparams)
> -        goto success;
>      /* return percpu vcputime in index 1 */
> -    param_idx++;
> -
> -    if (VIR_ALLOC_N(sum_cpu_time, need_cpus) < 0)
> -        goto cleanup;
> -    if (virCgroupGetPercpuVcpuSum(group, nvcpupids, sum_cpu_time, need_cpus,
> -                                  cpumap) < 0)
> -        goto cleanup;
> +    param_idx = 1;
> 
> -    for (i = start_cpu; i < need_cpus; i++) {
> -        if (virTypedParameterAssign(&params[(i - start_cpu) * nparams +
> -                                            param_idx],
> -                                    VIR_DOMAIN_CPU_STATS_VCPUTIME,
> -                                    VIR_TYPED_PARAM_ULLONG,
> -                                    sum_cpu_time[i]) < 0)
> +    if (nvcpupids > 0 && param_idx < nparams) {
> +        if (VIR_ALLOC_N(sum_cpu_time, need_cpus) < 0)
>              goto cleanup;
> +        if (virCgroupGetPercpuVcpuSum(group, nvcpupids, sum_cpu_time, need_cpus,
> +                                      cpumap) < 0)
> +            goto cleanup;
> +
> +        for (i = start_cpu; i < need_cpus; i++) {
> +            if (virTypedParameterAssign(&params[(i - start_cpu) * nparams +
> +                                                param_idx],
> +                                        VIR_DOMAIN_CPU_STATS_VCPUTIME,
> +                                        VIR_TYPED_PARAM_ULLONG,
> +                                        sum_cpu_time[i]) < 0)
> +                goto cleanup;
> +        }
>      }
> 
> - success:
> -    rv = param_idx + 1;
> +    ret = param_idx + 1;
> 
>   cleanup:
>      virBitmapFree(cpumap);
>      VIR_FREE(sum_cpu_time);
>      VIR_FREE(buf);
> -    return rv;
> +    return ret;
>  }
> 
>