On Oct 16, 2015 19:11, "Serge Hallyn" <serge.hal...@ubuntu.com> 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. > > > > > > -serge > > > > It's not bogus. It just misses the single case where the container itself is an > > overlay container. :) > > Or blockd-dev-backed, or any case where lxc.rootfs is in a nonstandard > location - but that's ok, > Right, I always forget the block-devs...
> > Let's merge this patch as it is. > > Yeah, that's fine. > > @Stgraber - please do apply this :) > > > 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.) > > Great, thanks. > _______________________________________________ > lxc-devel mailing list > lxc-devel@lists.linuxcontainers.org > http://lists.linuxcontainers.org/listinfo/lxc-devel
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel