cgroups/cgfsng: rework cgroup removal

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>

diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index 7ddde025284da2d43173a97c003ff303fca00b84..b5f64cc774d2e81df6bf372c2f1fbdde8bb4ffaf 100644 (file)
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -382,7 +382,6 @@ static bool cg_legacy_filter_and_set_cpus(const char *parent_cgroup,
        ssize_t maxisol = 0, maxoffline = 0, maxposs = 0;
        bool flipped_bit = false;
 
-       SYSERROR("AAAA: %s | %s", parent_cgroup, child_cgroup);
        fpath = must_make_path(parent_cgroup, "cpuset.cpus", NULL);
        posscpus = read_file(fpath);
        if (!posscpus)
@@ -998,12 +997,10 @@ static void lxc_cgfsng_print_basecg_debuginfo(char *basecginfo, char **klist,
 static int cgroup_rmdir(struct hierarchy **hierarchies,
                        const char *container_cgroup)
 {
-       int i;
-
        if (!container_cgroup || !hierarchies)
                return 0;
 
-       for (i = 0; hierarchies[i]; i++) {
+       for (int i = 0; hierarchies[i]; i++) {
                int ret;
                struct hierarchy *h = hierarchies[i];
 
@@ -1031,30 +1028,26 @@ struct generic_userns_exec_data {
 
 static int cgroup_rmdir_wrapper(void *data)
 {
-       int ret;
        struct generic_userns_exec_data *arg = data;
        uid_t nsuid = (arg->conf->root_nsuid_map != NULL) ? 0 : arg->conf->init_uid;
        gid_t nsgid = (arg->conf->root_nsgid_map != NULL) ? 0 : arg->conf->init_gid;
+       int ret;
 
        ret = setresgid(nsgid, nsgid, nsgid);
-       if (ret < 0) {
-               SYSERROR("Failed to setresgid(%d, %d, %d)", (int)nsgid,
-                        (int)nsgid, (int)nsgid);
-               return -1;
-       }
+       if (ret < 0)
+               return log_error_errno(-1, errno,
+                                      "Failed to setresgid(%d, %d, %d)",
+                                      (int)nsgid, (int)nsgid, (int)nsgid);
 
        ret = setresuid(nsuid, nsuid, nsuid);
-       if (ret < 0) {
-               SYSERROR("Failed to setresuid(%d, %d, %d)", (int)nsuid,
-                        (int)nsuid, (int)nsuid);
-               return -1;
-       }
+       if (ret < 0)
+               return log_error_errno(-1, errno,
+                                      "Failed to setresuid(%d, %d, %d)",
+                                      (int)nsuid, (int)nsuid, (int)nsuid);
 
        ret = setgroups(0, NULL);
-       if (ret < 0 && errno != EPERM) {
-               SYSERROR("Failed to setgroups(0, NULL)");
-               return -1;
-       }
+       if (ret < 0 && errno != EPERM)
+               return log_error_errno(-1, errno, "Failed to setgroups(0, NULL)");
 
        return cgroup_rmdir(arg->hierarchies, arg->container_cgroup);
 }
@@ -1063,7 +1056,6 @@ __cgfsng_ops static void cgfsng_payload_destroy(struct cgroup_ops *ops,
                                                struct lxc_handler *handler)
 {
        int ret;
-       struct generic_userns_exec_data wrap;
 
        if (!ops)
                log_error_errno(return, ENOENT, "Called with uninitialized cgroup operations");
@@ -1077,26 +1069,26 @@ __cgfsng_ops static void cgfsng_payload_destroy(struct cgroup_ops *ops,
        if (!handler->conf)
                log_error_errno(return, EINVAL, "Called with uninitialized conf");
 
-       wrap.origuid = 0;
-       wrap.container_cgroup = ops->container_cgroup;
-       wrap.hierarchies = ops->hierarchies;
-       wrap.conf = handler->conf;
-
 #ifdef HAVE_STRUCT_BPF_CGROUP_DEV_CTX
        ret = bpf_program_cgroup_detach(handler->conf->cgroup2_devices);
        if (ret < 0)
                WARN("Failed to detach bpf program from cgroup");
 #endif
 
-       if (handler->conf && !lxc_list_empty(&handler->conf->id_map))
+       if (handler->conf && !lxc_list_empty(&handler->conf->id_map)) {
+               struct generic_userns_exec_data wrap = {
+                   .origuid = 0,
+                   .container_cgroup = ops->container_cgroup,
+                   .hierarchies = ops->hierarchies,
+                   .conf = handler->conf,
+               };
                ret = userns_exec_1(handler->conf, cgroup_rmdir_wrapper, &wrap,
                                    "cgroup_rmdir_wrapper");
-       else
+       } else {
                ret = cgroup_rmdir(ops->hierarchies, ops->container_cgroup);
-       if (ret < 0) {
-               WARN("Failed to destroy cgroups");
-               return;
        }
+       if (ret < 0)
+               log_warn_errno(return, errno, "Failed to destroy cgroups");
 }
 
 __cgfsng_ops static void cgfsng_monitor_destroy(struct cgroup_ops *ops,
@@ -1104,7 +1096,6 @@ __cgfsng_ops static void cgfsng_monitor_destroy(struct cgroup_ops *ops,
 {
        int len;
        char pidstr[INTTYPE_TO_STRLEN(pid_t)];
-       struct lxc_conf *conf;
 
        if (!ops)
                log_error_errno(return, ENOENT, "Called with uninitialized cgroup operations");
@@ -1115,56 +1106,24 @@ __cgfsng_ops static void cgfsng_monitor_destroy(struct cgroup_ops *ops,
        if (!handler)
                log_error_errno(return, EINVAL, "Called with uninitialized handler");
 
-       if (!handler->conf)
-               log_error_errno(return, EINVAL, "Called with uninitialized conf");
-
-       conf = handler->conf;
-
        len = snprintf(pidstr, sizeof(pidstr), "%d", handler->monitor_pid);
        if (len < 0 || (size_t)len >= sizeof(pidstr))
                return;
 
        for (int i = 0; ops->hierarchies[i]; i++) {
-               __do_free char *pivot_path = NULL;
-               char pivot_cgroup[] = CGROUP_PIVOT;
+               __do_free char *base_path = NULL;
                struct hierarchy *h = ops->hierarchies[i];
                int ret;
 
                if (!h->monitor_full_path)
                        continue;
 
-               if (conf && conf->cgroup_meta.dir)
-                       pivot_path = must_make_path(h->mountpoint,
-                                                   h->container_base_path,
-                                                   conf->cgroup_meta.dir,
-                                                   CGROUP_PIVOT, NULL);
-               else
-                       pivot_path = must_make_path(h->mountpoint,
-                                                   h->container_base_path,
-                                                   CGROUP_PIVOT, NULL);
-
-               /*
-                * Make sure not to pass in the ro string literal CGROUP_PIVOT
-                * here.
-                */
-               if (cg_legacy_handle_cpuset_hierarchy(h, pivot_cgroup) < 0)
-                       log_warn_errno(continue, errno, "Failed to handle legacy cpuset controller");
-
-               ret = mkdir_p(pivot_path, 0755);
-               if (ret < 0 && errno != EEXIST)
-                       log_warn_errno(continue, errno,
-                                      "Failed to create cgroup \"%s\"\n",
-                                      pivot_path);
-
-               /*
-                * Move ourselves into the pivot cgroup to delete our own
-                * cgroup.
-                */
-               ret = lxc_write_openat(pivot_path, "cgroup.procs", pidstr, len);
+               base_path = must_make_path(h->mountpoint, h->container_base_path, NULL);
+               ret = lxc_write_openat(base_path, "cgroup.procs", pidstr, len);
                if (ret != 0)
                        log_warn_errno(continue, errno,
-                                      "Failed to move monitor %s to \"%s\"\n",
-                                      pidstr, pivot_path);
+                                      "Failed to move monitor %s to \"%s\"",
+                                      pidstr, base_path);
 
                ret = recursive_destroy(h->monitor_full_path);
                if (ret < 0)

diff --git a/src/lxc/cgroups/cgroup.h b/src/lxc/cgroups/cgroup.h
index 14daa1fc215a9a9ef9a56516a65284fabaaa66f8..dccc454462d6cf5880164888c0d21b1f332fcbcb 100644 (file)
--- a/src/lxc/cgroups/cgroup.h
+++ b/src/lxc/cgroups/cgroup.h
@@ -14,7 +14,6 @@
 #define DEFAULT_MONITOR_CGROUP_PREFIX "lxc.monitor."
 #define CGROUP_CREATE_RETRY "-NNNN"
 #define CGROUP_CREATE_RETRY_LEN (STRLITERALLEN(CGROUP_CREATE_RETRY))
-#define CGROUP_PIVOT "lxc.pivot"
 
 struct lxc_handler;
 struct lxc_conf;

diff --git a/src/lxc/file_utils.c b/src/lxc/file_utils.c
index b6003a3a5c2eb4a3ec415ee0018e95e752af7cdf..14ff7a93aa40d27ea4ae6c8b37dcd2ed5ab7680b 100644 (file)
--- a/src/lxc/file_utils.c
+++ b/src/lxc/file_utils.c
@@ -13,6 +13,7 @@
 
 #include "config.h"
 #include "file_utils.h"
+#include "log.h"
 #include "macro.h"
 #include "memory_utils.h"
 #include "string_utils.h"

diff --git a/src/lxc/utils.c b/src/lxc/utils.c
index 06011f96bc7279d8341c61b236d9706089dd7dc8..da2d3112bd65179887c79a3e47bac92c8e1a92ce 100644 (file)
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -1791,21 +1791,19 @@ int fd_cloexec(int fd, bool cloexec)
        return 0;
 }
 
-int recursive_destroy(char *dirname)
+int recursive_destroy(const char *dirname)
 {
+       __do_closedir DIR *dir = NULL;
+       int fret = 0;
        int ret;
        struct dirent *direntp;
-       DIR *dir;
-       int r = 0;
 
        dir = opendir(dirname);
-       if (!dir) {
-               SYSERROR("Failed to open dir \"%s\"", dirname);
-               return -1;
-       }
+       if (!dir)
+               return log_error_errno(-1, errno, "Failed to open dir \"%s\"", dirname);
 
        while ((direntp = readdir(dir))) {
-               char *pathname;
+               __do_free char *pathname = NULL;
                struct stat mystat;
 
                if (!strcmp(direntp->d_name, ".") ||
@@ -1813,44 +1811,28 @@ int recursive_destroy(char *dirname)
                        continue;
 
                pathname = must_make_path(dirname, direntp->d_name, NULL);
-
                ret = lstat(pathname, &mystat);
                if (ret < 0) {
-                       if (!r)
+                       if (!fret)
                                SYSWARN("Failed to stat \"%s\"", pathname);
 
-                       r = -1;
-                       goto next;
+                       fret = -1;
+                       continue;
                }
 
                if (!S_ISDIR(mystat.st_mode))
-                       goto next;
+                       continue;
 
                ret = recursive_destroy(pathname);
                if (ret < 0)
-                       r = -1;
-
-       next:
-               free(pathname);
+                       fret = -1;
        }
 
        ret = rmdir(dirname);
-       if (ret < 0) {
-               if (!r)
-                       SYSWARN("Failed to delete \"%s\"", dirname);
-
-               r = -1;
-       }
-
-       ret = closedir(dir);
-       if (ret < 0) {
-               if (!r)
-                       SYSWARN("Failed to delete \"%s\"", dirname);
-
-               r = -1;
-       }
+       if (ret < 0)
+               return log_warn_errno(-1, errno, "Failed to delete \"%s\"", dirname);
 
-       return r;
+       return fret;
 }
 
 int lxc_setup_keyring(void)

diff --git a/src/lxc/utils.h b/src/lxc/utils.h
index 5f07150dbc3f5490f633fe201ca843ab435d197a..8b3cfdfa04adfccd0fbd19e55a66f35463e8d0ac 100644 (file)
--- a/src/lxc/utils.h
+++ b/src/lxc/utils.h
@@ -231,7 +231,7 @@ extern uint64_t lxc_find_next_power2(uint64_t n);
 /* Set a signal the child process will receive after the parent has died. */
 extern int lxc_set_death_signal(int signal, pid_t parent, int parent_status_fd);
 extern int fd_cloexec(int fd, bool cloexec);
-extern int recursive_destroy(char *dirname);
+extern int recursive_destroy(const char *dirname);
 extern int lxc_setup_keyring(void);
 
 #endif /* __LXC_UTILS_H */