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.

Sign up for GitHub

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

Jump to bottom

Introduce getting holds and listing bookmarks through ZCP #7902

Merged
merged 1 commit into from
Aug 12, 2019
Merged

Introduce getting holds and listing bookmarks through ZCP #7902

merged 1 commit into from
Aug 12, 2019

Conversation

sdimitro
Copy link
Contributor

Consumers of ZFS Channel Programs can now list bookmarks,
and get holds from datasets. A minor-refactoring was also
applied to distinguish between user and system properties
in ZCP.

How Has This Been Tested?

New tests added to ZTS.
Has been in DelphixOS(illumos) for more than a year.

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.
  • All new and existing tests passed.
  • All commit messages are properly formatted and contain Signed-off-by.
  • Change has been approved by a ZFS on Linux member.

@sdimitro
Copy link
Contributor Author

These changes are based on top of redacted send receive and should be open for a clean review once that lands first.

@ahrens ahrens added the Type: Feature Feature request or new feature label Sep 14, 2018
@behlendorf behlendorf added the Status: Work in Progress Not yet ready for general review label Sep 19, 2018
@ahrens
Copy link
Member

ahrens commented Sep 21, 2018

I think this is ready to go, but it's labeled "work in progress". Is there something else we're looking for here? I checked one of the failed tests and it has:

Tests with results other than PASS that are unexpected:
    FAIL cli_root/zfs_receive/zfs_receive_raw_incremental (expected PASS)
    FAIL cli_root/zfs_rename/zfs_rename_004_neg (expected PASS)
    FAIL cli_root/zfs_send/zfs_send-b (expected PASS)
    FAIL cli_root/zpool_get/zpool_get_002_pos (expected PASS)
    FAIL libzfs/libzfs_input (expected PASS)
    FAIL rsend/send-cD (expected PASS)

Which I'm not sure if they are related to this change.

@sdimitro
Copy link
Contributor Author

@ahrens This work is based on top of Redacted Send/Receive so that should go in first before this one is applied.

@behlendorf behlendorf added Status: Blocked Depends on another pending change and removed Status: Work in Progress Not yet ready for general review labels Sep 25, 2018
@sdimitro sdimitro removed the Status: Blocked Depends on another pending change label Jul 16, 2019
@sdimitro
Copy link
Contributor Author

Now that Redacted Send/Receive is in, we should get this in. I'm rebasing this work to rerun the tests

@sdimitro sdimitro requested a review from pcd1193182 July 17, 2019 16:47
@sdimitro
Copy link
Contributor Author

sdimitro commented Jul 17, 2019 •
edited
Loading

@ahrens @pcd1193182 this looks like it can get reviewed now that redacted send/receive is in and the tests pass.

@ahrens ahrens added the Status: Code Review Needed Ready for review and testing label Jul 17, 2019
@ahrens ahrens requested a review from shartse July 17, 2019 18:12
@sdimitro sdimitro force-pushed the redacted branch 2 times, most recently from 4e6c17b to 2b38299 Compare July 30, 2019 15:07
pcd1193182
pcd1193182 reviewed Jul 30, 2019
View reviewed changes
tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.list_bookmarks.ksh

# Can't have bookmarks on snapshots, only on filesystems
log_mustnot_program $TESTPOOL - <<-EOF
zfs.list.bookmarks("$TESTPOOL/$TESTFS@$TESTSNAP")
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this be a possible future enhancement? You could filter by the txg of the bookmark to find ones that match the snapshot.

@sdimitro sdimitro force-pushed the redacted branch 4 times, most recently from f4a3536 to 31744cf Compare July 31, 2019 20:19
@dankimmel @sdimitro
Introduce getting holds and listing bookmarks through ZCP
7e11741 
Consumers of ZFS Channel Programs can now list bookmarks,
and get holds from datasets. A minor-refactoring was also
applied to distinguish between user and system properties
in ZCP.

Reviewed by: Matt Ahrens <[email protected]>
Reviewed by: Serapheim Dimitropoulos <[email protected]>
Ported-by: Serapheim Dimitropoulos <[email protected]>
OpenZFS-issue: https://illumos.org/issues/8862
Signed-off-by: Dan Kimmel <[email protected]>
@ahrens ahrens requested a review from behlendorf August 5, 2019 17:53
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Aug 11, 2019
@behlendorf behlendorf merged commit 3b9edd7 into openzfs:master Aug 12, 2019
@problame problame mentioned this pull request Sep 15, 2019
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tonyhutter tonyhutter tonyhutter left review comments

@pcd1193182 pcd1193182 pcd1193182 approved these changes

@behlendorf behlendorf behlendorf approved these changes

@ahrens ahrens ahrens approved these changes

@shartse shartse Awaiting requested review from shartse

Assignees
No one assigned
Labels
Status: Accepted Ready to integrate (reviewed, tested) Type: Feature Feature request or new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sdimitro @ahrens @pcd1193182 @behlendorf @tonyhutter @dankimmel