If 'iscsiadm -m node --logoutall=all' fails when stopping
the open-iscsi service, we shouldn't kill iscsid.

This solves the following race:
1. A logout from a node is initiated by the user.
2. Before the logout completes, the user runs /etc/init.d/iscsi stop.
   The 'stop' method logs out from all nodes. When it tries to logout
   from the node that is already logging out (step #1), it fails
   because it is already logging out. Then, the 'stop' method kills
   iscsid.
3. The logout command form step #1 returns and notifies the (dead) daemon.

Now, running 'iscsiadm -m session' shows a session (which, actually, doesn't
exist anymore) and the iscsi service is down.

Signed-off-by: Erez Zilber <erezzi.l...@gmail.com>

On Wed, Jul 22, 2009 at 8:27 PM, Mike Christie<micha...@cs.wisc.edu> wrote:
>
> On 07/22/2009 09:09 AM, Erez Zilber wrote:
>> Mike,
>>
>> I'm seeing from time to time the following scenario (although
>> currently I'm not able to reproduce it):
>>
>
> No need to replicate it. I know what you are referring to.
>
>> 1. A logout from a node is initiated by the user.
>> 2. Before the logout completes, the user runs /etc/init.d/iscsi stop.
>> The 'stop' method logs out from all nodes. When it tries to logout
>> from the node that is already logging out (step #1), it fails because
>> it is already logging out (see initiator.c::session_logout_task when
>> conn->logout_qtask != NULL). Then, the 'stop' method kills iscsid.
>> 3. The logout command form step #1 returns and notifies the (dead) daemon.
>>
>> I was thinking about something like this to solve that:
>>
>> diff --git a/etc/initd/initd.redhat b/etc/initd/initd.redhat
>> index d68f135..55d35ec 100644
>> --- a/etc/initd/initd.redhat
>> +++ b/etc/initd/initd.redhat
>> @@ -39,6 +39,11 @@ stop()
>>          echo -n $"Stopping iSCSI initiator service: "
>>          sync
>>          iscsiadm -m node --logoutall=all
>> +       RETVAL=$?
>> +       if [ $RETVAL -ne 0 ]; then
>> +               echo "Could not logout from all nodes, try again later"
>> +               return $RETVAL
>> +       fi
>>          killproc iscsid
>>          rm -f /var/run/iscsid.pid
>>          [ $RETVAL -eq 0 ]&&  rm -f /var/lock/subsys/open-iscsi
>> @@ -76,6 +81,10 @@ case "$1" in
>>                          ;;
>>          restart)
>>                          stop
>> +                       if [ $RETVAL -ne 0 ]; then
>> +                               echo "Stopping iSCSI initiator service
>> failed, not starting"
>> +                               exit $RETVAL
>> +                       fi
>>                          start
>>                          ;;
>>          status)
>>
>> I'm not sure if there are other logout failure scenarios in which we
>> want to shutdown the open-iscsi service anyway. If not, I guess that
>> this fix (and a similar fix for SuSE&  Debian) should do the work.
>>
>
> This looks ok to me. We could make iscsid more complex and send
> notifications to multiple iscsiadm requests, but I am thinking this does
> not come up very much in normal use, so I am ok with the simple fix in
> the patch.
>
> >
>

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~----------~----~----~----~------~----~------~--~---

Don't kill iscsid if logout from all nodes fail

If 'iscsiadm -m node --logoutall=all' fails when stopping
the open-iscsi service, we shouldn't kill iscsid.

This solves the following race:
1. A logout from a node is initiated by the user.
2. Before the logout completes, the user runs /etc/init.d/iscsi stop.
   The 'stop' method logs out from all nodes. When it tries to logout
   from the node that is already logging out (step #1), it fails
   because it is already logging out. Then, the 'stop' method kills
   iscsid.
3. The logout command form step #1 returns and notifies the (dead) daemon.

Now, running 'iscsiadm -m session' shows a session (which, actually, doesn't
exist anymore) and the iscsi service is down.

Signed-off-by: Erez Zilber <erezzi.l...@gmail.com>
---
 etc/initd/initd.debian |   16 +++++++++++++++-
 etc/initd/initd.redhat |   13 +++++++++++++
 etc/initd/initd.suse   |    5 +++++
 3 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/etc/initd/initd.debian b/etc/initd/initd.debian
index c0dfd1e..59bf59b 100644
--- a/etc/initd/initd.debian
+++ b/etc/initd/initd.debian
@@ -46,11 +46,17 @@ stoptargets() {
 	log_daemon_msg "Disconnecting iSCSI targets"
 	sync
 	$ADM -m node --logoutall=all
-	log_end_msg 0
+	RETVAL=$?
+	log_end_msg $RETVAL
 }
 
 stop() {
 	stoptargets
+	if [ $RETVAL -ne 0 ]; then
+		log_failure_msg "Could not stop all targets, try again later"
+		return $RETVAL
+	fi
+
 	log_daemon_msg "Stopping iSCSI initiator service"
 	start-stop-daemon --stop --quiet --pidfile $PIDFILE --exec $DAEMON
 	rm -f $PIDFILE
@@ -68,11 +74,19 @@ stop() {
 
 restart() {
 	stop
+	if [ $RETVAL -ne 0 ]; then
+		log_failure_msg "Stopping iSCSI initiator service failed, not starting"
+		return $RETVAL
+	fi
 	start
 }
 
 restarttargets() {
 	stoptargets
+	if [ $RETVAL -ne 0 ]; then
+		log_failure_msg "Could not stop all targets, try again later"
+		return $RETVAL
+	fi
 	starttargets
 }
 
diff --git a/etc/initd/initd.redhat b/etc/initd/initd.redhat
index d68f135..c767cfb 100644
--- a/etc/initd/initd.redhat
+++ b/etc/initd/initd.redhat
@@ -39,6 +39,11 @@ stop()
 	echo -n $"Stopping iSCSI initiator service: "
 	sync
 	iscsiadm -m node --logoutall=all
+	RETVAL=$?
+	if [ $RETVAL -ne 0 ]; then
+		echo "Could not logout from all nodes, try again later"
+		return $RETVAL
+	fi
 	killproc iscsid
 	rm -f /var/run/iscsid.pid
 	[ $RETVAL -eq 0 ] && rm -f /var/lock/subsys/open-iscsi
@@ -63,6 +68,10 @@ stop()
 restart()
 {
 	stop
+	if [ $RETVAL -ne 0 ]; then
+		echo "Stopping iSCSI initiator service failed, not starting"
+		return $RETVAL
+	fi
 	start
 
 }
@@ -76,6 +85,10 @@ case "$1" in
 			;;
 	restart)
 			stop
+			if [ $RETVAL -ne 0 ]; then
+				echo "Stopping iSCSI initiator service failed, not starting"
+				exit $RETVAL
+			fi
 			start
 			;;
 	status)
diff --git a/etc/initd/initd.suse b/etc/initd/initd.suse
index 23bbac0..bdb4dc7 100644
--- a/etc/initd/initd.suse
+++ b/etc/initd/initd.suse
@@ -192,6 +192,11 @@ case "$1" in
 	;;
     restart)
 	$0 stop
+	RETVAL=$?
+	if [ "$RETVAL" != "0" ]; then
+		echo "Stopping iSCSI initiator service failed, not starting"
+		exit $RETVAL
+	fi
 	sleep 1
 	$0 start
 	;;
-- 
1.5.5.6

Reply via email to