<p dir="ltr">Yup. I think I have an idea for the do_lxcapi_clone() changes (using lxc_list_for_each()). Unless you have a smarter approach already.</p>
<p dir="ltr">On Oct 8, 2015 4:59 PM, "Serge Hallyn" <<a href="mailto:serge.hallyn@ubuntu.com">serge.hallyn@ubuntu.com</a>> wrote:<br>
><br>
> Yes, I think so. Worse, I think that'll mean we'll have to also restore<br>
> those at snapshot restore.<br>
><br>
> Quoting Christian Brauner (<a href="mailto:christianvanbrauner@gmail.com">christianvanbrauner@gmail.com</a>):<br>
> > Fair enough. Should we then adapt do_lxcapi_clone() to replace any mountentries<br>
> > that involve references to the original container with the name of the new<br>
> > container or should we just have users do it manually? (Because currently the<br>
> > presence of any lxc.mount.entry = ... overlay ... will prevent clones from<br>
> > working out of the box... Maybe I'm just being pedantic here...)<br>
> ><br>
> > On Wed, Oct 07, 2015 at 01:20:35PM +0000, Serge Hallyn wrote:<br>
> > > mount targets when relative are relative to the mounted rootfs dir<br>
> > > (with absolute paths under the *not-mounted* rootfs dir being auto-<br>
> > > translated to being under the mounted rootfs dir).<br>
> > ><br>
> > > These paths would be relative to the containerdir. I think that's too<br>
> > > confusing.<br>
> > ><br>
> > > Quoting Christian Brauner (<a href="mailto:christianvanbrauner@gmail.com">christianvanbrauner@gmail.com</a>):<br>
> > > > I now wonder if it wouldn't be smarter to force users to specify relative paths<br>
> > > > for upper and workdir<br>
> > > ><br>
> > > > lxc.mount.entry = /lower merged overlay lowerdir=/lower,upper=upper,workdir=workdir,create=dir<br>
> > > ><br>
> > > > and fill in the missing path in mount_entry_create_*_dirs(). Otherwise these<br>
> > > > mounts won't work out of the box when a clone of the container is made and<br>
> > > > started... Thoughts?<br>
> > > ><br>
> > > > On Tue, Oct 06, 2015 at 08:38:13PM +0200, Christian Brauner wrote:<br>
> > > > > When users wanted to mount overlay directories with lxc.mount.entry they had to<br>
> > > > > create upperdirs and workdirs beforehand in order to mount them. To create it<br>
> > > > > for them we add the functions mount_entry_create_overlay_dirs() and<br>
> > > > > mount_entry_create_aufs_dirs() which do this for them. User can now simply<br>
> > > > > specify e.g.:<br>
> > > > ><br>
> > > > > lxc.mount.entry = /lower merged overlay lowerdir=/lower,upper=/upper,workdir=/workdir,create=dir<br>
> > > > ><br>
> > > > > and /upper and /workdir will be created for them. /upper and /workdir need to<br>
> > > > > be absolute paths to directories which are created under the containerdir (e.g.<br>
> > > > > under $lxcpath/CONTAINERNAME/). Relative mountpoints, mountpoints outside the<br>
> > > > > containerdir, and mountpoints within the container's rootfs are ignored. (The<br>
> > > > > latter *might* change in the future should it be considered safe/useful.)<br>
> > > > ><br>
> > > > > Specifying<br>
> > > > ><br>
> > > > > lxc.mount.entry = /lower merged overlay lowerdir=/lower:/lower2,create=dir<br>
> > > > ><br>
> > > > > will lead to a read-only overlay mount in accordance with the<br>
> > > > > kernel-documentation.<br>
> > > > ><br>
> > > > > Specifying<br>
> > > > ><br>
> > > > > lxc.mount.entry = /lower merged overlay lowerdir=/lower,create=dir<br>
> > > > ><br>
> > > > > will fail when no upperdir and workdir options are given.<br>
> > > > ><br>
> > > > > Signed-off-by: Christian Brauner <<a href="mailto:christianvanbrauner@gmail.com">christianvanbrauner@gmail.com</a>><br>
> > > > > Acked-by: Serge E. Hallyn <<a href="mailto:serge.hallyn@ubuntu.com">serge.hallyn@ubuntu.com</a>><br>
> > > > > ---<br>
> > > > > src/lxc/conf.c | 162 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----<br>
> > > > > 1 file changed, 150 insertions(+), 12 deletions(-)<br>
> > > > ><br>
> > > > > diff --git a/src/lxc/conf.c b/src/lxc/conf.c<br>
> > > > > index 6728c78..5a3209a 100644<br>
> > > > > --- a/src/lxc/conf.c<br>
> > > > > +++ b/src/lxc/conf.c<br>
> > > > > @@ -1815,13 +1815,151 @@ static void cull_mntent_opt(struct mntent *mntent)<br>
> > > > > }<br>
> > > > > }<br>
> > > > ><br>
> > > > > +static int mount_entry_create_overlay_dirs(const struct mntent *mntent,<br>
> > > > > + const struct lxc_rootfs *rootfs)<br>
> > > > > +{<br>
> > > > > + char *del = NULL;<br>
> > > > > + char *lxcpath = NULL;<br>
> > > > > + char *upperdir = NULL;<br>
> > > > > + char *workdir = NULL;<br>
> > > > > + char **opts = NULL;<br>
> > > > > + size_t arrlen = 0;<br>
> > > > > + size_t dirlen = 0;<br>
> > > > > + size_t i;<br>
> > > > > + size_t len = 0;<br>
> > > > > + size_t rootfslen = 0;<br>
> > > > > +<br>
> > > > > + if (!rootfs->path)<br>
> > > > > + return -1;<br>
> > > > > +<br>
> > > > > + opts = lxc_string_split(mntent->mnt_opts, ',');<br>
> > > > > + if (opts)<br>
> > > > > + arrlen = lxc_array_len((void **)opts);<br>
> > > > > + else<br>
> > > > > + return -1;<br>
> > > > > +<br>
> > > > > + for (i = 0; i < arrlen; i++) {<br>
> > > > > + if (strstr(opts[i], "upperdir=") && (strlen(opts[i]) > (len = strlen("upperdir="))))<br>
> > > > > + upperdir = opts[i] + len;<br>
> > > > > + else if (strstr(opts[i], "workdir=") && (strlen(opts[i]) > (len = strlen("workdir="))))<br>
> > > > > + workdir = opts[i] + len;<br>
> > > > > + }<br>
> > > > > +<br>
> > > > > + lxcpath = strdup(rootfs->path);<br>
> > > > > + if (!lxcpath) {<br>
> > > > > + lxc_free_array((void **)opts, free);<br>
> > > > > + return -1;<br>
> > > > > + }<br>
> > > > > +<br>
> > > > > + del = strstr(lxcpath, "/rootfs");<br>
> > > > > + if (!del) {<br>
> > > > > + free(lxcpath);<br>
> > > > > + lxc_free_array((void **)opts, free);<br>
> > > > > + return -1;<br>
> > > > > + }<br>
> > > > > + *del = '\0';<br>
> > > > > +<br>
> > > > > + dirlen = strlen(lxcpath);<br>
> > > > > + rootfslen = strlen(rootfs->path);<br>
> > > > > +<br>
> > > > > + /* We neither allow users to create upperdirs and workdirs outside the<br>
> > > > > + * containerdir nor inside the rootfs. The latter might be debatable. */<br>
> > > > > + if (upperdir)<br>
> > > > > + if ((strncmp(upperdir, lxcpath, dirlen) == 0) && (strncmp(upperdir, rootfs->path, rootfslen) != 0))<br>
> > > > > + if (mkdir_p(upperdir, 0755) < 0) {<br>
> > > > > + WARN("Failed to create upperdir");<br>
> > > > > + }<br>
> > > > > +<br>
> > > > > +<br>
> > > > > + if (workdir)<br>
> > > > > + if ((strncmp(workdir, lxcpath, dirlen) == 0) && (strncmp(workdir, rootfs->path, rootfslen) != 0))<br>
> > > > > + if (mkdir_p(workdir, 0755) < 0) {<br>
> > > > > + WARN("Failed to create workdir");<br>
> > > > > + }<br>
> > > > > +<br>
> > > > > + free(lxcpath);<br>
> > > > > + lxc_free_array((void **)opts, free);<br>
> > > > > + return 0;<br>
> > > > > +}<br>
> > > > > +<br>
> > > > > +static int mount_entry_create_aufs_dirs(const struct mntent *mntent,<br>
> > > > > + const struct lxc_rootfs *rootfs)<br>
> > > > > +{<br>
> > > > > + char *del = NULL;<br>
> > > > > + char *lxcpath = NULL;<br>
> > > > > + char *scratch = NULL;<br>
> > > > > + char *tmp = NULL;<br>
> > > > > + char *upperdir = NULL;<br>
> > > > > + char **opts = NULL;<br>
> > > > > + size_t arrlen = 0;<br>
> > > > > + size_t i;<br>
> > > > > + size_t len = 0;<br>
> > > > > +<br>
> > > > > + if (!rootfs->path)<br>
> > > > > + return -1;<br>
> > > > > +<br>
> > > > > + opts = lxc_string_split(mntent->mnt_opts, ',');<br>
> > > > > + if (opts)<br>
> > > > > + arrlen = lxc_array_len((void **)opts);<br>
> > > > > + else<br>
> > > > > + return -1;<br>
> > > > > +<br>
> > > > > + for (i = 0; i < arrlen; i++) {<br>
> > > > > + if (strstr(opts[i], "br=") && (strlen(opts[i]) > (len = strlen("br="))))<br>
> > > > > + tmp = opts[i] + len;<br>
> > > > > + }<br>
> > > > > + if (!tmp) {<br>
> > > > > + lxc_free_array((void **)opts, free);<br>
> > > > > + return -1;<br>
> > > > > + }<br>
> > > > > +<br>
> > > > > + upperdir = strtok_r(tmp, ":=", &scratch);<br>
> > > > > + if (!upperdir) {<br>
> > > > > + lxc_free_array((void **)opts, free);<br>
> > > > > + return -1;<br>
> > > > > + }<br>
> > > > > +<br>
> > > > > + lxcpath = strdup(rootfs->path);<br>
> > > > > + if (!lxcpath) {<br>
> > > > > + lxc_free_array((void **)opts, free);<br>
> > > > > + return -1;<br>
> > > > > + }<br>
> > > > > +<br>
> > > > > + del = strstr(lxcpath, "/rootfs");<br>
> > > > > + if (!del) {<br>
> > > > > + free(lxcpath);<br>
> > > > > + lxc_free_array((void **)opts, free);<br>
> > > > > + return -1;<br>
> > > > > + }<br>
> > > > > + *del = '\0';<br>
> > > > > +<br>
> > > > > + /* We neither allow users to create upperdirs outside the containerdir<br>
> > > > > + * nor inside the rootfs. The latter might be debatable. */<br>
> > > > > + if ((strncmp(upperdir, lxcpath, strlen(lxcpath)) == 0) && (strncmp(upperdir, rootfs->path, strlen(rootfs->path)) != 0))<br>
> > > > > + if (mkdir_p(upperdir, 0755) < 0) {<br>
> > > > > + WARN("Failed to create upperdir");<br>
> > > > > + }<br>
> > > > > +<br>
> > > > > + free(lxcpath);<br>
> > > > > + lxc_free_array((void **)opts, free);<br>
> > > > > + return 0;<br>
> > > > > +}<br>
> > > > > +<br>
> > > > > static int mount_entry_create_dir_file(const struct mntent *mntent,<br>
> > > > > - const char* path)<br>
> > > > > + const char* path, const struct lxc_rootfs *rootfs)<br>
> > > > > {<br>
> > > > > char *pathdirname = NULL;<br>
> > > > > int ret = 0;<br>
> > > > > FILE *pathfile = NULL;<br>
> > > > ><br>
> > > > > + if (strncmp(mntent->mnt_type, "overlay", 7) == 0) {<br>
> > > > > + if (mount_entry_create_overlay_dirs(mntent, rootfs) < 0)<br>
> > > > > + return -1;<br>
> > > > > + } else if (strncmp(mntent->mnt_type, "aufs", 4) == 0) {<br>
> > > > > + if (mount_entry_create_aufs_dirs(mntent, rootfs) < 0)<br>
> > > > > + return -1;<br>
> > > > > + }<br>
> > > > > +<br>
> > > > > if (hasmntopt(mntent, "create=dir")) {<br>
> > > > > if (mkdir_p(path, 0755) < 0) {<br>
> > > > > WARN("Failed to create mount target '%s'", path);<br>
> > > > > @@ -1839,23 +1977,23 @@ static int mount_entry_create_dir_file(const struct mntent *mntent,<br>
> > > > > if (!pathfile) {<br>
> > > > > WARN("Failed to create mount target '%s'", path);<br>
> > > > > ret = -1;<br>
> > > > > - }<br>
> > > > > - else<br>
> > > > > + } else {<br>
> > > > > fclose(pathfile);<br>
> > > > > + }<br>
> > > > > }<br>
> > > > > free(pathdirname);<br>
> > > > > return ret;<br>
> > > > > }<br>
> > > > ><br>
> > > > > static inline int mount_entry_on_generic(struct mntent *mntent,<br>
> > > > > - const char* path, const char *rootfs)<br>
> > > > > + const char* path, const struct lxc_rootfs *rootfs)<br>
> > > > > {<br>
> > > > > unsigned long mntflags;<br>
> > > > > char *mntdata;<br>
> > > > > int ret;<br>
> > > > > bool optional = hasmntopt(mntent, "optional") != NULL;<br>
> > > > ><br>
> > > > > - ret = mount_entry_create_dir_file(mntent, path);<br>
> > > > > + ret = mount_entry_create_dir_file(mntent, path, rootfs);<br>
> > > > ><br>
> > > > > if (ret < 0)<br>
> > > > > return optional ? 0 : -1;<br>
> > > > > @@ -1867,11 +2005,11 @@ static inline int mount_entry_on_generic(struct mntent *mntent,<br>
> > > > > return -1;<br>
> > > > > }<br>
> > > > ><br>
> > > > > - ret = mount_entry(mntent->mnt_fsname, path, mntent->mnt_type,<br>
> > > > > - mntflags, mntdata, optional, rootfs);<br>
> > > > > + ret = mount_entry(mntent->mnt_fsname, path, mntent->mnt_type, mntflags,<br>
> > > > > + mntdata, optional,<br>
> > > > > + rootfs->path ? rootfs->mount : NULL);<br>
> > > > ><br>
> > > > > free(mntdata);<br>
> > > > > -<br>
> > > > > return ret;<br>
> > > > > }<br>
> > > > ><br>
> > > > > @@ -1924,17 +2062,17 @@ skipabs:<br>
> > > > > return -1;<br>
> > > > > }<br>
> > > > ><br>
> > > > > - return mount_entry_on_generic(mntent, path, rootfs->mount);<br>
> > > > > + return mount_entry_on_generic(mntent, path, rootfs);<br>
> > > > > }<br>
> > > > ><br>
> > > > > static int mount_entry_on_relative_rootfs(struct mntent *mntent,<br>
> > > > > - const char *rootfs)<br>
> > > > > + const struct lxc_rootfs *rootfs)<br>
> > > > > {<br>
> > > > > char path[MAXPATHLEN];<br>
> > > > > int ret;<br>
> > > > ><br>
> > > > > /* relative to root mount point */<br>
> > > > > - ret = snprintf(path, sizeof(path), "%s/%s", rootfs, mntent->mnt_dir);<br>
> > > > > + ret = snprintf(path, sizeof(path), "%s/%s", rootfs->mount, mntent->mnt_dir);<br>
> > > > > if (ret >= sizeof(path)) {<br>
> > > > > ERROR("path name too long");<br>
> > > > > return -1;<br>
> > > > > @@ -1961,7 +2099,7 @@ static int mount_file_entries(const struct lxc_rootfs *rootfs, FILE *file,<br>
> > > > > /* We have a separate root, mounts are relative to it */<br>
> > > > > if (mntent.mnt_dir[0] != '/') {<br>
> > > > > if (mount_entry_on_relative_rootfs(&mntent,<br>
> > > > > - rootfs->mount))<br>
> > > > > + rootfs))<br>
> > > > > goto out;<br>
> > > > > continue;<br>
> > > > > }<br>
> > > > > --<br>
> > > > > 2.6.1<br>
> > > > ><br>
> > ><br>
> > ><br>
> > ><br>
> > > > _______________________________________________<br>
> > > > lxc-devel mailing list<br>
> > > > <a href="mailto:lxc-devel@lists.linuxcontainers.org">lxc-devel@lists.linuxcontainers.org</a><br>
> > > > <a href="http://lists.linuxcontainers.org/listinfo/lxc-devel">http://lists.linuxcontainers.org/listinfo/lxc-devel</a><br>
> > ><br>
> > > _______________________________________________<br>
> > > lxc-devel mailing list<br>
> > > <a href="mailto:lxc-devel@lists.linuxcontainers.org">lxc-devel@lists.linuxcontainers.org</a><br>
> > > <a href="http://lists.linuxcontainers.org/listinfo/lxc-devel">http://lists.linuxcontainers.org/listinfo/lxc-devel</a><br>
><br>
><br>
><br>
> > _______________________________________________<br>
> > lxc-devel mailing list<br>
> > <a href="mailto:lxc-devel@lists.linuxcontainers.org">lxc-devel@lists.linuxcontainers.org</a><br>
> > <a href="http://lists.linuxcontainers.org/listinfo/lxc-devel">http://lists.linuxcontainers.org/listinfo/lxc-devel</a><br>
><br>
> _______________________________________________<br>
> lxc-devel mailing list<br>
> <a href="mailto:lxc-devel@lists.linuxcontainers.org">lxc-devel@lists.linuxcontainers.org</a><br>
> <a href="http://lists.linuxcontainers.org/listinfo/lxc-devel">http://lists.linuxcontainers.org/listinfo/lxc-devel</a><br>
</p>