---------- Forwarded message ---------- From: "Christian Brauner" <christianvanbrau...@gmail.com> Date: Oct 30, 2015 4:27 PM Subject: Re: [PATCH 2/2 v2] Update absolute paths for overlay and aufs mounts To: "Serge E. Hallyn" <serge.hal...@ubuntu.com> Cc: "lxc-dev" <lxc-devel@lists.linuxcontainers.org>
> > On Oct 30, 2015 4:22 PM, "Serge Hallyn" <serge.hal...@ubuntu.com> wrote: > > > > Quoting Christian Brauner (christianvanbrau...@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 <christianvanbrau...@gmail.com> > > > > Acked-by: Serge E. Hallyn <serge.hal...@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? > > > > > + for (i = 0; i < sizeof(ovl_dirs) / sizeof(ovl_dirs[0]); i++) { > > > + if (!clone_update_unexp_ovl_dir(lxc_conf, lxc_path, newpath, > > > + lxc_name, newname, ovl_dirs[i])) > > > + goto err; > > > + } > > > + > > > + ret = snprintf(old_work, MAXPATHLEN, "workdir=%s/%s", lxc_path, lxc_name); > > > + if (ret < 0 || ret >= MAXPATHLEN) > > > + goto err; > > > + > > > + ret = snprintf(new_work, MAXPATHLEN, "workdir=%s/%s", cleanpath, newname); > > > + if (ret < 0 || ret >= MAXPATHLEN) > > > + goto err; > > > + > > > + lxc_list_for_each(iterator, &lxc_conf->mount_list) { > > > + char *mnt_entry = NULL; > > > + char *new_mnt_entry = NULL; > > > + char *tmp = NULL; > > > + char *tmp_mnt_entry = NULL; > > > + mnt_entry = iterator->elem; > > > + > > > + if (strstr(mnt_entry, "overlay")) > > > + tmp = "upperdir"; > > > + else if (strstr(mnt_entry, "aufs")) > > > + tmp = "br"; > > > + > > > + if (!tmp) > > > + continue; > > > + > > > + ret = snprintf(old_upper, MAXPATHLEN, "%s=%s/%s", tmp, lxc_path, lxc_name); > > > + if (ret < 0 || ret >= MAXPATHLEN) > > > + goto err; > > > + > > > + ret = snprintf(new_upper, MAXPATHLEN, "%s=%s/%s", tmp, cleanpath, newname); > > > + if (ret < 0 || ret >= MAXPATHLEN) > > > + goto err; > > > + > > > + if (strstr(mnt_entry, old_upper)) { > > > + tmp_mnt_entry = lxc_string_replace(old_upper, new_upper, mnt_entry); > > > + } > > > + > > > + if (strstr(mnt_entry, old_work)) { > > > + if (tmp_mnt_entry) > > > + new_mnt_entry = lxc_string_replace(old_work, new_work, tmp_mnt_entry); > > > + } > > > + > > > + if (new_mnt_entry) { > > > + free(iterator->elem); > > > + iterator->elem = strdup(new_mnt_entry); > > > + } else if (tmp_mnt_entry) { > > > + free(iterator->elem); > > > + iterator->elem = strdup(tmp_mnt_entry); > > > + } > > > + > > > + free(new_mnt_entry); > > > + free(tmp_mnt_entry); > > > + } > > > + > > > + fret = 0; > > > +err: > > > + free(cleanpath); > > > + return fret; > > > +} > > > + > > > static struct lxc_container *do_lxcapi_clone(struct lxc_container *c, const char *newname, > > > const char *lxcpath, int flags, > > > const char *bdevtype, const char *bdevdata, uint64_t newsize, > > > @@ -3009,6 +3102,10 @@ static struct lxc_container *do_lxcapi_clone(struct lxc_container *c, const char > > > } > > > } > > > > > > + // update absolute paths for union mount directories > > > + if (update_union_mount_entry_paths(c2->lxc_conf, c->config_path, c->name, lxcpath, newname) < 0) > > > + goto out; > > > + > > > // We've now successfully created c2's storage, so clear it out if we > > > // fail after this > > > storage_copied = 1; > > > -- > > > 2.6.2 > > >
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel