[libvirt] [PATCH 4/9] util: cgroups do not implicitly add task to new machine cgroup

[John Ferlan]

On 02/23/2016 10:58 AM, Henning Schild wrote:
> virCgroupNewMachine used to add the pidleader to the newly created
> machine cgroup. Do not do this implicit anymore.
> 
> Signed-off-by: Henning Schild <henning.schild at siemens.com>
> ---
>  src/lxc/lxc_cgroup.c   | 11 +++++++++++
>  src/qemu/qemu_cgroup.c | 11 +++++++++++
>  src/util/vircgroup.c   | 22 ----------------------
>  3 files changed, 22 insertions(+), 22 deletions(-)
> 

So here we are again... Trying to recall where we left off... Ah yes,
the entrails for this particular patch point me at the following:

http://www.redhat.com/archives/libvir-list/2015-December/msg00759.html

for lxc, and the following for qemu:

http://www.redhat.com/archives/libvir-list/2016-January/msg00513.html

Essentially the problem being that we can get a 0 return status *and*
the returned cgroup pointer is still NULL

That still needs to be addressed here (if this gets accepted)

> diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
> index ad254e4..609e9ea 100644
> --- a/src/lxc/lxc_cgroup.c
> +++ b/src/lxc/lxc_cgroup.c
> @@ -504,6 +504,17 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def,
>                              &cgroup) < 0)

IOW:

< 0 || !cgroup)

>          goto cleanup;
>  
> +    if (virCgroupAddTask(cgroup, initpid) < 0) {
> +        virErrorPtr saved = virSaveLastError();
> +        virCgroupRemove(cgroup);
> +        virCgroupFree(&cgroup);
> +        if (saved) {
> +            virSetError(saved);
> +            virFreeError(saved);
> +        }
> +        goto cleanup;
> +    }
> +
>      /* setup control group permissions for user namespace */
>      if (def->idmap.uidmap) {
>          if (virCgroupSetOwner(cgroup,
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index e41f461..66dc782 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -789,6 +789,17 @@ qemuInitCgroup(virQEMUDriverPtr driver,

and a few lines above here:

&priv->cgroup) < 0 || !priv->cgroup) {


John
>          goto cleanup;
>      }
>  
> +    if (virCgroupAddTask(priv->cgroup, vm->pid) < 0) {
> +        virErrorPtr saved = virSaveLastError();
> +        virCgroupRemove(priv->cgroup);
> +        virCgroupFree(&priv->cgroup);
> +        if (saved) {
> +            virSetError(saved);
> +            virFreeError(saved);
> +        }
> +        goto cleanup;
> +    }
> +
>   done:
>      ret = 0;
>   cleanup:
> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> index 11f33ab..aef8e8c 100644
> --- a/src/util/vircgroup.c
> +++ b/src/util/vircgroup.c
> @@ -1682,16 +1682,6 @@ virCgroupNewMachineSystemd(const char *name,
>          }
>      }
>  
> -    if (virCgroupAddTask(*group, pidleader) < 0) {
> -        virErrorPtr saved = virSaveLastError();
> -        virCgroupRemove(*group);
> -        virCgroupFree(group);
> -        if (saved) {
> -            virSetError(saved);
> -            virFreeError(saved);
> -        }
> -    }
> -
>      ret = 0;
>   cleanup:
>      virCgroupFree(&parent);
> @@ -1714,7 +1704,6 @@ int virCgroupTerminateMachine(const char *name,
>  static int
>  virCgroupNewMachineManual(const char *name,
>                            const char *drivername,
> -                          pid_t pidleader,
>                            const char *partition,
>                            int controllers,
>                            virCgroupPtr *group)
> @@ -1740,16 +1729,6 @@ virCgroupNewMachineManual(const char *name,
>                                      group) < 0)
>          goto cleanup;
>  
> -    if (virCgroupAddTask(*group, pidleader) < 0) {
> -        virErrorPtr saved = virSaveLastError();
> -        virCgroupRemove(*group);
> -        virCgroupFree(group);
> -        if (saved) {
> -            virSetError(saved);
> -            virFreeError(saved);
> -        }
> -    }
> -
>   done:
>      ret = 0;
>  
> @@ -1796,7 +1775,6 @@ virCgroupNewMachine(const char *name,
>  
>      return virCgroupNewMachineManual(name,
>                                       drivername,
> -                                     pidleader,
>                                       partition,
>                                       controllers,
>                                       group);
>