[PATCH] scsi: vpd pages are mandatory for SPC-2

[PATCH] scsi: vpd pages are mandatory for SPC-2

[Hannes Reinecke]

VPD pages 0x0 and 0x83 are mandatory even for SPC-2, so we should
be lowering the restriction to avoid having to whitelist every
SPC-2 compliant device.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 include/scsi/scsi_device.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 06a2f83..2536ade 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -538,9 +538,9 @@ static inline int scsi_device_supports_vpd(struct scsi_device *sdev)
 	/*
 	 * Although VPD inquiries can go to SCSI-2 type devices,
 	 * some USB ones crash on receiving them, and the pages
-	 * we currently ask for are for SPC-3 and beyond
+	 * we currently ask for are mandatory for SPC-2 and beyond
 	 */
-	if (sdev->scsi_level > SCSI_SPC_2 && !sdev->skip_vpd_pages)
+	if (sdev->scsi_level >= SCSI_SPC_2 && !sdev->skip_vpd_pages)
 		return 1;
 	return 0;
 }
-- 
1.8.5.6

Re: [PATCH] scsi: vpd pages are mandatory for SPC-2

[Martin K. Petersen]

>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:

Hannes,

Hannes> VPD pages 0x0 and 0x83 are mandatory even for SPC-2, so we
Hannes> should be lowering the restriction to avoid having to whitelist
Hannes> every SPC-2 compliant device.

I wonder how many non-compliant devices we would have to blacklist as a
result of this change...

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] scsi: vpd pages are mandatory for SPC-2

[Hannes Reinecke]

On 03/15/2016 12:37 AM, Martin K. Petersen wrote:
>>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:
> 
> Hannes,
> 
> Hannes> VPD pages 0x0 and 0x83 are mandatory even for SPC-2, so we
> Hannes> should be lowering the restriction to avoid having to whitelist
> Hannes> every SPC-2 compliant device.
> 
> I wonder how many non-compliant devices we would have to blacklist as a
> result of this change...
> 
But it feels even sillier having to whitelist every
standards-conformant device here; I certainly was when I figured
that EMC Clariion won't work properly without this patch.

And the idea was to mark off _misbehaving_ drives, not the other way
round.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (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] scsi: vpd pages are mandatory for SPC-2

[Christoph Hellwig]

On Tue, Mar 15, 2016 at 08:28:42AM +0100, Hannes Reinecke wrote:
> But it feels even sillier having to whitelist every
> standards-conformant device here; I certainly was when I figured
> that EMC Clariion won't work properly without this patch.
> 
> And the idea was to mark off _misbehaving_ drives, not the other way
> round.

It's the idea, but it has to prove workable as well in the end.

That being said I'd prefer to allow Hannes patch in and see what
the fallout is. 

Re: [PATCH] scsi: vpd pages are mandatory for SPC-2

[Douglas Gilbert]

On 16-03-15 08:42 AM, Christoph Hellwig wrote:
> On Tue, Mar 15, 2016 at 08:28:42AM +0100, Hannes Reinecke wrote:
>> But it feels even sillier having to whitelist every
>> standards-conformant device here; I certainly was when I figured
>> that EMC Clariion won't work properly without this patch.
>>
>> And the idea was to mark off _misbehaving_ drives, not the other way
>> round.
>
> It's the idea, but it has to prove workable as well in the end.
>
> That being said I'd prefer to allow Hannes patch in and see what
> the fallout is.

I don't have many drafts archived for SPC-2 but the two that I do have:
    spc2r18.pdf [20000521]
    spc2r20.pdf [20010718]

indicate that those two VPD pages were made mandatory right at the
end of the draft cycle for SPC-2. spc2r20.pdf was the last draft
prior to the standard and it does show them as mandatory in table 185
on page 218. The corresponding table in spc2r18.pdf does not have
a column indicating whether the listed VPD pages are mandatory or
not (or words that I can find in the description of VPD pages 0x0
and 0x83 to suggest they are mandatory).

Doug Gilbert

Re: [PATCH] scsi: vpd pages are mandatory for SPC-2

[Hannes Reinecke]

On 03/15/2016 06:19 PM, Douglas Gilbert wrote:
> On 16-03-15 08:42 AM, Christoph Hellwig wrote:
>> On Tue, Mar 15, 2016 at 08:28:42AM +0100, Hannes Reinecke wrote:
>>> But it feels even sillier having to whitelist every
>>> standards-conformant device here; I certainly was when I figured
>>> that EMC Clariion won't work properly without this patch.
>>>
>>> And the idea was to mark off _misbehaving_ drives, not the other way
>>> round.
>>
>> It's the idea, but it has to prove workable as well in the end.
>>
>> That being said I'd prefer to allow Hannes patch in and see what
>> the fallout is.
> 
> I don't have many drafts archived for SPC-2 but the two that I do have:
>    spc2r18.pdf [20000521]
>    spc2r20.pdf [20010718]
> 
> indicate that those two VPD pages were made mandatory right at the
> end of the draft cycle for SPC-2. spc2r20.pdf was the last draft
> prior to the standard and it does show them as mandatory in table 185
> on page 218. The corresponding table in spc2r18.pdf does not have
> a column indicating whether the listed VPD pages are mandatory or
> not (or words that I can find in the description of VPD pages 0x0
> and 0x83 to suggest they are mandatory).
> 
The point here is not so much whether the pages are mandatory, but
whether access to VPD pages crashes the device.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.com			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (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] scsi: vpd pages are mandatory for SPC-2

[Martin K. Petersen]

>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:

>> I wonder how many non-compliant devices we would have to blacklist as
>> a result of this change...

Hannes> But it feels even sillier having to whitelist every
Hannes> standards-conformant device here; I certainly was when I figured
Hannes> that EMC Clariion won't work properly without this patch.

Hannes> And the idea was to mark off _misbehaving_ drives, not the other
Hannes> way round.

I understand. But my concern is that the number of broken USB doodad
model strings far outnumber enterprise storage vendor ditto. And there
is no point in reversing the polarity if it increases the maintenance
burden.

So I would be more comfortable with just widening the VPD whitelist and
assume that everything with an EMC, Dell, NetApp, whatever vendor ID is
compliant.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] scsi: vpd pages are mandatory for SPC-2

[Hannes Reinecke]

On 03/15/2016 10:36 PM, Martin K. Petersen wrote:
>>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:
> 
>>> I wonder how many non-compliant devices we would have to blacklist as
>>> a result of this change...
> 
> Hannes> But it feels even sillier having to whitelist every
> Hannes> standards-conformant device here; I certainly was when I figured
> Hannes> that EMC Clariion won't work properly without this patch.
> 
> Hannes> And the idea was to mark off _misbehaving_ drives, not the other
> Hannes> way round.
> 
> I understand. But my concern is that the number of broken USB doodad
> model strings far outnumber enterprise storage vendor ditto. And there
> is no point in reversing the polarity if it increases the maintenance
> burden.
> 
> So I would be more comfortable with just widening the VPD whitelist and
> assume that everything with an EMC, Dell, NetApp, whatever vendor ID is
> compliant.
> 
Well, yes, that's the bugger. You never know until you try.

But we have been asking for VPD pages per default for quite some
time now from userspace (thanks to udev), and so far have had only a
few bug reports for misbehaving devices.

And again, it just feels wrong to punish every well-behaved device
out there, for the sake of some buggy USB implementation of a few
bucks worth.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.com			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (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] scsi: vpd pages are mandatory for SPC-2

[Hannes Reinecke]

On 03/16/2016 08:38 AM, Hannes Reinecke wrote:
> On 03/15/2016 10:36 PM, Martin K. Petersen wrote:
>>>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:
>>
>>>> I wonder how many non-compliant devices we would have to blacklist as
>>>> a result of this change...
>>
>> Hannes> But it feels even sillier having to whitelist every
>> Hannes> standards-conformant device here; I certainly was when I figured
>> Hannes> that EMC Clariion won't work properly without this patch.
>>
>> Hannes> And the idea was to mark off _misbehaving_ drives, not the other
>> Hannes> way round.
>>
>> I understand. But my concern is that the number of broken USB doodad
>> model strings far outnumber enterprise storage vendor ditto. And there
>> is no point in reversing the polarity if it increases the maintenance
>> burden.
>>
>> So I would be more comfortable with just widening the VPD whitelist and
>> assume that everything with an EMC, Dell, NetApp, whatever vendor ID is
>> compliant.
>>
> Well, yes, that's the bugger. You never know until you try.
> 
> But we have been asking for VPD pages per default for quite some
> time now from userspace (thanks to udev), and so far have had only a
> few bug reports for misbehaving devices.
> 
> And again, it just feels wrong to punish every well-behaved device
> out there, for the sake of some buggy USB implementation of a few
> bucks worth.
> 
And incidentally, for most USB drives VPD pages are already blanked out:

drivers/usb/storage/scsiglue.c:slave_configure()
[ .. ]
		/* Some devices don't handle VPD pages correctly */
		sdev->skip_vpd_pages = 1;


So it looks we're dealing with a non-issue here.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (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] scsi: vpd pages are mandatory for SPC-2

[Martin K. Petersen]

>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:

Hannes> VPD pages 0x0 and 0x83 are mandatory even for SPC-2, so we
Hannes> should be lowering the restriction to avoid having to whitelist
Hannes> every SPC-2 compliant device.

You forgot to properly submit the patch that moves the vpd heuristic to
scsi_device.h (I think you originally sent that as an inline RFC).

In any case. This patch depends on it. Please resend these two as a
series.

Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering