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

Added test for being able to read various variants of zstd #12030

Merged
merged 1 commit into from
Sep 20, 2021

Conversation

rincebrain
Copy link
Contributor

@rincebrain rincebrain commented May 12, 2021

As detailed in #12022 and #12008, it turns out the current zstd implementation
is quite nonportable, and results in various configurations of ondisk header
that only each platform can read.

So I've added a test which contains a dataset with a file written by
Linux/x86_64, and one written by FBSD/ppc64.

(Note that for obvious reasons, this PR will currently always fail the testbots if I implemented it correctly.) Nah, #12022 landed.

Motivation and Context

#12022

Description

Added a test, compression/compress_zstd_bswap, which untars a premade sparse pool file containing 2 datasets, with different "variants" of zstd encoding used on the single file on each.

It then tries copying the files off of each dataset and dies if any of them error out.

I also added it to the "sanity" runfile.

How Has This Been Tested?

I ran it on an otherwise unmodified git master-running system, verified that it successfully copied the x86_64 file and failed the test when it EIOed on the ppc64_fbsd file.

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)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@rincebrain
Copy link
Contributor Author

rincebrain commented May 19, 2021

Well, the improbable existence of the third byte layout kept bothering me, so I tried to reproduce it with vanilla(...plus the trivial patch for sparc64) git, and found that Linux/sparc64, Linux/ppc64, and FBSD/ppc64 wrote identical encodings, so I'm forced to conclude I accidentally generated the Linux/sparc64 dataset with one of my WIP patchsets that encoded things wrong. So I've dropped it from the test pool; I can put it back if people feel like having a deliberately mangled dataset that couldn't naturally arise through any known method short of patching would be good.

@rincebrain
Copy link
Contributor Author

Well, #12022 got merged, so...

@behlendorf
Copy link
Contributor

behlendorf commented Sep 17, 2021

Thanks for the reminder, this should be good but can you rebase it on master so we can get a fresh CI run.

@behlendorf behlendorf added Component: Test Suite Indicates an issue with the test framework or a test case Status: Code Review Needed Ready for review and testing labels Sep 17, 2021
As detailed in openzfs#12022 and openzfs#12008, it turns out the current zstd
implementation is quite nonportable, and results in various
configurations of ondisk header that only each platform can read.

So I've added a test which contains a dataset with a file written by
Linux/x86_64 and one written by FBSD/ppc64.

Signed-off-by: Rich Ercolani <[email protected]>
@rincebrain
Copy link
Contributor Author

Sure, off it goes.

@jwk404 jwk404 merged commit 8a3fe59 into openzfs:master Sep 20, 2021
rincebrain added a commit to rincebrain/zfs that referenced this pull request Sep 22, 2021
As detailed in openzfs#12022 and openzfs#12008, it turns out the current zstd
implementation is quite nonportable, and results in various
configurations of ondisk header that only each platform can read.

So I've added a test which contains a dataset with a file written by
Linux/x86_64 and one written by FBSD/ppc64.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Rich Ercolani <[email protected]>
Closes openzfs#12030
rincebrain added a commit to rincebrain/zfs that referenced this pull request Oct 2, 2021
As detailed in openzfs#12022 and openzfs#12008, it turns out the current zstd
implementation is quite nonportable, and results in various
configurations of ondisk header that only each platform can read.

So I've added a test which contains a dataset with a file written by
Linux/x86_64 and one written by FBSD/ppc64.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Rich Ercolani <[email protected]>
Closes openzfs#12030
tonyhutter pushed a commit that referenced this pull request Nov 1, 2021
As detailed in #12022 and #12008, it turns out the current zstd
implementation is quite nonportable, and results in various
configurations of ondisk header that only each platform can read.

So I've added a test which contains a dataset with a file written by
Linux/x86_64 and one written by FBSD/ppc64.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Rich Ercolani <[email protected]>
Closes #12030
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Nov 13, 2021
As detailed in openzfs#12022 and openzfs#12008, it turns out the current zstd
implementation is quite nonportable, and results in various
configurations of ondisk header that only each platform can read.

So I've added a test which contains a dataset with a file written by
Linux/x86_64 and one written by FBSD/ppc64.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Rich Ercolani <[email protected]>
Closes openzfs#12030
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Test Suite Indicates an issue with the test framework or a test case Status: Code Review Needed Ready for review and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants