Skip to content

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
sourcejedi:kbrequest-jobmode2
Sep 1, 2017
Merged

Fix #6484 "4 or 5 out of 7 targets triggered by SIGRTMIN+X use the wrong job mode"#6715
poettering merged 4 commits intosystemd:masterfrom
sourcejedi:kbrequest-jobmode2

Conversation

@sourcejedi
Copy link
Contributor

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 needs isolate.

Commits 2&3 are non-job-mode fixes for exit.target.

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
@poettering
Copy link
Member

lgtm

@poettering poettering added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed pid1 labels Aug 31, 2017
@poettering
Copy link
Member

CI failure appears unrelated

@poettering poettering merged commit d9ada1e into systemd:master Sep 1, 2017
@sourcejedi sourcejedi deleted the kbrequest-jobmode2 branch September 1, 2017 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed pid1

Development

Successfully merging this pull request may close these issues.

drop Alias=kbrequest.target from rescue.target

2 participants