diff --git a/cmd/zfs/zfs_main.c b/cmd/zfs/zfs_main.c index 4c21c92bcd2f..e8e403dbf16a 100644 --- a/cmd/zfs/zfs_main.c +++ b/cmd/zfs/zfs_main.c @@ -410,11 +410,14 @@ get_usage(zfs_help_t idx) "\tproject -C [-k] [-r] \n" "\tproject [-p id] [-r] [-s] \n")); case HELP_HOLD: - return (gettext("\thold [-r] ...\n")); + return (gettext("\thold [-r] " + " ...\n")); case HELP_HOLDS: - return (gettext("\tholds [-rHp] ...\n")); + return (gettext("\tholds [-rHp] " + " ...\n")); case HELP_RELEASE: - return (gettext("\trelease [-r] ...\n")); + return (gettext("\trelease [-r] " + " ...\n")); case HELP_DIFF: return (gettext("\tdiff [-FHth] " "[snapshot|filesystem]\n")); @@ -6677,6 +6680,63 @@ zfs_do_unallow(int argc, char **argv) return (zfs_do_allow_unallow_impl(argc, argv, B_TRUE)); } +typedef struct dataset_hold_arg { + nvlist_t *nvl; + const char *tag; + boolean_t holding; + boolean_t recursive; +} dataset_hold_arg_t; + +static int +zfs_dataset_hold_rele_one(zfs_handle_t *zhp, void *arg) +{ + dataset_hold_arg_t *ha = arg; + int rv = 0; + const char *name = zfs_get_name(zhp); + + if (ha->holding) { + fnvlist_add_string(ha->nvl, name, ha->tag); + } else { + nvlist_t *torelease = fnvlist_alloc(); + fnvlist_add_boolean(torelease, ha->tag); + fnvlist_add_nvlist(ha->nvl, name, torelease); + fnvlist_free(torelease); + } + + if (ha->recursive) + rv = zfs_iter_filesystems_v2(zhp, 0, + zfs_dataset_hold_rele_one, ha); + zfs_close(zhp); + return (rv); +} + +static int +zfs_dataset_hold_rele_recurse(zfs_handle_t *zhp, const char *tag, + boolean_t holding, boolean_t recursive) +{ + dataset_hold_arg_t ha = { + .nvl = fnvlist_alloc(), + .tag = tag, + .holding = holding, + .recursive = recursive, + }; + int ret; + + ret = zfs_dataset_hold_rele_one(zfs_handle_dup(zhp), &ha); + if (ret != 0) { + fnvlist_free(ha.nvl); + return (ret); + } + + if (holding) + ret = zfs_hold_nvl(zhp, -1, ha.nvl); + else + ret = lzc_release(ha.nvl, NULL); + + fnvlist_free(ha.nvl); + return (ret); +} + static int zfs_do_hold_rele_impl(int argc, char **argv, boolean_t holding) { @@ -6725,9 +6785,16 @@ zfs_do_hold_rele_impl(int argc, char **argv, boolean_t holding) delim = strchr(path, '@'); if (delim == NULL) { - (void) fprintf(stderr, - gettext("'%s' is not a snapshot\n"), path); - ++errors; + zhp = zfs_open(g_zfs, path, + ZFS_TYPE_FILESYSTEM | ZFS_TYPE_VOLUME); + if (zhp == NULL) { + ++errors; + continue; + } + if (zfs_dataset_hold_rele_recurse(zhp, tag, holding, + recursive) != 0) + ++errors; + zfs_close(zhp); continue; } (void) strlcpy(parent, path, MIN(sizeof (parent), @@ -6865,7 +6932,7 @@ holds_callback(zfs_handle_t *zhp, void *data) const char *zname = zfs_get_name(zhp); size_t znamelen = strlen(zname); - if (cbp->cb_recursive) { + if (cbp->cb_recursive && cbp->cb_snapname != NULL) { const char *snapname; const char *delim = strchr(zname, '@'); if (delim == NULL) @@ -6893,9 +6960,9 @@ holds_callback(zfs_handle_t *zhp, void *data) } /* - * zfs holds [-rHp] ... + * zfs holds [-rHp] ... * - * -r Lists holds that are set on the named snapshots recursively. + * -r Lists holds that are set on the named datasets recursively. * -H Scripted mode; elide headers and separate columns by tabs. * -p Display values in parsable (literal) format. */ @@ -6955,9 +7022,14 @@ zfs_do_holds(int argc, char **argv) delim = strchr(snapshot, '@'); if (delim == NULL) { - (void) fprintf(stderr, - gettext("'%s' is not a snapshot\n"), snapshot); - errors = B_TRUE; + cb.cb_recursive = recursive; + cb.cb_snapname = NULL; + cb.cb_nvlp = &nvl; + ret = zfs_for_each(1, argv + i, flags, + ZFS_TYPE_FILESYSTEM | ZFS_TYPE_VOLUME, + NULL, NULL, limit, holds_callback, &cb); + if (ret != 0) + errors = B_TRUE; continue; } snapname = delim + 1; diff --git a/man/man8/zfs-hold.8 b/man/man8/zfs-hold.8 index a877e428f88b..c0326b957ffb 100644 --- a/man/man8/zfs-hold.8 +++ b/man/man8/zfs-hold.8 @@ -30,26 +30,26 @@ .\" Copyright 2018 Nexenta Systems, Inc. .\" Copyright 2019 Joyent, Inc. .\" -.Dd November 8, 2022 +.Dd May 9, 2026 .Dt ZFS-HOLD 8 .Os . .Sh NAME .Nm zfs-hold -.Nd hold ZFS snapshots to prevent their removal +.Nd hold datasets to prevent their removal .Sh SYNOPSIS .Nm zfs .Cm hold .Op Fl r -.Ar tag Ar snapshot Ns … +.Ar tag Ar filesystem Ns | Ns Ar volume Ns | Ns Ar snapshot Ns … .Nm zfs .Cm holds .Op Fl rHp -.Ar snapshot Ns … +.Ar filesystem Ns | Ns Ar volume Ns | Ns Ar snapshot Ns … .Nm zfs .Cm release .Op Fl r -.Ar tag Ar snapshot Ns … +.Ar tag Ar filesystem Ns | Ns Ar volume Ns | Ns Ar snapshot Ns … . .Sh DESCRIPTION .Bl -tag -width "" @@ -57,34 +57,34 @@ .Nm zfs .Cm hold .Op Fl r -.Ar tag Ar snapshot Ns … +.Ar tag Ar filesystem Ns | Ns Ar volume Ns | Ns Ar snapshot Ns … .Xc Adds a single reference, named with the .Ar tag -argument, to the specified snapshots. -Each snapshot has its own tag namespace, and tags must be unique within that +argument, to the specified datasets. +Each target has its own tag namespace, and tags must be unique within that space. .Pp -If a hold exists on a snapshot, attempts to destroy that snapshot by using the +If a hold exists on a dataset, attempts to destroy it with .Nm zfs Cm destroy -command return +return .Sy EBUSY . .Bl -tag -width "-r" .It Fl r -Specifies that a hold with the given tag is applied recursively to the snapshots -of all descendent file systems. +Apply the hold recursively to the named target and all descendent +file systems +.Pq for snapshots, to the same-named snapshot of each descendent . .El .It Xo .Nm zfs .Cm holds .Op Fl rHp -.Ar snapshot Ns … +.Ar filesystem Ns | Ns Ar volume Ns | Ns Ar snapshot Ns … .Xc -Lists all existing user references for the given snapshot or snapshots. +Lists all existing user references for the given datasets. .Bl -tag -width "-r" .It Fl r -Lists the holds that are set on the named descendent snapshots, in addition to -listing the holds on the named snapshot. +List holds set on the named target and on all descendent file systems. .It Fl H Do not print headers, use tab-delimited output. .It Fl p @@ -94,20 +94,20 @@ Prints holds timestamps as Unix epoch timestamps. .Nm zfs .Cm release .Op Fl r -.Ar tag Ar snapshot Ns … +.Ar tag Ar filesystem Ns | Ns Ar volume Ns | Ns Ar snapshot Ns … .Xc Removes a single reference, named with the .Ar tag -argument, from the specified snapshot or snapshots. -The tag must already exist for each snapshot. -If a hold exists on a snapshot, attempts to destroy that snapshot by using the +argument, from the specified datasets. +The tag must already exist for each target. +If a hold exists on a dataset, attempts to destroy it with .Nm zfs Cm destroy -command return +return .Sy EBUSY . .Bl -tag -width "-r" .It Fl r -Recursively releases a hold with the given tag on the snapshots of all -descendent file systems. +Recursively release the hold from the named target and all descendent +file systems. .El .El . diff --git a/man/man8/zfs.8 b/man/man8/zfs.8 index ba535c0f4561..15435c2b60f8 100644 --- a/man/man8/zfs.8 +++ b/man/man8/zfs.8 @@ -186,10 +186,10 @@ Creates snapshots with the given names. .It Xr zfs-rollback 8 Roll back the given dataset to a previous snapshot. .It Xr zfs-hold 8 Ns / Ns Xr zfs-release 8 -Add or remove a hold reference to the specified snapshot or snapshots. -If a hold exists on a snapshot, attempts to destroy that snapshot by using the +Add or remove a hold reference on the specified snapshots or datasets. +If a hold exists, attempts to destroy the held target with .Nm zfs Cm destroy -command return +return .Sy EBUSY . .It Xr zfs-diff 8 Display the difference between a snapshot of a given filesystem and another diff --git a/module/zfs/dsl_dataset.c b/module/zfs/dsl_dataset.c index 0d90293d7324..7e7db0208203 100644 --- a/module/zfs/dsl_dataset.c +++ b/module/zfs/dsl_dataset.c @@ -676,13 +676,14 @@ dsl_dataset_hold_obj(dsl_pool_t *dp, uint64_t dsobj, const void *tag, } else { if (zfs_flags & ZFS_DEBUG_SNAPNAMES) err = dsl_dataset_get_snapname(ds); - if (err == 0 && - dsl_dataset_phys(ds)->ds_userrefs_obj != 0) { - err = zap_count( - ds->ds_dir->dd_pool->dp_meta_objset, - dsl_dataset_phys(ds)->ds_userrefs_obj, - &ds->ds_userrefs); - } + } + + if (err == 0 && + dsl_dataset_phys(ds)->ds_userrefs_obj != 0) { + err = zap_count( + ds->ds_dir->dd_pool->dp_meta_objset, + dsl_dataset_phys(ds)->ds_userrefs_obj, + &ds->ds_userrefs); } if (err == 0 && !ds->ds_is_snapshot) { diff --git a/module/zfs/dsl_destroy.c b/module/zfs/dsl_destroy.c index ea01ee586f8b..f83ea2eab405 100644 --- a/module/zfs/dsl_destroy.c +++ b/module/zfs/dsl_destroy.c @@ -776,6 +776,9 @@ dsl_destroy_head_check_impl(dsl_dataset_t *ds, int expected_holds) if (zfs_refcount_count(&ds->ds_longholds) != expected_holds) return (SET_ERROR(EBUSY)); + if (ds->ds_userrefs > 0) + return (SET_ERROR(EBUSY)); + ASSERT0(ds->ds_dir->dd_activity_waiters); mos = ds->ds_dir->dd_pool->dp_meta_objset; @@ -1159,7 +1162,10 @@ dsl_destroy_head_sync_impl(dsl_dataset_t *ds, dmu_tx_t *tx) ASSERT0(dsl_dataset_phys(ds)->ds_next_clones_obj); ASSERT0(dsl_dataset_phys(ds)->ds_props_obj); - ASSERT0(dsl_dataset_phys(ds)->ds_userrefs_obj); + if (dsl_dataset_phys(ds)->ds_userrefs_obj != 0) { + VERIFY0(zap_destroy(mos, + dsl_dataset_phys(ds)->ds_userrefs_obj, tx)); + } dsl_dir_rele(ds->ds_dir, ds); ds->ds_dir = NULL; dmu_object_free_zapified(mos, obj, tx); diff --git a/module/zfs/dsl_userhold.c b/module/zfs/dsl_userhold.c index f91b7a1eb69a..3679337e4806 100644 --- a/module/zfs/dsl_userhold.c +++ b/module/zfs/dsl_userhold.c @@ -118,13 +118,8 @@ dsl_dataset_user_hold_check(void *arg, dmu_tx_t *tx) int error = 0; const char *htag, *name; - /* must be a snapshot */ name = nvpair_name(pair); - if (strchr(name, '@') == NULL) - error = SET_ERROR(EINVAL); - - if (error == 0) - error = nvpair_value_string(pair, &htag); + error = nvpair_value_string(pair, &htag); if (error == 0) error = dsl_dataset_hold(dp, name, FTAG, &ds); @@ -375,9 +370,6 @@ dsl_dataset_user_release_check_one(dsl_dataset_user_release_arg_t *ddura, objset_t *mos; int numholds; - if (!ds->ds_is_snapshot) - return (SET_ERROR(EINVAL)); - if (nvlist_empty(holds)) return (0); diff --git a/module/zfs/zfs_ioctl.c b/module/zfs/zfs_ioctl.c index d31aa80641ce..ff699d31ff22 100644 --- a/module/zfs/zfs_ioctl.c +++ b/module/zfs/zfs_ioctl.c @@ -1389,23 +1389,27 @@ zfs_secpolicy_userspace_upgrade(zfs_cmd_t *zc, nvlist_t *innvl, cred_t *cr) } static int -zfs_secpolicy_hold(zfs_cmd_t *zc, nvlist_t *innvl, cred_t *cr) +zfs_secpolicy_hold_release(nvlist_t *nvl, cred_t *cr, const char *perm) { - (void) zc; nvpair_t *pair; - nvlist_t *holds; int error; - holds = fnvlist_lookup_nvlist(innvl, "holds"); - - for (pair = nvlist_next_nvpair(holds, NULL); pair != NULL; - pair = nvlist_next_nvpair(holds, pair)) { + for (pair = nvlist_next_nvpair(nvl, NULL); pair != NULL; + pair = nvlist_next_nvpair(nvl, pair)) { char fsname[ZFS_MAX_DATASET_NAME_LEN]; - error = dmu_fsname(nvpair_name(pair), fsname); - if (error != 0) - return (error); - error = zfs_secpolicy_write_perms(fsname, - ZFS_DELEG_PERM_HOLD, cr); + const char *name = nvpair_name(pair); + if (dataset_namecheck(name, NULL, NULL) != 0) + return (SET_ERROR(EINVAL)); + if (strchr(name, '@') != NULL) { + error = dmu_fsname(name, fsname); + if (error != 0) + return (error); + } else { + if (strlcpy(fsname, name, sizeof (fsname)) >= + sizeof (fsname)) + return (SET_ERROR(ENAMETOOLONG)); + } + error = zfs_secpolicy_write_perms(fsname, perm, cr); if (error != 0) return (error); } @@ -1413,24 +1417,20 @@ zfs_secpolicy_hold(zfs_cmd_t *zc, nvlist_t *innvl, cred_t *cr) } static int -zfs_secpolicy_release(zfs_cmd_t *zc, nvlist_t *innvl, cred_t *cr) +zfs_secpolicy_hold(zfs_cmd_t *zc, nvlist_t *innvl, cred_t *cr) { (void) zc; - nvpair_t *pair; - int error; + nvlist_t *holds = fnvlist_lookup_nvlist(innvl, "holds"); + return (zfs_secpolicy_hold_release(holds, cr, + ZFS_DELEG_PERM_HOLD)); +} - for (pair = nvlist_next_nvpair(innvl, NULL); pair != NULL; - pair = nvlist_next_nvpair(innvl, pair)) { - char fsname[ZFS_MAX_DATASET_NAME_LEN]; - error = dmu_fsname(nvpair_name(pair), fsname); - if (error != 0) - return (error); - error = zfs_secpolicy_write_perms(fsname, - ZFS_DELEG_PERM_RELEASE, cr); - if (error != 0) - return (error); - } - return (0); +static int +zfs_secpolicy_release(zfs_cmd_t *zc, nvlist_t *innvl, cred_t *cr) +{ + (void) zc; + return (zfs_secpolicy_hold_release(innvl, cr, + ZFS_DELEG_PERM_RELEASE)); } /* diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run index 6e62b552a0db..81ea4db6b49d 100644 --- a/tests/runfiles/common.run +++ b/tests/runfiles/common.run @@ -232,7 +232,7 @@ tests = ['zfs_clone_livelist_condense_and_disable', 'zfs_destroy_007_neg', 'zfs_destroy_008_pos', 'zfs_destroy_009_pos', 'zfs_destroy_010_pos', 'zfs_destroy_011_pos', 'zfs_destroy_012_pos', 'zfs_destroy_013_neg', 'zfs_destroy_014_pos', 'zfs_destroy_015_pos', - 'zfs_destroy_016_pos', 'zfs_destroy_clone_livelist', + 'zfs_destroy_016_pos', 'zfs_destroy_017_neg', 'zfs_destroy_clone_livelist', 'zfs_destroy_dev_removal', 'zfs_destroy_dev_removal_condense'] tags = ['functional', 'cli_root', 'zfs_destroy'] diff --git a/tests/zfs-tests/tests/Makefile.am b/tests/zfs-tests/tests/Makefile.am index 98f392538821..f512828c3752 100644 --- a/tests/zfs-tests/tests/Makefile.am +++ b/tests/zfs-tests/tests/Makefile.am @@ -745,6 +745,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \ functional/cli_root/zfs_destroy/zfs_destroy_014_pos.ksh \ functional/cli_root/zfs_destroy/zfs_destroy_015_pos.ksh \ functional/cli_root/zfs_destroy/zfs_destroy_016_pos.ksh \ + functional/cli_root/zfs_destroy/zfs_destroy_017_neg.ksh \ functional/cli_root/zfs_destroy/zfs_destroy_clone_livelist.ksh \ functional/cli_root/zfs_destroy/zfs_destroy_dev_removal_condense.ksh \ functional/cli_root/zfs_destroy/zfs_destroy_dev_removal.ksh \ diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_destroy/zfs_destroy_017_neg.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_destroy/zfs_destroy_017_neg.ksh new file mode 100755 index 000000000000..3e762bf22b74 --- /dev/null +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_destroy/zfs_destroy_017_neg.ksh @@ -0,0 +1,132 @@ +#!/bin/ksh -p +# SPDX-License-Identifier: CDDL-1.0 +# +# CDDL HEADER START +# +# The contents of this file are subject to the terms of the +# Common Development and Distribution License (the "License"). +# You may not use this file except in compliance with the License. +# +# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE +# or https://opensource.org/licenses/CDDL-1.0. +# See the License for the specific language governing permissions +# and limitations under the License. +# +# When distributing Covered Code, include this CDDL HEADER in each +# file and include the License file at usr/src/OPENSOLARIS.LICENSE. +# If applicable, add the following below this CDDL HEADER, with the +# fields enclosed by brackets "[]" replaced with your own identifying +# information: Portions Copyright [yyyy] [name of copyright owner] +# +# CDDL HEADER END +# + +# +# Copyright (c) 2026, Christos Longros. All rights reserved. +# + +. $STF_SUITE/include/libtest.shlib + +# +# DESCRIPTION: +# Verify that user holds on datasets prevent destruction. +# 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. +# +# STRATEGY: +# 1. Create a filesystem and a volume. +# 2. Place a user hold on the filesystem; verify destroy fails. +# 3. Place a second hold; release one at a time; verify destroy +# only succeeds after both are released. +# 4. Repeat for a volume. +# 5. Create a dataset hierarchy and verify 'zfs hold -r' holds +# every descendent and that 'zfs holds -r' lists holds on every +# descendent. +# 6. Verify 'zfs destroy -r' on the parent fails while a child is +# held and succeeds after the child's hold is released. +# + +verify_runnable "both" + +function cleanup +{ + for tag in hold1 hold2 rhold; do + zfs release -r $tag $TESTPOOL/$TESTFS1 2>/dev/null + zfs release $tag $TESTPOOL/$TESTVOL1 2>/dev/null + done + destroy_dataset $TESTPOOL/$TESTFS1/child 2>/dev/null + destroy_dataset $TESTPOOL/$TESTFS1 + destroy_dataset $TESTPOOL/$TESTVOL1 +} + +log_assert "User holds on datasets prevent destruction" +log_onexit cleanup + +TESTFS1=${TESTFS}-holddataset +TESTVOL1=${TESTVOL}-holddataset + +# Create test datasets +log_must zfs create $TESTPOOL/$TESTFS1 +if is_global_zone; then + log_must zfs create -V 64M $TESTPOOL/$TESTVOL1 +fi + +# Test filesystem holds +log_must zfs hold hold1 $TESTPOOL/$TESTFS1 +log_must eval "zfs holds $TESTPOOL/$TESTFS1 | grep -q hold1" +log_mustnot zfs destroy $TESTPOOL/$TESTFS1 + +# Multiple holds +log_must zfs hold hold2 $TESTPOOL/$TESTFS1 +log_must eval "zfs holds $TESTPOOL/$TESTFS1 | grep -q hold1" +log_must eval "zfs holds $TESTPOOL/$TESTFS1 | grep -q hold2" +log_mustnot zfs destroy $TESTPOOL/$TESTFS1 + +# Release first hold, destroy should still fail +log_must zfs release hold1 $TESTPOOL/$TESTFS1 +log_mustnot zfs destroy $TESTPOOL/$TESTFS1 + +# Release second hold, destroy should succeed +log_must zfs release hold2 $TESTPOOL/$TESTFS1 +log_must zfs destroy $TESTPOOL/$TESTFS1 + +# Test volume holds +if is_global_zone; then + log_must zfs hold hold1 $TESTPOOL/$TESTVOL1 + log_must eval "zfs holds $TESTPOOL/$TESTVOL1 | grep -q hold1" + log_mustnot zfs destroy $TESTPOOL/$TESTVOL1 + log_must zfs release hold1 $TESTPOOL/$TESTVOL1 + log_must zfs destroy $TESTPOOL/$TESTVOL1 +fi + +# Recursive hold on a dataset hierarchy +log_must zfs create $TESTPOOL/$TESTFS1 +log_must zfs create $TESTPOOL/$TESTFS1/child +log_must zfs hold -r rhold $TESTPOOL/$TESTFS1 +log_must eval "zfs holds $TESTPOOL/$TESTFS1 | grep -q rhold" +log_must eval "zfs holds $TESTPOOL/$TESTFS1/child | grep -q rhold" + +# 'zfs holds -r' on a dataset descends into children +log_must eval "[[ \$(zfs holds -r $TESTPOOL/$TESTFS1 | grep -c rhold) -eq 2 ]]" + +# Recursive destroy must fail while any child is held +log_mustnot zfs destroy -r $TESTPOOL/$TESTFS1 + +# Release the child's hold, parent's hold still blocks destroy +log_must zfs release rhold $TESTPOOL/$TESTFS1/child +log_mustnot zfs destroy -r $TESTPOOL/$TESTFS1 + +# Release parent's hold; recursive destroy now succeeds +log_must zfs release rhold $TESTPOOL/$TESTFS1 +log_must zfs destroy -r $TESTPOOL/$TESTFS1 + +# Bookmarks are not valid hold targets +log_must zfs create $TESTPOOL/$TESTFS1 +log_must zfs snapshot $TESTPOOL/$TESTFS1@bmsnap +log_must zfs bookmark $TESTPOOL/$TESTFS1@bmsnap $TESTPOOL/$TESTFS1#bm +log_mustnot zfs hold bhold $TESTPOOL/$TESTFS1#bm +log_must zfs destroy -r $TESTPOOL/$TESTFS1 + +log_pass "User holds on datasets prevent destruction"