The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxc/pull/3203
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 0b3f6ed86c64c2afa2e39191a18db198e7bbb18c Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Tue, 3 Dec 2019 17:33:11 +0100 Subject: [PATCH] freezer: cleanup Signed-off-by: Christian Brauner <christian.brau...@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;
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel