From 650258d7c786b8e62ca847a926f6f038cd3e5d94 Mon Sep 17 00:00:00 2001 From: LOLi Date: Fri, 28 Jul 2017 23:12:34 +0200 Subject: [PATCH] zfs promote|rename .../%recv should be an error If we are in the middle of an incremental 'zfs receive', the child .../%recv will exist. If we run 'zfs promote' .../%recv, it will "work", but then zfs gets confused about the status of the new dataset. Attempting to do this promote should be an error. Similarly renaming .../%recv datasets should not be allowed. Reviewed-by: Giuseppe Di Natale Reviewed-by: Brian Behlendorf Signed-off-by: loli10K Closes #4843 Closes #6339 --- lib/libzfs/libzfs_dataset.c | 7 ++++ module/zfs/zfs_ioctl.c | 12 +++++-- tests/zfs-tests/include/libtest.shlib | 35 +++++++++++++++++++ .../zfs_promote/zfs_promote_006_neg.ksh | 15 +++++--- .../cli_root/zfs_rename/zfs_rename.cfg | 1 + .../cli_root/zfs_rename/zfs_rename.kshlib | 5 +++ .../zfs_rename/zfs_rename_004_neg.ksh | 4 +-- 7 files changed, 70 insertions(+), 9 deletions(-) diff --git a/lib/libzfs/libzfs_dataset.c b/lib/libzfs/libzfs_dataset.c index d6e85024d..b37c89e41 100644 --- a/lib/libzfs/libzfs_dataset.c +++ b/lib/libzfs/libzfs_dataset.c @@ -3751,6 +3751,9 @@ zfs_promote(zfs_handle_t *zhp) return (zfs_error(hdl, EZFS_BADTYPE, errbuf)); } + if (!zfs_validate_name(hdl, zhp->zfs_name, zhp->zfs_type, B_TRUE)) + return (zfs_error(hdl, EZFS_INVALIDNAME, errbuf)); + ret = lzc_promote(zhp->zfs_name, snapname, sizeof (snapname)); if (ret != 0) { @@ -4080,6 +4083,10 @@ zfs_rename(zfs_handle_t *zhp, const char *target, boolean_t recursive, (void) snprintf(errbuf, sizeof (errbuf), dgettext(TEXT_DOMAIN, "cannot rename to '%s'"), target); + /* make sure source name is valid */ + if (!zfs_validate_name(hdl, zhp->zfs_name, zhp->zfs_type, B_TRUE)) + return (zfs_error(hdl, EZFS_INVALIDNAME, errbuf)); + /* * Make sure the target name is valid */ diff --git a/module/zfs/zfs_ioctl.c b/module/zfs/zfs_ioctl.c index d195eded7..21fefe57f 100644 --- a/module/zfs/zfs_ioctl.c +++ b/module/zfs/zfs_ioctl.c @@ -3738,9 +3738,12 @@ zfs_ioc_rename(zfs_cmd_t *zc) boolean_t recursive = zc->zc_cookie & 1; char *at; + /* "zfs rename" from and to ...%recv datasets should both fail */ + zc->zc_name[sizeof (zc->zc_name) - 1] = '\0'; zc->zc_value[sizeof (zc->zc_value) - 1] = '\0'; - if (dataset_namecheck(zc->zc_value, NULL, NULL) != 0 || - strchr(zc->zc_value, '%')) + if (dataset_namecheck(zc->zc_name, NULL, NULL) != 0 || + dataset_namecheck(zc->zc_value, NULL, NULL) != 0 || + strchr(zc->zc_name, '%') || strchr(zc->zc_value, '%')) return (SET_ERROR(EINVAL)); at = strchr(zc->zc_name, '@'); @@ -5002,6 +5005,11 @@ zfs_ioc_promote(zfs_cmd_t *zc) char *cp; int error; + zc->zc_name[sizeof (zc->zc_name) - 1] = '\0'; + if (dataset_namecheck(zc->zc_name, NULL, NULL) != 0 || + strchr(zc->zc_name, '%')) + return (SET_ERROR(EINVAL)); + error = dsl_pool_hold(zc->zc_name, FTAG, &dp); if (error != 0) return (error); diff --git a/tests/zfs-tests/include/libtest.shlib b/tests/zfs-tests/include/libtest.shlib index 88594c201..b6ac41dfa 100644 --- a/tests/zfs-tests/include/libtest.shlib +++ b/tests/zfs-tests/include/libtest.shlib @@ -351,6 +351,41 @@ function create_bookmark log_must zfs bookmark $fs_vol@$snap $fs_vol#$bkmark } +# +# Create a temporary clone result of an interrupted resumable 'zfs receive' +# $1 Destination filesystem name. Must not exist, will be created as the result +# of this function along with its %recv temporary clone +# $2 Source filesystem name. Must not exist, will be created and destroyed +# +function create_recv_clone +{ + typeset recvfs="$1" + typeset sendfs="${2:-$TESTPOOL/create_recv_clone}" + typeset snap="$sendfs@snap1" + typeset incr="$sendfs@snap2" + typeset mountpoint="$TESTDIR/create_recv_clone" + typeset sendfile="$TESTDIR/create_recv_clone.zsnap" + + [[ -z $recvfs ]] && log_fail "Recv filesystem's name is undefined." + + datasetexists $recvfs && log_fail "Recv filesystem must not exist." + datasetexists $sendfs && log_fail "Send filesystem must not exist." + + log_must zfs create -o mountpoint="$mountpoint" $sendfs + log_must zfs snapshot $snap + log_must eval "zfs send $snap | zfs recv -u $recvfs" + log_must mkfile 1m "$mountpoint/data" + log_must zfs snapshot $incr + log_must eval "zfs send -i $snap $incr | dd bs=10K count=1 > $sendfile" + log_mustnot eval "zfs recv -su $recvfs < $sendfile" + log_must zfs destroy -r $sendfs + log_must rm -f "$sendfile" + + if [[ $(get_prop 'inconsistent' "$recvfs/%recv") -ne 1 ]]; then + log_fail "Error creating temporary $recvfs/%recv clone" + fi +} + function default_mirror_setup { default_mirror_setup_noexit $1 $2 $3 diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_promote/zfs_promote_006_neg.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_promote/zfs_promote_006_neg.ksh index cedfa676a..286c14ac1 100755 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_promote/zfs_promote_006_neg.ksh +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_promote/zfs_promote_006_neg.ksh @@ -40,6 +40,7 @@ # pool, fs, snapshot,volume # (4) too many arguments. # (5) invalid options +# (6) temporary %recv datasets # # STRATEGY: # 1. Create an array of invalid arguments @@ -50,11 +51,14 @@ verify_runnable "both" snap=$TESTPOOL/$TESTFS@$TESTSNAP +clone=$TESTPOOL/$TESTCLONE +recvfs=$TESTPOOL/recvfs set -A args "" \ "$TESTPOOL/blah" \ "$TESTPOOL" "$TESTPOOL/$TESTFS" "$snap" \ "$TESTPOOL/$TESTVOL" "$TESTPOOL $TESTPOOL/$TESTFS" \ - "$clone $TESTPOOL/$TESTFS" "- $clone" "-? $clone" + "$clone $TESTPOOL/$TESTFS" "- $clone" "-? $clone" \ + "$recvfs/%recv" function cleanup { @@ -62,6 +66,10 @@ function cleanup log_must zfs destroy $clone fi + if datasetexists $recvfs; then + log_must zfs destroy -r $recvfs + fi + if snapexists $snap; then destroy_snapshot $snap fi @@ -70,10 +78,7 @@ function cleanup log_assert "'zfs promote' will fail with invalid arguments. " log_onexit cleanup -snap=$TESTPOOL/$TESTFS@$TESTSNAP -clone=$TESTPOOL/$TESTCLONE -log_must zfs snapshot $snap -log_must zfs clone $snap $clone +create_recv_clone $recvfs typeset -i i=0 while (( i < ${#args[*]} )); do diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename.cfg b/tests/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename.cfg index 8c33b58b3..9a8f38dc2 100644 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename.cfg +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename.cfg @@ -36,3 +36,4 @@ export BS=512 export CNT=2048 export VOL_R_PATH=$ZVOL_RDEVDIR/$TESTPOOL/$TESTVOL export VOLDATA=$TESTDIR2/voldata.rename +export RECVFS=recvfs diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename.kshlib b/tests/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename.kshlib index f408d7120..9b8fb6b0e 100644 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename.kshlib +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename.kshlib @@ -63,6 +63,8 @@ function additional_setup log_must cp $DATA $(get_prop mountpoint $TESTPOOL/$TESTVOL)/$TESTFILE0 fi + # Create temporary %recv clone + create_recv_clone $TESTPOOL/$RECVFS } function rename_dataset # src dest @@ -110,6 +112,9 @@ function cleanup log_must zfs destroy -fR $TESTPOOL/$TESTFS@snapshot fi + if datasetexists $TESTPOOL/$RECVFS; then + log_must zfs destroy -r $TESTPOOL/$RECVFS + fi } function cmp_data #<$1 src data, $2 tgt data> diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename_004_neg.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename_004_neg.ksh index cafa74366..b1438e866 100755 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename_004_neg.ksh +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename_004_neg.ksh @@ -77,8 +77,8 @@ set -A bad_dataset $TESTPOOL/$TESTFS1 $TESTPOOL/$TESTCTR1 \ $TESTPOOL/$TESTFS1 $TESTPOOL/${TESTFS1}%x \ $TESTPOOL/$TESTFS1 $TESTPOOL/${TESTFS1}%p \ $TESTPOOL/$TESTFS1 $TESTPOOL/${TESTFS1}%s \ - $TESTPOOL/$TESTFS@snapshot \ - $TESTPOOL/$TESTFS@snapshot/fs + $TESTPOOL/$TESTFS@snapshot $TESTPOOL/$TESTFS@snapshot/fs \ + $TESTPOOL/$RECVFS/%recv $TESTPOOL/renamed.$$ # # cleanup defined in zfs_rename.kshlib -- 2.40.0