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

[Christian Brauner]

On Thu, Oct 22, 2015 at 02:15:18PM +0000, Serge Hallyn wrote:
> Quoting Christian Brauner (christianvanbrauner at gmail.com):
> > On Thu, Oct 22, 2015 at 01:13:35PM +0000, Serge Hallyn wrote:
> > > Quoting Christian Brauner (christianvanbrauner at gmail.com):
> > > > On Tue, Oct 20, 2015 at 04:17:18PM +0000, Serge Hallyn wrote:
> > > > > Quoting Christian Brauner (christianvanbrauner at gmail.com):
<snip>
> > > > > 
> > > > > Alternate suggestion.  How about using clear_unexp_config_line() to remove all
> > > > > lxc.mount.entry lines from unexpanded config first, then run through this loop
> > > > > and simply add the new ones?
> > > > > 
> > > > > The lxc_string_replace approach feels problematic.
> > > > 
> > > > I thought so too, but clear_unexp_config_line() will not just clear mount
> > > > entries from the containers config but also entries from files like common.conf
> > > > which are also present in lxc_conf->mount_list e.g.:
> > > 
> > > Hm, might be the other way around, but yeah we can't do that.
> > > 
> > > But if the config file has say a commented version of the line before
> > > the real line, you'll update only the comment right?
> > > 
> > > So can you do lik eclear_unexp_config_line() does and make sure you
> > > are looking for only lines beginning with "lxc.mount.entry += +"
> > > and then have the mnt_entry?  I guess that should do.
> > > 
> > 
> > I'll test that and get back with an updated patch. I *try* to think of something
> > smarter... Do you want me to avoid lxc_string_replace()?
> 
> Not because of lxc_string_replace itself, but because it won't confine
> itself to one ^.*$ line.  So if you cna make it work, that's fine with me,
> but it feels like a wrecking ball in our unexpanded_config china shop.

Ok, we are clear that we need to update both, the lxc_conf->mount_list and the
lxc_conf->unexpanded_config. I suggest we split this into separate functions.
For updating lxc_conf->unexpanded_config we could add a function to confile.c
which is similar to clone_update_unexp_hooks(). It would look like this:

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) + 1 + 1;
	size_t olddirlen = strlen(ovldir) + strlen(oldpath) + strlen(oldname) + 1 + 1;
	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;
		/* Should we check that when an lxc.mount.entry is found the
		 * substrings "overlay" or "aufs" are actually 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 "))
			continue;
		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(lend-diff, lend, strlen(lend)+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);
			if (!new) {
				ERROR("Out of memory");
				return false;
			}
			conf->unexpanded_len = newlen;
			new[newlen-1] = '\0';
			lend = new + (lend - conf->unexpanded_config);
			/* move over the remainder, /$hookname\n$rest */
			memmove(new+poffset+newdirlen,
					new+poffset+olddirlen,
					oldlen-poffset-olddirlen);
			conf->unexpanded_config = new;
			memcpy(new+poffset, newdir, newdirlen);
			lstart = lend + diff;
		}
	}
	return true;
}

If you agree with this approach we can put another function in lxccontainer.c
which updates lxc_conf->mount_list and which calls clone_update_unexp_ovl_dir().
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20151023/9c64e53e/attachment.sig>