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

[Pavel Hrdina]

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 at redhat.com>
> > ---
> >  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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20181003/c6c07fb9/attachment-0001.sig>