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.
> 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.
[..]
> -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?
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.
[..]
> +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
|
|