nspawn: Detect the outer_cgver once, and pass that around

Yes, the relevant functions in cgroup-util actually do cache the values
with static variables.  But passing it around as a value makes the flow
much nicer.  The symmetry of having both the inner and outer cg versions
as a CGroupUnified enum makes the code much easier to grok; this could be
done with cg_version(), but I still think this is more readable.

3 files changed, 40 insertions, 36 deletions

diff --git a/src/nspawn/nspawn-cgroup.c b/src/nspawn/nspawn-cgroup.c
index d09506fce5..38a273f42a 100644
--- a/src/nspawn/nspawn-cgroup.c
+++ b/src/nspawn/nspawn-cgroup.c
@@ -78,18 +78,18 @@ int chown_cgroup(pid_t pid, uid_t uid_shift) {
         return 0;
 }
 
-int sync_cgroup(pid_t pid, CGroupUnified inner_cgver, uid_t uid_shift) {
+int sync_cgroup(pid_t pid, CGroupUnified outer_cgver, CGroupUnified inner_cgver, uid_t uid_shift) {
         _cleanup_free_ char *cgroup = NULL;
         char tree[] = "/tmp/unifiedXXXXXX", pid_string[DECIMAL_STR_MAX(pid) + 1];
         bool undo_mount = false;
         const char *fn;
-        int unified, r;
+        int r;
 
         unified = cg_unified(SYSTEMD_CGROUP_CONTROLLER);
         if (unified < 0)
                 return log_error_errno(unified, "Failed to determine whether the unified hierarchy is used: %m");
 
-        if ((unified > 0) == (inner_cgver >= CGROUP_UNIFIED_SYSTEMD))
+        if ((outer_cgver >= CGROUP_UNIFIED_SYSTEMD) == (inner_cgver >= CGROUP_UNIFIED_SYSTEMD))
                 return 0;
 
         /* When the host uses the legacy cgroup setup, but the
@@ -105,7 +105,7 @@ int sync_cgroup(pid_t pid, CGroupUnified inner_cgver, uid_t uid_shift) {
         if (!mkdtemp(tree))
                 return log_error_errno(errno, "Failed to generate temporary mount point for unified hierarchy: %m");
 
-        if (unified)
+        if (outer_cgver >= CGROUP_UNIFIED_SYSTEMD) {
                 r = mount_verbose(LOG_ERR, "cgroup", tree, "cgroup",
                                   MS_NOSUID|MS_NOEXEC|MS_NODEV, "none,name=systemd,xattr");
         else
@@ -144,10 +144,10 @@ finish:
         return r;
 }
 
-int create_subcgroup(pid_t pid, CGroupUnified inner_cgver) {
+int create_subcgroup(pid_t pid, CGroupUnified outer_cgver, CGroupUnified inner_cgver) {
         _cleanup_free_ char *cgroup = NULL;
         const char *child;
-        int unified, r;
+        int r;
         CGroupMask supported;
 
         /* In the unified hierarchy inner nodes may only contain
@@ -156,13 +156,7 @@ int create_subcgroup(pid_t pid, CGroupUnified inner_cgver) {
          * did not create a scope unit for the container move us and
          * the container into two separate subcgroups. */
 
-        if (inner_cgver == CGROUP_UNIFIED_NONE)
-                return 0;
-
-        unified = cg_unified(SYSTEMD_CGROUP_CONTROLLER);
-        if (unified < 0)
-                return log_error_errno(unified, "Failed to determine whether the unified hierarchy is used: %m");
-        if (unified == 0)
+        if (inner_cgver == CGROUP_UNIFIED_NONE || outer_cgver == CGROUP_UNIFIED_NONE)
                 return 0;
 
         r = cg_mask_supported(&supported);
@@ -280,7 +274,7 @@ static int mount_legacy_cgroup_hierarchy(const char *dest, const char *controlle
 
 /* Mount a legacy cgroup hierarchy when cgroup namespaces are supported. */
 static int mount_legacy_cgns_supported(
-                CGroupUnified inner_cgver, bool userns, uid_t uid_shift,
+                CGroupUnified outer_cgver, CGroupUnified inner_cgver, bool userns, uid_t uid_shift,
                 uid_t uid_range, const char *selinux_apifs_context) {
         _cleanup_set_free_free_ Set *controllers = NULL;
         const char *cgroup_root = "/sys/fs/cgroup", *c;
@@ -312,7 +306,7 @@ static int mount_legacy_cgns_supported(
                         return r;
         }
 
-        if (cg_all_unified() > 0)
+        if (outer_cgver >= CGROUP_UNIFIED_ALL)
                 goto skip_controllers;
 
         controllers = set_new(&string_hash_ops);
@@ -378,7 +372,7 @@ skip_controllers:
 /* Mount legacy cgroup hierarchy when cgroup namespaces are unsupported. */
 static int mount_legacy_cgns_unsupported(
                 const char *dest,
-                CGroupUnified inner_cgver, bool userns, uid_t uid_shift, uid_t uid_range,
+                CGroupUnified outer_cgver, CGroupUnified inner_cgver, bool userns, uid_t uid_shift, uid_t uid_range,
                 const char *selinux_apifs_context) {
         _cleanup_set_free_free_ Set *controllers = NULL;
         const char *cgroup_root;
@@ -405,7 +399,7 @@ static int mount_legacy_cgns_unsupported(
                         return r;
         }
 
-        if (cg_all_unified() > 0)
+        if (outer_cgver >= CGROUP_UNIFIED_ALL)
                 goto skip_controllers;
 
         controllers = set_new(&string_hash_ops);
@@ -501,7 +495,7 @@ static int mount_unified_cgroups(const char *dest) {
 
 int mount_cgroups(
                 const char *dest,
-                CGroupUnified inner_cgver,
+                CGroupUnified outer_cgver, CGroupUnified inner_cgver,
                 bool userns, uid_t uid_shift, uid_t uid_range,
                 const char *selinux_apifs_context,
                 bool use_cgns) {
@@ -509,9 +503,9 @@ int mount_cgroups(
         if (inner_cgver >= CGROUP_UNIFIED_ALL)
                 return mount_unified_cgroups(dest);
         else if (use_cgns)
-                return mount_legacy_cgns_supported(inner_cgver, userns, uid_shift, uid_range, selinux_apifs_context);
+                return mount_legacy_cgns_supported(outer_cgver, inner_cgver, userns, uid_shift, uid_range, selinux_apifs_context);
 
-        return mount_legacy_cgns_unsupported(dest, inner_cgver, userns, uid_shift, uid_range, selinux_apifs_context);
+        return mount_legacy_cgns_unsupported(dest, outer_cgver, inner_cgver, userns, uid_shift, uid_range, selinux_apifs_context);
 }
 
 int mount_systemd_cgroup_writable(
diff --git a/src/nspawn/nspawn-cgroup.h b/src/nspawn/nspawn-cgroup.h
index d39883074f..d0e70b5172 100644
--- a/src/nspawn/nspawn-cgroup.h
+++ b/src/nspawn/nspawn-cgroup.h
@@ -25,8 +25,8 @@
 #include "cgroup-util.h"
 
 int chown_cgroup(pid_t pid, uid_t uid_shift);
-int sync_cgroup(pid_t pid, CGroupUnified inner_cgver, uid_t uid_shift);
-int create_subcgroup(pid_t pid, CGroupUnified inner_cgver);
+int sync_cgroup(pid_t pid, CGroupUnified outer_cgver, CGroupUnified inner_cgver, uid_t uid_shift);
+int create_subcgroup(pid_t pid, CGroupUnified outer_cgver, CGroupUnified inner_cgver);
 
-int mount_cgroups(const char *dest, CGroupUnified inner_cgver, bool userns, uid_t uid_shift, uid_t uid_range, const char *selinux_apifs_context, bool use_cgns);
+int mount_cgroups(const char *dest, CGroupUnified outer_cgver, CGroupUnified inner_cgver, bool userns, uid_t uid_shift, uid_t uid_range, const char *selinux_apifs_context, bool use_cgns);
 int mount_systemd_cgroup_writable(const char *dest, CGroupUnified inner_cgver);
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index c0b4c3142d..be1b054045 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -320,13 +320,8 @@ static int custom_mounts_prepare(void) {
         return 0;
 }
 
-static int pick_cgroup_version(const char *directory) {
+static int pick_cgroup_version(const char *directory, CGroupUnified outer) {
         int r;
-        CGroupUnified outer;
-
-        r = cg_version(&outer);
-        if (r < 0)
-                return log_error_errno(r, "Failed to determine whether the unified cgroups hierarchy is used: %m");
 
         /* Otherwise inherit the default from the host system, unless
          * the container doesn't have a new enough systemd (detected
@@ -2652,7 +2647,8 @@ static int inner_child(
                 bool secondary,
                 int kmsg_socket,
                 int rtnl_socket,
-                FDSet *fds) {
+                FDSet *fds,
+                CGroupUnified outer_cgver) {
 
         _cleanup_free_ char *home = NULL;
         char as_uuid[37];
@@ -2719,6 +2715,7 @@ static int inner_child(
                         return log_error_errno(errno, "Failed to unshare cgroup namespace");
                 r = mount_cgroups(
                                 "",
+                                outer_cgver,
                                 arg_unified_cgroup_hierarchy,
                                 arg_userns_mode != USER_NAMESPACE_NO,
                                 arg_uid_shift,
@@ -2925,7 +2922,8 @@ static int outer_child(
                 int kmsg_socket,
                 int rtnl_socket,
                 int uid_shift_socket,
-                FDSet *fds) {
+                FDSet *fds,
+                CGroupUnified outer_cgver) {
 
         pid_t pid;
         ssize_t l;
@@ -3122,6 +3120,7 @@ static int outer_child(
         if (!arg_use_cgns) {
                 r = mount_cgroups(
                                 directory,
+                                outer_cgver,
                                 arg_unified_cgroup_hierarchy,
                                 arg_userns_mode != USER_NAMESPACE_NO,
                                 arg_uid_shift,
@@ -3156,7 +3155,7 @@ static int outer_child(
                  * requested, so that we all are owned by the user if
                  * user namespaces are turned on. */
 
-                r = inner_child(barrier, directory, secondary, kmsg_socket, rtnl_socket, fds);
+                r = inner_child(barrier, directory, secondary, kmsg_socket, rtnl_socket, fds, outer_cgver);
                 if (r < 0)
                         _exit(EXIT_FAILURE);
 
@@ -3599,6 +3598,7 @@ static int run(int master,
                const char *esp_device,
                bool interactive,
                bool secondary,
+               CGroupUnified outer_cgver,
                FDSet *fds,
                char veth_name[IFNAMSIZ], bool *veth_created,
                union in_addr_union *exposed,
@@ -3715,7 +3715,8 @@ static int run(int master,
                                 kmsg_socket_pair[1],
                                 rtnl_socket_pair[1],
                                 uid_shift_socket_pair[1],
-                                fds);
+                                fds,
+                                outer_cgver);
                 if (r < 0)
                         _exit(EXIT_FAILURE);
 
@@ -3874,12 +3875,12 @@ static int run(int master,
                         return r;
         }
 
-        r = sync_cgroup(*pid, arg_unified_cgroup_hierarchy, arg_uid_shift);
+        r = sync_cgroup(*pid, outer_cgver, arg_unified_cgroup_hierarchy, arg_uid_shift);
         if (r < 0)
                 return r;
 
         if (arg_keep_unit) {
-                r = create_subcgroup(*pid, arg_unified_cgroup_hierarchy);
+                r = create_subcgroup(*pid, outer_cgver, arg_unified_cgroup_hierarchy);
                 if (r < 0)
                         return r;
         }
@@ -4025,6 +4026,7 @@ int main(int argc, char *argv[]) {
         union in_addr_union exposed = {};
         _cleanup_release_lock_file_ LockFile tree_global_lock = LOCK_FILE_INIT, tree_local_lock = LOCK_FILE_INIT;
         bool interactive, veth_created = false;
+        CGroupUnified outer_cgver;
 
         log_parse_environment();
         log_open();
@@ -4043,6 +4045,13 @@ int main(int argc, char *argv[]) {
                 r = -EPERM;
                 goto finish;
         }
+
+        r = cg_version(&outer_cgver);
+        if (r < 0) {
+                log_error_errno(r, "Failed to determine whether the unified cgroups hierarchy is used: %m");
+                goto finish;
+        }
+
         r = determine_names();
         if (r < 0)
                 goto finish;
@@ -4206,7 +4215,7 @@ int main(int argc, char *argv[]) {
                 goto finish;
 
         if (arg_unified_cgroup_hierarchy == CGROUP_UNIFIED_UNKNOWN) {
-                r = pick_cgroup_version(arg_directory);
+                r = pick_cgroup_version(arg_directory, outer_cgver);
                 if (r < 0)
                         goto finish;
         }
@@ -4257,6 +4266,7 @@ int main(int argc, char *argv[]) {
                         srv_device, srv_device_rw,
                         esp_device,
                         interactive, secondary,
+                        outer_cgver,
                         fds,
                         veth_name, &veth_created,
                         &exposed,