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.

Sign up for GitHub

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

Create symbolic links in /dev/disk/by-vdev for NVMe disk devices. #9730

Merged
merged 1 commit into from
Dec 18, 2019
Merged

Create symbolic links in /dev/disk/by-vdev for NVMe disk devices. #9730

merged 1 commit into from
Dec 18, 2019

Conversation

geppi
Copy link
Contributor

@geppi geppi commented Dec 16, 2019

The existing rules miss NVMe disk devices because of the trailing digits in the KERNEL device name, e.g. nvme0n1. The added rule does selectively match the device names of NVMe disk devices.
Partitions of nvme disk devices are already properly handled by the existing rule for ENV{DEVTYPE}=="partition".

Motivation and Context

Running "udevadm trigger" after setting up the /etc/zfs/vdev_id.conf file using device link aliases did create symbolic links in /dev/disk/by-vdev for SATA disks but not for NVMe disk devices.

Description

A udev rule is added to selectively match the device names of NVMe disk devices.

How Has This Been Tested?

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:

@PrivatePuffin
Copy link
Contributor

PrivatePuffin commented Dec 16, 2019 •
edited
Loading

@geppi
All commit messages are properly formatted and contain Signed-off-by.

Your commit does not seem to include your "signed-off by" tag, please add it.

Also:

I have read the contributing document.
I doubt it, as it includes quite clear guidelines about a max 72 commit message line-length ;)
Please fix that also.

No biggies, but those checkboxes are there for a reason ;)

@geppi geppi changed the title Create symbolic links in /dev/disk/by-vdev for NVMe disk devices. Create symbolic links in /dev/disk/by-vdev for NVMe disk devices. Dec 16, 2019
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Dec 16, 2019
@geppi
Copy link
Contributor Author

geppi commented Dec 16, 2019

I made the pull request via the github web-interface. This does not allow to amend the commit message. It looks like I have to do this localy on the command line and then force push the changed commit message to my github repo. I will look into this.

@behlendorf
Copy link
Contributor

@geppi functionally this works great, thanks for taking the time to open a PR. If you can simply force update this PR as mentioned above this should be good to go. Before pushing you can locally run make checkstyle to make sure everything is in order.

@codecov
Copy link

codecov bot commented Dec 17, 2019 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9730      +/-   ##
==========================================
+ Coverage      67%      79%     +12%     
==========================================
  Files         338      420      +82     
  Lines      106419   123654   +17235     
==========================================
+ Hits        71086    98003   +26917     
+ Misses      35333    25651    -9682
Flag Coverage Δ
#kernel 80% <ø> (?)
#user 66% <ø> (ø) ⬇️

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 ad97643...85f22d3. Read the comment docs.

PrivatePuffin
PrivatePuffin approved these changes Dec 17, 2019
View reviewed changes
Copy link
Contributor

@PrivatePuffin PrivatePuffin left a comment

Choose a reason for hiding this comment

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

To be clear: codewise it's great... no worries there :)

@geppi
Create symbolic links in /dev/disk/by-vdev for nvme disk devices.
85f22d3 
The existing rules miss nvme disk devices because of the trailing
digits in the KERNEL device name, e.g. nvme0n1. Partitions of nvme
disk devices are already properly handled by the existing rule for
ENV{DEVTYPE}=="partition".

Signed-off-by: Thomas Geppert <[email protected]>
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Dec 18, 2019
@behlendorf behlendorf merged commit fe56484 into openzfs:master Dec 18, 2019
@geppi geppi deleted the vdev.rules branch December 18, 2019 10:47
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Dec 26, 2019
@geppi @tonyhutter
Create symbolic links in /dev/disk/by-vdev for nvme disk devices
9038a39 
The existing rules miss nvme disk devices because of the trailing
digits in the KERNEL device name, e.g. nvme0n1. Partitions of nvme
disk devices are already properly handled by the existing rule for
ENV{DEVTYPE}=="partition".

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Kjeld Schouten <[email protected]>
Signed-off-by: Thomas Geppert <[email protected]>
Closes openzfs#9730
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Dec 27, 2019
@geppi @tonyhutter
Create symbolic links in /dev/disk/by-vdev for nvme disk devices
d37bea0 
The existing rules miss nvme disk devices because of the trailing
digits in the KERNEL device name, e.g. nvme0n1. Partitions of nvme
disk devices are already properly handled by the existing rule for
ENV{DEVTYPE}=="partition".

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Kjeld Schouten <[email protected]>
Signed-off-by: Thomas Geppert <[email protected]>
Closes openzfs#9730
tonyhutter pushed a commit that referenced this pull request Jan 23, 2020
@geppi @tonyhutter
Create symbolic links in /dev/disk/by-vdev for nvme disk devices
7f7c15c 
The existing rules miss nvme disk devices because of the trailing
digits in the KERNEL device name, e.g. nvme0n1. Partitions of nvme
disk devices are already properly handled by the existing rule for
ENV{DEVTYPE}=="partition".

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Kjeld Schouten <[email protected]>
Signed-off-by: Thomas Geppert <[email protected]>
Closes #9730
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PrivatePuffin PrivatePuffin PrivatePuffin approved these changes

@behlendorf behlendorf behlendorf approved these changes

Assignees
No one assigned
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@geppi @PrivatePuffin @behlendorf