-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
ZFS send/recv with ashift 9->12 leads to data corruption #12770
Conversation
Signed-off-by: Paul Dagnelie <pcd@delphix.com>
As an experiment, I did the following with this PR applied (and I'm reasonably confident I wasn't running the wrong module...):
Attempting the send a second time to capture the send stream for you (with the same command with | tee /tmp/sendstream in the middle) panicked the VM:
So that's new and exciting. (I was just looking to see how raidz played in the sandbox with p/lsize here...) I can't reproduce it on VM reboot though... (I rebooted the VM before running this in the first place, though, so ???) I'm going to go memtest just to be sure nothing is broken, since this system is reasonably new, but I haven't had any issues before this... edit: It reproduces if I freshly recreate the pools and write the data anew, but not on reboot. How very odd... |
According to zstreamdump on reproducing it...
(If you'd like, the "invalid checksum" send output is here.) (FYI, I just booted this patch on my non-VM testbed, and it panics the same way. So I don't think there's much chance of it being just my new system being flaky...) |
I had a long writeup of what's going wrong, but since GH won't let you comment on unchanged lines: I think the issue is that, as-is, your code doesn't ever set srdp->io_compressed if the arc_read call succeeds, so you end up if the ARC has the data treating it as uncompressed, and Bad Times Ensue. I would suggest setting it outside of the if, at which point, I don't think you need the one inside the if any more? Haven't tested it, yet, but it seems correct from what I observed (if it's in the ARC, you get into dmu_dump_write with io_compressed=0). (Edit: Quickly testing with just a |
Signed-off-by: Paul Dagnelie <pcd@delphix.com>
This new commit should address the issue; I also tested the patch with sends hitting and missing in the ARC. |
Improve the ability of zfs send to determine if a block is compressed or not by using information contained in the blkptr. Reviewed-by: Rich Ercolani <rincebrain@gmail.com> Reviewed-by: Matthew Ahrens <matthew.ahrens@delphix.com> Signed-off-by: Paul Dagnelie <pcd@delphix.com> Closes openzfs#12770
Improve the ability of zfs send to determine if a block is compressed or not by using information contained in the blkptr. Reviewed-by: Rich Ercolani <rincebrain@gmail.com> Reviewed-by: Matthew Ahrens <matthew.ahrens@delphix.com> Signed-off-by: Paul Dagnelie <pcd@delphix.com> Closes openzfs#12770
Improve the ability of zfs send to determine if a block is compressed or not by using information contained in the blkptr. Reviewed-by: Rich Ercolani <rincebrain@gmail.com> Reviewed-by: Matthew Ahrens <matthew.ahrens@delphix.com> Signed-off-by: Paul Dagnelie <pcd@delphix.com> Closes openzfs#12770
Improve the ability of zfs send to determine if a block is compressed or not by using information contained in the blkptr. Reviewed-by: Rich Ercolani <rincebrain@gmail.com> Reviewed-by: Matthew Ahrens <matthew.ahrens@delphix.com> Signed-off-by: Paul Dagnelie <pcd@delphix.com> Closes openzfs#12770
Improve the ability of zfs send to determine if a block is compressed or not by using information contained in the blkptr. Reviewed-by: Rich Ercolani <rincebrain@gmail.com> Reviewed-by: Matthew Ahrens <matthew.ahrens@delphix.com> Signed-off-by: Paul Dagnelie <pcd@delphix.com> Closes openzfs#12770
Improve the ability of zfs send to determine if a block is compressed or not by using information contained in the blkptr. Reviewed-by: Rich Ercolani <rincebrain@gmail.com> Reviewed-by: Matthew Ahrens <matthew.ahrens@delphix.com> Signed-off-by: Paul Dagnelie <pcd@delphix.com> Closes openzfs#12770
Improve the ability of zfs send to determine if a block is compressed or not by using information contained in the blkptr. Reviewed-by: Rich Ercolani <rincebrain@gmail.com> Reviewed-by: Matthew Ahrens <matthew.ahrens@delphix.com> Signed-off-by: Paul Dagnelie <pcd@delphix.com> Closes openzfs#12770
Motivation and Context
See #12762 for details.
Description
We improve the facility of zfs send to determine if a block is compressed or not by using the information contained in the blkptr, rather than relying on an lsize vs psize difference (which is usually, but not always, correct). Also removed an erroneous assertion in the arc codepath.
How Has This Been Tested?
Replicated the setup in the initial issue and verified that
dozer/test-second
andtank/test
contained the same contents after the fix was applied.Types of changes
Checklist:
Signed-off-by
.