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@xxxxxxxxxx>
---
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;
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
