Skip to content

Revert "Fix incorrect expected error in ztest" #17503

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

behlendorf
Copy link
Contributor

Motivation and Context

Revisiting #15295. A recent example of this failure in the CI can be found here:

https://github.com/openzfs/zfs/actions/runs/16007042925/attempts/3?pr=17501

Description

This reverts commit 2076011. The comment which explains EINVAL should be expected for this case was wrong, not the code. The kernel will return ENOTSUP when attaching a distributed spare to the wrong top-level dRAID vdev. See the check for this in spa_vdev_attach().

        /*
         * A dRAID spare can only replace a child of its parent dRAID vdev.
         */
        if (newvd->vdev_ops == &vdev_draid_spare_ops &&
            oldvd->vdev_top != vdev_draid_spare_get_parent(newvd)) {
                return (spa_vdev_exit(spa, newrootvd, txg, ENOTSUP));
        }

How Has This Been Tested?

Locally tested. During that testing I noticed a slightly different case which may explain the confusion here. During one run somehow ztest ended up adding a distributed spare for a top-level dRAID device which doesn't exist. In this case, EINVAL was returned instead ENOTSUP, however it should be impossible to manipulate a pool in to this layout. I wasn't able to manually reproduce the issue, so more debugging will be needed. That can be handled in a different PR.

Simplified example invalid pool config observed by ztest. Attaching draid1-1-0 will correctly return EINVAL, the draid1-1-0 distributed spare should not exist.

  pool: tank
 state: ONLINE
config:

	NAME                 STATE     READ WRITE CKSUM
	tank                 ONLINE       0     0     0
	  draid1:1d:3c:1s-0  ONLINE       0     0     0
	    /tmp/vdev1       ONLINE       0     0     0
	    /tmp/vdev2       ONLINE       0     0     0
	    /tmp/vdev3       ONLINE       0     0     0
	  draid1:1d:4c:2s-2  ONLINE       0     0     0
	    /tmp/vdev5       ONLINE       0     0     0
	    /tmp/vdev6       ONLINE       0     0     0
	    /tmp/vdev7       ONLINE       0     0     0
	    /tmp/vdev8       ONLINE       0     0     0
	spares
	  draid1-0-0         AVAIL   
	  draid1-2-0         AVAIL   
	  draid1-1-0         AVAIL   
	  draid1-2-1         AVAIL  

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)
  • Quality assurance (non-breaking change which makes the code more robust against bugs)
  • 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:

This reverts commit 2076011.  The
comment which explains EINVAL should be expected for this case was
wrong, not the code.  The kernel will return ENOTSUP when attaching
a distributed spare to the wrong top-level dRAID vdev.  See the
check for this in spa_vdev_attach().

Signed-off-by: Brian Behlendorf <[email protected]>
@behlendorf behlendorf requested a review from pcd1193182 July 1, 2025 20:37
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Jul 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Code Review Needed Ready for review and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant