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

Reply via email to