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

Add explicit error for device_rebuild being disabled #12680

Merged
merged 1 commit into from
Oct 29, 2021

Conversation

rincebrain
Copy link
Contributor

Motivation and Context

(I ran into #11414, that's really all.)

Currently, you get back "can only attach to mirrors and top-level disks"
unconditionally if zpool attach returns ENOTSUP, but that also happens
if, say, feature@device_rebuild=disabled and you tried attach -s.

Description

Adds an error case for ENOTSUP and feature@device_rebuild=disabled.

(I don't check for unsupported explicitly because it's checked earlier in the function; I considered adding this error check right below that one, but opted to put it with the ENOTSUP return handling rather than another special case early on. If people disagree strongly, I can move it.)

How Has This Been Tested?

$ sudo zpool attach -s ephemeral ata-HGST_HUH728080ALE604_1234 /badideatest
cannot attach /badideatest to ata-HGST_HUH728080ALE604_1234: can only attach to mirrors and top-level disks
$ sudo cmd/zpool/zpool attach -s ephemeral ata-HGST_HUH728080ALE604_2EGTR1JX /badideatest
cannot attach /badideatest to ata-HGST_HUH728080ALE604_1234: device_rebuild feature must be enabled in order to use sequential reconstruction
$ sudo zpool get feature@device_rebuild ephemeral
NAME       PROPERTY                VALUE                   SOURCE
ephemeral  feature@device_rebuild  disabled                local
$

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:

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.

Handling it here seems fine to me. Thanks for the fix.

Currently, you get back "can only attach to mirrors and top-level disks"
unconditionally if zpool attach returns ENOTSUP, but that also happens
if, say, feature@device_rebuild=disabled and you tried attach -s.

So let's print an error for that case, lest people go down a rabbit hole
looking into what they did wrong.

Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
@behlendorf behlendorf self-assigned this Oct 26, 2021
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Oct 29, 2021
@behlendorf behlendorf merged commit 1139e17 into openzfs:master Oct 29, 2021
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.

None yet

2 participants