vdev_id: new slot type SES. #6956

tesujimath · 2017-12-13T20:56:03Z

Description

This extends vdev_id to support a new slot type, SES, for SCSI Enclosure Services.

With slot type SES, the disk slot numbers are determined by using the device slot number reported by sg_ses for the device with matching SAS address.

Motivation and Context

This is primarily of use on systems with a deficient driver omitting support for bay_identifier in /sys/devices. For example, HP's hpsa driver used with their H241 cards doesn't support bay_identifier, instead returning invalid argument. The reason for this can be found in the source code for that driver, which has the following

hpsa_sas_get_bay_identifier(struct sas_rphy *rphy)
{
	return -ENXIO;
}

Oh dear.

In my testing, I found that the existing slot types of port and id were not stable across disk replacement, so an alternative was required.

How Has This Been Tested?

This has been tested on all our ZFS fileservers, which now produce a satisfactory bay identifier when udev is triggered to run the vdev_id script. These servers include:

HP DL385 Gen 7 using LSI 9201-16e HBA card and HP D6000/MDS600 external storage
HP DL380 Gen 8 using HP H221 HBA card and HP D2700/D6000 external storage
HP DL380 Gen 9 using HP H241 HBA card and HP D3700 external storage.

The combination of DL380 Gen 9, H241, and older D2700 external storage does not seem to support SCSI Enclosure Services.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Performance enhancement (non-breaking change which improves efficiency)
Code cleanup (non-breaking change which makes code smaller or more readable)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation (a change to man pages or other documentation)

Checklist:

My code follows the ZFS on Linux code style requirements.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.
All commit messages are properly formatted and contain Signed-off-by.
Change has been approved by a ZFS on Linux member.

behlendorf · 2017-12-15T18:34:13Z

cmd/vdev_id/vdev_id

+		# [5:0:61:0]  enclosu HP       D6000            2.38  -          /dev/sg141
+		# [6:0:25:0]  enclosu HP       D2700 SAS AJ941A 0147  -          /dev/sg167
+		# [6:0:51:0]  enclosu HP       D2700 SAS AJ941A 0147  -          /dev/sg193
+		enclosures=`lsscsi -g | sed -n -e '/enclosu/s/^.* \([^ ]*\)$/\1/p'`


The sed line here doesn't appear to correctly handle the following abbreviated output from lsscsi -g. Specifically it does not print the first device.

[0:0:0:0] enclosu RAIDINC 84BAY EBOD 0204 - /dev/sg0 [0:0:81:0] enclosu RAIDINC 84BAY EBOD 0204 - /dev/sg81 [11:0:1:0] enclosu RAIDINC 84BAY EBOD 0204 - /dev/sg163 [11:0:81:0] enclosu RAIDINC 84BAY EBOD 0204 - /dev/sg243

lsscsi -g | sed -n -e '/enclosu/s/^.* $[^ ]*$$/\1/p' /dev/sg81 /dev/sg163 /dev/sg243

Fixed to cope with the pesky trailing space.

behlendorf · 2017-12-15T18:35:35Z

cmd/vdev_id/vdev_id

@@ -276,6 +276,26 @@ sas_handler() {
 		d=$(eval echo \${$i})
 		SLOT=`echo $d | sed -e 's/^.*://'`
 		;;
+	"SES")


[nit] for consistency with "lun" let's go with a lower case "ses".

Initially I did write ses in lower case, but I decided it was too easily confused with sas, and that SES was much clearer. That is, clarity trumped consistency. Do you agree?

(I'll address the other points on Monday.)

I can see that, but in this case I don't think it will result in too much confusion since sas is not a valid value for slot.

OK, now lowercased.

behlendorf · 2017-12-15T18:39:11Z

man/man5/vdev_id.conf.5

@@ -103,6 +103,9 @@ taken.  The default is bay.
 \fIid\fR - use the scsi id as the slot number.

 \fIlun\fR - use the scsi lun as the slot number.
+
+\fISES\fR - use the SCSI Enclosure Services (SES) enclosure device slot number, as reported by


Can you expand on this slightly as you did in the commit comment to explain that this is not the preferred setting but is being provided as an option for drivers which don't provide any other stable identifier.

Please also mind the 80 character line limit here and in cmd/vdev_id/vdev_id.

Done, and now complies with the 80 character line limit, notwithstanding the tension between this and the 8 character (tab) indent level ;-)

This extends vdev_id to support a new slot type, ses, for SCSI Enclosure Services. With slot type ses, the disk slot numbers are determined by using the device slot number reported by sg_ses for the device with matching SAS address, found by querying all available enclosures. This is primarily of use on systems with a deficient driver omitting support for bay_identifier in /sys/devices. In my testing, I found that the existing slot types of port and id were not stable across disk replacement, so an alternative was required. Signed-off-by: Simon Guest <simon.guest@tesujimath.org>

behlendorf

Looks good and works as expected. Thanks!

codecov · 2017-12-20T03:27:20Z

Codecov Report

Merging #6956 into master will increase coverage by 0.12%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #6956      +/-   ##
==========================================
+ Coverage   75.25%   75.38%   +0.12%     
==========================================
  Files         296      296              
  Lines       95453    95453              
==========================================
+ Hits        71837    71958     +121     
+ Misses      23616    23495     -121

Flag	Coverage Δ
#kernel	`74.85% <ø> (+0.23%)`	⬆️
#user	`67.79% <ø> (+0.34%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2d936e...22bf1ad. Read the comment docs.

This extends vdev_id to support a new slot type, ses, for SCSI Enclosure Services. With slot type ses, the disk slot numbers are determined by using the device slot number reported by sg_ses for the device with matching SAS address, found by querying all available enclosures. This is primarily of use on systems with a deficient driver omitting support for bay_identifier in /sys/devices. In my testing, I found that the existing slot types of port and id were not stable across disk replacement, so an alternative was required. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Simon Guest <simon.guest@tesujimath.org> Closes openzfs#6956

This extends vdev_id to support a new slot type, ses, for SCSI Enclosure Services. With slot type ses, the disk slot numbers are determined by using the device slot number reported by sg_ses for the device with matching SAS address, found by querying all available enclosures. This is primarily of use on systems with a deficient driver omitting support for bay_identifier in /sys/devices. In my testing, I found that the existing slot types of port and id were not stable across disk replacement, so an alternative was required. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Simon Guest <simon.guest@tesujimath.org> Closes #6956

tesujimath force-pushed the vdev_id-ses-slot branch from 6527da3 to d650e40 Compare December 14, 2017 00:50

behlendorf requested changes Dec 15, 2017

View reviewed changes

tesujimath force-pushed the vdev_id-ses-slot branch from d650e40 to 22bf1ad Compare December 17, 2017 23:02

behlendorf approved these changes Dec 19, 2017

View reviewed changes

behlendorf merged commit 993669a into openzfs:master Dec 20, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

vdev_id: new slot type SES. #6956

vdev_id: new slot type SES. #6956

tesujimath commented Dec 13, 2017

behlendorf Dec 15, 2017

tesujimath Dec 17, 2017

behlendorf Dec 15, 2017

tesujimath Dec 15, 2017

behlendorf Dec 15, 2017

tesujimath Dec 17, 2017

behlendorf Dec 15, 2017

tesujimath Dec 17, 2017

behlendorf left a comment

codecov bot commented Dec 20, 2017

vdev_id: new slot type SES. #6956

vdev_id: new slot type SES. #6956

Conversation

tesujimath commented Dec 13, 2017

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

behlendorf left a comment

Choose a reason for hiding this comment

codecov bot commented Dec 20, 2017

Codecov Report