Quoting Christian Brauner (christianvanbrau...@gmail.com):
> On Fri, Oct 16, 2015 at 05:11:02PM +0000, Serge Hallyn wrote:
> > Quoting Christian Brauner (christianvanbrau...@gmail.com):
> > > On Fri, Oct 16, 2015 at 03:52:04PM +0000, Serge Hallyn wrote:
> > > > Quoting Christian Brauner (christianvanbrau...@gmail.com):
> > > > > On Thu, Oct 15, 2015 at 08:32:25PM +0000, Serge Hallyn wrote:
> > > > > > Quoting Christian Brauner (christianvanbrau...@gmail.com):
> > > > > > > The mount_entry_create_*_dirs() functions currently assume that 
> > > > > > > the rootfs of
> > > > > > > the container is actually named "rootfs". This has the 
> > > > > > > consequence that
> > > > > > > 
> > > > > > >   del = strstr(lxcpath, "/rootfs");
> > > > > > >   if (!del) {
> > > > > > >           free(lxcpath);
> > > > > > >           lxc_free_array((void **)opts, free);
> > > > > > >           return -1;
> > > > > > >   }
> > > > > > >   *del = '\0';
> > > > > > > 
> > > > > > > will return NULL when the rootfs of a container is not actually 
> > > > > > > named "rootfs".
> > > > > > > This means the we return -1 and do not create the necessary 
> > > > > > > upperdir/workdir
> > > > > > > directories required for the overlay/aufs mount to work. Hence, 
> > > > > > > let's not make
> > > > > > > that assumption. We now pass lxc_path and lxc_name to
> > > > > > > mount_entry_create_*_dirs() and create the path directly. To 
> > > > > > > prevent failure we
> > > > > > > also have mount_entry_create_*_dirs() check that lxc_name and 
> > > > > > > lxc_path are not
> > > > > > > empty when they are passed in.
> > > > > > > 
> > > > > > > Signed-off-by: Christian Brauner <christianvanbrau...@gmail.com>
> > > > > > 
> > > > > > Yeah this was bugging me a few years ago.  Overall the patch looks 
> > > > > > fine
> > > > > > to me, I'm running a full testsuite to ease my mind about it.  Will 
> > > > > > ack
> > > > > > after that passes and I look over it again.
> > > > > 
> > > > > We should also consider parsing path->rootfs when the container is an 
> > > > > overlay or
> > > > > aufs backed container. Because in this case the right hand side of 
> > > > > the check:
> > > > > 
> > > > >               if ((strncmp(upperdir, lxcpath, dirlen) == 0) && 
> > > > > (strncmp(upperdir, rootfs->path, rootfslen) != 0))
> > > > > 
> > > > > will be trivially true since path->rootfs will e.g. be 
> > > > > overlayfs:/path1:path2.
> > > > > Parsing path->rootfs to extract path2 before doing the second check 
> > > > > would be
> > > > > safer... Thoughts?
> > > > > 
> > > > 
> > > > True that the current check is bogus.  But I think you just want to
> > > > use rootfs->mount instead of rootfs->path.  By the time this code
> > > > hits we have converted whatever target path the user gave us into
> > > > concat(rootfs->mount, process(target)) where process(x) will take
> > > > off a leading $lxcpath/$lxcname/rootfs or rootfs->path.
> 
> Assume we have a cloned container. Then according to my testing
> 
>         rootfs->path
> 
> contains
> 
>         
> overlayfs:/path/to/the/original/containers/rootfs:/path/to/the/cloned/containers/rootfs
> 
> but
> 
>         rootfs->mount
> 
> contains
> 
>        /usr/lib/lxc/rootfs
> 
> or whatever directory is used for pivot_dir. Hence, rootfs->mount won't help 
> us
> with the right hand side of the strncmp() check since we want to ensure that 
> no
> upperdir or workdir folders are created under
> 
>         /path/to/the/containers/rootfs
> 
> I still think that we will simply need to parse rootfs->path and extract
> /path/to/the/cloned/containers/rootfs. Thoughts?

Oh yeah this is a src not a target.  You're right.
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to