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

Fix ZFS on FreeBSD so that snapshots under .zfs/snapshot are NFS visible #15563

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

rmacklem
Copy link
Contributor

Call vfs_exjail_clone() for mounts created under .zfs/snapshot to fill in the mnt_exjail field for the mount. If this is not done, the snapshots under .zfs/snapshot with not be accessible over NFS.

Signed-off by: Rick Macklem rmacklem@uoguelph.ca

Motivation and Context

Without this patch, newer versions of FreeBSD (post 13.2) cannot access
ZFS snapshots in .zfs/snapshot over NFS.

Description

Added a new argument to mount_snapshot() which is the parent
mount point so that vfs_exjail_clone() can be called with that
parent mount point.
vfs_exjail_clone() fills in mnt_exjail, so that the snapshot can
be accessed over NFS.

How Has This Been Tested?

An NFS mount of a dataset on a FreeBSD NFS server (post 13.2)
without this patch could not access the snapshots in .zfs/snapshot.
After applying this patch, the NFS client did access the snapshot
on .zfs/snapshot without difficulty.

Types of changes

  • [x ] 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:

  • My code follows the OpenZFS code style requirements.
  • I have updated the documentation accordingly.
  • I have read the contributing document.
  • I have added tests to cover my changes.
  • I have run the ZFS Test Suite with this change applied.
  • [x ] All commit messages are properly formatted and contain Signed-off-by.

@rmacklem
Copy link
Contributor Author

One of the required tests failed, but since it is a Linux
test and my patch only affects the freebsd parts of
the source tree, I don't see how it could be the cause
of the failure?

Call vfs_exjail_clone() for mounts created under .zfs/snapshot
to fill in the mnt_exjail field for the mount.  If this is not
done, the snapshots under .zfs/snapshot with not be accessible
over NFS.

This version has the argument name in vfs.h fixed to match that
of the name in spl_vfs.c, although it really does not matter.

Signed-off-by: Rick Macklem <rmacklem@uoguelph.ca>
@amotin
Copy link
Member

amotin commented Nov 23, 2023

@rmacklem Unfortunately some of the CI tests are not very stable lately. It can be unrelated.

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.

It got some review on FreeBSD side: https://reviews.freebsd.org/D42672 . I can not say much about the VFS code, but it looks OK.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Nov 28, 2023
@behlendorf behlendorf merged commit acb33ee into openzfs:master Nov 28, 2023
23 of 26 checks passed
tonyhutter pushed a commit that referenced this pull request Nov 30, 2023
Call vfs_exjail_clone() for mounts created under .zfs/snapshot
to fill in the mnt_exjail field for the mount.  If this is not
done, the snapshots under .zfs/snapshot with not be accessible
over NFS.

This version has the argument name in vfs.h fixed to match that
of the name in spl_vfs.c, although it really does not matter.

External-issue: https://reviews.freebsd.org/D42672
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Signed-off-by: Rick Macklem <rmacklem@uoguelph.ca>
Closes #15563
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Dec 12, 2023
Call vfs_exjail_clone() for mounts created under .zfs/snapshot
to fill in the mnt_exjail field for the mount.  If this is not
done, the snapshots under .zfs/snapshot with not be accessible
over NFS.

This version has the argument name in vfs.h fixed to match that
of the name in spl_vfs.c, although it really does not matter.

External-issue: https://reviews.freebsd.org/D42672
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Signed-off-by: Rick Macklem <rmacklem@uoguelph.ca>
Closes openzfs#15563
gamanakis pushed a commit to gamanakis/zfs that referenced this pull request Jan 19, 2024
Call vfs_exjail_clone() for mounts created under .zfs/snapshot
to fill in the mnt_exjail field for the mount.  If this is not
done, the snapshots under .zfs/snapshot with not be accessible
over NFS.

This version has the argument name in vfs.h fixed to match that
of the name in spl_vfs.c, although it really does not matter.

External-issue: https://reviews.freebsd.org/D42672
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Signed-off-by: Rick Macklem <rmacklem@uoguelph.ca>
Closes openzfs#15563
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.

3 participants