* [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