zfs hold: extend to filesystems and volumes#18515
Open
chrislongros wants to merge 3 commits into
Open
Conversation
chrislongros
force-pushed
the
feature/zfs-hold-datasets
branch
from
f85a37d to
f6ce72b
Compare
chrislongros
changed the title
chrislongros
force-pushed
the
feature/zfs-hold-datasets
branch
from
f6ce72b to
9e950c1
Compare
behlendorf
reviewed
May 12, 2026
Contributor
behlendorf
left a comment
behlendorf
left a comment
There was a problem hiding this comment.
It's nice to see this finally implemented! A few comments inline, and I'm sure you already saw but some of the hold tests in ./contrib/pyzfs/libzfs_core/test/test_libzfs_core.py will need to be updated.
12:40:14.53 FAIL: test_hold_invalid_snap_name_2 (libzfs_core.test.test_libzfs_core.ZFSTest)
12:40:14.53 ----------------------------------------------------------------------
12:40:14.53 Traceback (most recent call last):
12:40:14.53 File "/usr/lib/python3.6/site-packages/libzfs_core/test/test_libzfs_core.py", line 3286, in test_hold_invalid_snap_name_2
12:40:14.53 lzc.lzc_hold({snap: b'tag'}, fd)
12:40:14.53 AssertionError: HoldFailure not raised
Contributor
There was a problem hiding this comment.
This comment should be updated too.
| .Sh NAME | ||
| .Nm zfs-hold | ||
| .Nd hold ZFS snapshots to prevent their removal | ||
| .Nd hold ZFS snapshots or datasets to prevent their removal |
Contributor
There was a problem hiding this comment.
I think we should adopt similar phrasing as used in the zfs-get(8) man page. Specifically, ZFS snapshots, volumes, and filesystem are all just different types of datasets. We can refer to this set as "datasets" rather than "ZFS snapshots or datasets", so in this case:
hold datasets to prevent their removal
We should make the same change throughout the man page.
| .Cm hold | ||
| .Op Fl r | ||
| .Ar tag Ar snapshot Ns … | ||
| .Ar tag Ar snapshot Ns | Ns Ar dataset Ns … |
Contributor
There was a problem hiding this comment.
.Ar tag Ar filesystem Ns | Ns Ar volume Ns | Ns Ar snapshot Ns …
And for the usage we should call out each type.
| error = dmu_fsname(nvpair_name(pair), fsname); | ||
| if (error != 0) | ||
| return (error); | ||
| const char *name = nvpair_name(pair); |
Contributor
There was a problem hiding this comment.
Should return EINVAL for bookmarks, the easiest way to check is probably using dataset_namecheck(). zfs_secpolicy_release() will need the same check.
| # Verify that holds can be listed and released on datasets. | ||
| # Verify that destruction succeeds after all holds are released. | ||
| # Verify that 'zfs hold -r' applies to descendent datasets. | ||
| # Verify that recursive destroy fails while a child dataset is held. |
Contributor
There was a problem hiding this comment.
Verify the holds don't work on bookmarks.
chrislongros
force-pushed
the
feature/zfs-hold-datasets
branch
from
d122000 to
6a60624
Compare
Allow zfs hold/release/holds to operate on filesystems and volumes, not only snapshots, so users can protect datasets from accidental destruction with the existing hold mechanism. When a dataset has one or more user holds, zfs destroy returns EBUSY until all are released, matching snapshot behavior. Signed-off-by: Christos Longros <chris.longros@gmail.com>
Co-authored-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Christos Longros <98426896+chrislongros@users.noreply.github.com>
Use filesystem|volume|snapshot in zfs(8) and zfs-hold(8). Adopt "datasets" terminology matching zfs-get(8). Reject bookmarks in zfs_secpolicy_hold/release with EINVAL via dataset_namecheck; covered by zfs_destroy_017_neg. Signed-off-by: Christos Longros <chris.longros@gmail.com>
chrislongros
force-pushed
the
feature/zfs-hold-datasets
branch
from
6a60624 to
88a7776
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation and Context
Extends
zfs holdto filesystems and volumes.Description
Lift the snapshot-only checks in the user-hold code so
zfs hold,zfs release, andzfs holdsaccept filesystems and volumes. When a dataset has user holds,zfs destroyreturnsEBUSYuntil they are released, matching snapshot behavior.How Has This Been Tested?
zfs_destroy_017_negpasses; manual checks confirm hold release on filesystems, volumes, and pool root, pluszfs holds -ron a parent+child hierarchy.Types of changes
Checklist:
Signed-off-by.