X Tutup
Skip to content

Commit b4cccbc

Browse files
poetteringpoettering
committed
cgroup: change cg_unified() to possibly return errors again
We use our cgroup APIs in various contexts, including from our libraries sd-login, sd-bus. As we don#t control those environments we can't rely that the unified cgroup setup logic succeeds, and hence really shouldn't assert on it. This more or less reverts 415fc41.
1 parent fc9ae71 commit b4cccbc

File tree

12 files changed

+184
-74
lines changed

12 files changed

+184
-74
lines changed

‎src/basic/cgroup-util.c‎

Lines changed: 97 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,13 @@ int cg_rmdir(const char *controller, const char *path) {
208208
if (r < 0 && errno != ENOENT)
209209
return -errno;
210210

211-
if (streq(controller, SYSTEMD_CGROUP_CONTROLLER) && cg_hybrid_unified()) {
211+
r = cg_hybrid_unified();
212+
if (r < 0)
213+
return r;
214+
if (r == 0)
215+
return 0;
216+
217+
if (streq(controller, SYSTEMD_CGROUP_CONTROLLER)) {
212218
r = cg_rmdir(SYSTEMD_CGROUP_CONTROLLER_LEGACY, path);
213219
if (r < 0)
214220
log_warning_errno(r, "Failed to remove compat systemd cgroup %s: %m", path);
@@ -549,7 +555,7 @@ static const char *controller_to_dirname(const char *controller) {
549555
* hierarchies, if it is specified. */
550556

551557
if (streq(controller, SYSTEMD_CGROUP_CONTROLLER)) {
552-
if (cg_hybrid_unified())
558+
if (cg_hybrid_unified() > 0)
553559
controller = SYSTEMD_CGROUP_CONTROLLER_HYBRID;
554560
else
555561
controller = SYSTEMD_CGROUP_CONTROLLER_LEGACY;
@@ -636,7 +642,10 @@ int cg_get_path(const char *controller, const char *path, const char *suffix, ch
636642
if (!cg_controller_is_valid(controller))
637643
return -EINVAL;
638644

639-
if (cg_all_unified())
645+
r = cg_all_unified();
646+
if (r < 0)
647+
return r;
648+
if (r > 0)
640649
r = join_path_unified(path, suffix, fs);
641650
else
642651
r = join_path_legacy(controller, path, suffix, fs);
@@ -648,6 +657,7 @@ int cg_get_path(const char *controller, const char *path, const char *suffix, ch
648657
}
649658

650659
static int controller_is_accessible(const char *controller) {
660+
int r;
651661

652662
assert(controller);
653663

@@ -659,7 +669,10 @@ static int controller_is_accessible(const char *controller) {
659669
if (!cg_controller_is_valid(controller))
660670
return -EINVAL;
661671

662-
if (cg_all_unified()) {
672+
r = cg_all_unified();
673+
if (r < 0)
674+
return r;
675+
if (r > 0) {
663676
/* We don't support named hierarchies if we are using
664677
* the unified hierarchy. */
665678

@@ -736,7 +749,10 @@ int cg_trim(const char *controller, const char *path, bool delete_root) {
736749
return -errno;
737750
}
738751

739-
if (streq(controller, SYSTEMD_CGROUP_CONTROLLER) && cg_hybrid_unified()) {
752+
q = cg_hybrid_unified();
753+
if (q < 0)
754+
return q;
755+
if (q > 0 && streq(controller, SYSTEMD_CGROUP_CONTROLLER)) {
740756
q = cg_trim(SYSTEMD_CGROUP_CONTROLLER_LEGACY, path, delete_root);
741757
if (q < 0)
742758
log_warning_errno(q, "Failed to trim compat systemd cgroup %s: %m", path);
@@ -765,7 +781,11 @@ int cg_create(const char *controller, const char *path) {
765781
return -errno;
766782
}
767783

768-
if (streq(controller, SYSTEMD_CGROUP_CONTROLLER) && cg_hybrid_unified()) {
784+
r = cg_hybrid_unified();
785+
if (r < 0)
786+
return r;
787+
788+
if (r > 0 && streq(controller, SYSTEMD_CGROUP_CONTROLLER)) {
769789
r = cg_create(SYSTEMD_CGROUP_CONTROLLER_LEGACY, path);
770790
if (r < 0)
771791
log_warning_errno(r, "Failed to create compat systemd cgroup %s: %m", path);
@@ -812,7 +832,11 @@ int cg_attach(const char *controller, const char *path, pid_t pid) {
812832
if (r < 0)
813833
return r;
814834

815-
if (streq(controller, SYSTEMD_CGROUP_CONTROLLER) && cg_hybrid_unified()) {
835+
r = cg_hybrid_unified();
836+
if (r < 0)
837+
return r;
838+
839+
if (r > 0 && streq(controller, SYSTEMD_CGROUP_CONTROLLER)) {
816840
r = cg_attach(SYSTEMD_CGROUP_CONTROLLER_LEGACY, path, pid);
817841
if (r < 0)
818842
log_warning_errno(r, "Failed to attach %d to compat systemd cgroup %s: %m", pid, path);
@@ -871,7 +895,10 @@ int cg_set_group_access(
871895
if (r < 0)
872896
return r;
873897

874-
if (streq(controller, SYSTEMD_CGROUP_CONTROLLER) && cg_hybrid_unified()) {
898+
r = cg_hybrid_unified();
899+
if (r < 0)
900+
return r;
901+
if (r > 0 && streq(controller, SYSTEMD_CGROUP_CONTROLLER)) {
875902
r = cg_set_group_access(SYSTEMD_CGROUP_CONTROLLER_LEGACY, path, mode, uid, gid);
876903
if (r < 0)
877904
log_warning_errno(r, "Failed to set group access on compat systemd cgroup %s: %m", path);
@@ -906,14 +933,20 @@ int cg_set_task_access(
906933
if (r < 0)
907934
return r;
908935

909-
if (!cg_unified(controller)) {
936+
r = cg_unified(controller);
937+
if (r < 0)
938+
return r;
939+
if (r == 0) {
910940
/* Compatibility, Always keep values for "tasks" in sync with
911941
* "cgroup.procs" */
912942
if (cg_get_path(controller, path, "tasks", &procs) >= 0)
913943
(void) chmod_and_chown(procs, mode, uid, gid);
914944
}
915945

916-
if (streq(controller, SYSTEMD_CGROUP_CONTROLLER) && cg_hybrid_unified()) {
946+
r = cg_hybrid_unified();
947+
if (r < 0)
948+
return r;
949+
if (r > 0 && streq(controller, SYSTEMD_CGROUP_CONTROLLER)) {
917950
r = cg_set_task_access(SYSTEMD_CGROUP_CONTROLLER_LEGACY, path, mode, uid, gid);
918951
if (r < 0)
919952
log_warning_errno(r, "Failed to set task access on compat systemd cgroup %s: %m", path);
@@ -964,7 +997,7 @@ int cg_pid_get_path(const char *controller, pid_t pid, char **path) {
964997
char line[LINE_MAX];
965998
const char *fs, *controller_str;
966999
size_t cs = 0;
967-
bool unified;
1000+
int unified;
9681001

9691002
assert(path);
9701003
assert(pid >= 0);
@@ -976,7 +1009,9 @@ int cg_pid_get_path(const char *controller, pid_t pid, char **path) {
9761009
controller = SYSTEMD_CGROUP_CONTROLLER;
9771010

9781011
unified = cg_unified(controller);
979-
if (!unified) {
1012+
if (unified < 0)
1013+
return unified;
1014+
if (unified == 0) {
9801015
if (streq(controller, SYSTEMD_CGROUP_CONTROLLER))
9811016
controller_str = SYSTEMD_CGROUP_CONTROLLER_LEGACY;
9821017
else
@@ -1048,7 +1083,10 @@ int cg_install_release_agent(const char *controller, const char *agent) {
10481083

10491084
assert(agent);
10501085

1051-
if (cg_unified(controller)) /* doesn't apply to unified hierarchy */
1086+
r = cg_unified(controller);
1087+
if (r < 0)
1088+
return r;
1089+
if (r > 0) /* doesn't apply to unified hierarchy */
10521090
return -EOPNOTSUPP;
10531091

10541092
r = cg_get_path(controller, NULL, "release_agent", &fs);
@@ -1096,7 +1134,10 @@ int cg_uninstall_release_agent(const char *controller) {
10961134
_cleanup_free_ char *fs = NULL;
10971135
int r;
10981136

1099-
if (cg_unified(controller)) /* Doesn't apply to unified hierarchy */
1137+
r = cg_unified(controller);
1138+
if (r < 0)
1139+
return r;
1140+
if (r > 0) /* Doesn't apply to unified hierarchy */
11001141
return -EOPNOTSUPP;
11011142

11021143
r = cg_get_path(controller, NULL, "notify_on_release", &fs);
@@ -1149,7 +1190,10 @@ int cg_is_empty_recursive(const char *controller, const char *path) {
11491190
if (controller && (isempty(path) || path_equal(path, "/")))
11501191
return false;
11511192

1152-
if (cg_unified(controller)) {
1193+
r = cg_unified(controller);
1194+
if (r < 0)
1195+
return r;
1196+
if (r > 0) {
11531197
_cleanup_free_ char *t = NULL;
11541198

11551199
/* On the unified hierarchy we can check empty state
@@ -2034,7 +2078,10 @@ int cg_create_everywhere(CGroupMask supported, CGroupMask mask, const char *path
20342078
return r;
20352079

20362080
/* If we are in the unified hierarchy, we are done now */
2037-
if (cg_all_unified())
2081+
r = cg_all_unified();
2082+
if (r < 0)
2083+
return r;
2084+
if (r > 0)
20382085
return 0;
20392086

20402087
/* Otherwise, do the same in the other hierarchies */
@@ -2061,7 +2108,10 @@ int cg_attach_everywhere(CGroupMask supported, const char *path, pid_t pid, cg_m
20612108
if (r < 0)
20622109
return r;
20632110

2064-
if (cg_all_unified())
2111+
r = cg_all_unified();
2112+
if (r < 0)
2113+
return r;
2114+
if (r > 0)
20652115
return 0;
20662116

20672117
for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) {
@@ -2102,15 +2152,18 @@ int cg_attach_many_everywhere(CGroupMask supported, const char *path, Set* pids,
21022152

21032153
int cg_migrate_everywhere(CGroupMask supported, const char *from, const char *to, cg_migrate_callback_t to_callback, void *userdata) {
21042154
CGroupController c;
2105-
int r = 0;
2155+
int r = 0, q;
21062156

21072157
if (!path_equal(from, to)) {
21082158
r = cg_migrate_recursive(SYSTEMD_CGROUP_CONTROLLER, from, SYSTEMD_CGROUP_CONTROLLER, to, CGROUP_REMOVE);
21092159
if (r < 0)
21102160
return r;
21112161
}
21122162

2113-
if (cg_all_unified())
2163+
q = cg_all_unified();
2164+
if (q < 0)
2165+
return q;
2166+
if (q > 0)
21142167
return r;
21152168

21162169
for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) {
@@ -2134,13 +2187,16 @@ int cg_migrate_everywhere(CGroupMask supported, const char *from, const char *to
21342187

21352188
int cg_trim_everywhere(CGroupMask supported, const char *path, bool delete_root) {
21362189
CGroupController c;
2137-
int r;
2190+
int r, q;
21382191

21392192
r = cg_trim(SYSTEMD_CGROUP_CONTROLLER, path, delete_root);
21402193
if (r < 0)
21412194
return r;
21422195

2143-
if (cg_all_unified())
2196+
q = cg_all_unified();
2197+
if (q < 0)
2198+
return q;
2199+
if (q > 0)
21442200
return r;
21452201

21462202
for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) {
@@ -2163,7 +2219,10 @@ int cg_mask_supported(CGroupMask *ret) {
21632219
* includes controllers we can make sense of and that are
21642220
* actually accessible. */
21652221

2166-
if (cg_all_unified()) {
2222+
r = cg_all_unified();
2223+
if (r < 0)
2224+
return r;
2225+
if (r > 0) {
21672226
_cleanup_free_ char *root = NULL, *controllers = NULL, *path = NULL;
21682227
const char *c;
21692228

@@ -2336,9 +2395,12 @@ static int cg_update_unified(void) {
23362395
return 0;
23372396
}
23382397

2339-
bool cg_unified(const char *controller) {
2398+
int cg_unified(const char *controller) {
2399+
int r;
23402400

2341-
assert(cg_update_unified() >= 0);
2401+
r = cg_update_unified();
2402+
if (r < 0)
2403+
return r;
23422404

23432405
if (unified_cache == CGROUP_UNIFIED_NONE)
23442406
return false;
@@ -2349,14 +2411,16 @@ bool cg_unified(const char *controller) {
23492411
return streq_ptr(controller, SYSTEMD_CGROUP_CONTROLLER);
23502412
}
23512413

2352-
bool cg_all_unified(void) {
2353-
2414+
int cg_all_unified(void) {
23542415
return cg_unified(NULL);
23552416
}
23562417

2357-
bool cg_hybrid_unified(void) {
2418+
int cg_hybrid_unified(void) {
2419+
int r;
23582420

2359-
assert(cg_update_unified() >= 0);
2421+
r = cg_update_unified();
2422+
if (r < 0)
2423+
return r;
23602424

23612425
return unified_cache == CGROUP_UNIFIED_SYSTEMD && !unified_systemd_v232;
23622426
}
@@ -2377,7 +2441,10 @@ int cg_enable_everywhere(CGroupMask supported, CGroupMask mask, const char *p) {
23772441
if (supported == 0)
23782442
return 0;
23792443

2380-
if (!cg_all_unified()) /* on the legacy hiearchy there's no joining of controllers defined */
2444+
r = cg_all_unified();
2445+
if (r < 0)
2446+
return r;
2447+
if (r == 0) /* on the legacy hiearchy there's no joining of controllers defined */
23812448
return 0;
23822449

23832450
r = cg_get_path(SYSTEMD_CGROUP_CONTROLLER, p, "cgroup.subtree_control", &fs);
@@ -2420,7 +2487,7 @@ bool cg_is_unified_wanted(void) {
24202487
/* If the hierarchy is already mounted, then follow whatever
24212488
* was chosen for it. */
24222489
if (cg_unified_flush() >= 0)
2423-
return (wanted = cg_all_unified());
2490+
return (wanted = unified_cache >= CGROUP_UNIFIED_ALL);
24242491

24252492
/* Otherwise, let's see what the kernel command line has to say.
24262493
* Since checking is expensive, cache a non-error result. */

‎src/basic/cgroup-util.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -240,9 +240,9 @@ int cg_kernel_controllers(Set *controllers);
240240

241241
bool cg_ns_supported(void);
242242

243-
bool cg_all_unified(void);
244-
bool cg_hybrid_unified(void);
245-
bool cg_unified(const char *controller);
243+
int cg_all_unified(void);
244+
int cg_hybrid_unified(void);
245+
int cg_unified(const char *controller);
246246
int cg_unified_flush(void);
247247

248248
bool cg_is_unified_wanted(void);

‎src/cgls/cgls.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ static int parse_argv(int argc, char *argv[]) {
158158

159159
static void show_cg_info(const char *controller, const char *path) {
160160

161-
if (!cg_all_unified() && controller && !streq(controller, SYSTEMD_CGROUP_CONTROLLER))
161+
if (cg_all_unified() == 0 && controller && !streq(controller, SYSTEMD_CGROUP_CONTROLLER))
162162
printf("Controller %s; ", controller);
163163

164164
printf("Control group %s:\n", isempty(path) ? "/" : path);

0 commit comments

Comments
 (0)
X Tutup