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

Serge Hallyn serge.hallyn at ubuntu.com
Thu Jan 23 17:28:46 UTC 2014


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

Interesting;  I cut-pasted this from code we had quite some time ago,
else I would in fact have used lxc_read_from_file().

So given that in the past few years you are apparently the first person
to use this without cgroup.clone_children and without ns cgroup, I have
to ask is there another kernel you could just as easily be using?  If
not then let's proceed;  if so then I'd rather yank code that will very
rarely get tested.

(Of course long term I'd like to yank all the cgroupfs code :)

> >  /* 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.
> 

I've applied his patch.

> >  			} 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.

I don't seem to have that email, so could you please re-iterate?

thanks,
-serge


More information about the lxc-devel mailing list