[libvirt] [PATCH] virCgroupValidateMachineGroup: Reflect change in CGroup struct naming

[Pavel Hrdina]

On Thu, May 05, 2016 at 05:56:55PM +0200, Michal Privoznik wrote:
> Fron c3bd0019c0e on instead of creating the following path for
> cgroups:
> 
>   /sys/fs/cgroupX/$name.libvirt-$driver
> 
> we generate rather more verbose one:
> 
>   /sys/fs/cgroupX/$driver-$id-$name.libvirt-$driver
> 
> where $name is optional and included iff contains allowed chars.
> See original commit for more reasoning. Now, problem with the
> original commit is that we are unable to start any LXC domain
> after it. Because when starting LXC container, the CGroup layout
> is created by our lxc_controller process and then detected and
> validated by libvirtd. The validation is done by trying to match
> detected layout against all the possible patterns for cgroup
> paths that we've ever had. And the commit in question forgot to
> update this part of the code.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
> 
> I am really surprised how long this bug went unnoticed. Is
> anybody using LXC? When pushed, it should be backported to
> 1.3.2+.
> 
>  src/util/vircgroup.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> index da5ccff..add6c5f 100644
> --- a/src/util/vircgroup.c
> +++ b/src/util/vircgroup.c
> @@ -261,12 +261,17 @@ virCgroupValidateMachineGroup(virCgroupPtr group,
>      char *scopename_new = NULL;
>      char *machinename = virSystemdMakeMachineName(drivername, id,
>                                                    name, privileged);
> +    char *partmachinename = NULL;
>  
>      if (virAsprintf(&partname, "%s.libvirt-%s",
> -                    name, drivername) < 0)
> +                    name, drivername) < 0 ||
> +        (machinename &&
> +         virAsprintf(&partmachinename, "%s.libvirt-%s",
> +                     machinename, drivername) < 0))
>          goto cleanup;
>  
> -    if (virCgroupPartitionEscape(&partname) < 0)
> +    if (virCgroupPartitionEscape(&partname) < 0 ||
> +        (machinename && virCgroupPartitionEscape(&partmachinename) < 0))
>          goto cleanup;

Instead of those composite conditions I would rather make something like this:

if (machinename) {
    create partmachinename;
    run virCgroupPartitionEscape();
}

It's the same but for me more readable and cleaner code.

ACK with or without the change.

Pavel