[PATCH 1/2] alua: clarify messages when blk_execute_rq fails

[PATCH 1/2] alua: clarify messages when blk_execute_rq fails

[Rob Evers]

replace numeric messages with string error messages when blk_execute_rq
fails.  Also add printing of sense info.

Mike Christie suggested adding printing of sense info here

Signed-off-by: Rob Evers <revers@redhat.com>
---
 drivers/scsi/device_handler/scsi_dh_alua.c |   27 ++++++++++++++++++---------
 1 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 6fec9fe..58fdf64 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -24,6 +24,7 @@
 #include <scsi/scsi.h>
 #include <scsi/scsi_eh.h>
 #include <scsi/scsi_dh.h>
+#include <scsi/scsi_dbg.h>
 
 #define ALUA_DH_NAME "alua"
 #define ALUA_DH_VER "1.3"
@@ -153,9 +154,12 @@ static int submit_std_inquiry(struct scsi_device *sdev, struct alua_dh_data *h)
 
 	err = blk_execute_rq(rq->q, NULL, rq, 1);
 	if (err == -EIO) {
-		sdev_printk(KERN_INFO, sdev,
-			    "%s: std inquiry failed with %x\n",
-			    ALUA_DH_NAME, rq->errors);
+		sdev_printk(KERN_INFO, sdev, "%s: std inquiry failed\n",
+			    ALUA_DH_NAME);
+		scsi_show_result(rq->errors);
+		if (driver_byte(rq->errors) && DRIVER_SENSE)
+			__scsi_print_sense("alua std_inquiry", rq->sense,
+					   rq->sense_len);
 		h->senselen = rq->sense_len;
 		err = SCSI_DH_IO;
 	}
@@ -190,9 +194,12 @@ static int submit_vpd_inquiry(struct scsi_device *sdev, struct alua_dh_data *h)
 
 	err = blk_execute_rq(rq->q, NULL, rq, 1);
 	if (err == -EIO) {
-		sdev_printk(KERN_INFO, sdev,
-			    "%s: evpd inquiry failed with %x\n",
-			    ALUA_DH_NAME, rq->errors);
+		sdev_printk(KERN_INFO, sdev, "%s: evpd inquiry failed\n",
+			    ALUA_DH_NAME);
+		scsi_show_result(rq->errors);
+		if (driver_byte(rq->errors) && DRIVER_SENSE)
+			__scsi_print_sense("alua vpd_inquiry", rq->sense,
+					   rq->sense_len);
 		h->senselen = rq->sense_len;
 		err = SCSI_DH_IO;
 	}
@@ -229,9 +236,11 @@ static unsigned submit_rtpg(struct scsi_device *sdev, struct alua_dh_data *h)
 
 	err = blk_execute_rq(rq->q, NULL, rq, 1);
 	if (err == -EIO) {
-		sdev_printk(KERN_INFO, sdev,
-			    "%s: rtpg failed with %x\n",
-			    ALUA_DH_NAME, rq->errors);
+		sdev_printk(KERN_INFO, sdev, "%s: rtpg failed\n", ALUA_DH_NAME);
+		scsi_show_result(rq->errors);
+		if (driver_byte(rq->errors) && DRIVER_SENSE)
+			__scsi_print_sense("alua submit_rtpg", rq->sense,
+					   rq->sense_len);
 		h->senselen = rq->sense_len;
 		err = SCSI_DH_IO;
 	}
-- 
1.7.1

[PATCH 2/2] scsi_io_completion: remove confusing unhandled messages

[Rob Evers]

remove message when host_byte case is not specifically handled.
remove message when sense_key case is not specifically handled.

Signed-off-by: Rob Evers <revers@redhat.com>
---
 drivers/scsi/scsi_lib.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index fc3f168..e6a359c 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -920,12 +920,10 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 			action = ACTION_FAIL;
 			break;
 		default:
-			description = "Unhandled sense code";
 			action = ACTION_FAIL;
 			break;
 		}
 	} else {
-		description = "Unhandled error code";
 		action = ACTION_FAIL;
 	}
 
-- 
1.7.1

Re: [PATCH 1/2] alua: clarify messages when blk_execute_rq fails

[Rob Evers]

  On 08/12/2011 05:01 PM, Rob Evers wrote:
> replace numeric messages with string error messages when blk_execute_rq
> fails.  Also add printing of sense info.
>
> Mike Christie suggested adding printing of sense info here
>
> Signed-off-by: Rob Evers<revers@redhat.com>
>

Hi James,

Do you have any feedback on these patches?

(I already noticed the 2nd patch is a reply to the first)

I didn't see them in the last update of scsi-misc.

Thanks, Rob

Re: [PATCH 1/2] alua: clarify messages when blk_execute_rq fails

[James Bottomley]

On Tue, 2011-08-30 at 16:07 -0400, Rob Evers wrote:
> On 08/12/2011 05:01 PM, Rob Evers wrote:
> > replace numeric messages with string error messages when blk_execute_rq
> > fails.  Also add printing of sense info.
> >
> > Mike Christie suggested adding printing of sense info here
> >
> > Signed-off-by: Rob Evers<revers@redhat.com>
> >
> 
> Hi James,
> 
> Do you have any feedback on these patches?
> 
> (I already noticed the 2nd patch is a reply to the first)
> 
> I didn't see them in the last update of scsi-misc.

I don't really see any point in removing messages.  If Hannes is OK with
the ALUA sense one, he'll send it to me.

James

Re: [PATCH 1/2] alua: clarify messages when blk_execute_rq fails

[Rob Evers]

  On 08/30/2011 05:37 PM, James Bottomley wrote:
> On Tue, 2011-08-30 at 16:07 -0400, Rob Evers wrote:
>> On 08/12/2011 05:01 PM, Rob Evers wrote:
>>> replace numeric messages with string error messages when blk_execute_rq
>>> fails.  Also add printing of sense info.
>>>
>>> Mike Christie suggested adding printing of sense info here
>>>
>>> Signed-off-by: Rob Evers<revers@redhat.com>
>>>
>> Hi James,
>>
>> Do you have any feedback on these patches?
>>
>> (I already noticed the 2nd patch is a reply to the first)
>>
>> I didn't see them in the last update of scsi-misc.
> I don't really see any point in removing messages.

"Unhandled error/sense code" can be interpreted as something going
wrong when nothing actually went wrong.

For lack of ideas that were accurate, yet not alarming, removing the 
message
altogether seemed like the right option.

Re: [PATCH 1/2] alua: clarify messages when blk_execute_rq fails

[Rob Evers]

  On 08/30/2011 09:12 PM, Rob Evers wrote:
>  On 08/30/2011 05:37 PM, James Bottomley wrote:
>> On Tue, 2011-08-30 at 16:07 -0400, Rob Evers wrote:
>>> On 08/12/2011 05:01 PM, Rob Evers wrote:
>>>> replace numeric messages with string error messages when 
>>>> blk_execute_rq
>>>> fails.  Also add printing of sense info.
>>>>
>>>> Mike Christie suggested adding printing of sense info here
>>>>
>>>> Signed-off-by: Rob Evers<revers@redhat.com>
>>>>
>>> Hi James,
>>>
>>> Do you have any feedback on these patches?
>>>
>>> (I already noticed the 2nd patch is a reply to the first)
>>>
>>> I didn't see them in the last update of scsi-misc.
>> I don't really see any point in removing messages.
>
> "Unhandled error/sense code" can be interpreted as something going
> wrong when nothing actually went wrong.
>
> For lack of ideas that were accurate, yet not alarming, removing the 
> message
> altogether seemed like the right option.

Hi James,

Just a bit more info on these patches.

We are getting user reports that messages addressed
are causing confusion during fabric faults.

"Unhandled error code"

gets reported with no functional problems being reported.

and...

alua: rtpg failed with 10000
alua: rtpg failed with 800000

Which are cryptic.  I tried to clarify these.  Perhaps
Hannes will consider these.

I tried to address all the instances consistently even
though some are not actually being reported.

Apologies for not explaining the user reports at first.

Rob

Re: [PATCH 1/2] alua: clarify messages when blk_execute_rq fails

[Rob Evers]

  On 08/12/2011 05:01 PM, Rob Evers wrote:
> replace numeric messages with string error messages when blk_execute_rq
> fails.  Also add printing of sense info.
>
> Mike Christie suggested adding printing of sense info here
>
> Signed-off-by: Rob Evers<revers@redhat.com>
> ---
>   drivers/scsi/device_handler/scsi_dh_alua.c |   27 ++++++++++++++++++---------
>   1 files changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
> index 6fec9fe..58fdf64 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -24,6 +24,7 @@
>   #include<scsi/scsi.h>
>   #include<scsi/scsi_eh.h>
>   #include<scsi/scsi_dh.h>
> +#include<scsi/scsi_dbg.h>
>
>   #define ALUA_DH_NAME "alua"
>   #define ALUA_DH_VER "1.3"
> @@ -153,9 +154,12 @@ static int submit_std_inquiry(struct scsi_device *sdev, struct alua_dh_data *h)
>
>   	err = blk_execute_rq(rq->q, NULL, rq, 1);
>   	if (err == -EIO) {
> -		sdev_printk(KERN_INFO, sdev,
> -			    "%s: std inquiry failed with %x\n",
> -			    ALUA_DH_NAME, rq->errors);
> +		sdev_printk(KERN_INFO, sdev, "%s: std inquiry failed\n",
> +			    ALUA_DH_NAME);
> +		scsi_show_result(rq->errors);
> +		if (driver_byte(rq->errors)&&  DRIVER_SENSE)
> +			__scsi_print_sense("alua std_inquiry", rq->sense,
> +					   rq->sense_len);
>   		h->senselen = rq->sense_len;
>   		err = SCSI_DH_IO;
>   	}
> @@ -190,9 +194,12 @@ static int submit_vpd_inquiry(struct scsi_device *sdev, struct alua_dh_data *h)
>
>   	err = blk_execute_rq(rq->q, NULL, rq, 1);
>   	if (err == -EIO) {
> -		sdev_printk(KERN_INFO, sdev,
> -			    "%s: evpd inquiry failed with %x\n",
> -			    ALUA_DH_NAME, rq->errors);
> +		sdev_printk(KERN_INFO, sdev, "%s: evpd inquiry failed\n",
> +			    ALUA_DH_NAME);
> +		scsi_show_result(rq->errors);
> +		if (driver_byte(rq->errors)&&  DRIVER_SENSE)
> +			__scsi_print_sense("alua vpd_inquiry", rq->sense,
> +					   rq->sense_len);
>   		h->senselen = rq->sense_len;
>   		err = SCSI_DH_IO;
>   	}
> @@ -229,9 +236,11 @@ static unsigned submit_rtpg(struct scsi_device *sdev, struct alua_dh_data *h)
>
>   	err = blk_execute_rq(rq->q, NULL, rq, 1);
>   	if (err == -EIO) {
> -		sdev_printk(KERN_INFO, sdev,
> -			    "%s: rtpg failed with %x\n",
> -			    ALUA_DH_NAME, rq->errors);
> +		sdev_printk(KERN_INFO, sdev, "%s: rtpg failed\n", ALUA_DH_NAME);
> +		scsi_show_result(rq->errors);
> +		if (driver_byte(rq->errors)&&  DRIVER_SENSE)
> +			__scsi_print_sense("alua submit_rtpg", rq->sense,
> +					   rq->sense_len);
>   		h->senselen = rq->sense_len;
>   		err = SCSI_DH_IO;
>   	}

Hannes,

Did you get a chance to take a look at this patch?

Thanks, Rob

Re: [PATCH 1/2] alua: clarify messages when blk_execute_rq fails

[Hannes Reinecke]

On 10/07/2011 12:07 AM, Rob Evers wrote:
> On 08/12/2011 05:01 PM, Rob Evers wrote:
>> replace numeric messages with string error messages when
>> blk_execute_rq
>> fails. Also add printing of sense info.
>>
>> Mike Christie suggested adding printing of sense info here
>>
>> Signed-off-by: Rob Evers<revers@redhat.com>
>> ---
>> drivers/scsi/device_handler/scsi_dh_alua.c | 27
>> ++++++++++++++++++---------
>> 1 files changed, 18 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c
>> b/drivers/scsi/device_handler/scsi_dh_alua.c
>> index 6fec9fe..58fdf64 100644
>> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
>> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
>> @@ -24,6 +24,7 @@
>> #include<scsi/scsi.h>
>> #include<scsi/scsi_eh.h>
>> #include<scsi/scsi_dh.h>
>> +#include<scsi/scsi_dbg.h>
>>
>> #define ALUA_DH_NAME "alua"
>> #define ALUA_DH_VER "1.3"
>> @@ -153,9 +154,12 @@ static int submit_std_inquiry(struct
>> scsi_device *sdev, struct alua_dh_data *h)
>>
>> err = blk_execute_rq(rq->q, NULL, rq, 1);
>> if (err == -EIO) {
>> - sdev_printk(KERN_INFO, sdev,
>> - "%s: std inquiry failed with %x\n",
>> - ALUA_DH_NAME, rq->errors);
>> + sdev_printk(KERN_INFO, sdev, "%s: std inquiry failed\n",
>> + ALUA_DH_NAME);
>> + scsi_show_result(rq->errors);
>> + if (driver_byte(rq->errors)&& DRIVER_SENSE)
>> + __scsi_print_sense("alua std_inquiry", rq->sense,
>> + rq->sense_len);
>> h->senselen = rq->sense_len;
>> err = SCSI_DH_IO;
>> }
>> @@ -190,9 +194,12 @@ static int submit_vpd_inquiry(struct
>> scsi_device *sdev, struct alua_dh_data *h)
>>
>> err = blk_execute_rq(rq->q, NULL, rq, 1);
>> if (err == -EIO) {
>> - sdev_printk(KERN_INFO, sdev,
>> - "%s: evpd inquiry failed with %x\n",
>> - ALUA_DH_NAME, rq->errors);
>> + sdev_printk(KERN_INFO, sdev, "%s: evpd inquiry failed\n",
>> + ALUA_DH_NAME);
>> + scsi_show_result(rq->errors);
>> + if (driver_byte(rq->errors)&& DRIVER_SENSE)
>> + __scsi_print_sense("alua vpd_inquiry", rq->sense,
>> + rq->sense_len);
>> h->senselen = rq->sense_len;
>> err = SCSI_DH_IO;
>> }
>> @@ -229,9 +236,11 @@ static unsigned submit_rtpg(struct
>> scsi_device *sdev, struct alua_dh_data *h)
>>
>> err = blk_execute_rq(rq->q, NULL, rq, 1);
>> if (err == -EIO) {
>> - sdev_printk(KERN_INFO, sdev,
>> - "%s: rtpg failed with %x\n",
>> - ALUA_DH_NAME, rq->errors);
>> + sdev_printk(KERN_INFO, sdev, "%s: rtpg failed\n", ALUA_DH_NAME);
>> + scsi_show_result(rq->errors);
>> + if (driver_byte(rq->errors)&& DRIVER_SENSE)
>> + __scsi_print_sense("alua submit_rtpg", rq->sense,
>> + rq->sense_len);
>> h->senselen = rq->sense_len;
>> err = SCSI_DH_IO;
>> }
>
> Hannes,
>
> Did you get a chance to take a look at this patch?
>
Yes, I did.

And found it's actually not quite correct; we'll need to evaluate 
the sense code here to do a possible retry.

I'll be updating the patch.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html