RE: [PATCH] Add VPD support to aacraid

RE: [PATCH] Add VPD support to aacraid

[Salyzyn, Mark]

Hannes, I am working the firmware and management software folks to find out how to mine for this information. In your patch, the controller serial number may be helpful, but not necessarily compliant?

Besides persistent device id, what else is gained? The controller ensures persistent device id for the arrays through the meta-data, only migrating if it happens to be a foreign (from another controller) array that collides with a native array at the same ID. Plug the constituent drives into totally different IDs in the SAS infrastructure, the ID remains the same as presented by the controller. Plug those drives into another controller, and if not taken by an existing array, the ID will again remain the same.

If this is all of your concerns, adding the baggage to the driver will be redundant.

Sincerely -- Mark Salyzyn

> Hannes Reinecke wrote:
> > Hi Mark,
> > 
> > I found to my surprise that the aacraid driver does not support VPD
> > pages at all. I'm somewhat used to modern SCSI HBAs export SCSI-2 disks,
> > but not supporting VPD pages at all is really a bit .. hmm .. SCSI-1-ish.
> > And it makes it really impossible to assign a persistent device ID to
> > those drives.
> > 
> [ .. ]
> > 
> > Anyway, I don't think that it should be too hard to add proper VPD page
> > support (ie page 0x83), but for that one would have some documentation
> > how to ask the controller for it. I'll leave that to you :-)
> > You can use the page 0x80 support as a template.
>
> And here is even the patch, based on scsi-misc.
> Might help on reviewing.
> 
> Cheers,
> 
> Hannes
> -- 
> Dr. Hannes Reinecke			hare@suse.de
> SuSE Linux Products GmbH		S390 & zSeries
> Maxfeldstraße 5				+49 911 74053 688
> 90409 Nürnberg				http://www.suse.de
-
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

Re: [PATCH] Add VPD support to aacraid

[Hannes Reinecke]

Salyzyn, Mark wrote:
> Hannes, I am working the firmware and management software folks to find out how
> to mine for this information. In your patch, the controller serial
number may be
> helpful, but not necessarily compliant?
>
Well, the information in page 0x80 is 'vendor specific'. So anything we
put in there is compliant :-)

> Besides persistent device id, what else is gained? The controller ensures persistent
> device id for the arrays through the meta-data, only migrating if it
happens to be a
> foreign (from another controller) array that collides with a native
array at the same
> ID. Plug the constituent drives into totally different IDs in the SAS
infrastructure,
> the ID remains the same as presented by the controller. Plug those
drives into another
> controller, and if not taken by an existing array, the ID will again
remain the same.
>
Yes, that's what I thought would happen. Main reason for adding page
0x80 support is to get it to work with the current persistent device ID
setup from udev. There we're using the scsi_id program which doesn't
return a serial number if neither page 0x83 nor page 0x80 are supported.
So just hacking them into the driver was the easiest way to get it fixed.

And there is actually a second reason for adding page 0x80 support:
If there were something like a build uuid for the array (either a proper
uuid or maybe even the build date) we would be able to detect an array
re-initialisation. Currently we won't be able to deal with this as the
array will have the same ID, so we would try to mount a drive with a
totally different content.

That was actually what I was aiming for with that patch, but after two
days of hacking I gave up and settled for the easy way out.
I know that at least the array build date is somewhere, as the
management software displays them :-). So it should be relatively easy
to use that as the serial number.

> If this is all of your concerns, adding the baggage to the driver
> will be redundant.
> 
Actually, not quite. In not supporting EVPD page any compliant software
must assume that there is not way of reliably distinguish between two
drives from the same vendor/model.
And, incidentally, it should be possible to hook in two controllers, and
creating two arrays with the same name, right?
So we _don't_ have a reliable way for doing so.
And this is where EVPD page 0x80 support comes in.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke			hare@suse.de
SuSE Linux Products GmbH		S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de
-
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

[PATCH] Add VPD support to aacraid

[Hannes Reinecke]

Hi Mark,

I found to my surprise that the aacraid driver does not support VPD
pages at all. I'm somewhat used to modern SCSI HBAs export SCSI-2 disks,
but not supporting VPD pages at all is really a bit .. hmm .. SCSI-1-ish.
And it makes it really impossible to assign a persistent device ID to
those drives.

And it really made my eyes hurt. Now that I have the latest, top-of-the
range SAS drives with a real HW RAID controller and the disks are
exported as

SCSI-2.

Just as I thought we'd been finally past those days. Sigh.

Anyway, I don't think that it should be too hard to add proper VPD page
support (ie page 0x83), but for that one would have some documentation
how to ask the controller for it. I'll leave that to you :-)
You can use the page 0x80 support as a template.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke			hare@suse.de
SuSE Linux Products GmbH		S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de
-
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

Re: [PATCH] Add VPD support to aacraid

[Hannes Reinecke]

[-- Attachment #1: Type: text/plain, Size: 867 bytes --]

Hannes Reinecke wrote:
> Hi Mark,
> 
> I found to my surprise that the aacraid driver does not support VPD
> pages at all. I'm somewhat used to modern SCSI HBAs export SCSI-2 disks,
> but not supporting VPD pages at all is really a bit .. hmm .. SCSI-1-ish.
> And it makes it really impossible to assign a persistent device ID to
> those drives.
> 
[ .. ]
> 
> Anyway, I don't think that it should be too hard to add proper VPD page
> support (ie page 0x83), but for that one would have some documentation
> how to ask the controller for it. I'll leave that to you :-)
> You can use the page 0x80 support as a template.
> 
And here is even the patch, based on scsi-misc.
Might help on reviewing.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke			hare@suse.de
SuSE Linux Products GmbH		S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de

[-- Attachment #2: aacraid-vpd-support --]
[-- Type: text/plain, Size: 2889 bytes --]

diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index 426cd6f..410a127 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -666,6 +666,27 @@ static void setinqstr(struct aac_dev *de
 	inqstrcpy ("V1.0", str->prl);
 }
 
+/* Function: setinqserial
+ *
+ * Arguments: [1] pointer to void [1] int
+ *
+ * Purpose: Sets SCSI Unit Serial number.
+ *          This is a fake. We should read a proper
+ *          serial number from the container. But
+ *          without docs it's quite hard to do it :-)
+ *          So this will have to do in the meantime.
+ */
+
+static int setinqserial(struct aac_dev *dev, void *data, int cid)
+{
+	char *ep;
+
+	ep = (char *)(data);
+
+	return snprintf(ep, sizeof(struct scsi_inq) - 4, "%08X%02X",
+			le32_to_cpu(dev->adapter_info.serial[0]), cid);
+}
+
 static void set_sense(u8 *sense_buf, u8 sense_key, u8 sense_code,
 		      u8 a_sense_code, u8 incorrect_length,
 		      u8 bit_pointer, u16 field_pointer,
@@ -1580,6 +1601,46 @@ int aac_scsi_cmd(struct scsi_cmnd * scsi
 		dprintk((KERN_DEBUG "INQUIRY command, ID: %d.\n", scmd_id(scsicmd)));
 		memset(&inq_data, 0, sizeof (struct inquiry_data));
 
+		if (scsicmd->cmnd[1] & 0x1 ) {
+			char *arr = (char *)&inq_data;
+
+			/* EVPD bit set */
+			if (scmd_id(scsicmd) == host->this_id) {
+				arr[0] = INQD_PDT_PROC;
+			} else {
+				arr[0] = INQD_PDT_DA;
+			}
+			if (scsicmd->cmnd[2] == 0) {
+				int n;
+				/* supported vital product data pages */
+				arr[1] = scsicmd->cmnd[2];
+				n = 4;
+				arr[n++] = 0x0;;
+				arr[n++] = 0x80;
+				arr[3] = n - 4;
+				aac_internal_transfer(scsicmd, &inq_data, 0, sizeof(inq_data));
+				scsicmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8 | SAM_STAT_GOOD;
+			} else if (scsicmd->cmnd[2] == 0x80) {
+				/* unit serial number page */
+				arr[1] = scsicmd->cmnd[2];
+				arr[3] = setinqserial(dev, &arr[4], scmd_id(scsicmd));
+				aac_internal_transfer(scsicmd, &inq_data, 0, sizeof(inq_data));
+				scsicmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8 | SAM_STAT_GOOD;
+			} else {
+				/* vpd page not implemented */
+				scsicmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8 | SAM_STAT_CHECK_CONDITION;
+				set_sense((u8 *) &dev->fsa_dev[cid].sense_data,
+					  ILLEGAL_REQUEST,
+					  SENCODE_INVALID_CDB_FIELD,
+					  ASENCODE_NO_SENSE, 0, 7, 2, 0);
+				memcpy(scsicmd->sense_buffer, &dev->fsa_dev[cid].sense_data,
+				       (sizeof(dev->fsa_dev[cid].sense_data) > sizeof(scsicmd->sense_buffer))
+				       ? sizeof(scsicmd->sense_buffer)
+				       : sizeof(dev->fsa_dev[cid].sense_data));
+			}
+			scsicmd->scsi_done(scsicmd);
+			return 0;
+		}
 		inq_data.inqd_ver = 2;	/* claim compliance to SCSI-2 */
 		inq_data.inqd_rdf = 2;	/* A response data format value of two indicates that the data shall be in the format specified in SCSI-2 */
 		inq_data.inqd_len = 31;

Re: [PATCH] Add VPD support to aacraid

[Randy Dunlap]

On Thu, 25 Jan 2007 10:47:35 +0100 Hannes Reinecke wrote:

+/* Function: setinqserial
+ *
+ * Arguments: [1] pointer to void [1] int

Hi--
What does that notation mean?  (I know, you copied it. :)

Parts of this source file use kernel-doc and parts don't.
Would be nice to see it converted to all kernel-doc...

---
~Randy