From b1ca434ae52c6b1ac9f73030d022e2a2e537cb3f Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 22 Nov 2017 14:13:07 +0100 Subject: [PATCH 01/15] commands: don't traverse whole list When we remove a state client fd there's not reason to walk the whole list. We can simply break once we found and removed the fd. Signed-off-by: Christian Brauner --- src/lxc/commands.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/lxc/commands.c b/src/lxc/commands.c index 553382993..874a6f791 100644 --- a/src/lxc/commands.c +++ b/src/lxc/commands.c @@ -1176,6 +1176,10 @@ static void lxc_cmd_fd_cleanup(int fd, struct lxc_handler *handler, lxc_list_del(cur); free(cur->elem); free(cur); + /* No need to walk the whole list. If we found the state client + * fd there can't be a second one. + */ + break; } process_unlock(); } From e533be71c8a397ebc6d72c867ff8627d0a87260e Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 22 Nov 2017 14:25:10 +0100 Subject: [PATCH 02/15] commands: don't lock atomic operations We're dealing with an integer (lxc_state_t which is an enum). Any POSIX implementation makes those operations atomic so there's not need in locking this. Signed-off-by: Christian Brauner --- src/lxc/commands.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/lxc/commands.c b/src/lxc/commands.c index 874a6f791..a896a47c2 100644 --- a/src/lxc/commands.c +++ b/src/lxc/commands.c @@ -904,44 +904,35 @@ int lxc_cmd_add_state_client(const char *name, const char *lxcpath, }, }; - /* Lock the whole lxc_cmd_add_state_client_callback() call to ensure - * that lxc_set_state() doesn't cause us to miss a state. - */ - process_lock(); /* Check if already in requested state. */ state = lxc_getstate(name, lxcpath); if (state < 0) { - process_unlock(); TRACE("%s - Failed to retrieve state of container", strerror(errno)); 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)); return state; } + process_lock(); ret = lxc_cmd(name, &cmd, &stopped, lxcpath, NULL); process_unlock(); if (ret < 0) { From f3a2945e8852a50550d00fc3510f669629c28702 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 22 Nov 2017 14:29:32 +0100 Subject: [PATCH 03/15] commands: don't lock the whole command There are multiple reasons why this is not required: - every command is transactional - we only care about the list being modified not the memory allocation and other costly operations Signed-off-by: Christian Brauner --- src/lxc/commands.c | 2 -- src/lxc/commands_utils.c | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/lxc/commands.c b/src/lxc/commands.c index a896a47c2..8998dc373 100644 --- a/src/lxc/commands.c +++ b/src/lxc/commands.c @@ -932,9 +932,7 @@ int lxc_cmd_add_state_client(const char *name, const char *lxcpath, return state; } - process_lock(); ret = lxc_cmd(name, &cmd, &stopped, lxcpath, NULL); - process_unlock(); if (ret < 0) { ERROR("%s - Failed to execute command", strerror(errno)); return -1; diff --git a/src/lxc/commands_utils.c b/src/lxc/commands_utils.c index 30c224519..e03022f28 100644 --- a/src/lxc/commands_utils.c +++ b/src/lxc/commands_utils.c @@ -33,6 +33,7 @@ #include "commands_utils.h" #include "initutils.h" #include "log.h" +#include "lxclock.h" #include "monitor.h" #include "state.h" #include "utils.h" @@ -209,8 +210,10 @@ int lxc_add_state_client(int state_client_fd, struct lxc_handler *handler, return -ENOMEM; } + process_lock(); lxc_list_add_elem(tmplist, newclient); lxc_list_add_tail(&handler->state_clients, tmplist); + process_unlock(); TRACE("added state client %d to state client list", state_client_fd); From 20144819c4207911ac08654943e4273d7b97f353 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 22 Nov 2017 14:32:56 +0100 Subject: [PATCH 04/15] start: don't lock setting the state - setting the handler->state value is atomic on any POSIX implementation since we're dealing with an integer (enum/lxc_state_t) - while the state clients are served it is not possible for lxc_set_state() to transition to the next state anyway so there's no danger in moving to the next state with clients missing it - we only care about the list being modified Signed-off-by: Christian Brauner --- src/lxc/start.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/lxc/start.c b/src/lxc/start.c index 9bbf9756a..a7bb6a665 100644 --- a/src/lxc/start.c +++ b/src/lxc/start.c @@ -341,14 +341,10 @@ static int lxc_serve_state_clients(const char *name, struct state_client *client; struct lxc_msg msg = {.type = lxc_msg_state, .value = state}; - process_lock(); - - /* Only set state under process lock held so that we don't cause - * lxc_cmd_add_state_client() to miss a state. - */ handler->state = state; TRACE("Set container state to %s", lxc_state2str(state)); + process_lock(); if (lxc_list_empty(&handler->state_clients)) { TRACE("No state clients registered"); process_unlock(); From 24b0bd9a80a1a83b292afa61b1970730d01d4963 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Tue, 21 Nov 2017 20:42:28 +0100 Subject: [PATCH 05/15] commands: allow waiting for all states Signed-off-by: Christian Brauner --- src/lxc/commands.c | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/src/lxc/commands.c b/src/lxc/commands.c index 8998dc373..4056fb7ac 100644 --- a/src/lxc/commands.c +++ b/src/lxc/commands.c @@ -914,24 +914,6 @@ int lxc_cmd_add_state_client(const char *name, const char *lxcpath, return state; } - if ((state == STARTING) && !states[RUNNING] && !states[STOPPING] && !states[STOPPED]) { - 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]) { - 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]) { - 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)) { - TRACE("Container is in %s state and caller requested to be " - "informed about a previous state", lxc_state2str(state)); - return state; - } - ret = lxc_cmd(name, &cmd, &stopped, lxcpath, NULL); if (ret < 0) { ERROR("%s - Failed to execute command", strerror(errno)); @@ -941,7 +923,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 d39b10eba1837e3638925e653eb7121d1563cbe3 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Tue, 21 Nov 2017 21:29:57 +0100 Subject: [PATCH 06/15] lxccontainer: add reboot2() API extension This adds reboot2() as a new API extension. This function properly wait until a reboot succeeded. It takes a timeout argument. When set to > 0 reboot2() will block until the timeout is reached, if timeout is set to zero reboot2() will not block, if set to -1 reboot2() will block indefinitly. The struct state_client gets rename to lxc_state_client since it's more in line with other declarations. It also gets moved from the lxc_handler to the lxc_conf struct so that the state clients waiting for reboots don't get deallocated on reboot since the handler is deallocated on reboot. Signed-off-by: Christian Brauner --- src/lxc/commands.c | 8 ++--- src/lxc/commands_utils.c | 4 +-- src/lxc/conf.c | 1 + src/lxc/conf.h | 7 +++++ src/lxc/lxccontainer.c | 68 ++++++++++++++++++++++++++++++++++++++++ src/lxc/lxccontainer.h | 11 +++++++ src/lxc/start.c | 58 ++++++++++++++++++++++++---------- src/lxc/start.h | 8 ----- src/lxc/state.c | 6 ++-- src/lxc/tools/lxc_stop.c | 62 +++--------------------------------- 10 files changed, 143 insertions(+), 90 deletions(-) diff --git a/src/lxc/commands.c b/src/lxc/commands.c index 4056fb7ac..d7e41e5a7 100644 --- a/src/lxc/commands.c +++ b/src/lxc/commands.c @@ -909,8 +909,8 @@ int lxc_cmd_add_state_client(const char *name, const char *lxcpath, if (state < 0) { TRACE("%s - Failed to retrieve state of container", strerror(errno)); return -1; - } else if (states[state]) { - TRACE("Container is %s state", lxc_state2str(state)); + } else if (states[state] == 1) { + TRACE("Container is in %s state", lxc_state2str(state)); return state; } @@ -1125,7 +1125,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); @@ -1136,7 +1136,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 e03022f28..394f0a1d7 100644 --- a/src/lxc/commands_utils.c +++ b/src/lxc/commands_utils.c @@ -193,7 +193,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)); @@ -212,7 +212,7 @@ int lxc_add_state_client(int state_client_fd, struct lxc_handler *handler, process_lock(); lxc_list_add_elem(tmplist, newclient); - lxc_list_add_tail(&handler->state_clients, tmplist); + lxc_list_add_tail(&handler->conf->state_clients, tmplist); process_unlock(); 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 359482580..2aa057b1e 100644 --- a/src/lxc/conf.c +++ b/src/lxc/conf.c @@ -2419,6 +2419,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 43fc2b450..ce259c42b 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; @@ -363,6 +368,8 @@ struct lxc_conf { /* init working directory */ char* init_cwd; + /* A list of clients registered to be informed about a container state. */ + struct lxc_list state_clients; }; int write_id_mapping(enum idtype idtype, pid_t pid, const char *buf, diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c index 413dd375b..46ca47f0f 100644 --- a/src/lxc/lxccontainer.c +++ b/src/lxc/lxccontainer.c @@ -1793,6 +1793,73 @@ static bool do_lxcapi_reboot(struct lxc_container *c) WRAP_API(bool, lxcapi_reboot) +static bool do_lxcapi_reboot2(struct lxc_container *c, int timeout) +{ + int killret, ret; + pid_t pid; + int rebootsignal = SIGINT, state_client_fd = -1; + lxc_state_t states[MAX_STATE] = {0}; + + if (!c) + return false; + + if (!do_lxcapi_is_running(c)) + return true; + + pid = do_lxcapi_init_pid(c); + if (pid <= 0) + return true; + + if (c->lxc_conf && c->lxc_conf->rebootsignal) + rebootsignal = c->lxc_conf->rebootsignal; + + /* Add a new state client before sending the shutdown signal so that we + * don't miss a state. + */ + if (timeout != 0) { + states[RUNNING] = 2; + ret = lxc_cmd_add_state_client(c->name, c->config_path, states, + &state_client_fd); + if (ret < 0) + return false; + + if (state_client_fd < 0) + return false; + + if (ret == RUNNING) + return true; + + if (ret < MAX_STATE) + return false; + } + + /* Send reboot signal to container. */ + killret = kill(pid, rebootsignal); + if (killret < 0) { + WARN("Could not send signal %d to pid %d", rebootsignal, pid); + if (state_client_fd >= 0) + close(state_client_fd); + return false; + } + TRACE("Sent signal %d to pid %d", rebootsignal, pid); + + if (timeout == 0) + return true; + + ret = lxc_cmd_sock_rcv_state(state_client_fd, timeout); + close(state_client_fd); + if (ret < 0) + return false; + + TRACE("Received state \"%s\"", lxc_state2str(ret)); + if (ret != RUNNING) + return false; + + return true; +} + +WRAP_API_1(bool, lxcapi_reboot2, int) + static bool do_lxcapi_shutdown(struct lxc_container *c, int timeout) { int ret, state_client_fd = -1; @@ -4595,6 +4662,7 @@ struct lxc_container *lxc_container_new(const char *name, const char *configpath c->createl = lxcapi_createl; c->shutdown = lxcapi_shutdown; c->reboot = lxcapi_reboot; + c->reboot2 = lxcapi_reboot2; c->clear_config = lxcapi_clear_config; c->clear_config_item = lxcapi_clear_config_item; c->get_config_item = lxcapi_get_config_item; diff --git a/src/lxc/lxccontainer.h b/src/lxc/lxccontainer.h index 9ea67df7f..5938fa3d1 100644 --- a/src/lxc/lxccontainer.h +++ b/src/lxc/lxccontainer.h @@ -846,6 +846,17 @@ struct lxc_container { * \return \c 0 on success, nonzero on failure. */ int (*console_log)(struct lxc_container *c, struct lxc_console_log *log); + + /*! + * \brief Request the container reboot by sending it \c SIGINT. + * + * \param c Container. + * \param timeout Seconds to wait before returning false. + * (-1 to wait forever, 0 to avoid waiting). + * + * \return \c true if the container was rebooted successfully, else \c false. + */ + bool (*reboot2)(struct lxc_container *c, int timeout); }; /*! diff --git a/src/lxc/start.c b/src/lxc/start.c index a7bb6a665..ce13a3c50 100644 --- a/src/lxc/start.c +++ b/src/lxc/start.c @@ -200,6 +200,9 @@ restart: fddir = dirfd(dir); while ((direntp = readdir(dir))) { + struct lxc_list *cur; + bool matched = false; + if (!direntp) break; @@ -222,6 +225,20 @@ restart: (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,14 +355,14 @@ 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}; handler->state = state; TRACE("Set container state to %s", lxc_state2str(state)); process_lock(); - 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); @@ -355,10 +372,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; @@ -527,7 +544,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; @@ -550,10 +568,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); @@ -711,9 +731,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); @@ -735,8 +757,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); @@ -797,9 +824,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 53795a31e..9d2ba6ed1 100644 --- a/src/lxc/tools/lxc_stop.c +++ b/src/lxc/tools/lxc_stop.c @@ -94,62 +94,6 @@ Options :\n\ .timeout = -2, }; -/* 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; -} - int main(int argc, char *argv[]) { struct lxc_container *c; @@ -259,7 +203,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 = c->reboot2(c, my_args.timeout); + if (ret < 0) + ret = EXIT_FAILURE; + else + ret = EXIT_SUCCESS; goto out; } From c02c49ee3d01dec5c0cd558fe56725bb1a7df83d Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Tue, 21 Nov 2017 21:48:30 +0100 Subject: [PATCH 07/15] test: add test for reboot2() API extension Signed-off-by: Christian Brauner --- src/tests/Makefile.am | 4 +- src/tests/api_reboot.c | 125 +++++++++++++++++++++++++++++++++++++++++ src/tests/livepatch.c | 5 +- 3 files changed, 129 insertions(+), 5 deletions(-) create mode 100644 src/tests/api_reboot.c diff --git a/src/tests/Makefile.am b/src/tests/Makefile.am index c9a2dd275..9a0a09ea3 100644 --- a/src/tests/Makefile.am +++ b/src/tests/Makefile.am @@ -20,6 +20,7 @@ lxc_test_snapshot_SOURCES = snapshot.c lxc_test_concurrent_SOURCES = concurrent.c lxc_test_may_control_SOURCES = may_control.c lxc_test_reboot_SOURCES = reboot.c +lxc_test_api_reboot_SOURCES = api_reboot.c lxc_test_list_SOURCES = list.c lxc_test_attach_SOURCES = attach.c lxc_test_device_add_remove_SOURCES = device_add_remove.c @@ -57,7 +58,8 @@ 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-livepatch \ + lxc-test-api-reboot bin_SCRIPTS = lxc-test-automount \ lxc-test-autostart \ diff --git a/src/tests/api_reboot.c b/src/tests/api_reboot.c new file mode 100644 index 000000000..d609a3339 --- /dev/null +++ b/src/tests/api_reboot.c @@ -0,0 +1,125 @@ +/* liblxcapi + * + * Copyright © 2017 Christian Brauner . + * + * 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 +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "lxc/lxccontainer.h" +#include "lxctest.h" + +int main(int argc, char *argv[]) +{ + int i; + struct lxc_container *c; + int ret = EXIT_FAILURE; + + /* 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++) { + /* Give the init system some time to setup it's signal handlers + * otherwise we will hang indefinitely. + */ + sleep(5); + + if (!c->reboot2(c, -1)) { + lxc_error("%s\n", "Failed to reboot 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"); + } + + /* Give the init system some time to setup it's signal handlers + * otherwise we will hang indefinitely. + */ + sleep(5); + + /* Test non-blocking reboot2() */ + if (!c->reboot2(c, 0)) { + lxc_error("%s\n", "Failed to request non-blocking reboot of container \"reboot\""); + goto on_error_stop; + } + lxc_debug("%s\n", "Non-blocking reboot of container \"reboot\" succeeded"); + + ret = EXIT_SUCCESS; + +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); + if (ret == EXIT_SUCCESS) + lxc_debug("%s\n", "All reboot tests passed"); + exit(ret); +} diff --git a/src/tests/livepatch.c b/src/tests/livepatch.c index 7189cb36c..fb86757d9 100644 --- a/src/tests/livepatch.c +++ b/src/tests/livepatch.c @@ -203,14 +203,11 @@ int main(int argc, char *argv[]) } free(value); - if (!c->reboot(c)) { + if (!c->reboot2(c, -1)) { lxc_error("%s", "Failed to create container \"livepatch\""); 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; From fc788340f7b336aba2b8cf6a06ba5ff0a8a43b67 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 22 Nov 2017 21:32:07 +0100 Subject: [PATCH 08/15] test: add state server tests This checks whether multiple concurrent waiters all get notified by the state server. Signed-off-by: Christian Brauner --- src/tests/Makefile.am | 6 +- src/tests/state_server.c | 153 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 2 deletions(-) create mode 100644 src/tests/state_server.c diff --git a/src/tests/Makefile.am b/src/tests/Makefile.am index 9a0a09ea3..f223463d7 100644 --- a/src/tests/Makefile.am +++ b/src/tests/Makefile.am @@ -30,6 +30,7 @@ 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 AM_CFLAGS=-DLXCROOTFSMOUNT=\"$(LXCROOTFSMOUNT)\" \ -DLXCPATH=\"$(LXCPATH)\" \ @@ -59,7 +60,7 @@ bin_PROGRAMS = lxc-test-containertests lxc-test-locktests lxc-test-startone \ 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-api-reboot + lxc-test-api-reboot lxc-test-state-server bin_SCRIPTS = lxc-test-automount \ lxc-test-autostart \ @@ -119,7 +120,8 @@ EXTRA_DIST = \ shortlived.c \ shutdowntest.c \ snapshot.c \ - startone.c + startone.c \ + state_server.c clean-local: rm -f lxc-test-utils-* diff --git a/src/tests/state_server.c b/src/tests/state_server.c new file mode 100644 index 000000000..c59da12d6 --- /dev/null +++ b/src/tests/state_server.c @@ -0,0 +1,153 @@ +/* liblxcapi + * + * Copyright © 2017 Christian Brauner . + * + * 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 +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "lxc/lxccontainer.h" +#include "lxctest.h" + +struct thread_args { + int thread_id; + int timeout; + bool success; + struct lxc_container *c; +}; + +void *state_wrapper(void *data) +{ + struct thread_args *args = data; + + lxc_debug("Starting state server thread %d\n", args->thread_id); + + args->success = args->c->shutdown(args->c, args->timeout); + + lxc_debug("State server thread %d with shutdown timeout %d returned \"%s\"\n", + args->thread_id, args->timeout, args->success ? "SUCCESS" : "FAILED"); + + pthread_exit(NULL); + return NULL; +} + +int main(int argc, char *argv[]) +{ + int i, j; + pthread_attr_t attr; + pthread_t threads[10]; + struct thread_args args[10]; + struct lxc_container *c; + int ret = EXIT_FAILURE; + + c = lxc_container_new("state-server", NULL); + if (!c) { + lxc_error("%s", "Failed to create container \"state-server\""); + exit(ret); + } + + if (c->is_defined(c)) { + lxc_error("%s\n", "Container \"state-server\" is defined"); + goto on_error_put; + } + + if (!c->createl(c, "busybox", NULL, NULL, 0, NULL)) { + lxc_error("%s\n", "Failed to create busybox container \"state-server\""); + goto on_error_put; + } + + if (!c->is_defined(c)) { + lxc_error("%s\n", "Container \"state-server\" 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 \"state-server\""); + goto on_error_stop; + } + + if (!c->want_daemonize(c, true)) { + lxc_error("%s\n", "Failed to mark container \"state-server\" daemonized"); + goto on_error_stop; + } + + pthread_attr_init(&attr); + + for (j = 0; j < 10; j++) { + lxc_debug("Starting state server test iteration %d\n", j); + + if (!c->startl(c, 0, NULL)) { + lxc_error("%s\n", "Failed to start container \"state-server\" daemonized"); + goto on_error_stop; + } + + sleep(5); + + for (i = 0; i < 10; i++) { + int ret; + + args[i].thread_id = i; + args[i].c = c; + args[i].timeout = -1; + /* test non-blocking shutdown request */ + if (i == 0) + args[i].timeout = 0; + + ret = pthread_create(&threads[i], &attr, state_wrapper, (void *) &args[i]); + if (ret != 0) + goto on_error_stop; + } + + for (i = 0; i < 10; i++) { + int ret; + + ret = pthread_join(threads[i], NULL); + if (ret != 0) + goto on_error_stop; + + if (!args[i].success) { + lxc_error("State server thread %d failed\n", args[i].thread_id); + goto on_error_stop; + } + } + } + + ret = EXIT_SUCCESS; + +on_error_stop: + if (c->is_running(c) && !c->stop(c)) + lxc_error("%s\n", "Failed to stop container \"state-server\""); + + if (!c->destroy(c)) + lxc_error("%s\n", "Failed to destroy container \"state-server\""); + +on_error_put: + lxc_container_put(c); + if (ret == EXIT_SUCCESS) + lxc_debug("%s\n", "All state server tests passed"); + exit(ret); +} From bc631984fcc1c8b69cafbcce448f98961bca1e55 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Fri, 24 Nov 2017 12:51:04 +0100 Subject: [PATCH 09/15] commands: tweak locking Take the lock on the list after we've done all necessary work and check state. If we are in requested state, do cleanup and return without adding the state client to the state client list. Signed-off-by: Christian Brauner --- src/lxc/commands.c | 31 ++++++++++++++----------------- src/lxc/commands_utils.c | 17 ++++++++++++----- src/lxc/start.c | 2 +- 3 files changed, 27 insertions(+), 23 deletions(-) diff --git a/src/lxc/commands.c b/src/lxc/commands.c index d7e41e5a7..d95530ec7 100644 --- a/src/lxc/commands.c +++ b/src/lxc/commands.c @@ -893,9 +893,8 @@ int lxc_cmd_add_state_client(const char *name, const char *lxcpath, lxc_state_t states[MAX_STATE], int *state_client_fd) { - int stopped; + int state, stopped; ssize_t ret; - int state = -1; struct lxc_cmd_rr cmd = { .req = { .cmd = LXC_CMD_ADD_STATE_CLIENT, @@ -904,17 +903,10 @@ int lxc_cmd_add_state_client(const char *name, const char *lxcpath, }, }; - /* Check if already in requested state. */ - state = lxc_getstate(name, lxcpath); - if (state < 0) { - TRACE("%s - Failed to retrieve state of container", strerror(errno)); - return -1; - } else if (states[state] == 1) { - TRACE("Container is in %s state", lxc_state2str(state)); - return state; - } - ret = lxc_cmd(name, &cmd, &stopped, lxcpath, NULL); + if (states[STOPPED] != 0 && stopped != 0) + return STOPPED; + if (ret < 0) { ERROR("%s - Failed to execute command", strerror(errno)); return -1; @@ -924,11 +916,19 @@ int lxc_cmd_add_state_client(const char *name, const char *lxcpath, * function. */ if (cmd.rsp.ret < 0) { - ERROR("Failed to receive socket fd"); + ERROR("%s - Failed to receive socket fd", strerror(-cmd.rsp.ret)); return -1; } + state = PTR_TO_INT(cmd.rsp.data); + if (state < MAX_STATE) { + TRACE("Container is already in requested state %s", + lxc_state2str(state)); + return state; + } + *state_client_fd = cmd.rsp.ret; + TRACE("Added state client %d to state client list", cmd.rsp.ret); return MAX_STATE; } @@ -947,10 +947,7 @@ static int lxc_cmd_add_state_client_callback(int fd, struct lxc_cmd_req *req, return -1; rsp.ret = lxc_add_state_client(fd, handler, (lxc_state_t *)req->data); - if (rsp.ret < 0) - ERROR("Failed to add state client %d to state client list", fd); - else - TRACE("Added state client %d to state client list", fd); + rsp.data = INT_TO_PTR(rsp.ret); return lxc_cmd_rsp_send(fd, &rsp); } diff --git a/src/lxc/commands_utils.c b/src/lxc/commands_utils.c index 394f0a1d7..32831e2b6 100644 --- a/src/lxc/commands_utils.c +++ b/src/lxc/commands_utils.c @@ -193,6 +193,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]) { + int state; struct lxc_state_client *newclient; struct lxc_list *tmplist; @@ -211,11 +212,17 @@ int lxc_add_state_client(int state_client_fd, struct lxc_handler *handler, } process_lock(); - lxc_list_add_elem(tmplist, newclient); - lxc_list_add_tail(&handler->conf->state_clients, tmplist); - process_unlock(); + state = handler->state; + if (states[state] != 1) { + lxc_list_add_elem(tmplist, newclient); + lxc_list_add_tail(&handler->conf->state_clients, tmplist); + process_unlock(); + } else { + process_unlock(); + free(newclient); + return state; + } TRACE("added state client %d to state client list", state_client_fd); - - return 0; + return MAX_STATE; } diff --git a/src/lxc/start.c b/src/lxc/start.c index ce13a3c50..72e3f0146 100644 --- a/src/lxc/start.c +++ b/src/lxc/start.c @@ -358,10 +358,10 @@ static int lxc_serve_state_clients(const char *name, struct lxc_state_client *client; struct lxc_msg msg = {.type = lxc_msg_state, .value = state}; + process_lock(); handler->state = state; TRACE("Set container state to %s", lxc_state2str(state)); - process_lock(); if (lxc_list_empty(&handler->conf->state_clients)) { TRACE("No state clients registered"); process_unlock(); From f8bdb6dcc4d85d1b25a5926917cd6f882a9170e0 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Fri, 24 Nov 2017 23:19:34 +0100 Subject: [PATCH 10/15] lxccontainer: restore non-blocking shutdown If timeout is set to 0 don't block. Signed-off-by: Christian Brauner --- src/lxc/lxccontainer.c | 68 +++++++++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 28 deletions(-) diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c index 46ca47f0f..d0f9b341f 100644 --- a/src/lxc/lxccontainer.c +++ b/src/lxc/lxccontainer.c @@ -1862,10 +1862,9 @@ WRAP_API_1(bool, lxcapi_reboot2, int) static bool do_lxcapi_shutdown(struct lxc_container *c, int timeout) { - int ret, state_client_fd = -1; - bool retv = false; + int killret, ret; pid_t pid; - int haltsignal = SIGPWR; + int haltsignal = SIGPWR, state_client_fd = -1; lxc_state_t states[MAX_STATE] = {0}; if (!c) @@ -1873,6 +1872,7 @@ static bool do_lxcapi_shutdown(struct lxc_container *c, int timeout) if (!do_lxcapi_is_running(c)) return true; + pid = do_lxcapi_init_pid(c); if (pid <= 0) return true; @@ -1884,37 +1884,49 @@ static bool do_lxcapi_shutdown(struct lxc_container *c, int timeout) if (c->lxc_conf && c->lxc_conf->haltsignal) haltsignal = c->lxc_conf->haltsignal; - INFO("Using signal number '%d' as halt signal", haltsignal); - /* Add a new state client before sending the shutdown signal so that we * don't miss a state. */ - states[STOPPED] = 1; - ret = lxc_cmd_add_state_client(c->name, c->config_path, states, - &state_client_fd); - - /* Send shutdown signal to container. */ - if (kill(pid, haltsignal) < 0) - WARN("Could not send signal %d to pid %d", haltsignal, pid); - - /* Retrieve the state. */ - if (state_client_fd >= 0) { - int state; - state = lxc_cmd_sock_rcv_state(state_client_fd, timeout); - close(state_client_fd); - TRACE("Received state \"%s\"", lxc_state2str(state)); - if (state != STOPPED) + if (timeout != 0) { + states[STOPPED] = 1; + ret = lxc_cmd_add_state_client(c->name, c->config_path, states, + &state_client_fd); + if (ret < 0) + return false; + + if (state_client_fd < 0) + return false; + + if (ret == STOPPED) + return true; + + if (ret < MAX_STATE) return false; - retv = true; - } else if (ret == STOPPED) { - TRACE("Container is already stopped"); - retv = true; - } else { - TRACE("Received state \"%s\" instead of expected \"STOPPED\"", - lxc_state2str(ret)); } - return retv; + /* Send shutdown signal to container. */ + killret = kill(pid, haltsignal); + if (killret < 0) { + WARN("Could not send signal %d to pid %d", haltsignal, pid); + if (state_client_fd >= 0) + close(state_client_fd); + return false; + } + TRACE("Sent signal %d to pid %d", haltsignal, pid); + + if (timeout == 0) + return true; + + ret = lxc_cmd_sock_rcv_state(state_client_fd, timeout); + close(state_client_fd); + if (ret < 0) + return false; + + TRACE("Received state \"%s\"", lxc_state2str(ret)); + if (ret != STOPPED) + return false; + + return true; } WRAP_API_1(bool, lxcapi_shutdown, int) From 44552fb2b794573334136209dc44a4270f3acf45 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Sat, 25 Nov 2017 17:51:01 +0100 Subject: [PATCH 11/15] commands: tell mainloop to reap client fd on error This is the proper way to handle errors. Signed-off-by: Christian Brauner --- src/lxc/commands.c | 26 ++++++++++++++++++++------ src/lxc/commands_utils.c | 1 + 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/lxc/commands.c b/src/lxc/commands.c index d95530ec7..0a19f5f3d 100644 --- a/src/lxc/commands.c +++ b/src/lxc/commands.c @@ -922,8 +922,8 @@ int lxc_cmd_add_state_client(const char *name, const char *lxcpath, state = PTR_TO_INT(cmd.rsp.data); if (state < MAX_STATE) { - TRACE("Container is already in requested state %s", - lxc_state2str(state)); + TRACE("Container is already in requested state %s", lxc_state2str(state)); + close(cmd.rsp.ret); return state; } @@ -935,21 +935,35 @@ int lxc_cmd_add_state_client(const char *name, const char *lxcpath, static int lxc_cmd_add_state_client_callback(int fd, struct lxc_cmd_req *req, struct lxc_handler *handler) { + int ret; struct lxc_cmd_rsp rsp = {0}; if (req->datalen < 0) - return -1; + goto reap_client_fd; if (req->datalen > (sizeof(lxc_state_t) * MAX_STATE)) - return -1; + goto reap_client_fd; if (!req->data) - return -1; + goto reap_client_fd; rsp.ret = lxc_add_state_client(fd, handler, (lxc_state_t *)req->data); + if (rsp.ret < 0) + goto reap_client_fd; + rsp.data = INT_TO_PTR(rsp.ret); - return lxc_cmd_rsp_send(fd, &rsp); + ret = lxc_cmd_rsp_send(fd, &rsp); + if (ret < 0) + goto reap_client_fd; + + return 0; + +reap_client_fd: + /* Special indicator to lxc_cmd_handler() to close the fd and do related + * cleanup. + */ + return 1; } int lxc_cmd_console_log(const char *name, const char *lxcpath, diff --git a/src/lxc/commands_utils.c b/src/lxc/commands_utils.c index 32831e2b6..12b131707 100644 --- a/src/lxc/commands_utils.c +++ b/src/lxc/commands_utils.c @@ -220,6 +220,7 @@ int lxc_add_state_client(int state_client_fd, struct lxc_handler *handler, } else { process_unlock(); free(newclient); + free(tmplist); return state; } From 6b7f85cbcdce8bd5d6b1c4c719ae776d6b3ffb06 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 6 Dec 2017 15:33:23 +0100 Subject: [PATCH 12/15] commands: return -ECONNRESET to caller Callers can then make a decision whether they want to consider the peer closing the connection an error or not. For example, a c->wait(c, "STOPPED", -1) call can then consider a ECONNRESET not an error but rather see it - correctly - as a container exiting before being able to register a state client. Signed-off-by: Christian Brauner --- src/lxc/commands.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/lxc/commands.c b/src/lxc/commands.c index 0a19f5f3d..d99f06780 100644 --- a/src/lxc/commands.c +++ b/src/lxc/commands.c @@ -128,6 +128,9 @@ static int lxc_cmd_rsp_recv(int sock, struct lxc_cmd_rr *cmd) if (ret < 0) { WARN("%s - Failed to receive response for command \"%s\"", strerror(errno), lxc_cmd_str(cmd->req.cmd)); + if (errno == ECONNRESET) + return -ECONNRESET; + return -1; } TRACE("Command \"%s\" received response", lxc_cmd_str(cmd->req.cmd)); @@ -318,6 +321,8 @@ static int lxc_cmd(const char *name, struct lxc_cmd_rr *cmd, int *stopped, } ret = lxc_cmd_rsp_recv(client_fd, cmd); + if (ret == -ECONNRESET) + *stopped = 1; out: if (!stay_connected || ret <= 0) close(client_fd); From 8f98ac7b0f3f25e42f6de50c4d43ec74bcb90c8d Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 6 Dec 2017 15:37:12 +0100 Subject: [PATCH 13/15] execute: pass logfile to lxc-init Signed-off-by: Christian Brauner --- src/lxc/execute.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/lxc/execute.c b/src/lxc/execute.c index 1b142aeca..40856a337 100644 --- a/src/lxc/execute.c +++ b/src/lxc/execute.c @@ -77,6 +77,11 @@ static int execute_start(struct lxc_handler *handler, void* data) argv[i++] = (char *)lxc_log_priority_to_string(lxc_log_get_level()); } + if (handler->conf->logfile) { + argv[i++] = "-o"; + argv[i++] = (char *)handler->conf->logfile; + } + if (my_args->quiet) argv[i++] = "--quiet"; From fa30091bb5bef1c1267357d876e5de51f3d990f5 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 6 Dec 2017 15:37:40 +0100 Subject: [PATCH 14/15] lxccontainer: handle execute containers correctly It doesn't make sense to error out when an app container doesn't pass explicit arguments through c->start{l}(). This is especially true since we implemented lxc.execute.cmd. However, even before we could have always relied on lxc.init.cmd and errored out after that. Signed-off-by: Christian Brauner --- src/lxc/lxccontainer.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c index d0f9b341f..59e11b80a 100644 --- a/src/lxc/lxccontainer.c +++ b/src/lxc/lxccontainer.c @@ -828,10 +828,6 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a return false; } - /* Is this app meant to be run through lxcinit, as in lxc-execute? */ - if (useinit && !argv) - return false; - if (container_mem_lock(c)) return false; conf = c->lxc_conf; @@ -843,15 +839,20 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a if (!handler) return false; - /* If no argv was passed in, use lxc.init_cmd if provided in the - * configuration - */ - if (!argv) - argv = init_cmd = split_init_cmd(conf->init_cmd); + if (!argv) { + if (useinit && conf->execute_cmd) + argv = init_cmd = split_init_cmd(conf->execute_cmd); + else + argv = init_cmd = split_init_cmd(conf->init_cmd); + } /* ... otherwise use default_args. */ - if (!argv) - argv = default_args; + if (!argv) { + if (useinit) + return false; + else + argv = default_args; + } /* I'm not sure what locks we want here.Any? Is liblxc's locking enough * here to protect the on disk container? We don't want to exclude From 77f76f316a9478e64654fe11ca233d485b1abe9f Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 6 Dec 2017 15:56:35 +0100 Subject: [PATCH 15/15] tests: expand tests for shortlived init processes This adds additional test for app containers. Signed-off-by: Christian Brauner --- src/tests/shortlived.c | 119 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 108 insertions(+), 11 deletions(-) diff --git a/src/tests/shortlived.c b/src/tests/shortlived.c index 6472ce7e0..b5397e7eb 100644 --- a/src/tests/shortlived.c +++ b/src/tests/shortlived.c @@ -28,6 +28,9 @@ #include #include +#include "lxctest.h" +#include "utils.h" + #define MYNAME "shortlived" static int destroy_container(void) @@ -92,12 +95,33 @@ again: int main(int argc, char *argv[]) { - struct lxc_container *c; + int i; const char *s; bool b; - int i; + struct lxc_container *c; + struct lxc_log log; + char template[sizeof(P_tmpdir"/shortlived_XXXXXX")]; int ret = 0; + strcpy(template, P_tmpdir"/shortlived_XXXXXX"); + i = lxc_make_tmpfile(template, false); + if (i < 0) { + lxc_error("Failed to create temporary log file for container %s\n", MYNAME); + exit(EXIT_FAILURE); + } else { + lxc_debug("Using \"%s\" as temporary log file for container %s\n", template, MYNAME); + close(i); + } + + log.name = MYNAME; + log.file = template; + log.level = "TRACE"; + log.prefix = "shortlived"; + log.quiet = false; + log.lxcpath = NULL; + if (lxc_log_init(&log)) + exit(EXIT_FAILURE); + ret = 1; /* test a real container */ c = lxc_container_new(MYNAME, NULL); @@ -141,16 +165,51 @@ int main(int argc, char *argv[]) goto out; } + if (!c->set_config_item(c, "lxc.execute.cmd", "echo hello")) { + fprintf(stderr, "%d: failed setting lxc.init.cmd\n", __LINE__); + goto out; + } + c->want_daemonize(c, true); - /* Test whether we can start a really short-lived daemonized container. - */ + /* Test whether we can start a really short-lived daemonized container. */ for (i = 0; i < 10; i++) { if (!c->startl(c, 0, NULL)) { - fprintf(stderr, "%d: %s failed to start\n", __LINE__, c->name); + fprintf(stderr, "%d: %s failed to start on %dth iteration\n", __LINE__, c->name, i); + goto out; + } + + /* The container needs to exit with a successful error code. */ + if (c->error_num != 0) { + fprintf(stderr, "%d: %s exited successfully on %dth iteration\n", __LINE__, c->name, i); + goto out; + } + fprintf(stderr, "%d: %s exited correctly with error code %d on %dth iteration\n", __LINE__, c->name, c->error_num, i); + + if (!c->wait(c, "STOPPED", 30)) { + fprintf(stderr, "%d: %s failed to wait on %dth iteration\n", __LINE__, c->name, i); + goto out; + } + } + + /* Test whether we can start a really short-lived daemonized container with lxc-init. */ + for (i = 0; i < 10; i++) { + if (!c->startl(c, 1, NULL)) { + fprintf(stderr, "%d: %s failed to start on %dth iteration\n", __LINE__, c->name, i); + goto out; + } + + /* The container needs to exit with a successful error code. */ + if (c->error_num != 0) { + fprintf(stderr, "%d: %s exited successfully on %dth iteration\n", __LINE__, c->name, i); + goto out; + } + fprintf(stderr, "%d: %s exited correctly with error code %d on %dth iteration\n", __LINE__, c->name, c->error_num, i); + + if (!c->wait(c, "STOPPED", 30)) { + fprintf(stderr, "%d: %s failed to wait on %dth iteration\n", __LINE__, c->name, i); goto out; } - sleep(1); } if (!c->set_config_item(c, "lxc.init.cmd", "you-shall-fail")) { @@ -158,15 +217,52 @@ int main(int argc, char *argv[]) goto out; } - /* Test whether we catch the start failure of a really short-lived - * daemonized container. - */ + if (!c->set_config_item(c, "lxc.execute.cmd", "you-shall-fail")) { + fprintf(stderr, "%d: failed setting lxc.init.cmd\n", __LINE__); + goto out; + } + + /* Test whether we can start a really short-lived daemonized container. */ for (i = 0; i < 10; i++) { if (c->startl(c, 0, NULL)) { - fprintf(stderr, "%d: %s failed to start\n", __LINE__, c->name); + fprintf(stderr, "%d: %s failed to start on %dth iteration\n", __LINE__, c->name, i); + goto out; + } + + /* The container needs to exit with an error code.*/ + if (c->error_num == 0) { + fprintf(stderr, "%d: %s exited successfully on %dth iteration\n", __LINE__, c->name, i); + goto out; + } + fprintf(stderr, "%d: %s exited correctly with error code %d on %dth iteration\n", __LINE__, c->name, c->error_num, i); + + if (!c->wait(c, "STOPPED", 30)) { + fprintf(stderr, "%d: %s failed to wait on %dth iteration\n", __LINE__, c->name, i); + goto out; + } + } + + /* Test whether we can start a really short-lived daemonized container with lxc-init. */ + for (i = 0; i < 10; i++) { + /* An container started with lxc-init will always start + * succesfully unless lxc-init has a bug. + */ + if (!c->startl(c, 1, NULL)) { + fprintf(stderr, "%d: %s failed to start on %dth iteration\n", __LINE__, c->name, i); + goto out; + } + + /* The container needs to exit with an error code.*/ + if (c->error_num == 0) { + fprintf(stderr, "%d: %s exited successfully on %dth iteration\n", __LINE__, c->name, i); + goto out; + } + fprintf(stderr, "%d: %s exited correctly with error code %d on %dth iteration\n", __LINE__, c->name, c->error_num, i); + + if (!c->wait(c, "STOPPED", 30)) { + fprintf(stderr, "%d: %s failed to wait on %dth iteration\n", __LINE__, c->name, i); goto out; } - sleep(1); } c->stop(c); @@ -180,5 +276,6 @@ out: destroy_container(); } lxc_container_put(c); + unlink(template); exit(ret); }