Skip to content

Support inheriting properties in channel programs #9738

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
Jan 23, 2020

Conversation

jasonbking
Copy link
Contributor

@jasonbking jasonbking commented Dec 17, 2019

Add zfs.sync.inherit and zfs.check.inherit functions for ZFS channel programs that functions like the zfs inherit command.

Motivation and Context

Joyent has been using this as component of our key management system we've been developing for Triton (as described in the OpenZFS summit presentation). The feature itself seems generally useful in more contexts that it makes sense to contribute it to the broader community.

Description

Functions where added to implement the zfs.sync.inherit and zfs.check.inherit LUA functions, following the form of the existing channel program functions. The check function performs the same checks on the property that the zfs inherit command performs. We did not however implement support for zfs inherit -S, as it wasn't necessary for our uses, though nothing in the implementation should prohibit adding the functionality in the future if desired.

How Has This Been Tested?

This has been running on a SmartOS branch for some time (we want to get wider review before integrating it into the master branch of any repo) using the added functionality. In addition, the added zfs test case runs successfully.

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:

@behlendorf behlendorf added the Type: Feature Feature request or new feature label Dec 17, 2019
@jasonbking jasonbking marked this pull request as ready for review December 17, 2019 23:37
@codecov
Copy link

codecov bot commented Dec 18, 2019

Codecov Report

Merging #9738 into master will increase coverage by <1%.
The diff coverage is 77%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #9738    +/-   ##
========================================
+ Coverage      79%      80%   +<1%     
========================================
  Files         385      385            
  Lines      121615   121645    +30     
========================================
+ Hits        96636    96868   +232     
+ Misses      24979    24777   -202
Flag Coverage Δ
#kernel 80% <85%> (ø) ⬆️
#user 67% <0%> (ø) ⬆️

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 6f48161...76465fd. Read the comment docs.

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.

Thanks for pushing this upstream I'm sure it will be useful.

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Dec 23, 2019
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.

Looks good, and works as intended for me. If you can rebase this on master, which will resolve the make lint warnings, and resolve the minor man page edits this should be good to go.

@behlendorf behlendorf requested review from sdimitro and ahrens January 14, 2020 20:44
@ahrens ahrens requested a review from shartse January 14, 2020 23:12
This adds support in channel programs to inherit properties analogous
to `zfs inherit` by adding `zfs.sync.inherit` and `zfs.check.inherit`
functions to the ZFS LUA API.

Signed-off-by: Jason King <[email protected]>
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jan 17, 2020
@behlendorf
Copy link
Contributor

behlendorf commented Jan 17, 2020

I'd like to get this merged by next Wednesday, please post any comments you may have by then.

@behlendorf behlendorf merged commit e2ef1cb into openzfs:master Jan 23, 2020
@ahrens
Copy link
Member

ahrens commented Jan 23, 2020

@jasonbking Sorry I didn't have time to look at this before it was integrated. The code looks good to me. Thanks for extending zfs channel programs!

jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
This adds support in channel programs to inherit properties analogous
to `zfs inherit` by adding `zfs.sync.inherit` and `zfs.check.inherit`
functions to the ZFS LUA API.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Jason King <[email protected]>
Closes openzfs#9738
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) Type: Feature Feature request or new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants