On Wed, Sep 11, 2013 at 1:18 PM, Daniel P. Berrange <berrange@xxxxxxxxxx> wrote: > From: "Daniel P. Berrange" <berrange@xxxxxxxxxx> > > Debian systems may run the 'systemd-logind' daemon, which causes the > /sys/fs/cgroup/systemd mount to be setup, but no other cgroup > controllers are created. While the LXC driver considers cgroups to > be mandatory, the QEMU driver is supposed to accept them as optional. > > We detect whether they are present by looking in /proc/mounts for > any mounts of type 'cgroups', but this is not sufficient. We need to > skip any named mounts (as seen by a name=XXX string in the mount > options), so that we only detect actual resource controllers. > > http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=721979 > > Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx> > --- > src/util/vircgroup.c | 5 +++- > tests/vircgroupmock.c | 40 ++++++++++++++++++++++++--- > tests/vircgrouptest.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 116 insertions(+), 4 deletions(-) > > diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c > index a260356..626bbc6 100644 > --- a/src/util/vircgroup.c > +++ b/src/util/vircgroup.c > @@ -91,7 +91,10 @@ virCgroupAvailable(void) > return false; > > while (getmntent_r(mounts, &entry, buf, sizeof(buf)) != NULL) { > - if (STREQ(entry.mnt_type, "cgroup")) { > + /* We're looking for at least one 'cgroup' fs mount, > + * which is *not* a named mount. */ > + if (STREQ(entry.mnt_type, "cgroup") && > + !strstr(entry.mnt_opts, "name=")) { > ret = true; > break; > } > diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c > index d83496c..adc1718 100644 > --- a/tests/vircgroupmock.c > +++ b/tests/vircgroupmock.c > @@ -124,6 +124,27 @@ const char *proccgroupsallinone = > "devices 6 1 1\n" > "blkio 6 1 1\n"; > > +const char *procmountslogind = > + "none /not/really/sys/fs/cgroup tmpfs rw,rootcontext=system_u:object_r:sysfs_t:s0,seclabel,relatime,size=4k,mode=755 0 0\n" > + "systemd /not/really/sys/fs/cgroup/systemd cgroup rw,nosuid,nodev,noexec,relatime,name=systemd 0 0\n"; > + > +const char *procselfcgroupslogind = > + "1:name=systemd:/\n"; > + > +const char *proccgroupslogind = > + "#subsys_name hierarchy num_cgroups enabled\n" > + "cpuset 0 1 1\n" > + "cpu 0 1 1\n" > + "cpuacct 0 1 1\n" > + "memory 0 1 0\n" > + "devices 0 1 1\n" > + "freezer 0 1 1\n" > + "net_cls 0 1 1\n" > + "blkio 0 1 1\n" > + "perf_event 0 1 1\n"; > + > + > + > static int make_file(const char *path, > const char *name, > const char *value) > @@ -400,18 +421,25 @@ static void init_sysfs(void) > FILE *fopen(const char *path, const char *mode) > { > const char *mock; > - bool allinone = false; > + bool allinone = false, logind = false; > init_syms(); > > mock = getenv("VIR_CGROUP_MOCK_MODE"); > - if (mock && STREQ(mock, "allinone")) > - allinone = true; > + if (mock) { > + if (STREQ(mock, "allinone")) > + allinone = true; > + else if (STREQ(mock, "logind")) > + logind = true; > + } > > if (STREQ(path, "/proc/mounts")) { > if (STREQ(mode, "r")) { > if (allinone) > return fmemopen((void *)procmountsallinone, > strlen(procmountsallinone), mode); > + else if (logind) > + return fmemopen((void *)procmountslogind, > + strlen(procmountslogind), mode); > else > return fmemopen((void *)procmounts, strlen(procmounts), mode); > } else { > @@ -424,6 +452,9 @@ FILE *fopen(const char *path, const char *mode) > if (allinone) > return fmemopen((void *)proccgroupsallinone, > strlen(proccgroupsallinone), mode); > + else if (logind) > + return fmemopen((void *)proccgroupslogind, > + strlen(proccgroupslogind), mode); > else > return fmemopen((void *)proccgroups, strlen(proccgroups), mode); > } else { > @@ -436,6 +467,9 @@ FILE *fopen(const char *path, const char *mode) > if (allinone) > return fmemopen((void *)procselfcgroupsallinone, > strlen(procselfcgroupsallinone), mode); > + else if (logind) > + return fmemopen((void *)procselfcgroupslogind, > + strlen(procselfcgroupslogind), mode); > else > return fmemopen((void *)procselfcgroups, strlen(procselfcgroups), mode); > } else { > diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c > index f12587c..570e061 100644 > --- a/tests/vircgrouptest.c > +++ b/tests/vircgrouptest.c > @@ -109,6 +109,16 @@ const char *mountsAllInOne[VIR_CGROUP_CONTROLLER_LAST] = { > [VIR_CGROUP_CONTROLLER_BLKIO] = "/not/really/sys/fs/cgroup", > [VIR_CGROUP_CONTROLLER_SYSTEMD] = NULL, > }; > +const char *mountsLogind[VIR_CGROUP_CONTROLLER_LAST] = { > + [VIR_CGROUP_CONTROLLER_CPU] = NULL, > + [VIR_CGROUP_CONTROLLER_CPUACCT] = NULL, > + [VIR_CGROUP_CONTROLLER_CPUSET] = NULL, > + [VIR_CGROUP_CONTROLLER_MEMORY] = NULL, > + [VIR_CGROUP_CONTROLLER_DEVICES] = NULL, > + [VIR_CGROUP_CONTROLLER_FREEZER] = NULL, > + [VIR_CGROUP_CONTROLLER_BLKIO] = NULL, > + [VIR_CGROUP_CONTROLLER_SYSTEMD] = "/not/really/sys/fs/cgroup/systemd", > +}; > > const char *links[VIR_CGROUP_CONTROLLER_LAST] = { > [VIR_CGROUP_CONTROLLER_CPU] = "/not/really/sys/fs/cgroup/cpu", > @@ -132,6 +142,17 @@ const char *linksAllInOne[VIR_CGROUP_CONTROLLER_LAST] = { > [VIR_CGROUP_CONTROLLER_SYSTEMD] = NULL, > }; > > +const char *linksLogind[VIR_CGROUP_CONTROLLER_LAST] = { > + [VIR_CGROUP_CONTROLLER_CPU] = NULL, > + [VIR_CGROUP_CONTROLLER_CPUACCT] = NULL, > + [VIR_CGROUP_CONTROLLER_CPUSET] = NULL, > + [VIR_CGROUP_CONTROLLER_MEMORY] = NULL, > + [VIR_CGROUP_CONTROLLER_DEVICES] = NULL, > + [VIR_CGROUP_CONTROLLER_FREEZER] = NULL, > + [VIR_CGROUP_CONTROLLER_BLKIO] = NULL, > + [VIR_CGROUP_CONTROLLER_SYSTEMD] = NULL, > +}; > + > > static int testCgroupNewForSelf(const void *args ATTRIBUTE_UNUSED) > { > @@ -466,6 +487,48 @@ cleanup: > } > > > +static int testCgroupNewForSelfLogind(const void *args ATTRIBUTE_UNUSED) > +{ > + virCgroupPtr cgroup = NULL; > + int ret = -1; > + const char *placement[VIR_CGROUP_CONTROLLER_LAST] = { > + [VIR_CGROUP_CONTROLLER_CPU] = NULL, > + [VIR_CGROUP_CONTROLLER_CPUACCT] = NULL, > + [VIR_CGROUP_CONTROLLER_CPUSET] = NULL, > + [VIR_CGROUP_CONTROLLER_MEMORY] = NULL, > + [VIR_CGROUP_CONTROLLER_DEVICES] = NULL, > + [VIR_CGROUP_CONTROLLER_FREEZER] = NULL, > + [VIR_CGROUP_CONTROLLER_BLKIO] = NULL, > + [VIR_CGROUP_CONTROLLER_SYSTEMD] = "/", > + }; > + > + if (virCgroupNewSelf(&cgroup) < 0) { > + fprintf(stderr, "Cannot create cgroup for self\n"); > + goto cleanup; > + } > + > + ret = validateCgroup(cgroup, "", mountsLogind, linksLogind, placement); > + > +cleanup: > + virCgroupFree(&cgroup); > + return ret; > +} > + > + > +static int testCgroupAvailable(const void *args) > +{ > + bool got = virCgroupAvailable(); > + bool want = args == (void*)0x1; > + > + if (got != want) { > + fprintf(stderr, "Expected cgroup %savailable, but state was wrong\n", Missing a space between the %s and available. > + want ? "" : "not "); > + return -1; > + } > + > + return 0; > +} > + > > # define FAKESYSFSDIRTEMPLATE abs_builddir "/fakesysfsdir-XXXXXX" > > @@ -505,9 +568,21 @@ mymain(void) > if (virtTestRun("New cgroup for domain partition escaped", 1, testCgroupNewForPartitionDomainEscaped, NULL) < 0) > ret = -1; > > + if (virtTestRun("Cgroup available", 1, testCgroupAvailable, (void*)0x1) < 0) > + ret = -1; > + > setenv("VIR_CGROUP_MOCK_MODE", "allinone", 1); > if (virtTestRun("New cgroup for self (allinone)", 1, testCgroupNewForSelfAllInOne, NULL) < 0) > ret = -1; > + if (virtTestRun("Cgroup available", 1, testCgroupAvailable, (void*)0x1) < 0) > + ret = -1; > + unsetenv("VIR_CGROUP_MOCK_MODE"); > + > + setenv("VIR_CGROUP_MOCK_MODE", "logind", 1); > + if (virtTestRun("New cgroup for self (logind)", 1, testCgroupNewForSelfLogind, NULL) < 0) > + ret = -1; > + if (virtTestRun("Cgroup available", 1, testCgroupAvailable, (void*)0x0) < 0) > + ret = -1; > unsetenv("VIR_CGROUP_MOCK_MODE"); > > if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) > -- > 1.8.3.1 > > -- > libvir-list mailing list > libvir-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list ACK with the fix in the white space. -- Doug Goldstein -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list