diff mbox series

[2/7] sd: implement ->get_unique_id

Message ID 20211012120445.861860-3-hch@lst.de (mailing list archive)
State New, archived
Headers show
Series [1/7] block: add a ->get_unique_id method | expand

Commit Message

Christoph Hellwig Oct. 12, 2021, 12:04 p.m. UTC
Add the method to query for a uniqueue ID of a given type by looking
it up in the cached device identification VPD page.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/sd.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

Comments

Keith Busch Oct. 12, 2021, 1:13 p.m. UTC | #1
On Tue, Oct 12, 2021 at 02:04:40PM +0200, Christoph Hellwig wrote:
> +static int sd_get_unique_id(struct gendisk *disk, u8 id[16], u8 type)
> +{
> +	struct scsi_device *sdev = scsi_disk(disk)->device;
> +	const struct scsi_vpd *vpd;
> +	const unsigned char *d;
> +	int len = -ENXIO;
> +
> +	rcu_read_lock();
> +	vpd = rcu_dereference(sdev->vpd_pg83);
> +	if (!vpd)
> +		goto out_unlock;
> +
> +	len = -EINVAL;
> +	for (d = vpd->data + 4; d < vpd->data + vpd->len; d += d[3] + 4) {
> +		/* we only care about designators with LU association */
> +		if (((d[1] >> 4) & 0x3) != 0x00)
> +			continue;
> +		if ((d[1] & 0xf) != type)
> +			continue;
> +
> +		/*
> +		 * Only exit early if a 16-byte descriptor was found.  Otherwise
> +		 * keep looking as one with more entropy might still show up.
> +		 */
> +		len = d[3];
> +		if (len != 8 && len != 12 && len != 16)
> +			continue;

I think you need a temporary variable instead of assigning directly to
'len' here. Otherwise, the 'len' returned will be whatever the last
iteration was, which may not be then len that was copied into the 'id'.

> +		memcpy(id, d + 4, len);
> +		if (len == 16)
> +			break;
> +	}
> +out_unlock:
> +	rcu_read_unlock();
> +	return len;
> +}
Hannes Reinecke Oct. 14, 2021, 7:30 a.m. UTC | #2
On 10/12/21 2:04 PM, Christoph Hellwig wrote:
> Add the method to query for a uniqueue ID of a given type by looking
> it up in the cached device identification VPD page.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/scsi/sd.c | 37 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 37 insertions(+)
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index d8f6add416c0a..ea1489d3e8497 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -1757,6 +1757,42 @@ static void sd_rescan(struct device *dev)
>   	sd_revalidate_disk(sdkp->disk);
>   }
>   
> +static int sd_get_unique_id(struct gendisk *disk, u8 id[16], u8 type)
> +{
> +	struct scsi_device *sdev = scsi_disk(disk)->device;
> +	const struct scsi_vpd *vpd;
> +	const unsigned char *d;
> +	int len = -ENXIO;
> +
> +	rcu_read_lock();
> +	vpd = rcu_dereference(sdev->vpd_pg83);
> +	if (!vpd)
> +		goto out_unlock;
> +
> +	len = -EINVAL;
> +	for (d = vpd->data + 4; d < vpd->data + vpd->len; d += d[3] + 4) {
> +		/* we only care about designators with LU association */
> +		if (((d[1] >> 4) & 0x3) != 0x00)
> +			continue;
> +		if ((d[1] & 0xf) != type)
> +			continue;
> +
> +		/*
> +		 * Only exit early if a 16-byte descriptor was found.  Otherwise
> +		 * keep looking as one with more entropy might still show up.
> +		 */
> +		len = d[3];
> +		if (len != 8 && len != 12 && len != 16)
> +			continue;
> +		memcpy(id, d + 4, len);
> +		if (len == 16)
> +			break;
> +	}
> +out_unlock:
> +	rcu_read_unlock();
> +	return len;
> +}
> +
>   static char sd_pr_type(enum pr_type type)
>   {
>   	switch (type) {
> @@ -1861,6 +1897,7 @@ static const struct block_device_operations sd_fops = {
>   	.check_events		= sd_check_events,
>   	.unlock_native_capacity	= sd_unlock_native_capacity,
>   	.report_zones		= sd_zbc_report_zones,
> +	.get_unique_id		= sd_get_unique_id,
>   	.pr_ops			= &sd_pr_ops,
>   };
>   
> 
Errm.

What's wrong with scsi_vpd_lun_id() ?

Cheers,

Hannes
Christoph Hellwig Oct. 14, 2021, 8:01 a.m. UTC | #3
On Thu, Oct 14, 2021 at 09:30:51AM +0200, Hannes Reinecke wrote:
> What's wrong with scsi_vpd_lun_id() ?

It doesn't allow the caller to pick a specific ID type.
Christoph Hellwig Oct. 14, 2021, 8:03 a.m. UTC | #4
On Thu, Oct 14, 2021 at 10:01:34AM +0200, Christoph Hellwig wrote:
> On Thu, Oct 14, 2021 at 09:30:51AM +0200, Hannes Reinecke wrote:
> > What's wrong with scsi_vpd_lun_id() ?
> 
> It doesn't allow the caller to pick a specific ID type.

... and of course that it returns an ASCSII string instead of the binary
ID.
diff mbox series

Patch

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index d8f6add416c0a..ea1489d3e8497 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1757,6 +1757,42 @@  static void sd_rescan(struct device *dev)
 	sd_revalidate_disk(sdkp->disk);
 }
 
+static int sd_get_unique_id(struct gendisk *disk, u8 id[16], u8 type)
+{
+	struct scsi_device *sdev = scsi_disk(disk)->device;
+	const struct scsi_vpd *vpd;
+	const unsigned char *d;
+	int len = -ENXIO;
+
+	rcu_read_lock();
+	vpd = rcu_dereference(sdev->vpd_pg83);
+	if (!vpd)
+		goto out_unlock;
+
+	len = -EINVAL;
+	for (d = vpd->data + 4; d < vpd->data + vpd->len; d += d[3] + 4) {
+		/* we only care about designators with LU association */
+		if (((d[1] >> 4) & 0x3) != 0x00)
+			continue;
+		if ((d[1] & 0xf) != type)
+			continue;
+
+		/*
+		 * Only exit early if a 16-byte descriptor was found.  Otherwise
+		 * keep looking as one with more entropy might still show up.
+		 */
+		len = d[3];
+		if (len != 8 && len != 12 && len != 16)
+			continue;
+		memcpy(id, d + 4, len);
+		if (len == 16)
+			break;
+	}
+out_unlock:
+	rcu_read_unlock();
+	return len;
+}
+
 static char sd_pr_type(enum pr_type type)
 {
 	switch (type) {
@@ -1861,6 +1897,7 @@  static const struct block_device_operations sd_fops = {
 	.check_events		= sd_check_events,
 	.unlock_native_capacity	= sd_unlock_native_capacity,
 	.report_zones		= sd_zbc_report_zones,
+	.get_unique_id		= sd_get_unique_id,
 	.pr_ops			= &sd_pr_ops,
 };