Re: [PATCH 0/4] scsi: 64-bit LUN support

[Hannes Reinecke <hare@suse.de>]

On 03/31/2013 07:44 PM, Tomas Henzl wrote:
> On 03/30/2013 05:53 PM, Hannes Reinecke wrote:
>> On 03/29/2013 05:32 PM, Tomas Henzl wrote:
[ .. ]
>>>
>>> in scsi_report_lun_scan is max_lun compared with the result of scsilun_to_int,
>>> but in that value is also stored the address method. This means, that we compare
>>> the max_lun to a LUN 'handle' which doesn't seem to make much sense.
>>> This makes that test dependent on which address method is used and not
>>> only to the LUN number which is I think expected.
>>> The solution is to have a new function 'scsilun_to_num', (I can send a patch)
>>> or let the individual drivers set the max_lun to -1 and test for the allowed LUNs
>>> in the driver.
>>>
>> You sure this is necessary?
> 
> This is not directly related to your 64bit patch, I just 'used' this thread
> to discuss another issue - sorry.
> 
Yeah, and thereby decreasing the chance of getting my patchset in :-(

>>
>> I would like to avoid having to parse the LUN number for validity as we 
>> cannot guarantee this check has any meaning for the target.
>> The only authoritative check can be made by the target.
> 
> What we can do is to decode the LUN and compare it to max_lun provided by the driver,
> I think that sg_luns is able to do that, so what is needed is just to follow the SAM. 
> 
> I have seen reports of problem on three different drivers connected to various 
> external storage, all of them having the same basic reason - the driver sets a max_lun
> and then LUN comes encoded with a newer addressing method and something like this is shown
> 'kernel: scsi: host 2 channel 0 id 2 lun16643 has a LUN larger than allowed by the host adapter'
> 
> Decoding the real LUN value would fix this problem, by decoding is only meant the use in 
> scsi_report_lun_scan. The LUN would be stored exactly the same way as it is now.
> I know we can patch the certain drivers too, but when max_lun were  what the name says 
> - max LU number, it would fix my problem very easy.
> 
Errm.

No. Decoding LUNs is _evil_. It has only a relevance on the target,
and even then it might choose to ignore it.
So we cannot try to out-guess the target here.

The error you're reporting is that lpfc is setting max_luns to
'255', which of course is less than 16643. Increasing max_luns on
lpfc to '0xFFFF' will fix your problem; nothing to do with 64-bit
LUNs ...

>>
>> In the 64-bit context the max_luns should rather be interpreted as a
>> 'max bits' setting, ie the number of _bits_ per LUN number the HBA is 
>> able to transport.
>> But renaming 'max_luns' to 'max_bits' is a bit pointless, as it would 
>> break backwards compability for no real gain.
>>
>> So with my patchset we have a two-step LUN validation:
>> - max_luns controls the max LUN number
>>    (read: max number of bits per LUN) which can be transported
>>    to the target
> 
> This means in fact only 32 or 64 bit - so a single bit flag is enough?
> 
No, it is not.

We have SPI narrow with max_luns = 8, SPI wide with max_luns = 16,
some (older) FC HBAs supporting only 8 or 16 bits LUNs, some RAID
arrays supporting 32-bit LUNs etc.

Cheers,

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