Quoting Robert Vogelgesang ([email protected]): > Hi Serge, > > your patch uses the same concept as mine, but it won't work, as noted > below. > > If you can wait until Monday, I could send you a cleaned-up and tested > version of my patch; I have to do other things today and tomorrow.
Thanks, Monday will be great. > On Wed, Jan 22, 2014 at 10:49:00AM -0600, Serge Hallyn wrote: > > When creating a cgroup, detect whether cgroup.clone_children > > exists. If not, then manually copy the parent's cpuset.cpus > > and cpuset.mems values. > > > > Signed-off-by: Serge Hallyn <[email protected]> > > --- > > src/lxc/cgroup.c | 72 > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 72 insertions(+) > > > > diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c > > index 4482b32..49f1ba1 100644 > > --- a/src/lxc/cgroup.c > > +++ b/src/lxc/cgroup.c > > @@ -74,6 +74,7 @@ static int do_setup_cgroup_limits(struct lxc_handler *h, > > struct lxc_list *cgroup > > static int cgroup_recursive_task_count(const char *cgroup_path); > > static int count_lines(const char *fn); > > static int handle_cgroup_settings(struct cgroup_mount_point *mp, char > > *cgroup_path); > > +static void setup_cpuset_if_needed(char **subsystems, char *path); > > > > static struct cgroup_ops cgfs_ops; > > struct cgroup_ops *active_cg_ops = &cgfs_ops; > > @@ -723,6 +724,71 @@ static char *cgroup_rename_nsgroup(const char > > *mountpath, const char *oldname, p > > return newname; > > } > > > > +static long get_value(const char *dir, const char *file) > > +{ > > + FILE *f; > > + char path[MAXPATHLEN]; > > + int ret, retv; > > + > > + retv = snprintf(path, MAXPATHLEN, "%s/%s", dir, file); > > + if (retv < 0 || retv >= MAXPATHLEN) > > + return 0; > > + f = fopen(path, "r"); > > + ret = fscanf(f, "%d", &retv); > > This is not sufficient, because cpuset.cpus and cpuset.mems do not contain > plain decimals, but lists and ranges of decimals. You have to use %s here. > > I used lxc_read_from_file() to read the values, a buffer of size 128, > checked if the buffer was large enough, and errored out if it was not. > > > > + fclose(f); > > + if (ret != 1) > > + return 0; > > + return retv; > > +} > > + > > +static void set_value(const char *dir, const char *file, long v) > > +{ > > + FILE *f; > > + char path[MAXPATHLEN]; > > + int retv; > > + > > + retv = snprintf(path, MAXPATHLEN, "%s/%s", dir, file); > > + if (retv < 0 || retv >= MAXPATHLEN) > > + return; > > + f = fopen(path, "w"); > > + fprintf(f, "%ld\n", v); > > + fclose(f); > > +} > > This could be replaced with lxc_write_to_file(). > > > + > > +static bool file_exists(const char *dir, const char *file) > > +{ > > + char path[MAXPATHLEN]; > > + struct stat sb; > > + int ret; > > + > > + ret = snprintf(path, MAXPATHLEN, "%s/%s", dir, file); > > + if (ret < 0 || ret >= MAXPATHLEN) > > + return true; > > + ret = stat(path, &sb); > > + return ret == 0; > > +} > > + > > +static void setup_cpuset_if_needed(char **subsystems, char *path) > > +{ > > + char *parentpath, *p; > > + long v; > > + > > + if (!lxc_string_in_array("cpuset", (const char **) subsystems)) > > + return; > > + if (file_exists(path, "cgroup.clone_children")) > > + return; > > + parentpath = strdup(path); > > + if (!parentpath) > > + return; > > + if ((p = rindex(parentpath, '/'))) > > + *p = '\0'; > > + v = get_value(parentpath, "cpuset.mems"); > > + set_value(path, "cpuset.mems", v); > > + v = get_value(parentpath, "cpuset.cpus"); > > + set_value(path, "cpuset.cpus", v); > > + free(parentpath); > > +} > > + > > /* create a new cgroup */ > > struct cgroup_process_info *lxc_cgroupfs_create(const char *name, const > > char *path_pattern, struct cgroup_meta_data *meta_data, const char > > *sub_pattern) > > { > > @@ -898,6 +964,8 @@ struct cgroup_process_info *lxc_cgroupfs_create(const > > char *name, const char *pa > > if (r < 0) > > goto cleanup_from_error; > > > > info_ptr->created_paths[info_ptr->created_paths_count++] = > > current_entire_path; > > + > > setup_cpuset_if_needed(info_ptr->hierarchy->subsystems, > > + current_entire_path); > > As Qiang Huang already wrote to the list, you need the full path here. > > > > } else { > > /* if we didn't create the cgroup, then we have > > to make sure that > > * further cgroups will be created properly > > What about this "else" code path, shouldn't setup_cpuset_if_needed() > be called here, too? > > Note: This comment is followed by a call to handle_cgroup_settings() > that has the first argument wrong, as I already wrote to the list > yesterday in the "Containers do not start with lxc-1.0.0.beta2 on > RHEL-6.5" thread. > > Robert > > > > @@ -2039,8 +2107,12 @@ static int handle_cgroup_settings(struct > > cgroup_mount_point *mp, > > */ > > if (lxc_string_in_array("cpuset", (const char > > **)mp->hierarchy->subsystems)) { > > char *cc_path = cgroup_to_absolute_path(mp, cgroup_path, > > "/cgroup.clone_children"); > > + struct stat sb; > > + > > if (!cc_path) > > return -1; > > + if (stat(cc_path, &sb) != 0 && errno == ENOENT) > > + return 0; > > r = lxc_read_from_file(cc_path, buf, 1); > > if (r == 1 && buf[0] == '1') { > > free(cc_path); > > -- > > 1.8.5.3 _______________________________________________ lxc-devel mailing list [email protected] http://lists.linuxcontainers.org/listinfo/lxc-devel
