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

Send / Recv Fixes following b52563 #6545

Merged
merged 1 commit into from
Aug 23, 2017
Merged

Conversation

tcaputi
Copy link
Contributor

@tcaputi tcaputi commented Aug 22, 2017

This patch fixes several issues discovered after
the encryption patch was merged:

  • Fixed a bug where encrypted datasets could attempt
    to receive embedded data records.
  • Fixed a bug where dirty records created by the recv
    code wasn't properly setting the dr_raw flag.
  • Fixed a typo where a dmu_tx_commit() was changed to
    dmu_tx_abort(), causing issues with invalid send streams.
  • Fixed a few error handling bugs unrelated to the
    encryption patch in dmu_recv_stream()

Signed-off-by: Tom Caputi tcaputi@datto.com

Motivation and Context

#6524
#6512
openzfs/openzfs#124 (comment)

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:

  • My code follows the ZFS on Linux code style requirements.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • All commit messages are properly formatted and contain Signed-off-by.
  • Change has been approved by a ZFS on Linux member.

This patch fixes several issues discovered after
the encryption patch was merged:

Fixed a bug where encrypted datasets could attempt
to receive embedded data records.

Fixed a bug where dirty records created by the recv
code wasn't properly setting the dr_raw flag.

Fixed a typo where a dmu_tx_commit() was changed to
dmu_tx_abort()

Fixed a few error handling bugs unrelated to the
encryption patch in dmu_recv_stream()

Signed-off-by: Tom Caputi <tcaputi@datto.com>
@stewartadam
Copy link

I've been testing the original encryption patch and ran into the bugs mention here, zfs streams have been locking up the pool. Should I destroy my pool and re-copy data or will zfs send resume as expected after patching without affecting transferred data?

@tcaputi
Copy link
Contributor Author

tcaputi commented Aug 23, 2017

@stewartadam This patch slightly changed the format of zfs send files. So you will have to upgrade both and then send the data. You should not have to recreate your pool. I'm not 100% sure if a resumed send will work, though. I would probably abort any in-progress send and redo it from scratch.

@sempervictus
Copy link
Contributor

@tcaputi: the format change does mean its a "breaking change" IMO, will get it pushed to builders and testers this week. Thanks as always.

@behlendorf behlendorf merged commit 9b84076 into openzfs:master Aug 23, 2017
@sempervictus
Copy link
Contributor

sempervictus commented Aug 23, 2017 via email

@tcaputi
Copy link
Contributor Author

tcaputi commented Aug 24, 2017

@sempervictus

Would it benefit users to know that partial upgrades across infrastructure are ill advised?

Probably. I really didn't think many people would be using this quite yet since it just got merged into master a week ago and is still not yet tagged (and won't be for some time). This is technically a breaking change to the send file format (albeit a very small one), so I would say it isn't advised to resume a send with the new code after starting it with the old one. The new code will not be able to recognize send files produced by the old code. It is not a breaking change to the on-disk structure of the pool.

@nedbass nedbass requested a review from behlendorf August 24, 2017 20:59
@jgottula jgottula mentioned this pull request Sep 20, 2017
FransUrbo pushed a commit to FransUrbo/zfs that referenced this pull request Apr 28, 2019
This patch fixes several issues discovered after
the encryption patch was merged:

* Fixed a bug where encrypted datasets could attempt
  to receive embedded data records.

* Fixed a bug where dirty records created by the recv
  code wasn't properly setting the dr_raw flag.

* Fixed a typo where a dmu_tx_commit() was changed to
  dmu_tx_abort()

* Fixed a few error handling bugs unrelated to the
  encryption patch in dmu_recv_stream()

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tom Caputi <tcaputi@datto.com>
Closes openzfs#6512 
Closes openzfs#6524 
Closes openzfs#6545
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants