Re: [libvirt] [PATCH 02/53] vircgroup: introduce virCgroupV2Available

On 10/02/2018 10:43 AM, Pavel Hrdina wrote: > We cannot detect only mount points to figure out whether cgroup v2 > is available because systemd uses cgroup v2 for process tracking and > all controllers are mounted as cgroup v1 controllers. > > To make sure that this is no the situation we need to check > 'cgroup.controllers' file if it's not empty to make sure that cgroup > v2 is not mounted only for process tracking. > > Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; > --- > src/util/vircgroupv2.c | 52 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 52 insertions(+) > > diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c > index 23bf81dae2..78497bd31d 100644 > --- a/src/util/vircgroupv2.c > +++ b/src/util/vircgroupv2.c > @@ -19,6 +19,10 @@ > */ > #include <config.h> > > +#ifdef __linux__ > +# include <mntent.h> > +#endif /* __linux__ */ > + > #include "internal.h" > > #define __VIR_CGROUP_ALLOW_INCLUDE_PRIV_H__ > @@ -28,7 +32,9 @@ > #include "vircgroup.h" > #include "vircgroupbackend.h" > #include "vircgroupv2.h" > +#include "virfile.h" > #include "virlog.h" > +#include "virstring.h" > > VIR_LOG_INIT("util.cgroup"); > > @@ -41,8 +47,54 @@ VIR_ENUM_IMPL(virCgroupV2Controller, VIR_CGROUP_CONTROLLER_LAST, > > #ifdef __linux__ > > +/* We're looking for one 'cgroup2' fs mount which has some > + * controllers enabled. */ > +static bool > +virCgroupV2Available(void) > +{ > + bool ret = false; > + FILE *mounts = NULL; > + struct mntent entry; > + char buf[CGROUP_MAX_VAL]; > + > + if (!(mounts = fopen("/proc/mounts", "r"))) > + return false; > + > + while (getmntent_r(mounts, &entry, buf, sizeof(buf)) != NULL) { > + if (STREQ(entry.mnt_type, "cgroup2")) { > + ret = true; > + break; > + } > + } > + > + /* Systemd uses cgroup v2 for process tracking but no controller is > + * available. We should consider this configuration as cgroup v2 is > + * not available. */ > + if (ret) { > + int rc; > + VIR_AUTOFREE(char *) contFile = NULL; > + VIR_AUTOFREE(char *) contStr = NULL; > + > + if (virAsprintf(&contFile, "%s/cgroup.controllers", entry.mnt_dir) < 0) > + return false; > + > + rc = virFileReadAll(contFile, 1024 * 1024, &contStr); > + if (rc < 0) > + return false; > + > + if (STREQ(contStr, "")) > + return false; > + } > + > + VIR_FORCE_FCLOSE(mounts); > + return ret; This is wrong on two levels. Firstly, if any of those conditions under 'if (ret)' fail, then @mounts is leaked. Secondly, @ret is set but then it's ignored and false is returned regardless.