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

zed: add hotplug support for spare vdevs #14295

Merged
merged 1 commit into from
Jan 9, 2023
Merged

zed: add hotplug support for spare vdevs #14295

merged 1 commit into from
Jan 9, 2023

Conversation

ixhamza
Copy link
Member

@ixhamza ixhamza commented Dec 15, 2022 •
edited
Loading

Motivation and Context

Add hotplug support for spare vdevs

Description

This commit supports for spare vdev hotplug. The spare vdev associated with all the pools will be marked as "Removed" when the drive is physically detached and will become "Available" when the drive is reattached. Currently, the spare vdev status does not change on the drive removal and the same is the case with reattachment.

How Has This Been Tested?

By adding and removing spares through QEMU-emulated drives.

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:

@ixhamza
Copy link
Member Author

ixhamza commented Dec 15, 2022 •
edited
Loading

@amotin, @freqlabs, @tonyhutter.

@ixhamza
Copy link
Member Author

ixhamza commented Dec 16, 2022 •
edited
Loading

Right now I am removing spares completely from the pool on the removal event, which does not seem right after a discussion with @amotin. I will update the PR to change the spare state to REMOVED when detached and will make it AVAILABLE if the disk is reattached. Making this PR Draft at the moment.

@ixhamza ixhamza marked this pull request as draft December 16, 2022 17:49
@ghost ghost added Component: ZED ZFS Event Daemon Status: Work in Progress Not yet ready for general review labels Dec 16, 2022
@ixhamza ixhamza force-pushed the NAS-115112 branch 2 times, most recently from 347623d to 2a36440 Compare December 17, 2022 00:19
@ixhamza ixhamza marked this pull request as ready for review December 17, 2022 00:19
@ixhamza
Copy link
Member Author

ixhamza commented Dec 17, 2022

I have updated the PR. Spare vdev now transitions to the "Removed" state when the drive is removed and becomes "Available" when reattached.

@ghost
Copy link

ghost commented Dec 19, 2022

Would it be a problem to just remove the checks for it being a spare in libzfs instead of changing the function signatures?

@ixhamza
Copy link
Member Author

ixhamza commented Dec 19, 2022

@freqlabs Apparently, it should not be a problem. I will run perform some manual testing to check for corner cases. Thank you.

@ixhamza
Copy link
Member Author

ixhamza commented Dec 19, 2022

@freqlabs - I have checked and there is no problem in removing the spare checks in zpool_vdev_remove_wanted() and I will remove that. For the second check, zpool_vdev_online(), I don't think we should change that as it seems like we don't allow doing offline/online spare vdevs from the command line, e.g., zpool offline pool vdev and zpool online pool vdev.

@ixhamza ixhamza force-pushed the NAS-115112 branch 2 times, most recently from 519bb63 to cd7c43a Compare December 19, 2022 21:16
@ghost
Copy link

ghost commented Dec 20, 2022

I looks like we could add a ZFS_ONLINE_SPARE flag to pass through the existing flags param and avoid the API change.

@ghost ghost added Status: Code Review Needed Ready for review and testing and removed Status: Work in Progress Not yet ready for general review labels Dec 20, 2022
@ixhamza
Copy link
Member Author

ixhamza commented Dec 20, 2022

@freqlabs Yes, you are right. I have updated the PR. Thank you.

ghost
ghost reviewed Dec 21, 2022
View reviewed changes
amotin
amotin approved these changes Dec 27, 2022
View reviewed changes
Copy link
Member

@amotin amotin left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the couple things we discussed. Now looks good to me.

@ixhamza
zed: add hotplug support for spare vdevs
5966a9b 
This commit supports for spare vdev hotplug. The
spare vdev associated with all the pools will be
marked as "Removed" when the drive is physically
detached and will become "Available" when the
drive is reattached. Currently, the spare vdev
status does not change on the drive removal and
the same is the case with reattachment.

Signed-off-by: Ameer Hamza <[email protected]>
@ixhamza
Copy link
Member Author

ixhamza commented Jan 9, 2023

@behlendorf I just rebased this PR with the latest master. It would be appreciated if you can review/approve it.

@behlendorf behlendorf merged commit 5091867 into openzfs:master Jan 9, 2023
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jan 9, 2023
This was referenced Jan 26, 2023
Merged
Merged
Merged
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Mar 3, 2023
@ixhamza @lundman
zed: add hotplug support for spare vdevs
115670d 
This commit supports for spare vdev hotplug. The
spare vdev associated with all the pools will be
marked as "Removed" when the drive is physically
detached and will become "Available" when the
drive is reattached. Currently, the spare vdev
status does not change on the drive removal and
the same is the case with reattachment.

Reviewed-by: Tony Hutter <[email protected]>
Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ameer Hamza <[email protected]>
Closes openzfs#14295
@ixhamza ixhamza mentioned this pull request Mar 20, 2023
Closed
ixhamza added a commit to truenas/zfs that referenced this pull request Mar 24, 2023
@ixhamza
zed: add hotplug support for spare vdevs
b8f4eed 
This commit supports for spare vdev hotplug. The
spare vdev associated with all the pools will be
marked as "Removed" when the drive is physically
detached and will become "Available" when the
drive is reattached. Currently, the spare vdev
status does not change on the drive removal and
the same is the case with reattachment.

Reviewed-by: Tony Hutter <[email protected]>
Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ameer Hamza <[email protected]>
Closes openzfs#14295
behlendorf pushed a commit that referenced this pull request Mar 27, 2023
@ixhamza @behlendorf
zed: add hotplug support for spare vdevs
dedd824 
This commit supports for spare vdev hotplug. The
spare vdev associated with all the pools will be
marked as "Removed" when the drive is physically
detached and will become "Available" when the
drive is reattached. Currently, the spare vdev
status does not change on the drive removal and
the same is the case with reattachment.

Reviewed-by: Tony Hutter <[email protected]>
Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ameer Hamza <[email protected]>
Closes #14295
ofaaland pushed a commit to LLNL/zfs that referenced this pull request Jun 16, 2023
@ixhamza @defaziogiancarlo
zed: add hotplug support for spare vdevs
e47ca4b 
This commit supports for spare vdev hotplug. The
spare vdev associated with all the pools will be
marked as "Removed" when the drive is physically
detached and will become "Available" when the
drive is reattached. Currently, the spare vdev
status does not change on the drive removal and
the same is the case with reattachment.

Reviewed-by: Tony Hutter <[email protected]>
Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ameer Hamza <[email protected]>
Closes openzfs#14295
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@behlendorf behlendorf behlendorf approved these changes

@amotin amotin amotin approved these changes

@tonyhutter tonyhutter tonyhutter approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@ixhamza @behlendorf @amotin @tonyhutter