Merge pull request #1721 from brauner/2017-07-30/fix_saved_config_handling

src/lxc/confile.c

@@ -3680,25 +3680,34 @@ static int get_config_includefiles(const char *key, char *retv, int inlen,
 	return -ENOSYS;
 }
 
-static struct lxc_config_t *
-get_network_config_ops(const char *key, struct lxc_conf *lxc_conf, ssize_t *idx)
+static struct lxc_config_t *get_network_config_ops(const char *key,
+						   struct lxc_conf *lxc_conf,
+						   ssize_t *idx,
+						   char **deindexed_key)
 {
+	int ret;
+	unsigned int tmpidx;
+	size_t numstrlen;
 	char *copy, *idx_start, *idx_end;
 	struct lxc_config_t *config = NULL;
 
 	/* check that this is a sensible network key */
-	if (strncmp("lxc.net.", key, 8))
+	if (strncmp("lxc.net.", key, 8)) {
+		ERROR("Invalid network configuration key \"%s\"", key);
 		return NULL;
+	}
 
 	copy = strdup(key);
-	if (!copy)
+	if (!copy) {
+		ERROR("Failed to duplicate string \"%s\"", key);
 		return NULL;
+	}
 
 	/* lxc.net.<n> */
-	if (isdigit(*(key + 8))) {
-		int ret;
-		unsigned int tmpidx;
-		size_t numstrlen;
+	if (!isdigit(*(key + 8))) {
+		ERROR("Failed to detect digit in string \"%s\"", key + 8);
+		goto on_error;
+	}
 
 	/* beginning of index string */
 	idx_start = (copy + 7);
@@ -3706,13 +3715,17 @@ get_network_config_ops(const char *key, struct lxc_conf *lxc_conf, ssize_t *idx)
 
 	/* end of index string */
 	idx_end = strchr((copy + 8), '.');
-		if (!idx_end)
+	if (!idx_end) {
+		ERROR("Failed to detect \".\" in string \"%s\"", (copy + 8));
 		goto on_error;
+	}
 	*idx_end = '\0';
 
 	/* parse current index */
 	ret = lxc_safe_uint((idx_start + 1), &tmpidx);
 	if (ret < 0) {
+		ERROR("Failed to parse usigned integer from string \"%s\": %s",
+		      idx_start + 1, strerror(-ret));
 		*idx = ret;
 		goto on_error;
 	}
@@ -3722,8 +3735,7 @@ get_network_config_ops(const char *key, struct lxc_conf *lxc_conf, ssize_t *idx)
 	 * (Checking for INT_MAX here is intentional.)
 	 */
 	if (tmpidx == INT_MAX) {
-			SYSERROR(
-			    "number of configured networks would overflow the "
+		SYSERROR("number of configured networks would overflow the "
 			 "counter... what are you doing?");
 		goto on_error;
 	}
@@ -3737,15 +3749,19 @@ get_network_config_ops(const char *key, struct lxc_conf *lxc_conf, ssize_t *idx)
 
 	memmove(copy + 8, idx_end + 1, strlen(idx_end + 1));
 	copy[strlen(key) - numstrlen + 1] = '\0';
-	}
 
 	config = lxc_getconfig(copy);
-	if (!config)
+	if (!config) {
 		ERROR("unknown network configuration key %s", key);
+		goto on_error;
+	}
+
+	*deindexed_key = copy;
+	return config;
 
 on_error:
 	free(copy);
-	return config;
+	return NULL;
 }
 
 /*
@@ -3756,22 +3772,33 @@ on_error:
 static int set_config_net_nic(const char *key, const char *value,
 			      struct lxc_conf *lxc_conf, void *data)
 {
+	int ret;
+	const char *idxstring;
 	struct lxc_config_t *config;
 	struct lxc_netdev *netdev;
 	ssize_t idx = -1;
+	char *deindexed_key = NULL;
+
+	idxstring = key + 8;
+	if (!isdigit(*idxstring))
+		return -1;
 
 	if (lxc_config_value_empty(value))
 		return clr_config_net_nic(key, lxc_conf, data);
 
-	config = get_network_config_ops(key, lxc_conf, &idx);
+	config = get_network_config_ops(key, lxc_conf, &idx, &deindexed_key);
 	if (!config || idx < 0)
 		return -1;
 
 	netdev = lxc_get_netdev_by_idx(lxc_conf, (unsigned int)idx, true);
-	if (!netdev)
+	if (!netdev) {
+		free(deindexed_key);
 		return -1;
+	}
 
-	return config->set(key, value, lxc_conf, netdev);
+	ret = config->set(deindexed_key, value, lxc_conf, netdev);
+	free(deindexed_key);
+	return ret;
 }
 
 /*
@@ -3782,18 +3809,19 @@ static int set_config_net_nic(const char *key, const char *value,
 static int clr_config_net_nic(const char *key, struct lxc_conf *lxc_conf,
 			      void *data)
 {
+	int ret;
 	const char *idxstring;
 	struct lxc_config_t *config;
 	struct lxc_netdev *netdev;
-	ssize_t idx;
+	ssize_t idx = -1;
+	char *deindexed_key = NULL;
 
-	/* If we get passed "lxc.net.<n>" we clear the whole network. */
-	if (strncmp("lxc.net.", key, 8))
+	idxstring = key + 8;
+	if (!isdigit(*idxstring))
 		return -1;
 
-	idxstring = key + 8;
 	/* The left conjunct is pretty self-explanatory. The right conjunct
-	 * checks whether the two pointers are equal. If they are we now that
+	 * checks whether the two pointers are equal. If they are we know that
 	 * this is not a key that is namespaced any further and so we are
 	 * supposed to clear the whole network.
 	 */
@@ -3808,15 +3836,19 @@ static int clr_config_net_nic(const char *key, struct lxc_conf *lxc_conf,
 		return 0;
 	}
 
-	config = get_network_config_ops(key, lxc_conf, &idx);
+	config = get_network_config_ops(key, lxc_conf, &idx, &deindexed_key);
 	if (!config || idx < 0)
 		return -1;
 
 	netdev = lxc_get_netdev_by_idx(lxc_conf, (unsigned int)idx, false);
-	if (!netdev)
+	if (!netdev) {
+		free(deindexed_key);
 		return -1;
+	}
 
-	return config->clr(key, lxc_conf, netdev);
+	ret = config->clr(deindexed_key, lxc_conf, netdev);
+	free(deindexed_key);
+	return ret;
 }
 
 static int clr_config_net_type(const char *key, struct lxc_conf *lxc_conf,
@@ -4099,19 +4131,30 @@ static int clr_config_net_ipv6_address(const char *key,
 static int get_config_net_nic(const char *key, char *retv, int inlen,
 			      struct lxc_conf *c, void *data)
 {
+	int ret;
+	const char *idxstring;
 	struct lxc_config_t *config;
 	struct lxc_netdev *netdev;
 	ssize_t idx = -1;
+	char *deindexed_key = NULL;
 
-	config = get_network_config_ops(key, c, &idx);
+	idxstring = key + 8;
+	if (!isdigit(*idxstring))
+		return -1;
+
+	config = get_network_config_ops(key, c, &idx, &deindexed_key);
 	if (!config || idx < 0)
 		return -1;
 
 	netdev = lxc_get_netdev_by_idx(c, (unsigned int)idx, false);
-	if (!netdev)
+	if (!netdev) {
+		free(deindexed_key);
 		return -1;
+	}
 
-	return config->get(key, retv, inlen, c, netdev);
+	ret = config->get(deindexed_key, retv, inlen, c, netdev);
+	free(deindexed_key);
+	return ret;
 }
 
 static int get_config_net_type(const char *key, char *retv, int inlen,

src/lxc/lxccontainer.c

@@ -50,6 +50,7 @@
 #include "commands_utils.h"
 #include "confile.h"
 #include "confile_legacy.h"
+#include "confile_utils.h"
 #include "console.h"
 #include "criu.h"
 #include "log.h"
@@ -1839,18 +1840,30 @@ out:
 
 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.net") == 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);
+	if (!strcmp(key, "lxc.cgroup"))
+		return clear_unexp_config_line(conf, key, true);
+
+	if (!strcmp(key, "lxc.network"))
+		return clear_unexp_config_line(conf, key, true);
+
+	if (!strcmp(key, "lxc.net"))
+		return clear_unexp_config_line(conf, key, true);
+
+	/* Clear a network with a specific index. */
+	if (!strncmp(key, "lxc.net.", 8)) {
+		int ret;
+		const char *idx;
+
+		idx = key + 8;
+		ret = lxc_safe_uint(idx, &(unsigned int){0});
+		if (!ret)
+			return clear_unexp_config_line(conf, key, true);
+	}
+
+	if (!strcmp(key, "lxc.hook"))
+		return clear_unexp_config_line(conf, key, true);
+
+	return clear_unexp_config_line(conf, key, false);
 }
 
 static bool do_lxcapi_clear_config_item(struct lxc_container *c,
@@ -2656,13 +2669,22 @@ static bool set_config_item_locked(struct lxc_container *c, const char *key, con
 
 	if (!c->lxc_conf)
 		c->lxc_conf = lxc_conf_init();
+
 	if (!c->lxc_conf)
 		return false;
+
 	config = lxc_getconfig(key);
 	if (!config)
 		return false;
+
 	if (config->set(key, v, c->lxc_conf, NULL) != 0)
 		return false;
+
+	if (lxc_config_value_empty(v)) {
+		do_clear_unexp_config_line(c->lxc_conf, key);
+		return true;
+	}
+
 	return do_append_unexp_config_line(c->lxc_conf, key, v);
 }
 

src/tests/parse_config_file.c

@@ -942,6 +942,12 @@ int main(int argc, char *argv[])
 		goto non_test_error;
 	}
 
+	if (!set_get_compare_clear_save_load(c, "lxc.net.0.asdf", "veth",
+					    tmpf, true)) {
+		lxc_error("%s\n", "lxc.net.0.asdf");
+		goto non_test_error;
+	}
+
 	if (set_get_compare_clear_save_load_network(
 		c, "lxc.net.0.macvlan.mode", "private", tmpf, true,
 		"macvlan")) {