From 4e8b11ca16a94c2fd6e49e1585a941bb968031dc Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 30 May 2024 20:54:24 +0800 Subject: [PATCH 01/16] core/dbus-unit: use UNIT_IS_LOAD_ERROR where appropriate --- src/core/dbus-unit.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 707644d2f9..0161a1bf45 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -1807,9 +1807,7 @@ int bus_unit_queue_job_one( type = JOB_TRY_RELOAD; } - if (type == JOB_STOP && - IN_SET(u->load_state, UNIT_NOT_FOUND, UNIT_ERROR, UNIT_BAD_SETTING) && - unit_active_state(u) == UNIT_INACTIVE) + if (type == JOB_STOP && UNIT_IS_LOAD_ERROR(u->load_state) && unit_active_state(u) == UNIT_INACTIVE) return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_UNIT, "Unit %s not loaded.", u->id); if ((type == JOB_START && u->refuse_manual_start) || From e2f81f8d40c3e7d99c61e4642040ce4da562b7ac Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Fri, 24 May 2024 22:27:52 +0800 Subject: [PATCH 02/16] core/unit: drop pointless unit_freezer_state wrapper --- src/core/dbus-unit.c | 4 ++-- src/core/unit-serialize.c | 3 ++- src/core/unit.c | 6 ------ src/core/unit.h | 1 - 4 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 0161a1bf45..a9ae81aed5 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -34,9 +34,9 @@ static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_collect_mode, collect_mode, CollectMode); static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_load_state, unit_load_state, UnitLoadState); static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_job_mode, job_mode, JobMode); +static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_freezer_state, freezer_state, FreezerState); static BUS_DEFINE_PROPERTY_GET(property_get_description, "s", Unit, unit_description); static BUS_DEFINE_PROPERTY_GET2(property_get_active_state, "s", Unit, unit_active_state, unit_active_state_to_string); -static BUS_DEFINE_PROPERTY_GET2(property_get_freezer_state, "s", Unit, unit_freezer_state, freezer_state_to_string); static BUS_DEFINE_PROPERTY_GET(property_get_sub_state, "s", Unit, unit_sub_state_to_string); static BUS_DEFINE_PROPERTY_GET2(property_get_unit_file_state, "s", Unit, unit_get_unit_file_state, unit_file_state_to_string); static BUS_DEFINE_PROPERTY_GET(property_get_can_reload, "b", Unit, unit_can_reload); @@ -864,7 +864,7 @@ const sd_bus_vtable bus_unit_vtable[] = { SD_BUS_PROPERTY("AccessSELinuxContext", "s", NULL, offsetof(Unit, access_selinux_context), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("LoadState", "s", property_get_load_state, offsetof(Unit, load_state), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("ActiveState", "s", property_get_active_state, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), - SD_BUS_PROPERTY("FreezerState", "s", property_get_freezer_state, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), + SD_BUS_PROPERTY("FreezerState", "s", property_get_freezer_state, offsetof(Unit, freezer_state), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("SubState", "s", property_get_sub_state, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("FragmentPath", "s", NULL, offsetof(Unit, fragment_path), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("SourcePath", "s", NULL, offsetof(Unit, source_path), SD_BUS_VTABLE_PROPERTY_CONST), diff --git a/src/core/unit-serialize.c b/src/core/unit-serialize.c index 175e327d92..8baa30f9e5 100644 --- a/src/core/unit-serialize.c +++ b/src/core/unit-serialize.c @@ -117,7 +117,8 @@ int unit_serialize_state(Unit *u, FILE *f, FDSet *fds, bool switching_root) { if (!sd_id128_is_null(u->invocation_id)) (void) serialize_item_format(f, "invocation-id", SD_ID128_FORMAT_STR, SD_ID128_FORMAT_VAL(u->invocation_id)); - (void) serialize_item_format(f, "freezer-state", "%s", freezer_state_to_string(unit_freezer_state(u))); + (void) serialize_item(f, "freezer-state", freezer_state_to_string(u->freezer_state)); + (void) serialize_markers(f, u->markers); bus_track_serialize(u->bus_track, f, "ref"); diff --git a/src/core/unit.c b/src/core/unit.c index 8ac0aad2d7..8cdb3a025f 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -864,12 +864,6 @@ Unit* unit_free(Unit *u) { return mfree(u); } -FreezerState unit_freezer_state(Unit *u) { - assert(u); - - return u->freezer_state; -} - UnitActiveState unit_active_state(Unit *u) { assert(u); diff --git a/src/core/unit.h b/src/core/unit.h index 042ec3b3ea..c961edef5f 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -849,7 +849,6 @@ const char* unit_status_string(Unit *u, char **combined); bool unit_has_name(const Unit *u, const char *name); UnitActiveState unit_active_state(Unit *u); -FreezerState unit_freezer_state(Unit *u); const char* unit_sub_state_to_string(Unit *u); From 21fed6eaec011969c8550c6009c000e6e2e86852 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Fri, 24 May 2024 22:52:28 +0800 Subject: [PATCH 03/16] core,unit-def: use our usual way of asserting enums --- src/basic/unit-def.c | 4 +++- src/core/cgroup.c | 4 ++-- src/core/cgroup.h | 2 +- src/core/slice.c | 4 ++-- src/core/unit.c | 4 ++-- 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/basic/unit-def.c b/src/basic/unit-def.c index 6ffdcd37c8..4dc8ceef86 100644 --- a/src/basic/unit-def.c +++ b/src/basic/unit-def.c @@ -140,7 +140,9 @@ static const FreezerState freezer_state_finish_table[_FREEZER_STATE_MAX] = { }; FreezerState freezer_state_finish(FreezerState state) { - assert(state >= 0 && state < _FREEZER_STATE_MAX); + assert(state >= 0); + assert(state < _FREEZER_STATE_MAX); + return freezer_state_finish_table[state]; } diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 9620436f68..218f867f24 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -5110,8 +5110,8 @@ int unit_cgroup_freezer_action(Unit *u, FreezerAction action) { int r; assert(u); - assert(IN_SET(action, FREEZER_FREEZE, FREEZER_PARENT_FREEZE, - FREEZER_THAW, FREEZER_PARENT_THAW)); + assert(action >= 0); + assert(action < _FREEZER_ACTION_MAX); if (!cg_freezer_supported()) return 0; diff --git a/src/core/cgroup.h b/src/core/cgroup.h index e31e69364e..1c5c1f75ab 100644 --- a/src/core/cgroup.h +++ b/src/core/cgroup.h @@ -60,7 +60,6 @@ typedef enum FreezerAction { FREEZER_PARENT_FREEZE, FREEZER_THAW, FREEZER_PARENT_THAW, - _FREEZER_ACTION_MAX, _FREEZER_ACTION_INVALID = -EINVAL, } FreezerAction; @@ -514,6 +513,7 @@ void unit_cgroup_catchup(Unit *u); bool unit_cgroup_delegate(Unit *u); int unit_get_cpuset(Unit *u, CPUSet *cpus, const char *name); + int unit_cgroup_freezer_action(Unit *u, FreezerAction action); const char* freezer_action_to_string(FreezerAction a) _const_; diff --git a/src/core/slice.c b/src/core/slice.c index 4e71976e4f..d8c507804f 100644 --- a/src/core/slice.c +++ b/src/core/slice.c @@ -356,8 +356,8 @@ static int slice_freezer_action(Unit *s, FreezerAction action) { int r; assert(s); - assert(IN_SET(action, FREEZER_FREEZE, FREEZER_PARENT_FREEZE, - FREEZER_THAW, FREEZER_PARENT_THAW)); + assert(action >= 0); + assert(action < _FREEZER_ACTION_MAX); if (action == FREEZER_FREEZE && !slice_can_freeze(s)) { /* We're intentionally only checking for FREEZER_FREEZE here and ignoring the diff --git a/src/core/unit.c b/src/core/unit.c index 8cdb3a025f..01763d7560 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -6161,8 +6161,8 @@ void unit_next_freezer_state(Unit *u, FreezerAction action, FreezerState *ret, F FreezerState curr, parent, next, tgt; assert(u); - assert(IN_SET(action, FREEZER_FREEZE, FREEZER_PARENT_FREEZE, - FREEZER_THAW, FREEZER_PARENT_THAW)); + assert(action >= 0); + assert(action < _FREEZER_ACTION_MAX); assert(ret); assert(ret_target); From b21bebbe70313646ef16a7d812f8d8e092f77d99 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Fri, 24 May 2024 22:57:18 +0800 Subject: [PATCH 04/16] core: make unit_can_freeze take const Unit* --- src/core/slice.c | 10 +++++----- src/core/unit.c | 2 +- src/core/unit.h | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/core/slice.c b/src/core/slice.c index d8c507804f..12ecd1610d 100644 --- a/src/core/slice.c +++ b/src/core/slice.c @@ -339,14 +339,14 @@ static void slice_enumerate_perpetual(Manager *m) { (void) slice_make_perpetual(m, SPECIAL_SYSTEM_SLICE, NULL); } -static bool slice_can_freeze(Unit *s) { +static bool slice_can_freeze(const Unit *u) { + assert(u); + Unit *member; - - assert(s); - - UNIT_FOREACH_DEPENDENCY(member, s, UNIT_ATOM_SLICE_OF) + UNIT_FOREACH_DEPENDENCY(member, u, UNIT_ATOM_SLICE_OF) if (!unit_can_freeze(member)) return false; + return true; } diff --git a/src/core/unit.c b/src/core/unit.c index 01763d7560..3a83ed0878 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -6228,7 +6228,7 @@ void unit_next_freezer_state(Unit *u, FreezerAction action, FreezerState *ret, F *ret_target = tgt; } -bool unit_can_freeze(Unit *u) { +bool unit_can_freeze(const Unit *u) { assert(u); if (unit_has_name(u, SPECIAL_ROOT_SLICE) || unit_has_name(u, SPECIAL_INIT_SCOPE)) diff --git a/src/core/unit.h b/src/core/unit.h index c961edef5f..8150226510 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -577,7 +577,7 @@ typedef struct UnitVTable { /* Freeze or thaw the unit. Returns > 0 to indicate that the request will be handled asynchronously; unit_frozen * or unit_thawed should be called once the operation is done. Returns 0 if done successfully, or < 0 on error. */ int (*freezer_action)(Unit *u, FreezerAction a); - bool (*can_freeze)(Unit *u); + bool (*can_freeze)(const Unit *u); /* Return which kind of data can be cleaned */ int (*can_clean)(Unit *u, ExecCleanMask *ret); @@ -1036,7 +1036,7 @@ bool unit_can_start_refuse_manual(Unit *u); bool unit_can_stop_refuse_manual(Unit *u); bool unit_can_isolate_refuse_manual(Unit *u); -bool unit_can_freeze(Unit *u); +bool unit_can_freeze(const Unit *u); int unit_freezer_action(Unit *u, FreezerAction action); void unit_next_freezer_state(Unit *u, FreezerAction a, FreezerState *ret, FreezerState *ret_tgt); void unit_frozen(Unit *u); From f375a9686f6286eba9de20e0a0d9077960ccdc90 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 25 May 2024 18:45:31 +0800 Subject: [PATCH 05/16] core/slice: simplify slice_freezer_action a bit --- src/core/slice.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/core/slice.c b/src/core/slice.c index 12ecd1610d..d9ba4a83a0 100644 --- a/src/core/slice.c +++ b/src/core/slice.c @@ -352,7 +352,6 @@ static bool slice_can_freeze(const Unit *u) { static int slice_freezer_action(Unit *s, FreezerAction action) { FreezerAction child_action; - Unit *member; int r; assert(s); @@ -364,7 +363,7 @@ static int slice_freezer_action(Unit *s, FreezerAction action) { * _BY_PARENT variant. If we're being frozen by parent, that means someone has * already checked if we can be frozen further up the call stack. No point to * redo that work */ - log_unit_warning(s, "Requested freezer operation is not supported by all children of the slice"); + log_unit_warning(s, "Requested freezer operation is not supported by all children of the slice."); return 0; } @@ -375,15 +374,13 @@ static int slice_freezer_action(Unit *s, FreezerAction action) { else child_action = action; - UNIT_FOREACH_DEPENDENCY(member, s, UNIT_ATOM_SLICE_OF) { - if (UNIT_VTABLE(member)->freezer_action) + Unit *member; + UNIT_FOREACH_DEPENDENCY(member, s, UNIT_ATOM_SLICE_OF) + if (UNIT_VTABLE(member)->freezer_action) { r = UNIT_VTABLE(member)->freezer_action(member, child_action); - else - /* Only thawing will reach here, since freezing checks for a method in can_freeze */ - r = 0; - if (r < 0) - return r; - } + if (r < 0) + return r; + } return unit_cgroup_freezer_action(s, action); } From f27f461b01926f08e9d1e88833b69b9b2ba4995c Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 25 May 2024 18:44:51 +0800 Subject: [PATCH 06/16] core/unit: rename freezer "target" to "objective" --- src/core/cgroup.c | 12 ++++++------ src/core/unit.c | 28 ++++++++++++++-------------- src/core/unit.h | 2 +- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 218f867f24..e370a04b8f 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -5106,7 +5106,7 @@ static int unit_cgroup_freezer_kernel_state(Unit *u, FreezerState *ret) { int unit_cgroup_freezer_action(Unit *u, FreezerAction action) { _cleanup_free_ char *path = NULL; - FreezerState target, current, next; + FreezerState current, next, objective; int r; assert(u); @@ -5116,7 +5116,7 @@ int unit_cgroup_freezer_action(Unit *u, FreezerAction action) { if (!cg_freezer_supported()) return 0; - unit_next_freezer_state(u, action, &next, &target); + unit_next_freezer_state(u, action, &next, &objective); CGroupRuntime *crt = unit_get_cgroup_runtime(u); if (!crt || !crt->cgroup_path) { @@ -5129,11 +5129,11 @@ int unit_cgroup_freezer_action(Unit *u, FreezerAction action) { if (r < 0) return r; - if (current == target) + if (current == objective) next = freezer_state_finish(next); else if (IN_SET(next, FREEZER_FROZEN, FREEZER_FROZEN_BY_PARENT, FREEZER_RUNNING)) { /* We're transitioning into a finished state, which implies that the cgroup's - * current state already matches the target and thus we'd return 0. But, reality + * current state already matches the objective and thus we'd return 0. But, reality * shows otherwise. This indicates that our freezer_state tracking has diverged * from the real state of the cgroup, which can happen if someone meddles with the * cgroup from underneath us. This really shouldn't happen during normal operation, @@ -5158,12 +5158,12 @@ int unit_cgroup_freezer_action(Unit *u, FreezerAction action) { freezer_state_to_string(u->freezer_state), freezer_state_to_string(next)); - r = write_string_file(path, one_zero(target == FREEZER_FROZEN), WRITE_STRING_FILE_DISABLE_BUFFER); + r = write_string_file(path, one_zero(objective == FREEZER_FROZEN), WRITE_STRING_FILE_DISABLE_BUFFER); if (r < 0) return r; u->freezer_state = next; - return target != current; + return current != objective; } int unit_get_cpuset(Unit *u, CPUSet *cpus, const char *name) { diff --git a/src/core/unit.c b/src/core/unit.c index 3a83ed0878..ced23a83b2 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -6156,26 +6156,26 @@ bool unit_can_isolate_refuse_manual(Unit *u) { return unit_can_isolate(u) && !u->refuse_manual_start; } -void unit_next_freezer_state(Unit *u, FreezerAction action, FreezerState *ret, FreezerState *ret_target) { - Unit *slice; - FreezerState curr, parent, next, tgt; +void unit_next_freezer_state(Unit *u, FreezerAction action, FreezerState *ret, FreezerState *ret_objective) { + FreezerState curr, parent, next, objective; assert(u); assert(action >= 0); assert(action < _FREEZER_ACTION_MAX); assert(ret); - assert(ret_target); + assert(ret_objective); /* This function determines the correct freezer state transitions for a unit - * given the action being requested. It returns the next state, and also the "target", + * given the action being requested. It returns the next state, and also the "objective", * which is either FREEZER_FROZEN or FREEZER_RUNNING, depending on what actual state we * ultimately want to achieve. */ - curr = u->freezer_state; - slice = UNIT_GET_SLICE(u); - if (slice) + curr = u->freezer_state; + + Unit *slice = UNIT_GET_SLICE(u); + if (slice) parent = slice->freezer_state; - else + else parent = FREEZER_RUNNING; if (action == FREEZER_FREEZE) { @@ -6219,13 +6219,13 @@ void unit_next_freezer_state(Unit *u, FreezerAction action, FreezerState *ret, F next = FREEZER_THAWING; } - tgt = freezer_state_finish(next); - if (tgt == FREEZER_FROZEN_BY_PARENT) - tgt = FREEZER_FROZEN; - assert(IN_SET(tgt, FREEZER_RUNNING, FREEZER_FROZEN)); + objective = freezer_state_finish(next); + if (objective == FREEZER_FROZEN_BY_PARENT) + objective = FREEZER_FROZEN; + assert(IN_SET(objective, FREEZER_RUNNING, FREEZER_FROZEN)); *ret = next; - *ret_target = tgt; + *ret_objective = objective; } bool unit_can_freeze(const Unit *u) { diff --git a/src/core/unit.h b/src/core/unit.h index 8150226510..a55754ad48 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -1038,7 +1038,7 @@ bool unit_can_isolate_refuse_manual(Unit *u); bool unit_can_freeze(const Unit *u); int unit_freezer_action(Unit *u, FreezerAction action); -void unit_next_freezer_state(Unit *u, FreezerAction a, FreezerState *ret, FreezerState *ret_tgt); +void unit_next_freezer_state(Unit *u, FreezerAction a, FreezerState *ret, FreezerState *ret_objective); void unit_frozen(Unit *u); void unit_thawed(Unit *u); From aba5e4660fe12679bec5cd598b9a49a180b83f43 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 25 May 2024 18:46:23 +0800 Subject: [PATCH 07/16] core/unit: use switch for unit_next_freezer_state --- src/core/unit.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/src/core/unit.c b/src/core/unit.c index ced23a83b2..241f09bf5a 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -6178,45 +6178,56 @@ void unit_next_freezer_state(Unit *u, FreezerAction action, FreezerState *ret, F else parent = FREEZER_RUNNING; - if (action == FREEZER_FREEZE) { + switch (action) { + + case FREEZER_FREEZE: /* We always "promote" a freeze initiated by parent into a normal freeze */ if (IN_SET(curr, FREEZER_FROZEN, FREEZER_FROZEN_BY_PARENT)) next = FREEZER_FROZEN; else next = FREEZER_FREEZING; - } else if (action == FREEZER_THAW) { + break; + + case FREEZER_THAW: /* Thawing is the most complicated operation here, because we can't thaw a unit * if its parent is frozen. So we instead "demote" a normal freeze into a freeze * initiated by parent if the parent is frozen */ - if (IN_SET(curr, FREEZER_RUNNING, FREEZER_THAWING, FREEZER_FREEZING_BY_PARENT, FREEZER_FROZEN_BY_PARENT)) + if (IN_SET(curr, FREEZER_RUNNING, FREEZER_THAWING, + FREEZER_FREEZING_BY_PARENT, FREEZER_FROZEN_BY_PARENT)) /* Should usually be refused by unit_freezer_action */ next = curr; else if (curr == FREEZER_FREEZING) { if (IN_SET(parent, FREEZER_RUNNING, FREEZER_THAWING)) next = FREEZER_THAWING; else next = FREEZER_FREEZING_BY_PARENT; - } else { - assert(curr == FREEZER_FROZEN); + } else if (curr == FREEZER_FROZEN) { if (IN_SET(parent, FREEZER_RUNNING, FREEZER_THAWING)) next = FREEZER_THAWING; else next = FREEZER_FROZEN_BY_PARENT; - } - } else if (action == FREEZER_PARENT_FREEZE) { + } else + assert_not_reached(); + break; + + case FREEZER_PARENT_FREEZE: /* We need to avoid accidentally demoting units frozen manually */ if (IN_SET(curr, FREEZER_FREEZING, FREEZER_FROZEN, FREEZER_FROZEN_BY_PARENT)) next = curr; else next = FREEZER_FREEZING_BY_PARENT; - } else { - assert(action == FREEZER_PARENT_THAW); + break; + case FREEZER_PARENT_THAW: /* We don't want to thaw units from a parent if they were frozen * manually, so for such units this action is a no-op */ if (IN_SET(curr, FREEZER_RUNNING, FREEZER_FREEZING, FREEZER_FROZEN)) next = curr; else next = FREEZER_THAWING; + break; + + default: + assert_not_reached(); } objective = freezer_state_finish(next); From 54e1f676a2d3fa8c39beac84fe5027d9276d491d Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 17 Jul 2024 17:34:16 +0200 Subject: [PATCH 08/16] core/unit: rename a few more vars for unit_next_freezer_state() --- src/core/unit.c | 28 ++++++++++++++-------------- src/core/unit.h | 2 +- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/core/unit.c b/src/core/unit.c index 241f09bf5a..999e690a32 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -6156,13 +6156,13 @@ bool unit_can_isolate_refuse_manual(Unit *u) { return unit_can_isolate(u) && !u->refuse_manual_start; } -void unit_next_freezer_state(Unit *u, FreezerAction action, FreezerState *ret, FreezerState *ret_objective) { - FreezerState curr, parent, next, objective; +void unit_next_freezer_state(Unit *u, FreezerAction action, FreezerState *ret_next, FreezerState *ret_objective) { + FreezerState current, parent, next, objective; assert(u); assert(action >= 0); assert(action < _FREEZER_ACTION_MAX); - assert(ret); + assert(ret_next); assert(ret_objective); /* This function determines the correct freezer state transitions for a unit @@ -6170,7 +6170,7 @@ void unit_next_freezer_state(Unit *u, FreezerAction action, FreezerState *ret, F * which is either FREEZER_FROZEN or FREEZER_RUNNING, depending on what actual state we * ultimately want to achieve. */ - curr = u->freezer_state; + current = u->freezer_state; Unit *slice = UNIT_GET_SLICE(u); if (slice) @@ -6182,7 +6182,7 @@ void unit_next_freezer_state(Unit *u, FreezerAction action, FreezerState *ret, F case FREEZER_FREEZE: /* We always "promote" a freeze initiated by parent into a normal freeze */ - if (IN_SET(curr, FREEZER_FROZEN, FREEZER_FROZEN_BY_PARENT)) + if (IN_SET(current, FREEZER_FROZEN, FREEZER_FROZEN_BY_PARENT)) next = FREEZER_FROZEN; else next = FREEZER_FREEZING; @@ -6192,15 +6192,15 @@ void unit_next_freezer_state(Unit *u, FreezerAction action, FreezerState *ret, F /* Thawing is the most complicated operation here, because we can't thaw a unit * if its parent is frozen. So we instead "demote" a normal freeze into a freeze * initiated by parent if the parent is frozen */ - if (IN_SET(curr, FREEZER_RUNNING, FREEZER_THAWING, + if (IN_SET(current, FREEZER_RUNNING, FREEZER_THAWING, FREEZER_FREEZING_BY_PARENT, FREEZER_FROZEN_BY_PARENT)) /* Should usually be refused by unit_freezer_action */ - next = curr; - else if (curr == FREEZER_FREEZING) { + next = current; + else if (current == FREEZER_FREEZING) { if (IN_SET(parent, FREEZER_RUNNING, FREEZER_THAWING)) next = FREEZER_THAWING; else next = FREEZER_FREEZING_BY_PARENT; - } else if (curr == FREEZER_FROZEN) { + } else if (current == FREEZER_FROZEN) { if (IN_SET(parent, FREEZER_RUNNING, FREEZER_THAWING)) next = FREEZER_THAWING; else @@ -6211,8 +6211,8 @@ void unit_next_freezer_state(Unit *u, FreezerAction action, FreezerState *ret, F case FREEZER_PARENT_FREEZE: /* We need to avoid accidentally demoting units frozen manually */ - if (IN_SET(curr, FREEZER_FREEZING, FREEZER_FROZEN, FREEZER_FROZEN_BY_PARENT)) - next = curr; + if (IN_SET(current, FREEZER_FREEZING, FREEZER_FROZEN, FREEZER_FROZEN_BY_PARENT)) + next = current; else next = FREEZER_FREEZING_BY_PARENT; break; @@ -6220,8 +6220,8 @@ void unit_next_freezer_state(Unit *u, FreezerAction action, FreezerState *ret, F case FREEZER_PARENT_THAW: /* We don't want to thaw units from a parent if they were frozen * manually, so for such units this action is a no-op */ - if (IN_SET(curr, FREEZER_RUNNING, FREEZER_FREEZING, FREEZER_FROZEN)) - next = curr; + if (IN_SET(current, FREEZER_RUNNING, FREEZER_FREEZING, FREEZER_FROZEN)) + next = current; else next = FREEZER_THAWING; break; @@ -6235,7 +6235,7 @@ void unit_next_freezer_state(Unit *u, FreezerAction action, FreezerState *ret, F objective = FREEZER_FROZEN; assert(IN_SET(objective, FREEZER_RUNNING, FREEZER_FROZEN)); - *ret = next; + *ret_next = next; *ret_objective = objective; } diff --git a/src/core/unit.h b/src/core/unit.h index a55754ad48..9c8dbcf12a 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -1038,7 +1038,7 @@ bool unit_can_isolate_refuse_manual(Unit *u); bool unit_can_freeze(const Unit *u); int unit_freezer_action(Unit *u, FreezerAction action); -void unit_next_freezer_state(Unit *u, FreezerAction a, FreezerState *ret, FreezerState *ret_objective); +void unit_next_freezer_state(Unit *u, FreezerAction action, FreezerState *ret_next, FreezerState *ret_objective); void unit_frozen(Unit *u); void unit_thawed(Unit *u); From 27344f9acf7b23225020cc4a2d63d5e10d35308f Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 15 Jul 2024 20:32:47 +0200 Subject: [PATCH 09/16] core/cgroup: replace hardcoded state set with freezer_state_finish() This makes code simpler and more readable. --- src/core/cgroup.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index e370a04b8f..8abfd31c7b 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -5131,10 +5131,11 @@ int unit_cgroup_freezer_action(Unit *u, FreezerAction action) { if (current == objective) next = freezer_state_finish(next); - else if (IN_SET(next, FREEZER_FROZEN, FREEZER_FROZEN_BY_PARENT, FREEZER_RUNNING)) { - /* We're transitioning into a finished state, which implies that the cgroup's - * current state already matches the objective and thus we'd return 0. But, reality - * shows otherwise. This indicates that our freezer_state tracking has diverged + else if (next == freezer_state_finish(next)) { + /* We're directly transitioning into a finished state, which in theory means that + * the cgroup's current state already matches the objective and thus we'd return 0. + * But, reality shows otherwise (such case would have been handled by current == objective + * branch above). This indicates that our freezer_state tracking has diverged * from the real state of the cgroup, which can happen if someone meddles with the * cgroup from underneath us. This really shouldn't happen during normal operation, * though. So, let's warn about it and fix up the state to be valid */ From a9dc19617943d1db4f137005f5e467188c66e5a9 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 25 May 2024 18:46:55 +0800 Subject: [PATCH 10/16] core/cgroup: skip freezer action wholly if current == objective --- src/core/cgroup.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 8abfd31c7b..a8bdd0dd0d 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -5119,19 +5119,18 @@ int unit_cgroup_freezer_action(Unit *u, FreezerAction action) { unit_next_freezer_state(u, action, &next, &objective); CGroupRuntime *crt = unit_get_cgroup_runtime(u); - if (!crt || !crt->cgroup_path) { + if (!crt || !crt->cgroup_path) /* No realized cgroup = nothing to freeze */ - u->freezer_state = freezer_state_finish(next); - return 0; - } + goto skip; r = unit_cgroup_freezer_kernel_state(u, ¤t); if (r < 0) return r; if (current == objective) - next = freezer_state_finish(next); - else if (next == freezer_state_finish(next)) { + goto skip; + + if (next == freezer_state_finish(next)) { /* We're directly transitioning into a finished state, which in theory means that * the cgroup's current state already matches the objective and thus we'd return 0. * But, reality shows otherwise (such case would have been handled by current == objective @@ -5149,6 +5148,8 @@ int unit_cgroup_freezer_action(Unit *u, FreezerAction action) { next = FREEZER_FREEZING_BY_PARENT; else if (next == FREEZER_RUNNING) next = FREEZER_THAWING; + else + assert_not_reached(); } r = cg_get_path(SYSTEMD_CGROUP_CONTROLLER, crt->cgroup_path, "cgroup.freeze", &path); @@ -5164,7 +5165,11 @@ int unit_cgroup_freezer_action(Unit *u, FreezerAction action) { return r; u->freezer_state = next; - return current != objective; + return 1; /* Wait for cgroup event before replying */ + +skip: + u->freezer_state = freezer_state_finish(next); + return 0; } int unit_get_cpuset(Unit *u, CPUSet *cpus, const char *name) { From 0064290a54fe46047889d44b3215d04b60e77c17 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Fri, 31 May 2024 20:46:16 +0800 Subject: [PATCH 11/16] core/unit: introduce unit_set_freezer_state, make logging consistent Also, emit PropertiesChanged signal for FreezerState too. Fixes #31115 --- src/core/cgroup.c | 8 ++------ src/core/unit.c | 16 ++++++++++++++++ src/core/unit.h | 1 + 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index a8bdd0dd0d..98f33fe580 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -5156,19 +5156,15 @@ int unit_cgroup_freezer_action(Unit *u, FreezerAction action) { if (r < 0) return r; - log_unit_debug(u, "Unit freezer state was %s, now %s.", - freezer_state_to_string(u->freezer_state), - freezer_state_to_string(next)); - r = write_string_file(path, one_zero(objective == FREEZER_FROZEN), WRITE_STRING_FILE_DISABLE_BUFFER); if (r < 0) return r; - u->freezer_state = next; + unit_set_freezer_state(u, next); return 1; /* Wait for cgroup event before replying */ skip: - u->freezer_state = freezer_state_finish(next); + unit_set_freezer_state(u, freezer_state_finish(next)); return 0; } diff --git a/src/core/unit.c b/src/core/unit.c index 999e690a32..01f2ca189c 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -6251,6 +6251,22 @@ bool unit_can_freeze(const Unit *u) { return UNIT_VTABLE(u)->freezer_action; } +void unit_set_freezer_state(Unit *u, FreezerState state) { + assert(u); + assert(state >= 0); + assert(state < _FREEZER_STATE_MAX); + + if (u->freezer_state == state) + return; + + log_unit_debug(u, "Freezer state changed %s -> %s", + freezer_state_to_string(u->freezer_state), freezer_state_to_string(state)); + + u->freezer_state = state; + + unit_add_to_dbus_queue(u); +} + void unit_frozen(Unit *u) { assert(u); diff --git a/src/core/unit.h b/src/core/unit.h index 9c8dbcf12a..2003b6a779 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -1039,6 +1039,7 @@ bool unit_can_isolate_refuse_manual(Unit *u); bool unit_can_freeze(const Unit *u); int unit_freezer_action(Unit *u, FreezerAction action); void unit_next_freezer_state(Unit *u, FreezerAction action, FreezerState *ret_next, FreezerState *ret_objective); +void unit_set_freezer_state(Unit *u, FreezerState state); void unit_frozen(Unit *u); void unit_thawed(Unit *u); From e1ac52594d3c9bbe8b5d58d5ebb19db3b4e612a8 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Fri, 31 May 2024 20:43:53 +0800 Subject: [PATCH 12/16] core/unit: introduce unit_freezer_complete, correctly report end state --- src/core/cgroup.c | 8 ++------ src/core/unit.c | 26 ++++++++++---------------- src/core/unit.h | 3 +-- 3 files changed, 13 insertions(+), 24 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 98f33fe580..e9eb6f96e8 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -4044,12 +4044,8 @@ static int unit_check_cgroup_events(Unit *u) { /* Disregard freezer state changes due to operations not initiated by us. * See: https://github.com/systemd/systemd/pull/13512/files#r416469963 and * https://github.com/systemd/systemd/pull/13512#issuecomment-573007207 */ - if (values[1] && IN_SET(u->freezer_state, FREEZER_FREEZING, FREEZER_FREEZING_BY_PARENT, FREEZER_THAWING)) { - if (streq(values[1], "0")) - unit_thawed(u); - else - unit_frozen(u); - } + if (values[1] && IN_SET(u->freezer_state, FREEZER_FREEZING, FREEZER_FREEZING_BY_PARENT, FREEZER_THAWING)) + unit_freezer_complete(u, streq(values[1], "0") ? FREEZER_RUNNING : FREEZER_FROZEN); free(values[0]); free(values[1]); diff --git a/src/core/unit.c b/src/core/unit.c index 01f2ca189c..2ef2a1fbe8 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -6267,26 +6267,20 @@ void unit_set_freezer_state(Unit *u, FreezerState state) { unit_add_to_dbus_queue(u); } -void unit_frozen(Unit *u) { +void unit_freezer_complete(Unit *u, FreezerState kernel_state) { + bool expected; + assert(u); + assert(IN_SET(kernel_state, FREEZER_RUNNING, FREEZER_FROZEN)); - u->freezer_state = u->freezer_state == FREEZER_FREEZING_BY_PARENT - ? FREEZER_FROZEN_BY_PARENT - : FREEZER_FROZEN; + expected = IN_SET(u->freezer_state, FREEZER_RUNNING, FREEZER_THAWING) == (kernel_state == FREEZER_RUNNING); - log_unit_debug(u, "Unit now %s.", freezer_state_to_string(u->freezer_state)); + unit_set_freezer_state(u, expected ? freezer_state_finish(u->freezer_state) : kernel_state); + log_unit_info(u, "Unit now %s.", u->freezer_state == FREEZER_RUNNING ? "thawed" : + freezer_state_to_string(u->freezer_state)); - bus_unit_send_pending_freezer_message(u, false); -} - -void unit_thawed(Unit *u) { - assert(u); - - u->freezer_state = FREEZER_RUNNING; - - log_unit_debug(u, "Unit thawed."); - - bus_unit_send_pending_freezer_message(u, false); + /* If the cgroup's final state is against what's requested by us, report as canceled. */ + bus_unit_send_pending_freezer_message(u, /* canceled = */ !expected); } int unit_freezer_action(Unit *u, FreezerAction action) { diff --git a/src/core/unit.h b/src/core/unit.h index 2003b6a779..31a1a13df1 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -1040,8 +1040,7 @@ bool unit_can_freeze(const Unit *u); int unit_freezer_action(Unit *u, FreezerAction action); void unit_next_freezer_state(Unit *u, FreezerAction action, FreezerState *ret_next, FreezerState *ret_objective); void unit_set_freezer_state(Unit *u, FreezerState state); -void unit_frozen(Unit *u); -void unit_thawed(Unit *u); +void unit_freezer_complete(Unit *u, FreezerState kernel_state); Condition *unit_find_failed_condition(Unit *u); From 6798fa11d2888f95907028d7bc43defa05102c74 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Tue, 11 Jun 2024 19:09:05 +0200 Subject: [PATCH 13/16] core/dbus-unit: add an explicit bus error when unit is frozen by parent While at it, use more accurate errno (EDEADLK) instead of ECHILD. --- src/core/dbus-unit.c | 4 ++-- src/core/unit.c | 2 +- src/libsystemd/sd-bus/bus-common-errors.c | 1 + src/libsystemd/sd-bus/bus-common-errors.h | 1 + 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index a9ae81aed5..a51d1da3df 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -755,8 +755,8 @@ static int bus_unit_method_freezer_generic(sd_bus_message *message, void *userda return sd_bus_error_set(error, BUS_ERROR_UNIT_INACTIVE, "Unit is not active"); if (r == -EALREADY) return sd_bus_error_set(error, BUS_ERROR_UNIT_BUSY, "Previously requested freezer operation for unit is still in progress"); - if (r == -ECHILD) - return sd_bus_error_set(error, SD_BUS_ERROR_FAILED, "Unit is frozen by a parent slice"); + if (r == -EDEADLK) + return sd_bus_error_set(error, BUS_ERROR_FROZEN_BY_PARENT, "Unit is frozen by a parent slice"); if (r < 0) return r; diff --git a/src/core/unit.c b/src/core/unit.c index 2ef2a1fbe8..c281a29391 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -6308,7 +6308,7 @@ int unit_freezer_action(Unit *u, FreezerAction action) { if (action == FREEZER_THAW && u->freezer_state == FREEZER_THAWING) return -EALREADY; if (action == FREEZER_THAW && IN_SET(u->freezer_state, FREEZER_FREEZING_BY_PARENT, FREEZER_FROZEN_BY_PARENT)) - return -ECHILD; + return -EDEADLK; r = UNIT_VTABLE(u)->freezer_action(u, action); if (r <= 0) diff --git a/src/libsystemd/sd-bus/bus-common-errors.c b/src/libsystemd/sd-bus/bus-common-errors.c index a4b54f6619..5b18241f00 100644 --- a/src/libsystemd/sd-bus/bus-common-errors.c +++ b/src/libsystemd/sd-bus/bus-common-errors.c @@ -34,6 +34,7 @@ BUS_ERROR_MAP_ELF_REGISTER const sd_bus_error_map bus_common_errors[] = { SD_BUS_ERROR_MAP(BUS_ERROR_DISK_FULL, ENOSPC), SD_BUS_ERROR_MAP(BUS_ERROR_FILE_DESCRIPTOR_STORE_DISABLED, EHOSTDOWN), + SD_BUS_ERROR_MAP(BUS_ERROR_FROZEN_BY_PARENT, EDEADLK), SD_BUS_ERROR_MAP(BUS_ERROR_NO_SUCH_MACHINE, ENXIO), SD_BUS_ERROR_MAP(BUS_ERROR_NO_SUCH_IMAGE, ENOENT), diff --git a/src/libsystemd/sd-bus/bus-common-errors.h b/src/libsystemd/sd-bus/bus-common-errors.h index 4ef42af7a9..fb1d421168 100644 --- a/src/libsystemd/sd-bus/bus-common-errors.h +++ b/src/libsystemd/sd-bus/bus-common-errors.h @@ -34,6 +34,7 @@ #define BUS_ERROR_FREEZE_CANCELLED "org.freedesktop.systemd1.FreezeCancelled" #define BUS_ERROR_FILE_DESCRIPTOR_STORE_DISABLED \ "org.freedesktop.systemd1.FileDescriptorStoreDisabled" +#define BUS_ERROR_FROZEN_BY_PARENT "org.freedesktop.systemd1.FrozenByParent" #define BUS_ERROR_NO_SUCH_MACHINE "org.freedesktop.machine1.NoSuchMachine" #define BUS_ERROR_NO_SUCH_IMAGE "org.freedesktop.machine1.NoSuchImage" From a99935557a664d6e70dc03944216834c224532b1 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Fri, 31 May 2024 12:18:44 +0800 Subject: [PATCH 14/16] sleep: explicitly list valid sleep operations in switch To follow our usual coding style. --- src/sleep/sleep.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index d8491ca7b3..88fb5ecab2 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -631,10 +631,13 @@ static int run(int argc, char *argv[]) { break; - default: + case SLEEP_SUSPEND: + case SLEEP_HIBERNATE: r = execute(sleep_config, arg_operation, NULL); break; + default: + assert_not_reached(); } if (user_slice_freezer) From 1b5caddfeeaee678d5568db38faaae82a922e19e Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Tue, 11 Jun 2024 18:17:01 +0200 Subject: [PATCH 15/16] sleep: also log about errno when getenv_bool fails --- src/sleep/sleep.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index 88fb5ecab2..7e5075a63c 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -603,7 +603,7 @@ static int run(int argc, char *argv[]) { /* Freeze the user sessions */ r = getenv_bool("SYSTEMD_SLEEP_FREEZE_USER_SESSIONS"); if (r < 0 && r != -ENXIO) - log_warning_errno(r, "Cannot parse value of $SYSTEMD_SLEEP_FREEZE_USER_SESSIONS, ignoring."); + log_warning_errno(r, "Cannot parse value of $SYSTEMD_SLEEP_FREEZE_USER_SESSIONS, ignoring: %m"); if (r != 0) (void) unit_freezer_new_freeze(SPECIAL_USER_SLICE, &user_slice_freezer); else From b1ed7e6749541f03c2a35f05a91c5d950908e71f Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Tue, 11 Jun 2024 16:00:22 +0200 Subject: [PATCH 16/16] sleep,home: always initialize UnitFreezer if used Previously, unit_freezer_new_freeze() would only return UnitFreezer object if FreezeUnit() succeeds. This is not ideal though, as a failed bus call doesn't mean the action actually failed. E.g. a timeout might occur because pid1 is waiting for cgroup event from kernel, while the bus call timeout was exceeded (#33269). In such a case, ThawUnit() will never be called, resulting in frozen units remain that way after resuming from sleep. Therefore, let's get rid of unit_freezer_new_freeze(), and make sure as long as unit freezer is involved, we'll call ThawUnit() when we're done. This should make things a lot more robust. --- src/home/homework.c | 22 +++++++++++----------- src/shared/bus-unit-util.c | 19 ------------------- src/shared/bus-unit-util.h | 2 -- src/sleep/sleep.c | 10 +++++++--- 4 files changed, 18 insertions(+), 35 deletions(-) diff --git a/src/home/homework.c b/src/home/homework.c index 2537e8d1dd..00e74894b3 100644 --- a/src/home/homework.c +++ b/src/home/homework.c @@ -1870,7 +1870,7 @@ static int home_inspect(UserRecord *h, UserRecord **ret_home) { return 1; } -static int user_session_freezer(uid_t uid, bool freeze_now, UnitFreezer **ret) { +static int user_session_freezer_new(uid_t uid, UnitFreezer **ret) { _cleanup_free_ char *unit = NULL; int r; @@ -1881,10 +1881,6 @@ static int user_session_freezer(uid_t uid, bool freeze_now, UnitFreezer **ret) { if (r < 0 && r != -ENXIO) log_warning_errno(r, "Cannot parse value of $SYSTEMD_HOME_LOCK_FREEZE_SESSION, ignoring: %m"); else if (r == 0) { - if (freeze_now) - log_notice("Session remains unfrozen on explicit request ($SYSTEMD_HOME_LOCK_FREEZE_SESSION=0).\n" - "This is not recommended, and might result in unexpected behavior including data loss!"); - *ret = NULL; return 0; } @@ -1892,10 +1888,7 @@ static int user_session_freezer(uid_t uid, bool freeze_now, UnitFreezer **ret) { if (asprintf(&unit, "user-" UID_FMT ".slice", uid) < 0) return log_oom(); - if (freeze_now) - r = unit_freezer_new_freeze(unit, ret); - else - r = unit_freezer_new(unit, ret); + r = unit_freezer_new(unit, ret); if (r < 0) return r; @@ -1921,9 +1914,16 @@ static int home_lock(UserRecord *h) { _cleanup_(unit_freezer_freep) UnitFreezer *f = NULL; - r = user_session_freezer(h->uid, /* freeze_now= */ true, &f); + r = user_session_freezer_new(h->uid, &f); if (r < 0) return r; + if (r > 0) { + r = unit_freezer_freeze(f); + if (r < 0) + return r; + } else + log_notice("Session remains unfrozen on explicit request ($SYSTEMD_HOME_LOCK_FREEZE_SESSION=0).\n" + "This is not recommended, and might result in unexpected behavior including data loss!"); r = home_lock_luks(h, &setup); if (r < 0) { @@ -1966,7 +1966,7 @@ static int home_unlock(UserRecord *h) { _cleanup_(unit_freezer_freep) UnitFreezer *f = NULL; /* We want to thaw the session only after it's safe to access $HOME */ - r = user_session_freezer(h->uid, /* freeze_now= */ false, &f); + r = user_session_freezer_new(h->uid, &f); if (r > 0) r = unit_freezer_thaw(f); if (r < 0) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 751cb29c62..259fbeaf5c 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -3025,22 +3025,3 @@ int unit_freezer_freeze(UnitFreezer *f) { int unit_freezer_thaw(UnitFreezer *f) { return unit_freezer_action(f, false); } - -int unit_freezer_new_freeze(const char *name, UnitFreezer **ret) { - _cleanup_(unit_freezer_freep) UnitFreezer *f = NULL; - int r; - - assert(name); - assert(ret); - - r = unit_freezer_new(name, &f); - if (r < 0) - return r; - - r = unit_freezer_freeze(f); - if (r < 0) - return r; - - *ret = TAKE_PTR(f); - return 0; -} diff --git a/src/shared/bus-unit-util.h b/src/shared/bus-unit-util.h index ea4056c691..6f0d55971c 100644 --- a/src/shared/bus-unit-util.h +++ b/src/shared/bus-unit-util.h @@ -45,5 +45,3 @@ int unit_freezer_new(const char *name, UnitFreezer **ret); int unit_freezer_freeze(UnitFreezer *f); int unit_freezer_thaw(UnitFreezer *f); - -int unit_freezer_new_freeze(const char *name, UnitFreezer **ret); diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index 7e5075a63c..15ef7ae913 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -604,9 +604,13 @@ static int run(int argc, char *argv[]) { r = getenv_bool("SYSTEMD_SLEEP_FREEZE_USER_SESSIONS"); if (r < 0 && r != -ENXIO) log_warning_errno(r, "Cannot parse value of $SYSTEMD_SLEEP_FREEZE_USER_SESSIONS, ignoring: %m"); - if (r != 0) - (void) unit_freezer_new_freeze(SPECIAL_USER_SLICE, &user_slice_freezer); - else + if (r != 0) { + r = unit_freezer_new(SPECIAL_USER_SLICE, &user_slice_freezer); + if (r < 0) + return r; + + (void) unit_freezer_freeze(user_slice_freezer); + } else log_notice("User sessions remain unfrozen on explicit request ($SYSTEMD_SLEEP_FREEZE_USER_SESSIONS=0).\n" "This is not recommended, and might result in unexpected behavior, particularly\n" "in suspend-then-hibernate operations or setups with encrypted home directories.");