The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxc/pull/3202
This e-mail was sent by the LXC bot, direct replies will not reach the author unless they happen to be subscribed to this list. === Description (from pull-request) === Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
From 30d0484a541b117098a202fb65d162872282e14b Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Tue, 3 Dec 2019 01:24:55 +0100 Subject: [PATCH 1/2] cgroups: add DEFAULT_MOUNTPOINT #define Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/lxc/cgroups/cgfsng.c | 12 ++++++------ src/lxc/cgroups/cgroup.h | 3 +++ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c index 306530097f..b3d5740d07 100644 --- a/src/lxc/cgroups/cgfsng.c +++ b/src/lxc/cgroups/cgfsng.c @@ -742,8 +742,8 @@ static char **cg_hybrid_get_controllers(char **klist, char **nlist, char *line, /* Note, if we change how mountinfo works, then our caller will need to * verify /sys/fs/cgroup/ in this field. */ - if (strncmp(p, "/sys/fs/cgroup/", 15) != 0) { - ERROR("Found hierarchy not under /sys/fs/cgroup: \"%s\"", p); + if (strncmp(p, DEFAULT_MOUNTPOINT "/", 15) != 0) { + ERROR("Found hierarchy not under " DEFAULT_MOUNTPOINT ": \"%s\"", p); return NULL; } @@ -844,7 +844,7 @@ static char *cg_hybrid_get_mountpoint(char *line) p++; } - if (strncmp(p, "/sys/fs/cgroup/", 15) != 0) + if (strncmp(p, DEFAULT_MOUNTPOINT "/", 15) != 0) return NULL; p2 = strchr(p + 15, ' '); @@ -1811,7 +1811,7 @@ __cgfsng_ops static bool cgfsng_mount(struct cgroup_ops *ops, else if (type == LXC_AUTO_CGROUP_FULL_NOSPEC) type = LXC_AUTO_CGROUP_FULL_MIXED; - cgroup_root = must_make_path(root, "/sys/fs/cgroup", NULL); + cgroup_root = must_make_path(root, DEFAULT_MOUNTPOINT, NULL); if (ops->cgroup_layout == CGROUP_LAYOUT_UNIFIED) { if (has_cgns && wants_force_mount) { /* If cgroup namespaces are supported but the container @@ -2953,7 +2953,7 @@ static int cg_is_pure_unified(void) int ret; struct statfs fs; - ret = statfs("/sys/fs/cgroup", &fs); + ret = statfs(DEFAULT_MOUNTPOINT, &fs); if (ret < 0) return -ENOMEDIUM; @@ -3019,7 +3019,7 @@ static int cg_unified_init(struct cgroup_ops *ops, bool relative, * further down the hierarchy. If not it is up to the user to delegate * them to us. */ - mountpoint = must_copy_string("/sys/fs/cgroup"); + mountpoint = must_copy_string(DEFAULT_MOUNTPOINT); subtree_path = must_make_path(mountpoint, base_cgroup, "cgroup.subtree_control", NULL); delegatable = cg_unified_get_controllers(subtree_path); diff --git a/src/lxc/cgroups/cgroup.h b/src/lxc/cgroups/cgroup.h index a3eb46b9d6..9cb7036f71 100644 --- a/src/lxc/cgroups/cgroup.h +++ b/src/lxc/cgroups/cgroup.h @@ -28,6 +28,7 @@ #include <stddef.h> #include <sys/types.h> +#define DEFAULT_MOUNTPOINT "/sys/fs/cgroup" #define PAYLOAD_CGROUP "lxc.payload" #define MONITOR_CGROUP "lxc.monitor" #define PIVOT_CGROUP "lxc.pivot" @@ -92,6 +93,8 @@ struct hierarchy { char *container_full_path; char *monitor_full_path; int version; + + /* cgroup2 only */ int bpf_device_controller:1; }; From 9e9ee9050572f1217cea7e79b0dfa9942da475f3 Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Tue, 3 Dec 2019 02:23:34 +0100 Subject: [PATCH 2/2] cgroups/freezer: fix and improve cgroup2 freezer implementation Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/lxc/cgroups/cgfsng.c | 176 +++++++++++++++++++++++++-------------- src/lxc/cgroups/cgroup.h | 4 +- src/lxc/commands.c | 75 ++++++++++++++++- src/lxc/commands.h | 4 + src/lxc/freezer.c | 9 +- src/lxc/mainloop.c | 2 + src/lxc/mainloop.h | 9 ++ 7 files changed, 208 insertions(+), 71 deletions(-) diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c index b3d5740d07..b75f7d4de4 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,169 @@ 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 *events_file = NULL, *basepath = NULL, *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) { + 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"); - f = fdopen(fd, "re"); - if (!f) - return false; - move_fd(fd); + ret = lxc_mainloop_open(&descr); + if (ret) + return error_log_errno(errno, "Failed to create epoll instance to wait for container freeze"); - for (int i = 0; i < 10 && poll_file_ready(fd); i++) { - size_t len; + /* automatically cleaned up now */ + descr_ptr = &descr; - while (getline(&line, &len, f) != -1) { - if (strcmp(line, "frozen 1") == 0) - return true; - } + 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"); + } + + 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 -1; - fseek(f, 0, SEEK_SET); - }; + if (timeout != 0 && lxc_mainloop(&descr, timeout)) + return error_log_errno(errno, "Failed to wait for container to be frozen"); - return false; + 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_free char *path = NULL; + __do_close_prot_errno int fd = -EBADF; + __do_free char *events_file = NULL, *basepath = NULL, *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) { + 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 -1; + + 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 9cb7036f71..3067b0dac2 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 1605fe5e22..879860f125 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) @@ -655,10 +657,7 @@ static int lxc_cmd_stop_callback(int fd, struct lxc_cmd_req *req, * 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)) + if (cgroup_ops->unfreeze(cgroup_ops, -1)) return 0; ERROR("Failed to unfreeze container \"%s\"", handler->name); @@ -1228,6 +1227,72 @@ static int lxc_cmd_seccomp_notify_add_listener_callback(int fd, 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 +1316,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 008b7c30e2..4346d86b5b 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 1843f2a606..4b04e4ef4d 100644 --- a/src/lxc/freezer.c +++ b/src/lxc/freezer.c @@ -98,12 +98,17 @@ 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"); + lxc_cmd_serve_state_clients(name, lxcpath, freeze ? FROZEN : RUNNING); + lxc_monitor_send_state(name, freeze ? FROZEN : RUNNING, lxcpath); return 0; } @@ -116,5 +121,7 @@ int lxc_freeze(struct lxc_conf *conf, const char *name, const char *lxcpath) int lxc_unfreeze(struct lxc_conf *conf, const char *name, const char *lxcpath) { + lxc_cmd_serve_state_clients(name, lxcpath, THAWED); + lxc_monitor_send_state(name, THAWED, lxcpath); return do_freeze_thaw(false, conf, name, lxcpath); } diff --git a/src/lxc/mainloop.c b/src/lxc/mainloop.c index b169aa6fe2..6de74dd69c 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 903246a19f..85c54f99dd 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
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel