Skip to content

Fix nfs_truncate_shares without /etc/exports.d #15468

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

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

siv0
Copy link
Contributor

@siv0 siv0 commented Oct 30, 2023

calling nfs_reset_shares on Linux prints a warning:
failed to lock /etc/exports.d/zfs.exports.lock: No such file or directory
when /etc/exports.d does not exist. The directory gets created, when a filesystem is actually exported through nfs_toggle_share and nfs_init_share. The truncation of /etc/exports.d/zfs.exports happens unconditionally when calling zfs mount -a (via zfs_do_mount and share_mount in cmd/zfs/zfs_main.c).

Fixing the issue only in the Linux part, since the exports file on FreeBSD is in /etc/zfs/, which seems present on 2 FreeBSD systems I have access to (through /etc/zfs/compatibility.d/), while a Debian box does not have the directory even if /usr/sbin/exportfs is present through the nfs-kernel-server package.

The code for exports_available is copied from nfs_available above.

Fixes: ede037c
("Make zfs-share service resilient to stale exports")
Closes #15369

The commit adds a check for existence of /etc/exports.d, before trying to lock /etc/exports.d/zfs.exports.lock.
This prevents the printing of a warning message on zfs mount -a or zfs share -a

Motivation and Context

#15369

Description

How Has This Been Tested?

Compiled Proxmox utils and libs packages with the patch applied on top of ZFS-2.0.0 and verified that
zfs mount -a does not print the warning anymore

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:

@behlendorf behlendorf requested a review from grwilson October 30, 2023 22:45
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Oct 30, 2023
@@ -539,6 +540,8 @@ nfs_commit_shares(void)
static void
nfs_truncate_shares(void)
{
if (!exports_available())
return (SA_OK);
Copy link
Contributor

Choose a reason for hiding this comment

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

lib/libshare/os/linux/nfs.c: In function 'nfs_truncate_shares':
lib/libshare/os/linux/nfs.c:544:3: error: 'return' with a value, in function returning void [-Werror]
   return (SA_OK);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry for the noise - and not including --enable-debug in the config options

calling nfs_reset_shares on Linux prints a warning:
`failed to lock /etc/exports.d/zfs.exports.lock: No such file or
directory`
when /etc/exports.d does not exist. The directory gets created, when a
filesystem is actually exported through nfs_toggle_share and
nfs_init_share. The truncation of /etc/exports.d/zfs.exports happens
unconditionally when calling `zfs mount -a` (via zfs_do_mount and
share_mount in `cmd/zfs/zfs_main.c`).

Fixing the issue only in the Linux part, since the exports file on
freebsd is in `/etc/zfs/`, which seems present on 2 FreeBSD systems I
have access to (through `/etc/zfs/compatibility.d/`), while a Debian
box does not have the directory even if `/usr/sbin/exportfs` is
present through the `nfs-kernel-server` package.

The code for exports_available is copied from nfs_available above.

Fixes: ede037c
("Make zfs-share service resilient to stale exports")
Closes openzfs#15369

Signed-off-by: Stoiko Ivanov <[email protected]>
@siv0 siv0 force-pushed the nonexisting_exports_truncate branch from 85a05d9 to d732cf7 Compare October 31, 2023 00:47
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Oct 31, 2023
@behlendorf behlendorf merged commit 41e55b4 into openzfs:master Oct 31, 2023
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Dec 12, 2023
Calling nfs_reset_shares on Linux prints a warning:
`failed to lock /etc/exports.d/zfs.exports.lock: No such file or
directory`
when /etc/exports.d does not exist. The directory gets created, when a
filesystem is actually exported through nfs_toggle_share and
nfs_init_share. The truncation of /etc/exports.d/zfs.exports happens
unconditionally when calling `zfs mount -a` (via zfs_do_mount and
share_mount in `cmd/zfs/zfs_main.c`).

Fixing the issue only in the Linux part, since the exports file on
freebsd is in `/etc/zfs/`, which seems present on 2 FreeBSD systems I
have access to (through `/etc/zfs/compatibility.d/`), while a Debian
box does not have the directory even if `/usr/sbin/exportfs` is
present through the `nfs-kernel-server` package.

The code for exports_available is copied from nfs_available above.

Fixes: ede037c
("Make zfs-share service resilient to stale exports")

Reviewed-by: Brian Atkinson <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Stoiko Ivanov <[email protected]>
Closes openzfs#15369 
Closes openzfs#15468
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.

"failed to lock /etc/exports.d/zfs.exports.lock: No such file or directory"
3 participants