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 <dinatale2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: loli10K <ezomori.nozomu@gmail.com>
Closes #4843
Closes #6339

diff --git a/lib/libzfs/libzfs_dataset.c b/lib/libzfs/libzfs_dataset.c
index d6e85024d55014ac9502218c1f69b045a57c6808..b37c89e41620e8176b6b661ced336fa12798a891 100644 (file)
--- 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 d195eded76dc818f2f5da68677ce53915449ba41..21fefe57fbbe7c0b3d63528ebacfe18606c92cc5 100644 (file)
--- 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 88594c20109c9960d92aee1d5eb1b788cc4c0052..b6ac41dfa987ba44f6f21b31f974d573dae67b30 100644 (file)
--- 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 cedfa676a1675ef81e82355a70f747a5a1d84b2f..286c14ac12d9722288aea5e5ef4ee9471e064856 100755 (executable)
--- 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 8c33b58b387640289be1010861c03ae592e610ea..9a8f38dc24439d1c068d97ee6077a716baca37fc 100644 (file)
--- 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 f408d712052aaec38664067c4046165292fd77b2..9b8fb6b0edc104a3a0ae1e6a67d133459a6fc630 100644 (file)
--- 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 cafa74366d096adf3dd84854011fed43470c39d4..b1438e8663e3e00da06821049b8f139d95f18e2e 100755 (executable)
--- 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