Hi Serge,
On Thu, Jan 23, 2014 at 11:28:46AM -0600, Serge Hallyn wrote:
> Quoting Robert Vogelgesang (vo...@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
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
