Restrict cloning with different properties

While technically its not a problem to clone between datasets with
different properties, it might create expectation of new properties
being applied during data move, while actually it won't happen.
For copies and checksum it may mean incorrect safety expectations.
For dedup, compression and special_small_blocks -- performance and
space usage. New zfs_bclone_strict_properties tunable controls it.

Reviewed-by: Rob Norris <robn@despairlabs.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Alexander Motin <alexander.motin@TrueNAS.com>
Closes #18180

man/man4/zfs.4

@@ -1448,6 +1448,10 @@ If this setting is 0, then even if feature@block_cloning is enabled,
 using functions and system calls that attempt to clone blocks will act as
 though the feature is disabled.
 .
+.It Sy zfs_bclone_strict_properties Ns = Ns Sy 1 Ns | Ns 0 Pq int
+Restricts block cloning between datasets with different properties
+(checksum, compression, copies, dedup, or special_small_blocks).
+.
 .It Sy zfs_bclone_wait_dirty Ns = Ns Sy 1 Ns | Ns 0 Pq int
 When set to 1 the FICLONE and FICLONERANGE ioctls will wait for any dirty
 data to be written to disk before proceeding.

module/zfs/zfs_vnops.c

@@ -69,6 +69,12 @@
  */
 int zfs_bclone_enabled = 1;
 
+/*
+ * Restricts block cloning between datasets with different properties
+ * (checksum, compression, copies, dedup, or special_small_blocks).
+ */
+int zfs_bclone_strict_properties = 1;
+
 /*
  * When set to 1 the FICLONE and FICLONERANGE ioctls will wait for any dirty
  * data to be written to disk before proceeding. This ensures that the clone
@@ -1677,6 +1683,21 @@ zfs_clone_range(znode_t *inzp, uint64_t *inoffp, znode_t *outzp,
 		return (SET_ERROR(EXDEV));
 	}
 
+	/*
+	 * Cloning between datasets with different properties is possible,
+	 * but it may cause confusions when copying data between them and
+	 * expecting new properties to apply.
+	 */
+	if (zfs_bclone_strict_properties && inos != outos &&
+	    !inzfsvfs->z_issnap &&
+	    (inos->os_checksum != outos->os_checksum ||
+	    inos->os_compress != outos->os_compress ||
+	    inos->os_copies != outos->os_copies ||
+	    inos->os_dedup_checksum != outos->os_dedup_checksum)) {
+		zfs_exit_two(inzfsvfs, outzfsvfs, FTAG);
+		return (SET_ERROR(EXDEV));
+	}
+
 	error = zfs_verify_zp(inzp);
 	if (error == 0)
 		error = zfs_verify_zp(outzp);
@@ -1758,6 +1779,26 @@ zfs_clone_range(znode_t *inzp, uint64_t *inoffp, znode_t *outzp,
 
 	inblksz = inzp->z_blksz;
 
+	/*
+	 * Cloning between datasets with different special_small_blocks would
+	 * bypass storage tier migration that would occur with a regular copy.
+	 */
+	if (zfs_bclone_strict_properties && inos != outos &&
+	    !inzfsvfs->z_issnap && spa_has_special(dmu_objset_spa(inos))) {
+		uint64_t in_smallblk = inos->os_zpl_special_smallblock;
+		uint64_t out_smallblk = outos->os_zpl_special_smallblock;
+		if (in_smallblk != out_smallblk) {
+			uint64_t min_smallblk = MIN(in_smallblk, out_smallblk);
+			uint64_t max_smallblk = MAX(in_smallblk, out_smallblk);
+			if (min_smallblk < inblksz &&
+			    (inos->os_compress != ZIO_COMPRESS_OFF ||
+			    max_smallblk >= inblksz)) {
+				error = SET_ERROR(EXDEV);
+				goto unlock;
+			}
+		}
+	}
+
 	/*
 	 * We cannot clone into a file with different block size if we can't
 	 * grow it (block size is already bigger, has more than one block, or
@@ -2116,6 +2157,9 @@ ZFS_MODULE_PARAM(zfs_vnops, zfs_vnops_, read_chunk_size, U64, ZMOD_RW,
 ZFS_MODULE_PARAM(zfs, zfs_, bclone_enabled, INT, ZMOD_RW,
 	"Enable block cloning");
 
+ZFS_MODULE_PARAM(zfs, zfs_, bclone_strict_properties, INT, ZMOD_RW,
+	"Restrict cross-dataset cloning with different properties");
+
 ZFS_MODULE_PARAM(zfs, zfs_, bclone_wait_dirty, INT, ZMOD_RW,
 	"Wait for dirty blocks when cloning");
 

tests/zfs-tests/include/tunables.cfg

@@ -110,6 +110,7 @@ VOL_RECURSIVE			vol.recursive			UNSUPPORTED
 VOL_REQUEST_SYNC		vol.request_sync		zvol_request_sync
 VOL_USE_BLK_MQ			UNSUPPORTED			zvol_use_blk_mq
 BCLONE_ENABLED			bclone_enabled			zfs_bclone_enabled
+BCLONE_STRICT_PROPERTIES	bclone_strict_properties	zfs_bclone_strict_properties
 BCLONE_WAIT_DIRTY		bclone_wait_dirty		zfs_bclone_wait_dirty
 DIO_ENABLED			dio_enabled			zfs_dio_enabled
 DIO_STRICT			dio_strict			zfs_dio_strict

tests/zfs-tests/tests/functional/bclone/bclone_crossfs_corner_cases.ksh

@@ -32,6 +32,15 @@ verify_runnable "both"
 
 verify_crossfs_block_cloning
 
+function cleanup
+{
+	log_must zfs inherit compress $TESTSRCFS
+	log_must zfs inherit compress $TESTDSTFS
+	log_must zfs inherit recordsize $TESTSRCFS
+	log_must zfs inherit recordsize $TESTDSTFS
+}
+log_onexit cleanup
+
 log_assert "Verify various corner cases in block cloning across datasets"
 
 # Disable compression to make sure we won't use embedded blocks.

tests/zfs-tests/tests/functional/bclone/bclone_crossfs_corner_cases_limited.ksh

@@ -32,6 +32,15 @@ verify_runnable "both"
 
 verify_crossfs_block_cloning
 
+function cleanup
+{
+	log_must zfs inherit compress $TESTSRCFS
+	log_must zfs inherit compress $TESTDSTFS
+	log_must zfs inherit recordsize $TESTSRCFS
+	log_must zfs inherit recordsize $TESTDSTFS
+}
+log_onexit cleanup
+
 log_assert "Verify various corner cases in block cloning across datasets"
 
 # Disable compression to make sure we won't use embedded blocks.

tests/zfs-tests/tests/functional/bclone/bclone_crossfs_data.ksh

@@ -32,6 +32,13 @@ verify_runnable "both"
 
 verify_crossfs_block_cloning
 
+function cleanup
+{
+	log_must zfs inherit compress $TESTSRCFS
+	log_must zfs inherit compress $TESTDSTFS
+}
+log_onexit cleanup
+
 log_assert "Verify block cloning properly clones regular files across datasets"
 
 # Disable compression to make sure we won't use embedded blocks.

tests/zfs-tests/tests/functional/bclone/bclone_crossfs_embedded.ksh

@@ -32,6 +32,13 @@ verify_runnable "both"
 
 verify_crossfs_block_cloning
 
+function cleanup
+{
+	log_must zfs inherit compress $TESTSRCFS
+	log_must zfs inherit compress $TESTDSTFS
+}
+log_onexit cleanup
+
 log_assert "Verify block cloning properly clones small files (with embedded blocks) across datasets"
 
 # Enable ZLE compression to make sure what is the maximum amount of data we

tests/zfs-tests/tests/functional/bclone/bclone_diffprops_all.ksh

@@ -33,8 +33,27 @@ verify_runnable "both"
 
 verify_crossfs_block_cloning
 
+save_tunable BCLONE_STRICT_PROPERTIES
+
+function cleanup
+{
+	restore_tunable BCLONE_STRICT_PROPERTIES
+	log_must zfs inherit checksum $TESTSRCFS
+	log_must zfs inherit compress $TESTSRCFS
+	log_must zfs inherit copies $TESTSRCFS
+	log_must zfs inherit recordsize $TESTSRCFS
+	log_must zfs inherit checksum $TESTDSTFS
+	log_must zfs inherit compress $TESTDSTFS
+	log_must zfs inherit copies $TESTDSTFS
+	log_must zfs inherit recordsize $TESTDSTFS
+}
+log_onexit cleanup
+
 log_assert "Verify block cloning across datasets with different properties"
 
+# Disable strict property checking to allow cross-dataset cloning with different properties
+log_must set_tunable32 BCLONE_STRICT_PROPERTIES 0
+
 log_must zfs set checksum=off $TESTSRCFS
 log_must zfs set compress=off $TESTSRCFS
 log_must zfs set copies=1 $TESTSRCFS
@@ -74,13 +93,4 @@ FILESIZE=$(random_int_between 2 32767)
 FILESIZE=$((FILESIZE * 64))
 bclone_test text $FILESIZE false $TESTSRCDIR $TESTDSTDIR
 
-log_must zfs inherit checksum $TESTSRCFS
-log_must zfs inherit compress $TESTSRCFS
-log_must zfs inherit copies $TESTSRCFS
-log_must zfs inherit recordsize $TESTSRCFS
-log_must zfs inherit checksum $TESTDSTFS
-log_must zfs inherit compress $TESTDSTFS
-log_must zfs inherit copies $TESTDSTFS
-log_must zfs inherit recordsize $TESTDSTFS
-
 log_pass

tests/zfs-tests/tests/functional/bclone/bclone_diffprops_checksum.ksh

@@ -34,8 +34,23 @@ verify_runnable "both"
 
 verify_crossfs_block_cloning
 
+save_tunable BCLONE_STRICT_PROPERTIES
+
+function cleanup
+{
+	restore_tunable BCLONE_STRICT_PROPERTIES
+	log_must zfs inherit checksum $TESTSRCFS
+	log_must zfs inherit compress $TESTSRCFS
+	log_must zfs inherit checksum $TESTDSTFS
+	log_must zfs inherit compress $TESTDSTFS
+}
+log_onexit cleanup
+
 log_assert "Verify block cloning across datasets with different checksum properties"
 
+# Disable strict property checking to allow cross-dataset cloning with different properties
+log_must set_tunable32 BCLONE_STRICT_PROPERTIES 0
+
 log_must zfs set compress=off $TESTSRCFS
 log_must zfs set compress=off $TESTDSTFS
 
@@ -56,7 +71,4 @@ for srcprop in "${checksum_prop_vals[@]}"; do
     done
 done
 
-log_must zfs inherit checksum $TESTSRCFS
-log_must zfs inherit checksum $TESTDSTFS
-
 log_pass

tests/zfs-tests/tests/functional/bclone/bclone_diffprops_compress.ksh

@@ -34,8 +34,21 @@ verify_runnable "both"
 
 verify_crossfs_block_cloning
 
+save_tunable BCLONE_STRICT_PROPERTIES
+
+function cleanup
+{
+	restore_tunable BCLONE_STRICT_PROPERTIES
+	log_must zfs inherit compress $TESTSRCFS
+	log_must zfs inherit compress $TESTDSTFS
+}
+log_onexit cleanup
+
 log_assert "Verify block cloning across datasets with different compression properties"
 
+# Disable strict property checking to allow cross-dataset cloning with different properties
+log_must set_tunable32 BCLONE_STRICT_PROPERTIES 0
+
 for srcprop in "${compress_prop_vals[@]}"; do
     for dstprop in "${compress_prop_vals[@]}"; do
         if [[ $srcprop == $dstprop ]]; then
@@ -53,7 +66,4 @@ for srcprop in "${compress_prop_vals[@]}"; do
     done
 done
 
-log_must zfs inherit compress $TESTSRCFS
-log_must zfs inherit compress $TESTDSTFS
-
 log_pass

tests/zfs-tests/tests/functional/bclone/bclone_diffprops_copies.ksh

@@ -34,8 +34,23 @@ verify_runnable "both"
 
 verify_crossfs_block_cloning
 
+save_tunable BCLONE_STRICT_PROPERTIES
+
+function cleanup
+{
+	restore_tunable BCLONE_STRICT_PROPERTIES
+	log_must zfs inherit copies $TESTSRCFS
+	log_must zfs inherit compress $TESTSRCFS
+	log_must zfs inherit copies $TESTDSTFS
+	log_must zfs inherit compress $TESTDSTFS
+}
+log_onexit cleanup
+
 log_assert "Verify block cloning across datasets with different copies properties"
 
+# Disable strict property checking to allow cross-dataset cloning with different properties
+log_must set_tunable32 BCLONE_STRICT_PROPERTIES 0
+
 log_must zfs set compress=off $TESTSRCFS
 log_must zfs set compress=off $TESTDSTFS
 
@@ -53,7 +68,4 @@ for srcprop in "${copies_prop_vals[@]}"; do
     done
 done
 
-log_must zfs inherit copies $TESTSRCFS
-log_must zfs inherit copies $TESTDSTFS
-
 log_pass

tests/zfs-tests/tests/functional/bclone/bclone_diffprops_recordsize.ksh

@@ -34,8 +34,23 @@ verify_runnable "both"
 
 verify_crossfs_block_cloning
 
+save_tunable BCLONE_STRICT_PROPERTIES
+
+function cleanup
+{
+	restore_tunable BCLONE_STRICT_PROPERTIES
+	log_must zfs inherit recordsize $TESTSRCFS
+	log_must zfs inherit compress $TESTSRCFS
+	log_must zfs inherit recordsize $TESTDSTFS
+	log_must zfs inherit compress $TESTDSTFS
+}
+log_onexit cleanup
+
 log_assert "Verify block cloning across datasets with different recordsize properties"
 
+# Disable strict property checking to allow cross-dataset cloning with different properties
+log_must set_tunable32 BCLONE_STRICT_PROPERTIES 0
+
 log_must zfs set compress=off $TESTSRCFS
 log_must zfs set compress=off $TESTDSTFS
 
@@ -59,7 +74,4 @@ for srcprop in "${bclone_recsize_prop_vals[@]}"; do
     done
 done
 
-log_must zfs inherit recordsize $TESTSRCFS
-log_must zfs inherit recordsize $TESTDSTFS
-
 log_pass

tests/zfs-tests/tests/functional/bclone/bclone_prop_sync.ksh

@@ -34,6 +34,15 @@ verify_runnable "both"
 
 verify_crossfs_block_cloning
 
+function cleanup
+{
+	log_must zfs inherit compress $TESTSRCFS
+	log_must zfs inherit compress $TESTDSTFS
+	log_must zfs inherit sync $TESTSRCFS
+	log_must zfs inherit sync $TESTDSTFS
+}
+log_onexit cleanup
+
 log_assert "Verify block cloning with all sync property settings"
 
 log_must zfs set compress=zle $TESTSRCFS
@@ -64,7 +73,4 @@ for srcprop in "${sync_prop_vals[@]}"; do
     done
 done
 
-log_must zfs inherit sync $TESTSRCFS
-log_must zfs inherit sync $TESTDSTFS
-
 log_pass

tests/zfs-tests/tests/functional/bclone/bclone_samefs_corner_cases.ksh

@@ -30,6 +30,13 @@
 
 verify_runnable "both"
 
+function cleanup
+{
+	log_must zfs inherit compress $TESTSRCFS
+	log_must zfs inherit recordsize $TESTSRCFS
+}
+log_onexit cleanup
+
 log_assert "Verify various corner cases in block cloning within the same dataset"
 
 # Disable compression to make sure we won't use embedded blocks.

tests/zfs-tests/tests/functional/bclone/bclone_samefs_corner_cases_limited.ksh

@@ -30,6 +30,13 @@
 
 verify_runnable "both"
 
+function cleanup
+{
+	log_must zfs inherit compress $TESTSRCFS
+	log_must zfs inherit recordsize $TESTSRCFS
+}
+log_onexit cleanup
+
 log_assert "Verify various corner cases in block cloning within the same dataset"
 
 # Disable compression to make sure we won't use embedded blocks.

tests/zfs-tests/tests/functional/bclone/bclone_samefs_data.ksh

@@ -30,6 +30,12 @@
 
 verify_runnable "both"
 
+function cleanup
+{
+	log_must zfs inherit compress $TESTSRCFS
+}
+log_onexit cleanup
+
 log_assert "Verify block cloning properly clones regular files within the same dataset"
 
 # Disable compression to make sure we won't use embedded blocks.

tests/zfs-tests/tests/functional/bclone/bclone_samefs_embedded.ksh

@@ -30,6 +30,12 @@
 
 verify_runnable "both"
 
+function cleanup
+{
+	log_must zfs inherit compress $TESTSRCFS
+}
+log_onexit cleanup
+
 log_assert "Verify block cloning properly clones small files (with embedded blocks) within the same dataset"
 
 # Enable ZLE compression to make sure what is the maximum amount of data we