[lxc-devel] [PATCH] unexpanded config file: turn into a string

Stéphane Graber stgraber at ubuntu.com
Mon Aug 4 19:50:56 UTC 2014


On Fri, Aug 01, 2014 at 11:34:16PM +0000, Serge Hallyn wrote:
> Originally, we only kept a struct lxc_conf representing the current
> container configuration.  This was insufficient because lxc.include's
> were expanded, so a clone or a snapshot would contain the expanded
> include file contents, rather than the original "lxc.include".  If
> the host's include files are updated, clones and snapshots would not
> inherit those updates.
> 
> To address this, we originally added a lxc_unexp_conf, which mirrored
> the lxc_conf, except that lxc.include was not expanded.
> 
> This has its own cshortcomings, however,  In particular, if a lxc.include
> has a lxc.cgroup setting, and you use the api to say:
> 
> c.clear_config_item("lxc.cgroup")
> 
> this is not representable in the lxc_unexp_conf.  (The original problem,
> which was pointed out to me by stgraber, was slightly different, but
> unlike this problem it was not unsolvable).
> 
> This patch changes the unexpanded configuration  to be a textual
> representation of the configuration.  This allows us *order* the
> configuration commands, which is what was not possible using the
> struct lxc_conf *lxc_unexp_conf.
> 
> The write_config() now becomes a simple fwrite.  However, lxc_clone
> is slightly complicated in parts, the worst of which is the need to
> rewrite the network configuration if we are changing the macaddrs.
> 
> With this patch, lxc-clone and clear_config_item do the right thing.
> lxc-test-saveconfig and lxc-test-clonetest both pass.
> 
> There is room for improvement - multiple calls to
> 
> c.append_config_item("lxc.network.link", "lxcbr0")
> 
> will result in multiple such lines in the configuration file.  In that
> particular case it is harmless.  There may be cases where it is not.
> 
> Overall, this should be a huge improvement in terms of correctness.
> 
> Changelog: Aug 1: updated to current lxc git head.  All lxc-test* and
>    python api test passed.
> 
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>

Acked-by: Stéphane Graber <stgraber at ubuntu.com>

> ---
>  src/lxc/conf.c         |   1 +
>  src/lxc/conf.h         |   5 +-
>  src/lxc/confile.c      | 406 ++++++++++++++++++++++---------------------------
>  src/lxc/confile.h      |  11 +-
>  src/lxc/lxccontainer.c |  79 ++++++----
>  src/lxc/lxccontainer.h |  10 --
>  6 files changed, 241 insertions(+), 271 deletions(-)
> 
> diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> index 473d076..799f011 100644
> --- a/src/lxc/conf.c
> +++ b/src/lxc/conf.c
> @@ -4495,6 +4495,7 @@ void lxc_conf_free(struct lxc_conf *conf)
>  		free(conf->fstab);
>  	if (conf->rcfile)
>  		free(conf->rcfile);
> +	free(conf->unexpanded_config);
>  	lxc_clear_config_network(conf);
>  	if (conf->lsm_aa_profile)
>  		free(conf->lsm_aa_profile);
> diff --git a/src/lxc/conf.h b/src/lxc/conf.h
> index 615f276..a97aa26 100644
> --- a/src/lxc/conf.h
> +++ b/src/lxc/conf.h
> @@ -287,7 +287,6 @@ struct saved_nic {
>  
>  struct lxc_conf {
>  	int is_execute;
> -	bool unexpanded;
>  	char *fstab;
>  	int tty;
>  	int pts;
> @@ -352,6 +351,10 @@ struct lxc_conf {
>  	/* list of environment variables we'll add to the container when
>  	 * started */
>  	struct lxc_list environment;
> +
> +	/* text representation of the config file */
> +	char *unexpanded_config;
> +	size_t unexpanded_len, unexpanded_alloced;
>  };
>  
>  int run_lxc_hooks(const char *name, char *hook, struct lxc_conf *conf,
> diff --git a/src/lxc/confile.c b/src/lxc/confile.c
> index 1e5ffe3..26ac682 100644
> --- a/src/lxc/confile.c
> +++ b/src/lxc/confile.c
> @@ -70,6 +70,7 @@ static int config_rootfs_options(const char *, const char *, struct lxc_conf *);
>  static int config_pivotdir(const char *, const char *, struct lxc_conf *);
>  static int config_utsname(const char *, const char *, struct lxc_conf *);
>  static int config_hook(const char *, const char *, struct lxc_conf *lxc_conf);
> +static int config_network(const char *, const char *, struct lxc_conf *);
>  static int config_network_type(const char *, const char *, struct lxc_conf *);
>  static int config_network_flags(const char *, const char *, struct lxc_conf *);
>  static int config_network_link(const char *, const char *, struct lxc_conf *);
> @@ -124,6 +125,7 @@ static struct lxc_config_t config[] = {
>  	{ "lxc.hook.start",           config_hook                 },
>  	{ "lxc.hook.post-stop",       config_hook                 },
>  	{ "lxc.hook.clone",           config_hook                 },
> +	{ "lxc.hook",                 config_hook                 },
>  	{ "lxc.network.type",         config_network_type         },
>  	{ "lxc.network.flags",        config_network_flags        },
>  	{ "lxc.network.link",         config_network_link         },
> @@ -141,6 +143,7 @@ static struct lxc_config_t config[] = {
>  	{ "lxc.network.ipv6",         config_network_ipv6         },
>  	/* config_network_nic must come after all other 'lxc.network.*' entries */
>  	{ "lxc.network.",             config_network_nic          },
> +	{ "lxc.network",              config_network              },
>  	{ "lxc.cap.drop",             config_cap_drop             },
>  	{ "lxc.cap.keep",             config_cap_keep             },
>  	{ "lxc.console",              config_console              },
> @@ -305,6 +308,17 @@ out:
>  	return ret;
>  }
>  
> +static int config_network(const char *key, const char *value,
> +		struct lxc_conf *lxc_conf)
> +{
> +	if (value && strlen(value)) {
> +		ERROR("lxc.network must not have a value");
> +		return -1;
> +	}
> +
> +	return lxc_clear_config_network(lxc_conf);
> +}
> +
>  static int macvlan_mode(int *valuep, const char *value);
>  
>  static int config_network_type(const char *key, const char *value,
> @@ -954,6 +968,10 @@ static int config_hook(const char *key, const char *value,
>  	if (!value || strlen(value) == 0)
>  		return lxc_clear_hooks(lxc_conf, key);
>  
> +	if (strcmp(key, "lxc.hook") == 0) {
> +		ERROR("lxc.hook cannot take a value");
> +		return -1;
> +	}
>  	copy = strdup(value);
>  	if (!copy) {
>  		SYSERROR("failed to dup string '%s'", value);
> @@ -1202,16 +1220,6 @@ static int rt_sig_num(const char *signame)
>  	return sig_n;
>  }
>  
> -static const char *sig_name(int signum) {
> -	int n;
> -
> -	for (n = 0; n < sizeof(signames) / sizeof((signames)[0]); n++) {
> -		if (signum == signames[n].num)
> -			return signames[n].name;
> -	}
> -	return NULL;
> -}
> -
>  static int sig_parse(const char *signame) {
>  	int n;
>  
> @@ -1596,34 +1604,32 @@ static int config_console(const char *key, const char *value,
>  	return config_path_item(&lxc_conf->console.path, value);
>  }
>  
> -static int add_include_file(const char *fname, struct lxc_conf *lxc_conf)
> +int append_unexp_config_line(const char *line, struct lxc_conf *conf)
>  {
> -	struct lxc_list *list;
> -	char *v;
> -	int len = strlen(fname);
> +	size_t len = conf->unexpanded_len, linelen = strlen(line);
>  
> -	list = malloc(sizeof(*list));
> -	if (!list)
> -		return -1;
> -	lxc_list_init(list);
> -	v = malloc(len+1);
> -	if (!v) {
> -		free(list);
> -		return -1;
> +	while (conf->unexpanded_alloced <= len + linelen + 2) {
> +		char *tmp = realloc(conf->unexpanded_config, conf->unexpanded_alloced + 1024);
> +		if (!tmp)
> +			return -1;
> +		if (!conf->unexpanded_config)
> +			*tmp = '\0';
> +		conf->unexpanded_config = tmp;
> +		conf->unexpanded_alloced += 1024;
> +	}
> +	strcat(conf->unexpanded_config, line);
> +	conf->unexpanded_len += linelen;
> +	if (line[linelen-1] != '\n') {
> +		strcat(conf->unexpanded_config, "\n");
> +		conf->unexpanded_len++;
>  	}
> -	strncpy(v, fname, len);
> -	v[len] = '\0';
> -	list->elem = v;
> -	lxc_list_add_tail(&lxc_conf->includes, list);
>  	return 0;
>  }
>  
>  static int config_includefile(const char *key, const char *value,
>  			  struct lxc_conf *lxc_conf)
>  {
> -	if (lxc_conf->unexpanded)
> -		return add_include_file(value, lxc_conf);
> -	return lxc_config_read(value, lxc_conf, NULL);
> +	return lxc_config_read(value, lxc_conf, true);
>  }
>  
>  static int config_rootfs(const char *key, const char *value,
> @@ -1676,30 +1682,10 @@ static int config_utsname(const char *key, const char *value,
>  	return 0;
>  }
>  
> -static int store_martian_option(char *line, void *data)
> -{
> -	struct lxc_conf *conf = data;
> -	char *str;
> -	struct lxc_list *list;
> -	size_t len = strlen(line);
> -
> -	if (!conf->unexpanded)
> -		return 0;
> -	list = malloc(sizeof(*list));
> -	if (!list)
> -		return -1;
> -	lxc_list_init(list);
> -	str = malloc(len+1);
> -	if (!str) {
> -		free(list);
> -		return -1;
> -	}
> -	strncpy(str, line, len);
> -	str[len] = '\0';
> -	list->elem = str;
> -	lxc_list_add_tail(&conf->aliens, list);
> -	return 0;
> -}
> +struct parse_line_conf {
> +	struct lxc_conf *conf;
> +	bool from_include;
> +};
>  
>  static int parse_line(char *buffer, void *data)
>  {
> @@ -1709,6 +1695,7 @@ static int parse_line(char *buffer, void *data)
>  	char *key;
>  	char *value;
>  	int ret = 0;
> +	struct parse_line_conf *plc = data;
>  
>  	if (lxc_is_line_empty(buffer))
>  		return 0;
> @@ -1723,17 +1710,19 @@ static int parse_line(char *buffer, void *data)
>  		return -1;
>  	}
>  
> +	if (!plc->from_include)
> +		if ((ret = append_unexp_config_line(line, plc->conf)))
> +			goto out;
> +
>  	line += lxc_char_left_gc(line, strlen(line));
>  
>  	/* ignore comments */
>  	if (line[0] == '#')
>  		goto out;
>  
> -	/* martian option - save it in the unexpanded config only */
> -	if (strncmp(line, "lxc.", 4)) {
> -		ret = store_martian_option(line, data);
> +	/* martian option - don't add it to the config itself */
> +	if (strncmp(line, "lxc.", 4))
>  		goto out;
> -	}
>  
>  	ret = -1;
>  
> @@ -1758,7 +1747,7 @@ static int parse_line(char *buffer, void *data)
>  		goto out;
>  	}
>  
> -	ret = config->cb(key, value, data);
> +	ret = config->cb(key, value, plc->conf);
>  
>  out:
>  	free(linep);
> @@ -1767,30 +1756,30 @@ out:
>  
>  static int lxc_config_readline(char *buffer, struct lxc_conf *conf)
>  {
> -	return parse_line(buffer, conf);
> +	struct parse_line_conf c;
> +
> +	c.conf = conf;
> +	c.from_include = false;
> +
> +	return parse_line(buffer, &c);
>  }
>  
> -int lxc_config_read(const char *file, struct lxc_conf *conf, struct lxc_conf *unexp_conf)
> +int lxc_config_read(const char *file, struct lxc_conf *conf, bool from_include)
>  {
> -	int ret;
> +	struct parse_line_conf c;
> +
> +	c.conf = conf;
> +	c.from_include = from_include;
>  
>  	if( access(file, R_OK) == -1 ) {
>  		return -1;
>  	}
> +
>  	/* Catch only the top level config file name in the structure */
> -	if( ! conf->rcfile ) {
> +	if( ! conf->rcfile )
>  		conf->rcfile = strdup( file );
> -	}
> -	ret = lxc_file_for_each_line(file, parse_line, conf);
> -	if (ret)
> -		return ret;
> -	if (!unexp_conf)
> -		return 0;
> -	if (!unexp_conf->rcfile) {
> -		unexp_conf->rcfile = strdup( file );
> -	}
>  
> -	return lxc_file_for_each_line(file, parse_line, unexp_conf);
> +	return lxc_file_for_each_line(file, parse_line, &c);
>  }
>  
>  int lxc_config_define_add(struct lxc_list *defines, char* arg)
> @@ -2348,110 +2337,105 @@ int lxc_clear_config_item(struct lxc_conf *c, const char *key)
>   */
>  void write_config(FILE *fout, struct lxc_conf *c)
>  {
> -	struct lxc_list *it;
> -	int i;
> -	const char *signame;
> +	size_t len = c->unexpanded_len;
> +	int ret;
>  
> -	/* first write any includes */
> -	lxc_list_for_each(it, &c->includes) {
> -		fprintf(fout, "lxc.include = %s\n", (char *)it->elem);
> -	}
> +	if (!len)
> +		return;
> +	ret = fwrite(c->unexpanded_config, 1, len, fout);
> +	if (ret != len)
> +		SYSERROR("Error writing configuration file");
> +}
>  
> -	/* now write any aliens */
> -	lxc_list_for_each(it, &c->aliens) {
> -		fprintf(fout, "%s\n", (char *)it->elem);
> -	}
> +bool do_append_unexp_config_line(struct lxc_conf *conf, const char *key, const char *v)
> +{
> +	int ret;
> +	size_t len = strlen(key) + strlen(v) + 4;
> +	char *tmp = alloca(len);
>  
> -	if (c->fstab)
> -		fprintf(fout, "lxc.mount = %s\n", c->fstab);
> -	lxc_list_for_each(it, &c->mount_list) {
> -		fprintf(fout, "lxc.mount.entry = %s\n", (char *)it->elem);
> -	}
> -	if (c->auto_mounts & LXC_AUTO_ALL_MASK) {
> -		fprintf(fout, "lxc.mount.auto =");
> -		switch (c->auto_mounts & LXC_AUTO_PROC_MASK) {
> -			case LXC_AUTO_PROC_MIXED:         fprintf(fout, " proc:mixed");        break;
> -			case LXC_AUTO_PROC_RW:            fprintf(fout, " proc:rw");           break;
> -			default: break;
> -		}
> -		switch (c->auto_mounts & LXC_AUTO_SYS_MASK) {
> -			case LXC_AUTO_SYS_RO:             fprintf(fout, " sys:ro");            break;
> -			case LXC_AUTO_SYS_RW:             fprintf(fout, " sys:rw");            break;
> -			default: break;
> +	ret = snprintf(tmp, len, "%s = %s", key, v);
> +	if (ret < 0 || ret >= len)
> +		return false;
> +
> +	/* Save the line verbatim into unexpanded_conf */
> +	if (append_unexp_config_line(tmp, conf))
> +		return false;
> +
> +	return true;
> +}
> +
> +void clear_unexp_config_line(struct lxc_conf *conf, const char *key, bool rm_subkeys)
> +{
> +	char *lstart = conf->unexpanded_config, *lend;
> +
> +	if (!conf->unexpanded_config)
> +		return;
> +	while (*lstart) {
> +		lend = strchr(lstart, '\n');
> +		char v;
> +		if (!lend)
> +			lend = lstart + strlen(lstart);
> +		else
> +			lend++;
> +		if (strncmp(lstart, key, strlen(key)) != 0) {
> +			lstart = lend;
> +			continue;
>  		}
> -		switch (c->auto_mounts & LXC_AUTO_CGROUP_MASK) {
> -			case LXC_AUTO_CGROUP_NOSPEC:      fprintf(fout, " cgroup");            break;
> -			case LXC_AUTO_CGROUP_MIXED:       fprintf(fout, " cgroup:mixed");      break;
> -			case LXC_AUTO_CGROUP_RO:          fprintf(fout, " cgroup:ro");         break;
> -			case LXC_AUTO_CGROUP_RW:          fprintf(fout, " cgroup:rw");         break;
> -			case LXC_AUTO_CGROUP_FULL_NOSPEC: fprintf(fout, " cgroup-full");       break;
> -			case LXC_AUTO_CGROUP_FULL_MIXED:  fprintf(fout, " cgroup-full:mixed"); break;
> -			case LXC_AUTO_CGROUP_FULL_RO:     fprintf(fout, " cgroup-full:ro");    break;
> -			case LXC_AUTO_CGROUP_FULL_RW:     fprintf(fout, " cgroup-full:rw");    break;
> -			default: break;
> +		if (!rm_subkeys) {
> +			v = lstart[strlen(key)];
> +			if (!isspace(v) && v != '=') {
> +				lstart = lend;
> +				continue;
> +			}
>  		}
> -		fprintf(fout, "\n");
> -	}
> -	if (c->tty)
> -		fprintf(fout, "lxc.tty = %d\n", c->tty);
> -	if (c->pts)
> -		fprintf(fout, "lxc.pts = %d\n", c->pts);
> -	if (c->ttydir)
> -		fprintf(fout, "lxc.devttydir = %s\n", c->ttydir);
> -	if (c->haltsignal) {
> -		signame = sig_name(c->haltsignal);
> -		if (signame == NULL) {
> -			fprintf(fout, "lxc.haltsignal = %d\n", c->haltsignal);
> -		} else {
> -			fprintf(fout, "lxc.haltsignal = SIG%s\n", signame);
> +		conf->unexpanded_len -= (lend - lstart);
> +		if (*lend == '\0') {
> +			*lstart = '\0';
> +			return;
>  		}
> +		memmove(lstart, lend, strlen(lend)+1);
>  	}
> -	if (c->stopsignal) {
> -		signame = sig_name(c->stopsignal);
> -		if (signame == NULL) {
> -			fprintf(fout, "lxc.stopsignal = %d\n", c->stopsignal);
> -		} else {
> -			fprintf(fout, "lxc.stopsignal = SIG%s\n", signame);
> +}
> +
> +bool clone_update_unexp_hooks(struct lxc_conf *c)
> +{
> +	struct lxc_list *it;
> +	int i;
> +
> +	clear_unexp_config_line(c, "lxc.hook", true);
> +	for (i=0; i<NUM_LXC_HOOKS; i++) {
> +		lxc_list_for_each(it, &c->hooks[i]) {
> +			if (!do_append_unexp_config_line(c, lxchook_names[i], (char *)it->elem))
> +				return false;
>  		}
>  	}
> -	#if HAVE_SYS_PERSONALITY_H
> -	switch(c->personality) {
> -	case PER_LINUX32: fprintf(fout, "lxc.arch = i686\n"); break;
> -	case PER_LINUX: fprintf(fout, "lxc.arch = x86_64\n"); break;
> -	default: break;
> -	}
> -	#endif
> -	if (c->lsm_aa_profile)
> -		fprintf(fout, "lxc.aa_profile = %s\n", c->lsm_aa_profile);
> -	if (c->lsm_se_context)
> -		fprintf(fout, "lxc.se_context = %s\n", c->lsm_se_context);
> -	if (c->seccomp)
> -		fprintf(fout, "lxc.seccomp = %s\n", c->seccomp);
> -	if (c->kmsg == 0)
> -		fprintf(fout, "lxc.kmsg = 0\n");
> -	if (c->autodev > 0)
> -		fprintf(fout, "lxc.autodev = 1\n");
> -	if (c->loglevel != LXC_LOG_PRIORITY_NOTSET)
> -		fprintf(fout, "lxc.loglevel = %s\n", lxc_log_priority_to_string(c->loglevel));
> -	if (c->logfile)
> -		fprintf(fout, "lxc.logfile = %s\n", c->logfile);
> -	lxc_list_for_each(it, &c->cgroup) {
> -		struct lxc_cgroup *cg = it->elem;
> -		fprintf(fout, "lxc.cgroup.%s = %s\n", cg->subsystem, cg->value);
> -	}
> -	if (c->utsname)
> -		fprintf(fout, "lxc.utsname = %s\n", c->utsname->nodename);
> +	return true;
> +}
> +
> +#define DO(cmd) { \
> +	if (!(cmd)) { \
> +		ERROR("Error writing to new config"); \
> +		return false; \
> +	} \
> +}
> +
> +bool clone_update_unexp_network(struct lxc_conf *c)
> +{
> +	struct lxc_list *it;
> +
> +	clear_unexp_config_line(c, "lxc.network", true);
> +
>  	lxc_list_for_each(it, &c->network) {
>  		struct lxc_netdev *n = it->elem;
>  		const char *t = lxc_net_type_to_str(n->type);
>  		struct lxc_list *it2;
> -		fprintf(fout, "lxc.network.type = %s\n", t ? t : "(invalid)");
> +		DO(do_append_unexp_config_line(c, "lxc.network.type", t ? t : "(invalid)"));
>  		if (n->flags & IFF_UP)
> -			fprintf(fout, "lxc.network.flags = up\n");
> +			DO(do_append_unexp_config_line(c, "lxc.network.flags", "up"));
>  		if (n->link)
> -			fprintf(fout, "lxc.network.link = %s\n", n->link);
> +			DO(do_append_unexp_config_line(c, "lxc.network.link", n->link));
>  		if (n->name)
> -			fprintf(fout, "lxc.network.name = %s\n", n->name);
> +			DO(do_append_unexp_config_line(c, "lxc.network.name", n->name));
>  		if (n->type == LXC_NET_MACVLAN) {
>  			const char *mode;
>  			switch (n->priv.macvlan_attr.mode) {
> @@ -2460,98 +2444,68 @@ void write_config(FILE *fout, struct lxc_conf *c)
>  			case MACVLAN_MODE_BRIDGE: mode = "bridge"; break;
>  			default: mode = "(invalid)"; break;
>  			}
> -			fprintf(fout, "lxc.network.macvlan.mode = %s\n", mode);
> +			DO(do_append_unexp_config_line(c, "lxc.network.macvlan.mode", mode));
>  		} else if (n->type == LXC_NET_VETH) {
>  			if (n->priv.veth_attr.pair)
> -				fprintf(fout, "lxc.network.veth.pair = %s\n",
> -					n->priv.veth_attr.pair);
> +				DO(do_append_unexp_config_line(c, "lxc.network.veth.pair", n->priv.veth_attr.pair));
>  		} else if (n->type == LXC_NET_VLAN) {
> -			fprintf(fout, "lxc.network.vlan.id = %d\n", n->priv.vlan_attr.vid);
> +			char vid[20];
> +			sprintf(vid, "%d", n->priv.vlan_attr.vid);
> +			DO(do_append_unexp_config_line(c, "lxc.network.vlan.id", vid));
>  		}
>  		if (n->upscript)
> -			fprintf(fout, "lxc.network.script.up = %s\n", n->upscript);
> +			DO(do_append_unexp_config_line(c, "lxc.network.script.up", n->upscript));
>  		if (n->downscript)
> -			fprintf(fout, "lxc.network.script.down = %s\n", n->downscript);
> +			DO(do_append_unexp_config_line(c, "lxc.network.script.down", n->downscript));
>  		if (n->hwaddr)
> -			fprintf(fout, "lxc.network.hwaddr = %s\n", n->hwaddr);
> +			DO(do_append_unexp_config_line(c, "lxc.network.hwaddr", n->hwaddr));
>  		if (n->mtu)
> -			fprintf(fout, "lxc.network.mtu = %s\n", n->mtu);
> -		if (n->ipv4_gateway_auto)
> -			fprintf(fout, "lxc.network.ipv4.gateway = auto\n");
> -		else if (n->ipv4_gateway) {
> +			DO(do_append_unexp_config_line(c, "lxc.network.mtu", n->mtu));
> +		if (n->ipv4_gateway_auto) {
> +			DO(do_append_unexp_config_line(c, "lxc.network.ipv4.gateway", "auto"));
> +		} else if (n->ipv4_gateway) {
>  			char buf[INET_ADDRSTRLEN];
>  			inet_ntop(AF_INET, n->ipv4_gateway, buf, sizeof(buf));
> -			fprintf(fout, "lxc.network.ipv4.gateway = %s\n", buf);
> +			DO(do_append_unexp_config_line(c, "lxc.network.ipv4.gateway", buf));
>  		}
>  		lxc_list_for_each(it2, &n->ipv4) {
>  			struct lxc_inetdev *i = it2->elem;
> -			char buf[INET_ADDRSTRLEN];
> -			inet_ntop(AF_INET, &i->addr, buf, sizeof(buf));
> -			fprintf(fout, "lxc.network.ipv4 = %s", buf);
> +			char buf[2*INET_ADDRSTRLEN+20], buf2[INET_ADDRSTRLEN], prefix[20];
> +			inet_ntop(AF_INET, &i->addr, buf, INET_ADDRSTRLEN);
>  
> -			if (i->prefix)
> -				fprintf(fout, "/%d", i->prefix);
> +			if (i->prefix) {
> +				sprintf(prefix, "/%d", i->prefix);
> +				strcat(buf, prefix);
> +			}
>  
>  			if (i->bcast.s_addr != (i->addr.s_addr |
>  			    htonl(INADDR_BROADCAST >>  i->prefix))) {
>  
> -				inet_ntop(AF_INET, &i->bcast, buf, sizeof(buf));
> -				fprintf(fout, " %s\n", buf);
> +				inet_ntop(AF_INET, &i->bcast, buf2, sizeof(buf2));
> +				strcat(buf, " ");
> +				strcat(buf, buf2);
>  			}
> -			else
> -				fprintf(fout, "\n");
> +			DO(do_append_unexp_config_line(c, "lxc.network.ipv4", buf));
>  		}
> -		if (n->ipv6_gateway_auto)
> -			fprintf(fout, "lxc.network.ipv6.gateway = auto\n");
> -		else if (n->ipv6_gateway) {
> +		if (n->ipv6_gateway_auto) {
> +			DO(do_append_unexp_config_line(c, "lxc.network.ipv6.gateway", "auto"));
> +		} else if (n->ipv6_gateway) {
>  			char buf[INET6_ADDRSTRLEN];
>  			inet_ntop(AF_INET6, n->ipv6_gateway, buf, sizeof(buf));
> -			fprintf(fout, "lxc.network.ipv6.gateway = %s\n", buf);
> +			DO(do_append_unexp_config_line(c, "lxc.network.ipv6.gateway", buf));
>  		}
>  		lxc_list_for_each(it2, &n->ipv6) {
>  			struct lxc_inet6dev *i = it2->elem;
> -			char buf[INET6_ADDRSTRLEN];
> -			inet_ntop(AF_INET6, &i->addr, buf, sizeof(buf));
> -			if (i->prefix)
> -				fprintf(fout, "lxc.network.ipv6 = %s/%d\n",
> -					buf, i->prefix);
> -			else
> -				fprintf(fout, "lxc.network.ipv6 = %s\n", buf);
> +			char buf[INET6_ADDRSTRLEN + 20], prefix[20];
> +			inet_ntop(AF_INET6, &i->addr, buf, INET6_ADDRSTRLEN);
> +			if (i->prefix) {
> +				sprintf(prefix, "/%d", i->prefix);
> +				strcat(buf, prefix);
> +			}
> +			DO(do_append_unexp_config_line(c, "lxc.network.ipv6", buf));
>  		}
>  	}
> -	lxc_list_for_each(it, &c->caps)
> -		fprintf(fout, "lxc.cap.drop = %s\n", (char *)it->elem);
> -	lxc_list_for_each(it, &c->keepcaps)
> -		fprintf(fout, "lxc.cap.keep = %s\n", (char *)it->elem);
> -	lxc_list_for_each(it, &c->id_map) {
> -		struct id_map *idmap = it->elem;
> -		fprintf(fout, "lxc.id_map = %c %lu %lu %lu\n",
> -			idmap->idtype == ID_TYPE_UID ? 'u' : 'g', idmap->nsid,
> -			idmap->hostid, idmap->range);
> -	}
> -	for (i=0; i<NUM_LXC_HOOKS; i++) {
> -		lxc_list_for_each(it, &c->hooks[i])
> -			fprintf(fout, "lxc.hook.%s = %s\n",
> -				lxchook_names[i], (char *)it->elem);
> -	}
> -	if (c->console.path)
> -		fprintf(fout, "lxc.console = %s\n", c->console.path);
> -	if (c->rootfs.path)
> -		fprintf(fout, "lxc.rootfs = %s\n", c->rootfs.path);
> -	if (c->rootfs.mount && strcmp(c->rootfs.mount, LXCROOTFSMOUNT) != 0)
> -		fprintf(fout, "lxc.rootfs.mount = %s\n", c->rootfs.mount);
> -	if (c->rootfs.options)
> -		fprintf(fout, "lxc.rootfs.options = %s\n", c->rootfs.options);
> -	if (c->rootfs.pivot)
> -		fprintf(fout, "lxc.pivotdir = %s\n", c->rootfs.pivot);
> -	if (c->start_auto)
> -		fprintf(fout, "lxc.start.auto = %d\n", c->start_auto);
> -	if (c->start_delay)
> -		fprintf(fout, "lxc.start.delay = %d\n", c->start_delay);
> -	if (c->start_order)
> -		fprintf(fout, "lxc.start.order = %d\n", c->start_order);
> -	lxc_list_for_each(it, &c->groups)
> -		fprintf(fout, "lxc.group = %s\n", (char *)it->elem);
>  	lxc_list_for_each(it, &c->environment)
> -		fprintf(fout, "lxc.environment = %s\n", (char *)it->elem);
> +		DO(do_append_unexp_config_line(c, "lxc.environment", (char *)it->elem));
> +	return true;
>  }
> diff --git a/src/lxc/confile.h b/src/lxc/confile.h
> index 7e34089..6a81e02 100644
> --- a/src/lxc/confile.h
> +++ b/src/lxc/confile.h
> @@ -26,6 +26,7 @@
>  
>  #include <stdio.h>
>  #include <lxc/attach_options.h>
> +#include <stdbool.h>
>  
>  struct lxc_conf;
>  struct lxc_list;
> @@ -39,7 +40,8 @@ struct lxc_config_t {
>  extern struct lxc_config_t *lxc_getconfig(const char *key);
>  extern int lxc_list_nicconfigs(struct lxc_conf *c, const char *key, char *retv, int inlen);
>  extern int lxc_listconfigs(char *retv, int inlen);
> -extern int lxc_config_read(const char *file, struct lxc_conf *conf, struct lxc_conf *unexp_conf);
> +extern int lxc_config_read(const char *file, struct lxc_conf *conf, bool from_include);
> +extern int append_unexp_config_line(const char *line, struct lxc_conf *conf);
>  
>  extern int lxc_config_define_add(struct lxc_list *defines, char* arg);
>  extern int lxc_config_define_load(struct lxc_list *defines,
> @@ -52,4 +54,11 @@ extern int lxc_fill_elevated_privileges(char *flaglist, int *flags);
>  extern int lxc_get_config_item(struct lxc_conf *c, const char *key, char *retv, int inlen);
>  extern int lxc_clear_config_item(struct lxc_conf *c, const char *key);
>  extern void write_config(FILE *fout, struct lxc_conf *c);
> +
> +extern bool do_append_unexp_config_line(struct lxc_conf *conf, const char *key, const char *v);
> +
> +/* These are used when cloning a container */
> +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 *c);
> +extern bool clone_update_unexp_network(struct lxc_conf *c);
>  #endif
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index ca5da87..814aeb1 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -238,10 +238,6 @@ static void lxc_container_free(struct lxc_container *c)
>  		lxc_conf_free(c->lxc_conf);
>  		c->lxc_conf = NULL;
>  	}
> -	if (c->lxc_unexp_conf) {
> -		lxc_conf_free(c->lxc_unexp_conf);
> -		c->lxc_unexp_conf = NULL;
> -	}
>  	if (c->config_path) {
>  		free(c->config_path);
>  		c->config_path = NULL;
> @@ -414,14 +410,9 @@ static bool load_config_locked(struct lxc_container *c, const char *fname)
>  {
>  	if (!c->lxc_conf)
>  		c->lxc_conf = lxc_conf_init();
> -	if (!c->lxc_unexp_conf) {
> -		c->lxc_unexp_conf = lxc_conf_init();
> -		if (c->lxc_unexp_conf)
> -			c->lxc_unexp_conf->unexpanded = true;
> -	}
> -	if (c->lxc_conf && c->lxc_unexp_conf &&
> -			!lxc_config_read(fname, c->lxc_conf,
> -					 c->lxc_unexp_conf))
> +	if (!c->lxc_conf)
> +		return false;
> +	if (!lxc_config_read(fname, c->lxc_conf, false))
>  		return true;
>  	return false;
>  }
> @@ -1214,10 +1205,6 @@ static void lxcapi_clear_config(struct lxc_container *c)
>  			lxc_conf_free(c->lxc_conf);
>  			c->lxc_conf = NULL;
>  		}
> -		if (c->lxc_unexp_conf) {
> -			lxc_conf_free(c->lxc_unexp_conf);
> -			c->lxc_unexp_conf = NULL;
> -		}
>  	}
>  }
>  
> @@ -1342,7 +1329,6 @@ static bool lxcapi_create(struct lxc_container *c, const char *t,
>  	/* reload config to get the rootfs */
>  	lxc_conf_free(c->lxc_conf);
>  	c->lxc_conf = NULL;
> -	c->lxc_unexp_conf = NULL;
>  	if (!load_config_locked(c, c->configfile))
>  		goto out_unlock;
>  
> @@ -1440,6 +1426,20 @@ out:
>  	return bret;
>  }
>  
> +static void do_clear_unexp_config_line(struct lxc_conf *conf, const char *key)
> +{
> +	if (strcmp(key, "lxc.cgroup") == 0)
> +		clear_unexp_config_line(conf, key, true);
> +	else if (strcmp(key, "lxc.network") == 0)
> +		clear_unexp_config_line(conf, key, true);
> +	else if (strcmp(key, "lxc.hook") == 0)
> +		clear_unexp_config_line(conf, key, true);
> +	else
> +		clear_unexp_config_line(conf, key, false);
> +	if (!do_append_unexp_config_line(conf, key, ""))
> +		WARN("Error clearing configuration for %s", key);
> +}
> +
>  static bool lxcapi_clear_config_item(struct lxc_container *c, const char *key)
>  {
>  	int ret;
> @@ -1449,6 +1449,8 @@ static bool lxcapi_clear_config_item(struct lxc_container *c, const char *key)
>  	if (container_mem_lock(c))
>  		return false;
>  	ret = lxc_clear_config_item(c->lxc_conf, key);
> +	if (!ret)
> +		do_clear_unexp_config_line(c->lxc_conf, key);
>  	container_mem_unlock(c);
>  	return ret == 0;
>  }
> @@ -1867,7 +1869,7 @@ static bool lxcapi_save_config(struct lxc_container *c, const char *alt_file)
>  	fout = fopen(alt_file, "w");
>  	if (!fout)
>  		goto out;
> -	write_config(fout, c->lxc_unexp_conf);
> +	write_config(fout, c->lxc_conf);
>  	fclose(fout);
>  	ret = true;
>  
> @@ -2140,26 +2142,20 @@ static bool lxcapi_destroy_with_snapshots(struct lxc_container *c)
>  	return lxcapi_destroy(c);
>  }
>  
> -
>  static bool set_config_item_locked(struct lxc_container *c, const char *key, const char *v)
>  {
>  	struct lxc_config_t *config;
>  
>  	if (!c->lxc_conf)
>  		c->lxc_conf = lxc_conf_init();
> -	if (!c->lxc_unexp_conf) {
> -		c->lxc_unexp_conf = lxc_conf_init();
> -		if (c->lxc_unexp_conf)
> -			c->lxc_unexp_conf->unexpanded = true;
> -	}
> -	if (!c->lxc_conf || !c->lxc_unexp_conf)
> +	if (!c->lxc_conf)
>  		return false;
>  	config = lxc_getconfig(key);
>  	if (!config)
>  		return false;
> -	if (config->cb(key, v, c->lxc_unexp_conf) != 0)
> +	if (config->cb(key, v, c->lxc_conf) != 0)
>  		return false;
> -	return (0 == config->cb(key, v, c->lxc_conf));
> +	return do_append_unexp_config_line(c->lxc_conf, key, v);
>  }
>  
>  static bool lxcapi_set_config_item(struct lxc_container *c, const char *key, const char *v)
> @@ -2415,6 +2411,10 @@ static int copyhooks(struct lxc_container *oldc, struct lxc_container *c)
>  		}
>  	}
>  
> +	if (!clone_update_unexp_hooks(c->lxc_conf)) {
> +		ERROR("Error saving new hooks in clone");
> +		return -1;
> +	}
>  	c->save_config(c, NULL);
>  	return 0;
>  }
> @@ -2456,6 +2456,8 @@ static int copy_fstab(struct lxc_container *oldc, struct lxc_container *c)
>  	if (!oldpath)
>  		return 0;
>  
> +	clear_unexp_config_line(c->lxc_conf, "lxc.mount", false);
> +
>  	char *p = strrchr(oldpath, '/');
>  	if (!p)
>  		return -1;
> @@ -2480,6 +2482,10 @@ static int copy_fstab(struct lxc_container *oldc, struct lxc_container *c)
>  		ERROR("error: allocating pathname");
>  		return -1;
>  	}
> +	if (!do_append_unexp_config_line(c->lxc_conf, "lxc.mount", newpath)) {
> +		ERROR("error saving new lxctab");
> +		return -1;
> +	}
>  
>  	return 0;
>  }
> @@ -2549,10 +2555,10 @@ static int copy_storage(struct lxc_container *c0, struct lxc_container *c,
>  		ERROR("Out of memory while setting storage path");
>  		return -1;
>  	}
> -	free(c->lxc_unexp_conf->rootfs.path);
> -	c->lxc_unexp_conf->rootfs.path = strdup(c->lxc_conf->rootfs.path);
> -	if (!c->lxc_unexp_conf->rootfs.path) {
> -		ERROR("Out of memory while setting storage path");
> +	// We will simply append a new lxc.rootfs entry to the unexpanded config
> +	clear_unexp_config_line(c->lxc_conf, "lxc.rootfs", false);
> +	if (!do_append_unexp_config_line(c->lxc_conf, "lxc.rootfs", c->lxc_conf->rootfs.path)) {
> +		ERROR("Error saving new rootfs to cloend config");
>  		return -1;
>  	}
>  	if (flags & LXC_CLONE_SNAPSHOT)
> @@ -2765,7 +2771,7 @@ static struct lxc_container *lxcapi_clone(struct lxc_container *c, const char *n
>  		SYSERROR("open %s", newpath);
>  		goto out;
>  	}
> -	write_config(fout, c->lxc_unexp_conf);
> +	write_config(fout, c->lxc_conf);
>  	fclose(fout);
>  	c->lxc_conf->rootfs.path = origroot;
>  
> @@ -2793,6 +2799,8 @@ static struct lxc_container *lxcapi_clone(struct lxc_container *c, const char *n
>  	if (ret < 0)
>  		goto out;
>  
> +	clear_unexp_config_line(c2->lxc_conf, "lxc.utsname", false);
> +
>  	// update utsname
>  	if (!set_config_item_locked(c2, "lxc.utsname", newname)) {
>  		ERROR("Error setting new hostname");
> @@ -2812,8 +2820,13 @@ static struct lxc_container *lxcapi_clone(struct lxc_container *c, const char *n
>  	}
>  
>  	// update macaddrs
> -	if (!(flags & LXC_CLONE_KEEPMACADDR))
> +	if (!(flags & LXC_CLONE_KEEPMACADDR)) {
>  		network_new_hwaddrs(c2);
> +		if (!clone_update_unexp_network(c2->lxc_conf)) {
> +			ERROR("Error updating network for clone");
> +			goto out;
> +		}
> +	}
>  
>  	// We've now successfully created c2's storage, so clear it out if we
>  	// fail after this
> diff --git a/src/lxc/lxccontainer.h b/src/lxc/lxccontainer.h
> index 5e74c2e..5085c43 100644
> --- a/src/lxc/lxccontainer.h
> +++ b/src/lxc/lxccontainer.h
> @@ -99,16 +99,6 @@ struct lxc_container {
>  	 */
>  	struct lxc_conf *lxc_conf;
>  
> -	/*!
> -	 * \private
> -	 * The non-common, unexpanded configuration.  This includes the
> -	 * list of lxc.include files, and does not contain any
> -	 * individual configuration items from the include files.
> -	 * Anything coming from the container's own configuration file
> -	 * or from lxcapi_set_config_item() does get added here.
> -	 */
> -	struct lxc_conf *lxc_unexp_conf;
> -
>  	// public fields
>  	/*! Human-readable string representing last error */
>  	char *error_string;
> -- 
> 2.0.1
> 
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel

-- 
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20140804/568bf8ce/attachment-0001.sig>


More information about the lxc-devel mailing list