Pete Wyckoff wrote:
> markh794 at gmail.com wrote on Fri, 24 Aug 2007 18:15 +1000:
>
>> Unfortunately I had to change the lu_init() interface so I could more
>> easily include the TID in the SCSI VPD page 80h and 83h
>>
>> Hence the number of files touched with this patch.
>>
>> As indicated in the patch, I am unsure from the osd2r01.pdf
>> documentation of what data should be included within VPD pages B0h and
>> B1h for the OSD module.
>>
>
> This looks great. Thanks for looking at the OSD oddball parameters.
> I'll patch them up once this goes in.
>
>
I need them for the SSC devices as well :)
>> From: Mark Harvey <markh794 at gmail.com>
>> Date: Fri, 24 Aug 2007 18:04:24 +1000
>> Subject: Add support for VPD pages 0x80 - 0xff
>>
>> Ability to add VPD pages between 80h and FFh per SCSI device type.
>>
>> An array of 128 vpd structures added to ly_phy_attr struct.
>>
>> Use alloc_vpd(data size) to pre-alloc data for custom VPD page.
>> - This data is appended to the 4 byte VPD header at runtime
>> i.e. When an INQUIRY for the VPD page is received.
>>
>> - A custom vpd_update(struct scsi_lu *lu, void *) is used to
>> set/change data pre-allocated by alloc_vpd()
>>
>> - All modules except use default page 80h & 83h defined in spc.c
>> The osd module has two extra for VPD pages B0h and B1h
>> - Note: garbage values are set and should be reviewed/updated
>> by somebody who knows what should be set here.
>>
>> Signed-off-by: Mark Harvey <markh794 at gmail.com>
>> ---
>>
> [..]
>
>> diff --git a/scripts/tgt-core-test b/scripts/tgt-core-test
>>
> [..]
>
>> +tgtadm --lld iscsi --mode logicalunit --op update --tid $TID --lun $LUN \
>> + --params vendor_id=OSD,product_id=OSD00,product_rev=0010,removable=1,sense_format=1
>>
>
> OSD sense format is always descriptor. Cannot be old style, says the
> spec. But there's lots more ways root can break things.
>
> [..]
>
Yep - Give the user a loaded gun....
Its up the them if they ignore the standards and 'pull the trigger'.
Maybe I should log an error via the OSD device if set contrary to the spec.
- Hopefully make it a little easier for somebody setting up an OSD
device and discover what they did wrong.
>> -int spc_lu_init(struct scsi_lu *lu)
>> +int spc_lu_init(struct scsi_lu *lu, int tid)
>> {
>> + struct vpd **lu_vpd = lu->attrs.vpd;
>> +
>> + lu->attrs.device_type = lu->dev_type_template.type;
>> + lu->attrs.qualifier = 0x0;
>> +
>> snprintf(lu->attrs.vendor_id, sizeof(lu->attrs.vendor_id),
>> "%-16s", VENDOR_ID);
>> snprintf(lu->attrs.product_rev, sizeof(lu->attrs.product_rev),
>> "%s", "0001");
>> + snprintf(lu->attrs.scsi_id, sizeof(lu->attrs.scsi_id),
>> + "deadbeaf%d:%" PRIu64, tid, lu->lun);
>> + snprintf(lu->attrs.scsi_sn, sizeof(lu->attrs.scsi_sn),
>> + "beaf%d%" PRIu64, tid, lu->lun);
>>
>
> The only reason you pass in the tid is to create these fake values?
>
Yes
> Would it be better to add a struct target * in struct scsi_lu
> instead? Seems a bit hacky to pass around tid just for this.
>
Adding TID into the scsi_lu struct did cross my mind when I was doing this.
- More space consumed in the structure for a 'one off' use vs passing
the TID.. I went with the passing of TID.
I don't mind either way.
> [..]
>
>> +struct vpd {
>> + uint16_t size;
>> + void (*vpd_update)(struct scsi_lu *lu, void *data);
>> + uint8_t data[0];
>> +};
>> +
>> struct lu_phy_attr {
>> char scsi_id[SCSI_ID_LEN + 1];
>> char scsi_sn[SCSI_SN_LEN + 1];
>> @@ -52,10 +61,9 @@ struct lu_phy_attr {
>> char online; /* Logical Unit online */
>> char reset; /* Power-on or reset has occured */
>> char sense_format; /* Descrptor format sense data supported */
>> -};
>>
>> -struct scsi_lu;
>> -struct scsi_cmd;
>> + struct vpd *vpd[0x80]; /* VPD pages 0x80 -> 0xff masked with 0x80*/
>> +};
>>
>
> 1 kB of pointer space. Guess that's not a big deal. A linked list
> would have been the other option you could consider. I was going to
> say the need to add and subtract 0x80 here and there was cumbersome,
> but fixing that would cost another 1 kB.
>
> -- Pete
>
>
Original patches for MODE SENSE/SELECT used a linked list, which was
then simplified to an array. Hence I went with the array this time..
As you say allocating 256 entries for less then 10 active entries
(typically 2) seemed a little extreme. The mask of 0x7f seemed to be a
simple enough trade off.
I have a similar issue when I implement LOG SENSE/SELECT set of commands
as well.
Cheers
Mark
|
|