On Thu, 2015-08-27 at 14:41 +0200, Hannes Reinecke wrote:
> Add a function scsi_vpd_lun_id() to return a unique device
> identifcation based on the designation descriptors of
> VPD page 0x83.
> 
> As devices might implement several descriptors the order
> of preference is:
> - NAA IEE Registered Extended
> - EUI-64 based 16-byte
> - EUI-64 based 12-byte
> - NAA IEEE Registered
> - NAA IEEE Extended
> A SCSI name string descriptor is preferred to all of them
> if the identification is longer than 16 bytes.
> 
> The returned unique device identification will be formatted
> as a SCSI Name string to avoid clashes between different
> designator types.
> 
> Signed-off-by: Hannes Reinecke <h...@suse.de>
> ---
>  drivers/scsi/scsi_lib.c    | 121 
> +++++++++++++++++++++++++++++++++++++++++++++
>  include/scsi/scsi_device.h |   1 +
>  2 files changed, 122 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 2a864b8..48d6ff6 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -3145,3 +3145,124 @@ void sdev_enable_disk_events(struct scsi_device *sdev)
>       atomic_dec(&sdev->disk_events_disable_depth);
>  }
>  EXPORT_SYMBOL(sdev_enable_disk_events);
> +
> +/*
> + * scsi_vpd_lun_id - return a unique device identification
> + * @sdev: SCSI device
> + * @id:   buffer for the identification
> + * @id_len:  length of the buffer
> + *
> + * Copies a unique device identification into @id based
> + * on the information in the VPD page 0x83 of the device.
> + * The string will be formatted as a SCSI name string.
> + *
> + * Returns the length of the identification or error on failure.
> + */
> +int scsi_vpd_lun_id(struct scsi_device *sdev, char *id, size_t id_len)
> +{
> +     u8 cur_id_type = 0xff;
> +     u8 cur_id_size = 0;
> +     unsigned char *d, *cur_id_str;
> +     int id_size = -EAGAIN;
> +
> +     /*
> +      * Look for the correct descriptor.
> +      * Order of preference for lun descriptor:
> +      * - SCSI name string
> +      * - NAA IEEE Registered Extended
> +      * - EUI-64 based 16-byte
> +      * - EUI-64 based 12-byte
> +      * - NAA IEEE Registered
> +      * - NAA IEEE Extended
> +      * as longer descriptors reduce the likelyhood
> +      * of identification clashes.
> +      */
> +
> +     /* The id string must be at least 20 bytes + terminating NULL byte */
> +     if (id_len < 21)
> +             return -EINVAL;
> +
> +     memset(id, 0, id_len);
> +     d = sdev->vpd_pg83 + 4;
> +     while (d < sdev->vpd_pg83 + sdev->vpd_pg83_len) {
> +             /* Skip designators not referring to the LUN */
> +             if ((d[1] & 0x30) != 0x00)
> +                     goto next_desig;
> +
> +             switch (d[1] & 0xf) {
> +             case 0x2:
> +                     /* EUI-64 */
> +                     if (cur_id_size > d[3])
> +                             break;
> +                     /* Prefer NAA IEEE Registered Extended */
> +                     if (cur_id_type == 0x3 &&
> +                         cur_id_size == d[3])
> +                             break;
> +                     cur_id_size = d[3];
> +                     cur_id_str = d + 4;
> +                     cur_id_type = d[1] & 0xf;
> +                     switch (cur_id_size) {
> +                     case 8:
> +                             id_size = snprintf(id, id_len,
> +                                                "eui.%8phN",
> +                                                cur_id_str);
> +                             break;
> +                     case 12:
> +                             id_size = snprintf(id, id_len,
> +                                                "eui.%12phN",
> +                                                cur_id_str);
> +                             break;
> +                     case 16:
> +                             id_size = snprintf(id, id_len,
> +                                                "eui.%16phN",
> +                                                cur_id_str);
> +                             break;
> +                     default:
> +                             cur_id_size = 0;
> +                             break;
> +                     }
> +                     break;
> +             case 0x3:
> +                     /* NAA */
> +                     if (cur_id_size > d[3])
> +                             break;
> +                     cur_id_size = d[3];
> +                     cur_id_str = d + 4;
> +                     cur_id_type = d[1] & 0xf;
> +                     switch (cur_id_size) {
> +                     case 8:
> +                             id_size = snprintf(id, id_len,
> +                                                "naa.%8phN",
> +                                                cur_id_str);
> +                             break;
> +                     case 16:
> +                             id_size = snprintf(id, id_len,
> +                                                "naa.%16phN",
> +                                                cur_id_str);
> +                             break;
> +                     default:
> +                             cur_id_size = 0;
> +                             break;
> +                     }
> +                     break;
> +             case 0x8:
> +                     /* SCSI name string */
> +                     if (cur_id_size + 4 > d[3])
> +                             break;
> +                     cur_id_size = d[3];
> +                     cur_id_str = d + 4;
> +                     cur_id_type = d[1] & 0xf;
> +                     if (cur_id_size >= id_len)
> +                             cur_id_size = id_len - 1;
> +                     memcpy(id, cur_id_str, cur_id_size);
> +                     break;
> +             default:
> +                     break;
> +             }
> +next_desig:
> +             d += d[3] + 4;
> +     }
> +
> +     return id_size;
> +}
> +EXPORT_SYMBOL(scsi_vpd_lun_id);
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index b6b1dc4..595a9be 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -413,6 +413,7 @@ static inline int scsi_execute_req(struct scsi_device 
> *sdev,
>  }
>  extern void sdev_disable_disk_events(struct scsi_device *sdev);
>  extern void sdev_enable_disk_events(struct scsi_device *sdev);
> +extern int scsi_vpd_lun_id(struct scsi_device *, char *, size_t);
>  
>  #ifdef CONFIG_PM
>  extern int scsi_autopm_get_device(struct scsi_device *);

scsi_vpd_lun_id() is an exported function, but does not check if sdev->vpd_pg83 
!= NULL
(i.e. no VPD 83 info), and we will crash if it is NULL.  Also, why return 
-EAGAIN?
What will be different next time?  Maybe a different errno would be better, like
-ENOENT or -ENODATA, I'm not sure.

In the SCSI name string case, if the name has to be truncated to fit the 
supplied buffer,
scsi_vpd_lun_id() does not ensure that the last byte is zero, it relies on the 
caller to
have done this, (which alua_check_vpd() does not do).  I'm not sure if we 
should truncate
the SCSI name string here or return an error here, as a truncated name will 
likely not
be unique.

Reviewed-by: Ewan D. Milne <emi...@redhat.com>




--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to