[libvirt] [PATCH 1/2] cgroup: unavailable controller prevents controller disabling

[Michal Privoznik]

On 08.12.2016 14:24, Boris Fiuczynski wrote:
> The cgroup controller filtering in virCgroupDetect does not work
> properly if the following conditions are met:
> 1) the host system does not have a cgroup controller which
> libvirt requests (unavailable controller) and
> 2) libvirt is configured to disable a controller (disabled controller) and
> 3) the disabled controller is located before the unavailable controller
> in virCgroupController.
> 
> As an example: The memory controller is unavailable and the cpuset
> controller is configured to be disabled.
> In this scenario trying to start a domain results in the error
> error: Controller 'cpuset' is not wanted, but 'memory' is co-mounted: Invalid argument
> 
> This error occurs when virCgroupDetect is called with a valid parent group.
> The resulting group created by virCgroupCopyMounts holds for cpuset and
> memory controller empty mount points. The filtering of disabled controllers
> checks for co-mounts by comparing the mount points. The cpuset controller
> causes the filtering to occur before the memory controller is marked as to be
> ignored by modifying the controller mask since it is unavailable.
> Therefore the co-mount detection logic compares the cpuset and memory controller
> mount points and since both are empty the memory controller is regarded
> erroneously as being co-mounted.
> 
> Signed-off-by: Boris Fiuczynski <fiuczy at linux.vnet.ibm.com>
> Reviewed-by: Marc Hartmayer <mhartmay at linux.vnet.ibm.com>
> Reviewed-by: Bjoern Walk <bwalk at linux.vnet.ibm.com>
> ---
>  src/util/vircgroup.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> index b6affe3..47691e2 100644
> --- a/src/util/vircgroup.c
> +++ b/src/util/vircgroup.c
> @@ -669,6 +669,8 @@ virCgroupDetect(virCgroupPtr group,
>                      controllers &= ~(1 << i);
>                  }
>              } else {
> +                if (!group->controllers[i].mountPoint)
> +                    continue; /* without controller co-mounting is impossible */
>                  /* Check whether a request to disable a controller
>                   * clashes with co-mounting of controllers */
>                  for (j = 0; j < VIR_CGROUP_CONTROLLER_LAST; j++) {
> 

The only small nit here: I'd prefer the comment to be before the continue;.

Michal