From: Christian Brauner Date: Tue, 3 Dec 2019 01:23:34 +0000 (+0100) Subject: cgroups/freezer: fix and improve cgroup2 freezer implementation X-Git-Tag: lxc-4.0.0~88^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=018051e37df71276980c6238eee44632a53f37e3;p=thirdparty%2Flxc.git cgroups/freezer: fix and improve cgroup2 freezer implementation Signed-off-by: Christian Brauner --- diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c index e3f435d1f..12cd4b923 100644 --- a/src/lxc/cgroups/cgfsng.c +++ b/src/lxc/cgroups/cgfsng.c @@ -61,6 +61,7 @@ #include "config.h" #include "log.h" #include "macro.h" +#include "mainloop.h" #include "memory_utils.h" #include "storage/storage.h" #include "utils.h" @@ -1989,27 +1990,6 @@ __cgfsng_ops static bool cgfsng_get_hierarchies(struct cgroup_ops *ops, int n, c return true; } -static bool poll_file_ready(int lfd) -{ - int ret; - struct pollfd pfd = { - .fd = lfd, - .events = POLLIN, - .revents = 0, - }; - -again: - ret = poll(&pfd, 1, 60000); - if (ret < 0) { - if (errno == EINTR) - goto again; - - SYSERROR("Failed to poll() on file descriptor"); - return false; - } - - return (pfd.revents & POLLIN); -} static bool cg_legacy_freeze(struct cgroup_ops *ops) { @@ -2018,101 +1998,173 @@ static bool cg_legacy_freeze(struct cgroup_ops *ops) h = get_hierarchy(ops, "freezer"); if (!h) - return false; + return minus_one_set_errno(ENOENT); path = must_make_path(h->container_full_path, "freezer.state", NULL); - return lxc_write_to_file(path, "FROZEN", STRLITERALLEN("FROZEN"), false, - 0666) == 0; + return lxc_write_to_file(path, "FROZEN", STRLITERALLEN("FROZEN"), false, 0666); } -static bool cg_unified_freeze(struct cgroup_ops *ops) +static int freezer_cgroup_events_cb(int fd, uint32_t events, void *cbdata, + struct lxc_epoll_descr *descr) { - int ret; - __do_close_prot_errno int fd = -EBADF; - __do_free char *events_file = NULL, *path = NULL, *line = NULL; + __do_close_prot_errno int duped_fd = -EBADF; + __do_free char *line = NULL; __do_fclose FILE *f = NULL; + int state = PTR_TO_INT(cbdata); + size_t len; + const char *state_string; + + duped_fd = dup(fd); + if (duped_fd < 0) + return LXC_MAINLOOP_ERROR; + + if (lseek(duped_fd, 0, SEEK_SET) < (off_t)-1) + return LXC_MAINLOOP_ERROR; + + f = fdopen(duped_fd, "re"); + if (!f) + return LXC_MAINLOOP_ERROR; + move_fd(duped_fd); + + if (state == 1) + state_string = "frozen 1"; + else + state_string = "frozen 0"; + + while (getline(&line, &len, f) != -1) + if (strncmp(line, state_string, STRLITERALLEN("frozen") + 2) == 0) + return LXC_MAINLOOP_CLOSE; + + return LXC_MAINLOOP_CONTINUE; +} + +static int cg_unified_freeze(struct cgroup_ops *ops, int timeout) +{ + __do_close_prot_errno int fd = -EBADF; + __do_free char *path = NULL; + __do_lxc_mainloop_close struct lxc_epoll_descr *descr_ptr = NULL; + int ret; + struct lxc_epoll_descr descr; struct hierarchy *h; h = ops->unified; if (!h) - return false; + return minus_one_set_errno(ENOENT); - path = must_make_path(h->container_full_path, "cgroup.freeze", NULL); - ret = lxc_write_to_file(path, "1", 1, false, 0666); - if (ret < 0) - return false; + if (!h->container_full_path) + return minus_one_set_errno(EEXIST); - events_file = must_make_path(h->container_full_path, "cgroup.events", NULL); - fd = open(events_file, O_RDONLY | O_CLOEXEC); - if (fd < 0) - return false; + if (timeout != 0) { + __do_free char *events_file = NULL; - f = fdopen(fd, "re"); - if (!f) - return false; - move_fd(fd); + events_file = must_make_path(h->container_full_path, "cgroup.events", NULL); + fd = open(events_file, O_RDONLY | O_CLOEXEC); + if (fd < 0) + return error_log_errno(errno, "Failed to open cgroup.events file"); - for (int i = 0; i < 10 && poll_file_ready(fd); i++) { - size_t len; + ret = lxc_mainloop_open(&descr); + if (ret) + return error_log_errno(errno, "Failed to create epoll instance to wait for container freeze"); - while (getline(&line, &len, f) != -1) { - if (strcmp(line, "frozen 1") == 0) - return true; - } + /* automatically cleaned up now */ + descr_ptr = &descr; - fseek(f, 0, SEEK_SET); - }; + ret = lxc_mainloop_add_handler(&descr, fd, freezer_cgroup_events_cb, INT_TO_PTR((int){1})); + if (ret < 0) + return error_log_errno(errno, "Failed to add cgroup.events fd handler to mainloop"); + } - return false; + path = must_make_path(h->container_full_path, "cgroup.freeze", NULL); + ret = lxc_write_to_file(path, "1", 1, false, 0666); + if (ret < 0) + return error_log_errno(errno, "Failed to open cgroup.freeze file"); + + if (timeout != 0 && lxc_mainloop(&descr, timeout)) + return error_log_errno(errno, "Failed to wait for container to be frozen"); + + return 0; } -__cgfsng_ops static bool cgfsng_freeze(struct cgroup_ops *ops) +__cgfsng_ops static int cgfsng_freeze(struct cgroup_ops *ops, int timeout) { if (!ops->hierarchies) - return true; + return minus_one_set_errno(ENOENT); if (ops->cgroup_layout != CGROUP_LAYOUT_UNIFIED) return cg_legacy_freeze(ops); - return cg_unified_freeze(ops); + return cg_unified_freeze(ops, timeout); } -static bool cg_legacy_unfreeze(struct cgroup_ops *ops) +static int cg_legacy_unfreeze(struct cgroup_ops *ops) { __do_free char *path = NULL; struct hierarchy *h; h = get_hierarchy(ops, "freezer"); if (!h) - return false; + return minus_one_set_errno(ENOENT); path = must_make_path(h->container_full_path, "freezer.state", NULL); - return lxc_write_to_file(path, "THAWED", STRLITERALLEN("THAWED"), false, - 0666) == 0; + return lxc_write_to_file(path, "THAWED", STRLITERALLEN("THAWED"), false, 0666); } -static bool cg_unified_unfreeze(struct cgroup_ops *ops) +static int cg_unified_unfreeze(struct cgroup_ops *ops, int timeout) { + __do_close_prot_errno int fd = -EBADF; __do_free char *path = NULL; + __do_lxc_mainloop_close struct lxc_epoll_descr *descr_ptr = NULL; + int ret; + struct lxc_epoll_descr descr; struct hierarchy *h; h = ops->unified; if (!h) - return false; + return minus_one_set_errno(ENOENT); + + if (!h->container_full_path) + return minus_one_set_errno(EEXIST); + + if (timeout != 0) { + __do_free char *events_file = NULL; + + events_file = must_make_path(h->container_full_path, "cgroup.events", NULL); + fd = open(events_file, O_RDONLY | O_CLOEXEC); + if (fd < 0) + return error_log_errno(errno, "Failed to open cgroup.events file"); + + ret = lxc_mainloop_open(&descr); + if (ret) + return error_log_errno(errno, "Failed to create epoll instance to wait for container unfreeze"); + + /* automatically cleaned up now */ + descr_ptr = &descr; + + ret = lxc_mainloop_add_handler(&descr, fd, freezer_cgroup_events_cb, INT_TO_PTR((int){0})); + if (ret < 0) + return error_log_errno(errno, "Failed to add cgroup.events fd handler to mainloop"); + } path = must_make_path(h->container_full_path, "cgroup.freeze", NULL); - return lxc_write_to_file(path, "0", 1, false, 0666) == 0; + ret = lxc_write_to_file(path, "0", 1, false, 0666); + if (ret < 0) + return error_log_errno(errno, "Failed to open cgroup.freeze file"); + + if (timeout != 0 && lxc_mainloop(&descr, timeout)) + return error_log_errno(errno, "Failed to wait for container to be unfrozen"); + + return 0; } -__cgfsng_ops static bool cgfsng_unfreeze(struct cgroup_ops *ops) +__cgfsng_ops static int cgfsng_unfreeze(struct cgroup_ops *ops, int timeout) { if (!ops->hierarchies) - return true; + return minus_one_set_errno(ENOENT); if (ops->cgroup_layout != CGROUP_LAYOUT_UNIFIED) return cg_legacy_unfreeze(ops); - return cg_unified_unfreeze(ops); + return cg_unified_unfreeze(ops, timeout); } __cgfsng_ops static const char *cgfsng_get_cgroup(struct cgroup_ops *ops, diff --git a/src/lxc/cgroups/cgroup.h b/src/lxc/cgroups/cgroup.h index 6c875b2a6..3c62afefc 100644 --- a/src/lxc/cgroups/cgroup.h +++ b/src/lxc/cgroups/cgroup.h @@ -165,8 +165,8 @@ struct cgroup_ops { const char *value, const char *name, const char *lxcpath); int (*get)(struct cgroup_ops *ops, const char *filename, char *value, size_t len, const char *name, const char *lxcpath); - bool (*freeze)(struct cgroup_ops *ops); - bool (*unfreeze)(struct cgroup_ops *ops); + int (*freeze)(struct cgroup_ops *ops, int timeout); + int (*unfreeze)(struct cgroup_ops *ops, int timeout); bool (*setup_limits)(struct cgroup_ops *ops, struct lxc_conf *conf, bool with_devices); bool (*chown)(struct cgroup_ops *ops, struct lxc_conf *conf); diff --git a/src/lxc/commands.c b/src/lxc/commands.c index 1605fe5e2..fb40039d3 100644 --- a/src/lxc/commands.c +++ b/src/lxc/commands.c @@ -101,6 +101,8 @@ static const char *lxc_cmd_str(lxc_cmd_t cmd) [LXC_CMD_SERVE_STATE_CLIENTS] = "serve_state_clients", [LXC_CMD_SECCOMP_NOTIFY_ADD_LISTENER] = "seccomp_notify_add_listener", [LXC_CMD_ADD_BPF_DEVICE_CGROUP] = "add_bpf_device_cgroup", + [LXC_CMD_FREEZE] = "freeze", + [LXC_CMD_UNFREEZE] = "unfreeze", }; if (cmd >= LXC_CMD_MAX) @@ -650,19 +652,14 @@ static int lxc_cmd_stop_callback(int fd, struct lxc_cmd_req *req, memset(&rsp, 0, sizeof(rsp)); rsp.ret = kill(handler->pid, stopsignal); if (!rsp.ret) { - /* We can't just use lxc_unfreeze() since we are already in the - * context of handling the STOP cmd in lxc-start, and calling - * lxc_unfreeze() would do another cmd (GET_CGROUP) which would - * deadlock us. - */ - if (!cgroup_ops->get_cgroup(cgroup_ops, "freezer")) - return 0; - - if (cgroup_ops->unfreeze(cgroup_ops)) + rsp.ret = cgroup_ops->unfreeze(cgroup_ops, -1); + if (!rsp.ret) return 0; ERROR("Failed to unfreeze container \"%s\"", handler->name); - rsp.ret = -1; + rsp.ret = -errno; + } else { + rsp.ret = -errno; } return lxc_cmd_rsp_send(fd, &rsp); @@ -1228,6 +1225,72 @@ out: return lxc_cmd_rsp_send(fd, &rsp); } +int lxc_cmd_freeze(const char *name, const char *lxcpath, int timeout) +{ + int ret, stopped; + struct lxc_cmd_rr cmd = { + .req = { + .cmd = LXC_CMD_FREEZE, + .data = INT_TO_PTR(timeout), + }, + }; + + ret = lxc_cmd(name, &cmd, &stopped, lxcpath, NULL); + if (ret <= 0 || cmd.rsp.ret < 0) + return error_log_errno(errno, "Failed to freeze container"); + + return cmd.rsp.ret; +} + +static int lxc_cmd_freeze_callback(int fd, struct lxc_cmd_req *req, + struct lxc_handler *handler, + struct lxc_epoll_descr *descr) +{ + int timeout = PTR_TO_INT(req->data); + struct lxc_cmd_rsp rsp = { + .ret = -ENOENT, + }; + struct cgroup_ops *ops = handler->cgroup_ops; + + if (ops->cgroup_layout == CGROUP_LAYOUT_UNIFIED) + rsp.ret = ops->freeze(ops, timeout); + + return lxc_cmd_rsp_send(fd, &rsp); +} + +int lxc_cmd_unfreeze(const char *name, const char *lxcpath, int timeout) +{ + int ret, stopped; + struct lxc_cmd_rr cmd = { + .req = { + .cmd = LXC_CMD_UNFREEZE, + .data = INT_TO_PTR(timeout), + }, + }; + + ret = lxc_cmd(name, &cmd, &stopped, lxcpath, NULL); + if (ret <= 0 || cmd.rsp.ret < 0) + return error_log_errno(errno, "Failed to unfreeze container"); + + return cmd.rsp.ret; +} + +static int lxc_cmd_unfreeze_callback(int fd, struct lxc_cmd_req *req, + struct lxc_handler *handler, + struct lxc_epoll_descr *descr) +{ + int timeout = PTR_TO_INT(req->data); + struct lxc_cmd_rsp rsp = { + .ret = -ENOENT, + }; + struct cgroup_ops *ops = handler->cgroup_ops; + + if (ops->cgroup_layout == CGROUP_LAYOUT_UNIFIED) + rsp.ret = ops->unfreeze(ops, timeout); + + return lxc_cmd_rsp_send(fd, &rsp); +} + static int lxc_cmd_process(int fd, struct lxc_cmd_req *req, struct lxc_handler *handler, struct lxc_epoll_descr *descr) @@ -1251,6 +1314,8 @@ static int lxc_cmd_process(int fd, struct lxc_cmd_req *req, [LXC_CMD_SERVE_STATE_CLIENTS] = lxc_cmd_serve_state_clients_callback, [LXC_CMD_SECCOMP_NOTIFY_ADD_LISTENER] = lxc_cmd_seccomp_notify_add_listener_callback, [LXC_CMD_ADD_BPF_DEVICE_CGROUP] = lxc_cmd_add_bpf_device_cgroup_callback, + [LXC_CMD_FREEZE] = lxc_cmd_freeze_callback, + [LXC_CMD_UNFREEZE] = lxc_cmd_unfreeze_callback, }; if (req->cmd >= LXC_CMD_MAX) { diff --git a/src/lxc/commands.h b/src/lxc/commands.h index 008b7c30e..4346d86b5 100644 --- a/src/lxc/commands.h +++ b/src/lxc/commands.h @@ -48,6 +48,8 @@ typedef enum { LXC_CMD_SERVE_STATE_CLIENTS, LXC_CMD_SECCOMP_NOTIFY_ADD_LISTENER, LXC_CMD_ADD_BPF_DEVICE_CGROUP, + LXC_CMD_FREEZE, + LXC_CMD_UNFREEZE, LXC_CMD_MAX, } lxc_cmd_t; @@ -135,5 +137,7 @@ extern int lxc_cmd_seccomp_notify_add_listener(const char *name, struct device_item; extern int lxc_cmd_add_bpf_device_cgroup(const char *name, const char *lxcpath, struct device_item *device); +extern int lxc_cmd_freeze(const char *name, const char *lxcpath, int timeout); +extern int lxc_cmd_unfreeze(const char *name, const char *lxcpath, int timeout); #endif /* __commands_h */ diff --git a/src/lxc/freezer.c b/src/lxc/freezer.c index 1843f2a60..3f6aaa546 100644 --- a/src/lxc/freezer.c +++ b/src/lxc/freezer.c @@ -98,23 +98,40 @@ static int do_freeze_thaw(bool freeze, struct lxc_conf *conf, const char *name, } } - ret = cgroup_ops->freeze(cgroup_ops); + if (freeze) + ret = lxc_cmd_freeze(name, lxcpath, -1); + else + ret = lxc_cmd_unfreeze(name, lxcpath, -1); cgroup_exit(cgroup_ops); if (ret < 0) return error_log_errno(-1, "Failed to %s container", freeze ? "freeze" : "unfreeze"); - return 0; } +static void notify_state_listeners(const char *name, const char *lxcpath, + lxc_state_t state) +{ + lxc_cmd_serve_state_clients(name, lxcpath, state); + lxc_monitor_send_state(name, state, lxcpath); +} + int lxc_freeze(struct lxc_conf *conf, const char *name, const char *lxcpath) { - lxc_cmd_serve_state_clients(name, lxcpath, FREEZING); - lxc_monitor_send_state(name, FREEZING, lxcpath); - return do_freeze_thaw(true, conf, name, lxcpath); + int ret; + + notify_state_listeners(name, lxcpath, FREEZING); + ret = do_freeze_thaw(true, conf, name, lxcpath); + notify_state_listeners(name, lxcpath, !ret ? FROZEN : RUNNING); + return ret; } int lxc_unfreeze(struct lxc_conf *conf, const char *name, const char *lxcpath) { - return do_freeze_thaw(false, conf, name, lxcpath); + int ret; + + notify_state_listeners(name, lxcpath, THAWED); + ret = do_freeze_thaw(false, conf, name, lxcpath); + notify_state_listeners(name, lxcpath, !ret ? RUNNING : FROZEN); + return ret; } diff --git a/src/lxc/mainloop.c b/src/lxc/mainloop.c index b169aa6fe..6de74dd69 100644 --- a/src/lxc/mainloop.c +++ b/src/lxc/mainloop.c @@ -66,6 +66,8 @@ int lxc_mainloop(struct lxc_epoll_descr *descr, int timeout_ms) */ ret = handler->callback(handler->fd, events[i].events, handler->data, descr); + if (ret == LXC_MAINLOOP_ERROR) + return -1; if (ret == LXC_MAINLOOP_CLOSE) return 0; } diff --git a/src/lxc/mainloop.h b/src/lxc/mainloop.h index 903246a19..85c54f99d 100644 --- a/src/lxc/mainloop.h +++ b/src/lxc/mainloop.h @@ -52,4 +52,13 @@ extern int lxc_mainloop_open(struct lxc_epoll_descr *descr); extern int lxc_mainloop_close(struct lxc_epoll_descr *descr); +static inline void __auto_lxc_mainloop_close__(struct lxc_epoll_descr **descr) +{ + if (*descr) + lxc_mainloop_close(*descr); +} + +#define __do_lxc_mainloop_close \ + __attribute__((__cleanup__(__auto_lxc_mainloop_close__))) + #endif