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.
>
> -sergeIt's not bogus. It just misses the single case where the container itself is an overlay container. :) Let's merge this patch as it is. I will test whether rootfs->mount really is what we want and send a follow-up patch that is based off of this one. (I'll probably send it tomorrow.)
signature.asc
Description: PGP signature
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel
