* [PATCH 2/2] libata-scsi: do not return t10 designator if drive has WWN
[not found] <20160706221213.1928-1-me>
@ 2016-07-06 22:12 ` tom.ty89
2016-07-07 10:51 ` Sergei Shtylyov
2016-07-07 10:56 ` Hannes Reinecke
0 siblings, 2 replies; 6+ messages in thread
From: tom.ty89 @ 2016-07-06 22:12 UTC (permalink / raw)
To: tj; +Cc: linux-ide, linux-scsi, Tom Yan
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;
}
--
2.9.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] libata-scsi: do not return t10 designator if drive has WWN
2016-07-06 22:12 ` [PATCH 2/2] libata-scsi: do not return t10 designator if drive has WWN tom.ty89
@ 2016-07-07 10:51 ` Sergei Shtylyov
2016-07-07 10:56 ` Hannes Reinecke
1 sibling, 0 replies; 6+ messages in thread
From: Sergei Shtylyov @ 2016-07-07 10:51 UTC (permalink / raw)
To: tom.ty89, tj; +Cc: linux-ide, linux-scsi
Hello.
On 7/7/2016 1: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
[...]
> @@ -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 {
CodingStyle, should be:
} else {
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] libata-scsi: do not return t10 designator if drive has WWN
2016-07-06 22:12 ` [PATCH 2/2] libata-scsi: do not return t10 designator if drive has WWN tom.ty89
2016-07-07 10:51 ` Sergei Shtylyov
@ 2016-07-07 10:56 ` Hannes Reinecke
2016-07-07 12:40 ` Tom Yan
1 sibling, 1 reply; 6+ messages in thread
From: Hannes Reinecke @ 2016-07-07 10:56 UTC (permalink / raw)
To: tom.ty89, tj; +Cc: linux-ide, linux-scsi
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)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] libata-scsi: do not return t10 designator if drive has WWN
2016-07-07 10:56 ` Hannes Reinecke
@ 2016-07-07 12:40 ` Tom Yan
2016-07-07 12:44 ` Hannes Reinecke
0 siblings, 1 reply; 6+ messages in thread
From: Tom Yan @ 2016-07-07 12:40 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: Tejun Heo, linux-ide, linux-scsi
Well, udev uses its own `ata_id` (which issues IDENTIFY DEVICE through
ATA PASS-THROUGH) though.
Anyway I expected the reasoning you gave and I can't really argue with
you. It's just personally I still prefer a cleaner SATL implementation
(considering Linux is open source and can be deemed as some sort of
reference), so I gave it a go.
Not that SAT requires the DI VPD return only one desingator / LU name though.
On 7 July 2016 at 18:56, Hannes Reinecke <hare@suse.de> wrote:
> 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)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] libata-scsi: do not return t10 designator if drive has WWN
2016-07-07 12:40 ` Tom Yan
@ 2016-07-07 12:44 ` Hannes Reinecke
2016-07-07 13:18 ` Tom Yan
0 siblings, 1 reply; 6+ messages in thread
From: Hannes Reinecke @ 2016-07-07 12:44 UTC (permalink / raw)
To: Tom Yan; +Cc: Tejun Heo, linux-ide, linux-scsi
On 07/07/2016 02:40 PM, Tom Yan wrote:
> Well, udev uses its own `ata_id` (which issues IDENTIFY DEVICE through
> ATA PASS-THROUGH) though.
>
> Anyway I expected the reasoning you gave and I can't really argue with
> you. It's just personally I still prefer a cleaner SATL implementation
> (considering Linux is open source and can be deemed as some sort of
> reference), so I gave it a go.
>
> Not that SAT requires the DI VPD return only one desingator / LU name though.
>
Really?
sat-r08 has:
One identification descriptor for a logical unit (i.e., a logical unit
name) shall be included (see clause 10.3.4.2).
In some environments, one or more additional identification descriptors
may be included (see clause 10.3.4.3).
Am I misreading something?
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)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] libata-scsi: do not return t10 designator if drive has WWN
2016-07-07 12:44 ` Hannes Reinecke
@ 2016-07-07 13:18 ` Tom Yan
0 siblings, 0 replies; 6+ messages in thread
From: Tom Yan @ 2016-07-07 13:18 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: Tejun Heo, linux-ide, linux-scsi
Yeah it said "One identification descriptor..." but not "Only one
identification descriptor...". So I suppose it is *alright* to also
return t10 designator in addition to the WWN. It's just redundunt and
unnecessary (and that's why I sent the patch), since we only need one
LU name (either derive from the WWN, or the t10 identification when
WWN is not available):
If the ATA IDENTIFY DEVICE data word 87 bit 8 is set to one indicating
that the ATA device supports the
WORLD WIDE NAME field (i.e., ATA IDENTIFY DEVICE data words 108 to
111), then the SATL shall include a
designation descriptor containing a logical unit name as defined in 10.5.4.2.2.
If the ATA IDENTIFY DEVICE data word 87 bit 8 is set to zero,
indicating that the ATA device does not support
the WORLD WIDE NAME field (i.e., ATA IDENTIFY DEVICE data words 108 to
111), then the SATL shall include
an identification descriptor containing a logical unit name as defined
in 10.5.4.2.3.
(sat4r05f.pdf)
On 7 July 2016 at 20:44, Hannes Reinecke <hare@suse.de> wrote:
> On 07/07/2016 02:40 PM, Tom Yan wrote:
>> Well, udev uses its own `ata_id` (which issues IDENTIFY DEVICE through
>> ATA PASS-THROUGH) though.
>>
>> Anyway I expected the reasoning you gave and I can't really argue with
>> you. It's just personally I still prefer a cleaner SATL implementation
>> (considering Linux is open source and can be deemed as some sort of
>> reference), so I gave it a go.
>>
>> Not that SAT requires the DI VPD return only one desingator / LU name though.
>>
> Really?
>
> sat-r08 has:
>
> One identification descriptor for a logical unit (i.e., a logical unit
> name) shall be included (see clause 10.3.4.2).
> In some environments, one or more additional identification descriptors
> may be included (see clause 10.3.4.3).
>
> Am I misreading something?
>
> 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)
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-07-07 13:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20160706221213.1928-1-me>
2016-07-06 22:12 ` [PATCH 2/2] libata-scsi: do not return t10 designator if drive has WWN tom.ty89
2016-07-07 10:51 ` Sergei Shtylyov
2016-07-07 10:56 ` Hannes Reinecke
2016-07-07 12:40 ` Tom Yan
2016-07-07 12:44 ` Hannes Reinecke
2016-07-07 13:18 ` Tom Yan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox