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

libzfs: add zfs_mount_at() function #9833

Merged
merged 1 commit into from Jan 14, 2020
Merged

Conversation

kevans91
Copy link
Contributor

Motivation and Context

zfs_mount_at will be used by FreeBSD's libbe to transition away from directly using zmount.

Description

This adds the capability for libzfs consumers to mount a dataset at an arbitrary mountpoint without resorting to digging deeper into libzfs internals to use hidden OS-specific mounting interfaces, and to do so without modifying dataset/pool properties to achieve the desired effect.

How Has This Been Tested?

Replaced libbe's zmount() with zfs_mount_at() in our local zfs; diff is similar enough that I wouldn't anticipate any issues in the transition here.

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.

I'm not entirely sure where one would find libzfs documentation, given the nature of it. Also unsure of where one would add tests for it, but given that this just pulls the 'grab mountpoint 'part out into zfs_mount() and doesn't functionally change much- I guess the test coverage doesn't drop all that much.

zfs_mount_at() mounts a dataset at an arbitrary mountpoint rather than
at the configured mountpoint. This may be used by consumers that wish to
temporarily expose a dataset at another mountpoint without altering
dataset/pool properties.

This will be used by FreeBSD's libbe be_mount(), which mounts a boot
environment at an arbitrary mountpoint.

Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
@codecov
Copy link

codecov bot commented Jan 11, 2020

Codecov Report

Merging #9833 into master will increase coverage by <1%.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #9833    +/-   ##
========================================
+ Coverage      79%      80%   +<1%     
========================================
  Files         385      385            
  Lines      121610   121615     +5     
========================================
+ Hits        96429    96706   +277     
+ Misses      25181    24909   -272
Flag Coverage Δ
#kernel 80% <ø> (ø) ⬆️
#user 67% <100%> (+1%) ⬆️

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 6e1c594...cc776ca. Read the comment docs.

@ghost ghost added Component: Userspace user space functionality Status: Code Review Needed Ready for review and testing Type: Feature Feature request or new feature labels Jan 11, 2020
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Providing such an interface makes sense. Are you aware of anything other than the mountpoint which would be useful to specify?

@kevans91
Copy link
Contributor Author

I had put some thought into it, but hadn't come up with anything (unless it's feasible to, say, set sync=disabled for just this mount).

Perhaps it would be better to structure this as some kind of zfs_mount_nvl instead to leave it open-ended for future expansion, them for the time being we just reject any property that isn't "mountpoint" -- worst case scenario, we're stuck with a function that's needlessly flexible. We can avoid referring to the extra parameter as "properties" so we aren't tied to 1:1 mapping property names.

@behlendorf
Copy link
Contributor

OK, I couldn't think of anything immediately useful either. Why don't we go ahead with the PR as proposed but keep the idea for a zfs_mount_nvl in our back pocket. We can always implement it if someone comes up with a consumer that needs a more flexible interface.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jan 14, 2020
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

We have test coverage from existing tests since it's used by zfs_mount(). 👍

@kevans91
Copy link
Contributor Author

OK, I couldn't think of anything immediately useful either. Why don't we go ahead with the PR as proposed but keep the idea for a zfs_mount_nvl in our back pocket. We can always implement it if someone comes up with a consumer that needs a more flexible interface.

Sounds good to me!

@behlendorf behlendorf merged commit 68a192e into openzfs:master Jan 14, 2020
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
zfs_mount_at() mounts a dataset at an arbitrary mountpoint rather than
at the configured mountpoint. This may be used by consumers that wish to
temporarily expose a dataset at another mountpoint without altering
dataset/pool properties.

This will be used by FreeBSD's libbe be_mount(), which mounts a boot
environment at an arbitrary mountpoint.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Ryan Moeller <ryan@ixsystems.com>
Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
Closes openzfs#9833
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Userspace user space functionality Status: Accepted Ready to integrate (reviewed, tested) Type: Feature Feature request or new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants