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 <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: loli10K <[email protected]>
Closes openzfs#4843 
Closes openzfs#6339

Author: loli10K

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
 	 */

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);

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

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,18 +51,25 @@
 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
 {
 	if datasetexists $clone; then
 		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

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

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>

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