Re: [PATCH RFC] use struct scsi_lun in generic code

[Patrick Mansfield <patmans@us.ibm.com>]

Nice work Jeff ...

On Sun, Oct 23, 2005 at 12:33:01AM -0400, Jeff Garzik wrote:

> * export scsilun_to_int()
> 
> * create scsilun_to_str() helper

> -	if (sdev->scsi_level <= SCSI_2)
> +	if (sdev->scsi_level <= SCSI_2) {
> +		unsigned int tmp_lun = scsilun_to_int(&sdev->lun);
>  		scmd->cmnd[1] = (scmd->cmnd[1] & 0x1f) |
> -			(sdev->lun << 5 & 0xe0);
> +			(tmp_lun << 5 & 0xe0);
> +	}

Why convert the lun value for every IO? Not efficient here and in the
driver code.

Instead call scsilun_to_int once per sdev, and store the value in
sdev->lun, sort of like James earlier patch, but only do so based on a
shost->opaque_lun_value or better if we could call a transport function
once to setup sdev->lun as needed by the driver, except we have transports
that include drivers/HBA's that use different sized LUNs.

Then struct scsi_lun has to be a u64 or another sXXX_lun(XXX).

>  	snprintf(sdev->sdev_classdev.class_id, BUS_ID_SIZE,
> -		 "%d:%d:%d:%d", sdev->host->host_no,
> -		 sdev_channel(sdev), sdev_id(sdev), sdev->lun);
> +		 "%d:%d:%d:%s", sdev->host->host_no,
> +		 sdev_channel(sdev), sdev_id(sdev),
> +		scsilun_to_str(sdev, lunstr));

BUS_ID_SIZE is KOBJ_NAME_LEN is still 20. I thought we were moving to hex
for the bus_id value?

> +static inline char *scsilun_to_str(struct scsi_device *sdev, char *s)
> +{
> +	sprintf(s, "%d", scsilun_to_int(&sdev->lun));
> +	return s;

And format the LUN output based on the shost->opaque_luns. IMO for
!opaque_lun_value print LUN in the same order as today, else print 8 byte
hex string.

Some (probably most) users of drivers/transports with 8 byte LUN support
will be confused one way or another. Yes, most will expect to see 1 for
LUN 0001000000000000. We can't always display the LUN the same as used by
storage vendors in their setup/administration tools.

Perhaps base parsing of user space LUN values on the shost->opaque_lun_value,
I don't know how we can support 8 byte LUNs from user space and be
backwards compatible without adding another attribute or interface.

Note that current SAM (sam4r02) sort of allows for different sized LUNs,
it says "A logical unit number shall contain 64 bits or 16 bits, with the
size being defined by the SCSI transport protocol.  For SCSI transport
protocols that define 16-bit logical unit numbers, the two bytes shall be
formatted as described for the FIRST LEVEL ADDRESSING field (see table 5
in 4.9.5)."

-- Patrick Mansfield