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

Notify on UNAVAIL statechange #12630

Merged
merged 1 commit into from
Oct 15, 2021
Merged

Notify on UNAVAIL statechange #12630

merged 1 commit into from
Oct 15, 2021

Conversation

bitonic
Copy link
Contributor

@bitonic bitonic commented Oct 10, 2021

UNAVAIL is maybe not quite as concerning as DEGRADED, but still an event of notice, in my opinion. For example it is triggered when a drive goes missing.

Motivation and Context

See #12629 for the context of this change. If a drive goes missing, there are no default zedlets that send a notification.

How Has This Been Tested?

I've tested this change by modifying the notify-statechange.sh script on my machine.

This is what the email looks once this patch is applied:

Subject: ZFS device fault for pool 0xE244C57CC81BE54B on XYZ

ZFS has detected that a device was removed.

 impact: Fault tolerance of the pool may be compromised.
    eid: 20
  class: statechange
  state: UNAVAIL
   host: XYZ
   time: 2021-10-10 16:00:55+0000
  vpath: /dev/disk/by-id/ata-ST10000NM0568-2H5110_ZHZ54DBW-part1
  vguid: 0x7E580703404ED3FF
   pool: 0xE244C57CC81BE54B

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:

@behlendorf behlendorf added Status: Code Review Needed Ready for review and testing Component: ZED ZFS Event Daemon labels Oct 11, 2021
@behlendorf behlendorf requested a review from tonyhutter October 11, 2021 17:39
Copy link
Contributor

@don-brady don-brady left a comment

Choose a reason for hiding this comment

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

Looks like the commit message is failing checkstyle. The actual code change LGTM.

Copy link
Contributor

@tonyhutter tonyhutter left a comment

Choose a reason for hiding this comment

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

I approve, pending the commit style change

@bitonic
Copy link
Contributor Author

bitonic commented Oct 14, 2021

Do I need to do anything? It says it's missing Signed-off-by.

@behlendorf
Copy link
Contributor

@bitonic you'll need to force update this PR to include your signed-off-by in the commit message. This will resolve the style error. While you're at it I'd suggest rebasing against the latest version of the master branch as well.

https://github.com/openzfs/zfs/blob/master/.github/CONTRIBUTING.md#signed-off-by

@bitonic
Copy link
Contributor Author

bitonic commented Oct 14, 2021

@behlendorf done.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Oct 14, 2021
Signed-off-by: Francesco Mazzoli <f@mazzo.li>

`UNAVAIL` is maybe not quite as concerning as `DEGRADED`, but still an
event of notice, in my opinion. For example it is triggered when a
drive goes missing.
@behlendorf behlendorf merged commit fd778e4 into openzfs:master Oct 15, 2021
cbane referenced this pull request in cbane/zfs Nov 6, 2021
Unplugging a disk results in it having state UNAVAIL, but zed would only
report states FAULTED, DEGRADED, and REMOVED. This commit makes it also
report on state UNAVAIL.

Signed-off-by: Courtney Bane <courtney.bane@gmail.com>
@cbane
Copy link

cbane commented Nov 6, 2021

This also fixes #10123. I had a nearly identical fix for that that I never got around to actually sending a pull request for.

@cholzer79
Copy link

I installed zed on my proxmox machine yesterday (apt-get install zfs-zed) and this fix was not present.
When can we expect a new build that includes this fix so that I don't have to remember altering notify-statechange.sh myself? :)

tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Feb 10, 2022
`UNAVAIL` is maybe not quite as concerning as `DEGRADED`, but still an
event of notice, in my opinion. For example it is triggered when a
drive goes missing.

Reviewed-by: Don Brady <don.brady@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Francesco Mazzoli <f@mazzo.li>
Closes openzfs#12629
Closes openzfs#12630
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: ZED ZFS Event Daemon Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants