Quoting Daniel Lezcano (daniel.lezc...@free.fr):
> Hi Serge,
>
> your patch was doing a nested call to the setmntent which is not reentrant.
>
> Fixed with the patch in attachment. Some other nits fixed too.
Looks good, thanks.
Acked-by: Serge Hallyn <serge.hal...@ubuntu.com>
Once this all settles down, we can of course consider re-introducing
caching the mounts we find so multiple lxc_cgroup_{sg}et()s are
quicker. But I figured optimizations should be done later.
> ---
> src/lxc/cgroup.c | 81
> +++++++++++++++++++++----------------------------------
> 1 file changed, 32 insertions(+), 49 deletions(-)
>
> Index: lxc/src/lxc/cgroup.c
> ===================================================================
> --- lxc.orig/src/lxc/cgroup.c
> +++ lxc/src/lxc/cgroup.c
> @@ -82,40 +82,19 @@ static int get_cgroup_mount(const char *
> return -1;
> }
>
> -static int get_cgroup_flags(const char *mtab, const char *mnt_dir, int
> *flags)
> +static int get_cgroup_flags(const struct mntent *mntent)
> {
> - struct mntent *mntent;
> - FILE *file = NULL;
> - int err = -1;
> + int flags = 0;
>
> - file = setmntent(mtab, "r");
> - if (!file) {
> - SYSERROR("failed to open %s", mtab);
> - return -1;
> - }
> + if (hasmntopt(mntent, "ns"))
> + flags |= CGROUP_NS_CGROUP;
>
> - *flags = 0;
> -- Daniel
> ---
> src/lxc/cgroup.c | 81
> +++++++++++++++++++++----------------------------------
> 1 file changed, 32 insertions(+), 49 deletions(-)
>
> Index: lxc/src/lxc/cgroup.c
> ===================================================================
> --- lxc.orig/src/lxc/cgroup.c
> +++ lxc/src/lxc/cgroup.c
> @@ -82,40 +82,19 @@ static int get_cgroup_mount(const char *
> return -1;
> }
>
> -static int get_cgroup_flags(const char *mtab, const char *mnt_dir, int
> *flags)
> +static int get_cgroup_flags(const struct mntent *mntent)
> {
> - struct mntent *mntent;
> - FILE *file = NULL;
> - int err = -1;
> + int flags = 0;
>
> - file = setmntent(mtab, "r");
> - if (!file) {
> - SYSERROR("failed to open %s", mtab);
> - return -1;
> - }
> + if (hasmntopt(mntent, "ns"))
> + flags |= CGROUP_NS_CGROUP;
>
> - *flags = 0;
> + if (hasmntopt(mntent, "clone_children"))
> + flags |= CGROUP_CLONE_CHILDREN;
>
> - while ((mntent = getmntent(file))) {
> - if (strcmp(mntent->mnt_type, "cgroup"))
> - continue;
> - if (strcmp(mntent->mnt_dir, mnt_dir))
> - continue;
> - if (hasmntopt(mntent, "ns")) {
> - *flags |= CGROUP_NS_CGROUP;
> - err = 0;
> - }
> - if (hasmntopt(mntent, "clone_children")) {
> - *flags |= CGROUP_CLONE_CHILDREN;
> - err = 0;
> - }
> - fclose(file);
> - DEBUG("cgroup flags for %s is 0x%x", mnt_dir, *flags);
> - return err;
> - }
> + DEBUG("cgroup '%s' has flags 0x%x", mntent->mnt_dir, flags);
>
> - fclose(file);
> - return err;
> + return flags;
> }
>
> static int cgroup_rename_nsgroup(const char *mnt, const char *name, pid_t
> pid)
> @@ -184,14 +163,15 @@ static int cgroup_attach(const char *pat
> * XXX TODO we will of course want to use cgroup_path{subsystem}/lxc/name,
> * not just cgroup_path{subsystem}/name.
> */
> -int lxc_one_cgroup_create(const char *name, const char *cgmnt, pid_t pid)
> +static int lxc_one_cgroup_create(const char *name,
> + const struct mntent *mntent, pid_t pid)
> {
> char cgname[MAXPATHLEN];
> char clonechild[MAXPATHLEN];
> int flags;
>
> - 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,
> * otherwise that fails
> @@ -201,18 +181,18 @@ int lxc_one_cgroup_create(const char *na
> return -1;
> }
>
> - if (get_cgroup_flags(MTAB, cgmnt, &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);
> +
> + DEBUG("checking '%s' presence", clonechild);
>
> /* 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
> @@ -242,6 +222,8 @@ int lxc_one_cgroup_create(const char *na
> return -1;
> }
>
> + INFO("created cgroup '%s'", cgname);
> +
> return 0;
> }
>
> @@ -252,9 +234,8 @@ int lxc_cgroup_create(const char *name,
> {
> struct mntent *mntent;
> FILE *file = NULL;
> - int ret, err = -1;
> + int err = -1;
>
> - DEBUG("%s: starting\n", __func__);
> file = setmntent(MTAB, "r");
> if (!file) {
> SYSERROR("failed to open %s", MTAB);
> @@ -262,19 +243,21 @@ int lxc_cgroup_create(const char *name,
> }
>
> while ((mntent = getmntent(file))) {
> +
> + DEBUG("checking '%s' (%s)", mntent->mnt_dir, mntent->mnt_type);
> +
> if (!strcmp(mntent->mnt_type, "cgroup")) {
> - DEBUG("creating %s %s %d\n", name, mntent->mnt_dir,
> pid);
> - err = lxc_one_cgroup_create(name, mntent->mnt_dir, pid);
> - if (ret) {
> - fclose(file);
> - return ret;
> - }
> - err = 0;
> +
> + INFO("found cgroup mounted at '%s'", mntent->mnt_dir);
> + err = lxc_one_cgroup_create(name, mntent, pid);
> + if (err)
> + goto out;
> +
> }
> };
>
> - fclose(file);
> -
> +out:
> + endmntent(file);
> return err;
> }
>
------------------------------------------------------------------------------
All of the data generated in your IT infrastructure is seriously valuable.
Why? It contains a definitive record of application performance, security
threats, fraudulent activity, and more. Splunk takes this data and makes
sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-d2d-c2
_______________________________________________
Lxc-devel mailing list
Lxc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-devel
