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

zed: Control NVMe fault LEDs #12695

Merged
merged 1 commit into from
Nov 10, 2021
Merged

Conversation

tonyhutter
Copy link
Contributor

Motivation and Context

#12648

Description

The ZED code currently can only turn on the fault LED for a faulted disk in a JBOD enclosure. This extends support
for faulted NVMe disks as well.

How Has This Been Tested?

Force faulted a nvme drive with zed running. Verified that zed turned on/off the fault LED with zpool status -c fault_led.

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)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@tonyhutter
Copy link
Contributor Author

Please wait on reviewing this - the attention sysfs file is not only the attention LED, but may serve as a virtual attention button for PCI hotplug. "hitting" the attention button on drive fault may end up being be the behavior we want, but I need to think it over.

@behlendorf behlendorf added the Status: Work in Progress Not yet ready for general review label Oct 29, 2021
@tonyhutter
Copy link
Contributor Author

Please wait on reviewing this - the attention sysfs file is not only the attention LED, but may serve as a virtual attention button for PCI hotplug.

Ignore my comment above. It seems the attention sysfs file is just for the LED itself, so we're good:
https://elixir.bootlin.com/linux/v5.15/source/drivers/pci/hotplug/pciehp_core.c#L97

Reviewers please take a look at this when you get a chance.

@tonyhutter tonyhutter added Status: Code Review Needed Ready for review and testing and removed Status: Work in Progress Not yet ready for general review labels Nov 3, 2021
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.

Functionally it looks good, just a few suggestions.

cmd/zed/zed.d/statechange-led.sh Outdated Show resolved Hide resolved
lib/libzutil/os/linux/zutil_device_path_os.c Outdated Show resolved Hide resolved
lib/libzutil/os/linux/zutil_device_path_os.c Show resolved Hide resolved
lib/libzutil/os/linux/zutil_device_path_os.c Outdated Show resolved Hide resolved
lib/libzutil/os/linux/zutil_device_path_os.c Outdated Show resolved Hide resolved
lib/libzutil/os/linux/zutil_device_path_os.c Outdated Show resolved Hide resolved
lib/libzutil/os/linux/zutil_device_path_os.c Outdated Show resolved Hide resolved
lib/libzutil/os/linux/zutil_device_path_os.c Outdated Show resolved Hide resolved
lib/libzutil/os/linux/zutil_device_path_os.c Outdated Show resolved Hide resolved
The ZED code currently can only turn on the fault LED for
a faulted disk in a JBOD enclosure.  This extends support
for faulted NVMe disks as well.

Closes: openzfs#12648

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

@behlendorf thanks for looking it over. I just pushed out a newer version with your fixes included.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Nov 10, 2021
@behlendorf behlendorf merged commit ae70d62 into openzfs:master Nov 10, 2021
tonyhutter added a commit to tonyhutter/zfs that referenced this pull request Nov 10, 2021
The ZED code currently can only turn on the fault LED for
a faulted disk in a JBOD enclosure.  This extends support
for faulted NVMe disks as well.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Closes openzfs#12648
Closes openzfs#12695
tonyhutter added a commit to tonyhutter/zfs that referenced this pull request Nov 10, 2021
The ZED code currently can only turn on the fault LED for
a faulted disk in a JBOD enclosure.  This extends support
for faulted NVMe disks as well.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Closes openzfs#12648
Closes openzfs#12695
tonyhutter added a commit to tonyhutter/zfs that referenced this pull request Nov 13, 2021
The ZED code currently can only turn on the fault LED for
a faulted disk in a JBOD enclosure.  This extends support
for faulted NVMe disks as well.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Closes openzfs#12648
Closes openzfs#12695
tonyhutter added a commit to tonyhutter/zfs that referenced this pull request Nov 13, 2021
The ZED code currently can only turn on the fault LED for
a faulted disk in a JBOD enclosure.  This extends support
for faulted NVMe disks as well.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Closes openzfs#12648
Closes openzfs#12695
ofaaland pushed a commit to LLNL/zfs that referenced this pull request Mar 9, 2023
The ZED code currently can only turn on the fault LED for
a faulted disk in a JBOD enclosure.  This extends support
for faulted NVMe disks as well.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Closes openzfs#12648
Closes openzfs#12695
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants