Re: REPORT LUNS

["Josef Möllers" <josef.moellers@fujitsu-siemens.com>]

Patrick Mansfield wrote:
> 
> On Mon, Jun 16, 2003 at 08:55:18AM +0200, Josef Möllers wrote:
> > Patrick Mansfield wrote:
> > >
> > > On Fri, Jun 13, 2003 at 04:04:48PM +0200, Josef Möllers wrote:
> > > > Hi,
> > > >
> > > > Wile trying to figure out how to ignore non-existing LUNs on RAID
> > > > devices, I have fallen across an error in scsilun_to_int() where it
> > > > doesn't behave as the comment suggests and a problematic piece of code
> > > > in scsi_report_lun_scan().
> > > >
> > > > Patch is included.
> > >
> > > If we clear the upper bits (address method, per SAM-3), someone has to
> > > eventually set them again, and this is not handled today in linux scsi
> > > core.
> >
> > The comment said that the upper two bits were ignored which they were
> > not in the original code, so, apart from fixing the shifts, I also added
> > the mask to keep the code in sync with the comment. I haven't yet
> > followed the use of the value thus created.
> 
> Ignored meaning "does nothing with it", when we should probably do
> something - proper conversion, an error or warning output. The current
> code might work (with high bits set), if we did not exceed the
> host->max_lun. We might need an 8 byte lun everywhere to make it work
> right for all cases.

OK.

> Can you give more details of the problem you are seeing, and software
> (adapter driver) and hardware in use?

The main problem we have is with current 2.4 kernels that some RAID
systems report SCSI-2 but allow for more than 8 LUNs, so rather than put
each box into the device_list[] (BLIST_SPARSELUN), I was investigating
how this problem could be solved in an orderly fashion: either by using
REPORT LUNS or by examining the Peripheral Qualifier in the INQUIRY data
(which would require BLIST_SPARSELUN, nonetheless).
I looked at the 2.5 code and found that REPORT LUNS was implemented but
stumbled across the comment. I couln't really make sense out of SCSI-3
and a manual I have for EMC RAID subsystems on the data returned by the
REPORT LUNS command. I then looked at the code, got confused about the
shifts and broke the code with my patch.

> I did not notice your shift changes - I don't see how they work or what
> it fixes.

Looking at it fresh in the morning: no, it doesn't fix anything, it
breaks code which is OK. I learnt my lesson to be more careful.

> If scsi_lun was: 0x1 0x2 0x3 0x4 0 0 0 0
> 
> Current code:
> 
> First loop in scsilun_to_int, with i = 0:
> 
>         lun = 0 | ( (0x1 << 8 | 0x2) << 0) = 0x0102
> 
> Second loop i = 2:
> 
>         lun = 0x0102 | ( (0x3 << 8 | 0x4) << 16) = 0x03040102
> 
> Your patch would give:
> 
> First loop i = 0:
> 
>         lun = 0 | ((0x1 & 0x3f) << 1) | (0x2 << 0) = 0x02
> 
> Second loop i = 2:
> 
>         lun = 0x02 | ( ((0x3 & 0x3f) << 17) | (0x4 << 16) = 0x00060002

You're absolutely right. If i needed an excuse (other than not having
been careful enough), I'd attribute it to the heat.

I'll test my patches more thoroughly in the future.

Sorry,

Josef
-- 
Josef Möllers (Pinguinpfleger bei FSC)
	If failure had no penalty success would not be a prize
						-- T.  Pratchett
-
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