[lxc-devel] [lxc/master] freezer: cleanup

brauner on Github lxc-bot at linuxcontainers.org
Tue Dec 3 16:44:30 UTC 2019


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 364 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20191203/c3b0a7d0/attachment.bin>
-------------- next part --------------
From 0b3f6ed86c64c2afa2e39191a18db198e7bbb18c Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 3 Dec 2019 17:33:11 +0100
Subject: [PATCH] freezer: cleanup

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/cgroups/cgfsng.c          | 41 ++++++----------
 src/lxc/cgroups/cgroup.h          |  8 ++++
 src/lxc/cgroups/cgroup2_devices.c | 16 +++----
 src/lxc/cgroups/cgroup_utils.c    | 19 ++++++++
 src/lxc/cgroups/cgroup_utils.h    |  2 +
 src/lxc/freezer.c                 | 77 +++++++++++++------------------
 src/lxc/log.h                     | 16 +++++--
 7 files changed, 94 insertions(+), 85 deletions(-)

diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index 12cd4b9237..66ff9bbf87 100644
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -2745,23 +2745,24 @@ __cgfsng_ops bool cgfsng_devices_activate(struct cgroup_ops *ops,
 
 	devices = bpf_program_new(BPF_PROG_TYPE_CGROUP_DEVICE);
 	if (!devices)
-		return log_error(false, ENOMEM,
-				 "Failed to create new bpf program");
+		return log_error_errno(false, ENOMEM,
+				       "Failed to create new bpf program");
 
 	ret = bpf_program_init(devices);
 	if (ret)
-		return log_error(false, ENOMEM,
-				 "Failed to initialize bpf program");
+		return log_error_errno(false, ENOMEM,
+				       "Failed to initialize bpf program");
 
 	lxc_list_for_each(it, &conf->devices) {
 		struct device_item *cur = it->elem;
 
 		ret = bpf_program_append_device(devices, cur);
 		if (ret)
-			return log_error(false,
-					 ENOMEM, "Failed to add new rule to bpf device program: type %c, major %d, minor %d, access %s, allow %d, global_rule %d",
-					 cur->type, cur->major, cur->minor,
-					 cur->access, cur->allow, cur->global_rule);
+			return log_error_errno(false,
+					       ENOMEM, "Failed to add new rule to bpf device program: type %c, major %d, minor %d, access %s, allow %d, global_rule %d",
+					       cur->type, cur->major,
+					       cur->minor, cur->access,
+					       cur->allow, cur->global_rule);
 		TRACE("Added rule to bpf device program: type %c, major %d, minor %d, access %s, allow %d, global_rule %d",
 		      cur->type, cur->major, cur->minor, cur->access,
 		      cur->allow, cur->global_rule);
@@ -2769,13 +2770,15 @@ __cgfsng_ops bool cgfsng_devices_activate(struct cgroup_ops *ops,
 
 	ret = bpf_program_finalize(devices);
 	if (ret)
-		return log_error(false, ENOMEM, "Failed to finalize bpf program");
+		return log_error_errno(false, ENOMEM,
+				       "Failed to finalize bpf program");
 
 	ret = bpf_program_cgroup_attach(devices, BPF_CGROUP_DEVICE,
 					unified->container_full_path,
 					BPF_F_ALLOW_MULTI);
 	if (ret)
-		return log_error(false, ENOMEM, "Failed to attach bpf program");
+		return log_error_errno(false, ENOMEM,
+				       "Failed to attach bpf program");
 
 	/* Replace old bpf program. */
 	devices_old = move_ptr(conf->cgroup2_devices);
@@ -2999,22 +3002,6 @@ static bool cg_hybrid_init(struct cgroup_ops *ops, bool relative,
 	return true;
 }
 
-static int cg_is_pure_unified(void)
-{
-
-	int ret;
-	struct statfs fs;
-
-	ret = statfs(DEFAULT_CGROUP_MOUNTPOINT, &fs);
-	if (ret < 0)
-		return -ENOMEDIUM;
-
-	if (is_fs_type(&fs, CGROUP2_SUPER_MAGIC))
-		return CGROUP2_SUPER_MAGIC;
-
-	return 0;
-}
-
 /* Get current cgroup from /proc/self/cgroup for the cgroupfs v2 hierarchy. */
 static char *cg_unified_get_current_cgroup(bool relative)
 {
@@ -3055,7 +3042,7 @@ static int cg_unified_init(struct cgroup_ops *ops, bool relative,
 	struct hierarchy *new;
 	char *base_cgroup = NULL;
 
-	ret = cg_is_pure_unified();
+	ret = unified_cgroup_hierarchy();
 	if (ret == -ENOMEDIUM)
 		return -ENOMEDIUM;
 
diff --git a/src/lxc/cgroups/cgroup.h b/src/lxc/cgroups/cgroup.h
index 3c62afefc1..edc8258fb3 100644
--- a/src/lxc/cgroups/cgroup.h
+++ b/src/lxc/cgroups/cgroup.h
@@ -184,4 +184,12 @@ extern void cgroup_exit(struct cgroup_ops *ops);
 
 extern void prune_init_scope(char *cg);
 
+static inline void __auto_cgroup_exit__(struct cgroup_ops **ops)
+{
+	if (*ops)
+		cgroup_exit(*ops);
+}
+
+#define __do_cgroup_exit __attribute__((__cleanup__(__auto_cgroup_exit__)))
+
 #endif
diff --git a/src/lxc/cgroups/cgroup2_devices.c b/src/lxc/cgroups/cgroup2_devices.c
index aa6eff884c..e72cffc1c6 100644
--- a/src/lxc/cgroups/cgroup2_devices.c
+++ b/src/lxc/cgroups/cgroup2_devices.c
@@ -511,23 +511,23 @@ bool bpf_devices_cgroup_supported(void)
 	int ret;
 
 	if (geteuid() != 0)
-		return log_error(false, EINVAL,
-				 "The bpf device cgroup requires real root");
+		return log_error_errno(false,
+				       EINVAL, "The bpf device cgroup requires real root");
 
 	prog = bpf_program_new(BPF_PROG_TYPE_CGROUP_DEVICE);
 	if (prog < 0)
-		return log_error(false,
-				 errno, "Failed to allocate new bpf device cgroup program");
+		return log_error_errno(false,
+				       errno, "Failed to allocate new bpf device cgroup program");
 
 	ret = bpf_program_add_instructions(prog, dummy, ARRAY_SIZE(dummy));
 	if (ret < 0)
-		return log_error(false,
-				 errno, "Failed to add new instructions to bpf device cgroup program");
+		return log_error_errno(false,
+				       errno, "Failed to add new instructions to bpf device cgroup program");
 
 	ret = bpf_program_load_kernel(prog, NULL, 0);
 	if (ret < 0)
-		return log_error(false,
-				 errno, "Failed to load new bpf device cgroup program");
+		return log_error_errno(false,
+				       errno, "Failed to load new bpf device cgroup program");
 
 	return log_trace(true, "The bpf device cgroup is supported");
 }
diff --git a/src/lxc/cgroups/cgroup_utils.c b/src/lxc/cgroups/cgroup_utils.c
index 1b780b2bc2..c03c99a4bd 100644
--- a/src/lxc/cgroups/cgroup_utils.c
+++ b/src/lxc/cgroups/cgroup_utils.c
@@ -28,10 +28,13 @@
 #include <stdbool.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sys/vfs.h>
 #include <unistd.h>
 
+#include "cgroup.h"
 #include "cgroup_utils.h"
 #include "config.h"
+#include "file_utils.h"
 #include "macro.h"
 #include "memory_utils.h"
 #include "utils.h"
@@ -101,3 +104,19 @@ bool test_writeable_v2(char *mountpoint, char *path)
 
 	return (access(cgroup_threads_file, W_OK) == 0);
 }
+
+int unified_cgroup_hierarchy(void)
+{
+
+	int ret;
+	struct statfs fs;
+
+	ret = statfs(DEFAULT_CGROUP_MOUNTPOINT, &fs);
+	if (ret < 0)
+		return -ENOMEDIUM;
+
+	if (is_fs_type(&fs, CGROUP2_SUPER_MAGIC))
+		return CGROUP2_SUPER_MAGIC;
+
+	return 0;
+}
diff --git a/src/lxc/cgroups/cgroup_utils.h b/src/lxc/cgroups/cgroup_utils.h
index 3a4726e5b9..04e35f2c11 100644
--- a/src/lxc/cgroups/cgroup_utils.h
+++ b/src/lxc/cgroups/cgroup_utils.h
@@ -48,4 +48,6 @@ extern bool test_writeable_v1(char *mountpoint, char *path);
  */
 extern bool test_writeable_v2(char *mountpoint, char *path);
 
+extern int unified_cgroup_hierarchy(void);
+
 #endif /* __LXC_CGROUP_UTILS_H */
diff --git a/src/lxc/freezer.c b/src/lxc/freezer.c
index 3f6aaa546b..4cb0ba5d89 100644
--- a/src/lxc/freezer.c
+++ b/src/lxc/freezer.c
@@ -33,7 +33,8 @@
 #include <sys/types.h>
 #include <unistd.h>
 
-#include "cgroup.h"
+#include "cgroups/cgroup.h"
+#include "cgroups/cgroup_utils.h"
 #include "commands.h"
 #include "config.h"
 #include "error.h"
@@ -48,12 +49,12 @@ lxc_log_define(freezer, lxc);
 static int do_freeze_thaw(bool freeze, struct lxc_conf *conf, const char *name,
 			  const char *lxcpath)
 {
+	__do_cgroup_exit struct cgroup_ops *cgroup_ops = NULL;
+	lxc_state_t new_state = freeze ? FROZEN : THAWED;
 	int ret;
 	char v[100];
-	struct cgroup_ops *cgroup_ops;
 	const char *state;
 	size_t state_len;
-	lxc_state_t new_state = freeze ? FROZEN : THAWED;
 
 	state = lxc_state2str(new_state);
 	state_len = strlen(state);
@@ -62,50 +63,30 @@ static int do_freeze_thaw(bool freeze, struct lxc_conf *conf, const char *name,
 	if (!cgroup_ops)
 		return -1;
 
-	if (cgroup_ops->cgroup_layout != CGROUP_LAYOUT_UNIFIED) {
-		ret = cgroup_ops->set(cgroup_ops, "freezer.state", state, name,
-				      lxcpath);
-		if (ret < 0) {
-			cgroup_exit(cgroup_ops);
-			ERROR("Failed to %s %s",
-			      (new_state == FROZEN ? "freeze" : "unfreeze"),
-			      name);
-			return -1;
+	ret = cgroup_ops->set(cgroup_ops, "freezer.state", state, name, lxcpath);
+	if (ret < 0)
+		return log_error(-1, "Failed to %s %s",
+				 freeze ? "freeze" : "unfreeze", name);
+
+	for (;;) {
+		ret = cgroup_ops->get(cgroup_ops, "freezer.state", v, sizeof(v),
+				      name, lxcpath);
+		if (ret < 0)
+			return log_error(-1, "Failed to get freezer state of %s", name);
+
+		v[sizeof(v) - 1] = '\0';
+		v[lxc_char_right_gc(v, strlen(v))] = '\0';
+
+		ret = strncmp(v, state, state_len);
+		if (ret == 0) {
+			lxc_cmd_serve_state_clients(name, lxcpath, new_state);
+			lxc_monitor_send_state(name, new_state, lxcpath);
+			return 0;
 		}
 
-		for (;;) {
-			ret = cgroup_ops->get(cgroup_ops, "freezer.state", v,
-					      sizeof(v), name, lxcpath);
-			if (ret < 0) {
-				cgroup_exit(cgroup_ops);
-				ERROR("Failed to get freezer state of %s", name);
-				return -1;
-			}
-
-			v[sizeof(v) - 1] = '\0';
-			v[lxc_char_right_gc(v, strlen(v))] = '\0';
-
-			ret = strncmp(v, state, state_len);
-			if (ret == 0) {
-				cgroup_exit(cgroup_ops);
-				lxc_cmd_serve_state_clients(name, lxcpath,
-							    new_state);
-				lxc_monitor_send_state(name, new_state, lxcpath);
-				return 0;
-			}
-
-			sleep(1);
-		}
+		sleep(1);
 	}
 
-	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;
 }
 
@@ -121,7 +102,10 @@ int lxc_freeze(struct lxc_conf *conf, const char *name, const char *lxcpath)
 	int ret;
 
 	notify_state_listeners(name, lxcpath, FREEZING);
-	ret = do_freeze_thaw(true, conf, name, lxcpath);
+	if (unified_cgroup_hierarchy() > 0)
+		ret = lxc_cmd_freeze(name, lxcpath, -1);
+	else
+		ret = do_freeze_thaw(true, conf, name, lxcpath);
 	notify_state_listeners(name, lxcpath, !ret ? FROZEN : RUNNING);
 	return ret;
 }
@@ -131,7 +115,10 @@ int lxc_unfreeze(struct lxc_conf *conf, const char *name, const char *lxcpath)
 	int ret;
 
 	notify_state_listeners(name, lxcpath, THAWED);
-	ret = do_freeze_thaw(false, conf, name, lxcpath);
+	if (unified_cgroup_hierarchy() > 0)
+		ret = lxc_cmd_unfreeze(name, lxcpath, -1);
+	else
+		ret = do_freeze_thaw(false, conf, name, lxcpath);
 	notify_state_listeners(name, lxcpath, !ret ? RUNNING : FROZEN);
 	return ret;
 }
diff --git a/src/lxc/log.h b/src/lxc/log.h
index c6b2be2d6e..61de06b5a8 100644
--- a/src/lxc/log.h
+++ b/src/lxc/log.h
@@ -518,11 +518,17 @@ ATTR_UNUSED static inline void LXC_##LEVEL(struct lxc_log_locinfo* locinfo,	\
 		__ret__;                      \
 	})
 
-#define log_error(__ret__, __errno__, format, ...) \
-	({                                         \
-		errno = __errno__;                 \
-		SYSERROR(format, ##__VA_ARGS__);   \
-		__ret__;                           \
+#define log_error_errno(__ret__, __errno__, format, ...) \
+	({						 \
+		errno = __errno__;			 \
+		SYSERROR(format, ##__VA_ARGS__);	 \
+		__ret__;				 \
+	})
+
+#define log_error(__ret__, format, ...)	      \
+	({				      \
+		ERROR(format, ##__VA_ARGS__); \
+		__ret__;		      \
 	})
 
 extern int lxc_log_fd;


More information about the lxc-devel mailing list