[FFE] Add cgroup-lite package

I can confirm that this fixes the races I saw with cgroup-bin and works perfectly for me with libvirt, kvm and arkose.

I'd definitely +1 getting this uploaded and replace the current Recommends on the lxc package from "cgroup-bin" to "cgroup-lite | cgroup-bin" so that both are still allowed but cgroup-lite is preferred.

Subscribing ubuntu-release team to get the FFe reviewed.

If accepted, we'll then update the lxc package accordingly.

Worth noting that both lxc and cgroup-bin are currently in universe, though the lxc source package is in main (as lxcguest is in main).

This looks odd:

+cd /sys/fs/cgroup
+for d in `/bin/ls`; do
              ________
+ umount $d || true
+done

Is the following sufficient (no clue really, I've never been too intimate with upstart scripts)?
+start on mounted MOUNTPOINT=/sys

apart from that, I don't see if any mentioned bugs have been forwarded to upstream. Can you shed some light here please? Seeing upstream response (or even lack thereof) would certainly help to sort this request.

If the issue at hand was only boot speed, I'd probably wave through this request as all changes look sensible to me.

Cheers,
   Stefan.

Quoting StefanPotyra (<email address hidden>):

Thanks for looking, Stefan.

> This looks odd:
>
> +cd /sys/fs/cgroup
> +for d in `/bin/ls`; do
> ________
> + umount $d || true
> +done

Thanks for drawing my attention to this. This is indeed a problem. As an example, I did:

 (as root) mount --bind /dev/pts /mnt
 (as serge) (cd /sys/fs/cgroup; ln -s /mnt)
 (as root) umount /sys/fs/cgroup/mnt

and /mnt got umounted.

Of course serge shouldn't have been able to create a link under /sys/fs/cgroup
in the first place. Stephane, I'm actually out this week, I'll push a fix next
week, but if you have time to post a debdiff this week, that'd be great.

As for the "for d in `/bin/ls`; do" bit, what exactly looks odd about it? I do
it all the time by hand, but that doesn't mean it shouldn't be changed here...

I had considered specifying a (configurable) list of cgroups to mount,
and then only umount those. I.e.

CGROUPS="memory devices cpu freezer"

at top of the upstart job. Maybe 'all' option which then grabs all from 'cat
/proc/cgroups". Would that be preferable?

> Is the following sufficient (no clue really, I've never been too intimate with upstart scripts)?
> +start on mounted MOUNTPOINT=/sys

The cgroups get mounted under /sys/fs/cgroup, and have no userspace
dependencies, so it should be sufficient.

> apart from that, I don't see if any mentioned bugs have been forwarded
> to upstream. Can you shed some light here please? Seeing upstream

The bug about cgclear failing (so that the job fails to stop, and the
package can't be removed) was mentioned upstream, but I don't believe
they've pushed a patch for it yet. They announced in the last few days
that they expect all functionality to be pulled into systemd, and therefore
will stop developing.

> response (or even lack thereof) would certainly help to sort this
> request.

Note we don't intend to stop support of cgroup-bin (now), and want to fix
those bugs. However, some of the bugs are hard, and some I argue are
impossible, due to design flaws: the kernel cgroup API simply does not
provide for a race-free way for a daemon to reliably reclassify programs
as they start up.

This is an important point. It is why the libcgroup upstream is moving
the functionality into systemd. For us, the functionality belongs in
upstart (for initialization and service startup) and pam (for logins).

> If the issue at hand was only boot speed, I'd probably wave through this
> request as all changes look sensible to me.

IMO it's both boot speed and not running extraneous daemons on your
system.

thanks for your time,
-serge

Hi Serge,

On Thu, Aug 25, 2011 at 04:23:25AM -0000, Serge Hallyn wrote:
> Quoting StefanPotyra (<email address hidden>):
>
> Thanks for looking, Stefan.
>
> > This looks odd:
> >
> > +cd /sys/fs/cgroup
> > +for d in `/bin/ls`; do
> > ________
> > + umount $d || true
> > +done
>
> Thanks for drawing my attention to this. This is indeed a problem. As
> an example, I did:
>
> (as root) mount --bind /dev/pts /mnt
> (as serge) (cd /sys/fs/cgroup; ln -s /mnt)
> (as root) umount /sys/fs/cgroup/mnt
>
> and /mnt got umounted.

I didn't think about such a scenario, and believe that that would be
the fault of the admin to mount stuff there.

> As for the "for d in `/bin/ls`; do" bit, what exactly looks odd about it? I do
> it all the time by hand, but that doesn't mean it shouldn't be changed here...

I usually use for i in *; do ;)
The case where * doesn't match anything should be covered by || true, right?
Anyways, I guess I wasn't clear that this was just a small oddity, not at all
a blocker.

> I had considered specifying a (configurable) list of cgroups to mount,
> and then only umount those. I.e.
>
> CGROUPS="memory devices cpu freezer"
>
> at top of the upstart job. Maybe 'all' option which then grabs all from 'cat
> /proc/cgroups". Would that be preferable?

I like the current simplicity :). However that's up to you, I'd be equally happy
with it.

>
> > Is the following sufficient (no clue really, I've never been too intimate with upstart scripts)?
> > +start on mounted MOUNTPOINT=/sys
>
> The cgroups get mounted under /sys/fs/cgroup, and have no userspace
> dependencies, so it should be sufficient.

Having /proc mounted? (Is it guaranteed to be mounted prior to /sys?) Having
/usr mounted in case it is on a different partition? (Or don't we support such a
scenario). As I wrote, I don't have too much clue about upstart.

> > apart from that, I don't see if any mentioned bugs have been forwarded
> > to upstream. Can you shed some light here please? Seeing upstream
>
> The bug about cgclear failing (so that the job fails to stop, and the
> package can't be removed) was mentioned upstream, but I don't believe
> they've pushed a patch for it yet. They announced in the last few days
> that they expect all functionality to be pulled into systemd, and therefore
> will stop developing.

Ah, ok.

> IMO it's both boot speed and not running extraneous daemons on your
> system.

Go ahead then, FFe granted.

Cheers,
   Stefan.

Pushed as-is so we have it before beta freeze.
Issues noted above don't seem critical and I've been running with the current code for over a week now without seeing any problem.

This bug was fixed in the package libcgroup - 0.37.1-1ubuntu5

---------------
libcgroup (0.37.1-1ubuntu5) oneiric; urgency=low

  * Add cgroup-lite package as a light-weight alternative to cgroup-bin
    (LP: #829628)
 -- Serge Hallyn <email address hidden> Fri, 19 Aug 2011 10:30:05 -0500

Updated lxc's recommends to be cgroup-lite | cgroup-bin.