Fix "snapdev" property issues
When inheriting the "snapdev" property to we don't always call
zfs_prop_set_special(): this prevents device nodes from being created in
certain situations. Because "snapdev" is the only *special* property
that is also inheritable we need to call zfs_prop_set_special() even
when we're not reverting it to the received value ('zfs inherit -S').
Additionally, fix a NULL pointer dereference accidentally introduced in
5559ba0 that can be triggered when setting the "snapdev" property to
the value "hidden" twice.
Finally, add a new test case "zvol_misc_snapdev" to the ZFS Test Suite.
Reviewed by: Boris Protopopov <bprotopopov@hotmail.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: loli10K <ezomori.nozomu@gmail.com>
Closes #6131
Closes #6175
Closes #6176
This commit is contained in:
LOLi
Brian Behlendorf
parent
b870c4b5d7
commit
92aceb2a7e
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 <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"
|
||||
|
||||
# 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"
|
||||
Loading…
Reference in New Issue