public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: Patrick Mansfield <patmans@us.ibm.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: James Bottomley <James.Bottomley@SteelEye.com>,
	SCSI development list <linux-scsi@vger.kernel.org>,
	Douglas Gilbert <dougg@torque.net>
Subject: Re: [PATCH] SCSI core: always store >= 36 bytes of INQUIRY data
Date: Sat, 28 Oct 2006 11:30:21 -0700	[thread overview]
Message-ID: <20061028183021.GA26419@us.ibm.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.0610281125320.2757-100000@netrider.rowland.org>

On Sat, Oct 28, 2006 at 11:33:06AM -0400, Alan Stern wrote:
> On Fri, 27 Oct 2006, Patrick Mansfield wrote:
> 
> > On Thu, Oct 26, 2006 at 05:11:27PM -0400, Alan Stern wrote:
> > > This patch (as810) sets the length of the INQUIRY data to a minimum of
> > > 36 bytes, even if the device claims that not all of them are valid.
> > > Using the data sent by the device is better than allocating a short
> > > buffer and then reading beyond the end of it, which is what we do now.
> > > 
> > > Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> > > 
> > > ---
> > > 
> > > Index: usb-2.6/drivers/scsi/scsi_scan.c
> > > ===================================================================
> > > --- usb-2.6.orig/drivers/scsi/scsi_scan.c
> > > +++ usb-2.6/drivers/scsi/scsi_scan.c
> > > @@ -575,6 +575,19 @@ static int scsi_probe_lun(struct scsi_de
> > >  	 * short INQUIRY), an abort here prevents any further use of the
> > >  	 * device, including spin up.
> > >  	 *
> > > +	 * On the whole, the best approach seems to be to assume the first
> > > +	 * 36 bytes are valid no matter what the device says.  That's
> > 
> > The comment is confusing, as it implies the device will modify data past
> > the indicated length, but well behaved devices should not do that, and
> > with your patch should point to zero filled data.
> > 
> > Just comment on what its avoiding or such like:
> > 
> > 	Modify short inquiry_len values so we don't later point at random
> > 	values. Devices returning an incorrect value in the INQUIRY
> > 	additional length field will point at potentially valid data for
> > 	Vendor, Product and Revsion, while conforming devices will point
> > 	to zero filled data.
> > 
> > But definitely better to use possibly valid data for broken devices or
> > NUL for well behaved devices rather than garbage values.
> 
> In the context of the comment already there in the code, your suggested 
> text would be somewhat jarring.  I'll try to re-write it in a way that 
> should satisfy everybody.

OK, though not sure what you mean, we could still point at garbage data
and lookup in the devinfo list based on that garbage. And the issue with
INQUIRY potentially changing after the device is spun up still exists.

[There were even disk drives available in the past that modified the
vendor and product id after spin up, this would be confusing but not a big
problem as long as they are not needed in black/white lists, you could
still issue an INQUIRY from user space and see current values.]

> BTW, isn't it conceivable that well-behaved devices could send 36 bytes,
> with only the first 8 bytes marked valid and non-NULL garbage in the last
> 28?  The spec isn't very clear about this; it says only that the standard
> INQUIRY data contains 36 required bytes.

So the additional length would be 3, but 36 bytes are transferred
including some garbage ...

I don't know and could not find anything in the specs. Maybe Doug or
others can answer. AFAICT if the device doesn't have the data available it
should return space filled values.

I thought there was some mention in some SCSI spec of an INQUIRY of less
than 36 bytes and then mention of the handling of it, but can't find
anything (searched in SPC spc3r32.pdf, SPC 2 spc2r20.pdf, SAM sam4r04.pdf
and even MMC mmc5r01b.pdf). So I'm wrong to say conforming devices would
point to zero filled data if the INQUIRY should return at least 36 bytes.

-- Patrick Mansfield

  reply	other threads:[~2006-10-28 18:30 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-10-26 21:11 [PATCH] SCSI core: always store >= 36 bytes of INQUIRY data Alan Stern
2006-10-27 21:43 ` Patrick Mansfield
2006-10-27 22:10   ` Douglas Gilbert
2006-10-28 15:41     ` Alan Stern
2006-10-30 16:40       ` Douglas Gilbert
2006-10-28 15:33   ` Alan Stern
2006-10-28 18:30     ` Patrick Mansfield [this message]
2006-10-30 15:20   ` Alan Stern
2006-10-30 16:32     ` Stefan Richter
2006-10-31 21:26       ` Alan Stern

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20061028183021.GA26419@us.ibm.com \
    --to=patmans@us.ibm.com \
    --cc=James.Bottomley@SteelEye.com \
    --cc=dougg@torque.net \
    --cc=linux-scsi@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox