[PATCH] multipath: don't let init script stop multipathd for root devices

[PATCH] multipath: don't let init script stop multipathd for root devices

[Benjamin Marzinski]

This patch modifies the redhat init script, so that it doesn't allow
multipathd to be stopped when the root device is on it.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
:100644 100644 ee776f0... e46b1b8... M	multipathd/multipathd.init.redhat
 multipathd/multipathd.init.redhat |   38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

Index: multipath-tools-100322/multipathd/multipathd.init.redhat
===================================================================
--- multipath-tools-100322.orig/multipathd/multipathd.init.redhat
+++ multipath-tools-100322/multipathd/multipathd.init.redhat
@@ -2,7 +2,7 @@
 #
 # multipathd	Starts the multipath daemon
 #
-# chkconfig: - 13 87
+# chkconfig: - 06 87
 # description: Manages device-mapper multipath devices
 
 ### BEGIN INIT INFO
@@ -17,6 +17,7 @@ prog=`basename $DAEMON`
 initdir=/etc/rc.d/init.d
 lockdir=/var/lock/subsys
 sysconfig=/etc/sysconfig
+syspath=/sys/block
 
  
 . $initdir/functions
@@ -25,6 +26,36 @@ test -r $sysconfig/$prog && . $sysconfig
 
 RETVAL=0
 
+teardown_slaves()
+{
+pushd $1 > /dev/null
+if [ -d "slaves" ]; then
+for slave in slaves/*;
+do
+	if [ "$slave" = "slaves/*" ]; then
+		read dev <  $1/dev
+		tablename=`dmsetup table --target multipath | sed -n "s/\(.*\): .* $dev .*/\1/p"`
+		if ! [ -z $tablename ]; then
+			echo "Root is on a multipathed device, multipathd can not be stopped"
+			exit 1
+		fi
+	else
+		local_slave=`readlink -f $slave`;
+		teardown_slaves $local_slave;
+	fi
+	done
+
+else
+		read dev <  $1/dev
+		tablename=`dmsetup table --target multipath | sed -n "s/\(.*\): .* $dev .*/\1/p"`
+		if ! [ -z $tablename ]; then
+			echo "Root is on a multipathed device, multipathd can not be stopped"
+			exit 1
+		fi
+fi
+popd > /dev/null
+}
+
 #
 # See how we were called.
 #
@@ -39,6 +70,11 @@ start() {
 }
 
 stop() {
+        root_dev=$(awk '{ if ($1 !~ /^[ \t]*#/ && $2 == "/") { print $1; }}' /etc/mtab)
+	dm_num=`dmsetup info -c --noheadings -o minor $root_dev`
+	root_dm_device="dm-$dm_num"
+	[ -d $syspath/$root_dm_device ] && teardown_slaves $syspath/$root_dm_device
+
 	echo -n $"Stopping $prog daemon: "
 	killproc $DAEMON
 	RETVAL=$?

Re: [PATCH] multipath: don't let init script stop multipathd for root devices

[Kiyoshi Ueda]

Hi Ben,

On 03/26/2010 02:52 AM +0900, Benjamin Marzinski wrote:
> This patch modifies the redhat init script, so that it doesn't allow
> multipathd to be stopped when the root device is on it.

Why do you need to prevent stopping daemon by script-level forcibly?
I think that users/developers may want to restart the daemon at their
own risk when it starts to behave something wrong or they change
a configuration (since "reconfigure" may not be stable feature).

# Maybe others have different opinion but I often use "restart".

If you need this patch anyway, please add other options to stop/restart
the daemon forcibly.  (e.g. adding force-stop/force-restart)

Thanks,
Kiyoshi Ueda

Re: [PATCH] multipath: don't let init script stop multipathd for root devices

[Benjamin Marzinski]

On Fri, Mar 26, 2010 at 11:04:22AM +0900, Kiyoshi Ueda wrote:
> Hi Ben,
> 
> On 03/26/2010 02:52 AM +0900, Benjamin Marzinski wrote:
> > This patch modifies the redhat init script, so that it doesn't allow
> > multipathd to be stopped when the root device is on it.
> 
> Why do you need to prevent stopping daemon by script-level forcibly?
> I think that users/developers may want to restart the daemon at their
> own risk when it starts to behave something wrong or they change
> a configuration (since "reconfigure" may not be stable feature).
> 
> # Maybe others have different opinion but I often use "restart".
> 
> If you need this patch anyway, please add other options to stop/restart
> the daemon forcibly.  (e.g. adding force-stop/force-restart)

I can certainly add a force-stop/force-restart.  This can currently be
accomplished by simply killing the daemon, but admittedly doing it
through the init script is cleaner and more obvious to users, and it's
necessary to allow package upgrades to immediately start using
multipathd on systems with multipathed root disks.

However, what is there that you need to restart the daemon for that

# service multipathd reload

won't fix, besides upgrading the package.  If there is something, then
we should fix it.  I only stop the daemon when I'm testing shutting
down, and upgrading source. Otherwise, I've been using reconfigure
without problems.

-Ben

> 
> Thanks,
> Kiyoshi Ueda

Re: [PATCH] multipath: don't let init script stop multipathd for root devices

[Kiyoshi Ueda]

Hi Ben,

On 03/27/2010 01:23 AM +0900, Benjamin Marzinski wrote:
> On Fri, Mar 26, 2010 at 11:04:22AM +0900, Kiyoshi Ueda wrote:
>> Hi Ben,
>>
>> On 03/26/2010 02:52 AM +0900, Benjamin Marzinski wrote:
>>> This patch modifies the redhat init script, so that it doesn't allow
>>> multipathd to be stopped when the root device is on it.
>> Why do you need to prevent stopping daemon by script-level forcibly?
>> I think that users/developers may want to restart the daemon at their
>> own risk when it starts to behave something wrong or they change
>> a configuration (since "reconfigure" may not be stable feature).
>>
>> # Maybe others have different opinion but I often use "restart".
>>
>> If you need this patch anyway, please add other options to stop/restart
>> the daemon forcibly.  (e.g. adding force-stop/force-restart)
> 
> I can certainly add a force-stop/force-restart.  This can currently be
> accomplished by simply killing the daemon, but admittedly doing it
> through the init script is cleaner and more obvious to users, and it's
> necessary to allow package upgrades to immediately start using
> multipathd on systems with multipathed root disks.
> 
> However, what is there that you need to restart the daemon for that
> 
> # service multipathd reload
> 
> won't fix, besides upgrading the package.  If there is something, then
> we should fix it.  I only stop the daemon when I'm testing shutting
> down, and upgrading source. Otherwise, I've been using reconfigure
> without problems.

I may be confusing you.
I know/understand that current reconfigure works well and if it doesn't
work we should fix it.

My point is that "why do you need to prevent stopping daemon
by script-level forcibly?".
As you mentioned above, killing the daemon through the init script
is cleaner and more obvious.  So I think leaving something to stop
the daemon in the init script is more friendly for admins.

Thanks,
Kiyoshi Ueda

Re: [PATCH] multipath: don't let init script stop multipathd for root devices

[Benjamin Marzinski]

On Tue, Mar 30, 2010 at 09:11:13AM +0900, Kiyoshi Ueda wrote:
> Hi Ben,
> 
> On 03/27/2010 01:23 AM +0900, Benjamin Marzinski wrote:
> > On Fri, Mar 26, 2010 at 11:04:22AM +0900, Kiyoshi Ueda wrote:
> >> Hi Ben,
> >>
> >> On 03/26/2010 02:52 AM +0900, Benjamin Marzinski wrote:
> >>> This patch modifies the redhat init script, so that it doesn't allow
> >>> multipathd to be stopped when the root device is on it.
> >> Why do you need to prevent stopping daemon by script-level forcibly?
> >> I think that users/developers may want to restart the daemon at their
> >> own risk when it starts to behave something wrong or they change
> >> a configuration (since "reconfigure" may not be stable feature).
> >>
> >> # Maybe others have different opinion but I often use "restart".
> >>
> >> If you need this patch anyway, please add other options to stop/restart
> >> the daemon forcibly.  (e.g. adding force-stop/force-restart)
> > 
> > I can certainly add a force-stop/force-restart.  This can currently be
> > accomplished by simply killing the daemon, but admittedly doing it
> > through the init script is cleaner and more obvious to users, and it's
> > necessary to allow package upgrades to immediately start using
> > multipathd on systems with multipathed root disks.
> > 
> > However, what is there that you need to restart the daemon for that
> > 
> > # service multipathd reload
> > 
> > won't fix, besides upgrading the package.  If there is something, then
> > we should fix it.  I only stop the daemon when I'm testing shutting
> > down, and upgrading source. Otherwise, I've been using reconfigure
> > without problems.
> 
> I may be confusing you.
> I know/understand that current reconfigure works well and if it doesn't
> work we should fix it.
> 
> My point is that "why do you need to prevent stopping daemon
> by script-level forcibly?".
> As you mentioned above, killing the daemon through the init script
> is cleaner and more obvious.  So I think leaving something to stop
> the daemon in the init script is more friendly for admins.

The script only blocks stopping multipathd if the root device is
multipathed, since you won't be able to restart it if you lose all
the paths to your root device, ever for a short while.

Admittedly, if you are able to run the script, you most likely currently
have access to your root device, and running something like

# service multipathd restart

is likely (but not certain) to be safe.

This obviously isn't fixing a bug, so if people are against the patch,
I'm O.K. with leaving it out.

-Ben

> 
> Thanks,
> Kiyoshi Ueda

Re: [PATCH] multipath: don't let init script stop multipathd for root devices

[Kiyoshi Ueda]

Hi Ben,

On 04/02/2010 03:42 AM +0900, Benjamin Marzinski wrote:
> On Tue, Mar 30, 2010 at 09:11:13AM +0900, Kiyoshi Ueda wrote:
>> Hi Ben,
>>
>> On 03/27/2010 01:23 AM +0900, Benjamin Marzinski wrote:
>>> On Fri, Mar 26, 2010 at 11:04:22AM +0900, Kiyoshi Ueda wrote:
>>>> Hi Ben,
>>>>
>>>> On 03/26/2010 02:52 AM +0900, Benjamin Marzinski wrote:
>>>>> This patch modifies the redhat init script, so that it doesn't allow
>>>>> multipathd to be stopped when the root device is on it.
>>>> Why do you need to prevent stopping daemon by script-level forcibly?
>>>> I think that users/developers may want to restart the daemon at their
>>>> own risk when it starts to behave something wrong or they change
>>>> a configuration (since "reconfigure" may not be stable feature).
>>>>
>>>> # Maybe others have different opinion but I often use "restart".
>>>>
>>>> If you need this patch anyway, please add other options to stop/restart
>>>> the daemon forcibly.  (e.g. adding force-stop/force-restart)
>>> I can certainly add a force-stop/force-restart.  This can currently be
>>> accomplished by simply killing the daemon, but admittedly doing it
>>> through the init script is cleaner and more obvious to users, and it's
>>> necessary to allow package upgrades to immediately start using
>>> multipathd on systems with multipathed root disks.
>>>
>>> However, what is there that you need to restart the daemon for that
>>>
>>> # service multipathd reload
>>>
>>> won't fix, besides upgrading the package.  If there is something, then
>>> we should fix it.  I only stop the daemon when I'm testing shutting
>>> down, and upgrading source. Otherwise, I've been using reconfigure
>>> without problems.
>> I may be confusing you.
>> I know/understand that current reconfigure works well and if it doesn't
>> work we should fix it.
>>
>> My point is that "why do you need to prevent stopping daemon
>> by script-level forcibly?".
>> As you mentioned above, killing the daemon through the init script
>> is cleaner and more obvious.  So I think leaving something to stop
>> the daemon in the init script is more friendly for admins.
> 
> The script only blocks stopping multipathd if the root device is
> multipathed, since you won't be able to restart it if you lose all
> the paths to your root device, ever for a short while.
> 
> Admittedly, if you are able to run the script, you most likely currently
> have access to your root device, and running something like
> 
> # service multipathd restart
> 
> is likely (but not certain) to be safe.

Make sense.

 
> This obviously isn't fixing a bug, so if people are against the patch,
> I'm O.K. with leaving it out.

I don't object the patch.
I understand it's useful/friendly for admins not to allow stopping
multipathd easily when it can cause critical problems (and Christophe
has already commited the patch).

But for more friendness for admins who know it's their own risk,
force-stop/restart are good additional options, I think.

Thanks,
Kiyoshi Ueda