[PATCH 0/2] Minor scsi_dh_emc error path fixes

[PATCH 0/2] Minor scsi_dh_emc error path fixes

[Joe Lawrence]

These two patches spun out of the blk_get_request return type changes
I've been working on [1].

  [1] http://thread.gmane.org/gmane.linux.scsi/80934

Should that patch go in, there are a few other device handler changes
possible for 3.11 [2], but the following bugfixes are unrelated, so I
thought I'd post them in case they could apply for 3.10.

  [2] http://thread.gmane.org/gmane.linux.scsi/81130

The first bug was spotted by Mike in thread [2], the second when I
traced the call path of clariion_send_inquiry back out through
clariion_set_params and its caller parse_path. The latter fix could have
been made in parse_path, but it didn't seem appropriate for that file to
sort through SCSI_DH enums.

Comments welcome, unfortunately I don't have the HW to verify, so these
are only compile tested.

Thanks,

Joe Lawrence (2):
  scsi_dh_emc: handle zero-senselen send_inquiry_cmd errors
  scsi_dh_emc: set_params callback should consistently return errno

 drivers/scsi/device_handler/scsi_dh_emc.c |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

-- 
1.7.10.4

scsi_dh_emc: handle zero-senselen send_inquiry_cmd errors

[Joe Lawrence]

>From c6645f639f7e7551c7a6f0aacee78e57ddd37bc1 Mon Sep 17 00:00:00 2001
From: Joe Lawrence <joe.lawrence@stratus.com>
Date: Tue, 28 May 2013 14:49:37 -0400
Subject: [PATCH 1/2] scsi_dh_emc: handle zero-senselen send_inquiry_cmd
 errors

The send_inquiry_cmd function may exit without setting senselen if
get_req fails.  Callers shouldn't assume senselen is set non-zero in
their failure checking.

Signed-off-by: Joe Lawrence <joe.lawrence@stratus.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/device_handler/scsi_dh_emc.c |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_emc.c b/drivers/scsi/device_handler/scsi_dh_emc.c
index e1c8be0..0438ed6 100644
--- a/drivers/scsi/device_handler/scsi_dh_emc.c
+++ b/drivers/scsi/device_handler/scsi_dh_emc.c
@@ -468,9 +468,12 @@ static int clariion_std_inquiry(struct scsi_device *sdev,
 	char *sp_model;
 
 	err = send_inquiry_cmd(sdev, 0, csdev);
-	if (err != SCSI_DH_OK && csdev->senselen) {
+	if (err != SCSI_DH_OK) {
 		struct scsi_sense_hdr sshdr;
 
+		if (!csdev->senselen)
+			goto out;
+
 		if (scsi_normalize_sense(csdev->sense, SCSI_SENSE_BUFFERSIZE,
 					 &sshdr)) {
 			sdev_printk(KERN_ERR, sdev, "%s: INQUIRY sense code "
@@ -507,9 +510,12 @@ static int clariion_send_inquiry(struct scsi_device *sdev,
 
 retry:
 	err = send_inquiry_cmd(sdev, 0xC0, csdev);
-	if (err != SCSI_DH_OK && csdev->senselen) {
+	if (err != SCSI_DH_OK) {
 		struct scsi_sense_hdr sshdr;
 
+		if (!csdev->senselen)
+			goto out;
+
 		err = scsi_normalize_sense(csdev->sense, SCSI_SENSE_BUFFERSIZE,
 					   &sshdr);
 		if (!err)
@@ -527,6 +533,7 @@ retry:
 	} else {
 		err = parse_sp_info_reply(sdev, csdev);
 	}
+out:
 	return err;
 }
 
-- 
1.7.10.4

scsi_dh_emc: set_params callback should consistently return errno

[Joe Lawrence]

>From db6c2d4406732585fd7a658fc89b14fa26e7d1d4 Mon Sep 17 00:00:00 2001
From: Joe Lawrence <joe.lawrence@stratus.com>
Date: Tue, 28 May 2013 15:47:20 -0400
Subject: [PATCH 2/2] scsi_dh_emc: set_params callback should consistently
 return errno

A SCSI scsi_device_handler set_params routine should return a negative
errno value on failure. The scsi_dh_emc driver may return not only
-EINVAL but also also positive SCSI_DH enum values. Fix the
clariion_set_params implementation to return a negative value in all
error scenarios. To that end, make sure send_trespass_cmd consistently
returns a SCSI_DH enum.

Signed-off-by: Joe Lawrence <joe.lawrence@stratus.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/device_handler/scsi_dh_emc.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_emc.c b/drivers/scsi/device_handler/scsi_dh_emc.c
index 0438ed6..6ca9e84 100644
--- a/drivers/scsi/device_handler/scsi_dh_emc.c
+++ b/drivers/scsi/device_handler/scsi_dh_emc.c
@@ -389,6 +389,7 @@ static int send_trespass_cmd(struct scsi_device *sdev,
 		if (rq->sense_len) {
 			err = trespass_endio(sdev, csdev->sense);
 		} else {
+			err = SCSI_DH_IO;
 			sdev_printk(KERN_INFO, sdev,
 				    "%s: failed to send MODE SELECT: %x\n",
 				    CLARIION_NAME, rq->errors);
@@ -626,7 +627,10 @@ static int clariion_set_params(struct scsi_device *sdev, const char *params)
 	result = clariion_send_inquiry(sdev, csdev);
 
 done:
-	return result;
+	if (result != SCSI_DH_OK)
+		return -EIO;
+
+	return 0;
 }
 
 static const struct scsi_dh_devlist clariion_dev_list[] = {
-- 
1.7.10.4

Re: [PATCH 0/2] Minor scsi_dh_emc error path fixes

[Joe Lawrence]

On Tue, 28 May 2013 17:18:37 -0400
Joe Lawrence <Joe.Lawrence@stratus.com> wrote:

> These two patches spun out of the blk_get_request return type changes
> I've been working on [1].
> 
>   [1] http://thread.gmane.org/gmane.linux.scsi/80934
> 
> Should that patch go in, there are a few other device handler changes
> possible for 3.11 [2], but the following bugfixes are unrelated, so I
> thought I'd post them in case they could apply for 3.10.
> 
>   [2] http://thread.gmane.org/gmane.linux.scsi/81130
> 
> The first bug was spotted by Mike in thread [2], the second when I
> traced the call path of clariion_send_inquiry back out through
> clariion_set_params and its caller parse_path. The latter fix could have
> been made in parse_path, but it didn't seem appropriate for that file to
> sort through SCSI_DH enums.
> 
> Comments welcome, unfortunately I don't have the HW to verify, so these
> are only compile tested.
> 
> Thanks,
> 
> Joe Lawrence (2):
>   scsi_dh_emc: handle zero-senselen send_inquiry_cmd errors
>   scsi_dh_emc: set_params callback should consistently return errno
> 
>  drivers/scsi/device_handler/scsi_dh_emc.c |   17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 

Hello Hannes, Mike,

I'm cleaning out some older work and came across these patches.  These
appear to be very minor bugs for HW I don't have access to, but I
thought I'd give the patches one last ping.

(link to thread):
http://thread.gmane.org/gmane.linux.scsi/82290

Regards,

-- Joe