diff options
| -rw-r--r-- | module/zfs/zfs_ioctl.c | 97 | ||||
| -rw-r--r-- | module/zfs/zvol.c | 36 | ||||
| -rw-r--r-- | tests/runfiles/linux.run | 3 | ||||
| -rw-r--r-- | tests/zfs-tests/tests/functional/zvol/zvol_misc/Makefile.am | 3 | ||||
| -rwxr-xr-x | tests/zfs-tests/tests/functional/zvol/zvol_misc/zvol_misc_snapdev.ksh | 173 |
5 files changed, 248 insertions, 64 deletions
diff --git a/module/zfs/zfs_ioctl.c b/module/zfs/zfs_ioctl.c index c6a532123..4fda36c7f 100644 --- a/module/zfs/zfs_ioctl.c +++ b/module/zfs/zfs_ioctl.c @@ -2761,54 +2761,12 @@ zfs_ioc_inherit_prop(zfs_cmd_t *zc) zprop_source_t source = (received ? ZPROP_SRC_NONE /* revert to received value, if any */ : ZPROP_SRC_INHERITED); /* explicitly inherit */ + nvlist_t *dummy; + nvpair_t *pair; + zprop_type_t type; + int err; - if (received) { - nvlist_t *dummy; - nvpair_t *pair; - zprop_type_t type; - int err; - - /* - * zfs_prop_set_special() expects properties in the form of an - * nvpair with type info. - */ - if (prop == ZPROP_INVAL) { - if (!zfs_prop_user(propname)) - return (SET_ERROR(EINVAL)); - - type = PROP_TYPE_STRING; - } else if (prop == ZFS_PROP_VOLSIZE || - prop == ZFS_PROP_VERSION) { - return (SET_ERROR(EINVAL)); - } else { - type = zfs_prop_get_type(prop); - } - - VERIFY(nvlist_alloc(&dummy, NV_UNIQUE_NAME, KM_SLEEP) == 0); - - switch (type) { - case PROP_TYPE_STRING: - VERIFY(0 == nvlist_add_string(dummy, propname, "")); - break; - case PROP_TYPE_NUMBER: - case PROP_TYPE_INDEX: - VERIFY(0 == nvlist_add_uint64(dummy, propname, 0)); - break; - default: - nvlist_free(dummy); - return (SET_ERROR(EINVAL)); - } - - pair = nvlist_next_nvpair(dummy, NULL); - if (pair == NULL) { - nvlist_free(dummy); - return (SET_ERROR(EINVAL)); - } - err = zfs_prop_set_special(zc->zc_name, source, pair); - nvlist_free(dummy); - if (err != -1) - return (err); /* special property already handled */ - } else { + if (!received) { /* * Only check this in the non-received case. We want to allow * 'inherit -S' to revert non-inheritable properties like quota @@ -2819,8 +2777,49 @@ zfs_ioc_inherit_prop(zfs_cmd_t *zc) return (SET_ERROR(EINVAL)); } - /* property name has been validated by zfs_secpolicy_inherit_prop() */ - return (dsl_prop_inherit(zc->zc_name, zc->zc_value, source)); + if (prop == ZPROP_INVAL) { + if (!zfs_prop_user(propname)) + return (SET_ERROR(EINVAL)); + + type = PROP_TYPE_STRING; + } else if (prop == ZFS_PROP_VOLSIZE || prop == ZFS_PROP_VERSION) { + return (SET_ERROR(EINVAL)); + } else { + type = zfs_prop_get_type(prop); + } + + /* + * zfs_prop_set_special() expects properties in the form of an + * nvpair with type info. + */ + dummy = fnvlist_alloc(); + + switch (type) { + case PROP_TYPE_STRING: + VERIFY(0 == nvlist_add_string(dummy, propname, "")); + break; + case PROP_TYPE_NUMBER: + case PROP_TYPE_INDEX: + VERIFY(0 == nvlist_add_uint64(dummy, propname, 0)); + break; + default: + err = SET_ERROR(EINVAL); + goto errout; + } + + pair = nvlist_next_nvpair(dummy, NULL); + if (pair == NULL) { + err = SET_ERROR(EINVAL); + } else { + err = zfs_prop_set_special(zc->zc_name, source, pair); + if (err == -1) /* property is not "special", needs handling */ + err = dsl_prop_inherit(zc->zc_name, zc->zc_value, + source); + } + +errout: + nvlist_free(dummy); + return (err); } static int diff --git a/module/zfs/zvol.c b/module/zfs/zvol.c index cff3da8b4..1a86cd3cd 100644 --- a/module/zfs/zvol.c +++ b/module/zfs/zvol.c @@ -2018,7 +2018,7 @@ zvol_remove_minors_impl(const char *name) static void zvol_remove_minor_impl(const char *name) { - zvol_state_t *zv, *zv_next; + zvol_state_t *zv = NULL, *zv_next; if (zvol_inhibit_dev) return; @@ -2049,12 +2049,11 @@ zvol_remove_minor_impl(const char *name) } } + /* Drop zvol_state_lock before calling zvol_free() */ mutex_exit(&zvol_state_lock); - /* - * Drop zvol_state_lock before calling zvol_free() - */ - zvol_free(zv); + if (zv != NULL) + zvol_free(zv); } /* @@ -2224,20 +2223,18 @@ zvol_set_snapdev_check(void *arg, dmu_tx_t *tx) return (error); } +/* ARGSUSED */ static int zvol_set_snapdev_sync_cb(dsl_pool_t *dp, dsl_dataset_t *ds, void *arg) { - zvol_set_snapdev_arg_t *zsda = arg; char dsname[MAXNAMELEN]; zvol_task_t *task; + uint64_t snapdev; dsl_dataset_name(ds, dsname); - dsl_prop_set_sync_impl(ds, zfs_prop_to_name(ZFS_PROP_SNAPDEV), - zsda->zsda_source, sizeof (zsda->zsda_value), 1, - &zsda->zsda_value, zsda->zsda_tx); - - task = zvol_task_alloc(ZVOL_ASYNC_SET_SNAPDEV, dsname, - NULL, zsda->zsda_value); + if (dsl_prop_get_int_ds(ds, "snapdev", &snapdev) != 0) + return (0); + task = zvol_task_alloc(ZVOL_ASYNC_SET_SNAPDEV, dsname, NULL, snapdev); if (task == NULL) return (0); @@ -2247,7 +2244,11 @@ zvol_set_snapdev_sync_cb(dsl_pool_t *dp, dsl_dataset_t *ds, void *arg) } /* - * Traverse all child snapshot datasets and apply snapdev appropriately. + * Traverse all child datasets and apply snapdev appropriately. + * We call dsl_prop_set_sync_impl() here to set the value only on the toplevel + * dataset and read the effective "snapdev" on every child in the callback + * function: this is because the value is not guaranteed to be the same in the + * whole dataset hierarchy. */ static void zvol_set_snapdev_sync(void *arg, dmu_tx_t *tx) @@ -2255,10 +2256,19 @@ zvol_set_snapdev_sync(void *arg, dmu_tx_t *tx) zvol_set_snapdev_arg_t *zsda = arg; dsl_pool_t *dp = dmu_tx_pool(tx); dsl_dir_t *dd; + dsl_dataset_t *ds; + int error; VERIFY0(dsl_dir_hold(dp, zsda->zsda_name, FTAG, &dd, NULL)); zsda->zsda_tx = tx; + error = dsl_dataset_hold(dp, zsda->zsda_name, FTAG, &ds); + if (error == 0) { + dsl_prop_set_sync_impl(ds, zfs_prop_to_name(ZFS_PROP_SNAPDEV), + zsda->zsda_source, sizeof (zsda->zsda_value), 1, + &zsda->zsda_value, zsda->zsda_tx); + dsl_dataset_rele(ds, FTAG); + } dmu_objset_find_dp(dp, dd->dd_object, zvol_set_snapdev_sync_cb, zsda, DS_FIND_CHILDREN); diff --git a/tests/runfiles/linux.run b/tests/runfiles/linux.run index 7e64c6714..674684f1f 100644 --- a/tests/runfiles/linux.run +++ b/tests/runfiles/linux.run @@ -564,7 +564,8 @@ tests = ['zvol_cli_001_pos', 'zvol_cli_002_pos', 'zvol_cli_003_neg'] [tests/functional/zvol/zvol_misc] tests = ['zvol_misc_001_neg', 'zvol_misc_002_pos', 'zvol_misc_003_neg', - 'zvol_misc_004_pos', 'zvol_misc_005_neg', 'zvol_misc_006_pos'] + 'zvol_misc_004_pos', 'zvol_misc_005_neg', 'zvol_misc_006_pos', + 'zvol_misc_snapdev'] [tests/functional/zvol/zvol_swap] tests = ['zvol_swap_001_pos', 'zvol_swap_002_pos', 'zvol_swap_003_pos', diff --git a/tests/zfs-tests/tests/functional/zvol/zvol_misc/Makefile.am b/tests/zfs-tests/tests/functional/zvol/zvol_misc/Makefile.am index b52088ccc..f72970490 100644 --- a/tests/zfs-tests/tests/functional/zvol/zvol_misc/Makefile.am +++ b/tests/zfs-tests/tests/functional/zvol/zvol_misc/Makefile.am @@ -7,4 +7,5 @@ dist_pkgdata_SCRIPTS = \ zvol_misc_003_neg.ksh \ zvol_misc_004_pos.ksh \ zvol_misc_005_neg.ksh \ - zvol_misc_006_pos.ksh + zvol_misc_006_pos.ksh \ + zvol_misc_snapdev.ksh diff --git a/tests/zfs-tests/tests/functional/zvol/zvol_misc/zvol_misc_snapdev.ksh b/tests/zfs-tests/tests/functional/zvol/zvol_misc/zvol_misc_snapdev.ksh new file mode 100755 index 000000000..57002fe65 --- /dev/null +++ b/tests/zfs-tests/tests/functional/zvol/zvol_misc/zvol_misc_snapdev.ksh @@ -0,0 +1,173 @@ +#!/bin/ksh -p +# +# 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 http://www.opensolaris.org/os/licensing. +# 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 2017, loli10K <[email protected]>. All rights reserved. +# + +. $STF_SUITE/include/libtest.shlib +. $STF_SUITE/tests/functional/cli_root/zfs_set/zfs_set_common.kshlib +. $STF_SUITE/tests/functional/zvol/zvol_common.shlib + +# +# DESCRIPTION: +# Verify that ZFS volume property "snapdev" works as intended. +# +# STRATEGY: +# 1. Verify "snapdev" property does not accept invalid values +# 2. Verify "snapdev" adds and removes device nodes when updated +# 3. Verify "snapdev" is inherited correctly +# + +verify_runnable "global" + +function cleanup +{ + datasetexists $VOLFS && log_must zfs destroy -r $VOLFS + datasetexists $ZVOL && log_must zfs destroy -r $ZVOL + log_must zfs inherit snapdev $TESTPOOL + block_device_wait +} + +# +# Verify $device exists and is a block device +# +function blockdev_exists # device +{ + typeset device="$1" + + # we wait here instead of doing it in a wrapper around 'zfs set snapdev' + # because there are other commands (zfs snap, zfs inherit, zfs destroy) + # that can affect device nodes + block_device_wait + + if [[ ! -b "$device" ]]; then + log_fail "$device does not exist as a block device" + fi +} + +# +# Verify $device does not exist +# +function check_missing # device +{ + typeset device="$1" + + # we wait here instead of doing it in a wrapper around 'zfs set snapdev' + # because there are other commands (zfs snap, zfs inherit, zfs destroy) + # that can affect device nodes + block_device_wait + + if [[ -e "$device" ]]; then + log_fail "$device exists when not expected" + fi +} + +# +# Verify $property on $dataset is inherited by $parent and is set to $value +# +function verify_inherited # property value dataset parent +{ + typeset property="$1" + typeset value="$2" + typeset dataset="$3" + typeset parent="$4" + + typeset val=$(get_prop "$property" "$dataset") + typeset src=$(get_source "$property" "$dataset") + if [[ "$val" != "$value" || "$src" != "inherited from $parent" ]] + then + log_fail "Dataset $dataset did not inherit $property properly:"\ + "expected=$value, value=$val, source=$src." + fi + +} + +log_assert "Verify that ZFS volume property 'snapdev' works as expected." +log_onexit cleanup + +VOLFS="$TESTPOOL/volfs" +ZVOL="$TESTPOOL/vol" +SNAP="$ZVOL@snap" +SNAPDEV="${ZVOL_DEVDIR}/$SNAP" +SUBZVOL="$VOLFS/subvol" +SUBSNAP="$SUBZVOL@snap" +SUBSNAPDEV="${ZVOL_DEVDIR}/$SUBSNAP" + +log_must zfs create -o mountpoint=none $VOLFS +log_must zfs create -V $VOLSIZE -s $ZVOL +log_must zfs create -V $VOLSIZE -s $SUBZVOL + +# 1. Verify "snapdev" property does not accept invalid values +typeset badvals=("off" "on" "1" "nope" "-") +for badval in ${badvals[@]} +do + log_mustnot zfs set snapdev="$badval" $ZVOL +done + +# 2. Verify "snapdev" adds and removes device nodes when updated +# 2.1 First create a snapshot then change snapdev property +log_must zfs snapshot $SNAP +log_must zfs set snapdev=visible $ZVOL +blockdev_exists $SNAPDEV +log_must zfs set snapdev=hidden $ZVOL +check_missing $SNAPDEV +log_must zfs destroy $SNAP +# 2.2 First set snapdev property then create a snapshot +log_must zfs set snapdev=visible $ZVOL +log_must zfs snapshot $SNAP +blockdev_exists $SNAPDEV +log_must zfs destroy $SNAP +check_missing $SNAPDEV +# 2.3 Verify setting to the same value multiple times does not lead to issues +log_must zfs snapshot $SNAP +log_must zfs set snapdev=visible $ZVOL +blockdev_exists $SNAPDEV +log_must zfs set snapdev=visible $ZVOL +blockdev_exists $SNAPDEV +log_must zfs set snapdev=hidden $ZVOL +check_missing $SNAPDEV +log_must zfs set snapdev=hidden $ZVOL +check_missing $SNAPDEV +log_must zfs destroy $SNAP + +# 3. Verify "snapdev" is inherited correctly +# 3.1 Check snapdev=visible case +log_must zfs snapshot $SNAP +log_must zfs inherit snapdev $ZVOL +log_must zfs set snapdev=visible $TESTPOOL +verify_inherited 'snapdev' 'visible' $ZVOL $TESTPOOL +blockdev_exists $SNAPDEV +# 3.2 Check snapdev=hidden case +log_must zfs set snapdev=hidden $TESTPOOL +verify_inherited 'snapdev' 'hidden' $ZVOL $TESTPOOL +check_missing $SNAPDEV +# 3.3 Check inheritance on multiple levels +log_must zfs snapshot $SUBSNAP +log_must zfs inherit snapdev $SUBZVOL +log_must zfs set snapdev=hidden $VOLFS +log_must zfs set snapdev=visible $TESTPOOL +verify_inherited 'snapdev' 'hidden' $SUBZVOL $VOLFS +check_missing $SUBSNAPDEV +blockdev_exists $SNAPDEV + +log_pass "ZFS volume property 'snapdev' works as expected" |