[Lxc-users] Latest test results - Was: cgroups: support cgroups mounted in multiple places (v3)
Michael H. Warfield
mhw at WittsEnd.com
Sat Jul 2 17:28:14 UTC 2011
Hey all...
So my testing has continued and I've now regression tested the v3 patch
and extended my testing. Looks like, over all, everything came together
nicely. I'd ack that... Since Serge integrated my minor little patch
into his patch, I don't think this needs a formal ack from me but it
definitely gets a big thumbs up here.
Now, that being said... My original testbeds were 2 out-of-date F12
i686 systems which needed updating and had single cgroup mount points
and 1 F15 x86_64 system with systemd mounted cgroup on multiple mount
points. Since then, I've updated the F12 systems to F14 (with a test
stop at F13 along the way).
Test results with the v3 patch...
F15 systemd: Passed.
F12 single mount: Passed.
F13 single mount: Passed.
F14 single mount: Passed.
F14 libcgroup: Failed.
I had the default /etc/cgconfig.conf file and here are the results:
[root at berserker-base ~]# cat /etc/cgconfig.conf
#
# Copyright IBM Corporation. 2007
#
# Authors: Balbir Singh <balbir at linux.vnet.ibm.com>
# This program is free software; you can redistribute it and/or modify it
# under the terms of version 2.1 of the GNU Lesser General Public License
# as published by the Free Software Foundation.
#
# This program is distributed in the hope that it would be useful, but
# WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
#
# See man cgconfig.conf for further details.
#
# By default, mount all separately controllers
# to /cgroup/<controller name>
mount {
cpuset = /cgroup/cpuset;
cpu = /cgroup/cpu;
cpuacct = /cgroup/cpuacct;
memory = /cgroup/memory;
devices = /cgroup/devices;
freezer = /cgroup/freezer;
net_cls = /cgroup/net_cls;
ns = /cgroup/ns;
blkio = /cgroup/blkio;
}
[root at berserker-base ~]# uname -a
Linux berserker-base.wittsend.com 2.6.35.13-92.fc14.i686 #1 SMP Sat May 21 17:39:42 UTC 2011 i686 i686 i386 GNU/Linux
[root at berserker-base ~]# mount -t cgroup
cgroup on /cgroup/cpuset type cgroup (rw,relatime,cpuset)
cgroup on /cgroup/cpu type cgroup (rw,relatime,cpu)
cgroup on /cgroup/cpuacct type cgroup (rw,relatime,cpuacct)
cgroup on /cgroup/memory type cgroup (rw,relatime,memory)
cgroup on /cgroup/devices type cgroup (rw,relatime,devices)
cgroup on /cgroup/freezer type cgroup (rw,relatime,freezer)
cgroup on /cgroup/net_cls type cgroup (rw,relatime,net_cls)
cgroup on /cgroup/ns type cgroup (rw,relatime,ns)
cgroup on /cgroup/blkio type cgroup (rw,relatime,blkio)
[root at berserker-base ~]# lxc-start -n Ashaman
lxc-start: no ns_cgroup option specified
lxc-start: failed to spawn 'Ashaman'
lxc-start: No such file or directory - failed to remove cgroup '/cgroup/cpuset/Ashaman'
If I add a mount point to mount the whole cgroup thing on /sys/fs/cgroup
(this kernel supports it) and reboot then it all works. This is that
mount point:
[root at berserker-base ~]# mount -t cgroup
cgroup on /sys/fs/cgroup type cgroup (rw,relatime,blkio,net_cls,freezer,devices,memory,cpuacct,cpu,ns,cpuset)
IAC... The v3 patch does no harm to existing, working, cases and
certainly covers the systemd case with F15 and that multipoint mount
on /sys/fs/cgroup. The lxc stuff is broken on F15 without it. That's
an important step forward and needs to be pushed. Not sure what the
deal is here above with the libcgroup "cgconfig" service enabled on F14
(maybe I'm doing something wrong) but that should not be a show stopper
as a mount point in fstab deals with that situation nicely. I'd like to
see this applied ASAP and a turn cranked on the revision handle as this
is needed for F15 and beyond.
Regards,
Mike
On Thu, 2011-06-30 at 21:02 -0500, Serge E. Hallyn wrote:
> (sorry for the extra traffic.)
>
> With this patch, lxc works for me both with all cgroups mounted with
> ns cgroup on /cgroup, and with libcgroup mounting all cgroups
> separately.
>
> To do this, instead of looking for one cgroup called 'lxc' or
> otherwise taking the first cgroup we find, we actually create a
> container in every mounted cgroup fs. Right now it's done under the
> root of each fs. We may want to put that under lxc, or, better yet,
> make that configurable.
>
> Changelog:
> Michael H. Warfield: Handle the case where subsystem doesn't have '.'.
> Daniel Lezcano: clean up incorrect reentrant use of mntent helpers
> v3: use the rest of Daniel's cleanups
>
> TODO: add a configurable directory name, 'lxc' by default, under which
> all lxc cgroups are created (i.e. /sys/fs/cgroup/lxc)
>
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
> ---
> src/lxc/cgroup.c | 234 ++++++++++++++++++++++++++++++-----------------------
> src/lxc/cgroup.h | 2 +-
> src/lxc/freezer.c | 2 +-
> src/lxc/lxc.h | 8 +-
> src/lxc/state.c | 2 +-
> 5 files changed, 139 insertions(+), 109 deletions(-)
>
> diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
> index a068a01..950869a 100644
> --- a/src/lxc/cgroup.c
> +++ b/src/lxc/cgroup.c
> @@ -52,90 +52,49 @@ enum {
> CGROUP_CLONE_CHILDREN,
> };
>
> -static int get_cgroup_mount(const char *mtab, char *mnt)
> +static int get_cgroup_mount(const char *subsystem, char *mnt)
> {
> struct mntent *mntent;
> FILE *file = NULL;
> - int err = -1;
>
> - file = setmntent(mtab, "r");
> + file = setmntent(MTAB, "r");
> if (!file) {
> - SYSERROR("failed to open %s", mtab);
> + SYSERROR("failed to open %s", MTAB);
> return -1;
> }
>
> while ((mntent = getmntent(file))) {
>
> - /* there is a cgroup mounted named "lxc" */
> - if (!strcmp(mntent->mnt_fsname, "lxc") &&
> - !strcmp(mntent->mnt_type, "cgroup")) {
> - strcpy(mnt, mntent->mnt_dir);
> - err = 0;
> - break;
> - }
> -
> - /* fallback to the first non-lxc cgroup found */
> - if (!strcmp(mntent->mnt_type, "cgroup") && err) {
> + if (strcmp(mntent->mnt_type, "cgroup"))
> + continue;
> + if (!subsystem || hasmntopt(mntent, subsystem)) {
> strcpy(mnt, mntent->mnt_dir);
> - err = 0;
> + fclose(file);
> + DEBUG("using cgroup mounted at '%s'", mnt);
> + return 0;
> }
> };
>
> - DEBUG("using cgroup mounted at '%s'", mnt);
> + DEBUG("Failed to find cgroup for %s\n", subsystem ? subsystem : "(NULL)");
>
> fclose(file);
>
> - return err;
> + return -1;
> }
>
> -static int get_cgroup_flags(const char *mtab, int *flags)
> +static int get_cgroup_flags(struct mntent *mntent)
> {
> - struct mntent *mntent;
> - FILE *file = NULL;
> - int err = -1;
> -
> - file = setmntent(mtab, "r");
> - if (!file) {
> - SYSERROR("failed to open %s", mtab);
> - return -1;
> - }
> -
> - *flags = 0;
> -
> - while ((mntent = getmntent(file))) {
> -
> - /* there is a cgroup mounted named "lxc" */
> - if (!strcmp(mntent->mnt_fsname, "lxc") &&
> - !strcmp(mntent->mnt_type, "cgroup")) {
> -
> - if (hasmntopt(mntent, "ns"))
> - *flags |= CGROUP_NS_CGROUP;
> -
> - if (hasmntopt(mntent, "clone_children"))
> - *flags |= CGROUP_CLONE_CHILDREN;
> -
> - err = 0;
> - break;
> - }
> + int flags = 0;
>
> - /* fallback to the first non-lxc cgroup found */
> - if (!strcmp(mntent->mnt_type, "cgroup") && err) {
>
> - if (hasmntopt(mntent, "ns"))
> - *flags |= CGROUP_NS_CGROUP;
> + if (hasmntopt(mntent, "ns"))
> + flags |= CGROUP_NS_CGROUP;
>
> - if (hasmntopt(mntent, "clone_children"))
> - *flags |= CGROUP_CLONE_CHILDREN;
> + if (hasmntopt(mntent, "clone_children"))
> + flags |= CGROUP_CLONE_CHILDREN;
>
> - err = 0;
> - }
> - };
> -
> - DEBUG("cgroup flags is 0x%x", *flags);
> -
> - fclose(file);
> -
> - return err;
> + DEBUG("cgroup %s has flags 0x%x", mntent->mnt_dir, flags);
> + return flags;
> }
>
> static int cgroup_rename_nsgroup(const char *mnt, const char *name, pid_t pid)
> @@ -199,19 +158,19 @@ static int cgroup_attach(const char *path, pid_t pid)
> return ret;
> }
>
> -int lxc_cgroup_create(const char *name, pid_t pid)
> +/*
> + * create a cgroup for the container in a particular subsystem.
> + * XXX TODO we will of course want to use cgroup_path{subsystem}/lxc/name,
> + * not just cgroup_path{subsystem}/name.
> + */
> +static int lxc_one_cgroup_create(const char *name,
> + struct mntent *mntent, pid_t pid)
> {
> - char cgmnt[MAXPATHLEN];
> char cgname[MAXPATHLEN];
> char clonechild[MAXPATHLEN];
> int flags;
>
> - if (get_cgroup_mount(MTAB, cgmnt)) {
> - ERROR("cgroup is not mounted");
> - return -1;
> - }
> -
> - snprintf(cgname, MAXPATHLEN, "%s/%s", cgmnt, name);
> + snprintf(cgname, MAXPATHLEN, "%s/%s", mntent->mnt_dir, name);
>
> /*
> * There is a previous cgroup, assume it is empty,
> @@ -222,18 +181,16 @@ int lxc_cgroup_create(const char *name, pid_t pid)
> return -1;
> }
>
> - if (get_cgroup_flags(MTAB, &flags)) {
> - SYSERROR("failed to get cgroup flags");
> - return -1;
> - }
> + flags = get_cgroup_flags(mntent);
>
> /* We have the deprecated ns_cgroup subsystem */
> if (flags & CGROUP_NS_CGROUP) {
> WARN("using deprecated ns_cgroup");
> - return cgroup_rename_nsgroup(cgmnt, cgname, pid);
> + return cgroup_rename_nsgroup(mntent->mnt_dir, cgname, pid);
> }
>
> - snprintf(clonechild, MAXPATHLEN, "%s/cgroup.clone_children", cgmnt);
> + snprintf(clonechild, MAXPATHLEN, "%s/cgroup.clone_children",
> + mntent->mnt_dir);
>
> /* we check if the kernel has clone_children, at this point if there
> * no clone_children neither ns_cgroup, that means the cgroup is mounted
> @@ -263,19 +220,49 @@ int lxc_cgroup_create(const char *name, pid_t pid)
> return -1;
> }
>
> + INFO("created cgroup '%s'", cgname);
> +
> return 0;
> }
>
> -int lxc_cgroup_destroy(const char *name)
> +/*
> + * for each mounted cgroup, create a cgroup for the container
> + */
> +int lxc_cgroup_create(const char *name, pid_t pid)
> {
> - char cgmnt[MAXPATHLEN];
> - char cgname[MAXPATHLEN];
> + struct mntent *mntent;
> + FILE *file = NULL;
> + int err = -1;
>
> - if (get_cgroup_mount(MTAB, cgmnt)) {
> - ERROR("cgroup is not mounted");
> + file = setmntent(MTAB, "r");
> + if (!file) {
> + SYSERROR("failed to open %s", MTAB);
> return -1;
> }
>
> + while ((mntent = getmntent(file))) {
> +
> + DEBUG("checking '%s' (%s)", mntent->mnt_dir, mntent->mnt_type);
> +
> + if (!strcmp(mntent->mnt_type, "cgroup")) {
> +
> + INFO("found cgroup mounted at '%s'", mntent->mnt_dir);
> + err = lxc_one_cgroup_create(name, mntent, pid);
> + if (err)
> + goto out;
> + }
> + };
> +
> +out:
> + endmntent(file);
> + return err;
> +}
> +
> +
> +int lxc_one_cgroup_destroy(const char *cgmnt, const char *name)
> +{
> + char cgname[MAXPATHLEN];
> +
> snprintf(cgname, MAXPATHLEN, "%s/%s", cgmnt, name);
> if (rmdir(cgname)) {
> SYSERROR("failed to remove cgroup '%s'", cgname);
> @@ -287,37 +274,80 @@ int lxc_cgroup_destroy(const char *name)
> return 0;
> }
>
> -int lxc_cgroup_path_get(char **path, const char *name)
> +/*
> + * for each mounted cgroup, destroy the cgroup for the container
> + */
> +int lxc_cgroup_destroy(const char *name)
> {
> - static char cgroup[MAXPATHLEN];
> - static const char* cgroup_cached = 0;
> - static char buf[MAXPATHLEN];
> + struct mntent *mntent;
> + FILE *file = NULL;
> + int ret, err = -1;
> +
> + file = setmntent(MTAB, "r");
> + if (!file) {
> + SYSERROR("failed to open %s", MTAB);
> + return -1;
> + }
>
> - if (!cgroup_cached) {
> - if (get_cgroup_mount(MTAB, cgroup)) {
> - ERROR("cgroup is not mounted");
> - return -1;
> - } else {
> - cgroup_cached = cgroup;
> + while ((mntent = getmntent(file))) {
> + if (!strcmp(mntent->mnt_type, "cgroup")) {
> + DEBUG("destroying %s %s\n", mntent->mnt_dir, name);
> + ret = lxc_one_cgroup_destroy(mntent->mnt_dir, name);
> + if (ret) {
> + fclose(file);
> + return ret;
> + }
> + err = 0;
> }
> + }
> +
> + fclose(file);
> +
> + return err;
> +}
> +/*
> + * lxc_cgroup_path_get: put into *path the pathname for
> + * %subsystem and cgroup %name. If %subsystem is NULL, then
> + * the first mounted cgroup will be used (for nr_tasks)
> + */
> +int lxc_cgroup_path_get(char **path, const char *subsystem, const char *name)
> +{
> + static char buf[MAXPATHLEN];
> + static char retbuf[MAXPATHLEN];
> +
> + /* what lxc_cgroup_set calls subsystem is actually the filename, i.e.
> + 'devices.allow'. So for our purposee we trim it */
> + if (subsystem) {
> + snprintf(retbuf, MAXPATHLEN, "%s", subsystem);
> + char *s = index(retbuf, '.');
> + if (s)
> + *s = '\0';
> + DEBUG("%s: called for subsys %s name %s\n", __func__, retbuf, name);
> }
> + if (get_cgroup_mount(subsystem ? retbuf : NULL, buf)) {
> + ERROR("cgroup is not mounted");
> + return -1;
> + }
> +
> + snprintf(retbuf, MAXPATHLEN, "%s/%s", buf, name);
> +
> + DEBUG("%s: returning %s for subsystem %s", __func__, retbuf, subsystem);
>
> - snprintf(buf, MAXPATHLEN, "%s/%s", cgroup_cached, name);
> - *path = buf;
> + *path = retbuf;
> return 0;
> }
>
> -int lxc_cgroup_set(const char *name, const char *subsystem, const char *value)
> +int lxc_cgroup_set(const char *name, const char *filename, const char *value)
> {
> int fd, ret;
> - char *nsgroup;
> + char *dirpath;
> char path[MAXPATHLEN];
>
> - ret = lxc_cgroup_path_get(&nsgroup, name);
> + ret = lxc_cgroup_path_get(&dirpath, filename, name);
> if (ret)
> return -1;
>
> - snprintf(path, MAXPATHLEN, "%s/%s", nsgroup, subsystem);
> + snprintf(path, MAXPATHLEN, "%s/%s", dirpath, filename);
>
> fd = open(path, O_WRONLY);
> if (fd < 0) {
> @@ -337,18 +367,18 @@ out:
> return ret;
> }
>
> -int lxc_cgroup_get(const char *name, const char *subsystem,
> +int lxc_cgroup_get(const char *name, const char *filename,
> char *value, size_t len)
> {
> int fd, ret = -1;
> - char *nsgroup;
> + char *dirpath;
> char path[MAXPATHLEN];
>
> - ret = lxc_cgroup_path_get(&nsgroup, name);
> + ret = lxc_cgroup_path_get(&dirpath, filename, name);
> if (ret)
> return -1;
>
> - snprintf(path, MAXPATHLEN, "%s/%s", nsgroup, subsystem);
> + snprintf(path, MAXPATHLEN, "%s/%s", dirpath, filename);
>
> fd = open(path, O_RDONLY);
> if (fd < 0) {
> @@ -366,16 +396,16 @@ int lxc_cgroup_get(const char *name, const char *subsystem,
>
> int lxc_cgroup_nrtasks(const char *name)
> {
> - char *nsgroup;
> + char *dpath;
> char path[MAXPATHLEN];
> int pid, ret, count = 0;
> FILE *file;
>
> - ret = lxc_cgroup_path_get(&nsgroup, name);
> + ret = lxc_cgroup_path_get(&dpath, NULL, name);
> if (ret)
> return -1;
>
> - snprintf(path, MAXPATHLEN, "%s/tasks", nsgroup);
> + snprintf(path, MAXPATHLEN, "%s/tasks", dpath);
>
> file = fopen(path, "r");
> if (!file) {
> diff --git a/src/lxc/cgroup.h b/src/lxc/cgroup.h
> index 8607fa8..f6df8c5 100644
> --- a/src/lxc/cgroup.h
> +++ b/src/lxc/cgroup.h
> @@ -28,6 +28,6 @@
> struct lxc_handler;
> int lxc_cgroup_create(const char *name, pid_t pid);
> int lxc_cgroup_destroy(const char *name);
> -int lxc_cgroup_path_get(char **path, const char *name);
> +int lxc_cgroup_path_get(char **path, const char *subsystem, const char *name);
> int lxc_cgroup_nrtasks(const char *name);
> #endif
> diff --git a/src/lxc/freezer.c b/src/lxc/freezer.c
> index 07eede5..94984a3 100644
> --- a/src/lxc/freezer.c
> +++ b/src/lxc/freezer.c
> @@ -45,7 +45,7 @@ static int freeze_unfreeze(const char *name, int freeze)
> char tmpf[32];
> int fd, ret;
>
> - ret = lxc_cgroup_path_get(&nsgroup, name);
> + ret = lxc_cgroup_path_get(&nsgroup, "freezer", name);
> if (ret)
> return -1;
>
> diff --git a/src/lxc/lxc.h b/src/lxc/lxc.h
> index a091baa..04df1c6 100644
> --- a/src/lxc/lxc.h
> +++ b/src/lxc/lxc.h
> @@ -114,22 +114,22 @@ extern lxc_state_t lxc_state(const char *name);
> * Set a specified value for a specified subsystem. The specified
> * subsystem must be fully specified, eg. "cpu.shares"
> * @name : the name of the container
> - * @subsystem : the subsystem
> + * @filename : the cgroup attribute filename
> * @value : the value to be set
> * Returns 0 on success, < 0 otherwise
> */
> -extern int lxc_cgroup_set(const char *name, const char *subsystem, const char *value);
> +extern int lxc_cgroup_set(const char *name, const char *filename, const char *value);
>
> /*
> * Get a specified value for a specified subsystem. The specified
> * subsystem must be fully specified, eg. "cpu.shares"
> * @name : the name of the container
> - * @subsystem : the subsystem
> + * @filename : the cgroup attribute filename
> * @value : the value to be set
> * @len : the len of the value variable
> * Returns the number of bytes read, < 0 on error
> */
> -extern int lxc_cgroup_get(const char *name, const char *subsystem,
> +extern int lxc_cgroup_get(const char *name, const char *filename,
> char *value, size_t len);
>
> /*
> diff --git a/src/lxc/state.c b/src/lxc/state.c
> index 6720011..b435eba 100644
> --- a/src/lxc/state.c
> +++ b/src/lxc/state.c
> @@ -71,7 +71,7 @@ static int freezer_state(const char *name)
> FILE *file;
> int err;
>
> - err = lxc_cgroup_path_get(&nsgroup, name);
> + err = lxc_cgroup_path_get(&nsgroup, "freezer", name);
> if (err)
> return -1;
>
--
Michael H. Warfield (AI4NB) | (770) 985-6132 | mhw at WittsEnd.com
/\/\|=mhw=|\/\/ | (678) 463-0932 | http://www.wittsend.com/mhw/
NIC whois: MHW9 | An optimist believes we live in the best of all
PGP Key: 0x674627FF | possible worlds. A pessimist is sure of it!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 482 bytes
Desc: This is a digitally signed message part
URL: <http://lists.linuxcontainers.org/pipermail/lxc-users/attachments/20110702/3a8f67b1/attachment.pgp>
More information about the lxc-users
mailing list