Re: [RFC][PATCH] limit state change to SDEV_BLOCK devices in scsi_internal_device_unblock

[Takahiro Yasui <tyasui@redhat.com>]

James Bottomley wrote:
>> +	if (sdev->sdev_state != SDEV_BLOCK)
> 
> This isn't quite correct.  There are two blocked states in the model
> currently:  SDEV_BLOCK and SDEV_CREATED_BLOCK ... you'd need to check
> for both of them
> 
>> +		return 0;
> 
> Traditionally the return for an attempted invalid state transition is
> -EINVAL.
> 
> I suppose for lower down, if you check the state, now we know we go 
> 
> SDEV_CREATED_BLOCK -> SDEV_CREATED
> 
> or
> 
> SDEV_BLOCK -> SDEV_RUNNING
> 
> so there's no need for the dual scsi_device_set_state.

Thank you for the comments. If I understand your comments correctly, 
the state transition is better to be defined in scsi_device_set_state(),
and both transitions, "SDEV_CREATED_BLOCK -> SDEV_CREATED" and
"SDEV_BLOCK -> SDEV_RUNNING" need to be covered. I updated the patch
so that the transition of "SDEV_OFFLINE -> SDEV_RUNNING" is prohibited.

A note in this change is all transitions of "SDEV_OFFLINE -> SDEV_RUNNING"
is prohibited, and the following state change for devices in SDEV_OFFLINE
state fails.

  # echo running > /sys/block/sdX/device/state

Therefore, users need to delete the device once and then those devices
need to be scanned as follows.

  # echo yes > /sys/block/sdX/device/delete
  # echo "- - -" > /sys/class/scsi_host/hostX/scan

I appreciate your reviews on it.

Regards,
---
Takahiro Yasui
Hitachi Computer Products (America), Inc.


Signed-off-by: Takahiro Yasui <tyasui@redhat.com>
---
 drivers/scsi/scsi_lib.c |    1 -
 1 file changed, 1 deletion(-)

Index: linux-2.6.29/drivers/scsi/scsi_lib.c
===================================================================
--- linux-2.6.29.orig/drivers/scsi/scsi_lib.c
+++ linux-2.6.29/drivers/scsi/scsi_lib.c
@@ -2258,7 +2258,6 @@ scsi_device_set_state(struct scsi_device
 	case SDEV_RUNNING:
 		switch (oldstate) {
 		case SDEV_CREATED:
-		case SDEV_OFFLINE:
 		case SDEV_QUIESCE:
 		case SDEV_BLOCK:
 			break;