On Thu, May 18, 2006 at 12:36:52PM -0600, Matthew Wilcox wrote:
> I went for:
>
> scsi: 2:0:1:0: Vendor: HP 18.2G Model: ATLAS10K3_18_SCA Rev: HP05 ANSI rev: 02
That is very nice ... as is replacing print_inquiry with one line of code.
> SCSI device sda: 35566480 512-byte hdwr sectors (18210 MB)
> sda: Write Protect is off
> SCSI device sda: drive cache: write through w/ FUA
> sda: sda1 sda2 sda3
> sd 2:0:1:0: Attached scsi disk sda
> sd 2:0:1:0: Attached scsi generic sg0 type 0
>
>
> wouldn't this look better?
>
> sda: 35566480 512-byte hdwr sectors (18210 MB)
> sda: Write Protect is off
> sda: drive cache: write through w/ FUA
> sda: sda1 sda2 sda3
> sd 2:0:1:0: Attached scsi disk sda
> sd 2:0:1:0: Attached scsi generic sg0 type 0
Yes, better. I guess those should all be sdev_printk in sd.c.
Funky how loading sd after sg changes the output ... and using the driver
name as a prefix sometimes messes this up for scsi.
i.e. scan without sd_mod or sg loaded (and distro I'm using loads sg
before sd_mod via udev rules):
0:0:0:0: Attached scsi generic sg0 type 0
0:0:0:1: Attached scsi generic sg1 type 0
Then remove/add those devices, and sg lines become:
sd 1:0:0:0: Attached scsi generic sg0 type 0
sd 1:0:0:1: Attached scsi generic sg1 type 0
> > > And maybe use printk("%-16s") formatting? But garbage might get printed
> > > for non-ASCII (though the SCSI specs say it is not allowed ...).
>
> I think you meant %.16s, not %-16s; it's already padded, we just need a
> maximum byte count.
Yes, I was thinking it was not padded.
> Anyway, I noticed that sdev has everything I need in it, and it's
> definitely clearer than using the inquiry data directly. The one thing
> I don't do this for is scsi_level; we want what the device returned,
> not what we've mangled it to.
>
> The comment about BLIST_ISROM is a little too terse for me to know what
> to do; can anyone hazard a guess?
It is the only place that we modify the inquiry result, and I thought it
was gross and (a bit) confusing.
That is, after your patch, it could change to (and the no_uld_attach check
should not really be an "else"):
--- inq-print-linux-2.6.17-rc4-git6/drivers/scsi/o-scsi_scan.c 2006-05-18 11:57:28.000000000 -0700
+++ inq-print-linux-2.6.17-rc4-git6/drivers/scsi/scsi_scan.c 2006-05-18 12:06:05.000000000 -0700
@@ -598,18 +598,18 @@ static int scsi_add_lun(struct scsi_devi
sdev->model = (char *) (sdev->inquiry + 16);
sdev->rev = (char *) (sdev->inquiry + 32);
- if (*bflags & BLIST_ISROM) {
- /*
- * It would be better to modify sdev->type, and set
- * sdev->removable; this can now be done since
- * print_inquiry has gone away.
- */
- inq_result[0] = TYPE_ROM;
- inq_result[1] |= 0x80; /* removable */
- } else if (*bflags & BLIST_NO_ULD_ATTACH)
+ if (*bflags & BLIST_NO_ULD_ATTACH)
sdev->no_uld_attach = 1;
- switch (sdev->type = (inq_result[0] & 0x1f)) {
+ if (*bflags & BLIST_ISROM) {
+ sdev->type = TYPE_ROM;
+ sdev->removable = 1;
+ } else {
+ sdev->type = (inq_result[0] & 0x1f);
+ sdev->removable = (0x80 & inq_result[1]) >> 7;
+ }
+
+ switch (sdev->type) {
case TYPE_TAPE:
case TYPE_DISK:
case TYPE_PRINTER:
@@ -652,7 +652,6 @@ static int scsi_add_lun(struct scsi_devi
*/
sdev->inq_periph_qual = (inq_result[0] >> 5) & 7;
- sdev->removable = (0x80 & inq_result[1]) >> 7;
sdev->lockable = sdev->removable;
sdev->soft_reset = (inq_result[7] & 1) && ((inq_result[3] & 7) == 2);
-- Patrick Mansfield
-
: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
