From 52196e0b58ee0a8e78991862d49f49c7d7482d15 Mon Sep 17 00:00:00 2001 From: Luke Shumaker Date: Tue, 13 Jun 2017 23:43:04 -0400 Subject: 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. --- src/nspawn/nspawn-cgroup.c | 34 ++++++++++++++-------------------- src/nspawn/nspawn-cgroup.h | 6 +++--- src/nspawn/nspawn.c | 36 +++++++++++++++++++++++------------- 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, -- cgit v1.2.3