Updated version (v4) to follow.

On Mon, Oct 05, 2015 at 01:45:53PM +0000, Serge Hallyn wrote:
> Quoting Christian Brauner (christianvanbrau...@gmail.com):
> > When users wanted to mount overlay directories with lxc.mount.entry they 
> > had to
> > create upperdirs and workdirs beforehand in order to mount them. To create 
> > it
> > for them we add the functions mount_entry_create_overlay_dirs() and
> > mount_entry_create_aufs_dirs() which do this for them. User can now simply
> > specify e.g.:
> > 
> >         lxc.mount.entry = /lower merged overlay 
> > lowerdir=/lower,upper=/upper,workdir=/workdir,create=dir
> > 
> > and /upper and /workdir will be created for them. /upper and /workdir need 
> > to
> > be absolute paths to directories which are created under the containerdir 
> > (e.g.
> > $lxcpath/container/). Relative mountpoints and mountpoints within the
> > container's rootfs and outside the containerdir are ignored. (The latter
> > *might* change in the future should it be considered safe/useful.)
> > 
> > Specifying
> > 
> >         lxc.mount.entry = /lower merged overlay 
> > lowerdir=/lower:/lower2,create=dir
> > 
> > will lead to a read-only overlay mount in accordance with the
> > kernel-documentation.
> > 
> > Specifying
> > 
> >         lxc.mount.entry = /lower merged overlay lowerdir=/lower,create=dir
> > 
> > will fail when no upperdir and workdir options are given.
> > 
> > Signed-off-by: Christian Brauner <christianvanbrau...@gmail.com>
> > ---
> >  src/lxc/conf.c | 153 
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 141 insertions(+), 12 deletions(-)
> > 
> > diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> > index bb4c19f..bb415ba 100644
> > --- a/src/lxc/conf.c
> > +++ b/src/lxc/conf.c
> > @@ -1813,13 +1813,142 @@ static void cull_mntent_opt(struct mntent *mntent)
> >     }
> >  }
> >  
> > +static int mount_entry_create_overlay_dirs(const struct mntent *mntent,
> > +                                      const struct lxc_rootfs *rootfs)
> > +{
> > +   char *containerdir = NULL;
> > +   char *del = NULL;
> > +   char *upperdir = NULL;
> > +   char *workdir = NULL;
> > +   char **opts = NULL;
> > +   size_t arrlen = 0;
> > +   size_t dirlen = 0;
> > +   size_t i;
> > +   size_t len = 0;
> > +   size_t rootfslen = 0;
> > +
> > +   opts = lxc_string_split(mntent->mnt_opts, ',');
> > +   if (opts)
> > +           arrlen = lxc_array_len((void **)opts);
> > +   else
> > +           return -1;
> > +
> > +   for (i = 0; i < arrlen; i++) {
> > +           if (strstr(opts[i], "upperdir=") && (strlen(opts[i]) > (len = 
> > strlen("upperdir="))))
> > +                           upperdir = opts[i] + len;
> > +           else if (strstr(opts[i], "workdir=") && (strlen(opts[i]) > (len 
> > = strlen("workdir="))))
> > +                           workdir = opts[i] + len;
> > +   }
> > +
> > +   containerdir = strdup(rootfs->path);
> > +   if (!containerdir) {
> > +           lxc_free_array((void **)opts, free);
> > +           return -1;
> > +   }
> > +
> > +   if ((del = strstr(containerdir, "/rootfs"))) {
> > +           memmove(del, del + 7, strlen(del) - 7 + 1);
> 
> if rootfs->path happens to be /a/b/c/rootfs/d/e, you'll end up
> making containerpath = /a/b/c/d/e right?  Is that what you want, or
> do you just want to *del = 0; ?
> 
> > +   }
> > +
> > +   dirlen = strlen(containerdir);
> > +   rootfslen = strlen(rootfs->path);
> > +
> > +   /* We neither allow users to create upperdirs and workdirs outside the
> > +    * containerdir nor inside the rootfs. The latter might be debatable. */
> > +   if (upperdir)
> > +           if ((strncmp(upperdir, containerdir, dirlen) == 0) && 
> > (strncmp(upperdir, rootfs->path, rootfslen) != 0))
> > +                   if (mkdir_p(upperdir, 0755) < 0) {
> > +                           WARN("Failed to create upperdir");
> > +                   }
> > +
> > +
> > +   if (workdir)
> > +           if ((strncmp(workdir, containerdir, dirlen) == 0) && 
> > (strncmp(workdir, rootfs->path, rootfslen) != 0))
> > +                   if (mkdir_p(workdir, 0755) < 0) {
> > +                           WARN("Failed to create workdir");
> > +                   }
> > +
> > +   free(containerdir);
> > +   lxc_free_array((void **)opts, free);
> > +   return 0;
> > +}
> > +
> > +static int mount_entry_create_aufs_dirs(const struct mntent *mntent,
> > +                                   const struct lxc_rootfs *rootfs)
> > +{
> > +   char *containerdir = NULL;
> > +   char *del = NULL;
> > +   char *scratch = NULL;
> > +   char *tmp = NULL;
> > +   char *upperdir = NULL;
> > +   char **opts = NULL;
> > +   size_t arrlen = 0;
> > +   size_t dirlen = 0;
> > +   size_t i;
> > +   size_t len = 0;
> > +   size_t rootfslen = 0;
> > +
> > +   opts = lxc_string_split(mntent->mnt_opts, ',');
> > +   if (opts)
> > +           arrlen = lxc_array_len((void **)opts);
> > +   else
> > +           return -1;
> > +
> > +   for (i = 0; i < arrlen; i++) {
> > +           if (strstr(opts[i], "br=") && (strlen(opts[i]) > (len = 
> > strlen("br="))))
> > +                   tmp = opts[i] + len;
> > +   }
> > +   if (!tmp) {
> > +           lxc_free_array((void **)opts, free);
> > +           return -1;
> > +   }
> > +
> > +   upperdir = strtok_r(tmp, ":=", &scratch);
> > +   if (!upperdir) {
> > +           lxc_free_array((void **)opts, free);
> > +           return -1;
> > +   }
> > +
> > +   containerdir = strdup(rootfs->path);
> > +   if (!containerdir) {
> > +           lxc_free_array((void **)opts, free);
> > +           return -1;
> > +   }
> > +
> > +   if ((del = strstr(containerdir, "/rootfs"))) {
> > +           memmove(del, del + 7, strlen(del) - 7 + 1);
> > +   }
> > +
> > +   dirlen = strlen(containerdir);
> > +   rootfslen = strlen(rootfs->path);
> > +
> > +   /* We neither allow users to create upperdirs outside the containerdir
> > +    * nor inside the rootfs. The latter might be debatable. */
> > +   if ((strncmp(upperdir, containerdir, dirlen) == 0) && 
> > (strncmp(upperdir, rootfs->path, rootfslen) != 0))
> > +           if (mkdir_p(upperdir, 0755) < 0) {
> > +                   WARN("Failed to create upperdir");
> > +           }
> > +
> > +   free(containerdir);
> > +   lxc_free_array((void **)opts, free);
> > +   return 0;
> > +}
> > +
> >  static int mount_entry_create_dir_file(const struct mntent *mntent,
> > -                                  const char* path)
> > +                                  const char* path, const struct 
> > lxc_rootfs *rootfs)
> >  {
> >     char *pathdirname = NULL;
> >     int ret = 0;
> >     FILE *pathfile = NULL;
> >  
> > +   if (strncmp(mntent->mnt_type, "overlay", 7) == 0) {
> > +           if (mount_entry_create_overlay_dirs(mntent, rootfs) < 0)
> > +                   ret = -1;
> > +   } else if (strncmp(mntent->mnt_type, "aufs", 4) == 0) {
> > +           if (mount_entry_create_aufs_dirs(mntent, rootfs) < 0)
> > +                   ret = -1;
> > +   }
> 
> Why if mount_entry_create_*_dirs()  failed to you continue to try to
> create the file?  Seems better to simply return -1 right there.
> 
> > +
> >     if (hasmntopt(mntent, "create=dir")) {
> >             if (mkdir_p(path, 0755) < 0) {
> >                     WARN("Failed to create mount target '%s'", path);
> > @@ -1837,23 +1966,23 @@ static int mount_entry_create_dir_file(const struct 
> > mntent *mntent,
> >             if (!pathfile) {
> >                     WARN("Failed to create mount target '%s'", path);
> >                     ret = -1;
> > -           }
> > -           else
> > +           } else {
> >                     fclose(pathfile);
> > +           }
> >     }
> >     free(pathdirname);
> >     return ret;
> >  }
> >  
> >  static inline int mount_entry_on_generic(struct mntent *mntent,
> > -                 const char* path, const char *rootfs)
> > +                 const char* path, const struct lxc_rootfs *rootfs)
> >  {
> >     unsigned long mntflags;
> >     char *mntdata;
> >     int ret;
> >     bool optional = hasmntopt(mntent, "optional") != NULL;
> >  
> > -   ret = mount_entry_create_dir_file(mntent, path);
> > +   ret = mount_entry_create_dir_file(mntent, path, rootfs);
> >  
> >     if (ret < 0)
> >             return optional ? 0 : -1;
> > @@ -1865,11 +1994,11 @@ static inline int mount_entry_on_generic(struct 
> > mntent *mntent,
> >             return -1;
> >     }
> >  
> > -   ret = mount_entry(mntent->mnt_fsname, path, mntent->mnt_type,
> > -                     mntflags, mntdata, optional, rootfs);
> > +   ret = mount_entry(mntent->mnt_fsname, path, mntent->mnt_type, mntflags,
> > +                     mntdata, optional,
> > +                     rootfs->mount ? rootfs->mount : NULL);
> 
> This isn't right.  Normally it's done as
> 
>       rootfs->path ? rootfs->mount : NULL
> 
> because rootfs->mount is always allocated.  If no rootfs was
> specified it will be LXCROOTFSMOUNT which is something like
> $libdir/lxc/rootfs.
> 
> This is obviously begging for a cleanup, but this patch isn't
> the place to do that.
> 
> >  
> >     free(mntdata);
> > -
> >     return ret;
> >  }
> >  
> > @@ -1922,17 +2051,17 @@ skipabs:
> >             return -1;
> >     }
> >  
> > -   return mount_entry_on_generic(mntent, path, rootfs->mount);
> > +   return mount_entry_on_generic(mntent, path, rootfs);
> >  }
> >  
> >  static int mount_entry_on_relative_rootfs(struct mntent *mntent,
> > -                                     const char *rootfs)
> > +                                     const struct lxc_rootfs *rootfs)
> >  {
> >     char path[MAXPATHLEN];
> >     int ret;
> >  
> >     /* relative to root mount point */
> > -   ret = snprintf(path, sizeof(path), "%s/%s", rootfs, mntent->mnt_dir);
> > +   ret = snprintf(path, sizeof(path), "%s/%s", rootfs->mount, 
> > mntent->mnt_dir);
> >     if (ret >= sizeof(path)) {
> >             ERROR("path name too long");
> >             return -1;
> > @@ -1959,7 +2088,7 @@ static int mount_file_entries(const struct lxc_rootfs 
> > *rootfs, FILE *file,
> >             /* We have a separate root, mounts are relative to it */
> >             if (mntent.mnt_dir[0] != '/') {
> >                     if (mount_entry_on_relative_rootfs(&mntent,
> > -                                                      rootfs->mount))
> > +                                                      rootfs))
> >                             goto out;
> >                     continue;
> >             }
> > -- 
> > 2.6.0
> > 
> > _______________________________________________
> > 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

Attachment: signature.asc
Description: PGP signature

_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to