[Stgt-devel] [Patch 1/1] Add support for VPD pages 80h - ffh
Mark Harvey
markh794
Tue Sep 18 13:02:45 CEST 2007
FUJITA Tomonori wrote:
> On Wed, 05 Sep 2007 17:54:14 +1000
> Mark Harvey <markh794 at gmail.com> wrote:
>
>
>> Re-submit of patch with suggested improvements.
>>
>> - Added pointer struct target to scsi_lu struct
>> - Added lu_exit() to free allocation of VPD pages
>> - Removed osd from test startup script
>> - Removed osd specific VPD pages B0h and B1h
>>
>> - Passes checkpatch.pl sanity check.
>>
>
> Merged, Thanks! Sorry about the delay again.
>
>
Many thanks.
> Here are some comments.
>
>
Thanks..
>
>> +/** Protocol Identifier Values
>> + * 0 Fibre Channel (FCP-2)
>> + * 1 Parallel SCSI (SPI-5)
>>
>
> We use the following style:
>
>
OK - Noted for next time.
I tried to fit this into the kernel-doc style..
>> +/*
>> + * Protocol Identifier Values
>> + *
>> + * 0 Fibre Channel (FCP-2)
>> + * 1 Parallel SCSI (SPI-5)
>>
>
>
>
>> +static int valid_vpd(struct vpd **lu_vpd, uint8_t page)
>> +{
>> + if (lu_vpd[page & 0x7f])
>> + return 1;
>>
>
> I think that we use 0x7f and 0x80 at too many places. I put new macros
> for them.
>
Much nicer - thanks.
And for the macros.
For some reason I always seem to steer clear of macros - and I don't
really know why. Especially when examples are presented to make life easier.
>
>
>> + return 0;
>> +}
>> +
>> +void update_vpd_80(struct scsi_lu *lu, void *sn)
>> +{
>> + struct vpd *vpd_pg = lu->attrs.lu_vpd[0];
>> + char *data = (char *)vpd_pg->data;
>> +
>> + snprintf(data, SCSI_SN_LEN, "%-8s", (char *)sn);
>>
>
> This doesn't work. We need right-aligned ASCII data.
>
Thats what I was trying to achieve with %-8s...
Thanks for the correction.
>
>
>> +}
>> +
>> +void update_vpd_83(struct scsi_lu *lu, void *id)
>> +{
>> + struct vpd *vpd_pg = lu->attrs.lu_vpd[3];
>> + uint8_t *data = vpd_pg->data;
>> +
>> + data[0] = (PIV_ISCSI << 4) | INQ_CODE_ASCII;
>> + data[1] = PIV_VALID | ASS_TGT_PORT | DESG_VENDOR;
>> + data[3] = SCSI_ID_LEN;
>> +
>> + strncpy((char *)data + 4, id, SCSI_ID_LEN);
>> +}
>> +
>> int spc_inquiry(int host_no, struct scsi_cmd *cmd)
>> {
>> int len = 0, ret = SAM_STAT_CHECK_CONDITION;
>> @@ -47,6 +138,7 @@ int spc_inquiry(int host_no, struct scsi_cmd *cmd)
>> uint16_t asc = ASC_INVALID_FIELD_IN_CDB;
>> uint8_t devtype = 0;
>> struct lu_phy_attr *attrs;
>> + struct vpd *vpd_pg;
>>
>> if (((scb[1] & 0x3) == 0x3) || (!(scb[1] & 0x3) && scb[2]))
>> goto sense;
>> @@ -98,43 +190,35 @@ int spc_inquiry(int host_no, struct scsi_cmd *cmd)
>> } else if (scb[1] & 0x1) {
>> /* EVPD bit set */
>> if (scb[2] == 0x0) {
>> + int i, j, tmp;
>> +
>> + i = 5;
>> + tmp = 1;
>> data[0] = devtype;
>> - data[1] = 0x0;
>> - data[3] = 3;
>> + data[1] = 0;
>> + data[2] = 0;
>> + for (j = 0; j < 0x80; j++) {
>> + if (attrs->lu_vpd[j]) {
>> + data[i] = j | 0x80;
>> + tmp++;
>> + i++;
>> + }
>> + }
>>
>
> I use a more common way here.
>
Again, thanks.
>
>
>> + data[3] = tmp;
>> data[4] = 0x0;
>> - data[5] = 0x80;
>> - data[6] = 0x83;
>> - len = 7;
>> + len = tmp + 4;
>> ret = SAM_STAT_GOOD;
>> - } else if (scb[2] == 0x80) {
>> - int tmp = SCSI_SN_LEN;
>> -
>> - data[1] = 0x80;
>> - data[3] = SCSI_SN_LEN;
>> - memset(data + 4, 0x20, 4);
>> - len = 4 + SCSI_SN_LEN;
>> - ret = SAM_STAT_GOOD;
>> -
>> - if (strlen(attrs->scsi_sn)) {
>> - uint8_t *p;
>> - char *q;
>> + } else if (valid_vpd(attrs->lu_vpd, scb[2])) {
>> + int tmp;
>> + vpd_pg = attrs->lu_vpd[scb[2] & 0x7f];
>> + tmp = vpd_pg->size;
>>
>> - p = data + 4 + tmp - 1;
>> - q = attrs->scsi_sn + SCSI_SN_LEN - 1;
>> - for (; tmp > 0; tmp--, q)
>> - *(p--) = *(q--);
>> - }
>> - } else if (scb[2] == 0x83) {
>> - int tmp = SCSI_ID_LEN;
>> -
>> - data[1] = 0x83;
>> - data[3] = tmp + 4;
>> - data[4] = 0x1;
>> - data[5] = 0x1;
>> - data[7] = tmp;
>> - strncpy((char *) data + 8, attrs->scsi_id, SCSI_ID_LEN);
>> -
>> - len = tmp + 8;
>> + data[0] = devtype;
>> + data[1] = scb[2];
>> + data[2] = (tmp >> 8);
>> + data[3] = tmp & 0xff;
>> + memcpy(&data[4], vpd_pg->data, tmp);
>> + len = tmp + 4;
>> ret = SAM_STAT_GOOD;
>> }
>> }
>> @@ -358,6 +442,19 @@ int spc_request_sense(int host_no, struct scsi_cmd *cmd)
>> return SAM_STAT_GOOD;
>> }
>>
>> +struct vpd *alloc_vpd(uint16_t size)
>> +{
>> + struct vpd *vpd_pg;
>> +
>> + vpd_pg = zalloc(sizeof(struct vpd) + size);
>> + if (!vpd_pg)
>> + return NULL;
>> +
>> + vpd_pg->size = size;
>> +
>> + return vpd_pg;
>> +}
>> +
>> static struct mode_pg *alloc_mode_pg(uint8_t pcode, uint8_t subpcode,
>> uint16_t size)
>> {
>> @@ -500,6 +597,11 @@ int lu_config(struct scsi_lu *lu, char *params, match_fn_t *fn)
>> int err = TGTADM_SUCCESS;
>> char *p;
>> char buf[256];
>> + struct lu_phy_attr *attrs;
>> + struct vpd **lu_vpd;
>> +
>> + attrs = &lu->attrs;
>> + lu_vpd = attrs->lu_vpd;
>>
>> if (!strncmp("targetOps", params, 9))
>> params = params + 10;
>> @@ -512,36 +614,38 @@ int lu_config(struct scsi_lu *lu, char *params, match_fn_t *fn)
>> token = match_token(p, tokens, args);
>> switch (token) {
>> case Opt_scsi_id:
>> - match_strncpy(lu->attrs.scsi_id, &args[0],
>> - sizeof(lu->attrs.scsi_id));
>> + match_strncpy(attrs->scsi_id, &args[0],
>> + sizeof(attrs->scsi_id));
>>
>
> Wrong tabs. Should be:
>
> + match_strncpy(attrs->scsi_id, &args[0],
> + sizeof(attrs->scsi_id));
>
>
>
>> + lu_vpd[3]->vpd_update(lu, attrs->scsi_id);
>> break;
>> case Opt_scsi_sn:
>> - match_strncpy(lu->attrs.scsi_sn, &args[0],
>> - sizeof(lu->attrs.scsi_sn));
>> + match_strncpy(attrs->scsi_sn, &args[0],
>> + sizeof(attrs->scsi_sn));
>> + lu_vpd[0]->vpd_update(lu, attrs->scsi_sn);
>> break;
>> case Opt_vendor_id:
>> - match_strncpy(lu->attrs.vendor_id, &args[0],
>> - sizeof(lu->attrs.vendor_id));
>> + match_strncpy(attrs->vendor_id, &args[0],
>> + sizeof(attrs->vendor_id));
>> break;
>> case Opt_product_id:
>> - match_strncpy(lu->attrs.product_id, &args[0],
>> - sizeof(lu->attrs.product_id));
>> + match_strncpy(attrs->product_id, &args[0],
>> + sizeof(attrs->product_id));
>> break;
>> case Opt_product_rev:
>> - match_strncpy(lu->attrs.product_rev, &args[0],
>> - sizeof(lu->attrs.product_rev));
>> + match_strncpy(attrs->product_rev, &args[0],
>> + sizeof(attrs->product_rev));
>> break;
>> case Opt_sense_format:
>> match_strncpy(buf, &args[0], sizeof(buf));
>> - lu->attrs.sense_format = atoi(buf);
>> + attrs->sense_format = atoi(buf);
>> break;
>> case Opt_removable:
>> match_strncpy(buf, &args[0], sizeof(buf));
>> - lu->attrs.removable = atoi(buf);
>> + attrs->removable = atoi(buf);
>> break;
>> case Opt_online:
>> match_strncpy(buf, &args[0], sizeof(buf));
>> - lu->attrs.online = atoi(buf);
>> + attrs->online = atoi(buf);
>> break;
>> case Opt_mode_page:
>> match_strncpy(buf, &args[0], sizeof(buf));
>> @@ -561,10 +665,34 @@ int spc_lu_config(struct scsi_lu *lu, char *params)
>>
>> int spc_lu_init(struct scsi_lu *lu)
>> {
>> + struct vpd **lu_vpd = lu->attrs.lu_vpd;
>> + struct target *tgt = lu->tgt;
>> + int pg;
>> +
>> + 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, tgt->tid, lu->lun);
>> + snprintf(lu->attrs.scsi_sn, sizeof(lu->attrs.scsi_sn),
>> + "beaf%d%" PRIu64, tgt->tid, lu->lun);
>> +
>> + /* VPD page 0x80 */
>> + pg = 0x80 & 0x7f;
>> + lu_vpd[pg] = alloc_vpd(SCSI_SN_LEN);
>> + lu_vpd[pg]->vpd_update = update_vpd_80;
>> + lu_vpd[pg]->vpd_update(lu, lu->attrs.scsi_sn);
>> +
>> + /* VPD page 0x83 */
>> + pg = 0x83 & 0x7f;
>> + lu_vpd[pg] = alloc_vpd(SCSI_ID_LEN + 4); /* +Designator descriptor len*/
>> + lu_vpd[pg]->vpd_update = update_vpd_83;
>> + lu_vpd[pg]->vpd_update(lu, lu->attrs.scsi_id);
>> +
>> lu->attrs.removable = 0;
>> lu->attrs.sense_format = 0;
>> lu->attrs.online = 0;
>> @@ -572,3 +700,15 @@ int spc_lu_init(struct scsi_lu *lu)
>>
>> return 0;
>> }
>> +
>> +void spc_lu_exit(struct scsi_lu *lu)
>> +{
>> + int i;
>> + struct vpd **lu_vpd = lu->attrs.lu_vpd;
>> +
>> + for (i = 0; i < 0x80; i++)
>> + if (lu_vpd[i])
>> + free(lu_vpd[i]);
>> +
>> +}
>> +
>> diff --git a/usr/spc.h b/usr/spc.h
>> index 1cc8623..dfb8987 100644
>> --- a/usr/spc.h
>> +++ b/usr/spc.h
>> @@ -12,8 +12,10 @@ extern int spc_lu_init(struct scsi_lu *lu);
>> typedef int (match_fn_t)(struct scsi_lu *lu, char *params);
>> extern int lu_config(struct scsi_lu *lu, char *params, match_fn_t *);
>> extern int spc_lu_config(struct scsi_lu *lu, char *params);
>> +extern void spc_lu_exit(struct scsi_lu *lu);
>> extern void dump_cdb(struct scsi_cmd *cmd);
>> extern int spc_mode_sense(int host_no, struct scsi_cmd *cmd);
>> extern int add_mode_page(struct scsi_lu *lu, char *params);
>> +extern struct vpd *alloc_vpd(uint16_t size);
>>
>> #endif
>> diff --git a/usr/target.c b/usr/target.c
>> index 2474d05..22eec0b 100644
>> --- a/usr/target.c
>> +++ b/usr/target.c
>> @@ -261,12 +261,14 @@ int tgt_device_create(int tid, int dev_type, uint64_t lun, char *args, int backi
>> goto free_lu;
>> }
>>
>> + lu->tgt = target;
>> +
>> lu->lun = lun;
>> lu->lu_state = SCSI_LU_RUNNING;
>> tgt_cmd_queue_init(&lu->cmd_queue);
>>
>> if (lu->dev_type_template.lu_init) {
>> - ret = lu->dev_type_template.lu_init(lu);
>> + ret = lu->dev_type_template.lu_init(lu);
>> if (ret)
>> goto free_lu;
>> }
>> @@ -285,13 +287,6 @@ int tgt_device_create(int tid, int dev_type, uint64_t lun, char *args, int backi
>> goto free_lu;
>> }
>>
>> - lu->attrs.device_type = lu->dev_type_template.type;
>> - lu->attrs.qualifier = 0x0;
>> - snprintf(lu->attrs.scsi_id, sizeof(lu->attrs.scsi_id),
>> - "deadbeaf%d:%" PRIu64, tid, lun);
>> - snprintf(lu->attrs.scsi_sn, sizeof(lu->attrs.scsi_sn),
>> - "beaf%d%" PRIu64, tid, lun);
>> -
>> if (tgt_drivers[target->lid]->lu_create)
>> tgt_drivers[target->lid]->lu_create(lu);
>>
>> diff --git a/usr/tgtd.h b/usr/tgtd.h
>> index c39b9c2..45bd8f6 100644
>> --- a/usr/tgtd.h
>> +++ b/usr/tgtd.h
>> @@ -36,6 +36,15 @@ struct tgt_cmd_queue {
>> struct list_head queue;
>> };
>>
>> +struct scsi_lu;
>> +struct scsi_cmd;
>> +
>> +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 *lu_vpd[0x80]; /* VPD pages 0x80 -> 0xff masked with 0x80*/
>> +};
>>
>> struct device_type_operations {
>> int (*cmd_perform)(int host_no, struct scsi_cmd *cmd);
>> @@ -109,6 +117,8 @@ struct scsi_lu {
>>
>> struct backingstore_template *bst;
>>
>> + struct target *tgt;
>> +
>> uint8_t mode_block_descriptor[BLOCK_DESCRIPTOR_LEN];
>> struct mode_pg *mode_pgs[0x3f];
>>
>> --
>> 1.5.2.3
>>
>>
>>
>>
>> _______________________________________________
>> Stgt-devel mailing list
>> Stgt-devel at lists.berlios.de
>> https://lists.berlios.de/mailman/listinfo/stgt-devel
>>
>
>
Regards
Mark
More information about the stgt
mailing list