diff mbox

[v3,09/13] package/dhcp: SysV init scripts: remove PID files after stop

Message ID 1445734779-7212-9-git-send-email-benoit.thebaudeau.dev@gmail.com
State Changes Requested
Headers show

Commit Message

Benoît Thébaudeau Oct. 25, 2015, 12:59 a.m. UTC
From: Benoît Thébaudeau <benoit@wsystem.com>

These daemons do not remove their PID files, so do it manually in the
scripts.

Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>

---
Changes v2 -> v3: none.

Changes v1 -> v2:
 - Rebase.
---
 package/dhcp/S80dhcp-relay  | 12 ++++++++++--
 package/dhcp/S80dhcp-server | 12 ++++++++++--
 2 files changed, 20 insertions(+), 4 deletions(-)

Comments

Thomas Petazzoni Dec. 24, 2015, 2:26 p.m. UTC | #1
Benoît,

On Sun, 25 Oct 2015 02:59:35 +0200, Benoît Thébaudeau wrote:
> From: Benoît Thébaudeau <benoit@wsystem.com>
> 
> These daemons do not remove their PID files, so do it manually in the
> scripts.
> 
> Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>

I think this is not the right way of handling this and the IPv6
problem.

What I would suggest is that the S80dhcp-server script should start two
DHCP servers: one with -4 and one with -6. To avoid IPv6 failures, you
can test if /proc/net/if_inet6 exists.

Then, I was hoping you could use the --remove-pidfile option of
start-stop-daemon, but it unfortunately doesn't exist in the Busybox
implementation of start-stop-daemon.

That being said, I still believe starting two daemons is the best
solution. I don't think a Config.in option should be added for that.
Just always start both, with as I propose an exception to not start the
IPv6 daemon if /proc/net/if_inet6 does not exist.

Could you rework your patches 9 to 13 with this goal in mind ?

Thanks a lot!

Thomas
Benoît Thébaudeau Jan. 7, 2016, 8:40 p.m. UTC | #2
Thomas,

On Thu, Dec 24, 2015 at 3:26 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> On Sun, 25 Oct 2015 02:59:35 +0200, Benoît Thébaudeau wrote:
>> From: Benoît Thébaudeau <benoit@wsystem.com>
>>
>> These daemons do not remove their PID files, so do it manually in the
>> scripts.
>>
>> Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>
>
> I think this is not the right way of handling this and the IPv6
> problem.
>
> What I would suggest is that the S80dhcp-server script should start two
> DHCP servers: one with -4 and one with -6. To avoid IPv6 failures, you
> can test if /proc/net/if_inet6 exists.

OK, I'll do that.

> Then, I was hoping you could use the --remove-pidfile option of
> start-stop-daemon, but it unfortunately doesn't exist in the Busybox
> implementation of start-stop-daemon.

Indeed. I also went that way first. So what are you eventually
suggesting to change for 09/13 since there is apparently no other
solution for SysV? Even with two daemons started, the PID files will
have to be removed with rm for SysV, so 09/13 would only have to be
split. This is already handled for systemd.

> That being said, I still believe starting two daemons is the best
> solution. I don't think a Config.in option should be added for that.
> Just always start both, with as I propose an exception to not start the
> IPv6 daemon if /proc/net/if_inet6 does not exist.
>
> Could you rework your patches 9 to 13 with this goal in mind ?
>
> Thanks a lot!

Will do.

Benoît
diff mbox

Patch

diff --git a/package/dhcp/S80dhcp-relay b/package/dhcp/S80dhcp-relay
index 211431b..9b8d65f 100755
--- a/package/dhcp/S80dhcp-relay
+++ b/package/dhcp/S80dhcp-relay
@@ -17,6 +17,9 @@  OPTIONS=""
 CFG_FILE="/etc/default/dhcrelay"
 [ -r "${CFG_FILE}" ] && . "${CFG_FILE}"
 
+# PID files generated by the daemon
+PID_FILES="/var/run/dhcrelay.pid /var/run/dhcrelay6.pid"
+
 # Sanity checks
 test -f /usr/sbin/dhcrelay || exit 0
 test -n "$INTERFACES" || exit 0
@@ -38,8 +41,13 @@  case "$1" in
 		;;
 	stop)
 		printf "Stopping DHCP relay: "
-		start-stop-daemon -K -q -x /usr/sbin/dhcrelay
-		[ $? = 0 ] && echo "OK" || echo "FAIL"
+		if start-stop-daemon -K -q -x /usr/sbin/dhcrelay; then
+			# This daemon does not remove its PID file when it exits.
+			rm -f ${PID_FILES}
+			echo "OK"
+		else
+			echo "FAIL"
+		fi
 		;;
 	restart | force-reload)
 		$0 stop
diff --git a/package/dhcp/S80dhcp-server b/package/dhcp/S80dhcp-server
index dc9c433..1c2ff74 100755
--- a/package/dhcp/S80dhcp-server
+++ b/package/dhcp/S80dhcp-server
@@ -14,6 +14,9 @@  OPTIONS=""
 CFG_FILE="/etc/default/dhcpd"
 [ -r "${CFG_FILE}" ] && . "${CFG_FILE}"
 
+# PID files generated by the daemon
+PID_FILES="/var/run/dhcpd.pid /var/run/dhcpd6.pid"
+
 # Sanity checks
 test -f /usr/sbin/dhcpd || exit 0
 test -f /etc/dhcp/dhcpd.conf || exit 0
@@ -28,8 +31,13 @@  case "$1" in
 		;;
 	stop)
 		printf "Stopping DHCP server: "
-		start-stop-daemon -K -q -x /usr/sbin/dhcpd
-		[ $? = 0 ] && echo "OK" || echo "FAIL"
+		if start-stop-daemon -K -q -x /usr/sbin/dhcpd; then
+			# This daemon does not remove its PID file when it exits.
+			rm -f ${PID_FILES}
+			echo "OK"
+		else
+			echo "FAIL"
+		fi
 		;;
 	restart | force-reload)
 		$0 stop