[lxc-devel] [PATCH 2/2 v2] Update absolute paths for overlay and aufs mounts

[Serge Hallyn]

Quoting Christian Brauner (christianvanbrauner at gmail.com):
> On Oct 30, 2015 4:22 PM, "Serge Hallyn" <serge.hallyn at ubuntu.com> wrote:
> >
> > Quoting Christian Brauner (christianvanbrauner at gmail.com):
> > > When using overlay and aufs mounts with lxc.mount.entry users have to
> specify
> > > absolute paths for upperdir and workdir which will then get created
> > > automatically by mount_entry_create_overlay_dirs() and
> > > mount_entry_create_aufs_dirs() in conf.c. When we clone a container with
> > > overlay or aufs lxc.mount.entry entries we need to update these
> absolute paths.
> > > In order to do this we add the function
> update_union_mount_entry_paths() in
> > > lxccontainer.c. The function updates the mounts in two locations:
> > >
> > >         1) lxc_conf->mount_list
> > >
> > > and
> > >
> > >         2) lxc_conf->unexpanded_config (by calling
> clone_update_unexp_ovl_dir())
> > >
> > > If we were to only update 2) we would end up with wrong upperdir and
> workdir
> > > mounts as the absolute paths would still point to the container that
> serves as
> > > the base for the clone. If we were to only update 1) we would end up
> with wrong
> > > upperdir and workdir lxc.mount.entry entries in the clone's config as
> the
> > > absolute paths in upperdir and workdir would still point to the
> container that
> > > serves as the base for the clone. Updating both will get the job done.
> > >
> > > NOTE: This function does not sanitize paths apart from removing trailing
> > > slashes. (So when a user specifies //home//someone/// it will be
> cleaned to
> > > //home//someone. This is the minimal path cleansing which is also done
> by
> > > lxc_container_new().) But the mount_entry_create_overlay_dirs() and
> > > mount_entry_create_aufs_dirs() functions both try to be extremely
> strict about
> > > when to create upperdirs and workdirs. They will only accept sanitized
> paths,
> > > i.e. they require /home/someone. I think this is a (safety) virtue and
> we
> > > should consider sanitizing paths in general. In short:
> > > update_union_mount_entry_paths() does update all absolute paths to the
> new
> > > container but mount_entry_create_overlay_dirs() and
> > > mount_entry_create_aufs_dirs() will still refuse to create upperdir and
> workdir
> > > when the updated path is unclean. This happens easily when e.g. a user
> calls
> > > lxc-clone -o OLD -n NEW -P //home//chb///.
> > >
> > > Signed-off-by: Christian Brauner <christianvanbrauner at gmail.com>
> >
> > Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
> >
> > > ---
> > >  src/lxc/lxccontainer.c | 97
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 97 insertions(+)
> > >
> > > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> > > index 42e23e7..8edf6a2 100644
> > > --- a/src/lxc/lxccontainer.c
> > > +++ b/src/lxc/lxccontainer.c
> > > @@ -2894,6 +2894,99 @@ static int create_file_dirname(char *path,
> struct lxc_conf *conf)
> > >       return ret;
> > >  }
> > >
> > > +/* When we clone a container with overlay or aufs lxc.mount.entry
> entries we
> > > +*  need to update these absolute paths. In order to do this we add the
> function
> > > +*  update_union_mount_entry_paths() in lxccontainer.c. The function
> operates on
> > > +*  c->lxc_conf->unexpanded_config instead of the intuitively plausible
> > > +*  c->lxc_conf->mount_list because the latter also contains mounts
> from other
> > > +*  files as well as generic mounts. */
> > > +static int update_union_mount_entry_paths(struct lxc_conf *lxc_conf,
> > > +                                       const char *lxc_path,
> > > +                                       const char *lxc_name,
> > > +                                       const char *newpath,
> > > +                                       const char *newname)
> > > +{
> > > +     char new_upper[MAXPATHLEN];
> > > +     char new_work[MAXPATHLEN];
> > > +     char old_upper[MAXPATHLEN];
> > > +     char old_work[MAXPATHLEN];
> > > +     char *cleanpath = NULL;
> > > +     int i;
> > > +     int fret = -1;
> > > +     int ret = 0;
> > > +     struct lxc_list *iterator;
> > > +     const char *ovl_dirs[] = {"br", "upperdir", "workdir"};
> > > +
> > > +     cleanpath = strdup(newpath);
> > > +     if (!cleanpath)
> > > +             goto err;
> > > +
> > > +     remove_trailing_slashes(cleanpath);
> > > +
> >
> > Would be wortha comment here saying "we have to update the unexpanded
> > configuration separately from the expanded one."
> 
> I can insert it, add your Acked-by line to the cover letter and leave a
> short reminder in the cover letter that I only inserted a comment per your
> request so you know what's going on. Ok?

Yup, thanks!