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

'zfs share -a' should handle 'canmount=noauto' #10688

Merged
merged 1 commit into from
Aug 11, 2020
Merged

'zfs share -a' should handle 'canmount=noauto' #10688

merged 1 commit into from
Aug 11, 2020

Conversation

grwilson
Copy link
Member

@grwilson grwilson commented Aug 7, 2020 •
edited
Loading

Motivation and Context

The 'zfs share -a' currently skips any filesystems which
have 'canmount=noauto' set. This behavior is unexpected since the
one would expect 'zfs share -a' to share any mounted filesystem
that has the 'sharenfs' property already set.

Description

This changes the behavior of 'zfs share -a' to allow the sharing
of 'canmount=noauto' datasets if they are mounted. This is inline with
the manpage of 'zfs share -'a which states:

-a  Share all available ZFS file systems.  Invoked automatically as
           part of the boot process.

How Has This Been Tested?

ZTS
(test has been updated)

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)
  • Documentation (a change to man pages or other documentation)

Checklist:

  • My code follows the ZFS on Linux 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.
  • All commit messages are properly formatted and contain Signed-off-by.

@grwilson
'zfs share -a' should handle 'canmount=noauto'
8473ee9 
The 'zfs share -a' currently skips any filesystems which
have 'canmount=noauto' set. This behavior is unexpected since the
one would expect 'zfs share -a' to share any mounted filesystem
that has the 'sharenfs' property already set.

This changes the behavior of 'zfs share -a' to allow the sharing
of 'canmount=noauto' datasets if they are mounted.

Signed-off-by: George Wilson <[email protected]>
External-issue: DLPX-71313
@codecov
Copy link

codecov bot commented Aug 8, 2020 •
edited
Loading

Codecov Report

Merging #10688 into master will decrease coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10688      +/-   ##
==========================================
- Coverage   79.86%   79.77%   -0.09%     
==========================================
  Files         394      394              
  Lines      124653   124655       +2     
==========================================
- Hits        99551    99449     -102     
- Misses      25102    25206     +104
Flag Coverage Δ
#kernel 80.34% <ø> (+0.01%) ⬆️
#user 65.42% <100.00%> (-1.00%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cmd/zfs/zfs_main.c 82.80% <100.00%> (+0.22%) ⬆️
cmd/zdb/zdb_il.c 30.86% <0.00%> (-24.08%) ⬇️
module/zfs/vdev_indirect.c 73.83% <0.00%> (-11.17%) ⬇️
module/os/linux/zfs/vdev_file.c 80.37% <0.00%> (-3.74%) ⬇️
lib/libzpool/kernel.c 64.53% <0.00%> (-2.98%) ⬇️
module/zfs/ddt.c 88.20% <0.00%> (-2.71%) ⬇️
module/zfs/vdev_removal.c 94.40% <0.00%> (-2.63%) ⬇️
module/zfs/vdev_raidz.c 91.17% <0.00%> (-2.07%) ⬇️
module/zfs/zio.c 87.48% <0.00%> (-1.64%) ⬇️
module/zfs/vdev_indirect_mapping.c 97.10% <0.00%> (-1.45%) ⬇️
... and 51 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da60484...8473ee9. Read the comment docs.

@behlendorf behlendorf added the Status: Accepted Ready to integrate (reviewed, tested) label Aug 11, 2020
@behlendorf behlendorf merged commit 53c9d1d into openzfs:master Aug 11, 2020
behlendorf pushed a commit that referenced this pull request Aug 20, 2020
'zfs share -a' should clean noauto exports
7bba1d4 
This is a follow on to PR #10688 where `zfs share -a` allows the 
sharing of canmount=noauto datasets if they are mounted.  However, 
when a dataset with canmount=noauto is not mounted, the command 
should also purge any existing entries from the exports file. 
Otherwise, after a reboot, the nfs server attempts to export the 
underlying mountpath, not the dataset. This can lead to a hard hang 
for existing client mounts.

Instead of just skipping the adding of an export if not mounted 
and canmount=noauto, have it also remove an existing export of the 
dataset so that, after a reboot, we don't export an unmounted dataset.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: George Wilson <[email protected]>
Signed-off-by: Don Brady <[email protected]>
Closes #10747
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
@grwilson @jsai20
'zfs share -a' should handle 'canmount=noauto'
adcc112 
The 'zfs share -a' currently skips any filesystems which
have 'canmount=noauto' set. This behavior is unexpected since the
one would expect 'zfs share -a' to share any mounted filesystem
that has the 'sharenfs' property already set.

This changes the behavior of 'zfs share -a' to allow the sharing
of 'canmount=noauto' datasets if they are mounted.

Reviewed-by: Matt Ahrens <[email protected]>
Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Prakash Surya <[email protected]>
Signed-off-by: George Wilson <[email protected]>
External-issue: DLPX-71313
Closes openzfs#10688
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
@jsai20
'zfs share -a' should clean noauto exports
630a2a9 
This is a follow on to PR openzfs#10688 where `zfs share -a` allows the 
sharing of canmount=noauto datasets if they are mounted.  However, 
when a dataset with canmount=noauto is not mounted, the command 
should also purge any existing entries from the exports file. 
Otherwise, after a reboot, the nfs server attempts to export the 
underlying mountpath, not the dataset. This can lead to a hard hang 
for existing client mounts.

Instead of just skipping the adding of an export if not mounted 
and canmount=noauto, have it also remove an existing export of the 
dataset so that, after a reboot, we don't export an unmounted dataset.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: George Wilson <[email protected]>
Signed-off-by: Don Brady <[email protected]>
Closes openzfs#10747
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
@grwilson
'zfs share -a' should handle 'canmount=noauto'
bd056b8 
The 'zfs share -a' currently skips any filesystems which
have 'canmount=noauto' set. This behavior is unexpected since the
one would expect 'zfs share -a' to share any mounted filesystem
that has the 'sharenfs' property already set.

This changes the behavior of 'zfs share -a' to allow the sharing
of 'canmount=noauto' datasets if they are mounted.

Reviewed-by: Matt Ahrens <[email protected]>
Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Prakash Surya <[email protected]>
Signed-off-by: George Wilson <[email protected]>
External-issue: DLPX-71313
Closes openzfs#10688
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
'zfs share -a' should clean noauto exports
deb6959 
This is a follow on to PR openzfs#10688 where `zfs share -a` allows the 
sharing of canmount=noauto datasets if they are mounted.  However, 
when a dataset with canmount=noauto is not mounted, the command 
should also purge any existing entries from the exports file. 
Otherwise, after a reboot, the nfs server attempts to export the 
underlying mountpath, not the dataset. This can lead to a hard hang 
for existing client mounts.

Instead of just skipping the adding of an export if not mounted 
and canmount=noauto, have it also remove an existing export of the 
dataset so that, after a reboot, we don't export an unmounted dataset.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: George Wilson <[email protected]>
Signed-off-by: Don Brady <[email protected]>
Closes openzfs#10747
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@don-brady don-brady don-brady approved these changes

@ahrens ahrens ahrens approved these changes

@prakashsurya prakashsurya prakashsurya approved these changes

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

Successfully merging this pull request may close these issues.
5 participants
@grwilson @prakashsurya @ahrens @don-brady @behlendorf