Re: [libvirt] [RFC][PATCH] lxc: fix for ns cgroups subsystem

Hi Serge and Daniel, On Sat, May 9, 2009 at 4:03 AM, Serge E. Hallyn <serue(a)us.ibm.com&gt; wrote: > Quoting Daniel P. Berrange (berrange(a)redhat.com): >> On Fri, May 08, 2009 at 08:34:12AM -0500, Serge E. Hallyn wrote: >> > Quoting Ryota Ozaki (ozaki.ryota(a)gmail.com): >> > > Hi Serge, >> > > >> > > On Fri, May 8, 2009 at 11:48 AM, Serge E. Hallyn <serue(a)us.ibm.com&gt; wrote: >> > > > IIUC, the real problem is that src/cgroup.c assumes that the >> > > > cgroup name should be $CGROUP_MOUNTPOINT/groupname.  But of >> > > > course if the ns cgroup is enabled, then the unshare(CLONE_NEWNS) >> > > > to create a new namespace in which to mount the new devpts >> > > > locks the driver under $CGROUP_MOUNTPOINT/<pid_of_driver>/ >> > > > or somesuch. >> > > > >> > > > If this fixes the problem I have no objections, but it seems >> > > > more fragile than perhaps trying to teach src/cgroup.c to >> > > > consider it's current cgroup as a starting point. >> > > >> > > hmm, I don't know why the assumption is bad and how the approach >> > > you are suggesting helps the ns problem. >> > >> > To be clear, the asssumption is that the driver starts in the >> > root cgroup, i.e. it's pid is listed in $CGROUP_MOUNTPOINT/tasks. >> > And that it can create $CGROUP_MOUNTPOINT/groupname and move >> > itself into $CGROUP_MOUNTPOINT/groupname/tasks. >> > >> > So, the assumption is bad because when the driver does a >> > unshare(CLONE_NEWNS), it gets moved into $CGROUP_MOUNTPOINT/X, >> > and after that can only move itself into >> > $CGROUP_MOUNTPOINT/X/groupname. >> > >> > Even with your patch, it's possible for the lxc driver to have >> > been started under say $CGROUP_MOUNTPOINT/libvir or >> > $CGROUP_MOUNTPOINT/<username> through libcgroup/PAM for instance, >> > in which case your patch would be insufficient. >> >> Indeed, we can't assume we're in the root group at any time. Our >> general plan is that libvirt itself uses 3 levels of cgroup >> starting at the cgroup that libvirtd was placed in by the admin >> of the host, then a per-driver group, then per-guest groups >> >>   $LIBVIRTD_ROOT_CGROUP >>      | >>      +- lxc >>      |    | >>      |    +- LXC-GUEST-1 >>      |    +- LXC-GUEST-2 >>      |    +- LXC-GUEST-3 >>      |    +- ... >>      | >>      +- qemu >>           | >>           +- QEMU-GUEST-1 >>           +- QEMU-GUEST-2 >>           +- QEMU-GUEST-3 >>           +- ... >> >> $LIBVIRTD_ROOT_CGROUP, may be the actaul root mount point for >> the cgroup controller in question, or it may be a sub-directory >> that the admin chose to put it in. Also, if running libvirtd >> as a normal user, the admin may have created per-user account >> cgroups and so libvirtd would end up in there. So we have to >> be prepared for anything wrt initial libvirtd cgroup root. >> >> NB The code for putting QEMU in a cgroup isn't merged yet. > > That sounds good.  I just don't think the code currently does > it.  To do the right thing, IIUC, virCgroupPathOfGroup() should > parse the /proc/pid/cgroup contents of the 'controller' process, > and insert that between the virCgroupGetMount(controller) > result and the group name. > > Or something... > > (right?) > > thanks, > -serge > OK I probably understood the problem and wrote a patch for that according to the Serge's suggestion. I expect this patch fixes the problem, however unfortunately I've not tried it yet under the situation you are worrying and just done regression tests. Because I don't know easy way to produce the situation. If you know that, please let me know...

Thanks,  ozaki-r