[lxc-devel] [PATCH 1/2 v2] Add clone_update_unexp_ovl_dir() function

[Serge Hallyn]

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 | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/lxc/confile.h |   3 ++
>  2 files changed, 106 insertions(+)
> 
> diff --git a/src/lxc/confile.c b/src/lxc/confile.c
> index f7d6814..bc5b7ef 100644
> --- a/src/lxc/confile.c
> +++ b/src/lxc/confile.c
> @@ -2598,6 +2598,109 @@ void clear_unexp_config_line(struct lxc_conf *conf, const char *key, bool rm_sub
>  	}
>  }
>  
> +bool clone_update_unexp_ovl_dir(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)
> +			return true;

I think you should 

		if (p >= lend)
			continue;

(and then you can skip the !*p check above as this should catch that)

What you have will only get confused on invalid configs, but this would
be stricter and make me feel better.

> +		/* We check that when an lxc.mount.entry is found the substrings
> +		 * " overlay " or " aufs " are present before we try to update
> +		 * the line. This seems like a bit more safety. We can check for
> +		 * " overlay " and " aufs " since both substrings need to have
> +		 * at least one space before and after them. When the space
> +		 * before or after is missing it is very likely that these
> +		 * substrings are part of a path or something else. So we
> +		 * shouldn't bother to do any further work...
> +		 */
> +		if (!strstr(p, " overlay ") && !strstr(p, " aufs ")) {
> +			lstart = lend;
> +			continue;
> +		}

But this will find " overlay " in any subsequent lines too right?  Don't you
need to limit your search to the line?  Maybe by saving *lend and setting
*lend = '\0'?

> +		if (!(q = strstr(p, olddir))) {
> +			lstart = lend;
> +			continue;
> +		}
> +
> +		/* replace the olddir with newdir */
> +		if (olddirlen >= newdirlen) {
> +			size_t diff = olddirlen - newdirlen;
> +			memcpy(q, newdir, newdirlen);
> +			if (olddirlen != newdirlen) {
> +				memmove(q + newdirlen, q + newdirlen + diff,
> +					strlen(q) - newdirlen - diff + 1);
> +				lend -= diff;
> +				conf->unexpanded_len -= diff;
> +			}
> +			lstart = lend;
> +		} else {
> +			char *new;
> +			size_t diff = newdirlen - olddirlen;
> +			size_t oldlen = conf->unexpanded_len;
> +			size_t newlen = oldlen + diff;
> +			size_t poffset = q - conf->unexpanded_config;
> +			new = realloc(conf->unexpanded_config, newlen + 1);
> +			if (!new) {
> +				ERROR("Out of memory");
> +				return false;
> +			}
> +			conf->unexpanded_len = newlen;
> +			conf->unexpanded_alloced = newlen + 1;
> +			new[newlen-1] = '\0';
> +			lend = new + (lend - conf->unexpanded_config);
> +			/* move over the remainder to make room for the newdir */
> +			memmove(new + poffset + newdirlen,
> +				new + poffset + olddirlen,
> +				oldlen - poffset - olddirlen + 1);
> +			conf->unexpanded_config = new;
> +			memcpy(new + poffset, newdir, newdirlen);
> +			lstart = lend + diff;

Heh, I *think* the above bit is all right.

> +		}
> +	}
> +	return true;
> +}
> +
>  bool clone_update_unexp_hooks(struct lxc_conf *conf, const char *oldpath,
>  	const char *newpath, const char *oldname, const char *newname)
>  {
> diff --git a/src/lxc/confile.h b/src/lxc/confile.h
> index 8da3699..66af832 100644
> --- a/src/lxc/confile.h
> +++ b/src/lxc/confile.h
> @@ -61,5 +61,8 @@ extern bool do_append_unexp_config_line(struct lxc_conf *conf, const char *key,
>  extern void clear_unexp_config_line(struct lxc_conf *conf, const char *key, bool rm_subkeys);
>  extern bool clone_update_unexp_hooks(struct lxc_conf *conf, const char *oldpath,
>  	const char *newpath, const char *oldname, const char *newmame);
> +bool clone_update_unexp_ovl_dir(struct lxc_conf *conf, const char *oldpath,
> +				const char *newpath, const char *oldname,
> +				const char *newname, const char *ovldir);
>  extern bool network_new_hwaddrs(struct lxc_conf *conf);
>  #endif
> -- 
> 2.6.2
>