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

libzfs: Fix zpool_vdev_online for FreeBSD zfsd #12083

Merged
merged 1 commit into from
Mar 11, 2022
Merged

libzfs: Fix zpool_vdev_online for FreeBSD zfsd #12083

merged 1 commit into from
Mar 11, 2022

Conversation

ghost
Copy link

@ghost ghost commented May 19, 2021 •
edited by ghost
Loading

Motivation and Context

zfsd on FreeBSD uses zpool_vdev_online to set a vdev online when reinserted, much like ZED does on Linux.
zfsd passes the vdev guid as the path to zpool_vdev_online, which works until we enter some wholedisk logic that tries to resolve the guid as a device path and fails.

Description

We can skip the wholedisk partition-resizing section on FreeBSD. It ultimately accomplished nothing, because zpool_relabel_disk is a no-op.

How Has This Been Tested?

Manually pulled and reinserted a disk on FreeBSD to make sure zfsd can do its thing now.

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:

@ghost ghost added the Status: Code Review Needed Ready for review and testing label May 19, 2021
@ghost ghost requested review from behlendorf and amotin May 19, 2021 16:15
@ghost
Copy link
Author

ghost commented May 19, 2021

I'll need to go back to the drawing board on this one. Linux wants the path to be to the whole disk, not the specific partition that's in the config.

@ghost ghost added Status: Design Review Needed Architecture or design is under discussion and removed Status: Code Review Needed Ready for review and testing labels May 19, 2021
@ghost ghost force-pushed the pathname branch from 9674485 to 4decfdd Compare May 28, 2021 15:08
@ghost ghost added Status: Revision Needed Changes are required for the PR to be accepted and removed Status: Design Review Needed Architecture or design is under discussion labels Jun 4, 2021
@behlendorf behlendorf added the Status: Work in Progress Not yet ready for general review label Mar 5, 2022
@ghost ghost force-pushed the pathname branch 2 times, most recently from deb9288 to 3632343 Compare March 7, 2022 13:48
@ghost
Copy link
Author

ghost commented Mar 7, 2022

  • Skip the wholedisk relabeling section on FreeBSD

@ghost ghost force-pushed the pathname branch 2 times, most recently from dadb6b6 to e933d98 Compare March 7, 2022 14:07
amotin
amotin reviewed Mar 7, 2022
View reviewed changes
lib/libzfs/libzfs_pool.c Outdated Show resolved Hide resolved
lib/libzfs/libzfs_pool.c Show resolved Hide resolved
libzfs: FreeBSD doesn't resize partitions for you
666098f 
This code can be failure prone on FreeBSD, where zfsd will pass a guid
as the vdev path to online.  The guid causes zfs_resolve_shortname to
fail because it expects a path.  We can just skip the whole ordeal.

Signed-off-by: Ryan Moeller <[email protected]>
@ghost ghost force-pushed the pathname branch 2 times, most recently from 320c3de to 9d76c7b Compare March 8, 2022 15:55
@ghost
Copy link
Author

ghost commented Mar 8, 2022

  • Rebased
  • Use fnvpair functions in zpool_find_vdev
  • Use zfs_resolve_shortname in zpool_find_vdev

@ghost ghost added Component: Userspace user space functionality Status: Code Review Needed Ready for review and testing Type: Defect Incorrect behavior (e.g. crash, hang) and removed Status: Work in Progress Not yet ready for general review Status: Revision Needed Changes are required for the PR to be accepted labels Mar 9, 2022
@ghost
Copy link
Author

ghost commented Mar 9, 2022

I'm going to do manual testing again without the last commit. It looks like it shouldn't be needed after all, since the path comparisons used to search for the vdev should be accounting for the platform search prefixes already.

@ghost ghost force-pushed the pathname branch from 9d76c7b to 666098f Compare March 9, 2022 20:41
@ghost
Copy link
Author

ghost commented Mar 9, 2022

I've removed the commits in zpool_find_vdev. They aren't needed after all. I will open another PR later for the fnvlist conversion and extend it to the whole file.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Mar 10, 2022
@ghost ghost changed the title libzfs: Fix vdev path handling libzfs: Fix zpool_vdev_online for FreeBSD zfsd Mar 10, 2022
@behlendorf behlendorf merged commit 76bcffb into openzfs:master Mar 11, 2022
@ghost ghost deleted the pathname branch March 11, 2022 18:22
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
@nicman23
libzfs: FreeBSD doesn't resize partitions for you
5aa542d 
This code can be failure prone on FreeBSD, where zfsd will pass a guid
as the vdev path to online.  The guid causes zfs_resolve_shortname to
fail because it expects a path.  We can just skip the whole ordeal.

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ryan Moeller <[email protected]>
Closes openzfs#12083
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
@nicman23
libzfs: FreeBSD doesn't resize partitions for you
108da8a 
This code can be failure prone on FreeBSD, where zfsd will pass a guid
as the vdev path to online.  The guid causes zfs_resolve_shortname to
fail because it expects a path.  We can just skip the whole ordeal.

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ryan Moeller <[email protected]>
Closes openzfs#12083
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Aug 30, 2022
@andrewc12
libzfs: FreeBSD doesn't resize partitions for you
3990f72 
This code can be failure prone on FreeBSD, where zfsd will pass a guid
as the vdev path to online.  The guid causes zfs_resolve_shortname to
fail because it expects a path.  We can just skip the whole ordeal.

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ryan Moeller <[email protected]>
Closes openzfs#12083
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Sep 2, 2022
@lundman
libzfs: FreeBSD doesn't resize partitions for you
1126391 
This code can be failure prone on FreeBSD, where zfsd will pass a guid
as the vdev path to online.  The guid causes zfs_resolve_shortname to
fail because it expects a path.  We can just skip the whole ordeal.

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ryan Moeller <[email protected]>
Closes openzfs#12083
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
@andrewc12
libzfs: FreeBSD doesn't resize partitions for you
fff5c4b 
This code can be failure prone on FreeBSD, where zfsd will pass a guid
as the vdev path to online.  The guid causes zfs_resolve_shortname to
fail because it expects a path.  We can just skip the whole ordeal.

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ryan Moeller <[email protected]>
Closes openzfs#12083
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
@andrewc12
libzfs: FreeBSD doesn't resize partitions for you
703b8bf 
This code can be failure prone on FreeBSD, where zfsd will pass a guid
as the vdev path to online.  The guid causes zfs_resolve_shortname to
fail because it expects a path.  We can just skip the whole ordeal.

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ryan Moeller <[email protected]>
Closes openzfs#12083
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
@andrewc12
libzfs: FreeBSD doesn't resize partitions for you
1c129b9 
This code can be failure prone on FreeBSD, where zfsd will pass a guid
as the vdev path to online.  The guid causes zfs_resolve_shortname to
fail because it expects a path.  We can just skip the whole ordeal.

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ryan Moeller <[email protected]>
Closes openzfs#12083
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
@andrewc12
libzfs: FreeBSD doesn't resize partitions for you
8429e0b 
This code can be failure prone on FreeBSD, where zfsd will pass a guid
as the vdev path to online.  The guid causes zfs_resolve_shortname to
fail because it expects a path.  We can just skip the whole ordeal.

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ryan Moeller <[email protected]>
Closes openzfs#12083
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
@andrewc12
libzfs: FreeBSD doesn't resize partitions for you
ff31297 
This code can be failure prone on FreeBSD, where zfsd will pass a guid
as the vdev path to online.  The guid causes zfs_resolve_shortname to
fail because it expects a path.  We can just skip the whole ordeal.

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ryan Moeller <[email protected]>
Closes openzfs#12083
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
@andrewc12
libzfs: FreeBSD doesn't resize partitions for you
15f73b2 
This code can be failure prone on FreeBSD, where zfsd will pass a guid
as the vdev path to online.  The guid causes zfs_resolve_shortname to
fail because it expects a path.  We can just skip the whole ordeal.

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ryan Moeller <[email protected]>
Closes openzfs#12083
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
@andrewc12
libzfs: FreeBSD doesn't resize partitions for you
749aea4 
This code can be failure prone on FreeBSD, where zfsd will pass a guid
as the vdev path to online.  The guid causes zfs_resolve_shortname to
fail because it expects a path.  We can just skip the whole ordeal.

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ryan Moeller <[email protected]>
Closes openzfs#12083
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
@andrewc12
libzfs: FreeBSD doesn't resize partitions for you
0eee004 
This code can be failure prone on FreeBSD, where zfsd will pass a guid
as the vdev path to online.  The guid causes zfs_resolve_shortname to
fail because it expects a path.  We can just skip the whole ordeal.

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ryan Moeller <[email protected]>
Closes openzfs#12083
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
@andrewc12
libzfs: FreeBSD doesn't resize partitions for you
ae7bdc2 
This code can be failure prone on FreeBSD, where zfsd will pass a guid
as the vdev path to online.  The guid causes zfs_resolve_shortname to
fail because it expects a path.  We can just skip the whole ordeal.

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ryan Moeller <[email protected]>
Closes openzfs#12083
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
@andrewc12
libzfs: FreeBSD doesn't resize partitions for you
648944d 
This code can be failure prone on FreeBSD, where zfsd will pass a guid
as the vdev path to online.  The guid causes zfs_resolve_shortname to
fail because it expects a path.  We can just skip the whole ordeal.

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ryan Moeller <[email protected]>
Closes openzfs#12083
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
@andrewc12
libzfs: FreeBSD doesn't resize partitions for you
35ab724 
This code can be failure prone on FreeBSD, where zfsd will pass a guid
as the vdev path to online.  The guid causes zfs_resolve_shortname to
fail because it expects a path.  We can just skip the whole ordeal.

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

@jwk404 jwk404

Labels
Component: Userspace user space functionality Status: Accepted Ready to integrate (reviewed, tested) Type: Defect Incorrect behavior (e.g. crash, hang)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@behlendorf @amotin @jwk404