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

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(a)siemens.com&gt; > --- > 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, >