On 02/06/2014 08:14 PM, Douglas Gilbert wrote:
> On 14-02-06 08:04 AM, Hannes Reinecke wrote:
>> EVPD page 0x83 is used to uniquely identify the device.
>> So instead of having each and every program issue a separate
>> SG_IO call to retrieve this information it does make far more
>> sense to display it in sysfs.
>>
>> Cc: Jeremy Linton <jlinton@xxxxxxxxxxxxx>
>> Cc: Doug Gilbert <dgilbert@xxxxxxxxxxxx>
>
> See below.
>
>> Cc: Kai Makisara <kai.makisara@xxxxxxxxxxx>
>> Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
>> ---
>> drivers/scsi/scsi_scan.c | 3 +
>> drivers/scsi/scsi_sysfs.c | 166
>> ++++++++++++++++++++++++++++++++++++++++++++-
>> include/scsi/scsi_device.h | 3 +
>> 3 files changed, 171 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
>> index 307a811..073fb84 100644
>> --- a/drivers/scsi/scsi_scan.c
>> +++ b/drivers/scsi/scsi_scan.c
>> @@ -929,6 +929,9 @@ static int scsi_add_lun(struct scsi_device
>> *sdev, unsigned char *inq_result,
>> if (*bflags & BLIST_SKIP_VPD_PAGES)
>> sdev->skip_vpd_pages = 1;
>>
>> + if (sdev->scsi_level >= SCSI_3)
>> + scsi_attach_vpd_ident(sdev);
>> +
>> transport_configure_device(&sdev->sdev_gendev);
>>
>> if (sdev->host->hostt->slave_configure) {
>> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
>> index 9117d0b..ffe7cb5 100644
>> --- a/drivers/scsi/scsi_sysfs.c
>> +++ b/drivers/scsi/scsi_sysfs.c
>> @@ -725,8 +725,170 @@ show_queue_type_field(struct device *dev,
>> struct device_attribute *attr,
>>
>> static DEVICE_ATTR(queue_type, S_IRUGO, show_queue_type_field,
>> NULL);
>>
>> +void scsi_attach_vpd_ident(struct scsi_device *sdev)
>> +{
>> + int ret;
>> + int vpd_len = 255;
>> + unsigned char *buffer;
>> +retry:
>> + buffer = kmalloc(vpd_len, GFP_KERNEL);
>> + if (buffer) {
>> + ret = scsi_get_vpd_page(sdev, 0x83, buffer, vpd_len);
>> + if (ret == 0) {
>> + vpd_len = (buffer[2] << 8) + buffer[3];
>> + if (vpd_len > 255) {
>> + kfree(buffer);
>> + goto retry;
>> + }
>> + sdev->vpd_ident_len = vpd_len;
>> + sdev->vpd_ident = buffer;
>> + }
>> + }
>> +}
>> +
>> +#define SCSI_VPD_ASSOC_lun 0x0
>> +#define SCSI_VPD_ASSOC_port 0x10
>> +#define SCSI_VPD_ASSOC_target 0x20
>> +
>> +#define SCSI_VPD_DESIG_vendor 0x0
>> +#define SCSI_VPD_DESIG_t10 0x1
>> +#define SCSI_VPD_DESIG_eui 0x2
>> +#define SCSI_VPD_DESIG_naa 0x3
>> +#define SCSI_VPD_DESIG_relport 0x4
>> +#define SCSI_VPD_DESIG_tpgrp 0x5
>> +#define SCSI_VPD_DESIG_lugrp 0x6
>> +#define SCSI_VPD_DESIG_md5 0x7
>> +#define SCSI_VPD_DESIG_scsi_name 0x8
>> +
>> +int scsi_parse_vpd_ident(struct scsi_device *sdev,
>> + int assoc, int desig, char *buf)
>> +{
>> + unsigned char *d;
>> + int len = 0;
>> +
>> + if (!sdev->vpd_ident)
>> + return -EINVAL;
>> +
>> + d = sdev->vpd_ident + 4;
>> + while (d < sdev->vpd_ident + sdev->vpd_ident_len) {
>> + if ((d[1] & 0x30) == assoc &&
>
> Wouldn't it be clearer if:
> if (((d[1] & 0x30) >> 4) == assoc &&
>
> and the lun, port and target defines were changed to agree
> with T10?
>
Yes, I could. Just thought it a bit pointless given
that we're (currently) the only ones using it.
Will be fixing it up with the next version.
>> + (d[1] & 0xf) == desig) {
>> + switch (d[1] & 0xf) {
>> + case SCSI_VPD_DESIG_eui:
>> + case SCSI_VPD_DESIG_naa:
>> + case SCSI_VPD_DESIG_md5:
>> + switch (d[3]) {
>> + case 8:
>> + len += sprintf(buf + len,
>> + "%8phN\n", d + 4);
>> + break;
>> + case 12:
>> + len += sprintf(buf + len,
>> + "%12phN\n", d + 4);
>> + break;
>> + case 16:
>> + len += sprintf(buf + len,
>> + "%16phN\n", d + 4);
>> + break;
>> + }
>> + break;
>> + case SCSI_VPD_DESIG_relport:
>> + case SCSI_VPD_DESIG_tpgrp:
>> + case SCSI_VPD_DESIG_lugrp:
>> + len += sprintf(buf + len, "%d\n",
>> + (int)(d[6] << 8) + d[7]);
>> + break;
>> + case SCSI_VPD_DESIG_vendor:
>> + case SCSI_VPD_DESIG_t10:
>> + case SCSI_VPD_DESIG_scsi_name:
>> + len += snprintf(buf + len, d[3] + 2, "%s\n",
>> + d + 4);
>> + break;
>> + }
>> + }
>> + d += d[3] + 4;
>> + }
>> +
>> + return len ? len : -EINVAL;
>> +}
>> +
>> +#define sdev_evpd_ident(assoc,desig) \
>> +static ssize_t \
>> +sdev_show_evpd_ident_##assoc##_##desig (struct device *dev, \
>> + struct device_attribute *attr, \
>> + char * buf) \
>> +{ \
>> + struct scsi_device *sdev = to_scsi_device(dev); \
>> + return scsi_parse_vpd_ident(sdev, SCSI_VPD_ASSOC_##assoc, \
>> + SCSI_VPD_DESIG_##desig, buf); \
>> +}
>> +
>> +#define sdev_evpd_attr(assoc, desig) \
>> + sdev_evpd_ident(assoc, desig) \
>> +static DEVICE_ATTR(ident_##assoc##_##desig, S_IRUGO, \
>> + sdev_show_evpd_ident_##assoc##_##desig, NULL);
>> +
>
> You macros will have trouble picking up UAS and SOP port info
> stuff hiding behind the designator code 9h (Protocol specific
> port identifier). That may not matter.
>
As I don't have UAS and no SOP devices are on the market
it's not _that_ urgent.
But yeah, I'll be fixing it up with the next version.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@xxxxxxx +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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
