[lxc-devel] [PATCH 1/2 v3] Add clone_update_unexp_ovl_paths() function

[Christian Brauner]

On Mon, Nov 02, 2015 at 02:38:16PM +0000, Serge Hallyn wrote:
> Quoting Christian Brauner (christianvanbrauner at gmail.com):
> > This functions updates absolute paths for overlay upper- and workdirs so users
> > can simply clone and start new containers without worrying about absolute paths
> > in lxc.mount.entry overlay entries.
> > 
> > Signed-off-by: Christian Brauner <christianvanbrauner at gmail.com>
> > ---
> >  src/lxc/confile.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  src/lxc/confile.h |   3 ++
> >  2 files changed, 105 insertions(+)
> > 
> > diff --git a/src/lxc/confile.c b/src/lxc/confile.c
> > index f7d6814..47b0b8b 100644
> > --- a/src/lxc/confile.c
> > +++ b/src/lxc/confile.c
> > @@ -2598,6 +2598,108 @@ void clear_unexp_config_line(struct lxc_conf *conf, const char *key, bool rm_sub
> >  	}
> >  }
> >  
> > +bool clone_update_unexp_ovl_paths(struct lxc_conf *conf, const char *oldpath,
> > +				  const char *newpath, const char *oldname,
> > +				  const char *newname, const char *ovldir)
> > +{
> > +	const char *key = "lxc.mount.entry";
> > +	int ret;
> > +	char *lstart = conf->unexpanded_config;
> > +	char *lend;
> > +	char *p;
> > +	char *q;
> > +	size_t newdirlen = strlen(ovldir) + strlen(newpath) + strlen(newname) + 2;
> > +	size_t olddirlen = strlen(ovldir) + strlen(oldpath) + strlen(oldname) + 2;
> > +	char *olddir = alloca(olddirlen + 1);
> > +	char *newdir = alloca(newdirlen + 1);
> > +
> > +	ret = snprintf(olddir, olddirlen + 1, "%s=%s/%s", ovldir, oldpath, oldname);
> > +	if (ret < 0 || ret >= olddirlen + 1) {
> > +		ERROR("Bug in %s", __func__);
> > +		return false;
> > +	}
> > +	ret = snprintf(newdir, newdirlen + 1, "%s=%s/%s", ovldir, newpath, newname);
> > +	if (ret < 0 || ret >= newdirlen + 1) {
> > +		ERROR("Bug in %s", __func__);
> > +		return false;
> > +	}
> > +	if (!conf->unexpanded_config)
> > +		return true;
> > +	while (*lstart) {
> > +		lend = strchr(lstart, '\n');
> > +		if (!lend)
> > +			lend = lstart + strlen(lstart);
> > +		else
> > +			lend++;
> > +		if (strncmp(lstart, key, strlen(key)) != 0) {
> > +			lstart = lend;
> > +			continue;
> > +		}
> > +		p = strchr(lstart + strlen(key), '=');
> > +		if (!p) {
> > +			lstart = lend;
> > +			continue;
> > +		}
> > +		p++;
> > +		while (isblank(*p))
> > +			p++;
> > +		if (p >= lend)
> > +                        continue;
> > +                /* Whenever an lxc.mount.entry entry is found in a line we check
> > +                *  if the substring " overlay" or the substring " aufs" is
> > +                *  present before doing any further work. We check for "
> > +                *  overlay" and " aufs" since both substrings need to have at
> > +                *  least one space before them in a valid overlay
> > +                *  lxc.mount.entry (/A B overlay).  When the space before is
> > +                *  missing it is very likely that these substrings are part of a
> > +                *  path or something else. */
> > +		if (!strstr(p, " overlay") && !strstr(p, " aufs")) {
> > +			lstart = lend;
> > +			continue;
> > +		}
> > +		if (!(q = strstr(p, olddir))) {
> > +			lstart = lend;
> > +			continue;
> > +		}
> 
> You're still not doing anything to ensure that 'overlays'/'aufs' and the
> olddir are actually occurring on this same line.  So they could be showing
> up in a comment or just later line.  Temporarily turning lend into '\0' is
> the easiest way, but you could also do something like
> 
> 		if (!(q = strstr(p, " overlay")) && !(q=strstr(p, " aufs")))
> 			goto next;
> 		if (q >= lend)
> 			goto next;

Maybe we should avoid this check since it will fail when an aufs and overlay
lxc.mount.entry share (for whatever weird reason) a path. E.g.

lxc.mount.entry = /a b overlay upperdir=/some/path
lxc.mount.entry = /c d aufs br=/some/path

let's rather just use:

	if (!strstr(p, " overlay") && !strstr(p, " aufs"))
                goto next;
	if (!(q = strstr(p, olddir)) || (q >= lend))
                goto next;