From 3ed9d6883bcf3c55f92cdaaa6bf1aee2e6fb4115 Mon Sep 17 00:00:00 2001 From: Jitendra Patidar <53164267+jsai20@users.noreply.github.com> Date: Wed, 28 Sep 2022 05:04:27 +0530 Subject: [PATCH] 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 Reviewed-by: Chunwei Chen Signed-off-by: Jitendra Patidar Closes #13856 Closes #13857 --- contrib/pyzfs/libzfs_core/_constants.py | 1 + include/libzfs.h | 1 + include/sys/fs/zfs.h | 1 + lib/libzfs/libzfs_sendrecv.c | 40 +++++++++++ lib/libzfs/libzfs_util.c | 3 + module/zfs/dmu_recv.c | 11 ++++ tests/runfiles/common.run | 6 +- tests/zfs-tests/tests/Makefile.am | 1 + .../tests/functional/rsend/rsend_030_pos.ksh | 66 +++++++++++++++++++ 9 files changed, 127 insertions(+), 3 deletions(-) create mode 100755 tests/zfs-tests/tests/functional/rsend/rsend_030_pos.ksh diff --git a/contrib/pyzfs/libzfs_core/_constants.py b/contrib/pyzfs/libzfs_core/_constants.py index 2db2bba8a8..4db1de8d9a 100644 --- a/contrib/pyzfs/libzfs_core/_constants.py +++ b/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', ], {} ) diff --git a/include/libzfs.h b/include/libzfs.h index 4fc7761225..9bd4613c10 100644 --- a/include/libzfs.h +++ b/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; diff --git a/include/sys/fs/zfs.h b/include/sys/fs/zfs.h index dedee0e7bd..b3fecf489e 100644 --- a/include/sys/fs/zfs.h +++ b/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; /* diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index 577ebf6aad..9d514a37ee 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/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, diff --git a/lib/libzfs/libzfs_util.c b/lib/libzfs/libzfs_util.c index bc00a8dffd..28dd4f426a 100644 --- a/lib/libzfs/libzfs_util.c +++ b/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: diff --git a/module/zfs/dmu_recv.c b/module/zfs/dmu_recv.c index 0f3181f762..e22b6b13a4 100644 --- a/module/zfs/dmu_recv.c +++ b/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); diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run index e8443ffabc..65b64f4fa8 100644 --- a/tests/runfiles/common.run +++ b/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', diff --git a/tests/zfs-tests/tests/Makefile.am b/tests/zfs-tests/tests/Makefile.am index d53316643b..6aeb862fbb 100644 --- a/tests/zfs-tests/tests/Makefile.am +++ b/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 \ diff --git a/tests/zfs-tests/tests/functional/rsend/rsend_030_pos.ksh b/tests/zfs-tests/tests/functional/rsend/rsend_030_pos.ksh new file mode 100755 index 0000000000..a683f5befa --- /dev/null +++ b/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/2" +log_mustnot eval "zfs recv -suv $recvfs /$streamfs/3" +mess_send_file /$streamfs/3 +log_mustnot eval "zfs recv -suvF $recvfs /$streamfs/4" +log_must eval "zfs recv -suv $recvfs