[lxc-devel] [PATCH 1/1] cgroupfs: support older kernels without cgroup.clone_children

[Serge Hallyn]

Quoting Robert Vogelgesang (vogel at users.sourceforge.net):
> 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 <serge.hallyn at ubuntu.com>
> > ---
> >  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