Skip to content

L
lxc
Unverified Commit fd37327f by S.Çağlar Onur Committed by Stéphane Graber
Options

Support stopping containers concurrently

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. Signed-off-by: 's avatarS.Çağlar Onur <caglar@10ur.org> Acked-by: 's avatarSerge E. Hallyn <serge.hallyn@ubuntu.com>
parent cf0f9033
Showing with 128 additions and 61 deletions
src/lxc/cgroup.c
...@@ -54,6 +54,11 @@ lxc_log_define(lxc_cgroup, lxc); ...@@ -54,6 +54,11 @@ lxc_log_define(lxc_cgroup, lxc);
#define MTAB "/proc/mounts" #define MTAB "/proc/mounts"
/* In the case of a bind mount, there could be two long pathnames in the
* mntent plus options so use large enough buffer size
*/
#define LARGE_MAXPATHLEN 4 * MAXPATHLEN
/* Check if a mount is a cgroup hierarchy for any subsystem. /* Check if a mount is a cgroup hierarchy for any subsystem.
* Return the first subsystem found (or NULL if none). * Return the first subsystem found (or NULL if none).
*/ */
...@@ -100,29 +105,31 @@ static char *mount_has_subsystem(const struct mntent *mntent) ...@@ -100,29 +105,31 @@ static char *mount_has_subsystem(const struct mntent *mntent)
*/ */
static int get_cgroup_mount(const char *subsystem, char *mnt) static int get_cgroup_mount(const char *subsystem, char *mnt)
{ {
struct mntent *mntent; struct mntent *mntent, mntent_r;
FILE *file = NULL; FILE *file = NULL;
int ret, err = -1; int ret, err = -1;
char buf[LARGE_MAXPATHLEN] = {0};
file = setmntent(MTAB, "r"); file = setmntent(MTAB, "r");
if (!file) { if (!file) {
SYSERROR("failed to open %s", MTAB); SYSERROR("failed to open %s", MTAB);
return -1; 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") != 0)
continue; continue;
if (subsystem) { if (subsystem) {
if (!hasmntopt(mntent, subsystem)) if (!hasmntopt(&mntent_r, subsystem))
continue; continue;
} else { } else {
if (!mount_has_subsystem(mntent)) if (!mount_has_subsystem(&mntent_r))
continue; continue;
} }
ret = snprintf(mnt, MAXPATHLEN, "%s", mntent->mnt_dir); ret = snprintf(mnt, MAXPATHLEN, "%s", mntent_r.mnt_dir);
if (ret < 0 || ret >= MAXPATHLEN) if (ret < 0 || ret >= MAXPATHLEN)
goto fail; goto fail;
...@@ -148,22 +155,33 @@ out: ...@@ -148,22 +155,33 @@ out:
* *
* Returns 0 on success, -1 on error. * 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) extern int cgroup_path_get(char **path, const char *subsystem, const char *cgpath)
{ {
static char buf[MAXPATHLEN];
static char retbuf[MAXPATHLEN];
int rc; int rc;
char *buf = NULL;
char *retbuf = NULL;
buf = malloc(MAXPATHLEN * sizeof(char));
if (!buf) {
ERROR("malloc failed");
goto fail;
}
retbuf = malloc(MAXPATHLEN * sizeof(char));
if (!retbuf) {
ERROR("malloc failed");
goto fail;
}
/* lxc_cgroup_set passes a state object for the subsystem, /* lxc_cgroup_set passes a state object for the subsystem,
* so trim it to just the subsystem part */ * so trim it to just the subsystem part */
if (subsystem) { if (subsystem) {
rc = snprintf(retbuf, MAXPATHLEN, "%s", subsystem); rc = snprintf(retbuf, MAXPATHLEN, "%s", subsystem);
if (rc < 0 || rc >= MAXPATHLEN) { if (rc < 0 || rc >= MAXPATHLEN) {
ERROR("subsystem name too long"); ERROR("subsystem name too long");
return -1; goto fail;
} }
char *s = index(retbuf, '.'); char *s = index(retbuf, '.');
if (s) if (s)
...@@ -172,19 +190,28 @@ extern int cgroup_path_get(char **path, const char *subsystem, const char *cgpat ...@@ -172,19 +190,28 @@ extern int cgroup_path_get(char **path, const char *subsystem, const char *cgpat
} }
if (get_cgroup_mount(subsystem ? retbuf : NULL, buf)) { if (get_cgroup_mount(subsystem ? retbuf : NULL, buf)) {
ERROR("cgroup is not mounted"); ERROR("cgroup is not mounted");
return -1; goto fail;
} }
rc = snprintf(retbuf, MAXPATHLEN, "%s/%s", buf, cgpath); rc = snprintf(retbuf, MAXPATHLEN, "%s/%s", buf, cgpath);
if (rc < 0 || rc >= MAXPATHLEN) { if (rc < 0 || rc >= MAXPATHLEN) {
ERROR("name too long"); ERROR("name too long");
return -1; goto fail;
} }
DEBUG("%s: returning %s for subsystem %s", __func__, retbuf, subsystem); DEBUG("%s: returning %s for subsystem %s", __func__, retbuf, subsystem);
if(buf)
free(buf);
*path = retbuf; *path = retbuf;
return 0; return 0;
fail:
if (buf)
free(buf);
if (retbuf)
free(retbuf);
return -1;
} }
/* /*
...@@ -292,20 +319,25 @@ static int do_cgroup_set(const char *path, const char *value) ...@@ -292,20 +319,25 @@ static int do_cgroup_set(const char *path, const char *value)
int lxc_cgroup_set_bypath(const char *cgpath, const char *filename, const char *value) int lxc_cgroup_set_bypath(const char *cgpath, const char *filename, const char *value)
{ {
int ret; int ret;
char *dirpath; char *dirpath = NULL;
char path[MAXPATHLEN]; char path[MAXPATHLEN];
ret = cgroup_path_get(&dirpath, filename, cgpath); ret = cgroup_path_get(&dirpath, filename, cgpath);
if (ret) if (ret)
return -1; goto fail;
ret = snprintf(path, MAXPATHLEN, "%s/%s", dirpath, filename); ret = snprintf(path, MAXPATHLEN, "%s/%s", dirpath, filename);
if (ret < 0 || ret >= MAXPATHLEN) { if (ret < 0 || ret >= MAXPATHLEN) {
ERROR("pathname too long"); ERROR("pathname too long");
return -1; goto fail;
} }
return do_cgroup_set(path, value); return do_cgroup_set(path, value);
fail:
if(dirpath)
free(dirpath);
return -1;
} }
/* /*
...@@ -323,20 +355,25 @@ int lxc_cgroup_set(const char *name, const char *filename, const char *value, ...@@ -323,20 +355,25 @@ int lxc_cgroup_set(const char *name, const char *filename, const char *value,
const char *lxcpath) const char *lxcpath)
{ {
int ret; int ret;
char *dirpath; char *dirpath = NULL;
char path[MAXPATHLEN]; char path[MAXPATHLEN];
ret = lxc_cgroup_path_get(&dirpath, filename, name, lxcpath); ret = lxc_cgroup_path_get(&dirpath, filename, name, lxcpath);
if (ret) if (ret)
return -1; goto fail;
ret = snprintf(path, MAXPATHLEN, "%s/%s", dirpath, filename); ret = snprintf(path, MAXPATHLEN, "%s/%s", dirpath, filename);
if (ret < 0 || ret >= MAXPATHLEN) { if (ret < 0 || ret >= MAXPATHLEN) {
ERROR("pathname too long"); ERROR("pathname too long");
return -1; goto fail;
} }
return do_cgroup_set(path, value); return do_cgroup_set(path, value);
fail:
if(dirpath)
free(dirpath);
return -1;
} }
/* /*
...@@ -363,24 +400,24 @@ int lxc_cgroup_get(const char *name, const char *filename, char *value, ...@@ -363,24 +400,24 @@ int lxc_cgroup_get(const char *name, const char *filename, char *value,
size_t len, const char *lxcpath) size_t len, const char *lxcpath)
{ {
int fd, ret = -1; int fd, ret = -1;
char *dirpath; char *dirpath = NULL;
char path[MAXPATHLEN]; char path[MAXPATHLEN];
int rc; int rc;
ret = lxc_cgroup_path_get(&dirpath, filename, name, lxcpath); ret = lxc_cgroup_path_get(&dirpath, filename, name, lxcpath);
if (ret) if (ret)
return -1; goto fail;
rc = snprintf(path, MAXPATHLEN, "%s/%s", dirpath, filename); rc = snprintf(path, MAXPATHLEN, "%s/%s", dirpath, filename);
if (rc < 0 || rc >= MAXPATHLEN) { if (rc < 0 || rc >= MAXPATHLEN) {
ERROR("pathname too long"); ERROR("pathname too long");
return -1; goto fail;
} }
fd = open(path, O_RDONLY); fd = open(path, O_RDONLY);
if (fd < 0) { if (fd < 0) {
ERROR("open %s : %s", path, strerror(errno)); ERROR("open %s : %s", path, strerror(errno));
return -1; goto fail;
} }
if (!len || !value) { if (!len || !value) {
...@@ -400,24 +437,28 @@ int lxc_cgroup_get(const char *name, const char *filename, char *value, ...@@ -400,24 +437,28 @@ int lxc_cgroup_get(const char *name, const char *filename, char *value,
close(fd); close(fd);
return ret; return ret;
fail:
if(dirpath)
free(dirpath);
return -1;
} }
int lxc_cgroup_nrtasks(const char *cgpath) int lxc_cgroup_nrtasks(const char *cgpath)
{ {
char *dpath; char *dirpath = NULL;
char path[MAXPATHLEN]; char path[MAXPATHLEN];
int pid, ret, count = 0; int pid, ret, count = 0;
FILE *file; FILE *file;
int rc; int rc;
ret = cgroup_path_get(&dpath, NULL, cgpath); ret = cgroup_path_get(&dirpath, NULL, cgpath);
if (ret) 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) { if (rc < 0 || rc >= MAXPATHLEN) {
ERROR("pathname too long"); ERROR("pathname too long");
return -1; goto fail;
} }
file = fopen(path, "r"); file = fopen(path, "r");
...@@ -432,6 +473,10 @@ int lxc_cgroup_nrtasks(const char *cgpath) ...@@ -432,6 +473,10 @@ int lxc_cgroup_nrtasks(const char *cgpath)
fclose(file); fclose(file);
return count; return count;
fail:
if(dirpath)
free(dirpath);
return -1;
} }
/* /*
...@@ -472,33 +517,35 @@ static void set_clone_children(const char *mntdir) ...@@ -472,33 +517,35 @@ static void set_clone_children(const char *mntdir)
static int create_lxcgroups(const char *lxcgroup) static int create_lxcgroups(const char *lxcgroup)
{ {
FILE *file = NULL; FILE *file = NULL;
struct mntent *mntent; struct mntent *mntent, mntent_r;
int ret, retv = -1; int ret, retv = -1;
char path[MAXPATHLEN]; char path[MAXPATHLEN];
char buf[LARGE_MAXPATHLEN] = {0};
file = setmntent(MTAB, "r"); file = setmntent(MTAB, "r");
if (!file) { if (!file) {
SYSERROR("failed to open %s", MTAB); SYSERROR("failed to open %s", MTAB);
return -1; 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; continue;
if (!mount_has_subsystem(mntent)) if (!mount_has_subsystem(&mntent_r))
continue; continue;
/* /*
* TODO - handle case where lxcgroup has subdirs? (i.e. build/l1) * TODO - handle case where lxcgroup has subdirs? (i.e. build/l1)
* We probably only want to support that for /users/joe * We probably only want to support that for /users/joe
*/ */
ret = snprintf(path, MAXPATHLEN, "%s/%s", ret = snprintf(path, MAXPATHLEN, "%s/%s",
mntent->mnt_dir, lxcgroup ? lxcgroup : "lxc"); mntent_r.mnt_dir, lxcgroup ? lxcgroup : "lxc");
if (ret < 0 || ret >= MAXPATHLEN) if (ret < 0 || ret >= MAXPATHLEN)
goto fail; goto fail;
if (access(path, F_OK)) { if (access(path, F_OK)) {
set_clone_children(mntent->mnt_dir); set_clone_children(mntent_r.mnt_dir);
ret = mkdir(path, 0755); ret = mkdir(path, 0755);
if (ret == -1 && errno != EEXIST) { if (ret == -1 && errno != EEXIST) {
SYSERROR("failed to create '%s' directory", path); SYSERROR("failed to create '%s' directory", path);
...@@ -535,7 +582,7 @@ fail: ...@@ -535,7 +582,7 @@ fail:
* freezer cgroup's full path will be /sys/fs/cgroup/freezer/lxc/r1/. * freezer cgroup's full path will be /sys/fs/cgroup/freezer/lxc/r1/.
* *
* XXX This should probably be locked globally * XXX This should probably be locked globally
* *
* Races won't be determintal, you'll just end up with leftover unused cgroups * Races won't be determintal, you'll just end up with leftover unused cgroups
*/ */
char *lxc_cgroup_path_create(const char *lxcgroup, const char *name) char *lxc_cgroup_path_create(const char *lxcgroup, const char *name)
...@@ -544,7 +591,9 @@ char *lxc_cgroup_path_create(const char *lxcgroup, const char *name) ...@@ -544,7 +591,9 @@ char *lxc_cgroup_path_create(const char *lxcgroup, const char *name)
char *retpath, path[MAXPATHLEN]; char *retpath, path[MAXPATHLEN];
char tail[12]; char tail[12];
FILE *file = NULL; FILE *file = NULL;
struct mntent *mntent; struct mntent *mntent, mntent_r;
char buf[LARGE_MAXPATHLEN] = {0};
if (create_lxcgroups(lxcgroup) < 0) if (create_lxcgroups(lxcgroup) < 0)
return NULL; return NULL;
...@@ -561,15 +610,15 @@ again: ...@@ -561,15 +610,15 @@ again:
else else
*tail = '\0'; *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; continue;
if (!mount_has_subsystem(mntent)) if (!mount_has_subsystem(&mntent_r))
continue; continue;
/* find unused mnt_dir + lxcgroup + name + -$i */ /* 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); lxcgroup ? lxcgroup : "lxc", name, tail);
if (ret < 0 || ret >= MAXPATHLEN) if (ret < 0 || ret >= MAXPATHLEN)
goto fail; goto fail;
...@@ -609,8 +658,9 @@ int lxc_cgroup_enter(const char *cgpath, pid_t pid) ...@@ -609,8 +658,9 @@ int lxc_cgroup_enter(const char *cgpath, pid_t pid)
{ {
char path[MAXPATHLEN]; char path[MAXPATHLEN];
FILE *file = NULL, *fout; FILE *file = NULL, *fout;
struct mntent *mntent; struct mntent *mntent, mntent_r;
int ret, retv = -1; int ret, retv = -1;
char buf[LARGE_MAXPATHLEN] = {0};
file = setmntent(MTAB, "r"); file = setmntent(MTAB, "r");
if (!file) { if (!file) {
...@@ -618,13 +668,13 @@ int lxc_cgroup_enter(const char *cgpath, pid_t pid) ...@@ -618,13 +668,13 @@ int lxc_cgroup_enter(const char *cgpath, pid_t pid)
return -1; 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; continue;
if (!mount_has_subsystem(mntent)) if (!mount_has_subsystem(&mntent_r))
continue; continue;
ret = snprintf(path, MAXPATHLEN, "%s/%s/tasks", ret = snprintf(path, MAXPATHLEN, "%s/%s/tasks",
mntent->mnt_dir, cgpath); mntent_r.mnt_dir, cgpath);
if (ret < 0 || ret >= MAXPATHLEN) { if (ret < 0 || ret >= MAXPATHLEN) {
ERROR("entering cgroup"); ERROR("entering cgroup");
goto out; goto out;
...@@ -716,23 +766,25 @@ static int lxc_one_cgroup_destroy(struct mntent *mntent, const char *cgpath) ...@@ -716,23 +766,25 @@ static int lxc_one_cgroup_destroy(struct mntent *mntent, const char *cgpath)
*/ */
int lxc_cgroup_destroy(const char *cgpath) int lxc_cgroup_destroy(const char *cgpath)
{ {
struct mntent *mntent; struct mntent *mntent, mntent_r;
FILE *file = NULL; FILE *file = NULL;
int err, retv = 0; int err, retv = 0;
char buf[LARGE_MAXPATHLEN] = {0};
file = setmntent(MTAB, "r"); file = setmntent(MTAB, "r");
if (!file) { if (!file) {
SYSERROR("failed to open %s", MTAB); SYSERROR("failed to open %s", MTAB);
return -1; 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; continue;
if (!mount_has_subsystem(mntent)) if (!mount_has_subsystem(&mntent_r))
continue; 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 if (err) // keep trying to clean up the others
retv = -1; retv = -1;
} }
......
src/lxc/freezer.c
...@@ -120,14 +120,19 @@ out: ...@@ -120,14 +120,19 @@ out:
static int freeze_unfreeze(const char *name, int freeze, const char *lxcpath) static int freeze_unfreeze(const char *name, int freeze, const char *lxcpath)
{ {
char *nsgroup; char *nsgroup = NULL;
int ret; int ret;
ret = lxc_cgroup_path_get(&nsgroup, "freezer", name, lxcpath); ret = lxc_cgroup_path_get(&nsgroup, "freezer", name, lxcpath);
if (ret) if (ret)
return -1; goto fail;
return do_unfreeze(nsgroup, freeze, name, lxcpath); return do_unfreeze(nsgroup, freeze, name, lxcpath);
fail:
if (nsgroup)
free(nsgroup);
return -1;
} }
int lxc_freeze(const char *name, const char *lxcpath) int lxc_freeze(const char *name, const char *lxcpath)
...@@ -143,12 +148,17 @@ int lxc_unfreeze(const char *name, const char *lxcpath) ...@@ -143,12 +148,17 @@ int lxc_unfreeze(const char *name, const char *lxcpath)
int lxc_unfreeze_bypath(const char *cgpath) int lxc_unfreeze_bypath(const char *cgpath)
{ {
char *nsgroup; char *nsgroup = NULL;
int ret; int ret;
ret = cgroup_path_get(&nsgroup, "freezer", cgpath); ret = cgroup_path_get(&nsgroup, "freezer", cgpath);
if (ret) if (ret)
return -1; goto fail;
return do_unfreeze(nsgroup, 0, NULL, NULL); return do_unfreeze(nsgroup, 0, NULL, NULL);
fail:
if (nsgroup)
free(nsgroup);
return -1;
} }
src/lxc/state.c
...@@ -69,7 +69,7 @@ lxc_state_t lxc_str2state(const char *state) ...@@ -69,7 +69,7 @@ lxc_state_t lxc_str2state(const char *state)
static lxc_state_t freezer_state(const char *name, const char *lxcpath) static lxc_state_t freezer_state(const char *name, const char *lxcpath)
{ {
char *nsgroup; char *nsgroup = NULL;
char freezer[MAXPATHLEN]; char freezer[MAXPATHLEN];
char status[MAXPATHLEN]; char status[MAXPATHLEN];
FILE *file; FILE *file;
...@@ -77,25 +77,30 @@ static lxc_state_t freezer_state(const char *name, const char *lxcpath) ...@@ -77,25 +77,30 @@ static lxc_state_t freezer_state(const char *name, const char *lxcpath)
err = lxc_cgroup_path_get(&nsgroup, "freezer", name, lxcpath); err = lxc_cgroup_path_get(&nsgroup, "freezer", name, lxcpath);
if (err) if (err)
return -1; goto fail;
err = snprintf(freezer, MAXPATHLEN, "%s/freezer.state", nsgroup); err = snprintf(freezer, MAXPATHLEN, "%s/freezer.state", nsgroup);
if (err < 0 || err >= MAXPATHLEN) if (err < 0 || err >= MAXPATHLEN)
return -1; goto fail;
file = fopen(freezer, "r"); file = fopen(freezer, "r");
if (!file) if (!file)
return -1; goto fail;
err = fscanf(file, "%s", status); err = fscanf(file, "%s", status);
fclose(file); fclose(file);
if (err == EOF) { if (err == EOF) {
SYSERROR("failed to read %s", freezer); SYSERROR("failed to read %s", freezer);
return -1; goto fail;
} }
return lxc_str2state(status); return lxc_str2state(status);
fail:
if (nsgroup)
free(nsgroup);
return -1;
} }
static lxc_state_t __lxc_getstate(const char *name, const char *lxcpath) static lxc_state_t __lxc_getstate(const char *name, const char *lxcpath)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment