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

[John Ferlan]

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];
>  };
>