Enforce "-F" flag on resuming recv of full/newfs on existing dataset

When receiving full/newfs on existing dataset, then it should be done
with "-F" flag. Its enforced for initial receive in checks done in
zfs_receive_one function of libzfs. Similarly, on resuming full/newfs
recv on existing dataset, it should be done with "-F" flag.

When dataset doesn't exist, then full/new recv is done on newly created
dataset and it's marked INCONSISTENT. But when receiving on existing
dataset, recv is first done on %recv and its marked INCONSISTENT.
Existing dataset is not marked INCONSISTENT. Resume of full/newfs
receive with dataset not INCONSISTENT indicates that its resuming newfs
on existing dataset. So, enforce "-F" flag in this case.

Also return an error from dmu_recv_resume_begin_check() in zfs kernel,
when its resuming full/newfs recv without force.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Chunwei Chen <david.chen@nutanix.com>
Signed-off-by: Jitendra Patidar <jitendra.patidar@nutanix.com>
Closes #13856
Closes #13857

contrib/pyzfs/libzfs_core/_constants.py

@@ -101,6 +101,7 @@ zfs_errno = enum_with_offset(1024, [
         'ZFS_ERR_BADPROP',
         'ZFS_ERR_VDEV_NOTSUP',
         'ZFS_ERR_NOT_USER_NAMESPACE',
+        'ZFS_ERR_RESUME_EXISTS',
     ],
     {}
 )

include/libzfs.h

@@ -152,6 +152,7 @@ typedef enum zfs_error {
 	EZFS_VDEV_NOTSUP,	/* ops not supported for this type of vdev */
 	EZFS_NOT_USER_NAMESPACE,	/* a file is not a user namespace */
 	EZFS_CKSUM,		/* insufficient replicas */
+	EZFS_RESUME_EXISTS,	/* Resume on existing dataset without force */
 	EZFS_UNKNOWN
 } zfs_error_t;
 

include/sys/fs/zfs.h

@@ -1536,6 +1536,7 @@ typedef enum {
 	ZFS_ERR_BADPROP,
 	ZFS_ERR_VDEV_NOTSUP,
 	ZFS_ERR_NOT_USER_NAMESPACE,
+	ZFS_ERR_RESUME_EXISTS,
 } zfs_errno_t;
 
 /*

lib/libzfs/libzfs_sendrecv.c

@@ -4638,6 +4638,33 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const char *tosnap,
 			goto out;
 		}
 
+		/*
+		 * When receiving full/newfs on existing dataset, then it
+		 * should be done with "-F" flag. Its enforced for initial
+		 * receive in previous checks in this function.
+		 * Similarly, on resuming full/newfs recv on existing dataset,
+		 * it should be done with "-F" flag.
+		 *
+		 * When dataset doesn't exist, then full/newfs recv is done on
+		 * newly created dataset and it's marked INCONSISTENT. But
+		 * When receiving on existing dataset, recv is first done on
+		 * %recv and its marked INCONSISTENT. Existing dataset is not
+		 * marked INCONSISTENT.
+		 * Resume of full/newfs receive with dataset not INCONSISTENT
+		 * indicates that its resuming newfs on existing dataset. So,
+		 * enforce "-F" flag in this case.
+		 */
+		if (stream_resumingnewfs &&
+		    !zfs_prop_get_int(zhp, ZFS_PROP_INCONSISTENT) &&
+		    !flags->force) {
+			zfs_close(zhp);
+			zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
+			    "Resuming recv on existing destination '%s'\n"
+			    "must specify -F to overwrite it"), name);
+			err = zfs_error(hdl, EZFS_RESUME_EXISTS, errbuf);
+			goto out;
+		}
+
 		if (stream_wantsnewfs &&
 		    zhp->zfs_dmustats.dds_origin[0]) {
 			zfs_close(zhp);
@@ -5078,6 +5105,19 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const char *tosnap,
 			    "be updated."));
 			(void) zfs_error(hdl, EZFS_BADSTREAM, errbuf);
 			break;
+		case ZFS_ERR_RESUME_EXISTS:
+			cp = strchr(destsnap, '@');
+			if (newfs) {
+				/* it's the containing fs that exists */
+				*cp = '\0';
+			}
+			zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
+			    "Resuming recv on existing dataset without force"));
+			(void) zfs_error_fmt(hdl, EZFS_RESUME_EXISTS,
+			    dgettext(TEXT_DOMAIN, "cannot resume recv %s"),
+			    destsnap);
+			*cp = '@';
+			break;
 		case EBUSY:
 			if (hastoken) {
 				zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,

lib/libzfs/libzfs_util.c

@@ -304,6 +304,9 @@ libzfs_error_description(libzfs_handle_t *hdl)
 	case EZFS_NOT_USER_NAMESPACE:
 		return (dgettext(TEXT_DOMAIN, "the provided file "
 		    "was not a user namespace file"));
+	case EZFS_RESUME_EXISTS:
+		return (dgettext(TEXT_DOMAIN, "Resuming recv on existing "
+		    "dataset without force"));
 	case EZFS_UNKNOWN:
 		return (dgettext(TEXT_DOMAIN, "unknown error"));
 	default:

module/zfs/dmu_recv.c

@@ -1045,13 +1045,24 @@ dmu_recv_resume_begin_check(void *arg, dmu_tx_t *tx)
 		dsflags |= DS_HOLD_FLAG_DECRYPT;
 	}
 
+	boolean_t recvexist = B_TRUE;
 	if (dsl_dataset_hold_flags(dp, recvname, dsflags, FTAG, &ds) != 0) {
 		/* %recv does not exist; continue in tofs */
+		recvexist = B_FALSE;
 		error = dsl_dataset_hold_flags(dp, tofs, dsflags, FTAG, &ds);
 		if (error != 0)
 			return (error);
 	}
 
+	/*
+	 * Resume of full/newfs recv on existing dataset should be done with
+	 * force flag
+	 */
+	if (recvexist && drrb->drr_fromguid == 0 && !drc->drc_force) {
+		dsl_dataset_rele_flags(ds, dsflags, FTAG);
+		return (SET_ERROR(ZFS_ERR_RESUME_EXISTS));
+	}
+
 	/* check that ds is marked inconsistent */
 	if (!DS_IS_INCONSISTENT(ds)) {
 		dsl_dataset_rele_flags(ds, dsflags, FTAG);

tests/runfiles/common.run

@@ -836,9 +836,9 @@ tests = ['recv_dedup', 'recv_dedup_encrypted_zvol', 'rsend_001_pos',
     'rsend_014_pos', 'rsend_016_neg', 'rsend_019_pos', 'rsend_020_pos',
     'rsend_021_pos', 'rsend_022_pos', 'rsend_024_pos', 'rsend_025_pos',
     'rsend_026_neg', 'rsend_027_pos', 'rsend_028_neg', 'rsend_029_neg',
-    'send-c_verify_ratio', 'send-c_verify_contents', 'send-c_props',
-    'send-c_incremental', 'send-c_volume', 'send-c_zstreamdump',
-    'send-c_lz4_disabled', 'send-c_recv_lz4_disabled',
+    'rsend_030_pos', 'send-c_verify_ratio', 'send-c_verify_contents',
+    'send-c_props', 'send-c_incremental', 'send-c_volume',
+    'send-c_zstreamdump', 'send-c_lz4_disabled', 'send-c_recv_lz4_disabled',
     'send-c_mixed_compression', 'send-c_stream_size_estimate',
     'send-c_embedded_blocks', 'send-c_resume', 'send-cpL_varied_recsize',
     'send-c_recv_dedup', 'send-L_toggle', 'send_encrypted_hierarchy',

tests/zfs-tests/tests/Makefile.am

@@ -1780,6 +1780,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \
 	functional/rsend/rsend_027_pos.ksh \
 	functional/rsend/rsend_028_neg.ksh \
 	functional/rsend/rsend_029_neg.ksh \
+	functional/rsend/rsend_030_pos.ksh \
 	functional/rsend/send-c_embedded_blocks.ksh \
 	functional/rsend/send-c_incremental.ksh \
 	functional/rsend/send-c_lz4_disabled.ksh \

tests/zfs-tests/tests/functional/rsend/rsend_030_pos.ksh

@@ -0,0 +1,66 @@
+#!/bin/ksh -p
+
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# http://www.illumos.org/license/CDDL.
+#
+
+#
+# Copyright (c) 2022 by Nutanix. All rights reserved.
+#
+
+. $STF_SUITE/tests/functional/rsend/rsend.kshlib
+
+#
+# Description:
+# Verify resumability of full ZFS send/receive on existing dataset
+#
+# Strategy:
+# 1. Start a full ZFS send with redirect output to a file
+# 2. Mess up the contents of the stream state file on disk
+# 3. Try ZFS receive, which should fail with a checksum mismatch error
+# 4. ZFS send to the stream state file again using the receive_resume_token
+# 5. Verify ZFS receive without "-F" option (force recvflag) fails.
+# 6. Verify ZFS receive with "-F" option completes successfully.
+# 7. Repeat steps on an incremental ZFS send. It should complete
+#    successfully without "-F" option.
+#
+
+verify_runnable "both"
+
+sendfs=$POOL/sendfs
+recvfs=$POOL2/recvfs
+streamfs=$POOL/stream
+
+log_assert "Verify resumability of full ZFS send/receive on existing dataset"
+log_onexit resume_cleanup $sendfs $streamfs
+
+test_fs_setup $sendfs $recvfs $streamfs
+
+# Full send/recv on existing dataset
+log_must zfs create -o readonly=on $recvfs
+log_must eval "zfs send -c -v $sendfs@a >/$streamfs/1"
+mess_send_file /$streamfs/1
+log_mustnot eval "zfs recv -suvF $recvfs </$streamfs/1"
+token=$(get_prop receive_resume_token $recvfs)
+log_must eval "zfs send -t $token  >/$streamfs/2"
+log_mustnot eval "zfs recv -suv $recvfs </$streamfs/2"
+log_must eval "zfs recv -suvF $recvfs </$streamfs/2"
+file_check $sendfs $recvfs
+
+# Incremental send/recv
+log_must eval "zfs send -c -v -i @a $sendfs@b >/$streamfs/3"
+mess_send_file /$streamfs/3
+log_mustnot eval "zfs recv -suvF $recvfs </$streamfs/3"
+token=$(get_prop receive_resume_token $recvfs)
+log_must eval "zfs send -t $token  >/$streamfs/4"
+log_must eval "zfs recv -suv $recvfs </$streamfs/4"
+file_check $sendfs $recvfs
+
+log_pass "Verify resumability of full ZFS send/receive on existing dataset"