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

FreeBSD: Fix a pair of bugs in zfs_fhtovp() #14001

Merged
merged 1 commit into from
Oct 11, 2022
Merged

Conversation

markjdb
Copy link
Contributor

@markjdb markjdb commented Oct 7, 2022

This PR addresses two bugs reported here: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=266236

Motivation and Context

The change fixes a regression that prevents snapshot directories from being accessed over NFS.

Description

The change adds an unlock to an error path and removes a Linux-specific check to the function which translates NFS file IDs to vnodes.

How Has This Been Tested?

Manual testing on FreeBSD.

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:

- Add a zfs_exit() call in an error path, otherwise a lock is leaked.
- Remove the fid_gen > 1 check.  That appears to be Linux-specific:
  zfsctl_snapdir_fid() sets fid_gen to 0 or 1 depending on whether the
  snapshot directory is mounted.  On FreeBSD it fails, making snapshot
  dirs inaccessible via NFS.

Fixes: 43dbf88 ("FreeBSD: vfsops: use setgen for error case")
Signed-off-by: Mark Johnston <markj@FreeBSD.org>
@eborisch
Copy link

eborisch commented Oct 7, 2022

Ref: #13974

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Oct 7, 2022
@behlendorf behlendorf requested review from a user and amotin October 7, 2022 20:33
@ghost ghost added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Oct 9, 2022
@behlendorf behlendorf merged commit ed566bf into openzfs:master Oct 11, 2022
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Oct 12, 2022
- Add a zfs_exit() call in an error path, otherwise a lock is leaked.
- Remove the fid_gen > 1 check.  That appears to be Linux-specific:
  zfsctl_snapdir_fid() sets fid_gen to 0 or 1 depending on whether the
  snapshot directory is mounted.  On FreeBSD it fails, making snapshot
  dirs inaccessible via NFS.

Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Reviewed-by: Andriy Gapon <avg@FreeBSD.org>
Signed-off-by: Mark Johnston <markj@FreeBSD.org>
Fixes: 43dbf88 ("FreeBSD: vfsops: use setgen for error case")
Closes openzfs#14001
Closes openzfs#13974
ghost pushed a commit to truenas/zfs that referenced this pull request Oct 21, 2022
- Add a zfs_exit() call in an error path, otherwise a lock is leaked.
- Remove the fid_gen > 1 check.  That appears to be Linux-specific:
  zfsctl_snapdir_fid() sets fid_gen to 0 or 1 depending on whether the
  snapshot directory is mounted.  On FreeBSD it fails, making snapshot
  dirs inaccessible via NFS.

Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Reviewed-by: Andriy Gapon <avg@FreeBSD.org>
Signed-off-by: Mark Johnston <markj@FreeBSD.org>
Fixes: 43dbf88 ("FreeBSD: vfsops: use setgen for error case")
Closes openzfs#14001
Closes openzfs#13974
(cherry picked from commit ed566bf)
behlendorf pushed a commit that referenced this pull request Oct 26, 2022
- Add a zfs_exit() call in an error path, otherwise a lock is leaked.
- Remove the fid_gen > 1 check.  That appears to be Linux-specific:
  zfsctl_snapdir_fid() sets fid_gen to 0 or 1 depending on whether the
  snapshot directory is mounted.  On FreeBSD it fails, making snapshot
  dirs inaccessible via NFS.

Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Reviewed-by: Andriy Gapon <avg@FreeBSD.org>
Signed-off-by: Mark Johnston <markj@FreeBSD.org>
Fixes: 43dbf88 ("FreeBSD: vfsops: use setgen for error case")
Closes #14001
Closes #13974
(cherry picked from commit ed566bf)
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.

5 participants