Bug#538133: Your mon / s-p-u upload

2011-02-27 Thread Julien Cristau 
On Mon, Jan 24, 2011 at 22:56:08 +0100, Dario Minnucci wrote:

 +case $1 in
 +  start)
 + if [ -f $PIDFILE ] ; then
 + echo $NAME daemon is already running. 
 + else
 + start_deamon
 + fi
 + ;;

Dario, this is still not correct.  Existence of the pid file alone is
not proof that the daemon is running.  You should remove this check, and
let start-stop-daemon handle this.

Cheers,
Julien



-- 
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org

Bug#538133: Your mon / s-p-u upload

2011-01-24 Thread Dario Minnucci 

Hi Team,

Sorry for taking s long

On 11/01/2010 10:57 PM, Adam D. Barratt wrote:
 Hi,
 
 Many apologies for letting this keep slipping down my to-do list.
 
 On Thu, 2010-09-02 at 14:24 +0200, Dario Minnucci wrote:
 Hi again,

 On 08/25/2010 08:53 PM, Adam D. Barratt wrote:
 On Sun, 2010-07-18 at 17:33 +0200, Dario Minnucci wrote:
 [...]
 +case $1 in
 +  start)
 +   if [ -f $PIDFILE ] ; then
 +   echo $NAME daemon is already running. 
 +   else
 +   start_deamon
 +   fi

 The short-circuit case should be removed here; the existence of the
 pidfile does not imply that the daemon is (still) running and your
 start-stop-daemon call in start_daemon() already handles exiting
 successfully if the daemon is in fact running.

 I've tried what you suggest here but if I don't check for the
 existence of the PID file and the
 daemon is already running, starting it again fails.

 root@host:~# /etc/init.d/mon start ; echo $?
 Starting monitor daemon: 1
 
 A little debugging this evening suggests that this is a side-effect of
 mon being a perl script.
 
 On the first call to the init script, s-s-d checks that the PID file
 does not exist, and that there are no processes called /usr/sbin/mon
 running and duly starts one.  On the second time around, it finds the
 PID file and continues to check whether any instances of /usr/sbin/mon
 exist; they don't, as what's actually running is
 /usr/bin/perl /usr/sbin/mon   So it tries starting a second mon
 instance, which itself exit 1s as it's unable to bind to the port,
 which is occupied by the first instance.
 
 Changing --exec to --startas avoids this problem, although it doesn't
 rule out the possibility that the daemon has crashed and another process
 is then using the PID mentioned in the PID file.  Then again,
 stop_daemon() isn't using --exec anyway, so the script is already happy
 to stop whatever process happens to be on that PID.
 
 Regards,
 
 Adam
 


Debdiff attached.

I've modified the init script to use --startas and seems it works as expected.

Please, let me know if is possible to upload this s-p-u.

Regards,


-- 
 Dario Minnucci mid...@debian.org
 Phone: +34 902021030 | Fax: +34 902024417
 Key fingerprint = BAA1 7AAF B21D 6567 D457  D67D A82F BB83 F3D5 7033

diff -u mon-0.99.2/debian/mon.init.d mon-0.99.2/debian/mon.init.d
--- mon-0.99.2/debian/mon.init.d
+++ mon-0.99.2/debian/mon.init.d
@@ -44,16 +44,30 @@
 
 set -e
 
-case $1 in
-  start)
+
+function start_deamon {
echo -n Starting $DESC: 
-   start-stop-daemon --start --quiet --pidfile $PIDFILE --chuid $USER 
--group $GROUP --exec $DAEMON -- $DAEMON_OPTS
+   start-stop-daemon --start --oknodo --pidfile $PIDFILE --chuid $USER 
--group $GROUP --startas $DAEMON -- $DAEMON_OPTS
echo $NAME.
-   ;;
-  stop)
+}
+function stop_daemon {
echo -n Stopping $DESC: 
-   start-stop-daemon --stop --quiet --pidfile $PIDFILE 
+   start-stop-daemon --stop --oknodo --pidfile $PIDFILE 
echo $NAME.
+}
+
+
+
+case $1 in
+  start)
+   if [ -f $PIDFILE ] ; then
+   echo $NAME daemon is already running. 
+   else
+   start_deamon
+   fi
+   ;;
+  stop)
+   stop_daemon
;;
   #reload)
#
@@ -75,16 +89,14 @@
#   daemon isn't already running.
# check wether $DAEMON is running. If so, restart
start-stop-daemon --stop --test --quiet --pidfile \
-   $PIDFILE --exec $DAEMON \
+   $PIDFILE --startas $DAEMON \
 $0 restart \
|| exit 0
;;
   restart)
-echo -n Restarting $DESC: 
-   start-stop-daemon --stop --quiet --pidfile $PIDFILE 
-   sleep 1
-   start-stop-daemon --start --quiet --pidfile $PIDFILE --chuid $USER 
--group $GROUP --exec $DAEMON -- $DAEMON_OPTS
-   echo $NAME.
+   stop_daemon
+   sleep 1
+   start_deamon
;;
   *)
N=/etc/init.d/$NAME
diff -u mon-0.99.2/debian/changelog mon-0.99.2/debian/changelog
--- mon-0.99.2/debian/changelog
+++ mon-0.99.2/debian/changelog
@@ -1,3 +1,10 @@
+mon (0.99.2-13+lenny1) stable-proposed-updates; urgency=low
+
+  * debian/mon.init.d: Script fixes to return success when daemon 
+is restarted but is already running. (Closes: #538133)
+
+ -- Dario Minnucci mid...@debian.org  Sun, 18 Jul 2010 17:09:04 +0200
+
 mon (0.99.2-13) unstable; urgency=low
 
   * debian/control: Conforms with latest Standards Version 3.8.0


signature.asc
Description: OpenPGP digital signature

Bug#538133: Your mon / s-p-u upload

2010-11-01 Thread Adam D. Barratt 
Hi,

Many apologies for letting this keep slipping down my to-do list.

On Thu, 2010-09-02 at 14:24 +0200, Dario Minnucci wrote:
 Hi again,
 
 On 08/25/2010 08:53 PM, Adam D. Barratt wrote:
  On Sun, 2010-07-18 at 17:33 +0200, Dario Minnucci wrote:
[...]
  +case $1 in
  +  start)
  +   if [ -f $PIDFILE ] ; then
  +   echo $NAME daemon is already running. 
  +   else
  +   start_deamon
  +   fi
  
  The short-circuit case should be removed here; the existence of the
  pidfile does not imply that the daemon is (still) running and your
  start-stop-daemon call in start_daemon() already handles exiting
  successfully if the daemon is in fact running.
  
 I've tried what you suggest here but if I don't check for the
 existence of the PID file and the
 daemon is already running, starting it again fails.
 
 r...@host:~# /etc/init.d/mon start ; echo $?
 Starting monitor daemon: 1

A little debugging this evening suggests that this is a side-effect of
mon being a perl script.

On the first call to the init script, s-s-d checks that the PID file
does not exist, and that there are no processes called /usr/sbin/mon
running and duly starts one.  On the second time around, it finds the
PID file and continues to check whether any instances of /usr/sbin/mon
exist; they don't, as what's actually running is
/usr/bin/perl /usr/sbin/mon   So it tries starting a second mon
instance, which itself exit 1s as it's unable to bind to the port,
which is occupied by the first instance.

Changing --exec to --startas avoids this problem, although it doesn't
rule out the possibility that the daemon has crashed and another process
is then using the PID mentioned in the PID file.  Then again,
stop_daemon() isn't using --exec anyway, so the script is already happy
to stop whatever process happens to be on that PID.

Regards,

Adam




-- 
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org

Bug#538133: Your mon / s-p-u upload

2010-09-02 Thread Dario Minnucci 
Hi again,

On 08/25/2010 08:53 PM, Adam D. Barratt wrote:
 On Sun, 2010-07-18 at 17:33 +0200, Dario Minnucci wrote:
 On 06/19/2010 11:51 AM, Adam D. Barratt wrote:
 I've been reviewing the few remaining packages in s-p-u in
 preparation for the upcoming point release and had a couple of
 comments / queries on your mon upload.
 [...]
 As suggested, I'm sending this email to debian-release with the
 debdiff attached.

 The script seems to be Policy compliant now.
 
 Sorry for not getting back to you sooner; I seem to have managed to
 misfile your mail.
 


Same here... comming back from holidays is slow and painfull ;-)


 +case $1 in
 +  start)
 +   if [ -f $PIDFILE ] ; then
 +   echo $NAME daemon is already running. 
 +   else
 +   start_deamon
 +   fi
 
 The short-circuit case should be removed here; the existence of the
 pidfile does not imply that the daemon is (still) running and your
 start-stop-daemon call in start_daemon() already handles exiting
 successfully if the daemon is in fact running.
 


I've tried what you suggest here but if I don't check for the existence of the 
PID file and the
daemon is already running, starting it again fails.

r...@host:~# /etc/init.d/mon start ; echo $?
Starting monitor daemon: 1


restart)
 [...]
 +   if [ -f $PIDFILE ] ; then
 +   stop_daemon
 +   sleep 1
 +   start_deamon
 +   else
 +   echo $NAME daemon is not running. 
 +   start_deamon
 +   fi
 
 Similarly here.  If the daemon is not running, then stop_daemon() will
 successfully fail to stop it with no ill effects.  Admittedly, if the
 pidfile doesn't exist then stop_daemon() is basically a no-op, but not
 simply calling stop_daemon() followed by start_daemon() in all cases
 doesn't really provide any benefit.


Not checking for the PID file here is OK, because stop_daemon() is called 
before start_daemon().


The attached patch reflecting these changes produces:

--- Tests ---
r...@host:~# /etc/init.d/mon stop ; echo $?
Stopping monitor daemon: mon.
0
r...@host:~# /etc/init.d/mon stop ; echo $?
Stopping monitor daemon: No process in pidfile `/var/run/mon/mon.pid' found 
running; none killed.
mon.
0
r...@host:~# /etc/init.d/mon start ; echo $?
Starting monitor daemon: mon.
0
r...@host:~# /etc/init.d/mon start ; echo $?
mon daemon is already running.
0
r...@host:~# /etc/init.d/mon restart ; echo $?
Stopping monitor daemon: mon.
Starting monitor daemon: mon.
0
r...@host:~# /etc/init.d/mon restart ; echo $?
Stopping monitor daemon: mon.
Starting monitor daemon: mon.
0
r...@host:~# /etc/init.d/mon stop ; echo $?
Stopping monitor daemon: mon.
0
r...@host:~# /etc/init.d/mon restart ; echo $?
Stopping monitor daemon: No process in pidfile `/var/run/mon/mon.pid' found 
running; none killed.
mon.
Starting monitor daemon: mon.
0

--- /Tests ---

Thanks for your time.


-- 
 Dario Minnucci mid...@debian.org
 Phone: +34 902021030 | Fax: +34 902024417
 Key fingerprint = BAA1 7AAF B21D 6567 D457  D67D A82F BB83 F3D5 7033

diff -u mon-0.99.2/debian/mon.init.d mon-0.99.2/debian/mon.init.d
--- mon-0.99.2/debian/mon.init.d
+++ mon-0.99.2/debian/mon.init.d
@@ -44,16 +44,30 @@
 
 set -e
 
-case $1 in
-  start)
+
+function start_deamon {
 	echo -n Starting $DESC: 
-	start-stop-daemon --start --quiet --pidfile $PIDFILE --chuid $USER --group $GROUP --exec $DAEMON -- $DAEMON_OPTS
+	start-stop-daemon --start --oknodo --pidfile $PIDFILE --chuid $USER --group $GROUP --exec $DAEMON -- $DAEMON_OPTS
 	echo $NAME.
-	;;
-  stop)
+}
+function stop_daemon {
 	echo -n Stopping $DESC: 
-	start-stop-daemon --stop --quiet --pidfile $PIDFILE 
+	start-stop-daemon --stop --oknodo --pidfile $PIDFILE 
 	echo $NAME.
+}
+
+
+
+case $1 in
+  start)
+	if [ -f $PIDFILE ] ; then
+		echo $NAME daemon is already running.	
+	else
+		start_deamon
+	fi
+	;;
+  stop)
+		stop_daemon
 	;;
   #reload)
 	#
@@ -80,11 +94,9 @@
 	|| exit 0
 	;;
   restart)
-echo -n Restarting $DESC: 
-	start-stop-daemon --stop --quiet --pidfile $PIDFILE 
-	sleep 1
-	start-stop-daemon --start --quiet --pidfile $PIDFILE --chuid $USER --group $GROUP --exec $DAEMON -- $DAEMON_OPTS
-	echo $NAME.
+		stop_daemon
+		sleep 1
+		start_deamon
 	;;
   *)
 	N=/etc/init.d/$NAME
diff -u mon-0.99.2/debian/changelog mon-0.99.2/debian/changelog
--- mon-0.99.2/debian/changelog
+++ mon-0.99.2/debian/changelog
@@ -1,3 +1,10 @@
+mon (0.99.2-13+lenny1) stable-proposed-updates; urgency=low
+
+  * debian/mon.init.d: Script fixes to return success when daemon 
+is restarted but is already running. (Closes: #538133)
+
+ -- Dario Minnucci mid...@debian.org  Sun, 18 Jul 2010 17:09:04 +0200
+
 mon (0.99.2-13) unstable; urgency=low
 
   * debian/control: Conforms with latest Standards Version 3.8.0


signature.asc
Description: OpenPGP digital signature

Bug#538133: Your mon / s-p-u upload

2010-08-25 Thread Adam D. Barratt 
On Sun, 2010-07-18 at 17:33 +0200, Dario Minnucci wrote:
 On 06/19/2010 11:51 AM, Adam D. Barratt wrote:
  I've been reviewing the few remaining packages in s-p-u in
  preparation for the upcoming point release and had a couple of
  comments / queries on your mon upload.
[...]
 As suggested, I'm sending this email to debian-release with the
 debdiff attached.
 
 The script seems to be Policy compliant now.

Sorry for not getting back to you sooner; I seem to have managed to
misfile your mail.

 +case $1 in
 +  start)
 +   if [ -f $PIDFILE ] ; then
 +   echo $NAME daemon is already running. 
 +   else
 +   start_deamon
 +   fi

The short-circuit case should be removed here; the existence of the
pidfile does not imply that the daemon is (still) running and your
start-stop-daemon call in start_daemon() already handles exiting
successfully if the daemon is in fact running.

restart)
[...]
 +   if [ -f $PIDFILE ] ; then
 +   stop_daemon
 +   sleep 1
 +   start_deamon
 +   else
 +   echo $NAME daemon is not running. 
 +   start_deamon
 +   fi

Similarly here.  If the daemon is not running, then stop_daemon() will
successfully fail to stop it with no ill effects.  Admittedly, if the
pidfile doesn't exist then stop_daemon() is basically a no-op, but not
simply calling stop_daemon() followed by start_daemon() in all cases
doesn't really provide any benefit.

Regards,

Adam



--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org

Bug#538133: Your mon / s-p-u upload

2010-07-18 Thread Dario Minnucci 
Hi Team,

On 06/19/2010 11:51 AM, Adam D. Barratt wrote:
 Hi,
 
 I've been reviewing the few remaining packages in s-p-u in preparation
 for the upcoming point release and had a couple of comments / queries on
 your mon upload.
 
 - echo -n Starting $DESC: 
 - start-stop-daemon --start --quiet --pidfile $PIDFILE --chuid $USER 
 --group $GROUP --exec $DAEMON -- $DAEMON_OPTS
 - echo $NAME.
 + if [ -f $PIDFILE ] ; then
 + echo $NAME daemon is already running. 
 + else
 + echo -n Starting $DESC: 
 + start-stop-daemon --start --oknodo --pidfile $PIDFILE --chuid 
 $USER --group $GROUP --exec $DAEMON -- $DAEMON_OPTS
 + echo $NAME.
 + fi
 
 This looks like it should work, but would simply running the
 start-stop-daemon --start --oknodo call in all cases not work, given
 that it uses the pid file?
 
 The restart section, on the other hand, is still not policy compliant:
 
 restart)
 -echo -n Restarting $DESC: 
 - start-stop-daemon --stop --quiet --pidfile $PIDFILE 
 - sleep 1
 - start-stop-daemon --start --quiet --pidfile $PIDFILE --chuid $USER 
 --group $GROUP --exec $DAEMON -- $DAEMON_OPTS
 - echo $NAME.
 + if [ -f $PIDFILE ] ; then
 + echo -n Restarting $DESC: 
 + start-stop-daemon --stop --quiet --pidfile $PIDFILE 
 + sleep 1
 + start-stop-daemon --start --quiet --pidfile $PIDFILE --chuid 
 $USER --group $GROUP --exec $DAEMON -- $DAEMON_OPTS
 + echo $NAME.
 + else
 + echo $NAME daemon is not running. 
 + echo To start $NAME run: /etc/init.d/mon start
 + fi
 
 Policy 9.3.2 says:
 
 `restart'
   stop and restart the service if it's already running, otherwise
   start the service
 
 which your updated script does not fulfil (specifically the otherwise
 start the service section).
 
 For future reference, it would be appreciated if you could send a mail
 containing a debdiff and a brief explanation of the changes you'd like
 to make to debian-rele...@lists.debian.org and wait for an ok, rather
 than uploading directly to s-p-u; this allows us to review the patch and
 makes it easier to incorporate any required changes.
 
 Regards,
 
 Adam

Sorry, that was my first s-p-u upload :(

As suggested, I'm sending this email to debian-release with the debdiff 
attached.

The script seems to be Policy compliant now.

I keep on waiting for the OK to upload this new version.

Sorry for the inconvenience

Cheers

-- 
 Dario Minnucci mid...@debian.org
 Phone: +34 902021030 | Fax: +34 902024417
 Key fingerprint = BAA1 7AAF B21D 6567 D457  D67D A82F BB83 F3D5 7033

diff -u mon-0.99.2/debian/mon.init.d mon-0.99.2/debian/mon.init.d
--- mon-0.99.2/debian/mon.init.d
+++ mon-0.99.2/debian/mon.init.d
@@ -44,16 +44,30 @@
 
 set -e
 
-case $1 in
-  start)
+
+function start_deamon {
 	echo -n Starting $DESC: 
-	start-stop-daemon --start --quiet --pidfile $PIDFILE --chuid $USER --group $GROUP --exec $DAEMON -- $DAEMON_OPTS
+	start-stop-daemon --start --oknodo --pidfile $PIDFILE --chuid $USER --group $GROUP --exec $DAEMON -- $DAEMON_OPTS
 	echo $NAME.
-	;;
-  stop)
+}
+function stop_daemon {
 	echo -n Stopping $DESC: 
-	start-stop-daemon --stop --quiet --pidfile $PIDFILE 
+	start-stop-daemon --stop --oknodo --pidfile $PIDFILE 
 	echo $NAME.
+}
+
+
+
+case $1 in
+  start)
+	if [ -f $PIDFILE ] ; then
+		echo $NAME daemon is already running.	
+	else
+		start_deamon
+	fi
+	;;
+  stop)
+		stop_daemon
 	;;
   #reload)
 	#
@@ -80,11 +94,14 @@
 	|| exit 0
 	;;
   restart)
-echo -n Restarting $DESC: 
-	start-stop-daemon --stop --quiet --pidfile $PIDFILE 
-	sleep 1
-	start-stop-daemon --start --quiet --pidfile $PIDFILE --chuid $USER --group $GROUP --exec $DAEMON -- $DAEMON_OPTS
-	echo $NAME.
+	if [ -f $PIDFILE ] ; then
+		stop_daemon
+		sleep 1
+		start_deamon
+	else
+		echo $NAME daemon is not running.	
+		start_deamon
+	fi
 	;;
   *)
 	N=/etc/init.d/$NAME
diff -u mon-0.99.2/debian/changelog mon-0.99.2/debian/changelog
--- mon-0.99.2/debian/changelog
+++ mon-0.99.2/debian/changelog
@@ -1,3 +1,10 @@
+mon (0.99.2-13+lenny1) stable-proposed-updates; urgency=low
+
+  * debian/mon.init.d: Script fixes to return success when daemon 
+is restarted but is already running. (Closes: #538133)
+
+ -- Dario Minnucci mid...@debian.org  Sun, 18 Jul 2010 17:09:04 +0200
+
 mon (0.99.2-13) unstable; urgency=low
 
   * debian/control: Conforms with latest Standards Version 3.8.0


signature.asc
Description: OpenPGP digital signature

Bug#538133: Your mon / s-p-u upload

2010-06-19 Thread Adam D. Barratt 
Hi,

I've been reviewing the few remaining packages in s-p-u in preparation
for the upcoming point release and had a couple of comments / queries on
your mon upload.

-   echo -n Starting $DESC: 
-   start-stop-daemon --start --quiet --pidfile $PIDFILE --chuid $USER 
--group $GROUP --exec $DAEMON -- $DAEMON_OPTS
-   echo $NAME.
+   if [ -f $PIDFILE ] ; then
+   echo $NAME daemon is already running. 
+   else
+   echo -n Starting $DESC: 
+   start-stop-daemon --start --oknodo --pidfile $PIDFILE --chuid 
$USER --group $GROUP --exec $DAEMON -- $DAEMON_OPTS
+   echo $NAME.
+   fi

This looks like it should work, but would simply running the
start-stop-daemon --start --oknodo call in all cases not work, given
that it uses the pid file?

The restart section, on the other hand, is still not policy compliant:

restart)
-echo -n Restarting $DESC: 
-   start-stop-daemon --stop --quiet --pidfile $PIDFILE 
-   sleep 1
-   start-stop-daemon --start --quiet --pidfile $PIDFILE --chuid $USER 
--group $GROUP --exec $DAEMON -- $DAEMON_OPTS
-   echo $NAME.
+   if [ -f $PIDFILE ] ; then
+   echo -n Restarting $DESC: 
+   start-stop-daemon --stop --quiet --pidfile $PIDFILE 
+   sleep 1
+   start-stop-daemon --start --quiet --pidfile $PIDFILE --chuid 
$USER --group $GROUP --exec $DAEMON -- $DAEMON_OPTS
+   echo $NAME.
+   else
+   echo $NAME daemon is not running. 
+   echo To start $NAME run: /etc/init.d/mon start
+   fi

Policy 9.3.2 says:

`restart'
  stop and restart the service if it's already running, otherwise
  start the service

which your updated script does not fulfil (specifically the otherwise
start the service section).

For future reference, it would be appreciated if you could send a mail
containing a debdiff and a brief explanation of the changes you'd like
to make to debian-rele...@lists.debian.org and wait for an ok, rather
than uploading directly to s-p-u; this allows us to review the patch and
makes it easier to incorporate any required changes.

Regards,

Adam



--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org