Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add enclosure_symlinks option to vdev_id #8194

Merged
merged 1 commit into from Dec 15, 2018
Merged

Add enclosure_symlinks option to vdev_id #8194

merged 1 commit into from Dec 15, 2018

Conversation

tonyhutter
Copy link
Contributor

@tonyhutter tonyhutter commented Dec 7, 2018 •
edited

Motivation and Context

This updates vdev_id to create /dev/by-enclosure symlinks to the enclosure /dev/sg* devices. For example:

/dev/by-enclosure/enc-L0 -> /dev/sg0'

Description

Add an enclosure_symlinks option to vdev_id.conf. This creates consistently named symlinks to the enclosure devices (/dev/sg*) based off the configuration in vdev_id.conf. The enclosure symlinks show
up in /dev/by-enclosure/<prefix>-<channel><num>. The symlinks make easy to identify a particular enclosure device, since the underlying /dev/sg* devices can be instantiated differently at each boot. The enclosure links are created in addition to the normal /dev/disk/by-vdev links.

enclosure_symlinks is only valid in sas_direct configurations.

How Has This Been Tested?

Tested configuration manually on a system with four enclosure devices and and verified enclosure_symlinks and enclosure_symlinks_prefix worked as expcted.

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:

@tonyhutter tonyhutter added the Type: Feature Feature request or new feature label Dec 7, 2018
@tonyhutter tonyhutter self-assigned this Dec 7, 2018
@codecov
Copy link

codecov bot commented Dec 8, 2018 •
edited

Codecov Report

Merging #8194 into master will decrease coverage by 0.09%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #8194     +/-   ##
=========================================
- Coverage   78.52%   78.42%   -0.1%     
=========================================
  Files         379      378      -1     
  Lines      114880   114759    -121     
=========================================
- Hits        90210    90001    -209     
- Misses      24670    24758     +88
Flag Coverage Δ
#kernel 78.7% <ø> (-0.22%) ⬇️
#user 67.41% <ø> (-0.06%) ⬇️

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 eff7d78...f5b0125. Read the comment docs.

behlendorf
behlendorf reviewed Dec 11, 2018
View reviewed changes
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't manually tested it yet on a real system but it looks good. @tesujimath if you can spare the time it'd be great if you could review this change too since you've extended this script yourself once.

cmd/vdev_id/vdev_id
# When udev sees a scsi_generic device, it calls this script with -e to
# create the enclosure device symlinks only. We also need
# "enclosure_symlinks yes" set in vdev_id.config to actually create the
# symlink.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: check whitespace for this whole block

man/man5/vdev_id.conf.5
/dev/by-enclosure/enc-U0
/dev/by-enclosure/enc-U1
.fi
.P
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the vdev_id.conf.sas_direct.example file so it includes enclosure_symlinks yes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this be limited to sas_direct? It seems as though it could apply to multipath as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gdevenyi yep it will work (I tested with multipath). multipath is independent of the HBA topology mapping (sas_direct/sas_switch).

@tesujimath
Copy link
Contributor

@behlendorf I should be able to have a look at this next week.

tesujimath
tesujimath approved these changes Dec 14, 2018
View reviewed changes
Copy link
Contributor

@tesujimath tesujimath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked this out on my servers. All is fine as far as I am concerned.

@behlendorf behlendorf added the Status: Accepted Ready to integrate (reviewed, tested) label Dec 14, 2018
@tonyhutter
Add enclosure_symlinks option to vdev_id
f5b0125 
Add an 'enclosure_symlinks' option to vdev_id.conf.  This creates
consistently named symlinks to the enclosure devices (/dev/sg*) based
off the configuration in vdev_id.conf.  The enclosure symlinks show
up in /dev/by-enclosure/<prefix>-<channel><num>.  The links make it
make it easy to run sg_ses on a particular enclosure device.  The
enclosure links are created in addition to the normal
/dev/disk/by-vdev links.

'enclosure_symlinks' is only valid in sas_direct configurations.

Signed-off-by: Tony Hutter <hutter2@llnl.gov>
@tonyhutter
Copy link
Contributor Author

Commits squashed

@behlendorf behlendorf merged commit c66401f into openzfs:master Dec 15, 2018
@tonyhutter tonyhutter added this to To do in 0.7.13 Jan 7, 2019
@tonyhutter tonyhutter moved this from To do to In progress in 0.7.13 Jan 28, 2019
tonyhutter added a commit to tonyhutter/zfs that referenced this pull request Jan 30, 2019
@tonyhutter
Add enclosure_symlinks option to vdev_id
93865fc 
Add an 'enclosure_symlinks' option to vdev_id.conf.  This creates
consistently named symlinks to the enclosure devices (/dev/sg*) based
off the configuration in vdev_id.conf.  The enclosure symlinks show
up in /dev/by-enclosure/<prefix>-<channel><num>.  The links make it
make it easy to run sg_ses on a particular enclosure device.  The
enclosure links are created in addition to the normal
/dev/disk/by-vdev links.

'enclosure_symlinks' is only valid in sas_direct configurations.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Simon Guest <simon.guest@tesujimath.org>
Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Closes openzfs#8194
tonyhutter added a commit that referenced this pull request Mar 4, 2019
@tonyhutter
Add enclosure_symlinks option to vdev_id
89019a8 
Add an 'enclosure_symlinks' option to vdev_id.conf.  This creates
consistently named symlinks to the enclosure devices (/dev/sg*) based
off the configuration in vdev_id.conf.  The enclosure symlinks show
up in /dev/by-enclosure/<prefix>-<channel><num>.  The links make it
make it easy to run sg_ses on a particular enclosure device.  The
enclosure links are created in addition to the normal
/dev/disk/by-vdev links.

'enclosure_symlinks' is only valid in sas_direct configurations.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Simon Guest <simon.guest@tesujimath.org>
Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Closes #8194
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gdevenyi gdevenyi gdevenyi left review comments

@behlendorf behlendorf behlendorf approved these changes

@tesujimath tesujimath tesujimath approved these changes

Assignees

@tonyhutter tonyhutter

Labels
Status: Accepted Ready to integrate (reviewed, tested) Type: Feature Feature request or new feature
Projects
No open projects
0.7.13
  
In progress
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@tonyhutter @tesujimath @behlendorf @gdevenyi