[lxc-devel] [lxc/master] lxccontainer: revert set_running_config_item()

brauner on Github lxc-bot at linuxcontainers.org
Mon Jan 1 21:01:52 UTC 2018


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 852 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20180101/152990d8/attachment.bin>
-------------- next part --------------
From d393e6452566a2b537cfc34b0bc3b67b04193a46 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Mon, 1 Jan 2018 21:56:23 +0100
Subject: [PATCH] lxccontainer: revert set_running_config_item()

- As discussed we will have a proper API extension that will allow updating
  various parts of a running container. The prior approach wasn't a good idea.

- Revert this is not a problem since we haven't released any version with the
  set_running_config_item() API extension.

- I'm not simply reverting so that master users can still call into new
  liblxc's without crashing the container. This is achieved by keeping the
  commands callback struct member number identical.

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/commands.c     |  64 ++-----------
 src/lxc/commands.h     |  10 +-
 src/lxc/lxccontainer.c |  21 ----
 src/lxc/lxccontainer.h |  11 ---
 src/tests/Makefile.am  |   4 +-
 src/tests/livepatch.c  | 253 -------------------------------------------------
 6 files changed, 10 insertions(+), 353 deletions(-)
 delete mode 100644 src/tests/livepatch.c

diff --git a/src/lxc/commands.c b/src/lxc/commands.c
index 4adede557..bf8cc04de 100644
--- a/src/lxc/commands.c
+++ b/src/lxc/commands.c
@@ -92,7 +92,7 @@ static const char *lxc_cmd_str(lxc_cmd_t cmd)
 		[LXC_CMD_GET_NAME]         = "get_name",
 		[LXC_CMD_GET_LXCPATH]      = "get_lxcpath",
 		[LXC_CMD_ADD_STATE_CLIENT] = "add_state_client",
-		[LXC_CMD_SET_CONFIG_ITEM]  = "set_config_item",
+		[LXC_CMD_NOOP]             = "noop",
 		[LXC_CMD_CONSOLE_LOG]      = "console_log",
 	};
 
@@ -547,60 +547,6 @@ static int lxc_cmd_get_config_item_callback(int fd, struct lxc_cmd_req *req,
 	return lxc_cmd_rsp_send(fd, &rsp);
 }
 
-/*
- * lxc_cmd_set_config_item: Get config item the running container
- *
- * @name     : name of container to connect to
- * @item     : the configuration item to set (ex: lxc.net.0.veth.pair)
- * @value    : the value to set (ex: "eth0")
- * @lxcpath  : the lxcpath in which the container is running
- *
- * Returns 0 on success, negative errno on failure.
- */
-int lxc_cmd_set_config_item(const char *name, const char *item,
-			    const char *value, const char *lxcpath)
-{
-	int ret, stopped;
-	struct lxc_cmd_set_config_item_req_data data;
-	struct lxc_cmd_rr cmd;
-
-	/* pre-validate request
-	   Currently we only support live-patching network configurations.
-	 */
-	if (strncmp(item, "lxc.net.", 8))
-		return -EINVAL;
-
-	data.item = item;
-	data.value = (void *)value;
-
-	cmd.req.cmd = LXC_CMD_SET_CONFIG_ITEM;
-	cmd.req.data = &data;
-	cmd.req.datalen = sizeof(data);
-
-	ret = lxc_cmd(name, &cmd, &stopped, lxcpath, NULL);
-	if (ret < 0)
-		return ret;
-
-	return cmd.rsp.ret;
-}
-
-static int lxc_cmd_set_config_item_callback(int fd, struct lxc_cmd_req *req,
-					    struct lxc_handler *handler)
-{
-	const char *key, *value;
-	struct lxc_cmd_rsp rsp;
-	const struct lxc_cmd_set_config_item_req_data *data;
-
-	data = req->data;
-	key = data->item;
-	value = data->value;
-
-	memset(&rsp, 0, sizeof(rsp));
-	rsp.ret = lxc_set_config_item_locked(handler->conf, key, value);
-
-	return lxc_cmd_rsp_send(fd, &rsp);
-}
-
 /*
  * lxc_cmd_get_state: Get current state of the container
  *
@@ -1110,6 +1056,12 @@ static int lxc_cmd_console_log_callback(int fd, struct lxc_cmd_req *req,
 	return lxc_cmd_rsp_send(fd, &rsp);
 }
 
+static int lxc_cmd_noop_callback(int fd, struct lxc_cmd_req *req,
+				 struct lxc_handler *handler)
+{
+	return 0;
+}
+
 static int lxc_cmd_process(int fd, struct lxc_cmd_req *req,
 			   struct lxc_handler *handler)
 {
@@ -1127,7 +1079,7 @@ static int lxc_cmd_process(int fd, struct lxc_cmd_req *req,
 		[LXC_CMD_GET_NAME]         = lxc_cmd_get_name_callback,
 		[LXC_CMD_GET_LXCPATH]      = lxc_cmd_get_lxcpath_callback,
 		[LXC_CMD_ADD_STATE_CLIENT] = lxc_cmd_add_state_client_callback,
-		[LXC_CMD_SET_CONFIG_ITEM]  = lxc_cmd_set_config_item_callback,
+		[LXC_CMD_NOOP]             = lxc_cmd_noop_callback,
 		[LXC_CMD_CONSOLE_LOG]      = lxc_cmd_console_log_callback,
 	};
 
diff --git a/src/lxc/commands.h b/src/lxc/commands.h
index aeb34eeee..a114b6867 100644
--- a/src/lxc/commands.h
+++ b/src/lxc/commands.h
@@ -49,7 +49,7 @@ typedef enum {
 	LXC_CMD_GET_NAME,
 	LXC_CMD_GET_LXCPATH,
 	LXC_CMD_ADD_STATE_CLIENT,
-	LXC_CMD_SET_CONFIG_ITEM,
+	LXC_CMD_NOOP,
 	LXC_CMD_CONSOLE_LOG,
 	LXC_CMD_MAX,
 } lxc_cmd_t;
@@ -76,11 +76,6 @@ struct lxc_cmd_console_rsp_data {
 	int ttynum;
 };
 
-struct lxc_cmd_set_config_item_req_data {
-	const char *item;
-	void *value;
-};
-
 struct lxc_cmd_console_log {
 	bool clear;
 	bool read;
@@ -130,9 +125,6 @@ extern int lxc_cmd_init(const char *name, const char *lxcpath, const char *suffi
 extern int lxc_cmd_mainloop_add(const char *name, struct lxc_epoll_descr *descr,
 				    struct lxc_handler *handler);
 extern int lxc_try_cmd(const char *name, const char *lxcpath);
-
-extern int lxc_cmd_set_config_item(const char *name, const char *item,
-				   const char *value, const char *lxcpath);
 extern int lxc_cmd_console_log(const char *name, const char *lxcpath,
 			       struct lxc_console_log *log);
 
diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index 15c05521f..4a15982cb 100644
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -2933,26 +2933,6 @@ static bool do_lxcapi_set_config_item(struct lxc_container *c, const char *key,
 
 WRAP_API_2(bool, lxcapi_set_config_item, const char *, const char *)
 
-static bool do_lxcapi_set_running_config_item(struct lxc_container *c, const char *key, const char *v)
-{
-	int ret;
-
-	if (!c)
-		return false;
-
-	if (container_mem_lock(c))
-		return false;
-
-	ret = lxc_cmd_set_config_item(c->name, key, v, do_lxcapi_get_config_path(c));
-	if (ret < 0)
-		SYSERROR("%s - Failed to live patch container", strerror(-ret));
-
-	container_mem_unlock(c);
-	return ret == 0;
-}
-
-WRAP_API_2(bool, lxcapi_set_running_config_item, const char *, const char *)
-
 static char *lxcapi_config_file_name(struct lxc_container *c)
 {
 	if (!c || !c->configfile)
@@ -4740,7 +4720,6 @@ struct lxc_container *lxc_container_new(const char *name, const char *configpath
 	c->clear_config_item = lxcapi_clear_config_item;
 	c->get_config_item = lxcapi_get_config_item;
 	c->get_running_config_item = lxcapi_get_running_config_item;
-	c->set_running_config_item = lxcapi_set_running_config_item;
 	c->get_cgroup_item = lxcapi_get_cgroup_item;
 	c->set_cgroup_item = lxcapi_set_cgroup_item;
 	c->get_config_path = lxcapi_get_config_path;
diff --git a/src/lxc/lxccontainer.h b/src/lxc/lxccontainer.h
index da709cc90..47baed604 100644
--- a/src/lxc/lxccontainer.h
+++ b/src/lxc/lxccontainer.h
@@ -826,17 +826,6 @@ struct lxc_container {
 	 */
 	int (*migrate)(struct lxc_container *c, unsigned int cmd, struct migrate_opts *opts, unsigned int size);
 
-	/*!
-	 * \brief Set a key/value configuration option on a running container.
-	 *
-	 * \param c Container.
-	 * \param key Name of option to set.
-	 * \param value Value of \p name to set.
-	 *
-	 * \return \c true on success, else \c false.
-	 */
-	bool (*set_running_config_item)(struct lxc_container *c, const char *key, const char *value);
-
 	/*!
 	 * \brief Query the console log of a container.
 	 *
diff --git a/src/tests/Makefile.am b/src/tests/Makefile.am
index 6df1a9fa6..602f5a1a6 100644
--- a/src/tests/Makefile.am
+++ b/src/tests/Makefile.am
@@ -29,7 +29,6 @@ lxc_test_utils_SOURCES = lxc-test-utils.c lxctest.h
 lxc_test_parse_config_file_SOURCES = parse_config_file.c lxctest.h
 lxc_test_config_jump_table_SOURCES = config_jump_table.c lxctest.h
 lxc_test_shortlived_SOURCES = shortlived.c
-lxc_test_livepatch_SOURCES = livepatch.c lxctest.h
 lxc_test_state_server_SOURCES = state_server.c lxctest.h
 lxc_test_share_ns_SOURCES = share_ns.c lxctest.h
 lxc_test_criu_check_feature_SOURCES = criu_check_feature.c lxctest.h
@@ -62,7 +61,7 @@ bin_PROGRAMS = lxc-test-containertests lxc-test-locktests lxc-test-startone \
 	lxc-test-snapshot lxc-test-concurrent lxc-test-may-control \
 	lxc-test-reboot lxc-test-list lxc-test-attach lxc-test-device-add-remove \
 	lxc-test-apparmor lxc-test-utils lxc-test-parse-config-file \
-	lxc-test-config-jump-table lxc-test-shortlived lxc-test-livepatch \
+	lxc-test-config-jump-table lxc-test-shortlived \
 	lxc-test-api-reboot lxc-test-state-server lxc-test-share-ns \
 	lxc-test-criu-check-feature lxc-test-raw-clone
 
@@ -102,7 +101,6 @@ EXTRA_DIST = \
 	get_item.c \
 	getkeys.c \
 	list.c \
-	livepatch.c \
 	locktests.c \
 	lxcpath.c \
 	lxc_raw_clone.c \
diff --git a/src/tests/livepatch.c b/src/tests/livepatch.c
deleted file mode 100644
index fb86757d9..000000000
--- a/src/tests/livepatch.c
+++ /dev/null
@@ -1,253 +0,0 @@
-/* liblxcapi
- *
- * Copyright © 2017 Christian Brauner <christian.brauner at ubuntu.com>.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2, as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- */
-
-#include <errno.h>
-#include <signal.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
-#include <sys/types.h>
-#include <sys/wait.h>
-
-#include <lxc/lxccontainer.h>
-
-#include "lxctest.h"
-
-int main(int argc, char *argv[])
-{
-	char *value;
-	struct lxc_container *c;
-	int ret = EXIT_FAILURE;
-
-	c = lxc_container_new("livepatch", NULL);
-	if (!c) {
-		lxc_error("%s", "Failed to create container \"livepatch\"");
-		exit(ret);
-	}
-
-	if (c->is_defined(c)) {
-		lxc_error("%s\n", "Container \"livepatch\" is defined");
-		goto on_error_put;
-	}
-
-	if (!c->set_config_item(c, "lxc.net.0.type", "veth")) {
-		lxc_error("%s\n", "Failed to set network item \"lxc.net.0.type\"");
-		goto on_error_put;
-	}
-
-	if (!c->set_config_item(c, "lxc.net.0.link", "lxcbr0")) {
-		lxc_error("%s\n", "Failed to set network item \"lxc.net.0.link\"");
-		goto on_error_put;
-	}
-
-	if (!c->set_config_item(c, "lxc.net.0.flags", "up")) {
-		lxc_error("%s\n", "Failed to set network item \"lxc.net.0.flags\"");
-		goto on_error_put;
-	}
-
-	if (!c->set_config_item(c, "lxc.net.0.name", "eth0")) {
-		lxc_error("%s\n", "Failed to set network item \"lxc.net.0.name\"");
-		goto on_error_put;
-	}
-
-	if (!c->createl(c, "busybox", NULL, NULL, 0, NULL)) {
-		lxc_error("%s\n", "Failed to create busybox container \"livepatch\"");
-		goto on_error_put;
-	}
-
-	if (!c->is_defined(c)) {
-		lxc_error("%s\n", "Container \"livepatch\" is not defined");
-		goto on_error_put;
-	}
-
-	c->clear_config(c);
-
-	if (!c->load_config(c, NULL)) {
-		lxc_error("%s\n", "Failed to load config for container \"livepatch\"");
-		goto on_error_stop;
-	}
-
-	if (!c->want_daemonize(c, true)) {
-		lxc_error("%s\n", "Failed to mark container \"livepatch\" daemonized");
-		goto on_error_stop;
-	}
-
-	if (!c->startl(c, 0, NULL)) {
-		lxc_error("%s\n", "Failed to start container \"livepatch\" daemonized");
-		goto on_error_stop;
-	}
-
-	/* Test whether the current value is ok. */
-	value = c->get_running_config_item(c, "lxc.net.0.name");
-	if (!value) {
-		lxc_error("%s\n", "Failed to retrieve running config item \"lxc.net.0.name\"");
-		goto on_error_stop;
-	}
-
-	if (strcmp(value, "eth0")) {
-		lxc_error("Retrieved unexpected value for config item "
-			  "\"lxc.net.0.name\": eth0 != %s", value);
-		free(value);
-		goto on_error_stop;
-	}
-	free(value);
-
-	/* Change current in-memory value. */
-	if (!c->set_running_config_item(c, "lxc.net.0.name", "blabla")) {
-		lxc_error("%s\n", "Failed to set running config item "
-				  "\"lxc.net.0.name\" to \"blabla\"");
-		goto on_error_stop;
-	}
-
-	/* Verify change. */
-	value = c->get_running_config_item(c, "lxc.net.0.name");
-	if (!value) {
-		lxc_error("%s\n", "Failed to retrieve running config item \"lxc.net.0.name\"");
-		goto on_error_stop;
-	}
-
-	if (strcmp(value, "blabla")) {
-		lxc_error("Retrieved unexpected value for config item "
-			  "\"lxc.net.0.name\": blabla != %s", value);
-		free(value);
-		goto on_error_stop;
-	}
-	free(value);
-
-	/* Change current in-memory value. */
-	if (!c->set_running_config_item(c, "lxc.net.0.name", "eth0")) {
-		lxc_error("%s\n", "Failed to set running config item "
-				  "\"lxc.net.0.name\" to \"eth0\"");
-		goto on_error_stop;
-	}
-
-	/* Add new in-memory value. */
-	if (!c->set_running_config_item(c, "lxc.net.1.type", "veth")) {
-		lxc_error("%s\n", "Failed to set running config item "
-				  "\"lxc.net.1.type\" to \"veth\"");
-		goto on_error_stop;
-	}
-
-	/* Verify change. */
-	value = c->get_running_config_item(c, "lxc.net.1.type");
-	if (!value) {
-		lxc_error("%s\n", "Failed to retrieve running config item \"lxc.net.1.type\"");
-		goto on_error_stop;
-	}
-
-	if (strcmp(value, "veth")) {
-		lxc_error("Retrieved unexpected value for config item "
-			  "\"lxc.net.1.type\": veth != %s", value);
-		free(value);
-		goto on_error_stop;
-	}
-	free(value);
-
-	/* Add new in-memory value. */
-	if (!c->set_running_config_item(c, "lxc.net.1.flags", "up")) {
-		lxc_error("%s\n", "Failed to set running config item "
-				  "\"lxc.net.1.flags\" to \"up\"");
-		goto on_error_stop;
-	}
-
-	/* Verify change. */
-	value = c->get_running_config_item(c, "lxc.net.1.flags");
-	if (!value) {
-		lxc_error("%s\n", "Failed to retrieve running config item \"lxc.net.1.flags\"");
-		goto on_error_stop;
-	}
-
-	if (strcmp(value, "up")) {
-		lxc_error("Retrieved unexpected value for config item "
-			  "\"lxc.net.1.flags\": up != %s", value);
-		free(value);
-		goto on_error_stop;
-	}
-	free(value);
-
-	/* Add new in-memory value. */
-	if (!c->set_running_config_item(c, "lxc.net.1.link", "lxcbr0")) {
-		lxc_error("%s\n", "Failed to set running config item "
-				  "\"lxc.net.1.link\" to \"lxcbr0\"");
-		goto on_error_stop;
-	}
-
-	/* Verify change. */
-	value = c->get_running_config_item(c, "lxc.net.1.link");
-	if (!value) {
-		lxc_error("%s\n", "Failed to retrieve running config item \"lxc.net.1.link\"");
-		goto on_error_stop;
-	}
-
-	if (strcmp(value, "lxcbr0")) {
-		lxc_error("Retrieved unexpected value for config item "
-			  "\"lxc.net.1.link\": lxcbr0 != %s", value);
-		free(value);
-		goto on_error_stop;
-	}
-	free(value);
-
-	if (!c->reboot2(c, -1)) {
-		lxc_error("%s", "Failed to create container \"livepatch\"");
-		goto on_error_stop;
-	}
-
-	if (!c->is_running(c)) {
-		lxc_error("%s\n", "Failed to reboot container \"livepatch\"");
-		goto on_error_destroy;
-	}
-
-	/* Remove in-memory value. */
-	if (!c->set_running_config_item(c, "lxc.net.1.name", "eth1")) {
-		lxc_error("%s\n", "Failed to clear running config item "
-				  "\"lxc.net.1.name\"");
-		goto on_error_stop;
-	}
-
-	if (!c->stop(c)) {
-		lxc_error("%s\n", "Failed to stop container \"livepatch\"");
-		goto on_error_stop;
-	}
-
-	if (!c->startl(c, 0, NULL)) {
-		lxc_error("%s\n", "Failed to start container \"livepatch\" daemonized");
-		goto on_error_destroy;
-	}
-
-	/* Remove in-memory value. */
-	if (!c->set_running_config_item(c, "lxc.net.1.mtu", "3000")) {
-		lxc_error("%s\n", "Failed to set running config item "
-				  "\"lxc.net.1.mtu\"");
-		goto on_error_stop;
-	}
-
-	ret = 0;
-
-on_error_stop:
-	if (c->is_running(c) && !c->stop(c))
-		lxc_error("%s\n", "Failed to stop container \"livepatch\"");
-
-on_error_destroy:
-	if (!c->destroy(c))
-		lxc_error("%s\n", "Failed to destroy container \"livepatch\"");
-
-on_error_put:
-	lxc_container_put(c);
-	exit(ret);
-}


More information about the lxc-devel mailing list