Patchwork wpa_supplicant: systemd service directives

login
register
mail settings
Submitter Mark Oteiza
Date Jan. 21, 2014, 8:46 p.m.
Message ID <8761pdkqrb.fsf@holos.localdomain>
Download mbox | patch
Permalink /patch/313057/
State Under Review
Headers show

Comments

Mark Oteiza - Jan. 21, 2014, 8:46 p.m.
From: Mark Oteiza <mvoteiza@udel.edu>

Quoting 399ec170: "Use correct (multi-user) target when installing
systemd units [...] all services should always be installed in
multi-user.target"

These directives (Alias, WantedBy) are documented in systemd.unit(5)

Signed-hostap: Mark Oteiza <mvoteiza@udel.edu>
---
 wpa_supplicant/systemd/wpa_supplicant-nl80211.service.arg.in | 2 +-
 wpa_supplicant/systemd/wpa_supplicant-wired.service.arg.in   | 2 +-
 wpa_supplicant/systemd/wpa_supplicant.service.arg.in         | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
Jouni Malinen - Jan. 30, 2014, 11:35 a.m.
On Tue, Jan 21, 2014 at 03:46:00PM -0500, Mark Oteiza wrote:
> Quoting 399ec170: "Use correct (multi-user) target when installing
> systemd units [...] all services should always be installed in
> multi-user.target"
> 
> These directives (Alias, WantedBy) are documented in systemd.unit(5)

Could you please describe why this change is needed in the commit
message? The text here is just referring to some old commit without
giving any explanation why the additional change is needed. That
reference to systemd.unit(5) was not very helpful either for me:

$ man systemd.unit
No manual entry for systemd.unit


> +++ b/wpa_supplicant/systemd/wpa_supplicant-nl80211.service.arg.in
> @@ -10,4 +10,4 @@ Type=simple
>  [Install]
> -Alias=multi-user.target.wants/wpa_supplicant-nl80211@%i.service
> +WantedBy=multi-user.target

So what does this do and why is it needed?
Holger Schurig - Feb. 3, 2014, 8:43 a.m.
> $ man systemd.unit
> No manual entry for systemd.unit

It seems that you don't run systemd, because this man page comes with
systemd. You can find all of it's manpages online as well, see
http://0pointer.de/public/systemd-man/

The one in question is
http://0pointer.de/public/systemd-man/systemd.unit.html. It says ...

Alias=
Additional names this unit shall be installed under. The names listed
here must have the same suffix (i.e. type) as the unit file name. This
option may be specified more than once, in which case all listed names
are used. At installation time, systemctl enable will create symlinks
from these names to the unit filename.

WantedBy=, RequiredBy=
A symbolic link is created in the .wants/ or .requires/ directory of
the listed unit when this unit is activated by systemctl enable. This
has the effect that a dependency of type Wants= or Requires= is added
from the listed unit to the current unit. The primary result is that
the current unit will be started when the listed unit is started. See
the description of Wants= and Requires= in the [Unit] section for
details.

WantedBy=foo.service in a service bar.service is mostly equivalent to
Alias=foo.service.wants/bar.service in the same file. In case of
template units, systemctl enable must be called with an instance name,
and this instance will be added to the .wants/ or .requires/ list of
the listed unit. E.g. WantedBy=getty.target in a service
getty@.service will result in systemctl enable getty@tty2.service
creating a getty.target.wants/getty@tty2.service link to
getty@.service.



But with all of this info, I don't see why this comment is necessary.
And I don't know why the patched unit files are "systemd unit
templates" (they use %I inside),but why they aren't named like
template units: they lack the @ in their filename.
Mark Oteiza - Feb. 3, 2014, 6:18 p.m.
Holger Schurig <holgerschurig@gmail.com> writes:

>> $ man systemd.unit
>> No manual entry for systemd.unit
>
> It seems that you don't run systemd, because this man page comes with
> systemd. You can find all of it's manpages online as well, see
> http://0pointer.de/public/systemd-man/
>
> The one in question is
> http://0pointer.de/public/systemd-man/systemd.unit.html.

In addition to the documentation, perhaps examples from other projects
will help show how wpa_supplicant's dbus service is fine; the others
misuse Alias.

[/usr/lib/systemd/system]$ find -type f -name '*.service' -exec grep
'Alias' {} +
./wpa_supplicant@.service:Alias=multi-user.target.wants/wpa_supplicant@wlan0.service
./wpa_supplicant.service:Alias=dbus-fi.epitest.hostap.WPASupplicant.service
./avahi-daemon.service:Alias=dbus-org.freedesktop.Avahi.service
./wpa_supplicant-nl80211@.service:Alias=multi-user.target.wants/wpa_supplicant-nl80211@wlan0.service
./wpa_supplicant-wired@.service:Alias=multi-user.target.wants/wpa_supplicant-wired@wlan0.service

[/usr/lib/systemd/system]$ find -type f -name '*.service' -exec grep
'WantedBy' {} +
./sshd.service:WantedBy=multi-user.target
./ntpd.service:WantedBy=multi-user.target
./iptables.service:WantedBy=multi-user.target
./rfkill-block@.service:WantedBy=multi-user.target
./dhcpcd@.service:WantedBy=multi-user.target
./rsyncd.service:WantedBy=multi-user.target
./krb5-kpropd.service:WantedBy=multi-user.target
./systemd-readahead-drop.service:WantedBy=system-update.target
./dhcpcd.service:WantedBy=multi-user.target
./ftpd.service:WantedBy=multi-user.target
./lvm-monitoring.service:WantedBy=sysinit.target
./krb5-kadmind.service:WantedBy=multi-user.target
./wpa_supplicant.service:WantedBy=multi-user.target
./systemd-readahead-replay.service:WantedBy=default.target
./xinetd.service:WantedBy=multi-user.target
./ip6tables.service:WantedBy=multi-user.target
./getty@.service:WantedBy=getty.target
./avahi-daemon.service:WantedBy=multi-user.target
./atop.service:WantedBy=multi-user.target
./envoy@.service:WantedBy=multi-user.target
./console-shell.service:WantedBy=getty.target
./systemd-readahead-collect.service:WantedBy=default.target
./cpupower.service:WantedBy=multi-user.target
./bitlbee.service:WantedBy=multi-user.target
./lightd.service:WantedBy=multi-user.target
./debug-shell.service:WantedBy=sysinit.target
./connman-vpn.service:WantedBy=multi-user.target
./console-getty.service:WantedBy=getty.target
./gpm.service:WantedBy=multi-user.target
./ifplugd@.service:WantedBy=multi-user.target
./avahi-dnsconfd.service:WantedBy=multi-user.target
./nginx.service:WantedBy=multi-user.target
./mdadm.service:WantedBy=multi-user.target
./nscd.service:WantedBy=multi-user.target
./connman.service:WantedBy=multi-user.target
./rfkill-unblock@.service:WantedBy=multi-user.target
./krb5-kdc.service:WantedBy=multi-user.target
./mpd.service:WantedBy=multi-user.target
./ntpdate.service:WantedBy=multi-user.target
./vnstat.service:WantedBy=multi-user.target
./ppp@.service:WantedBy=multi-user.target

> But with all of this info, I don't see why this comment is necessary.
> And I don't know why the patched unit files are "systemd unit
> templates" (they use %I inside),but why they aren't named like
> template units: they lack the @ in their filename.

They are .in files, the build system renames them.

Patch

diff --git a/wpa_supplicant/systemd/wpa_supplicant-nl80211.service.arg.in b/wpa_supplicant/systemd/wpa_supplicant-nl80211.service.arg.in
index bfdee25..48e381b 100644
--- a/wpa_supplicant/systemd/wpa_supplicant-nl80211.service.arg.in
+++ b/wpa_supplicant/systemd/wpa_supplicant-nl80211.service.arg.in
@@ -10,4 +10,4 @@  Type=simple
 ExecStart=@BINDIR@/wpa_supplicant -c/etc/wpa_supplicant/wpa_supplicant-nl80211-%I.conf -Dnl80211 -i%I
 
 [Install]
-Alias=multi-user.target.wants/wpa_supplicant-nl80211@%i.service
+WantedBy=multi-user.target
diff --git a/wpa_supplicant/systemd/wpa_supplicant-wired.service.arg.in b/wpa_supplicant/systemd/wpa_supplicant-wired.service.arg.in
index 20ba0ad..b7876bd 100644
--- a/wpa_supplicant/systemd/wpa_supplicant-wired.service.arg.in
+++ b/wpa_supplicant/systemd/wpa_supplicant-wired.service.arg.in
@@ -10,4 +10,4 @@  Type=simple
 ExecStart=@BINDIR@/wpa_supplicant -c/etc/wpa_supplicant/wpa_supplicant-wired-%I.conf -Dwired -i%I
 
 [Install]
-Alias=multi-user.target.wants/wpa_supplicant-wired@%i.service
+WantedBy=multi-user.target
diff --git a/wpa_supplicant/systemd/wpa_supplicant.service.arg.in b/wpa_supplicant/systemd/wpa_supplicant.service.arg.in
index 10e62bc..b54ce8d 100644
--- a/wpa_supplicant/systemd/wpa_supplicant.service.arg.in
+++ b/wpa_supplicant/systemd/wpa_supplicant.service.arg.in
@@ -10,4 +10,4 @@  Type=simple
 ExecStart=@BINDIR@/wpa_supplicant -c/etc/wpa_supplicant/wpa_supplicant-%I.conf -i%I
 
 [Install]
-Alias=multi-user.target.wants/wpa_supplicant@%i.service
+WantedBy=multi-user.target