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 raw sends on encrypted datasets when copying back snapshots #11221

Merged
merged 1 commit into from
Dec 4, 2020

Conversation

gamanakis
Copy link
Contributor

@gamanakis gamanakis commented Nov 19, 2020

Motivation and Context

When sending raw encrypted datasets the user space accounting is present
when it's not expected to be. This leads to the subsequent mount failure
due a checksum error when verifying the local mac.

Closes #10523.

Description

Fix this by clearing the OBJSET_FLAG_USERACCOUNTING_COMPLETE and reset
the local mac. This allows the user accounting to be correctly updated
on first mount using the normal upgrade process.

How Has This Been Tested?

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)

Checklist:

@gamanakis
Copy link
Contributor Author

@tcaputi would you mind taking a look at this?

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Nov 19, 2020
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Thanks! Let's also add a new test to the userquota group for this. The reproducer in #10523 (comment) is a great place to start, but it should really also verify the resulting object accounting is correct. The existing userspace_encrypted.ksh test does a similar test but doesn't check sending it back to the source.

@gamanakis
Copy link
Contributor Author

45085ad: added a test. The object accounting seems to be correct between source and target. In 2/10 tests I see a discrepancy of 1024 bytes for a 20MB file. Is this to be expected? Right now the test rounds up the object accounting to MB (instead of bytes).

@behlendorf
Copy link
Contributor

Yes, some variance isn't entirely unexpected.

Copy link
Contributor

@tcaputi tcaputi left a comment

Choose a reason for hiding this comment

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

This generally looks good. Thanks a lot for picking this up. I kind of dropped the ball on fixing this. My apologies.

module/os/freebsd/zfs/zio_crypt.c Show resolved Hide resolved
module/os/linux/zfs/zio_crypt.c Show resolved Hide resolved
module/zfs/dsl_crypt.c Show resolved Hide resolved
@gamanakis
Copy link
Contributor Author

955f99b: Fixes from @tcaputi review. Thank you!

@behlendorf
Copy link
Contributor

@gamanakis looks like there's one more issue to investigate. With the change the zpool_import_errata3 test is consistently failing.

Tests with results other than PASS that are unexpected:
    FAIL cli_root/zpool_import/zpool_import_errata3 (expected PASS)

When sending raw encrypted datasets the user space accounting is present
when it's not expected to be. This leads to the subsequent mount failure
due a checksum error when verifying the local mac.
Fix this by clearing the OBJSET_FLAG_USERACCOUNTING_COMPLETE and reset
the local mac. This allows the user accounting to be correctly updated
on first mount using the normal upgrade process.

Closes openzfs#10523.

Signed-off-by: George Amanakis <[email protected]>
@gamanakis
Copy link
Contributor Author

gamanakis commented Nov 25, 2020

I think what is happening is that in Errata3 the useraccounting_complete flag is not set but ZFS expects a non-zeroed local_mac. Was encryption implemented before user accounting? I solved this by checking the encryption version, applied in 4c5efed.

Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Thanks, checking the zk_version in this context makes sense to me since the errata was specifically checking for an intermediate version of encryption which only existed for a short while in master between major releases. @tcaputi would you mind looking over this again.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Dec 1, 2020
Copy link
Contributor

@tcaputi tcaputi left a comment

Choose a reason for hiding this comment

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

Thanks for making the adjustments and for fixing errata 3 handling. This looks good to me.

@behlendorf behlendorf merged commit d1d4769 into openzfs:master Dec 4, 2020
ghost pushed a commit to zfsonfreebsd/ZoF that referenced this pull request Dec 23, 2020
When sending raw encrypted datasets the user space accounting is present
when it's not expected to be. This leads to the subsequent mount failure
due a checksum error when verifying the local mac.
Fix this by clearing the OBJSET_FLAG_USERACCOUNTING_COMPLETE and reset
the local mac. This allows the user accounting to be correctly updated
on first mount using the normal upgrade process.

Reviewed-By: Brian Behlendorf <[email protected]>
Reviewed-By: Tom Caputi <[email protected]>
Signed-off-by: George Amanakis <[email protected]>
Closes openzfs#10523 
Closes openzfs#11221
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
When sending raw encrypted datasets the user space accounting is present
when it's not expected to be. This leads to the subsequent mount failure
due a checksum error when verifying the local mac.
Fix this by clearing the OBJSET_FLAG_USERACCOUNTING_COMPLETE and reset
the local mac. This allows the user accounting to be correctly updated
on first mount using the normal upgrade process.

Reviewed-By: Brian Behlendorf <[email protected]>
Reviewed-By: Tom Caputi <[email protected]>
Signed-off-by: George Amanakis <[email protected]>
Closes openzfs#10523 
Closes openzfs#11221
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
When sending raw encrypted datasets the user space accounting is present
when it's not expected to be. This leads to the subsequent mount failure
due a checksum error when verifying the local mac.
Fix this by clearing the OBJSET_FLAG_USERACCOUNTING_COMPLETE and reset
the local mac. This allows the user accounting to be correctly updated
on first mount using the normal upgrade process.

Reviewed-By: Brian Behlendorf <[email protected]>
Reviewed-By: Tom Caputi <[email protected]>
Signed-off-by: George Amanakis <[email protected]>
Closes openzfs#10523 
Closes openzfs#11221
behlendorf pushed a commit that referenced this pull request Jan 21, 2022
Raw receiving a snapshot back to the originating dataset is currently
impossible because of user accounting being present in the originating
dataset.

One solution would be resetting user accounting when raw receiving on
the receiving dataset. However, to recalculate it we would have to dirty
all dnodes, which may not be preferable on big datasets.

Instead, we rely on the os_phys flag
OBJSET_FLAG_USERACCOUNTING_COMPLETE to indicate that user accounting is
incomplete when raw receiving. Thus, on the next mount of the receiving
dataset the local mac protecting user accounting is zeroed out.
The flag is then cleared when user accounting of the raw received
snapshot is calculated.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: George Amanakis <[email protected]>
Closes #12981 
Closes #10523
Closes #11221
Closes #11294
Closes #12594
Issue #11300
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Feb 5, 2022
Raw receiving a snapshot back to the originating dataset is currently
impossible because of user accounting being present in the originating
dataset.

One solution would be resetting user accounting when raw receiving on
the receiving dataset. However, to recalculate it we would have to dirty
all dnodes, which may not be preferable on big datasets.

Instead, we rely on the os_phys flag
OBJSET_FLAG_USERACCOUNTING_COMPLETE to indicate that user accounting is
incomplete when raw receiving. Thus, on the next mount of the receiving
dataset the local mac protecting user accounting is zeroed out.
The flag is then cleared when user accounting of the raw received
snapshot is calculated.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: George Amanakis <[email protected]>
Closes openzfs#12981 
Closes openzfs#10523
Closes openzfs#11221
Closes openzfs#11294
Closes openzfs#12594
Issue openzfs#11300
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Raw receiving a snapshot back to the originating dataset is currently
impossible because of user accounting being present in the originating
dataset.

One solution would be resetting user accounting when raw receiving on
the receiving dataset. However, to recalculate it we would have to dirty
all dnodes, which may not be preferable on big datasets.

Instead, we rely on the os_phys flag
OBJSET_FLAG_USERACCOUNTING_COMPLETE to indicate that user accounting is
incomplete when raw receiving. Thus, on the next mount of the receiving
dataset the local mac protecting user accounting is zeroed out.
The flag is then cleared when user accounting of the raw received
snapshot is calculated.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: George Amanakis <[email protected]>
Closes openzfs#12981 
Closes openzfs#10523
Closes openzfs#11221
Closes openzfs#11294
Closes openzfs#12594
Issue openzfs#11300
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Raw receiving a snapshot back to the originating dataset is currently
impossible because of user accounting being present in the originating
dataset.

One solution would be resetting user accounting when raw receiving on
the receiving dataset. However, to recalculate it we would have to dirty
all dnodes, which may not be preferable on big datasets.

Instead, we rely on the os_phys flag
OBJSET_FLAG_USERACCOUNTING_COMPLETE to indicate that user accounting is
incomplete when raw receiving. Thus, on the next mount of the receiving
dataset the local mac protecting user accounting is zeroed out.
The flag is then cleared when user accounting of the raw received
snapshot is calculated.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: George Amanakis <[email protected]>
Closes openzfs#12981 
Closes openzfs#10523
Closes openzfs#11221
Closes openzfs#11294
Closes openzfs#12594
Issue openzfs#11300
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Raw send on encrypted datasets does not work when copying snapshots back
3 participants