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

[Henning Schild]

Ok i will reorder, fix style and docs etc.

On Thu, 25 Feb 2016 17:52:55 -0500
John Ferlan <jferlan at redhat.com> wrote:

> 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 at siemens.com>
> > ---
> >  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]; };
> >