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

[Robert Vogelgesang]

Hi Serge,

On Thu, Jan 23, 2014 at 11:28:46AM -0600, Serge Hallyn wrote:
> 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().

this does only prove that no-one noticed that it did not work "full-spec".

If you copy only the first digit, the cgroup is initialized and works.
The difference is only that it is restricted to using less resources
than the admin might have intended.


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

No, I'm only the first person that is using 1.0.0.beta2 under RHEL-6.5.
lxc-0.9.0 does work with the standard RHEL-6 kernel, as long as the
admin doesn't care about cpuset cgroups.

RHEL-6 offers only one kernel for the x86_64 architecture (which is
currently version 2.6.32-431.3.1.el6.x86_64), there are no other
options, at least if you strictly follow Redhat.


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

:-)

> 
[...]
> > >  			} 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?

I basically comes down to this patch, against the cgroup.c from
the 1.0.0.beta2 tarball:

diff -u lxc-lxc-1.0.0.beta2/src/lxc/cgroup.c.orig lxc-lxc-1.0.0.beta2/src/lxc/cgroup.c
--- lxc-lxc-1.0.0.beta2/src/lxc/cgroup.c.orig	2014-01-16 01:07:33.000000000 +0100
+++ lxc-lxc-1.0.0.beta2/src/lxc/cgroup.c	2014-01-22 17:50:48.169119388 +0100
@@ -887,7 +887,7 @@
 				/* if we didn't create the cgroup, then we have to make sure that
 				 * further cgroups will be created properly
 				 */
-				if (handle_cgroup_settings(mp, info_ptr->cgroup_path) < 0) {
+				if (handle_cgroup_settings(info_ptr->designated_mount_point, info_ptr->cgroup_path) < 0) {
 					ERROR("Could not set clone_children to 1 for cpuset hierarchy in pre-existing cgroup.");
 					goto cleanup_from_error;
 				}
@@ -2005,7 +2005,7 @@
 			if (r < 1 || buf[0] != '1') {
 				r = lxc_write_to_file(cc_path, "1", 1, false);
 				if (r < 0)
-					SYSERROR("failed to set memory.use_hiararchy to 1; continuing");
+					SYSERROR("failed to set memory.use_hierarchy to 1; continuing");
 			}
 			free(cc_path);
 		}


I noticed this, because I originally had used the same arguments for
my version of setup_cpuset_if_needed(), which did not work.  "mp" is
set near the start of lxc_cgroupfs_create() and does not change in
the loop where this call to handle_cgroup_settings() takes place.

	Robert