Fix #6484 "4 or 5 out of 7 targets triggered by SIGRTMIN+X use the wrong job mode"#6715
Merged
poettering merged 4 commits intosystemd:masterfrom Sep 1, 2017
Merged
Conversation
rescue.target does not work well for this. It is not meant to be started, only isolated. Fixes systemd#6493
It's like Manager.PowerOff(), which does not start poweroff.target. Instead, the dbus methods are used for `systemctl --force exit` or `systemctl --force poweroff`. They shut down the system without processing individual unit's ExecStop or TimeoutStopSec.
The comment here was misleading: the job can fail to enqueue for reasons other than the target not existing. The fallback caused an error to be logged, and dates back to when the "user" directory was named "session". units/session/exit.target was added later the same year. This is consistent with the documentation (man systemd), and the handling of similar signals. It's also consistent with `systemctl exit`, which is what most people would expect.
The irreversible job mode is required to ensure that shutdown is not interrupted by the activation of a unit with a conflict. We already used the correct job mode for `ctrl-alt-del.target`. But not for `exit.target` (SIGINT of user manager). The SIGRT shutdown signals also needed fixing. Also change SIGRTMIN+0 to isolate default.target, instead of starting it. The previous behaviour was documented. However there was no reason given for it, nor can we provide one. The problem that isolate is too aggressive anywhere outside of emergency.target (systemd#2607) is orthogonal. This feature is "accessible by different means and only really a safety net"; it is confusing for it to differ from `systemctl default` without explanation. `AllowIsolate=yes` is retained on poweroff.target etc. for backwards compatibility. `sigpwr.target` is also an obvious candidate for linking to a shutdown target. Unforunately it is also a possible hook for implementing some logic like system V init did, reading `/etc/powerstatus`. If we switched to starting `sigpwr.target` with REPLACE_IRREVERSIBLY, attempts to run `systemctl shutdown` from it would fail, if they had not thought to set `DefaultDependencies=no`. We had provided no examples for `sigpwr`, and the whole idea is cruft to keep legacy people happy. For the moment, I leave `sigpwr` alone, with no risk of disrupting anyone's previously-working, half-working, or untested setup. Fixes systemd#6484. See also systemd#6471
Member
|
lgtm |
Member
|
CI failure appears unrelated |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix and full description is in commit 4 of 4.
Commit 1 resolves #6493 "drop Alias=kbrequest.target from rescue.target". I happened to have this on the same branch, as it was also due to the job mode used. I.e. we do not isolate
kbrequest.target, only start it. And rescue.target really needsisolate.Commits 2&3 are non-job-mode fixes for exit.target.