[libvirt] [PATCH 11/34] cgroup: Prepare for sparse vCPU topologies in virCgroupGetPercpuStats

[John Ferlan]

On 01/14/2016 11:26 AM, Peter Krempa wrote:
> Pass a bitmap of enabled guest vCPUs to virCgroupGetPercpuStats so that
> un-continuous vCPU topologies can be used.
> ---
>  src/lxc/lxc_driver.c   |  2 +-
>  src/qemu/qemu_driver.c |  7 ++++++-
>  src/util/vircgroup.c   | 16 ++++++++--------
>  src/util/vircgroup.h   |  3 ++-
>  tests/vircgrouptest.c  |  2 +-
>  5 files changed, 18 insertions(+), 12 deletions(-)
> 

/me realizing that patches 10 & 11 fix the opposite issue I noted during
review of patch 4...  The cgroup thread indices were created using the
vcpus array, but have been indexed using the nvcpupids ordering. Works
fine if vcpus are all online, but has side effects if they are not.

Would be nice to keep 7, 10, 11 (etc) together... 8 & 9 are unrelated.


> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index 67088c8..f6fe207 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -5702,7 +5702,7 @@ lxcDomainGetCPUStats(virDomainPtr dom,
>                                                params, nparams);
>      else
>          ret = virCgroupGetPercpuStats(priv->cgroup, params,
> -                                      nparams, start_cpu, ncpus, 0);
> +                                      nparams, start_cpu, ncpus, NULL);
>   cleanup:
>      virObjectUnlock(vm);
>      return ret;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 3102fc2..f3844d6 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -18092,6 +18092,7 @@ qemuDomainGetCPUStats(virDomainPtr domain,
>      virDomainObjPtr vm = NULL;
>      int ret = -1;
>      qemuDomainObjPrivatePtr priv;
> +    virBitmapPtr guestvcpus = NULL;
> 
>      virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
> 
> @@ -18115,13 +18116,17 @@ qemuDomainGetCPUStats(virDomainPtr domain,
>          goto cleanup;
>      }
> 
> +    if (!(guestvcpus = virDomainDefGetVcpumap(vm->def)))
> +        goto cleanup;
> +

If the VcpuOnlineMap was stored w/ vm->def - then it could be reused
rather than created this one time on the fly and deleted. Other places
traverse 'def->vcpus' via 'maxvcpus' (or virDomainDefGetVcpusMax) and
filter on vcpu->online.

e.g. after all patches were applied - qemuRestoreCgroupState,
qemuDomainHelperGetVcpus, qemuDomainSetNumaParamsLive,
qemuSetVcpusBWLive, and qemuProcessSetupVcpus

John

>      if (start_cpu == -1)
>          ret = virCgroupGetDomainTotalCpuStats(priv->cgroup,
>                                                params, nparams);
>      else
>          ret = virCgroupGetPercpuStats(priv->cgroup, params, nparams,
> -                                      start_cpu, ncpus, priv->nvcpupids);
> +                                      start_cpu, ncpus, guestvcpus);
>   cleanup:
> +    virBitmapFree(guestvcpus);
>      virDomainObjEndAPI(&vm);
>      return ret;
>  }
> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> index 2fc0276..e6dbc8d 100644
> --- a/src/util/vircgroup.c
> +++ b/src/util/vircgroup.c
> @@ -3121,17 +3121,17 @@ virCgroupDenyDevicePath(virCgroupPtr group, const char *path, int perms)
>   */
>  static int
>  virCgroupGetPercpuVcpuSum(virCgroupPtr group,
> -                          unsigned int nvcpupids,
> +                          virBitmapPtr guestvcpus,
>                            unsigned long long *sum_cpu_time,
>                            size_t nsum,
>                            virBitmapPtr cpumap)
>  {
>      int ret = -1;
> -    size_t i;
> +    ssize_t i = -1;
>      char *buf = NULL;
>      virCgroupPtr group_vcpu = NULL;
> 
> -    for (i = 0; i < nvcpupids; i++) {
> +    while ((i = virBitmapNextSetBit(guestvcpus, i)) >= 0) {
>          char *pos;
>          unsigned long long tmp;
>          ssize_t j;
> @@ -3173,7 +3173,7 @@ virCgroupGetPercpuStats(virCgroupPtr group,
>                          unsigned int nparams,
>                          int start_cpu,
>                          unsigned int ncpus,
> -                        unsigned int nvcpupids)
> +                        virBitmapPtr guestvcpus)
>  {
>      int ret = -1;
>      size_t i;
> @@ -3188,7 +3188,7 @@ virCgroupGetPercpuStats(virCgroupPtr group,
> 
>      /* return the number of supported params */
>      if (nparams == 0 && ncpus != 0) {
> -        if (nvcpupids == 0)
> +        if (!guestvcpus)
>              return CGROUP_NB_PER_CPU_STAT_PARAM;
>          else
>              return CGROUP_NB_PER_CPU_STAT_PARAM + 1;
> @@ -3243,11 +3243,11 @@ virCgroupGetPercpuStats(virCgroupPtr group,
>      /* return percpu vcputime in index 1 */
>      param_idx = 1;
> 
> -    if (nvcpupids > 0 && param_idx < nparams) {
> +    if (guestvcpus && 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)
> +        if (virCgroupGetPercpuVcpuSum(group, guestvcpus, sum_cpu_time,
> +                                      need_cpus, cpumap) < 0)
>              goto cleanup;
> 
>          for (i = start_cpu; i < need_cpus; i++) {
> diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
> index 63a9e1c..856b993 100644
> --- a/src/util/vircgroup.h
> +++ b/src/util/vircgroup.h
> @@ -26,6 +26,7 @@
>  # define __VIR_CGROUP_H__
> 
>  # include "virutil.h"
> +# include "virbitmap.h"
> 
>  struct virCgroup;
>  typedef struct virCgroup *virCgroupPtr;
> @@ -246,7 +247,7 @@ virCgroupGetPercpuStats(virCgroupPtr group,
>                          unsigned int nparams,
>                          int start_cpu,
>                          unsigned int ncpus,
> -                        unsigned int nvcpupids);
> +                        virBitmapPtr guestvcpus);
> 
>  int
>  virCgroupGetDomainTotalCpuStats(virCgroupPtr group,
> diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c
> index 7ea6e13..322f6cb 100644
> --- a/tests/vircgrouptest.c
> +++ b/tests/vircgrouptest.c
> @@ -656,7 +656,7 @@ static int testCgroupGetPercpuStats(const void *args ATTRIBUTE_UNUSED)
> 
>      if ((rv = virCgroupGetPercpuStats(cgroup,
>                                        params,
> -                                      1, 0, EXPECTED_NCPUS, 0)) < 0) {
> +                                      1, 0, EXPECTED_NCPUS, NULL)) < 0) {
>          fprintf(stderr, "Failed call to virCgroupGetPercpuStats for /virtualmachines cgroup: %d\n", -rv);
>          goto cleanup;
>      }
>