Merge pull request #3202 from brauner/cgroup2_freezer_fixes

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"
@@ -742,8 +743,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_CGROUP_MOUNTPOINT "/", 15) != 0) {
+		ERROR("Found hierarchy not under " DEFAULT_CGROUP_MOUNTPOINT ": \"%s\"", p);
 		return NULL;
 	}
 
@@ -844,7 +845,7 @@ static char *cg_hybrid_get_mountpoint(char *line)
 		p++;
 	}
 
-	if (strncmp(p, "/sys/fs/cgroup/", 15) != 0)
+	if (strncmp(p, DEFAULT_CGROUP_MOUNTPOINT "/", 15) != 0)
 		return NULL;
 
 	p2 = strchr(p + 15, ' ');
@@ -1811,7 +1812,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_CGROUP_MOUNTPOINT, NULL);
 	if (ops->cgroup_layout == CGROUP_LAYOUT_UNIFIED) {
 		if (has_cgns && wants_force_mount) {
 			/* If cgroup namespaces are supported but the container
@@ -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,
@@ -2953,7 +3005,7 @@ static int cg_is_pure_unified(void)
 	int ret;
 	struct statfs fs;
 
-	ret = statfs("/sys/fs/cgroup", &fs);
+	ret = statfs(DEFAULT_CGROUP_MOUNTPOINT, &fs);
 	if (ret < 0)
 		return -ENOMEDIUM;
 
@@ -3019,7 +3071,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_CGROUP_MOUNTPOINT);
 	subtree_path = must_make_path(mountpoint, base_cgroup,
 				      "cgroup.subtree_control", NULL);
 	delegatable = cg_unified_get_controllers(subtree_path);

src/lxc/cgroups/cgroup.h

@@ -28,6 +28,7 @@
 #include <stddef.h>
 #include <sys/types.h>
 
+#define DEFAULT_CGROUP_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;
 };
 
@@ -162,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);

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) {

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 */

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;
 }

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;
 		}

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