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.

Sign up for GitHub

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

Jump to bottom

syncoid: Fix typo preventing resumed transfer with --sendoptions #919

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

syncoid: Fix typo preventing resumed transfer with --sendoptions #919

wants to merge 1 commit into from

Conversation

Deltik
Copy link
Contributor

@Deltik Deltik commented Apr 24, 2024 •
edited

Motivation and Context

Fixes: #918

Description

fix(syncoid): zfs send arg allowlist when sendsource is receivetoken

The runsynccmd subroutine was not matching the $sendsource when a receive resume token is passed in. All usages that pass in the receive resume token do not begin with a space; instead, they start with -t .

How Has This Been Tested?

All the existing syncoid tests pass, and the new one I added based on the test in #918 also passes.

Before

root@demo:~/sanoid/tests/syncoid# time ./run-tests.sh 
Running test 001_bookmark_replication_intermediate ... [PASS]
Running test 002_bookmark_replication_no_intermediate ... [PASS]
Running test 003_force_delete ... [PASS]
Running test 004_bookmark_replication_edge_case ... [PASS]
Running test 005_reset_resume_state ... mbuffer: error: outputThread: error writing to <stdout> at offset 0x20000: Broken pipe
mbuffer: warning: error during output to <stdout>: Broken pipe
[PASS]
Running test 006_reset_resume_state2 ... [PASS]
Running test 007_preserve_recordsize ... [PASS]
Running test 008_force_delete_snapshot ... [PASS]
Running test 009_preserve_properties ... [PASS]
Running test 010_filter_snaps ... [PASS]
Running test 011_sync_out-of-order_snapshots ... [SKIPPED]
Running test 012_receive_resume_token ... [FAILED] (see /tmp/syncoid_test_run_012_receive_resume_token.log)

real    0m49.073s
user    0m2.609s
sys     0m4.445s
root@demo:~/sanoid/tests/syncoid# cat /tmp/syncoid_test_run_012_receive_resume_token.log
+ . ../../common/lib.sh
+++ uname
++ unamestr=Linux
+ POOL_IMAGE=/tmp/syncoid-test-012.zpool
+ POOL_SIZE=128M
+ POOL_NAME=syncoid-test-012
+ MOUNT_TARGET=/tmp/syncoid-test-012.mount
+ truncate -s 128M /tmp/syncoid-test-012.zpool
+ zpool create -O mountpoint=/tmp/syncoid-test-012.mount -f syncoid-test-012 /tmp/syncoid-test-012.zpool
+ trap cleanUp EXIT
+ zfs create syncoid-test-012/source
+ zfs snap syncoid-test-012/source@empty
+ dd if=/dev/urandom of=/tmp/syncoid-test-012.mount/source/garbage.bin bs=1M count=16
16+0 records in
16+0 records out
16777216 bytes (17 MB, 16 MiB) copied, 0.171202 s, 98.0 MB/s
+ zfs snap syncoid-test-012/source@something
+ zfs send -pwR syncoid-test-012/source@something
+ head --bytes=8M
+ zfs recv -s syncoid-test-012/destination
cannot receive incremental stream: checksum mismatch or incomplete stream.
Partially received snapshot is saved.
A resuming stream can be generated on the sending system by running:
    zfs send -t 1-10ab43bd0e-130-789c636064000310a501c49c50360710a715e5e7a69766a6304041569dd1936f1e13ac15806c762475f94959a9c9250c0c4c507518f26969c5a92520991ab83c1b927c5265496a31909e105c87557f493ec41556b1cfb79bfc0c4d764092e704cbe725e6a632301457e625e767a6e8024d2bd1353034d22fce2f2d4a4e7528cecf4d2dc9c8cc4b87ba4f026a2eccdfa9b949a929f9d9607bb991c493f3730b8a528b8b8152700000dad92ebd
+ ../../../syncoid --sendoptions=pw syncoid-test-012/source syncoid-test-012/destination
INFO: Resuming interrupted zfs send/receive from syncoid-test-012/source to syncoid-test-012/destination (~ 8.1 MB remaining):
invalid flags combined with -t
usage:
        send [-DLPbcehnpsVvw] [-i|-I snapshot]
             [-R [-X dataset[,dataset]...]]     <snapshot>
        send [-DnVvPLecw] [-i snapshot|bookmark] <filesystem|volume|snapshot>
        send [-DnPpVvLec] [-i bookmark|snapshot] --redact <bookmark> <snapshot>
        send [-nVvPe] -t <receive_resume_token>
        send [-PnVv] --saved filesystem

For the property list, run: zfs set|get

For the delegated permission list, run: zfs allow|unallow

For further help on a command or topic, run: zfs help [<topic>]
cannot receive: failed to read from stream
CRITICAL ERROR:  zfs send -p -w  -t 1-10ab43bd0e-130-789c636064000310a501c49c50360710a715e5e7a69766a6304041569dd1936f1e13ac15806c762475f94959a9c9250c0c4c507518f26969c5a92520991ab83c1b927c5265496a31909e105c87557f493ec41556b1cfb79bfc0c4d764092e704cbe725e6a632301457e625e767a6e8024d2bd1353034d22fce2f2d4a4e7528cecf4d2dc9c8cc4b87ba4f026a2eccdfa9b949a929f9d9607bb991c493f3730b8a528b8b8152700000dad92ebd | mbuffer  -q -s 128k -m 16M | pv -p -t -e -r -b -s 8536288 |  zfs receive  -s -F 'syncoid-test-012/destination' 2>&1 failed: 256
+ '[' 2 -eq 0 ']'
+ echo 'Regression detected: syncoid did not handle the resuming correctly.'
Regression detected: syncoid did not handle the resuming correctly.
+ exit 1
+ cleanUp
+ zpool destroy syncoid-test-012
+ rm -f /tmp/syncoid-test-012.zpool

After

root@demo:~/sanoid/tests/syncoid# time ./run-tests.sh 
Running test 001_bookmark_replication_intermediate ... [PASS]
Running test 002_bookmark_replication_no_intermediate ... [PASS]
Running test 003_force_delete ... [PASS]
Running test 004_bookmark_replication_edge_case ... [PASS]
Running test 005_reset_resume_state ... mbuffer: error: outputThread: error writing to <stdout> at offset 0x40000: Broken pipe
mbuffer: warning: error during output to <stdout>: Broken pipe
[PASS]
Running test 006_reset_resume_state2 ... [PASS]
Running test 007_preserve_recordsize ... [PASS]
Running test 008_force_delete_snapshot ... [PASS]
Running test 009_preserve_properties ... [PASS]
Running test 010_filter_snaps ... [PASS]
Running test 011_sync_out-of-order_snapshots ... [SKIPPED]
Running test 012_receive_resume_token ... [PASS]

real    0m53.306s
user    0m2.664s
sys     0m4.576s
root@demo:~/sanoid/tests/syncoid# cat /tmp/syncoid_test_run_012_receive_resume_token.log
+ . ../../common/lib.sh
+++ uname
++ unamestr=Linux
+ POOL_IMAGE=/tmp/syncoid-test-012.zpool
+ POOL_SIZE=128M
+ POOL_NAME=syncoid-test-012
+ MOUNT_TARGET=/tmp/syncoid-test-012.mount
+ truncate -s 128M /tmp/syncoid-test-012.zpool
+ zpool create -O mountpoint=/tmp/syncoid-test-012.mount -f syncoid-test-012 /tmp/syncoid-test-012.zpool
+ trap cleanUp EXIT
+ zfs create syncoid-test-012/source
+ zfs snap syncoid-test-012/source@empty
+ dd if=/dev/urandom of=/tmp/syncoid-test-012.mount/source/garbage.bin bs=1M count=16
16+0 records in
16+0 records out
16777216 bytes (17 MB, 16 MiB) copied, 0.223194 s, 75.2 MB/s
+ zfs snap syncoid-test-012/source@something
+ zfs send -pwR syncoid-test-012/source@something
+ head --bytes=8M
+ zfs recv -s syncoid-test-012/destination
cannot receive incremental stream: checksum mismatch or incomplete stream.
Partially received snapshot is saved.
A resuming stream can be generated on the sending system by running:
    zfs send -t 1-134f4f2d85-130-789c636064000310a501c49c50360710a715e5e7a69766a6304081d08d07139e872fb05100b2d991d4e52765a5269730303041d561c8a7a515a79680646ae0f26c48f2499525a9c5407a42701d56fd25f9105754ae6259537041c0ce01499e132c9f97989bcac0505c99979c9f99a20b34ad44d7c0d048bf38bfb42839d5a1383f37b52423332f1dea3e09a8b9307fa7e626a5a6e46783ede546124fcecf2d284a2d2e064ac10100ed112e71
+ ../../../syncoid --sendoptions=pw syncoid-test-012/source syncoid-test-012/destination
INFO: Resuming interrupted zfs send/receive from syncoid-test-012/source to syncoid-test-012/destination (~ 8.1 MB remaining):
INFO: Sending incremental syncoid-test-012/source@something ... syncoid_demo_2024-04-26:07:23:50-GMT00:00 to syncoid-test-012/destination (~ 4 KB):
+ '[' 0 -eq 0 ']'
+ echo 'Syncoid resumed transfer successfully.'
Syncoid resumed transfer successfully.
++ sha256sum /tmp/syncoid-test-012.mount/source/garbage.bin
++ cut -d ' ' -f 1
+ original_sum=67a35fa033bb5ea20e356b9f5cd3f5c3a707f5a73473160b7a2e7357a9976611
++ sha256sum /tmp/syncoid-test-012.mount/destination/garbage.bin
++ cut -d ' ' -f 1
+ received_sum=67a35fa033bb5ea20e356b9f5cd3f5c3a707f5a73473160b7a2e7357a9976611
+ '[' 67a35fa033bb5ea20e356b9f5cd3f5c3a707f5a73473160b7a2e7357a9976611 == 67a35fa033bb5ea20e356b9f5cd3f5c3a707f5a73473160b7a2e7357a9976611 ']'
+ echo 'Data integrity verified.'
Data integrity verified.
+ exit 0
+ cleanUp
+ zpool destroy syncoid-test-012
+ rm -f /tmp/syncoid-test-012.zpool

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

phreaker0
phreaker0 requested changes Apr 26, 2024
View reviewed changes
Copy link
Collaborator

@phreaker0 phreaker0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, pls resolve the minor things and i will test it.

tests/syncoid/012_receive_resume_token/run.sh Outdated Show resolved Hide resolved
tests/syncoid/012_receive_resume_token/run.sh Outdated Show resolved Hide resolved
tests/syncoid/012_receive_resume_token/run.sh Outdated Show resolved Hide resolved
tests/syncoid/012_receive_resume_token/run.sh Outdated Show resolved Hide resolved
tests/syncoid/012_receive_resume_token/run.sh Outdated Show resolved Hide resolved
@phreaker0 phreaker0 added bugfix revision needed Changes are required for this PR to be merged labels Apr 26, 2024
@Deltik
fix(syncoid): zfs send arg allowlist when sendsource is receivetoken
fab4b40 
The `runsynccmd` subroutine was not matching the `$sendsource` when a
receive resume token is passed in. All usages that pass in the receive
resume token do not begin with a space; instead, they start with `-t `.

Fixes: jimsalterjrs#918
@Deltik Deltik requested a review from phreaker0 April 26, 2024 07:24
@jimsalterjrs
Copy link
Owner

@phreaker0 tag me when this looks ready to you, please :)

phreaker0
phreaker0 approved these changes Apr 29, 2024
View reviewed changes
Copy link
Collaborator

@phreaker0 phreaker0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Deltik thx for the adaptions, i tested the PR and it looks good. thanks

@phreaker0 phreaker0 added ready to merge and removed revision needed Changes are required for this PR to be merged labels Apr 29, 2024
@phreaker0
Copy link
Collaborator

@jimsalterjrs ready

@MihaiBalint
Copy link

Would be awesome to have this merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phreaker0 phreaker0 phreaker0 approved these changes

Assignees

@jimsalterjrs jimsalterjrs

Labels
bugfix ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Regression: "invalid flags combined with -t" when resuming with syncoid --sendoptions
4 participants
@Deltik @jimsalterjrs @phreaker0 @MihaiBalint