Skip to content

zcp: get_prop: fix encryptionroot and encryption #17280

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

snajpa
Copy link
Contributor

@snajpa snajpa commented Apr 27, 2025

Motivation and Context

#12337

Description

It was reported that channel programs' zfs.get_prop doesn't work for
dataset properties encryption and encryptionroot.

They are handled in get_special_prop due to the need to call
dsl_dataset_crypt_stats to load those dsl props.

@grahamc already worked on a testcase for this (#12335), we could revive and merge that PR now

How Has This Been Tested?

Reproducer in #12337

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:

@amotin amotin added the Status: Code Review Needed Ready for review and testing label Apr 29, 2025
@snajpa snajpa force-pushed the zcp-props-fix branch 3 times, most recently from de799a2 to d8175b2 Compare May 21, 2025 00:25
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.

It would be great to include @grahamc's test case if he doesn't mind. Please just keep it as a separate commit on top of yours so we can properly attribute it.

It was reported that channel programs' zfs.get_prop doesn't work for
dataset properties encryption and encryptionroot.

They are handled in get_special_prop due to the need to call
dsl_dataset_crypt_stats to load those dsl props.

Signed-off-by: Pavel Snajdr <[email protected]>
@snajpa
Copy link
Contributor Author

snajpa commented May 21, 2025

Let's please wait for the added test's result, just to be sure :)

It appears that the current behavior is broken:

* 'encryption' always being "off"
* 'encryptionroot' always being null

Signed-off-by: Graham Christensen <[email protected]>

Merging notes from snajpa:

- added SPDX tags
- rebased (tests/zfs-tests/tests/Makefile.am)

Signed-off-by: Pavel Snajdr <[email protected]>
@snajpa
Copy link
Contributor Author

snajpa commented May 21, 2025

Ah, the checkstyle - license tags - added and pushed

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels May 21, 2025
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