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

libshare: nfs: deduplication, don't SIGHUP everyone, retry flock() on EINTR, don't reopen temporary file #12067

Closed
wants to merge 9 commits into from

Conversation

nabijaczleweli
Copy link
Contributor

@nabijaczleweli nabijaczleweli commented May 17, 2021

Motivation and Context

Assorted bag of duplicate code and subtle deficiencies and/or improvements I found.

Description

See individual commit messages, all logically disparate.

How Has This Been Tested?

Ran zfs unshare -a and zfs share -a on Linux.

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:

  • 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. – none apply hopefully
  • I have run the ZFS Test Suite with this change applied. – CI take my hand
  • All commit messages are properly formatted and contain Signed-off-by.

@nabijaczleweli nabijaczleweli changed the title libshare: nfs: don't SIGHUP everyone, retry flock() on EINTR, don't keep reopening temporary file libshare: nfs: don't SIGHUP everyone, retry flock() on EINTR, don't reopen temporary file May 17, 2021
@nabijaczleweli nabijaczleweli force-pushed the lack-a-too branch 3 times, most recently from 0afa4df to 6c11619 Compare May 17, 2021 19:11
@nabijaczleweli nabijaczleweli changed the title libshare: nfs: don't SIGHUP everyone, retry flock() on EINTR, don't reopen temporary file libshare: nfs: deduplication, don't SIGHUP everyone, retry flock() on EINTR, don't reopen temporary file May 17, 2021
@nabijaczleweli nabijaczleweli force-pushed the lack-a-too branch 2 times, most recently from 54adc9d to 9781df6 Compare May 18, 2021 15:39
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label May 19, 2021
@behlendorf behlendorf requested a review from grwilson May 19, 2021 04:28
@nabijaczleweli
Copy link
Contributor Author

2-week bump?

@nabijaczleweli
Copy link
Contributor Author

Rebased (this has a logical conflict with #12191 w.r.t. visibility(hidden) but it's trivial to update either if one's merged first)

@nabijaczleweli
Copy link
Contributor Author

3-and-a-bit-week bump!

@nabijaczleweli
Copy link
Contributor Author

Rebased cleanly

@nabijaczleweli
Copy link
Contributor Author

Another week bump

Description stolen from zfs-mount.8

Signed-off-by: Ahelenia Ziemiańska <[email protected]>
pidfile_open() sets *pidptr to -1 if the process currently holding
the lock is between pidfile_open() and pidfile_write(),
the subsequent kill(mountdpid) would potentially SIGHUP all
non-system processes except init: just sleep for half a millisecond
and try again in that case

Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
The shares are publicly known anyway and can be interrogated by any
user, so this is a debugging aid more than anything

Signed-off-by: Ahelenia Ziemiańska <[email protected]>
behlendorf pushed a commit that referenced this pull request Dec 17, 2021
Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes #12067
behlendorf pushed a commit that referenced this pull request Dec 17, 2021
Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes #12067
behlendorf pushed a commit that referenced this pull request Dec 17, 2021
The shares are publicly known anyway and can be interrogated by any
user, so this is a debugging aid more than anything.

Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes #12067
behlendorf pushed a commit that referenced this pull request Dec 17, 2021
This also works out to one syscall if the directory exists,
but is one syscall shorter if it doesn't.

Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes #12067
ghost pushed a commit to truenas/zfs that referenced this pull request Jan 19, 2022
pidfile_open() sets *pidptr to -1 if the process currently holding
the lock is between pidfile_open() and pidfile_write(),
the subsequent kill(mountdpid) would potentially SIGHUP all
non-system processes except init: just sleep for half a millisecond
and try again in that case

Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
ghost pushed a commit to truenas/zfs that referenced this pull request Feb 2, 2022
pidfile_open() sets *pidptr to -1 if the process currently holding
the lock is between pidfile_open() and pidfile_write(),
the subsequent kill(mountdpid) would potentially SIGHUP all
non-system processes except init: just sleep for half a millisecond
and try again in that case

Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
tonyhutter pushed a commit that referenced this pull request Feb 3, 2022
pidfile_open() sets *pidptr to -1 if the process currently holding
the lock is between pidfile_open() and pidfile_write(),
the subsequent kill(mountdpid) would potentially SIGHUP all
non-system processes except init: just sleep for half a millisecond
and try again in that case

Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes #12067
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Feb 10, 2022
Description stolen from zfs-mount.8

Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Feb 14, 2022
Description stolen from zfs-mount.8

Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Feb 16, 2022
Description stolen from zfs-mount.8

Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Feb 17, 2022
Description stolen from zfs-mount.8

Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Description stolen from zfs-mount.8

Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
pidfile_open() sets *pidptr to -1 if the process currently holding
the lock is between pidfile_open() and pidfile_write(),
the subsequent kill(mountdpid) would potentially SIGHUP all
non-system processes except init: just sleep for half a millisecond
and try again in that case

Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
The shares are publicly known anyway and can be interrogated by any
user, so this is a debugging aid more than anything.

Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
This also works out to one syscall if the directory exists,
but is one syscall shorter if it doesn't.

Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Description stolen from zfs-mount.8

Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
pidfile_open() sets *pidptr to -1 if the process currently holding
the lock is between pidfile_open() and pidfile_write(),
the subsequent kill(mountdpid) would potentially SIGHUP all
non-system processes except init: just sleep for half a millisecond
and try again in that case

Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
The shares are publicly known anyway and can be interrogated by any
user, so this is a debugging aid more than anything.

Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
This also works out to one syscall if the directory exists,
but is one syscall shorter if it doesn't.

Reviewed-by: Don Brady <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12067
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.

4 participants