Revert "Fix "snapdev" property inheritance behaviour"

This reverts commit 959f56b993.
An issue was uncovered by the new zvol_misc_snapdev test case
which needs to be investigated and resolved.

Reviewed-by: loli10K <ezomori.nozomu@gmail.com>
Reviewed-by: George Melikov <mail@gmelikov.ru>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #6174 
Issue #6131
This commit is contained in:
Brian Behlendorf 2017-05-26 11:40:44 -07:00 committed by GitHub
parent bda77af11c
commit 261c013fbf
5 changed files with 59 additions and 230 deletions

View File

@ -2761,12 +2761,54 @@ 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) {
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 {
/*
* Only check this in the non-received case. We want to allow
* 'inherit -S' to revert non-inheritable properties like quota
@ -2777,49 +2819,8 @@ zfs_ioc_inherit_prop(zfs_cmd_t *zc)
return (SET_ERROR(EINVAL));
}
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);
/* property name has been validated by zfs_secpolicy_inherit_prop() */
return (dsl_prop_inherit(zc->zc_name, zc->zc_value, source));
}
static int

View File

@ -2216,18 +2216,20 @@ 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);
if (dsl_prop_get_int_ds(ds, "snapdev", &snapdev) != 0)
return (0);
task = zvol_task_alloc(ZVOL_ASYNC_SET_SNAPDEV, dsname, NULL, snapdev);
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 (task == NULL)
return (0);
@ -2237,11 +2239,7 @@ zvol_set_snapdev_sync_cb(dsl_pool_t *dp, dsl_dataset_t *ds, void *arg)
}
/*
* 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.
* Traverse all child snapshot datasets and apply snapdev appropriately.
*/
static void
zvol_set_snapdev_sync(void *arg, dmu_tx_t *tx)
@ -2249,19 +2247,10 @@ 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);

View File

@ -564,8 +564,7 @@ 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_snapdev']
'zvol_misc_004_pos', 'zvol_misc_005_neg', 'zvol_misc_006_pos']
[tests/functional/zvol/zvol_swap]
tests = ['zvol_swap_001_pos', 'zvol_swap_002_pos', 'zvol_swap_003_pos',

View File

@ -7,5 +7,4 @@ 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_snapdev.ksh
zvol_misc_006_pos.ksh

View File

@ -1,159 +0,0 @@
#!/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 <ezomori.nozomu@gmail.com>. 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"
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"
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
block_device_wait
blockdev_exists $SNAPDEV
log_must zfs set snapdev=hidden $ZVOL
block_device_wait
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
block_device_wait
blockdev_exists $SNAPDEV
log_must zfs destroy $SNAP
block_device_wait
check_missing $SNAPDEV
# 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
block_device_wait
blockdev_exists $SNAPDEV
# 3.2 Check snapdev=hidden case
log_must zfs set snapdev=hidden $TESTPOOL
verify_inherited 'snapdev' 'hidden' $ZVOL $TESTPOOL
block_device_wait
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
block_device_wait
check_missing $SUBSNAPDEV
blockdev_exists $SNAPDEV
log_pass "ZFS volume property 'snapdev' works as expected"