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 recursive encrypted receive #9494
Fix incremental recursive encrypted receive #9494
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9494 +/- ##
==========================================
- Coverage 79.15% 79.06% -0.09%
==========================================
Files 416 403 -13
Lines 123655 122548 -1107
==========================================
- Hits 97876 96898 -978
+ Misses 25779 25650 -129
Continue to review full report at Codecov.
|
| # encryption settings, receiving it as an encrypted child. | ||
| log_note "Must be able to receive incremental stream with props to encrypted" | ||
| ds=$TESTPOOL/crypt/recv | ||
| log_must eval "zfs send -p $snap > $sendfile" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good, but the test case may need to be updated: zfs send -p $snap is not incremental.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its actually just the comment that's wrong (and git made it confusing). The new test case is the second one, which is largely the same as the first, except that it uses the second snapshot. I will fix the comment now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hum, now i see we are using zfs send -R instead of zfs send -p to generate an incremental send stream, which works but is kind of confusing: i would suggest using zfs send -i explicitly to generate an incremental stream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to do it like this because (1) it is a more complicated case to handle and (2) the problem was originally reported as a failure for zfs send -R to work incrementally and basically worked as it did in the test case.
Currently, incremental recersive encrypted receives fail to work for any snapshot after the first. The reason for this is because the check in zfs_setup_cmdline_props() did not properly realize that when the user attempts to use '-x encryption' in this situation, they are not really overriding the existing encryption property and instead are attempting to prevent it from changing. This resulted in an error message stating: "encryption property 'encryption' cannot be set or excluded for raw or incremental streams". This problem is fixed by updating the logic to expect this use case. Signed-off-by: Tom Caputi <tcaputi@datto.com>
de713a9
to
75ed693
Compare
Currently, incremental recursive encrypted receives fail to work for any snapshot after the first. The reason for this is because the check in zfs_setup_cmdline_props() did not properly realize that when the user attempts to use '-x encryption' in this situation, they are not really overriding the existing encryption property and instead are attempting to prevent it from changing. This resulted in an error message stating: "encryption property 'encryption' cannot be set or excluded for raw or incremental streams". This problem is fixed by updating the logic to expect this use case. Reviewed-by: loli10K <ezomori.nozomu@gmail.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Igor Kozhukhov <igor@dilos.org> Signed-off-by: Tom Caputi <tcaputi@datto.com> Closes openzfs#9494
Currently, incremental recursive encrypted receives fail to work for any snapshot after the first. The reason for this is because the check in zfs_setup_cmdline_props() did not properly realize that when the user attempts to use '-x encryption' in this situation, they are not really overriding the existing encryption property and instead are attempting to prevent it from changing. This resulted in an error message stating: "encryption property 'encryption' cannot be set or excluded for raw or incremental streams". This problem is fixed by updating the logic to expect this use case. Reviewed-by: loli10K <ezomori.nozomu@gmail.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Igor Kozhukhov <igor@dilos.org> Signed-off-by: Tom Caputi <tcaputi@datto.com> Closes openzfs#9494
Currently, incremental recursive encrypted receives fail to work for any snapshot after the first. The reason for this is because the check in zfs_setup_cmdline_props() did not properly realize that when the user attempts to use '-x encryption' in this situation, they are not really overriding the existing encryption property and instead are attempting to prevent it from changing. This resulted in an error message stating: "encryption property 'encryption' cannot be set or excluded for raw or incremental streams". This problem is fixed by updating the logic to expect this use case. Reviewed-by: loli10K <ezomori.nozomu@gmail.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Igor Kozhukhov <igor@dilos.org> Signed-off-by: Tom Caputi <tcaputi@datto.com> Closes #9494
Currently, incremental recersive encrypted receives fail to work
for any snapshot after the first. The reason for this is because
the check in zfs_setup_cmdline_props() did not properly realize
that when the user attempts to use '-x encryption' in this
situation, they are not really overriding the existing encryption
property and instead are attempting to prevent it from changing.
This resulted in an error message stating: "encryption property
'encryption' cannot be set or excluded for raw or incremental
streams".
This problem is fixed by updating the logic to expect this use
case.
Signed-off-by: Tom Caputi tcaputi@datto.com
Types of changes
Checklist:
Signed-off-by.