[PATCH] add LSB header to xen-watchdog runlevel script

[PATCH] add LSB header to xen-watchdog runlevel script

[Olaf Hering]

Add LSB Header to /etc/init.d/xen-watchdog


Signed-off-by: Olaf Hering <olaf@aepfle.de>

---
 tools/hotplug/Linux/init.d/xen-watchdog |   11 +++++++++++
 1 file changed, 11 insertions(+)

--- xen-unstable.hg-4.1.21836.orig/tools/hotplug/Linux/init.d/xen-watchdog
+++ xen-unstable.hg-4.1.21836/tools/hotplug/Linux/init.d/xen-watchdog
@@ -4,6 +4,17 @@
 #
 # chkconfig: 2345 21 79
 # description: Run domain watchdog daemon
+### BEGIN INIT INFO
+# Provides:          xen-watchdog
+# Required-Start:    xend
+# Should-Start:      $syslog $remote_fs
+# Required-Stop:     xend
+# Should-Stop:       $syslog $remote_fs
+# Default-Start:     3 4 5
+# Default-Stop:      0 1 2 6
+# Short-Description: Start/stop xen-watchdog
+# Description:       Run domain watchdog daemon.
+### END INIT INFO
 #
 
 # Source function library.

Re: [PATCH] add LSB header to xen-watchdog runlevel script

[Ian Jackson]

Olaf Hering writes ("[Xen-devel] [PATCH] add LSB header to xen-watchdog runlevel script"):
> Add LSB Header to /etc/init.d/xen-watchdog
...
> +# Default-Start:     3 4 5

This would have the effect of starting the watchdog by default on some
systems, as I understand it ?  Is that desirable ?

I don't think it's tested in our automatic tests.  Do other people
test and use it ?

Ian.

Re: [PATCH] add LSB header to xen-watchdog runlevel script

[Olaf Hering]

On Fri, Jul 23, Ian Jackson wrote:

> Olaf Hering writes ("[Xen-devel] [PATCH] add LSB header to xen-watchdog runlevel script"):
> > Add LSB Header to /etc/init.d/xen-watchdog
> ...
> > +# Default-Start:     3 4 5
> 
> This would have the effect of starting the watchdog by default on some
> systems, as I understand it ?  Is that desirable ?

This tells the insserv program to create a symlink in the specified
runlevels. xdm for example has only '5', because X11 is only supposed to
be started in runlevel 5.

There is nothing to worry about here.

Olaf

Re: [PATCH] add LSB header to xen-watchdog runlevel script

[Ian Jackson]

Olaf Hering writes ("Re: [Xen-devel] [PATCH] add LSB header to xen-watchdog runlevel script"):
> This tells the insserv program to create a symlink in the specified
> runlevels. xdm for example has only '5', because X11 is only supposed to
> be started in runlevel 5.

Yes, but the question is: do we want to start the watchdog by
default ?  Previously we didn't.

> There is nothing to worry about here.

I'm not so sure ...

Ian.

Re: [PATCH] add LSB header to xen-watchdog runlevel script

[Olaf Hering]

On Fri, Jul 23, Ian Jackson wrote:

> Olaf Hering writes ("Re: [Xen-devel] [PATCH] add LSB header to xen-watchdog runlevel script"):
> > This tells the insserv program to create a symlink in the specified
> > runlevels. xdm for example has only '5', because X11 is only supposed to
> > be started in runlevel 5.
> 
> Yes, but the question is: do we want to start the watchdog by
> default ?  Previously we didn't.

Thats up to the packager of the binary package.

Or do you mean there should be no runlevel script in the first place?


Olaf

Re: [PATCH] add LSB header to xen-watchdog runlevel script

[Tim Deegan]

At 17:45 +0100 on 23 Jul (1279907152), Ian Jackson wrote:
> I don't think it's tested in our automatic tests.  Do other people
> test and use it ?

XenServer uses it as part of its high-availability mechanism.

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

Re: [PATCH] add LSB header to xen-watchdog runlevel script [and 1 more messages]

[Ian Jackson]

Olaf Hering writes ("Re: [Xen-devel] [PATCH] add LSB header to xen-watchdog runlevel script"):
> On Fri, Jul 23, Ian Jackson wrote:
> > Olaf Hering writes ("Re: [Xen-devel] [PATCH] add LSB header to xen-watchdog runlevel script"):
> > > This tells the insserv program to create a symlink in the specified
> > > runlevels. xdm for example has only '5', because X11 is only supposed to
> > > be started in runlevel 5.
> > 
> > Yes, but the question is: do we want to start the watchdog by
> > default ?  Previously we didn't.
> 
> Thats up to the packager of the binary package.
> Or do you mean there should be no runlevel script in the first place?

I meant: adding these lines would seem to be likely to cause some
systems to automatically start the watchdog when previously they would
not have done so.  I wanted to know whether that was a good idea.

Tim Deegan writes ("Re: [Xen-devel] [PATCH] add LSB header to xen-watchdog runlevel script"):
> At 17:45 +0100 on 23 Jul (1279907152), Ian Jackson wrote:
> > I don't think it's tested in our automatic tests.  Do other people
> > test and use it ?
> 
> XenServer uses it as part of its high-availability mechanism.

That's good enough for me.  I'll apply Olaf's patch.

Thanks,
Ian.

Re: [PATCH] add LSB header to xen-watchdog runlevel script [and 1 more messages]

[Ian Campbell]

On Mon, 2010-07-26 at 11:08 +0100, Ian Jackson wrote:
> Olaf Hering writes ("Re: [Xen-devel] [PATCH] add LSB header to xen-watchdog runlevel script"):
> > On Fri, Jul 23, Ian Jackson wrote:
> > > Olaf Hering writes ("Re: [Xen-devel] [PATCH] add LSB header to xen-watchdog runlevel script"):
> > > > This tells the insserv program to create a symlink in the specified
> > > > runlevels. xdm for example has only '5', because X11 is only supposed to
> > > > be started in runlevel 5.
> > > 
> > > Yes, but the question is: do we want to start the watchdog by
> > > default ?  Previously we didn't.
> > 
> > Thats up to the packager of the binary package.
> > Or do you mean there should be no runlevel script in the first place?
> 
> I meant: adding these lines would seem to be likely to cause some
> systems to automatically start the watchdog when previously they would
> not have done so.  I wanted to know whether that was a good idea.

IMHO no -- enabling a watchdog should be an explicit admin action.

Ian.

Re: [PATCH] add LSB header to xen-watchdog runlevel script [and 1 more messages]

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] [PATCH] add LSB header to xen-watchdog runlevel script [and 1 more messages]"):
> On Mon, 2010-07-26 at 11:08 +0100, Ian Jackson wrote:
> > I meant: adding these lines would seem to be likely to cause some
> > systems to automatically start the watchdog when previously they would
> > not have done so.  I wanted to know whether that was a good idea.
> 
> IMHO no -- enabling a watchdog should be an explicit admin action.

So I conclude that you think the patch is wrong ?

Ian.

Re: [PATCH] add LSB header to xen-watchdog runlevel script [and 1 more messages]

[Ian Campbell]

On Mon, 2010-07-26 at 11:22 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [PATCH] add LSB header to xen-watchdog runlevel script [and 1 more messages]"):
> > On Mon, 2010-07-26 at 11:08 +0100, Ian Jackson wrote:
> > > I meant: adding these lines would seem to be likely to cause some
> > > systems to automatically start the watchdog when previously they would
> > > not have done so.  I wanted to know whether that was a good idea.
> > 
> > IMHO no -- enabling a watchdog should be an explicit admin action.
> 
> So I conclude that you think the patch is wrong ?

If it causes the watchdog to be enabled then yes.

However, looking back over the whole thread I think Olaf suggested that
just adding this header won't do that and that the admin (or the
packages) has to add an explicit request (I guess to chkconfig or
something similar) to cause the header to have any actual effect, is
that right?

BTW, from the original patch:
+# Required-Start:    xend

Is that accurate? I'd have thought the watchdog ought to function
independent of xend, e.g. on a pure xl system. I don't see anything to
the contrary in a quick glance through the code.

Ian.

Re: [PATCH] add LSB header to xen-watchdog runlevel script [and 1 more messages]

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] [PATCH] add LSB header to xen-watchdog runlevel script [and 1 more messages]"):
> On Mon, 2010-07-26 at 11:22 +0100, Ian Jackson wrote:
> > So I conclude that you think the patch is wrong ?
> 
> If it causes the watchdog to be enabled then yes.

LSB 3.2 Core 20.3 [1]
says:

  Default-Start: run_level_1 [run_level_2...]
  Default-Stop: run_level_1 [run_level_2...]

      which run levels should by default run the init script with a
      start (stop) argument to start (stop) the services controlled by
      the init script

      For example, if a service should run in runlevels 3,4, and 5
      only, specify "Default-Start 3 4 5" and "Default-Stop: 0 1 2 6"

That seems fairly clear that it's supposed to control the default.
I don't know whether any distros automatically honour these settings
but I wouldn't be surprised if some of the fancy new parallel init
systems did.

[1] http://refspecs.linux-foundation.org/LSB_3.2.0/LSB-Core-generic/LSB-Core-generic/initscrcomconv.html

> BTW, from the original patch:
> +# Required-Start:    xend
> 
> Is that accurate? I'd have thought the watchdog ought to function
> independent of xend, e.g. on a pure xl system. I don't see anything to
> the contrary in a quick glance through the code.

You are quite right.

Ian.

Re: [PATCH] add LSB header to xen-watchdog runlevel script [and 1 more messages]

[Olaf Hering]

On Mon, Jul 26, Ian Jackson wrote:

> Ian Campbell writes ("Re: [Xen-devel] [PATCH] add LSB header to xen-watchdog runlevel script [and 1 more messages]"):
> > On Mon, 2010-07-26 at 11:08 +0100, Ian Jackson wrote:
> > > I meant: adding these lines would seem to be likely to cause some
> > > systems to automatically start the watchdog when previously they would
> > > not have done so.  I wanted to know whether that was a good idea.
> > 
> > IMHO no -- enabling a watchdog should be an explicit admin action.
> 
> So I conclude that you think the patch is wrong ?

Ian,

the patch is not wrong. If it were, the chkconfig line had to be removed
as well. Both lines just tell the tools about the ordering in the
specifieed rulevels, when the admin actually wants to have the script
enabled by calling 'chkconfig -a xen-watchdog' or 'insserv watchdog'.

Olaf