openzfs
create zap for root vdev
Create a ZAP for the root vdev and the functionality to set/get properties on it.
To set a user property on the root vdev:
zpool set user:prop=666 <pool> root-0
The functionality already exists to set a property on a top-level vdev by specifying <type>-<id> (e.g., mirror-0). The changes were minimal to enable this pattern for the root vdev since the root vdev is of type 'root' with an id of '0'.
Motivation and Context
These changes don't enable vdev property inheritance, however, this is a step towards achieving that.
Description
Create a ZAP for the root vdev and store the ZAP in spa_t as spa_root_vdev_zap
Modify libzutil's for_each_vdev_cb() to iterate over the root vdev. Previously, the root vdev was explicitly being filtered out, I looked around a bit to see why, I didn't find a solid answer - perhaps someone may know.
How Has This Been Tested?
manual testing
test0: - import pool - set user property on the root vdev - export pool - import pool and verify property is set
test1: - use pool from test0 - export the pool - unload openzfs kernel module with root vdev patch - load openzfs kernel module that doesnt have root vdev zap patch - verify the pool still imports - export the pool
Types of changes
- [ ] Bug fix (non-breaking change which fixes an issue)
- [X] 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:
- [X] My code follows the OpenZFS code style requirements.
- [ ] I have updated the documentation accordingly.
- [X] 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.
- [X] All commit messages are properly formatted and contain
Signed-off-by
It's not super obvious what's failing in the test but, I'm guessing it has something to do with the zap being leaked:
01:47:25.83 MOS object 65 (zap) leaked
01:47:25.83 Verified large_blocks feature refcount of 0 is correct
01:47:25.83 Verified large_dnode feature refcount of 0 is correct
01:47:25.83 Verified sha512 feature refcount of 0 is correct
01:47:25.83 Verified skein feature refcount of 0 is correct
01:47:25.83 Verified edonr feature refcount of 0 is correct
01:47:25.83 Verified userobj_accounting feature refcount of 1 is correct
01:47:25.83 Verified encryption feature refcount of 0 is correct
01:47:25.83 Verified project_quota feature refcount of 1 is correct
01:47:25.83 Verified redaction_bookmarks feature refcount of 0 is correct
01:47:25.83 Verified redacted_datasets feature refcount of 0 is correct
01:47:25.83 Verified bookmark_written feature refcount of 0 is correct
01:47:25.83 Verified livelist feature refcount of 0 is correct
01:47:25.83 Verified zstd_compress feature refcount of 0 is correct
01:47:25.83 Verified zilsaxattr feature refcount of 0 is correct
01:47:25.83 Verified blake3 feature refcount of 0 is correct
I'll dig into it and see what I can figure out.
There's one thing that concerns me that the root vdev ZAP is not being handled correctly.
I've observed that the root vdev ZAP is still leaked when running the vdev_zaps_007 test...at the moment this test is disabled because the spacelog map is also leaked causing the test to fail - for what its worth, I understand why the spacelog map is being leaked. The vdev_zaps_007 tests pool splitting and the transferring of ZAP's.
I mention this to see if someone could give the ZAP handling extra scrutiny to see if I've missed a basic understanding of how ZAP's are supposed to be created/handled.
If anyone has any thoughts...let me know if this is a valid concern or not - thanks.
I ended up adding the root vdev ZAP to the AVZ and creating a feature flag, com.klarasystems@avz_v2.
I think this approach is cleaner and aligns with the reason why the AVZ was introduced in the first place. I'll note that this change would have been backwards compatible for !DEBUG builds.
I've added tests for get/set a property on the root vdev and to verify the ZAP is created for the root vdev.
