On Wed, Oct 03, 2018 at 04:49:35PM +0200, Michal Privoznik wrote:
> 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@xxxxxxxxxx>
> > ---
> > 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.
/me hides under table
Right, I totally missed that with all the rebases and code movement.
I'll fix that.
Thanks
Pavel
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
