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

Revert "Fix issues with truncated files in raw sends" #8584

Merged
merged 1 commit into from
Apr 6, 2019

Conversation

behlendorf
Copy link
Contributor

Motivation and Context

Resolve #8540 and #8565 which affect a more common use
case for send/recv.

Description

This partially reverts commit 5dbf8b4. This change resolved
the issues observed with truncated files in raw sends, #7378.
However, the required changes to dnode_allocate() appear
to have introduced a regression for non-raw streams which
needs to be understood, #8540 and #8565.

How Has This Been Tested?

Locally built, pending full CI results. Restores code to previous state.

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:

This partially reverts commit 5dbf8b4.  This change resolved
the issues observed with truncated files in raw sends.  However,
the required changes to dnode_allocate() introduced a regression
for non-raw streams which needs to be understood.

The additional debugging improvements from the original patch
were not reverted.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Issue openzfs#7378
Issue openzfs#8528
Issue openzfs#8540
Issue openzfs#8565
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Apr 5, 2019
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 is a revert, so it looks fine. Some of this will definitely still be necessary in the updated fix

@behlendorf behlendorf merged commit d93d4b1 into openzfs:master Apr 6, 2019
@codecov
Copy link

codecov bot commented Apr 6, 2019

Codecov Report

Merging #8584 into master will increase coverage by 0.09%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8584      +/-   ##
==========================================
+ Coverage    78.6%   78.69%   +0.09%     
==========================================
  Files         381      381              
  Lines      117526   117525       -1     
==========================================
+ Hits        92384    92490     +106     
+ Misses      25142    25035     -107
Flag Coverage Δ
#kernel 79.25% <100%> (-0.01%) ⬇️
#user 67.23% <0%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 944a372...e393d81. Read the comment docs.

@behlendorf
Copy link
Contributor Author

This was done to ensure continued correct behavior of one of the most heavily used long standing features. Raw send/recv is a great upcoming feature, but it has not yet been released. And won't be until this issue is resolved.

@beren12
Copy link
Contributor

beren12 commented Apr 8, 2019

I think your bot had the best responses to this kpande…

"ZFS native encryption is currently broken and not recommended for use due to issue #6845"
and
"ZFS native encryption is currently in TESTING, and not for PRODUCTION use unless you UNDERSTAND THE RISKS. See #6845."

I mean the new bugs have been found but the answer is the same.
At least this isn't all of encryption, just raw sends for it.

@behlendorf behlendorf deleted the revert-7378 branch April 19, 2021 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Code Review Needed Ready for review and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PANIC at dnode.c:715:dnode_reallocate() / Crash - interrupt took too long
3 participants