[Lxc-users] [PATCH 2/2] cgroups: support cgroups mounted in multiple places

[Serge E. Hallyn]

Quoting Michael H. Warfield (mhw at WittsEnd.com):

Thanks, Michael, good catch.

> > Now wait a minute.  Is that a typo here:

No it's not, but:

> > char *s = index(retbuf, '.');
> > 
> > If you're doing, in effect, a dirname here should that be this:
> > 
> > char *s = index(retbuf, '/');
> > 
> > IAC...  That "*s = '\0';" should include a NULL check.
> > 
> > Adding the NULL check and lxc-info works.
> > 
> > Looks like that subsystem name in the call to that routine is not what
> > Serge thought it was.  I threw a print above the snprintf about just for
> > giggles to print out the subsystem name being passed to it and this is
> > what I got back...
> > 
> > [mhw at forest SPECS]$ sudo lxc-info -n Alcove
> > subsystem name: "freezer"
> > 'Alcove' is RUNNING
> > 
> > No wonder "s" was null.  No dot and no /.
> 
> I applied this patch and it got lxc-info working.  But it was a quick
> hack just to address the NULL pointer.  Is it the correct fix?

No, it's not.

For the calls to this function that come from cgroup.c itself, '.' is the
right thing.  The problem is that lxc_cgroup_set() and lxc_cgroup_get()
pass in things like 'devices.allow'.  I was going to make the index
conditional, but all the callers of this function pass in either a filename
(with a '.' in it) or NULL.

I failed to notice these:

src/lxc/freezer.c:      ret = lxc_cgroup_path_get(&nsgroup, "freezer", name);
src/lxc/state.c:        err = lxc_cgroup_path_get(&nsgroup, "freezer", name);

:)

These are what you are running into.

So the thing to do is leave it searching for index(s, '.') but do nothing
if s is NULL.

Really it would be cleaner to have lxc_cgroup_{sg}et() do the index, so
that lxc_cgorup_path_get() always gets a subsystem or NULL.  I'm not doing
that patch right now, though, trivial as it ought to be.

thanks,
-serge