Re: [PATCH 2/2] libata-scsi: do not return t10 designator if drive has WWN

[Hannes Reinecke <hare@suse.de>]

On 07/07/2016 12:12 AM, tom.ty89@gmail.com wrote:
> From: Tom Yan <tom.ty89@gmail.com>
> 
> SAT (as of sat4r05f.pdf) only requires the t10 designator if the
> drive does not support/have WWN. Besides, we already have the ATA
> information VPD.
> 
> Signed-off-by: Tom Yan <tom.ty89@gmail.com>
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 9f478ad..84b3d42 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -2210,21 +2210,6 @@ static unsigned int ata_scsiop_inq_83(struct ata_scsi_args *args, u8 *rbuf)
>  	rbuf[1] = 0x83;			/* this page code */
>  	num = 4;
>  
> -	/* SAT defined lu model and serial numbers descriptor */
> -	/* piv=0, assoc=lu, code_set=ACSII, designator=t10 vendor id */
> -	rbuf[num + 0] = 2;
> -	rbuf[num + 1] = 1;
> -	rbuf[num + 3] = sat_model_serial_desc_len;
> -	num += 4;
> -	memcpy(rbuf + num, "ATA     ", 8);
> -	num += 8;
> -	ata_id_string(args->id, (unsigned char *) rbuf + num, ATA_ID_PROD,
> -		      ATA_ID_PROD_LEN);
> -	num += ATA_ID_PROD_LEN;
> -	ata_id_string(args->id, (unsigned char *) rbuf + num, ATA_ID_SERNO,
> -		      ATA_ID_SERNO_LEN);
> -	num += ATA_ID_SERNO_LEN;
> -
>  	if (ata_id_has_wwn(args->id)) {
>  		/* SAT defined lu world wide name */
>  		/* piv=0, assoc=lu, code_set=binary, designator=NAA */
> @@ -2236,6 +2221,23 @@ static unsigned int ata_scsiop_inq_83(struct ata_scsi_args *args, u8 *rbuf)
>  			      ATA_ID_WWN, ATA_ID_WWN_LEN);
>  		num += ATA_ID_WWN_LEN;
>  	}
> +	else {
> +		/* SAT defined lu model and serial numbers descriptor */
> +		/* piv=0, assoc=lu, code_set=ACSII, designator=t10 vendor id */
> +		rbuf[num + 0] = 2;
> +		rbuf[num + 1] = 1;
> +		rbuf[num + 3] = sat_model_serial_desc_len;
> +		num += 4;
> +		memcpy(rbuf + num, "ATA     ", 8);
> +		num += 8;
> +		ata_id_string(args->id, (unsigned char *) rbuf + num, ATA_ID_PROD,
> +			      ATA_ID_PROD_LEN);
> +		num += ATA_ID_PROD_LEN;
> +		ata_id_string(args->id, (unsigned char *) rbuf + num, ATA_ID_SERNO,
> +			      ATA_ID_SERNO_LEN);
> +		num += ATA_ID_SERNO_LEN;
> +	}
> +
>  	rbuf[3] = num - 4;    /* page len (assume less than 256 bytes) */
>  	return 0;
>  }
> 
Nope.
We cannot go about and remove VPD descriptors.
Existing systems might rely on the VPD attribute to be present (think of
udev persistent symlinks), and the system will refuse to boot.
NACK.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)