[libvirt] [PATCH 3/9] vircgroup: introduce controller mask for threads

[John Ferlan]

On 02/23/2016 10:58 AM, Henning Schild wrote:
> When using a cgroups hierarchy threads have child cgroups for
> certain controllers. Introduce an enum for later reuse.
> 
> Signed-off-by: Henning Schild <henning.schild at siemens.com>
> ---
>  src/util/vircgroup.c | 12 +++---------
>  src/util/vircgroup.h |  7 +++++++
>  2 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> index ad46dfc..11f33ab 100644
> --- a/src/util/vircgroup.c
> +++ b/src/util/vircgroup.c
> @@ -277,9 +277,7 @@ virCgroupValidateMachineGroup(virCgroupPtr group,
>              goto cleanup;
>  
>          if (stripEmulatorSuffix &&
> -            (i == VIR_CGROUP_CONTROLLER_CPU ||
> -             i == VIR_CGROUP_CONTROLLER_CPUACCT ||
> -             i == VIR_CGROUP_CONTROLLER_CPUSET)) {
> +            (i & VIR_CGROUP_THREAD_CONTROLLER_MASK)) {

Not sure this works as expected because 'i' is not a mask - it's just an
int... On entry, VIR_CGROUP_THREAD_CONTROLLER_MASK is 7...

The loop goes from 0 to VIR_CGROUP_CONTROLLER_LAST (11).

If you logically go through the values of 'i':

0 & 7
1 & 7
2 & 7
3 & 7
4 & 7
...
etc

You'd find 0 & 8 fail, but 1 -> 7, 9, & 10 succeed

So what "would" work is :

    mask = (1 << i);
    if (stripEmulatorSuffix &&
        mask & VIR_CGROUP_THREAD_CONTROLLER_MASK))

John

>              if (STREQ(tmp, "/emulator"))
>                  *tmp = '\0';
>              tmp = strrchr(group->controllers[i].placement, '/');
> @@ -1518,7 +1516,6 @@ virCgroupNewThread(virCgroupPtr domain,
>  {
>      int ret = -1;
>      char *name = NULL;
> -    int controllers;
>  
>      switch (nameval) {
>      case VIR_CGROUP_THREAD_VCPU:
> @@ -1539,11 +1536,8 @@ virCgroupNewThread(virCgroupPtr domain,
>          goto cleanup;
>      }
>  
> -    controllers = ((1 << VIR_CGROUP_CONTROLLER_CPU) |
> -                   (1 << VIR_CGROUP_CONTROLLER_CPUACCT) |
> -                   (1 << VIR_CGROUP_CONTROLLER_CPUSET));
> -
> -    if (virCgroupNew(-1, name, domain, controllers, group) < 0)
> +    if (virCgroupNew(-1, name, domain, VIR_CGROUP_THREAD_CONTROLLER_MASK,
> +        group) < 0)
>          goto cleanup;
>  
>      if (virCgroupMakeGroup(domain, *group, create, VIR_CGROUP_NONE) < 0) {
> diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
> index f244c24..f71aed5 100644
> --- a/src/util/vircgroup.h
> +++ b/src/util/vircgroup.h
> @@ -52,6 +52,13 @@ VIR_ENUM_DECL(virCgroupController);
>   * Make sure we will not overflow */
>  verify(VIR_CGROUP_CONTROLLER_LAST < 8 * sizeof(int));
>  
> +enum {
> +    VIR_CGROUP_THREAD_CONTROLLER_MASK =
> +        ((1 << VIR_CGROUP_CONTROLLER_CPU) |
> +         (1 << VIR_CGROUP_CONTROLLER_CPUACCT) |
> +         (1 << VIR_CGROUP_CONTROLLER_CPUSET))
> +};
> +
>  typedef enum {
>      VIR_CGROUP_THREAD_VCPU = 0,
>      VIR_CGROUP_THREAD_EMULATOR,
>