From: Nathaniel Roach <nroach44@gmail.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH] package/quagga: Add systemd.service file
Date: Wed, 29 Jun 2016 15:06:37 +0800 [thread overview]
Message-ID: <76555fc3-8603-91d7-ad8f-ecdef822df16@gmail.com> (raw)
In-Reply-To: <91c2c2b3-7715-2ab6-31ab-144aa346c54b@mind.be>
On 27/06/16 05:09, Arnout Vandecappelle wrote:
> On 25-06-16 19:02, Nathaniel Roach wrote:
>> Use a template service file as all of the daemons use almost
>> identical arguments and generally appear the same to the init
>> system.
>>
>> We "Wants=" zebra as that's the daemon for interfacing to the
>> kernel, and it's not required for the other daemons to work
>> but it's probably going to be used in nearly all setups.
>>
>> /usr/bin/env is needed as systemd doesn't allow the instance
>> variable (%i) in the executable path.
>>
>> We don't enable these services by default as this would require
>> creating configuration and /etc/default files. (And is easily
>> achieved with an FS overlay)
> ^missing .
>
> So the goal of this patch is that the user simply can symlink to instead of
> having to create the service file.
>
> However, since the Wants= is added for zebra, it would make sense to create the
> zebra symlink, no?
No, systemd will automatically start zebra when it's needed. It's not
needed until one of the other daemons start (which systemd will start
zebra beforehand if the daemon is local). If they are remote, the user
can enable zebra themselves.
>
>> Signed-off-by: Nathaniel Roach <nroach44@gmail.com>
>> ---
>> Changes v1 -> v2:
>> (As suggested by Arnout Vandecappelle)
>> - Completely remove shim and use /usr/bin/env instead
>> - Don't tell quagga to fork as systemd prefers it
>> - Add comment to .service file about /usr/bin/env
>> - Explain not enabling the service on build in patch
>>
>> ---
>> package/quagga/quagga.mk | 3 +++
>> package/quagga/quagga at .service | 20 ++++++++++++++++++++
>> 2 files changed, 23 insertions(+)
>> create mode 100644 package/quagga/quagga at .service
>>
>> diff --git a/package/quagga/quagga.mk b/package/quagga/quagga.mk
>> index 22e90ad..5b5623f 100644
>> --- a/package/quagga/quagga.mk
>> +++ b/package/quagga/quagga.mk
>> @@ -75,6 +75,9 @@ endif
>> define QUAGGA_INSTALL_INIT_SYSTEMD
>> $(INSTALL) -D -m 644 package/quagga/quagga_tmpfiles.conf \
>> $(TARGET_DIR)/usr/lib/tmpfiles.d/quagga.conf
>> +
> This empty line is redundant.
>
>> + $(INSTALL) -D -m 644 package/quagga/quagga at .service \
>> + $(TARGET_DIR)/usr/lib/systemd/system/quagga at .service
>> endef
>>
>> $(eval $(autotools-package))
>> diff --git a/package/quagga/quagga at .service b/package/quagga/quagga at .service
>> new file mode 100644
>> index 0000000..b17af89
>> --- /dev/null
>> +++ b/package/quagga/quagga at .service
>> @@ -0,0 +1,20 @@
>> +[Unit]
>> +Description=Quagga %i routing daemon
>> +PartOf=quagga.service
> I don't understand this. Doesn't the quagga.service actually have to exist for
> this to be valid?
That is true, it's left over from my openvpn service file. Will remove
that and ReloadPropagatedFrom=
>
>> +ReloadPropagatedFrom=quagga.service
>> +Wants=quagga at zebra.service
> I just realize now: this line should probably be removed when
> BR2_PACKAGE_QUAGGA_ZEBRA is not set.
Also true, however that's why I said Wants= and not Requires= so systemd
handles the lack of zebra fine.
>
>> +
>> +[Service]
>> +PrivateTmp=true
>> +KillMode=mixed
> It would make sense to keep this together with KillSignal.
>
>> +Type=simple
>> +EnvironmentFile=/etc/default/quagga-%i
>> +# Systemd doesn't like having %i in the executable path.
>> +ExecStart=/usr/bin/env /usr/sbin/%i $OPTS -f /etc/quagga/%i.conf -i /var/run/quagga/%i.pid
>> +PIDFile=/var/run/quagga/%i.pid
> I repeat, I'm not very familiar with systemd, but as I understand it PID files
> shouldn't be needed anymore with a Type=simple daemon. Unless of course if
> quagga uses them somewhere else too.
Also correct. I left the -i argument in as I thought it was required for
vtysh, but it is not.
>
>
> Regards,
> Arnout
>
>> +KillSignal=SIGINT
>> +Restart=on-failure
>> +RestartSec=1
>> +
>> +[Install]
>> +WantedBy=multi-user.target
>>
>
Thanks for the feedback!
Nathaniel Roach
next prev parent reply other threads:[~2016-06-29 7:06 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-25 17:02 [Buildroot] [PATCH] package/quagga: Add systemd.service file Nathaniel Roach
2016-06-26 21:09 ` Arnout Vandecappelle
2016-06-29 7:06 ` Nathaniel Roach [this message]
2016-06-29 9:53 ` Arnout Vandecappelle
2016-06-29 12:02 ` Nathaniel Roach
2016-07-01 21:47 ` Thomas Petazzoni
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=76555fc3-8603-91d7-ad8f-ecdef822df16@gmail.com \
--to=nroach44@gmail.com \
--cc=buildroot@busybox.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox