Re: [libvirt] [PATCH 2/9] vircgroup: add assertion to allow cgroup controllers to stay empty

On 02/23/2016 10:58 AM, Henning Schild wrote: > When using a hierarchy of cgroups we might want to add tasks just to > the children cgroups but never to the parent. To make sure we do not > use a parent cgroup by accident add a mechanism that lets us assert > a correct implementation in cases we want such a hierarchy. > > i.e. for qemu cpusets we want all tasks in /vcpuX or /emulator, not > in /. > > Signed-off-by: Henning Schild <henning.schild(a)siemens.com&gt; > --- > src/libvirt_private.syms | 2 ++ > src/util/vircgroup.c | 19 +++++++++++++++++++ > src/util/vircgroup.h | 3 +++ > src/util/vircgrouppriv.h | 1 + > 4 files changed, 25 insertions(+) > These aren't used until patch 9 - I think this should be closer to that patch... That is introduce it just before you use it.. > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > index 83f6e2c..cf93d06 100644 > --- a/src/libvirt_private.syms > +++ b/src/libvirt_private.syms > @@ -1201,6 +1201,7 @@ virCgroupDenyDeviceMajor; > virCgroupDenyDevicePath; > virCgroupDetectMountsFromFile; > virCgroupFree; > +virCgroupGetAssertEmpty; > virCgroupGetBlkioDeviceReadBps; > virCgroupGetBlkioDeviceReadIops; > virCgroupGetBlkioDeviceWeight; > @@ -1245,6 +1246,7 @@ virCgroupNewThread; > virCgroupPathOfController; > virCgroupRemove; > virCgroupRemoveRecursively; > +virCgroupSetAssertEmpty; > virCgroupSetBlkioDeviceReadBps; > virCgroupSetBlkioDeviceReadIops; > virCgroupSetBlkioDeviceWeight; > diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c > index 0b65238..ad46dfc 100644 > --- a/src/util/vircgroup.c > +++ b/src/util/vircgroup.c > @@ -1197,6 +1197,15 @@ virCgroupAddTaskController(virCgroupPtr > group, pid_t pid, int controller) return -1; > } > > + if(group->assert_empty & (1 << controller)) { should be : if (group... > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("Controller '%s' is not supposed to > contain any" > + " tasks. group=%s pid=%d\n"), > + virCgroupControllerTypeToString(controller), > + group->path, pid); > + return -1; > + } > + > return virCgroupSetValueU64(group, controller, "tasks", > (unsigned long long)pid); > } > @@ -1246,6 +1255,16 @@ virCgroupAddTaskStrController(virCgroupPtr > group, return rc; > } > Need to have some code comments regarding input and what these do... > +void > +virCgroupSetAssertEmpty(virCgroupPtr group, int mask) { > + group->assert_empty = mask; > +} > + > +int > +virCgroupGetAssertEmpty(virCgroupPtr group) { > + return group->assert_empty; > +} > + > You'll need to add the corresponding API in the "#else /* !VIR_CGROUP_SUPPORTED */" area... Search on virCgroupAddTaskController - you'll find a second entry in the module which reports a system error. That's what you'll need to add for these John > /** > * virCgroupMoveTask: > diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h > index 63a9e1c..f244c24 100644 > --- a/src/util/vircgroup.h > +++ b/src/util/vircgroup.h > @@ -131,6 +131,9 @@ int virCgroupAddTaskController(virCgroupPtr > group, pid_t pid, > int controller); > > +void virCgroupSetAssertEmpty(virCgroupPtr group, int mask); > +int virCgroupGetAssertEmpty(virCgroupPtr group); > + > int virCgroupMoveTask(virCgroupPtr src_group, > virCgroupPtr dest_group); > > diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h > index 722863e..944d6ae 100644 > --- a/src/util/vircgrouppriv.h > +++ b/src/util/vircgrouppriv.h > @@ -44,6 +44,7 @@ struct virCgroupController { > > struct virCgroup { > char *path; > + int assert_empty; > > struct virCgroupController > controllers[VIR_CGROUP_CONTROLLER_LAST]; }; >