From: S.Çağlar Onur Date: Wed, 17 Apr 2013 21:15:51 +0000 (-0400) Subject: Support stopping containers concurrently X-Git-Tag: lxc-2.1.1~2542^2~272 X-Git-Url: https://git.proxmox.com/?p=mirror_lxc.git;a=commitdiff_plain;h=fd37327f57a6d53692babcaf69dfbd8f62e59918 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.Çağlar Onur Acked-by: Serge E. Hallyn --- diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c index 368214f24..b6f3e6ea1 100644 --- a/src/lxc/cgroup.c +++ b/src/lxc/cgroup.c @@ -54,6 +54,11 @@ lxc_log_define(lxc_cgroup, lxc); #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. * Return the first subsystem found (or NULL if none). */ @@ -100,29 +105,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[LARGE_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; @@ -148,22 +155,33 @@ 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 = 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, * 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) @@ -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)) { 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; } /* @@ -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 ret; - char *dirpath; + char *dirpath = NULL; char path[MAXPATHLEN]; 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; } /* @@ -323,20 +355,25 @@ int lxc_cgroup_set(const char *name, const char *filename, const char *value, const char *lxcpath) { int ret; - char *dirpath; + char *dirpath = NULL; char path[MAXPATHLEN]; 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; } /* @@ -363,24 +400,24 @@ int lxc_cgroup_get(const char *name, const char *filename, char *value, size_t len, const char *lxcpath) { int fd, ret = -1; - char *dirpath; + char *dirpath = NULL; char path[MAXPATHLEN]; int rc; 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) { @@ -400,24 +437,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 = NULL; 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"); @@ -432,6 +473,10 @@ int lxc_cgroup_nrtasks(const char *cgpath) fclose(file); return count; +fail: + if(dirpath) + free(dirpath); + return -1; } /* @@ -472,33 +517,35 @@ 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[LARGE_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; - /* + /* * TODO - handle case where lxcgroup has subdirs? (i.e. build/l1) * 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); @@ -535,7 +582,7 @@ fail: * freezer cgroup's full path will be /sys/fs/cgroup/freezer/lxc/r1/. * * XXX This should probably be locked globally - * + * * 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) @@ -544,7 +591,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[LARGE_MAXPATHLEN] = {0}; if (create_lxcgroups(lxcgroup) < 0) return NULL; @@ -561,15 +610,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; @@ -609,8 +658,9 @@ 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[LARGE_MAXPATHLEN] = {0}; file = setmntent(MTAB, "r"); if (!file) { @@ -618,13 +668,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; @@ -716,23 +766,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[LARGE_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 111bc3540..35bf3a764 100644 --- a/src/lxc/freezer.c +++ b/src/lxc/freezer.c @@ -120,14 +120,19 @@ out: static int freeze_unfreeze(const char *name, int freeze, const char *lxcpath) { - char *nsgroup; + char *nsgroup = NULL; int ret; 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) @@ -143,12 +148,17 @@ int lxc_unfreeze(const char *name, const char *lxcpath) int lxc_unfreeze_bypath(const char *cgpath) { - char *nsgroup; + char *nsgroup = NULL; int ret; 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 437f11aa6..313cbb9ce 100644 --- a/src/lxc/state.c +++ b/src/lxc/state.c @@ -69,7 +69,7 @@ lxc_state_t lxc_str2state(const char *state) static lxc_state_t freezer_state(const char *name, const char *lxcpath) { - char *nsgroup; + char *nsgroup = NULL; char freezer[MAXPATHLEN]; char status[MAXPATHLEN]; FILE *file; @@ -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); 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)