Skip to content

Support inheriting properties in channel programs - #9738

Merged
behlendorf merged 1 commit into
openzfs:masterfrom
jasonbking:inherit_zcp
Jan 23, 2020
Merged

behlendorf merged 1 commit into
openzfs:masterfrom
jasonbking:inherit_zcp

Conversation

@jasonbking

@jasonbking jasonbking commented Dec 17, 2019 •

Copy link
Copy Markdown
Contributor

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

codecov Bot commented Dec 18, 2019 •

Copy link
Copy Markdown

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.

@behlendorf behlendorf left a comment •

Copy link
Copy Markdown
Contributor

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.

Comment thread man/man8/zfs-program.8 Outdated
Comment thread man/man8/zfs-program.8 Outdated
Comment thread module/zfs/zcp_synctask.c Outdated
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Dec 23, 2019
Comment thread module/zfs/zcp_synctask.c Outdated
Comment thread module/zfs/zcp_synctask.c Outdated

@behlendorf behlendorf left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread man/man8/zfs-program.8 Outdated
Comment thread man/man8/zfs-program.8 Outdated
@behlendorf
behlendorf requested review from ahrens and sdimitro 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 <jason.king@joyent.com>
@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

behlendorf commented Jan 17, 2020 •

Copy link
Copy Markdown
Contributor

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

ahrens commented Jan 23, 2020

Copy link
Copy Markdown
Member

@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 <behlendorf1@llnl.gov>
Signed-off-by: Jason King <jason.king@joyent.com>
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