[PATCH] Make errors printed from scsi_sysfs_add_sdev() mean something.

[PATCH] Make errors printed from scsi_sysfs_add_sdev() mean something.

[Michał Mirosław]

This is an updated version of patch I wrote long time ago, but looks like
it didn't get far. Patch against linux-2.6.git. And yes - printk() lines
are longer than 80 chars - it shouldn't matter here (grep is happy ;).

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 93c28f3..7a3389c 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -872,12 +872,12 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
 	error = device_add(&sdev->sdev_gendev);
 	if (error) {
 		put_device(sdev->sdev_gendev.parent);
-		printk(KERN_INFO "error 1\n");
+		printk(KERN_ERR "sysfs: SCSI device registration failed: %d\n", error);
 		return error;
 	}
 	error = device_add(&sdev->sdev_dev);
 	if (error) {
-		printk(KERN_INFO "error 2\n");
+		printk(KERN_ERR "sysfs: SCSI class device registration failed: %d\n", error);
 		goto clean_device;
 	}
 
--
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

Re: [PATCH] Make errors printed from scsi_sysfs_add_sdev() mean something.

[Hannes Reinecke]

Hi Michal,

Michał Mirosław wrote:
> This is an updated version of patch I wrote long time ago, but looks like
> it didn't get far. Patch against linux-2.6.git. And yes - printk() lines
> are longer than 80 chars - it shouldn't matter here (grep is happy ;).
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> 
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index 93c28f3..7a3389c 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -872,12 +872,12 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
>  	error = device_add(&sdev->sdev_gendev);
>  	if (error) {
>  		put_device(sdev->sdev_gendev.parent);
> -		printk(KERN_INFO "error 1\n");
> +		printk(KERN_ERR "sysfs: SCSI device registration failed: %d\n", error);
Shouldn't that be sdev_printk()?

>  		return error;
>  	}
>  	error = device_add(&sdev->sdev_dev);
>  	if (error) {
> -		printk(KERN_INFO "error 2\n");
> +		printk(KERN_ERR "sysfs: SCSI class device registration failed: %d\n", error);
Same here ...

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, 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

Re: [PATCH] Make errors printed from scsi_sysfs_add_sdev() mean something.

[Stefan Richter]

Hannes Reinecke wrote:
> Michał Mirosław wrote:
>> --- a/drivers/scsi/scsi_sysfs.c
>> +++ b/drivers/scsi/scsi_sysfs.c
>> @@ -872,12 +872,12 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
>>  	error = device_add(&sdev->sdev_gendev);
>>  	if (error) {
>>  		put_device(sdev->sdev_gendev.parent);
>> -		printk(KERN_INFO "error 1\n");
>> +		printk(KERN_ERR "sysfs: SCSI device registration failed: %d\n", error);
> Shouldn't that be sdev_printk()?

It seems it shouldn't.  sdev_printk() wants device name and bus ID of
the sdev_gendev.  AFAICS device_add() initializes them, and callers of
device_add() can't be quite sure how far device_add() got before an error.

>>  		return error;
>>  	}
>>  	error = device_add(&sdev->sdev_dev);
>>  	if (error) {
>> -		printk(KERN_INFO "error 2\n");
>> +		printk(KERN_ERR "sysfs: SCSI class device registration failed: %d\n", error);
> Same here ...

At that point, sdev_printk() should work indeed.
-- 
Stefan Richter
-=====-==--= ---= -==--
http://arcgraph.de/sr/
--
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

Re: [PATCH] Make errors printed from scsi_sysfs_add_sdev() mean something.

[Hannes Reinecke]

Hi Stefan,

Stefan Richter wrote:
> Hannes Reinecke wrote:
>> Michał Mirosław wrote:
>>> --- a/drivers/scsi/scsi_sysfs.c
>>> +++ b/drivers/scsi/scsi_sysfs.c
>>> @@ -872,12 +872,12 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
>>>  	error = device_add(&sdev->sdev_gendev);
>>>  	if (error) {
>>>  		put_device(sdev->sdev_gendev.parent);
>>> -		printk(KERN_INFO "error 1\n");
>>> +		printk(KERN_ERR "sysfs: SCSI device registration failed: %d\n", error);
>> Shouldn't that be sdev_printk()?
> 
> It seems it shouldn't.  sdev_printk() wants device name and bus ID of
> the sdev_gendev.  AFAICS device_add() initializes them, and callers of
> device_add() can't be quite sure how far device_add() got before an error.
> 
Hmm. True.

Easiest way would be to remove the printk altogether, as a failure of
device_add() will print out enough warnings as it stands.
To be precise you get something like

------------[ cut here ]------------
WARNING: at fs/sysfs/dir.c:463 sysfs_add_one+0x33/0x3a()
sysfs: duplicate filename '4:0:1:0' can not be created
...
---[ end trace dac6dbc3f072e760 ]---

and

kobject_add_internal failed for 4:0:1:0 with -EEXIST, don't try to register things with the same name in the same directory.
Pid: 2693, comm: scsi_wq_4 Tainted: G        W 2.6.27.10-20090109 #2
...

so we have enough information already.

>>>  		return error;
>>>  	}
>>>  	error = device_add(&sdev->sdev_dev);
>>>  	if (error) {
>>> -		printk(KERN_INFO "error 2\n");
>>> +		printk(KERN_ERR "sysfs: SCSI class device registration failed: %d\n", error);
>> Same here ...
> 
> At that point, sdev_printk() should work indeed.
But it'll spawn a similar error as above, so we
could remove it here, too.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, 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