* [PATCH] ses: fix error message in ses_intf_add()
@ 2017-05-17 13:00 Maurizio Lombardi
2017-05-17 13:41 ` Ewan D. Milne
0 siblings, 1 reply; 3+ messages in thread
From: Maurizio Lombardi @ 2017-05-17 13:00 UTC (permalink / raw)
To: linux-scsi; +Cc: jejb, martin.petersen
The "result" variable contains a negative error code and
should not be logged as an hex value.
This fixes the following error message:
[ 250.068869] scsi 8:0:2:254: Wrong diagnostic page; asked for 2 got 0
[ 250.068872] scsi 8:0:2:254: Failed to get diagnostic page 0xffffffea
Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
---
drivers/scsi/ses.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index f1cdf32..0ac45be 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -604,6 +604,7 @@ static int ses_intf_add(struct device *cdev,
unsigned char *buf = NULL, *hdr_buf, *type_ptr;
struct ses_device *ses_dev;
u32 result;
+ int page;
int i, types, len, components = 0;
int err = -ENOMEM;
int num_enclosures;
@@ -630,7 +631,8 @@ static int ses_intf_add(struct device *cdev,
if (!hdr_buf || !ses_dev)
goto err_init_free;
- result = ses_recv_diag(sdev, 1, hdr_buf, INIT_ALLOC_SIZE);
+ page = 1;
+ result = ses_recv_diag(sdev, page, hdr_buf, INIT_ALLOC_SIZE);
if (result)
goto recv_failed;
@@ -639,7 +641,7 @@ static int ses_intf_add(struct device *cdev,
if (!buf)
goto err_free;
- result = ses_recv_diag(sdev, 1, buf, len);
+ result = ses_recv_diag(sdev, page, buf, len);
if (result)
goto recv_failed;
@@ -669,7 +671,8 @@ static int ses_intf_add(struct device *cdev,
ses_dev->page1_len = len;
buf = NULL;
- result = ses_recv_diag(sdev, 2, hdr_buf, INIT_ALLOC_SIZE);
+ page = 2;
+ result = ses_recv_diag(sdev, page, hdr_buf, INIT_ALLOC_SIZE);
if (result)
goto recv_failed;
@@ -679,7 +682,7 @@ static int ses_intf_add(struct device *cdev,
goto err_free;
/* make sure getting page 2 actually works */
- result = ses_recv_diag(sdev, 2, buf, len);
+ result = ses_recv_diag(sdev, page, buf, len);
if (result)
goto recv_failed;
ses_dev->page2 = buf;
@@ -688,7 +691,8 @@ static int ses_intf_add(struct device *cdev,
/* The additional information page --- allows us
* to match up the devices */
- result = ses_recv_diag(sdev, 10, hdr_buf, INIT_ALLOC_SIZE);
+ page = 10;
+ result = ses_recv_diag(sdev, page, hdr_buf, INIT_ALLOC_SIZE);
if (!result) {
len = (hdr_buf[2] << 8) + hdr_buf[3] + 4;
@@ -696,7 +700,7 @@ static int ses_intf_add(struct device *cdev,
if (!buf)
goto err_free;
- result = ses_recv_diag(sdev, 10, buf, len);
+ result = ses_recv_diag(sdev, page, buf, len);
if (result)
goto recv_failed;
ses_dev->page10 = buf;
@@ -733,8 +737,9 @@ static int ses_intf_add(struct device *cdev,
return 0;
recv_failed:
- sdev_printk(KERN_ERR, sdev, "Failed to get diagnostic page 0x%x\n",
- result);
+ sdev_printk(KERN_ERR, sdev,
+ "Failed to get diagnostic page %d with error %d\n",
+ page, result);
err = -ENODEV;
err_free:
kfree(buf);
--
Maurizio Lombardi
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] ses: fix error message in ses_intf_add()
2017-05-17 13:00 [PATCH] ses: fix error message in ses_intf_add() Maurizio Lombardi
@ 2017-05-17 13:41 ` Ewan D. Milne
2017-05-18 0:18 ` Martin K. Petersen
0 siblings, 1 reply; 3+ messages in thread
From: Ewan D. Milne @ 2017-05-17 13:41 UTC (permalink / raw)
To: Maurizio Lombardi; +Cc: linux-scsi, jejb, martin.petersen
On Wed, 2017-05-17 at 15:00 +0200, Maurizio Lombardi wrote:
> The "result" variable contains a negative error code and
> should not be logged as an hex value.
> This fixes the following error message:
>
> [ 250.068869] scsi 8:0:2:254: Wrong diagnostic page; asked for 2 got 0
> [ 250.068872] scsi 8:0:2:254: Failed to get diagnostic page 0xffffffea
>
> Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
> ---
> drivers/scsi/ses.c | 21 +++++++++++++--------
> 1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
> index f1cdf32..0ac45be 100644
> --- a/drivers/scsi/ses.c
> +++ b/drivers/scsi/ses.c
> @@ -604,6 +604,7 @@ static int ses_intf_add(struct device *cdev,
> unsigned char *buf = NULL, *hdr_buf, *type_ptr;
> struct ses_device *ses_dev;
> u32 result;
> + int page;
> int i, types, len, components = 0;
> int err = -ENOMEM;
> int num_enclosures;
> @@ -630,7 +631,8 @@ static int ses_intf_add(struct device *cdev,
> if (!hdr_buf || !ses_dev)
> goto err_init_free;
>
> - result = ses_recv_diag(sdev, 1, hdr_buf, INIT_ALLOC_SIZE);
> + page = 1;
> + result = ses_recv_diag(sdev, page, hdr_buf, INIT_ALLOC_SIZE);
> if (result)
> goto recv_failed;
>
> @@ -639,7 +641,7 @@ static int ses_intf_add(struct device *cdev,
> if (!buf)
> goto err_free;
>
> - result = ses_recv_diag(sdev, 1, buf, len);
> + result = ses_recv_diag(sdev, page, buf, len);
> if (result)
> goto recv_failed;
>
> @@ -669,7 +671,8 @@ static int ses_intf_add(struct device *cdev,
> ses_dev->page1_len = len;
> buf = NULL;
>
> - result = ses_recv_diag(sdev, 2, hdr_buf, INIT_ALLOC_SIZE);
> + page = 2;
> + result = ses_recv_diag(sdev, page, hdr_buf, INIT_ALLOC_SIZE);
> if (result)
> goto recv_failed;
>
> @@ -679,7 +682,7 @@ static int ses_intf_add(struct device *cdev,
> goto err_free;
>
> /* make sure getting page 2 actually works */
> - result = ses_recv_diag(sdev, 2, buf, len);
> + result = ses_recv_diag(sdev, page, buf, len);
> if (result)
> goto recv_failed;
> ses_dev->page2 = buf;
> @@ -688,7 +691,8 @@ static int ses_intf_add(struct device *cdev,
>
> /* The additional information page --- allows us
> * to match up the devices */
> - result = ses_recv_diag(sdev, 10, hdr_buf, INIT_ALLOC_SIZE);
> + page = 10;
> + result = ses_recv_diag(sdev, page, hdr_buf, INIT_ALLOC_SIZE);
> if (!result) {
>
> len = (hdr_buf[2] << 8) + hdr_buf[3] + 4;
> @@ -696,7 +700,7 @@ static int ses_intf_add(struct device *cdev,
> if (!buf)
> goto err_free;
>
> - result = ses_recv_diag(sdev, 10, buf, len);
> + result = ses_recv_diag(sdev, page, buf, len);
> if (result)
> goto recv_failed;
> ses_dev->page10 = buf;
> @@ -733,8 +737,9 @@ static int ses_intf_add(struct device *cdev,
> return 0;
>
> recv_failed:
> - sdev_printk(KERN_ERR, sdev, "Failed to get diagnostic page 0x%x\n",
> - result);
> + sdev_printk(KERN_ERR, sdev,
> + "Failed to get diagnostic page %d with error %d\n",
> + page, result);
> err = -ENODEV;
> err_free:
> kfree(buf);
This looks OK but I think we should consider suppressing these
messages. There are a lot of devices that return page 0 instead of
the correct page, we should either say something like "enclosure
services not supported", or else work with what we get instead of
logging a message on every device probe that make it sound like
the device is failing. I've received several complaints about this.
Reviewed-by: Ewan D. Milne <emilne@redhat.com>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] ses: fix error message in ses_intf_add()
2017-05-17 13:41 ` Ewan D. Milne
@ 2017-05-18 0:18 ` Martin K. Petersen
0 siblings, 0 replies; 3+ messages in thread
From: Martin K. Petersen @ 2017-05-18 0:18 UTC (permalink / raw)
To: Ewan D. Milne; +Cc: Maurizio Lombardi, linux-scsi, jejb, martin.petersen
Ewan,
> This looks OK but I think we should consider suppressing these
> messages. There are a lot of devices that return page 0 instead of
> the correct page, we should either say something like "enclosure
> services not supported", or else work with what we get instead of
> logging a message on every device probe that make it sound like
> the device is failing.
I agree.
> I've received several complaints about this.
Me too. There appears to be a ton of bad devices out there.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-05-18 0:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-17 13:00 [PATCH] ses: fix error message in ses_intf_add() Maurizio Lombardi
2017-05-17 13:41 ` Ewan D. Milne
2017-05-18 0:18 ` Martin K. Petersen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox