From a3b22cc5962efef2f4cd57d49f59a67e0617e9e0 Mon Sep 17 00:00:00 2001 From: Alan Jenkins Date: Mon, 25 Sep 2017 11:38:08 +0100 Subject: [PATCH 1/5] units: do not kill rescue shell when machines.target is started The original aim of this commit is that starting machines.target from the rescue shell would not kill the rescue shell and lock you out of the system. This is similar to commit 6579a622, for the conflict between sysinit.target and the _emergency_ shell. That particular commit introduced an ordering cycle and will need to be reverted and/or fixed. This one does not, because it does not need to introduce any new dependencies. The reason why this commit is allowable also has it's own merit: machines.target was not marked as AllowIsolate. Also, the point of containers is to not escape them... I don't think we want to promote machines.target as a default target or similar; you would generally want some system service to allow you to shut down the machine, for example. I don't see this approach used in CoreOS, nor in Fedora Atomic Host; we are missing any positive examples of its utility. Requires=basic.target / After=basic.target can be removed for the same reason. --- units/machines.target | 3 --- 1 file changed, 3 deletions(-) diff --git a/units/machines.target b/units/machines.target index 99618a19f7..e07b0bb6ae 100644 --- a/units/machines.target +++ b/units/machines.target @@ -8,9 +8,6 @@ [Unit] Description=Containers Documentation=man:systemd.special(7) -Requires=basic.target -Conflicts=rescue.service rescue.target -After=basic.target rescue.service rescue.target Before=multi-user.target [Install] From 950d8fcdd9aa7e71ad32ea224e7d6fbe0f9f53de Mon Sep 17 00:00:00 2001 From: Alan Jenkins Date: Fri, 29 Sep 2017 17:20:31 +0100 Subject: [PATCH 2/5] units: express Conflict in syslog.socket instead of emergency.service Note this commit only changes how the code is expressed; it does not change the existence of any dependency. The `Conflicts=` was added in 3136ec90, "Stop syslog.socket when entering emergency mode". The discussion in the issue #266 raised concerns that this might be needed for other units, but failed to point out why syslog.socket is special. The reason is that syslog.socket has DefaultDepedencies=no, so it does not get Requires=sysinit.target like other socket units do. But syslog.service does require sysinit.target, among other things. We don't have many socket, path, or timer units with DefaultDependencies=no, and I don't think any of the triggered services have such additional hard dependencies as syslog.service does. It is much less confusing if we keep this `Conflicts=` in the same file as the `DefaultDependencies=no` which made it necessary. --- units/emergency.service.in | 1 - units/syslog.socket | 6 +++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/units/emergency.service.in b/units/emergency.service.in index c60fd6e256..ddf7d4cbe2 100644 --- a/units/emergency.service.in +++ b/units/emergency.service.in @@ -11,7 +11,6 @@ Documentation=man:sulogin(8) DefaultDependencies=no Conflicts=shutdown.target Conflicts=rescue.service -Conflicts=syslog.socket Before=shutdown.target [Service] diff --git a/units/syslog.socket b/units/syslog.socket index d3987cb9a8..372e8fcd45 100644 --- a/units/syslog.socket +++ b/units/syslog.socket @@ -10,10 +10,14 @@ Description=Syslog Socket Documentation=man:systemd.special(7) Documentation=https://www.freedesktop.org/wiki/Software/systemd/syslog DefaultDependencies=no -Before=sockets.target shutdown.target +Before=sockets.target # Don't allow logging until the very end Conflicts=shutdown.target +Before=shutdown.target + +# Don't try to activate syslog.service if sysinit.target has failed. +Conflicts=emergency.service [Socket] ListenDatagram=/run/systemd/journal/syslog From 6f6d1a8a6abae490e14e6a448895e36c6cefdefc Mon Sep 17 00:00:00 2001 From: Alan Jenkins Date: Mon, 25 Sep 2017 11:45:03 +0100 Subject: [PATCH 3/5] units: add missing ordering deps for Conflicts= of emergency.service 1. If we exited emergency mode immediately, we don't want to have an irreversible stop job still running for syslog.socket. I _suspect_ that can't happen, but let's not waste effort working out exactly why it's impossible and not just very improbable. 2. Similarly, it seems undesirable to have rescue.service and emergency.service both running with an open FD of /dev/console, for however short a period. --- units/emergency.service.in | 1 + units/syslog.socket | 1 + 2 files changed, 2 insertions(+) diff --git a/units/emergency.service.in b/units/emergency.service.in index ddf7d4cbe2..9a81fc4da9 100644 --- a/units/emergency.service.in +++ b/units/emergency.service.in @@ -12,6 +12,7 @@ DefaultDependencies=no Conflicts=shutdown.target Conflicts=rescue.service Before=shutdown.target +Before=rescue.service [Service] Environment=HOME=/root diff --git a/units/syslog.socket b/units/syslog.socket index 372e8fcd45..43981904ea 100644 --- a/units/syslog.socket +++ b/units/syslog.socket @@ -18,6 +18,7 @@ Before=shutdown.target # Don't try to activate syslog.service if sysinit.target has failed. Conflicts=emergency.service +Before=emergency.service [Socket] ListenDatagram=/run/systemd/journal/syslog From 0b9ad5bf2ec09e963f706fdba498d99f29f97a54 Mon Sep 17 00:00:00 2001 From: Alan Jenkins Date: Mon, 25 Sep 2017 12:10:38 +0100 Subject: [PATCH 4/5] units: add missing Before=shutdown.target for units which it Conflicts There's a few services missing this ordering. Also remove a duplicate Conflicts=shutdown.target from systemd-volatile-root.service. --- units/system-update-cleanup.service.in | 1 + units/systemd-networkd-wait-online.service.in | 2 +- units/systemd-volatile-root.service.in | 1 - 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/units/system-update-cleanup.service.in b/units/system-update-cleanup.service.in index 116be8bc2d..dc524da7a3 100644 --- a/units/system-update-cleanup.service.in +++ b/units/system-update-cleanup.service.in @@ -11,6 +11,7 @@ Documentation=man:systemd.special(5) man:systemd.offline-updates(7) After=system-update.target DefaultDependencies=no Conflicts=shutdown.target +Before=shutdown.target # system-update-generator uses laccess("/system-update"), while a plain # ConditionPathExists=/system-update uses access("/system-update"), so diff --git a/units/systemd-networkd-wait-online.service.in b/units/systemd-networkd-wait-online.service.in index a84e91906d..89ca865b55 100644 --- a/units/systemd-networkd-wait-online.service.in +++ b/units/systemd-networkd-wait-online.service.in @@ -12,7 +12,7 @@ DefaultDependencies=no Conflicts=shutdown.target Requires=systemd-networkd.service After=systemd-networkd.service -Before=network-online.target +Before=network-online.target shutdown.target [Service] Type=oneshot diff --git a/units/systemd-volatile-root.service.in b/units/systemd-volatile-root.service.in index cc4e604e4c..c5a4ca3c27 100644 --- a/units/systemd-volatile-root.service.in +++ b/units/systemd-volatile-root.service.in @@ -12,7 +12,6 @@ DefaultDependencies=no Conflicts=shutdown.target After=sysroot.mount Before=initrd-root-fs.target shutdown.target -Conflicts=shutdown.target AssertPathExists=/etc/initrd-release [Service] From 2d4b910383c6c14b3720e26cca11c223ad868d9d Mon Sep 17 00:00:00 2001 From: Alan Jenkins Date: Mon, 25 Sep 2017 12:46:32 +0100 Subject: [PATCH 5/5] units: DefaultDependencies already implies conflict with shutdown.target (and system-update.target does not have DefaultDependencies=no) --- units/system-update.target | 2 -- 1 file changed, 2 deletions(-) diff --git a/units/system-update.target b/units/system-update.target index 3542879706..b95639a876 100644 --- a/units/system-update.target +++ b/units/system-update.target @@ -10,8 +10,6 @@ Description=Offline System Update Documentation=man:systemd.offline-updates(7) Documentation=man:systemd.special(7) man:systemd-system-update-generator(8) Requires=sysinit.target -Conflicts=shutdown.target After=sysinit.target -Before=shutdown.target AllowIsolate=yes Wants=system-update-cleanup.service