]> git.proxmox.com Git - mirror_zfs.git/commitdiff
Fix incremental receive silently failing for recursive sends
authorPaul Dagnelie <paul.dagnelie@delphix.com>
Fri, 10 Mar 2023 17:52:44 +0000 (09:52 -0800)
committerGitHub <noreply@github.com>
Fri, 10 Mar 2023 17:52:44 +0000 (09:52 -0800)
The problem occurs because dmu_recv_begin pulls in the payload and
next header from the input stream in order to use the contents of
the begin record's nvlist. However, the change to do that before the
other checks in dmu_recv_begin occur caused a regression where an
empty send stream in a recursive send could have its END record
consumed by this, which broke the logic of recv_skip. A test is
also included to protect against this case in the future.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Paul Dagnelie <pcd@delphix.com>
Closes #12661
Closes #14568

module/zfs/dmu_recv.c
tests/runfiles/common.run
tests/zfs-tests/cmd/libzfs_input_check.c
tests/zfs-tests/tests/Makefile.am
tests/zfs-tests/tests/functional/rsend/rsend_031_pos.ksh [new file with mode: 0755]

index 1684d9d92ff5e07eff773095b60301b958caaad9..3763cba021355daf94fa2fee41eb6024f5276c23 100644 (file)
@@ -1255,7 +1255,6 @@ dmu_recv_begin(char *tofs, char *tosnap, dmu_replay_record_t *drr_begin,
            DMU_GET_FEATUREFLAGS(drc->drc_drrb->drr_versioninfo);
 
        uint32_t payloadlen = drc->drc_drr_begin->drr_payloadlen;
-       void *payload = NULL;
 
        /*
         * Since OpenZFS 2.0.0, we have enforced a 64MB limit in userspace
@@ -1266,16 +1265,23 @@ dmu_recv_begin(char *tofs, char *tosnap, dmu_replay_record_t *drr_begin,
        if (payloadlen > (MIN((1U << 28), arc_all_memory() / 4)))
                return (E2BIG);
 
-       if (payloadlen != 0)
-               payload = vmem_alloc(payloadlen, KM_SLEEP);
 
-       err = receive_read_payload_and_next_header(drc, payloadlen,
-           payload);
-       if (err != 0) {
-               vmem_free(payload, payloadlen);
-               return (err);
-       }
        if (payloadlen != 0) {
+               void *payload = vmem_alloc(payloadlen, KM_SLEEP);
+               /*
+                * For compatibility with recursive send streams, we don't do
+                * this here if the stream could be part of a package. Instead,
+                * we'll do it in dmu_recv_stream. If we pull the next header
+                * too early, and it's the END record, we break the `recv_skip`
+                * logic.
+                */
+
+               err = receive_read_payload_and_next_header(drc, payloadlen,
+                   payload);
+               if (err != 0) {
+                       vmem_free(payload, payloadlen);
+                       return (err);
+               }
                err = nvlist_unpack(payload, payloadlen, &drc->drc_begin_nvl,
                    KM_SLEEP);
                vmem_free(payload, payloadlen);
@@ -3315,6 +3321,17 @@ dmu_recv_stream(dmu_recv_cookie_t *drc, offset_t *voffp)
                        goto out;
        }
 
+       /*
+        * For compatibility with recursive send streams, we do this here,
+        * rather than in dmu_recv_begin. If we pull the next header too
+        * early, and it's the END record, we break the `recv_skip` logic.
+        */
+       if (drc->drc_drr_begin->drr_payloadlen == 0) {
+               err = receive_read_payload_and_next_header(drc, 0, NULL);
+               if (err != 0)
+                       goto out;
+       }
+
        /*
         * If we failed before this point we will clean up any new resume
         * state that was created. Now that we've gotten past the initial
index 132df492541da66c80ecde384e53f5f14e785fad..d30bcd804c1308d5bc687c3965921eb8ab49a3b7 100644 (file)
@@ -837,19 +837,20 @@ 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',
-    'rsend_030_pos', 'send-c_verify_ratio', 'send-c_verify_contents',
-    'send-c_props', 'send-c_incremental', 'send-c_volume',
-    'send-c_zstream_recompress', '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_incremental', 'send_encrypted_freeobjects',
-    'send_encrypted_hierarchy', 'send_encrypted_props',
-    'send_encrypted_truncated_files', 'send_freeobjects', 'send_realloc_files',
-    'send_realloc_encrypted_files', 'send_spill_block', 'send_holds',
-    'send_hole_birth', 'send_mixed_raw', 'send-wR_encrypted_zvol',
-    'send_partial_dataset', 'send_invalid', 'send_doall',
-    'send_raw_spill_block', 'send_raw_ashift', 'send_raw_large_blocks']
+    'rsend_030_pos', 'rsend_031_pos', 'send-c_verify_ratio',
+    'send-c_verify_contents', 'send-c_props', 'send-c_incremental',
+    'send-c_volume', 'send-c_zstream_recompress', '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_incremental',
+    'send_encrypted_freeobjects', 'send_encrypted_hierarchy',
+    'send_encrypted_props', 'send_encrypted_truncated_files',
+    'send_freeobjects', 'send_realloc_files', 'send_realloc_encrypted_files',
+    'send_spill_block', 'send_holds', 'send_hole_birth', 'send_mixed_raw',
+    'send-wR_encrypted_zvol', 'send_partial_dataset', 'send_invalid',
+    'send_doall', 'send_raw_spill_block', 'send_raw_ashift',
+    'send_raw_large_blocks']
 tags = ['functional', 'rsend']
 
 [tests/functional/scrub_mirror]
index 2e1859b1eef090121c0241fc908f6f0a9867fb59..a1dfaefd71050998483ef22664e1148a64133e04 100644 (file)
@@ -563,7 +563,7 @@ test_recv_new(const char *dataset, int fd)
        fnvlist_add_uint64(optional, "action_handle", *action_handle);
 #endif
        IOC_INPUT_TEST(ZFS_IOC_RECV_NEW, dataset, required, optional,
-           ZFS_ERR_STREAM_TRUNCATED);
+           ENOTSUP);
 
        nvlist_free(props);
        nvlist_free(optional);
index efce46123c32f8b4be55c54b9e27e38c0142041f..4b0a28a6ef625cb1f58c006922d585b0f7e74242 100644 (file)
@@ -1795,6 +1795,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \
        functional/rsend/rsend_028_neg.ksh \
        functional/rsend/rsend_029_neg.ksh \
        functional/rsend/rsend_030_pos.ksh \
+       functional/rsend/rsend_031_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_031_pos.ksh b/tests/zfs-tests/tests/functional/rsend/rsend_031_pos.ksh
new file mode 100755 (executable)
index 0000000..119da71
--- /dev/null
@@ -0,0 +1,63 @@
+#!/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) 2023 by Delphix. All rights reserved.
+#
+
+. $STF_SUITE/tests/functional/rsend/rsend.kshlib
+
+#
+# Description:
+# Verify recursive incremental sends missing snapshots behave correctly.
+#
+# Strategy:
+# 1. Create snapshots on source filesystem.
+# 2. Recursively send snapshots.
+# 3. Delete snapshot on source filesystem
+# 4. Perform incremental recursive send.
+# 5. Verify matching snapshot lists.
+#
+
+verify_runnable "both"
+
+sendfs=$POOL/sendfs
+recvfs=$POOL2/recvfs
+
+function cleanup {
+       rm $BACKDIR/stream1
+       rm $BACKDIR/stream2
+       zfs destroy -r $sendfs
+       zfs destroy -r $recvfs
+}
+
+log_assert "Verify recursive incremental sends missing snapshots behave correctly."
+log_onexit cleanup
+
+log_must zfs create $sendfs
+log_must zfs snapshot $sendfs@A
+log_must zfs snapshot $sendfs@B
+log_must zfs snapshot $sendfs@C
+log_must eval "zfs send -Rpv $sendfs@C > $BACKDIR/stream1"
+log_must eval "zfs receive -F $recvfs < $BACKDIR/stream1"
+log_must zfs list $sendfs@C
+
+log_must zfs destroy $sendfs@C
+log_must zfs snapshot $sendfs@D
+log_must zfs snapshot $sendfs@E
+log_must eval "zfs send -Rpv -I $sendfs@A $sendfs@E > $BACKDIR/stream2"
+log_must eval "zfs receive -Fv $recvfs < $BACKDIR/stream2"
+log_must zfs list $sendfs@D
+log_must zfs list $sendfs@E
+log_mustnot zfs list $sendfs@C
+log_pass "Verify recursive incremental sends missing snapshots behave correctly."