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

Michael H. Warfield mhw at WittsEnd.com
Sun Jun 26 18:12:35 UTC 2011


On Sun, 2011-06-26 at 13:56 -0400, Michael H. Warfield wrote: 
> Looking at the sources and Serge's patch...
> 
> On Sun, 2011-06-26 at 13:33 -0400, Michael H. Warfield wrote: 
> > On Sun, 2011-06-26 at 18:27 +0200, Daniel Lezcano wrote: 
> > > On 06/26/2011 05:52 PM, Michael H. Warfield wrote:
> > > > On Sun, 2011-06-26 at 17:27 +0200, Daniel Lezcano wrote: 
> > > >> On 06/26/2011 05:06 PM, Serge E. Hallyn wrote:
> > > >>> Quoting Michael H. Warfield (mhw at WittsEnd.com):
> > > >>>>> cd /sys/fs/cgroup
> > > >>>>>    for d in `/bin/ls`; do
> > > >>>>>       echo 1 > $d/cgroup.clone_children
> > > >>>>>    done
> > > >>>>
> > > >>>> Doing this step alone broke lxc totally for me, with or without the
> > > >>>> patch below.  This was on Fedora 15 testing with lxc 0.7.4.2 as well as
> > > >>>
> > > >>> Do you have the ns cgroup mounted?  The above is only for without
> > > >>> ns cgroup.
> > > > 
> > > >> Yes, the ns_cgroup and clone_children are mutually exclusive.
> > > > 
> > > >> If you try to create a namespace with a ns cgroup mounted and the
> > > >> clone_children flag is set. The 'clone' will return EINVAL.
> > > > 
> > > >> I noticed with the patch applied because there was a bug in it which was
> > > >> setting this flag even if the cgroup was ns.
> > > > 
> > > >> It is possible, you ran lxc with the patch, the clone_children was set,
> > > >> and then you remove the patch and try to run lxc again. As the
> > > >> clone_children is still set, that will make impossible to clone a new
> > > >> namespace.
> > > > 
> > > >> With the patch I sent previously on top of Serge's patch, that should be
> > > >> fixed.
> > > > 
> > > > Ok...  Got that an threw it into the mix.
> > > > 
> > > > I'm still seeing the segfaults on lxc-info.  Seems to fault even if I
> > > > specify the name of a non-existent machine as well.
> > 
> > > Oh, seems I missed this info :s
> > 
> > > If you launch lxc-info -n foo, that segfaults ?
> > 
> > > On what architecture does it fault ?
> > 
> > Fedora 15, x86_64.
> > 
> > 0.7.4.2 is fine.
> > 
> > Just rebuilt a fresh set of packages from GIT sources alone.  They're
> > fine and lxc-info from git works.
> > 
> > Git patched with Serge's patch.  Segfault.
> > 
> > Git patched with both patches.  Segfault.
> > 
> > Here's what gdb has to say about it:
> > 
> > [root at forest lxc]# gdb lxc-info core.13546 
> > GNU gdb (GDB) Fedora (7.2.90.20110525-39.fc15)
> > Copyright (C) 2011 Free Software Foundation, Inc.
> > License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
> > This is free software: you are free to change and redistribute it.
> > There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
> > and "show warranty" for details.
> > This GDB was configured as "x86_64-redhat-linux-gnu".
> > For bug reporting instructions, please see:
> > <http://www.gnu.org/software/gdb/bugs/>...
> > Reading symbols from /home/mhw/rpmbuild/BUILD/lxc-0.7.4.2git2/src/lxc/lxc-info...done.
> > [New LWP 13546]
> > Core was generated by `./lxc-info -n Alcove'.
> > Program terminated with signal 11, Segmentation fault.
> > #0  lxc_cgroup_path_get (path=0x7fff89b35928, subsystem=0x7fba52a4fdc1 "freezer", name=0x7fff89b3987d "Alcove") at cgroup.c:326
> > 326	               *s = '\0';
> > (gdb) trace
> > Tracepoint 1 at 0x7fba52a41e74: file cgroup.c, line 326.
> > (gdb) backtrace
> > #0  lxc_cgroup_path_get (path=0x7fff89b35928, subsystem=0x7fba52a4fdc1 "freezer", name=0x7fff89b3987d "Alcove") at cgroup.c:326
> > #1  0x00007fba52a4a337 in freezer_state (name=<optimized out>) at state.c:74
> > #2  0x00007fba52a4a42d in lxc_getstate (name=0x7fff89b3987d "Alcove") at state.c:130
> > #3  0x000000000040068d in main (argc=<optimized out>, argv=<optimized out>) at lxc_info.c:63
> 
> Ok...  That looks like this piece from that patch:
> 
> +/*
> + * lxc_cgroup_path_get: put into *path the pathname for
> + * %subsystem and cgroup %name.  If %subsystem is NULL, then
> + * the first mounted cgroup will be used (for nr_tasks)
> + */
> +int lxc_cgroup_path_get(char **path, const char *subsystem, const char
> *name)
> +{
> +       static char        buf[MAXPATHLEN];
> +       static char        retbuf[MAXPATHLEN];
> +
> +       /* what lxc_cgroup_set calls subsystem is actually the filename,
> i.e.
> +          'devices.allow'.  So for our purposee we trim it */
> +       if (subsystem) {
> +               snprintf(retbuf, MAXPATHLEN, "%s", subsystem);
> +               char *s = index(retbuf, '.');
> +               *s = '\0';
> +               DEBUG("%s: called for subsys %s name %s\n", __func__,
> retbuf, name);
>         }
> 
> Now wait a minute.  Is that a typo here:
> 
> 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?

diff -urN lxc-0.7.4.2git2/src/lxc/cgroup.c lxc-0.7.4.2git3/src/lxc/cgroup.c
--- lxc-0.7.4.2git2/src/lxc/cgroup.c	2011-06-26 14:05:43.212612575 -0400
+++ lxc-0.7.4.2git3/src/lxc/cgroup.c	2011-06-26 14:06:54.815612754 -0400
@@ -323,7 +323,9 @@
        if (subsystem) {
                snprintf(retbuf, MAXPATHLEN, "%s", subsystem);
                char *s = index(retbuf, '.');
-               *s = '\0';
+               if (s) {
+                      *s = '\0';
+               }
                DEBUG("%s: called for subsys %s name %s\n", __func__, retbuf, name);
 	}
        if (get_cgroup_mount(MTAB, subsystem ? retbuf : NULL, buf)) {

Regards,
Mike
-- 
Michael H. Warfield (AI4NB) | (770) 985-6132 |  mhw at WittsEnd.com
   /\/\|=mhw=|\/\/          | (678) 463-0932 |  http://www.wittsend.com/mhw/
   NIC whois: MHW9          | An optimist believes we live in the best of all
 PGP Key: 0x674627FF        | possible worlds.  A pessimist is sure of it!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 482 bytes
Desc: This is a digitally signed message part
URL: <http://lists.linuxcontainers.org/pipermail/lxc-users/attachments/20110626/8968e4cd/attachment.pgp>


More information about the lxc-users mailing list