Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incremental receive silently failing for recursive sends #14568

Merged
merged 5 commits into from
Mar 10, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 26 additions & 9 deletions module/zfs/dmu_recv.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
pcd1193182 marked this conversation as resolved.
Show resolved Hide resolved
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);
Expand Down Expand Up @@ -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
Expand Down
27 changes: 14 additions & 13 deletions tests/runfiles/common.run
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
2 changes: 1 addition & 1 deletion tests/zfs-tests/cmd/libzfs_input_check.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions tests/zfs-tests/tests/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
61 changes: 61 additions & 0 deletions tests/zfs-tests/tests/functional/rsend/rsend_031_pos.ksh
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
#!/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
}

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"
pcd1193182 marked this conversation as resolved.
Show resolved Hide resolved
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."