* [Buildroot] [PATCH v3] package/quagga: Add systemd.service file @ 2016-07-03 8:18 Nathaniel Roach 2016-07-03 10:42 ` Maxime Hadjinlian 2016-07-03 14:02 ` Peter Korsgaard 0 siblings, 2 replies; 9+ messages in thread From: Nathaniel Roach @ 2016-07-03 8:18 UTC (permalink / raw) To: buildroot 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) Signed-off-by: Nathaniel Roach <nroach44@gmail.com> --- Changes v2 -> v3: - Remove invalid references to quagga.service (Arnout) - Check if the binary is executable before trying to start it (Arnout) - Remove PID file arguments and options (Arnout) - Add reload capability as the daemons do support it 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 | 2 ++ package/quagga/quagga at .service | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 package/quagga/quagga at .service diff --git a/package/quagga/quagga.mk b/package/quagga/quagga.mk index 22e90ad..1bbc72d 100644 --- a/package/quagga/quagga.mk +++ b/package/quagga/quagga.mk @@ -75,6 +75,8 @@ endif define QUAGGA_INSTALL_INIT_SYSTEMD $(INSTALL) -D -m 644 package/quagga/quagga_tmpfiles.conf \ $(TARGET_DIR)/usr/lib/tmpfiles.d/quagga.conf + $(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..16acc30 --- /dev/null +++ b/package/quagga/quagga at .service @@ -0,0 +1,20 @@ +[Unit] +Description=Quagga %i routing daemon +ConditionFileIsExecutable=/usr/sbin/%i +Wants=quagga at zebra.service + +[Service] +Type=simple +EnvironmentFile=/etc/default/quagga-%i +PrivateTmp=true +# Systemd doesn't like having %i in the executable path. +ExecStart=/usr/bin/env /usr/sbin/%i $OPTS -f /etc/quagga/%i.conf +ExecReload=/bin/kill -HUP $MAINPID +KillMode=mixed +KillSignal=SIGINT +Restart=on-failure +RestartSec=1 + +[Install] +WantedBy=multi-user.target + -- 2.8.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Buildroot] [PATCH v3] package/quagga: Add systemd.service file 2016-07-03 8:18 [Buildroot] [PATCH v3] package/quagga: Add systemd.service file Nathaniel Roach @ 2016-07-03 10:42 ` Maxime Hadjinlian 2016-07-03 10:54 ` Nathaniel Roach 2016-07-03 14:02 ` Peter Korsgaard 1 sibling, 1 reply; 9+ messages in thread From: Maxime Hadjinlian @ 2016-07-03 10:42 UTC (permalink / raw) To: buildroot Hi Nathaniel, all On Sun, Jul 3, 2016 at 10:18 AM, Nathaniel Roach <nroach44@gmail.com> 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) > > Signed-off-by: Nathaniel Roach <nroach44@gmail.com> > --- > Changes v2 -> v3: > - Remove invalid references to quagga.service (Arnout) > - Check if the binary is executable before trying to start it > (Arnout) > - Remove PID file arguments and options (Arnout) > - Add reload capability as the daemons do support it > > 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 | 2 ++ > package/quagga/quagga at .service | 20 ++++++++++++++++++++ > 2 files changed, 22 insertions(+) > create mode 100644 package/quagga/quagga at .service > > diff --git a/package/quagga/quagga.mk b/package/quagga/quagga.mk > index 22e90ad..1bbc72d 100644 > --- a/package/quagga/quagga.mk > +++ b/package/quagga/quagga.mk > @@ -75,6 +75,8 @@ endif > define QUAGGA_INSTALL_INIT_SYSTEMD > $(INSTALL) -D -m 644 package/quagga/quagga_tmpfiles.conf \ > $(TARGET_DIR)/usr/lib/tmpfiles.d/quagga.conf > + $(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..16acc30 > --- /dev/null > +++ b/package/quagga/quagga at .service > @@ -0,0 +1,20 @@ > +[Unit] > +Description=Quagga %i routing daemon > +ConditionFileIsExecutable=/usr/sbin/%i > +Wants=quagga at zebra.service Just a though, I read about why you added the Wants, maybe a comment would be nice here ? > + > +[Service] > +Type=simple Since you have removed the '--daemon' option, maybe you should revert this to "forking" ? > +EnvironmentFile=/etc/default/quagga-%i > +PrivateTmp=true > +# Systemd doesn't like having %i in the executable path. > +ExecStart=/usr/bin/env /usr/sbin/%i $OPTS -f /etc/quagga/%i.conf > +ExecReload=/bin/kill -HUP $MAINPID > +KillMode=mixed > +KillSignal=SIGINT If I understand correctly, what you want to do is send a SIGINT to the main processes and SIGKILL to the other ? From the services file I can find in major distro, they don't specify it so it's left to the default SIGTERM, why did you need to change that ? (Maybe you already explained in your previous mail and I missed, sorry if that's the case). > +Restart=on-failure > +RestartSec=1 Why ? > + > +[Install] > +WantedBy=multi-user.target > + > -- > 2.8.1 > > _______________________________________________ > buildroot mailing list > buildroot at busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Buildroot] [PATCH v3] package/quagga: Add systemd.service file 2016-07-03 10:42 ` Maxime Hadjinlian @ 2016-07-03 10:54 ` Nathaniel Roach 2016-07-03 12:59 ` Maxime Hadjinlian 0 siblings, 1 reply; 9+ messages in thread From: Nathaniel Roach @ 2016-07-03 10:54 UTC (permalink / raw) To: buildroot Hi Maxime, On 03/07/16 18:42, Maxime Hadjinlian wrote: > Hi Nathaniel, all > > On Sun, Jul 3, 2016 at 10:18 AM, Nathaniel Roach <nroach44@gmail.com> 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) >> >> Signed-off-by: Nathaniel Roach <nroach44@gmail.com> >> --- >> Changes v2 -> v3: >> - Remove invalid references to quagga.service (Arnout) >> - Check if the binary is executable before trying to start it >> (Arnout) >> - Remove PID file arguments and options (Arnout) >> - Add reload capability as the daemons do support it >> >> 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 | 2 ++ >> package/quagga/quagga at .service | 20 ++++++++++++++++++++ >> 2 files changed, 22 insertions(+) >> create mode 100644 package/quagga/quagga at .service >> >> diff --git a/package/quagga/quagga.mk b/package/quagga/quagga.mk >> index 22e90ad..1bbc72d 100644 >> --- a/package/quagga/quagga.mk >> +++ b/package/quagga/quagga.mk >> @@ -75,6 +75,8 @@ endif >> define QUAGGA_INSTALL_INIT_SYSTEMD >> $(INSTALL) -D -m 644 package/quagga/quagga_tmpfiles.conf \ >> $(TARGET_DIR)/usr/lib/tmpfiles.d/quagga.conf >> + $(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..16acc30 >> --- /dev/null >> +++ b/package/quagga/quagga at .service >> @@ -0,0 +1,20 @@ >> +[Unit] >> +Description=Quagga %i routing daemon >> +ConditionFileIsExecutable=/usr/sbin/%i >> +Wants=quagga at zebra.service > Just a though, I read about why you added the Wants, maybe a comment > would be nice here ? >> + >> +[Service] >> +Type=simple > Since you have removed the '--daemon' option, maybe you should revert > this to "forking" ? I might have it backwards, but it would need "forking" if --daemon was specified, otherwise "systemctl start quagga at ospfd" would hang. >> +EnvironmentFile=/etc/default/quagga-%i >> +PrivateTmp=true >> +# Systemd doesn't like having %i in the executable path. >> +ExecStart=/usr/bin/env /usr/sbin/%i $OPTS -f /etc/quagga/%i.conf >> +ExecReload=/bin/kill -HUP $MAINPID >> +KillMode=mixed >> +KillSignal=SIGINT > If I understand correctly, what you want to do is send a SIGINT to the > main processes and SIGKILL to the other ? From the services file I can > find in major distro, they don't specify it so it's left to the > default SIGTERM, why did you need to change that ? (Maybe you already > explained in your previous mail and I missed, sorry if that's the > case). KillMode=Mixed is an artefact left over from the OpenVPN .service file I used as a reference. I (again) misread the systemd docs and left it in as it seemed more appropriate than the default if any child processes are started. This can be removed on commit, or I'll fix it if there's any other things that need to be changed. >> +Restart=on-failure >> +RestartSec=1 > Why ? I've had ospfd crash during configuration and general usage, this is undesirable as it's responsible for keeping a large amount of routes in my router's tables. >> + >> +[Install] >> +WantedBy=multi-user.target >> + >> -- >> 2.8.1 >> >> _______________________________________________ >> buildroot mailing list >> buildroot at busybox.net >> http://lists.busybox.net/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Buildroot] [PATCH v3] package/quagga: Add systemd.service file 2016-07-03 10:54 ` Nathaniel Roach @ 2016-07-03 12:59 ` Maxime Hadjinlian 2016-07-03 13:02 ` Nathaniel Roach 2016-07-03 13:06 ` Nathaniel Roach 0 siblings, 2 replies; 9+ messages in thread From: Maxime Hadjinlian @ 2016-07-03 12:59 UTC (permalink / raw) To: buildroot Hi Nathaniel, On Sun, Jul 3, 2016 at 12:54 PM, Nathaniel Roach <nroach44@gmail.com> wrote: > Hi Maxime, > > > > On 03/07/16 18:42, Maxime Hadjinlian wrote: >> >> Hi Nathaniel, all >> >> On Sun, Jul 3, 2016 at 10:18 AM, Nathaniel Roach <nroach44@gmail.com> >> 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) >>> >>> Signed-off-by: Nathaniel Roach <nroach44@gmail.com> >>> --- >>> Changes v2 -> v3: >>> - Remove invalid references to quagga.service (Arnout) >>> - Check if the binary is executable before trying to start it >>> (Arnout) >>> - Remove PID file arguments and options (Arnout) >>> - Add reload capability as the daemons do support it >>> >>> 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 | 2 ++ >>> package/quagga/quagga at .service | 20 ++++++++++++++++++++ >>> 2 files changed, 22 insertions(+) >>> create mode 100644 package/quagga/quagga at .service >>> >>> diff --git a/package/quagga/quagga.mk b/package/quagga/quagga.mk >>> index 22e90ad..1bbc72d 100644 >>> --- a/package/quagga/quagga.mk >>> +++ b/package/quagga/quagga.mk >>> @@ -75,6 +75,8 @@ endif >>> define QUAGGA_INSTALL_INIT_SYSTEMD >>> $(INSTALL) -D -m 644 package/quagga/quagga_tmpfiles.conf \ >>> $(TARGET_DIR)/usr/lib/tmpfiles.d/quagga.conf >>> + $(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..16acc30 >>> --- /dev/null >>> +++ b/package/quagga/quagga at .service >>> @@ -0,0 +1,20 @@ >>> +[Unit] >>> +Description=Quagga %i routing daemon >>> +ConditionFileIsExecutable=/usr/sbin/%i >>> +Wants=quagga at zebra.service >> >> Just a though, I read about why you added the Wants, maybe a comment >> would be nice here ? >>> >>> + >>> +[Service] >>> +Type=simple >> >> Since you have removed the '--daemon' option, maybe you should revert >> this to "forking" ? > > I might have it backwards, but it would need "forking" if --daemon was > specified, otherwise "systemctl start quagga at ospfd" would hang. You are absolutely right, I am the one that got confused. I missed the '-d' on the ExecStart line in the services in the quagga sources. >>> >>> +EnvironmentFile=/etc/default/quagga-%i >>> +PrivateTmp=true >>> +# Systemd doesn't like having %i in the executable path. >>> +ExecStart=/usr/bin/env /usr/sbin/%i $OPTS -f /etc/quagga/%i.conf >>> +ExecReload=/bin/kill -HUP $MAINPID >>> +KillMode=mixed >>> +KillSignal=SIGINT >> >> If I understand correctly, what you want to do is send a SIGINT to the >> main processes and SIGKILL to the other ? From the services file I can >> find in major distro, they don't specify it so it's left to the >> default SIGTERM, why did you need to change that ? (Maybe you already >> explained in your previous mail and I missed, sorry if that's the >> case). > > KillMode=Mixed is an artefact left over from the OpenVPN .service file I > used as a reference. I (again) misread the systemd docs and left it in as it > seemed more appropriate than the default if any child processes are started. > This can be removed on commit, or I'll fix it if there's any other things > that need to be changed. Thanks for the explanation. >>> >>> +Restart=on-failure >>> +RestartSec=1 >> >> Why ? > > I've had ospfd crash during configuration and general usage, this is > undesirable as it's responsible for keeping a large amount of routes in my > router's tables. I was thinking about the RestartSec delay, I agree on the "on-failure". > >>> + >>> +[Install] >>> +WantedBy=multi-user.target >>> + >>> -- >>> 2.8.1 >>> >>> _______________________________________________ >>> buildroot mailing list >>> buildroot at busybox.net >>> http://lists.busybox.net/mailman/listinfo/buildroot > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Buildroot] [PATCH v3] package/quagga: Add systemd.service file 2016-07-03 12:59 ` Maxime Hadjinlian @ 2016-07-03 13:02 ` Nathaniel Roach 2016-07-03 13:06 ` Nathaniel Roach 1 sibling, 0 replies; 9+ messages in thread From: Nathaniel Roach @ 2016-07-03 13:02 UTC (permalink / raw) To: buildroot On 03/07/16 20:59, Maxime Hadjinlian wrote: > Hi Nathaniel, > > On Sun, Jul 3, 2016 at 12:54 PM, Nathaniel Roach <nroach44@gmail.com> wrote: >> Hi Maxime, >> >> >> >> On 03/07/16 18:42, Maxime Hadjinlian wrote: >>> Hi Nathaniel, all >>> >>> On Sun, Jul 3, 2016 at 10:18 AM, Nathaniel Roach <nroach44@gmail.com> >>> 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) >>>> >>>> Signed-off-by: Nathaniel Roach <nroach44@gmail.com> >>>> --- >>>> Changes v2 -> v3: >>>> - Remove invalid references to quagga.service (Arnout) >>>> - Check if the binary is executable before trying to start it >>>> (Arnout) >>>> - Remove PID file arguments and options (Arnout) >>>> - Add reload capability as the daemons do support it >>>> >>>> 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 | 2 ++ >>>> package/quagga/quagga at .service | 20 ++++++++++++++++++++ >>>> 2 files changed, 22 insertions(+) >>>> create mode 100644 package/quagga/quagga at .service >>>> >>>> diff --git a/package/quagga/quagga.mk b/package/quagga/quagga.mk >>>> index 22e90ad..1bbc72d 100644 >>>> --- a/package/quagga/quagga.mk >>>> +++ b/package/quagga/quagga.mk >>>> @@ -75,6 +75,8 @@ endif >>>> define QUAGGA_INSTALL_INIT_SYSTEMD >>>> $(INSTALL) -D -m 644 package/quagga/quagga_tmpfiles.conf \ >>>> $(TARGET_DIR)/usr/lib/tmpfiles.d/quagga.conf >>>> + $(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..16acc30 >>>> --- /dev/null >>>> +++ b/package/quagga/quagga at .service >>>> @@ -0,0 +1,20 @@ >>>> +[Unit] >>>> +Description=Quagga %i routing daemon >>>> +ConditionFileIsExecutable=/usr/sbin/%i >>>> +Wants=quagga at zebra.service >>> Just a though, I read about why you added the Wants, maybe a comment >>> would be nice here ? >>>> + >>>> +[Service] >>>> +Type=simple >>> Since you have removed the '--daemon' option, maybe you should revert >>> this to "forking" ? >> I might have it backwards, but it would need "forking" if --daemon was >> specified, otherwise "systemctl start quagga at ospfd" would hang. > You are absolutely right, I am the one that got confused. I missed the > '-d' on the ExecStart line in the services in the quagga sources. >>>> +EnvironmentFile=/etc/default/quagga-%i >>>> +PrivateTmp=true >>>> +# Systemd doesn't like having %i in the executable path. >>>> +ExecStart=/usr/bin/env /usr/sbin/%i $OPTS -f /etc/quagga/%i.conf >>>> +ExecReload=/bin/kill -HUP $MAINPID >>>> +KillMode=mixed >>>> +KillSignal=SIGINT >>> If I understand correctly, what you want to do is send a SIGINT to the >>> main processes and SIGKILL to the other ? From the services file I can >>> find in major distro, they don't specify it so it's left to the >>> default SIGTERM, why did you need to change that ? (Maybe you already >>> explained in your previous mail and I missed, sorry if that's the >>> case). >> KillMode=Mixed is an artefact left over from the OpenVPN .service file I >> used as a reference. I (again) misread the systemd docs and left it in as it >> seemed more appropriate than the default if any child processes are started. >> This can be removed on commit, or I'll fix it if there's any other things >> that need to be changed. > Thanks for the explanation. >>>> +Restart=on-failure >>>> +RestartSec=1 >>> Why ? >> I've had ospfd crash during configuration and general usage, this is >> undesirable as it's responsible for keeping a large amount of routes in my >> router's tables. > I was thinking about the RestartSec delay, I agree on the "on-failure". Oh, derp. I didn't pick 1 second for any reason other than it's quickest. If there's a good argument for longer it can be set longer. >>>> + >>>> +[Install] >>>> +WantedBy=multi-user.target >>>> + >>>> -- >>>> 2.8.1 >>>> >>>> _______________________________________________ >>>> buildroot mailing list >>>> buildroot at busybox.net >>>> http://lists.busybox.net/mailman/listinfo/buildroot >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Buildroot] [PATCH v3] package/quagga: Add systemd.service file 2016-07-03 12:59 ` Maxime Hadjinlian 2016-07-03 13:02 ` Nathaniel Roach @ 2016-07-03 13:06 ` Nathaniel Roach 2016-07-03 13:09 ` Maxime Hadjinlian 1 sibling, 1 reply; 9+ messages in thread From: Nathaniel Roach @ 2016-07-03 13:06 UTC (permalink / raw) To: buildroot Oh, and it's not 0 such that it won't restart and hog resources quite as badly if it's instantly dieing. On 03/07/16 20:59, Maxime Hadjinlian wrote: > Hi Nathaniel, > > On Sun, Jul 3, 2016 at 12:54 PM, Nathaniel Roach <nroach44@gmail.com> wrote: >> Hi Maxime, >> >> >> >> On 03/07/16 18:42, Maxime Hadjinlian wrote: >>> Hi Nathaniel, all >>> >>> On Sun, Jul 3, 2016 at 10:18 AM, Nathaniel Roach <nroach44@gmail.com> >>> 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) >>>> >>>> Signed-off-by: Nathaniel Roach <nroach44@gmail.com> >>>> --- >>>> Changes v2 -> v3: >>>> - Remove invalid references to quagga.service (Arnout) >>>> - Check if the binary is executable before trying to start it >>>> (Arnout) >>>> - Remove PID file arguments and options (Arnout) >>>> - Add reload capability as the daemons do support it >>>> >>>> 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 | 2 ++ >>>> package/quagga/quagga at .service | 20 ++++++++++++++++++++ >>>> 2 files changed, 22 insertions(+) >>>> create mode 100644 package/quagga/quagga at .service >>>> >>>> diff --git a/package/quagga/quagga.mk b/package/quagga/quagga.mk >>>> index 22e90ad..1bbc72d 100644 >>>> --- a/package/quagga/quagga.mk >>>> +++ b/package/quagga/quagga.mk >>>> @@ -75,6 +75,8 @@ endif >>>> define QUAGGA_INSTALL_INIT_SYSTEMD >>>> $(INSTALL) -D -m 644 package/quagga/quagga_tmpfiles.conf \ >>>> $(TARGET_DIR)/usr/lib/tmpfiles.d/quagga.conf >>>> + $(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..16acc30 >>>> --- /dev/null >>>> +++ b/package/quagga/quagga at .service >>>> @@ -0,0 +1,20 @@ >>>> +[Unit] >>>> +Description=Quagga %i routing daemon >>>> +ConditionFileIsExecutable=/usr/sbin/%i >>>> +Wants=quagga at zebra.service >>> Just a though, I read about why you added the Wants, maybe a comment >>> would be nice here ? >>>> + >>>> +[Service] >>>> +Type=simple >>> Since you have removed the '--daemon' option, maybe you should revert >>> this to "forking" ? >> I might have it backwards, but it would need "forking" if --daemon was >> specified, otherwise "systemctl start quagga at ospfd" would hang. > You are absolutely right, I am the one that got confused. I missed the > '-d' on the ExecStart line in the services in the quagga sources. >>>> +EnvironmentFile=/etc/default/quagga-%i >>>> +PrivateTmp=true >>>> +# Systemd doesn't like having %i in the executable path. >>>> +ExecStart=/usr/bin/env /usr/sbin/%i $OPTS -f /etc/quagga/%i.conf >>>> +ExecReload=/bin/kill -HUP $MAINPID >>>> +KillMode=mixed >>>> +KillSignal=SIGINT >>> If I understand correctly, what you want to do is send a SIGINT to the >>> main processes and SIGKILL to the other ? From the services file I can >>> find in major distro, they don't specify it so it's left to the >>> default SIGTERM, why did you need to change that ? (Maybe you already >>> explained in your previous mail and I missed, sorry if that's the >>> case). >> KillMode=Mixed is an artefact left over from the OpenVPN .service file I >> used as a reference. I (again) misread the systemd docs and left it in as it >> seemed more appropriate than the default if any child processes are started. >> This can be removed on commit, or I'll fix it if there's any other things >> that need to be changed. > Thanks for the explanation. >>>> +Restart=on-failure >>>> +RestartSec=1 >>> Why ? >> I've had ospfd crash during configuration and general usage, this is >> undesirable as it's responsible for keeping a large amount of routes in my >> router's tables. > I was thinking about the RestartSec delay, I agree on the "on-failure". >>>> + >>>> +[Install] >>>> +WantedBy=multi-user.target >>>> + >>>> -- >>>> 2.8.1 >>>> >>>> _______________________________________________ >>>> buildroot mailing list >>>> buildroot at busybox.net >>>> http://lists.busybox.net/mailman/listinfo/buildroot >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Buildroot] [PATCH v3] package/quagga: Add systemd.service file 2016-07-03 13:06 ` Nathaniel Roach @ 2016-07-03 13:09 ` Maxime Hadjinlian 2016-07-03 13:12 ` Nathaniel Roach 0 siblings, 1 reply; 9+ messages in thread From: Maxime Hadjinlian @ 2016-07-03 13:09 UTC (permalink / raw) To: buildroot On Sun, Jul 3, 2016 at 3:06 PM, Nathaniel Roach <nroach44@gmail.com> wrote: > Oh, and it's not 0 such that it won't restart and hog resources quite as > badly if it's instantly dieing. > The default is 100ms, do you think that's still too fast ? Usually it's enough but if you encounter problem and 1s is the shortest it can do, then 1s it is. Other than that: Reviewed-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com> [...] >>>>> +EnvironmentFile=/etc/default/quagga-%i >>>>> +PrivateTmp=true >>>>> +# Systemd doesn't like having %i in the executable path. >>>>> +ExecStart=/usr/bin/env /usr/sbin/%i $OPTS -f /etc/quagga/%i.conf >>>>> +ExecReload=/bin/kill -HUP $MAINPID >>>>> +KillMode=mixed >>>>> +KillSignal=SIGINT >>>> >>>> If I understand correctly, what you want to do is send a SIGINT to the >>>> main processes and SIGKILL to the other ? From the services file I can >>>> find in major distro, they don't specify it so it's left to the >>>> default SIGTERM, why did you need to change that ? (Maybe you already >>>> explained in your previous mail and I missed, sorry if that's the >>>> case). >>> >>> KillMode=Mixed is an artefact left over from the OpenVPN .service file I >>> used as a reference. I (again) misread the systemd docs and left it in as >>> it >>> seemed more appropriate than the default if any child processes are >>> started. >>> This can be removed on commit, or I'll fix it if there's any other things >>> that need to be changed. [...] ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Buildroot] [PATCH v3] package/quagga: Add systemd.service file 2016-07-03 13:09 ` Maxime Hadjinlian @ 2016-07-03 13:12 ` Nathaniel Roach 0 siblings, 0 replies; 9+ messages in thread From: Nathaniel Roach @ 2016-07-03 13:12 UTC (permalink / raw) To: buildroot Ah, another case of me misinterpreting defaults. This can also be removed if it's preferred to leave it as default. On 3 July 2016 9:09:39 PM AWST, Maxime Hadjinlian <maxime.hadjinlian@gmail.com> wrote: >On Sun, Jul 3, 2016 at 3:06 PM, Nathaniel Roach <nroach44@gmail.com> >wrote: >> Oh, and it's not 0 such that it won't restart and hog resources quite >as >> badly if it's instantly dieing. >> >The default is 100ms, do you think that's still too fast ? Usually >it's enough but if you encounter problem and 1s is the shortest it can >do, then 1s it is. > >Other than that: >Reviewed-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com> >[...] >>>>>> +EnvironmentFile=/etc/default/quagga-%i >>>>>> +PrivateTmp=true >>>>>> +# Systemd doesn't like having %i in the executable path. >>>>>> +ExecStart=/usr/bin/env /usr/sbin/%i $OPTS -f /etc/quagga/%i.conf >>>>>> +ExecReload=/bin/kill -HUP $MAINPID >>>>>> +KillMode=mixed >>>>>> +KillSignal=SIGINT >>>>> >>>>> If I understand correctly, what you want to do is send a SIGINT to >the >>>>> main processes and SIGKILL to the other ? From the services file I >can >>>>> find in major distro, they don't specify it so it's left to the >>>>> default SIGTERM, why did you need to change that ? (Maybe you >already >>>>> explained in your previous mail and I missed, sorry if that's the >>>>> case). >>>> >>>> KillMode=Mixed is an artefact left over from the OpenVPN .service >file I >>>> used as a reference. I (again) misread the systemd docs and left it >in as >>>> it >>>> seemed more appropriate than the default if any child processes are >>>> started. >>>> This can be removed on commit, or I'll fix it if there's any other >things >>>> that need to be changed. >[...] -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20160703/7807af0b/attachment.html> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Buildroot] [PATCH v3] package/quagga: Add systemd.service file 2016-07-03 8:18 [Buildroot] [PATCH v3] package/quagga: Add systemd.service file Nathaniel Roach 2016-07-03 10:42 ` Maxime Hadjinlian @ 2016-07-03 14:02 ` Peter Korsgaard 1 sibling, 0 replies; 9+ messages in thread From: Peter Korsgaard @ 2016-07-03 14:02 UTC (permalink / raw) To: buildroot >>>>> "Nathaniel" == Nathaniel Roach <nroach44@gmail.com> writes: > 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) > Signed-off-by: Nathaniel Roach <nroach44@gmail.com> > --- > Changes v2 -> v3: > - Remove invalid references to quagga.service (Arnout) > - Check if the binary is executable before trying to start it > (Arnout) > - Remove PID file arguments and options (Arnout) > - Add reload capability as the daemons do support it Committed after removing killmode/killsignal/restartsec as suggested by Maxime, thanks. -- Bye, Peter Korsgaard ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-07-03 14:02 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-07-03 8:18 [Buildroot] [PATCH v3] package/quagga: Add systemd.service file Nathaniel Roach 2016-07-03 10:42 ` Maxime Hadjinlian 2016-07-03 10:54 ` Nathaniel Roach 2016-07-03 12:59 ` Maxime Hadjinlian 2016-07-03 13:02 ` Nathaniel Roach 2016-07-03 13:06 ` Nathaniel Roach 2016-07-03 13:09 ` Maxime Hadjinlian 2016-07-03 13:12 ` Nathaniel Roach 2016-07-03 14:02 ` Peter Korsgaard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox