[lxc-devel] [PATCH 2/2] split cgroup handling into discrete backends

[Serge Hallyn]

Quoting Dwight Engen (dwight.engen at oracle.com):
> On Thu, 6 Feb 2014 10:49:22 -0600
> Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> 
> > Quoting Dwight Engen (dwight.engen at oracle.com):
> > > On Wed, 5 Feb 2014 23:34:16 -0600
> > > Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> > > 
> > > > Quoting Serge Hallyn (serge.hallyn at ubuntu.com):
> > > > > Quoting Dwight Engen (dwight.engen at oracle.com):
> > > > > > - refactor cgroup into two backends, the classic cgfs driver
> > > > > > and the new cgmanager. Instead of lxc_handler knowing about
> > > > > > the internals of each, have it just store an opaque pointer
> > > > > > to a struct that is private to each backend.
> > > > > > 
> > > > > > - rename a couple of cgroup functions for consistency: those
> > > > > > that are considered an API (ie. exported by lxc.h) begin with
> > > > > > lxc_ and those that are not are just cgroup_*
> > > > > > 
> > > > > > - made as many backend routines static as possible, only
> > > > > > cg*_ops_init is exported
> > > > > > 
> > > > > > - made a nrtasks op which is needed by the utmp code for
> > > > > > monitoring container shutdown, currently only implemented for
> > > > > > the cgfs backend
> > > > > 
> > > > > Hi Dwight,
> > > > > 
> > > > > boy this diff got into some unfortunate strides (with
> > > > > near-identical code being removed then inserted pages later)
> > > > > making it harder to read than necessary :)
> > > > > 
> > > > > > Signed-off-by: Dwight Engen <dwight.engen at oracle.com>
> > > > > 
> > > > > Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
> > > > > 
> > > > > (I'll apply after I run some tests)
> > > > 
> > > > Unfortunately, while containers started fine, lxc-attach and
> > > > lxc-cgroup no longer seemed to work.  If it isn't obvious to
> > > > you what the problem migth be, I'll look into it tomorrow.
> > > 
> > > Hmm, thats strange. I tested both of those with both backends. I'll
> > > try to reproduce it here. I also tested the shutdown while frozen
> > > case and at first I thought it was broken but then I realized that
> > > you just have to wait for the timeout to occur (which is the same
> > > as it used to be).
> > 
> > Works now, not sure what I did last night.
> > 
> > Pushed - thanks.
> > 
> > -serge
> 
> Hey Serge,
> 
> I do have one more question: do we expect that cgmanager will only be
> used with a kernel that supports reboot(2) for container shutdown? (ie.
> kernel commit 923c7538). Looks like this has been in since 3.9.

No we expect it to work with older kernels (at least back to 3.2), else
the code could have been quite a bit simpler in some places.

> I ask because I tested that the utmp watch stuff still works for cgfs,
> but obviously it doesn't right now with cgmanager since the nrtasks op
> isn't filled in.

Should be trivial to fill in, using GetTasks and counting the results.
Do you think that a separate NrTasks dbus method would be worthwhile?

For what it's worth, some other methods which I do plan to implement at
some point but are lower priority for me (as I don't need them) include
GetChildren (see list of child cgroups), get_eventfd (to get something
to watch for changes), and remove_on_empty.  RemoveOnEmpty is waiting
for a redisgned notify-on-release which Tejun has said to be working on
right now, and likewise get_eventfd is waiting to see how that is
re-designed.  Patches for all of these welcomem :)

-serge