Comment 6 for bug 829628

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.