[lxc-devel] [lxc/master] lxccontainer: implement proper container reboot
brauner on Github
lxc-bot at linuxcontainers.org
Tue Nov 21 20:50:52 UTC 2017
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 452 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20171121/fcaa13ef/attachment.bin>
-------------- next part --------------
From 0728e5e03d7b36f509077cf5289a747a8f55617e Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 21 Nov 2017 20:42:28 +0100
Subject: [PATCH 1/3] commands: allow waiting for all states
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/commands.c | 25 +------------------------
1 file changed, 1 insertion(+), 24 deletions(-)
diff --git a/src/lxc/commands.c b/src/lxc/commands.c
index 553382993..93bdb4766 100644
--- a/src/lxc/commands.c
+++ b/src/lxc/commands.c
@@ -916,29 +916,7 @@ int lxc_cmd_add_state_client(const char *name, const char *lxcpath,
return -1;
} else if (states[state]) {
process_unlock();
- TRACE("Container is %s state", lxc_state2str(state));
- return state;
- }
-
- if ((state == STARTING) && !states[RUNNING] && !states[STOPPING] && !states[STOPPED]) {
- process_unlock();
- TRACE("Container is in %s state and caller requested to be "
- "informed about a previous state", lxc_state2str(state));
- return state;
- } else if ((state == RUNNING) && !states[STOPPING] && !states[STOPPED]) {
- process_unlock();
- TRACE("Container is in %s state and caller requested to be "
- "informed about a previous state", lxc_state2str(state));
- return state;
- } else if ((state == STOPPING) && !states[STOPPED]) {
- process_unlock();
- TRACE("Container is in %s state and caller requested to be "
- "informed about a previous state", lxc_state2str(state));
- return state;
- } else if ((state == STOPPED) || (state == ABORTING)) {
- process_unlock();
- TRACE("Container is in %s state and caller requested to be "
- "informed about a previous state", lxc_state2str(state));
+ TRACE("Container is in %s state", lxc_state2str(state));
return state;
}
@@ -952,7 +930,6 @@ int lxc_cmd_add_state_client(const char *name, const char *lxcpath,
/* We should now be guaranteed to get an answer from the state sending
* function.
*/
-
if (cmd.rsp.ret < 0) {
ERROR("Failed to receive socket fd");
return -1;
From 32d012f755ef68f14e22155a742491cf27590d58 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 21 Nov 2017 21:29:57 +0100
Subject: [PATCH 2/3] lxccontainer: implement proper container reboot
This enables the reboot() API function to properly wait until the reboot
succeeded.
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/commands.c | 6 ++---
src/lxc/commands_utils.c | 4 ++--
src/lxc/conf.c | 1 +
src/lxc/conf.h | 8 +++++++
src/lxc/lxccontainer.c | 34 +++++++++++++++++++++++++---
src/lxc/start.c | 58 +++++++++++++++++++++++++++++++++++-------------
src/lxc/start.h | 8 -------
src/lxc/state.c | 6 ++---
src/lxc/tools/lxc_stop.c | 49 +++++-----------------------------------
9 files changed, 96 insertions(+), 78 deletions(-)
diff --git a/src/lxc/commands.c b/src/lxc/commands.c
index 93bdb4766..735f5fcc1 100644
--- a/src/lxc/commands.c
+++ b/src/lxc/commands.c
@@ -914,7 +914,7 @@ int lxc_cmd_add_state_client(const char *name, const char *lxcpath,
process_unlock();
TRACE("%s - Failed to retrieve state of container", strerror(errno));
return -1;
- } else if (states[state]) {
+ } else if (states[state] == 1) {
process_unlock();
TRACE("Container is in %s state", lxc_state2str(state));
return state;
@@ -1132,7 +1132,7 @@ static void lxc_cmd_fd_cleanup(int fd, struct lxc_handler *handler,
struct lxc_epoll_descr *descr,
const lxc_cmd_t cmd)
{
- struct state_client *client;
+ struct lxc_state_client *client;
struct lxc_list *cur, *next;
lxc_console_free(handler->conf, fd);
@@ -1143,7 +1143,7 @@ static void lxc_cmd_fd_cleanup(int fd, struct lxc_handler *handler,
}
process_lock();
- lxc_list_for_each_safe(cur, &handler->state_clients, next) {
+ lxc_list_for_each_safe(cur, &handler->conf->state_clients, next) {
client = cur->elem;
if (client->clientfd != fd)
continue;
diff --git a/src/lxc/commands_utils.c b/src/lxc/commands_utils.c
index 30c224519..6d4fcbb7e 100644
--- a/src/lxc/commands_utils.c
+++ b/src/lxc/commands_utils.c
@@ -192,7 +192,7 @@ int lxc_cmd_connect(const char *name, const char *lxcpath,
int lxc_add_state_client(int state_client_fd, struct lxc_handler *handler,
lxc_state_t states[MAX_STATE])
{
- struct state_client *newclient;
+ struct lxc_state_client *newclient;
struct lxc_list *tmplist;
newclient = malloc(sizeof(*newclient));
@@ -210,7 +210,7 @@ int lxc_add_state_client(int state_client_fd, struct lxc_handler *handler,
}
lxc_list_add_elem(tmplist, newclient);
- lxc_list_add_tail(&handler->state_clients, tmplist);
+ lxc_list_add_tail(&handler->conf->state_clients, tmplist);
TRACE("added state client %d to state client list", state_client_fd);
diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index 4681a7274..47b9225b3 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -2466,6 +2466,7 @@ struct lxc_conf *lxc_conf_init(void)
for (i = 0; i < NUM_LXC_HOOKS; i++)
lxc_list_init(&new->hooks[i]);
lxc_list_init(&new->groups);
+ lxc_list_init(&new->state_clients);
new->lsm_aa_profile = NULL;
new->lsm_se_context = NULL;
new->tmp_umount_proc = 0;
diff --git a/src/lxc/conf.h b/src/lxc/conf.h
index 58302cf30..eca4c5120 100644
--- a/src/lxc/conf.h
+++ b/src/lxc/conf.h
@@ -248,6 +248,11 @@ enum lxchooks {
extern char *lxchook_names[NUM_LXC_HOOKS];
+struct lxc_state_client {
+ int clientfd;
+ lxc_state_t states[MAX_STATE];
+};
+
struct lxc_conf {
int is_execute;
char *fstab;
@@ -359,6 +364,9 @@ struct lxc_conf {
struct lxc_cgroup cgroup_meta;
char *inherit_ns[LXC_NS_MAX];
+
+ /* A list of clients registered to be informed about a container state. */
+ struct lxc_list state_clients;
};
#ifdef HAVE_TLS
diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index 413dd375b..aa3a479e6 100644
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -1771,24 +1771,52 @@ static bool lxcapi_create(struct lxc_container *c, const char *t,
static bool do_lxcapi_reboot(struct lxc_container *c)
{
+ int ret;
pid_t pid;
- int rebootsignal = SIGINT;
+ int rebootsignal = SIGINT, sock = -1;
+ bool retv = false;
+ lxc_state_t states[MAX_STATE] = {0};
if (!c)
return false;
+
if (!do_lxcapi_is_running(c))
return false;
+
pid = do_lxcapi_init_pid(c);
if (pid <= 0)
return false;
+
if (c->lxc_conf && c->lxc_conf->rebootsignal)
rebootsignal = c->lxc_conf->rebootsignal;
+
+ /* Add a new state client before sending the reboot signal so that we
+ * don't miss a state.
+ */
+ states[RUNNING] = 2;
+ ret = lxc_cmd_add_state_client(c->name, c->config_path, states, &sock);
+
if (kill(pid, rebootsignal) < 0) {
- WARN("Could not send signal %d to pid %d.", rebootsignal, pid);
+ WARN("Could not send signal %d to pid %d", rebootsignal, pid);
+ close(sock);
return false;
}
- return true;
+ /* Retrieve the state. */
+ if (sock >= 0) {
+ int state;
+ state = lxc_cmd_sock_rcv_state(sock, 0);
+ close(sock);
+ if (state != RUNNING)
+ return false;
+ TRACE("Container rebooted");
+ retv = true;
+ } else {
+ TRACE("Received state \"%s\" instead of expected \"RUNNING\"",
+ lxc_state2str(ret));
+ }
+
+ return retv;
}
WRAP_API(bool, lxcapi_reboot)
diff --git a/src/lxc/start.c b/src/lxc/start.c
index 4d583125b..c300d96e5 100644
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -200,6 +200,9 @@ int lxc_check_inherited(struct lxc_conf *conf, bool closeall,
fddir = dirfd(dir);
while ((direntp = readdir(dir))) {
+ struct lxc_list *cur;
+ bool matched = false;
+
if (!direntp)
break;
@@ -222,6 +225,20 @@ int lxc_check_inherited(struct lxc_conf *conf, bool closeall,
(i < len_fds && fd == fds_to_ignore[i]))
continue;
+ /* Keep state clients that wait on reboots. */
+ lxc_list_for_each(cur, &conf->state_clients) {
+ struct lxc_state_client *client = cur->elem;
+
+ if (client->clientfd != fd)
+ continue;
+
+ matched = true;
+ break;
+ }
+
+ if (matched)
+ continue;
+
if (current_config && fd == current_config->logfd)
continue;
@@ -338,7 +355,7 @@ static int lxc_serve_state_clients(const char *name,
{
ssize_t ret;
struct lxc_list *cur, *next;
- struct state_client *client;
+ struct lxc_state_client *client;
struct lxc_msg msg = {.type = lxc_msg_state, .value = state};
process_lock();
@@ -349,7 +366,7 @@ static int lxc_serve_state_clients(const char *name,
handler->state = state;
TRACE("Set container state to %s", lxc_state2str(state));
- if (lxc_list_empty(&handler->state_clients)) {
+ if (lxc_list_empty(&handler->conf->state_clients)) {
TRACE("No state clients registered");
process_unlock();
lxc_monitor_send_state(name, state, handler->lxcpath);
@@ -359,10 +376,10 @@ static int lxc_serve_state_clients(const char *name,
strncpy(msg.name, name, sizeof(msg.name));
msg.name[sizeof(msg.name) - 1] = 0;
- lxc_list_for_each_safe(cur, &handler->state_clients, next) {
+ lxc_list_for_each_safe(cur, &handler->conf->state_clients, next) {
client = cur->elem;
- if (!client->states[state]) {
+ if (client->states[state] == 0) {
TRACE("State %s not registered for state client %d",
lxc_state2str(state), client->clientfd);
continue;
@@ -531,7 +548,8 @@ struct lxc_handler *lxc_init_handler(const char *name, struct lxc_conf *conf,
handler->lxcpath = lxcpath;
handler->pinfd = -1;
handler->state_socket_pair[0] = handler->state_socket_pair[1] = -1;
- lxc_list_init(&handler->state_clients);
+ if (handler->conf->reboot == 0)
+ lxc_list_init(&handler->conf->state_clients);
for (i = 0; i < LXC_NS_MAX; i++)
handler->nsfd[i] = -1;
@@ -554,10 +572,12 @@ struct lxc_handler *lxc_init_handler(const char *name, struct lxc_conf *conf,
handler->state_socket_pair[1]);
}
- handler->conf->maincmd_fd = lxc_cmd_init(name, lxcpath, "command");
- if (handler->conf->maincmd_fd < 0) {
- ERROR("Failed to set up command socket");
- goto on_error;
+ if (handler->conf->reboot == 0) {
+ handler->conf->maincmd_fd = lxc_cmd_init(name, lxcpath, "command");
+ if (handler->conf->maincmd_fd < 0) {
+ ERROR("Failed to set up command socket");
+ goto on_error;
+ }
}
TRACE("Unix domain socket %d for command server is ready",
handler->conf->maincmd_fd);
@@ -713,9 +733,11 @@ void lxc_fini(const char *name, struct lxc_handler *handler)
lxc_set_state(name, handler, STOPPED);
- /* close command socket */
- close(handler->conf->maincmd_fd);
- handler->conf->maincmd_fd = -1;
+ if (handler->conf->reboot == 0) {
+ /* close command socket */
+ close(handler->conf->maincmd_fd);
+ handler->conf->maincmd_fd = -1;
+ }
if (run_lxc_hooks(name, "post-stop", handler->conf, handler->lxcpath, NULL)) {
ERROR("Failed to run lxc.hook.post-stop for container \"%s\".", name);
@@ -737,8 +759,13 @@ void lxc_fini(const char *name, struct lxc_handler *handler)
/* The command socket is now closed, no more state clients can register
* themselves from now on. So free the list of state clients.
*/
- lxc_list_for_each_safe(cur, &handler->state_clients, next) {
- struct state_client *client = cur->elem;
+ lxc_list_for_each_safe(cur, &handler->conf->state_clients, next) {
+ struct lxc_state_client *client = cur->elem;
+
+ /* Keep state clients that want to be notified about reboots. */
+ if ((handler->conf->reboot > 0) && (client->states[RUNNING] == 2))
+ continue;
+
/* close state client socket */
close(client->clientfd);
lxc_list_del(cur);
@@ -796,9 +823,8 @@ static int do_start(void *data)
lxc_sync_fini_parent(handler);
/* Don't leak the pinfd to the container. */
- if (handler->pinfd >= 0) {
+ if (handler->pinfd >= 0)
close(handler->pinfd);
- }
if (lxc_sync_wait_parent(handler, LXC_SYNC_STARTUP))
return -1;
diff --git a/src/lxc/start.h b/src/lxc/start.h
index c67322693..e2b13141b 100644
--- a/src/lxc/start.h
+++ b/src/lxc/start.h
@@ -85,9 +85,6 @@ struct lxc_handler {
/* The container's in-memory configuration. */
struct lxc_conf *conf;
- /* A list of clients registered to be informed about a container state. */
- struct lxc_list state_clients;
-
/* A set of operations to be performed at various stages of the
* container's life.
*/
@@ -110,11 +107,6 @@ struct lxc_operations {
int (*post_start)(struct lxc_handler *, void *);
};
-struct state_client {
- int clientfd;
- lxc_state_t states[MAX_STATE];
-};
-
extern int lxc_poll(const char *name, struct lxc_handler *handler);
extern int lxc_set_state(const char *name, struct lxc_handler *handler, lxc_state_t state);
extern void lxc_abort(const char *name, struct lxc_handler *handler);
diff --git a/src/lxc/state.c b/src/lxc/state.c
index 97b22a98e..9c9bf8318 100644
--- a/src/lxc/state.c
+++ b/src/lxc/state.c
@@ -45,9 +45,9 @@
lxc_log_define(lxc_state, lxc);
-static const char * const strstate[] = {
- "STOPPED", "STARTING", "RUNNING", "STOPPING",
- "ABORTING", "FREEZING", "FROZEN", "THAWED",
+static const char *const strstate[] = {
+ "STOPPED", "STARTING", "RUNNING", "STOPPING",
+ "ABORTING", "FREEZING", "FROZEN", "THAWED",
};
const char *lxc_state2str(lxc_state_t state)
diff --git a/src/lxc/tools/lxc_stop.c b/src/lxc/tools/lxc_stop.c
index 55674a2b3..807fc031c 100644
--- a/src/lxc/tools/lxc_stop.c
+++ b/src/lxc/tools/lxc_stop.c
@@ -99,56 +99,15 @@ Options :\n\
/* returns -1 on failure, 0 on success */
static int do_reboot_and_check(struct lxc_arguments *a, struct lxc_container *c)
{
- int ret;
pid_t pid;
- pid_t newpid;
- int timeout = a->timeout;
pid = c->init_pid(c);
if (pid == -1)
return -1;
+
if (!c->reboot(c))
return -1;
- if (a->nowait)
- return 0;
- if (timeout == 0)
- goto out;
-
- for (;;) {
- /* can we use c-> wait for this, assuming it will
- * re-enter RUNNING? For now just sleep */
- int elapsed_time, curtime = 0;
- struct timeval tv;
-
- newpid = c->init_pid(c);
- if (newpid != -1 && newpid != pid)
- return 0;
-
- if (timeout != -1) {
- ret = gettimeofday(&tv, NULL);
- if (ret)
- break;
- curtime = tv.tv_sec;
- }
-
- sleep(1);
- if (timeout != -1) {
- ret = gettimeofday(&tv, NULL);
- if (ret)
- break;
- elapsed_time = tv.tv_sec - curtime;
- if (timeout - elapsed_time <= 0)
- break;
- timeout -= elapsed_time;
- }
- }
-out:
- newpid = c->init_pid(c);
- if (newpid == -1 || newpid == pid) {
- printf("Reboot did not complete before timeout\n");
- return -1;
- }
return 0;
}
@@ -261,7 +220,11 @@ int main(int argc, char *argv[])
/* reboot */
if (my_args.reboot) {
- ret = do_reboot_and_check(&my_args, c) < 0 ? EXIT_SUCCESS : EXIT_FAILURE;
+ ret = do_reboot_and_check(&my_args, c);
+ if (ret < 0)
+ ret = EXIT_FAILURE;
+ else
+ ret = EXIT_SUCCESS;
goto out;
}
From 0626ffdf543956b9ebbec79e12de408affa77fdb Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 21 Nov 2017 21:48:30 +0100
Subject: [PATCH 3/3] test: add test for reboot() API function
Since reboot() now correctly waits until the container rebooted and receives a
proper notification add tests for it.
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/tests/livepatch.c | 3 ---
src/tests/reboot.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 72 insertions(+), 5 deletions(-)
diff --git a/src/tests/livepatch.c b/src/tests/livepatch.c
index 7189cb36c..6f2a96fb0 100644
--- a/src/tests/livepatch.c
+++ b/src/tests/livepatch.c
@@ -208,9 +208,6 @@ int main(int argc, char *argv[])
goto on_error_stop;
}
- /* Busybox shouldn't take long to reboot. Sleep for 5s. */
- sleep(5);
-
if (!c->is_running(c)) {
lxc_error("%s\n", "Failed to reboot container \"livepatch\"");
goto on_error_destroy;
diff --git a/src/tests/reboot.c b/src/tests/reboot.c
index 1f059e0d7..4562882f1 100644
--- a/src/tests/reboot.c
+++ b/src/tests/reboot.c
@@ -27,7 +27,9 @@
#include <sys/types.h>
#include <sys/wait.h>
+#include "lxc/lxccontainer.h"
#include "lxc/namespace.h"
+#include "lxctest.h"
#include <sched.h>
#include <linux/sched.h>
@@ -97,7 +99,9 @@ static int have_reboot_patch(void)
int main(int argc, char *argv[])
{
- int status;
+ int i, status;
+ struct lxc_container *c;
+ int ret = EXIT_FAILURE;
if (getuid() != 0) {
printf("Must run as root.\n");
@@ -137,6 +141,72 @@ int main(int argc, char *argv[])
return 1;
printf("reboot(LINUX_REBOOT_CMD_POWERR_OFF) succeed\n");
+ /* Test that the reboot() API function properly waits for containers to
+ * restart.
+ */
+ c = lxc_container_new("reboot", NULL);
+ if (!c) {
+ lxc_error("%s", "Failed to create container \"reboot\"");
+ exit(ret);
+ }
+
+ if (c->is_defined(c)) {
+ lxc_error("%s\n", "Container \"reboot\" is defined");
+ goto on_error_put;
+ }
+
+ if (!c->createl(c, "busybox", NULL, NULL, 0, NULL)) {
+ lxc_error("%s\n", "Failed to create busybox container \"reboot\"");
+ goto on_error_put;
+ }
+
+ if (!c->is_defined(c)) {
+ lxc_error("%s\n", "Container \"reboot\" 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 \"reboot\"");
+ goto on_error_stop;
+ }
+
+ if (!c->want_daemonize(c, true)) {
+ lxc_error("%s\n", "Failed to mark container \"reboot\" daemonized");
+ goto on_error_stop;
+ }
+
+ if (!c->startl(c, 0, NULL)) {
+ lxc_error("%s\n", "Failed to start container \"reboot\" daemonized");
+ goto on_error_stop;
+ }
+
+ /* reboot 10 times */
+ for (i = 0; i < 10; i++) {
+ if (!c->reboot(c)) {
+ lxc_error("%s\n", "Failed to create container \"reboot\"");
+ goto on_error_stop;
+ }
+
+ if (!c->is_running(c)) {
+ lxc_error("%s\n", "Failed to reboot container \"reboot\"");
+ goto on_error_stop;
+ }
+ lxc_debug("%s\n", "Container \"reboot\" rebooted successfully");
+ }
+
+ ret = 0;
+
+on_error_stop:
+ if (c->is_running(c) && !c->stop(c))
+ lxc_error("%s\n", "Failed to stop container \"reboot\"");
+
+ if (!c->destroy(c))
+ lxc_error("%s\n", "Failed to destroy container \"reboot\"");
+
+on_error_put:
+ lxc_container_put(c);
printf("All tests passed\n");
- return 0;
+ exit(ret);
}
More information about the lxc-devel
mailing list