From: "S.Çağlar Onur" <cag...@10ur.org>
Trying to stop multiple containers concurrently ends up with "cgroup is not
mounted" errors as multiple threads corrupts the shared variables.
Fix that stack corruption and start to use getmntent_r to support stopping
multiple containers concurrently.
---
src/lxc/cgroup.c | 133 ++++++++++++++++++++++++++++++++++-------------------
src/lxc/freezer.c | 14 +++++-
src/lxc/state.c | 13 ++++--
3 files changed, 107 insertions(+), 53 deletions(-)
diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
index 812bfb8..fa85bcc 100644
--- a/src/lxc/cgroup.c
+++ b/src/lxc/cgroup.c
@@ -98,29 +98,31 @@ static char *mount_has_subsystem(const struct mntent
*mntent)
*/
static int get_cgroup_mount(const char *subsystem, char *mnt)
{
- struct mntent *mntent;
+ struct mntent *mntent, mntent_r;
FILE *file = NULL;
int ret, err = -1;
+ char buf[MAXPATHLEN] = {0};
+
file = setmntent(MTAB, "r");
if (!file) {
SYSERROR("failed to open %s", MTAB);
return -1;
}
- while ((mntent = getmntent(file))) {
- if (strcmp(mntent->mnt_type, "cgroup"))
+ while ((mntent = getmntent_r(file, &mntent_r, buf, sizeof(buf)))) {
+ if (strcmp(mntent_r.mnt_type, "cgroup") != 0)
continue;
-
+
if (subsystem) {
- if (!hasmntopt(mntent, subsystem))
+ if (!hasmntopt(&mntent_r, subsystem))
continue;
} else {
- if (!mount_has_subsystem(mntent))
+ if (!mount_has_subsystem(&mntent_r))
continue;
}
- ret = snprintf(mnt, MAXPATHLEN, "%s", mntent->mnt_dir);
+ ret = snprintf(mnt, MAXPATHLEN, "%s", mntent_r.mnt_dir);
if (ret < 0 || ret >= MAXPATHLEN)
goto fail;
@@ -146,22 +148,24 @@ out:
*
* Returns 0 on success, -1 on error.
*
- * The answer is written in a static char[MAXPATHLEN] in this function and
- * should not be freed.
*/
extern int cgroup_path_get(char **path, const char *subsystem, const char
*cgpath)
{
- static char buf[MAXPATHLEN];
- static char retbuf[MAXPATHLEN];
int rc;
+ char *buf;
+ char *retbuf;
+
+ buf = malloc(MAXPATHLEN * sizeof(char));
+ retbuf = malloc(MAXPATHLEN * sizeof(char));
+
/* lxc_cgroup_set passes a state object for the subsystem,
* so trim it to just the subsystem part */
if (subsystem) {
rc = snprintf(retbuf, MAXPATHLEN, "%s", subsystem);
if (rc < 0 || rc >= MAXPATHLEN) {
ERROR("subsystem name too long");
- return -1;
+ goto fail;
}
char *s = index(retbuf, '.');
if (s)
@@ -170,19 +174,28 @@ extern int cgroup_path_get(char **path, const char
*subsystem, const char *cgpat
}
if (get_cgroup_mount(subsystem ? retbuf : NULL, buf)) {
ERROR("cgroup is not mounted");
- return -1;
+ goto fail;
}
rc = snprintf(retbuf, MAXPATHLEN, "%s/%s", buf, cgpath);
if (rc < 0 || rc >= MAXPATHLEN) {
ERROR("name too long");
- return -1;
+ goto fail;
}
DEBUG("%s: returning %s for subsystem %s", __func__, retbuf, subsystem);
+ if(buf)
+ free(buf);
+
*path = retbuf;
return 0;
+fail:
+ if (buf)
+ free(buf);
+ if (retbuf)
+ free(retbuf);
+ return -1;
}
/*
@@ -295,15 +308,20 @@ int lxc_cgroup_set_bypath(const char *cgpath, const char
*filename, const char *
ret = cgroup_path_get(&dirpath, filename, cgpath);
if (ret)
- return -1;
+ goto fail;
ret = snprintf(path, MAXPATHLEN, "%s/%s", dirpath, filename);
if (ret < 0 || ret >= MAXPATHLEN) {
ERROR("pathname too long");
- return -1;
+ goto fail;
}
return do_cgroup_set(path, value);
+
+fail:
+ if(dirpath)
+ free(dirpath);
+ return -1;
}
/*
@@ -326,15 +344,20 @@ int lxc_cgroup_set(const char *name, const char
*filename, const char *value,
ret = lxc_cgroup_path_get(&dirpath, filename, name, lxcpath);
if (ret)
- return -1;
+ goto fail;
ret = snprintf(path, MAXPATHLEN, "%s/%s", dirpath, filename);
if (ret < 0 || ret >= MAXPATHLEN) {
ERROR("pathname too long");
- return -1;
+ goto fail;
}
return do_cgroup_set(path, value);
+
+fail:
+ if(dirpath)
+ free(dirpath);
+ return -1;
}
/*
@@ -367,18 +390,18 @@ int lxc_cgroup_get(const char *name, const char
*filename, char *value,
ret = lxc_cgroup_path_get(&dirpath, filename, name, lxcpath);
if (ret)
- return -1;
+ goto fail;
rc = snprintf(path, MAXPATHLEN, "%s/%s", dirpath, filename);
if (rc < 0 || rc >= MAXPATHLEN) {
ERROR("pathname too long");
- return -1;
+ goto fail;
}
fd = open(path, O_RDONLY);
if (fd < 0) {
ERROR("open %s : %s", path, strerror(errno));
- return -1;
+ goto fail;
}
if (!len || !value) {
@@ -398,24 +421,28 @@ int lxc_cgroup_get(const char *name, const char
*filename, char *value,
close(fd);
return ret;
+fail:
+ if(dirpath)
+ free(dirpath);
+ return -1;
}
int lxc_cgroup_nrtasks(const char *cgpath)
{
- char *dpath;
+ char *dirpath;
char path[MAXPATHLEN];
int pid, ret, count = 0;
FILE *file;
int rc;
- ret = cgroup_path_get(&dpath, NULL, cgpath);
+ ret = cgroup_path_get(&dirpath, NULL, cgpath);
if (ret)
- return -1;
+ goto fail;
- rc = snprintf(path, MAXPATHLEN, "%s/tasks", dpath);
+ rc = snprintf(path, MAXPATHLEN, "%s/tasks", dirpath);
if (rc < 0 || rc >= MAXPATHLEN) {
ERROR("pathname too long");
- return -1;
+ goto fail;
}
file = fopen(path, "r");
@@ -430,6 +457,10 @@ int lxc_cgroup_nrtasks(const char *cgpath)
fclose(file);
return count;
+fail:
+ if(dirpath)
+ free(dirpath);
+ return -1;
}
/*
@@ -470,21 +501,23 @@ static void set_clone_children(const char *mntdir)
static int create_lxcgroups(const char *lxcgroup)
{
FILE *file = NULL;
- struct mntent *mntent;
+ struct mntent *mntent, mntent_r;
int ret, retv = -1;
char path[MAXPATHLEN];
+ char buf[MAXPATHLEN] = {0};
+
file = setmntent(MTAB, "r");
if (!file) {
SYSERROR("failed to open %s", MTAB);
return -1;
}
- while ((mntent = getmntent(file))) {
+ while ((mntent = getmntent_r(file, &mntent_r, buf, sizeof(buf)))) {
- if (strcmp(mntent->mnt_type, "cgroup"))
+ if (strcmp(mntent_r.mnt_type, "cgroup"))
continue;
- if (!mount_has_subsystem(mntent))
+ if (!mount_has_subsystem(&mntent_r))
continue;
/*
@@ -492,11 +525,11 @@ static int create_lxcgroups(const char *lxcgroup)
* We probably only want to support that for /users/joe
*/
ret = snprintf(path, MAXPATHLEN, "%s/%s",
- mntent->mnt_dir, lxcgroup ? lxcgroup : "lxc");
+ mntent_r.mnt_dir, lxcgroup ? lxcgroup : "lxc");
if (ret < 0 || ret >= MAXPATHLEN)
goto fail;
if (access(path, F_OK)) {
- set_clone_children(mntent->mnt_dir);
+ set_clone_children(mntent_r.mnt_dir);
ret = mkdir(path, 0755);
if (ret == -1 && errno != EEXIST) {
SYSERROR("failed to create '%s' directory",
path);
@@ -542,7 +575,9 @@ char *lxc_cgroup_path_create(const char *lxcgroup, const
char *name)
char *retpath, path[MAXPATHLEN];
char tail[12];
FILE *file = NULL;
- struct mntent *mntent;
+ struct mntent *mntent, mntent_r;
+
+ char buf[MAXPATHLEN] = {0};
if (create_lxcgroups(lxcgroup) < 0)
return NULL;
@@ -559,15 +594,15 @@ again:
else
*tail = '\0';
- while ((mntent = getmntent(file))) {
+ while ((mntent = getmntent_r(file, &mntent_r, buf, sizeof(buf)))) {
- if (strcmp(mntent->mnt_type, "cgroup"))
+ if (strcmp(mntent_r.mnt_type, "cgroup"))
continue;
- if (!mount_has_subsystem(mntent))
+ if (!mount_has_subsystem(&mntent_r))
continue;
/* find unused mnt_dir + lxcgroup + name + -$i */
- ret = snprintf(path, MAXPATHLEN, "%s/%s/%s%s", mntent->mnt_dir,
+ ret = snprintf(path, MAXPATHLEN, "%s/%s/%s%s", mntent_r.mnt_dir,
lxcgroup ? lxcgroup : "lxc", name, tail);
if (ret < 0 || ret >= MAXPATHLEN)
goto fail;
@@ -607,8 +642,10 @@ int lxc_cgroup_enter(const char *cgpath, pid_t pid)
{
char path[MAXPATHLEN];
FILE *file = NULL, *fout;
- struct mntent *mntent;
+ struct mntent *mntent, mntent_r;
int ret, retv = -1;
+ char buf[MAXPATHLEN] = {0};
+
file = setmntent(MTAB, "r");
if (!file) {
@@ -616,13 +653,13 @@ int lxc_cgroup_enter(const char *cgpath, pid_t pid)
return -1;
}
- while ((mntent = getmntent(file))) {
- if (strcmp(mntent->mnt_type, "cgroup"))
+ while ((mntent = getmntent_r(file, &mntent_r, buf, sizeof(buf)))) {
+ if (strcmp(mntent_r.mnt_type, "cgroup"))
continue;
- if (!mount_has_subsystem(mntent))
+ if (!mount_has_subsystem(&mntent_r))
continue;
ret = snprintf(path, MAXPATHLEN, "%s/%s/tasks",
- mntent->mnt_dir, cgpath);
+ mntent_r.mnt_dir, cgpath);
if (ret < 0 || ret >= MAXPATHLEN) {
ERROR("entering cgroup");
goto out;
@@ -714,23 +751,25 @@ static int lxc_one_cgroup_destroy(struct mntent *mntent,
const char *cgpath)
*/
int lxc_cgroup_destroy(const char *cgpath)
{
- struct mntent *mntent;
+ struct mntent *mntent, mntent_r;
FILE *file = NULL;
int err, retv = 0;
+ char buf[MAXPATHLEN] = {0};
+
file = setmntent(MTAB, "r");
if (!file) {
SYSERROR("failed to open %s", MTAB);
return -1;
}
- while ((mntent = getmntent(file))) {
- if (strcmp(mntent->mnt_type, "cgroup"))
+ while ((mntent = getmntent_r(file, &mntent_r, buf, sizeof(buf)))) {
+ if (strcmp(mntent_r.mnt_type, "cgroup"))
continue;
- if (!mount_has_subsystem(mntent))
+ if (!mount_has_subsystem(&mntent_r))
continue;
- err = lxc_one_cgroup_destroy(mntent, cgpath);
+ err = lxc_one_cgroup_destroy(&mntent_r, cgpath);
if (err) // keep trying to clean up the others
retv = -1;
}
diff --git a/src/lxc/freezer.c b/src/lxc/freezer.c
index 111bc35..ca9aa9c 100644
--- a/src/lxc/freezer.c
+++ b/src/lxc/freezer.c
@@ -125,9 +125,14 @@ static int freeze_unfreeze(const char *name, int freeze,
const char *lxcpath)
ret = lxc_cgroup_path_get(&nsgroup, "freezer", name, lxcpath);
if (ret)
- return -1;
+ goto fail;
return do_unfreeze(nsgroup, freeze, name, lxcpath);
+
+fail:
+ if (nsgroup)
+ free(nsgroup);
+ return -1;
}
int lxc_freeze(const char *name, const char *lxcpath)
@@ -148,7 +153,12 @@ int lxc_unfreeze_bypath(const char *cgpath)
ret = cgroup_path_get(&nsgroup, "freezer", cgpath);
if (ret)
- return -1;
+ goto fail;
return do_unfreeze(nsgroup, 0, NULL, NULL);
+
+fail:
+ if (nsgroup)
+ free(nsgroup);
+ return -1;
}
diff --git a/src/lxc/state.c b/src/lxc/state.c
index c50ef00..f83c16b 100644
--- a/src/lxc/state.c
+++ b/src/lxc/state.c
@@ -76,25 +76,30 @@ static int freezer_state(const char *name, const char
*lxcpath)
err = lxc_cgroup_path_get(&nsgroup, "freezer", name, lxcpath);
if (err)
- return -1;
+ goto fail;
err = snprintf(freezer, MAXPATHLEN, "%s/freezer.state", nsgroup);
if (err < 0 || err >= MAXPATHLEN)
- return -1;
+ goto fail;
file = fopen(freezer, "r");
if (!file)
- return -1;
+ goto fail;
err = fscanf(file, "%s", status);
fclose(file);
if (err == EOF) {
SYSERROR("failed to read %s", freezer);
- return -1;
+ goto fail;
}
return lxc_str2state(status);
+
+fail:
+ if (nsgroup)
+ free(nsgroup);
+ return -1;
}
static lxc_state_t __lxc_getstate(const char *name, const char *lxcpath)
--
1.7.10.4
------------------------------------------------------------------------------
Precog is a next-generation analytics platform capable of advanced
analytics on semi-structured data. The platform includes APIs for building
apps and a phenomenal toolset for data science. Developers can use
our toolset for easy data analysis & visualization. Get a free account!
http://www2.precog.com/precogplatform/slashdotnewsletter
_______________________________________________
Lxc-devel mailing list
Lxc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-devel
