Re: [PATCH] scsi: Erroneous handling of sense status in SCSI EH commands

[Mike Christie <michaelc@cs.wisc.edu>]

On 01/21/2011 02:07 AM, Hannes Reinecke wrote:
> Consider a scenario where a SCSI EH command like TUR fails with a NOT READY
> status - MANUAL INTERVENTION REQUIRED (02/04/03). As evident in the ASC/ASCQ
> description, manual intervention is required in this case i.e. the target wants
> TUR to fail here so that the host administrator can take corrective action.
>
> But this particular ASC/ASCQ is not handled in the scsi_check_sense, which
> ultimately causes scsi_eh_tur to erroneously report a SUCCESS (device ready)
> instead of the actual FAILURE state (device not ready).
>
> This patch converts scsi_check_sense() to return SOFT_ERROR in
> cases where an error has been signalled via the sense code, but
> we should not wake the error handler. This error code will be
> reverted back to SUCCESS for normal command handling, but
> scsi_eh_completed_normally() will convert it to FAILED.
>
> Reported-by: Martin George<marting@netapp.com>
> Signed-off-by: Hannes Reinecke<hare@suse.de>
>
> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> index 45c7564..e86e62e 100644
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -223,7 +223,7 @@ static inline void scsi_eh_prt_fail_stats(struct Scsi_Host *shost,
>    * @scmd:	Cmd to have sense checked.
>    *
>    * Return value:
> - * 	SUCCESS or FAILED or NEEDS_RETRY
> + * 	SUCCESS or FAILED or NEEDS_RETRY or SOFT_ERROR
>    *
>    * Notes:
>    *	When a deferred error is detected the current command has
> @@ -278,7 +278,7 @@ static int scsi_check_sense(struct scsi_cmnd *scmd)
>
>   	case ABORTED_COMMAND:
>   		if (sshdr.asc == 0x10) /* DIF */
> -			return SUCCESS;
> +			return SOFT_ERROR;
>
>   		return NEEDS_RETRY;
>   	case NOT_READY:
> @@ -324,19 +324,19 @@ static int scsi_check_sense(struct scsi_cmnd *scmd)
>   		 * Pass the UA upwards for a determination in the completion
>   		 * functions.
>   		 */
> -		return SUCCESS;
> +		return SOFT_ERROR;
>


For normal IO execution the UA, not ready, and illegal request handling 
in scsi_io_completion would determine that some of these are retryable 
conditions, but with the patch in the scsi_error.cs path we will return 
soft error and fail. Also a lot of these that were retryable were 
returned as success in the past, but with the patch are being failed.

Martin made me the same bugzilla :) I was thinking we needed to make 
check_sense smarter or move some of the scsi_io_completion code to some 
lib helpers so scsi_error.c could call them.

And does scsi_eh_stu need to be covnerted to check for soft error and 
failed.

Also if we did this patch, then I think we also have to convert the 
device handlers.