[libvirt] [PATCH v2 1/2] host-validate: Be more careful when checking for cgroup support

[Ján Tomko]

On Tue, Apr 05, 2016 at 03:08:40PM +0200, Andrea Bolognani wrote:
> Simply checking whether the cgroup name appears somewhere inside
> /proc/self/cgroup is enough most of the time, but there are some
> corner cases that require a more mindful parsing.
> 
> As a bonus, after the rewrite 'line' is no longer leaked.
> ---
>  tools/virt-host-validate-common.c | 49 +++++++++++++++++++++++++++++++++------
>  1 file changed, 42 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c
> index 8ebf73e..56c6b56 100644
> --- a/tools/virt-host-validate-common.c
> +++ b/tools/virt-host-validate-common.c
> @@ -265,18 +265,53 @@ static int virHostValidateCGroupSupport(const char *hvname,
>          goto error;
>  
>      while ((ret = getline(&line, &len, fp)) >= 0 && !matched) {
> -        char *tmp = strstr(line, cg_name);
> -        if (!tmp)
> +        char **cgroups;
> +        char *start;
> +        char *end;
> +        size_t ncgroups;
> +        size_t i;
> +
> +        /* Each line in this file looks like
> +         *
> +         *   4:cpu,cpuacct:/machine.slice/machine-qemu\x2dtest.scope/emulator
> +         *

> +         * Since multiple cgroups can be part of the same line and some cgroup
> +         * names can appear as part of other cgroup names (eg. 'cpu' is a
> +         * prefix for both 'cpuacct' and 'cpuset'), it's not enough to simply
> +         * check whether the cgroup name is present somewhere inside the file

Also, they could be present in the path.

> +         */
> +
> +        /* Look for the first colon.
> +         * The part we're interested in starts right after it */
> +        if (!(start = strchr(line, ':'))) {

> +            VIR_FREE(line);
>              continue;

This pattern keeps repeating.

How about calling VIR_FREE the first thing in the loop and moving the
getline call right after it?

ACK either way.

Jan