Backport zstd support for btrfs to rhel-8.5 #86
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Import zstd-1.3.6 from upstream - Add zstd's module.c file - Add the zstd module to Makefile.core.def Import zstd-1.3.6 from upstream [1]. Only the files need for decompression are imported. I used the latest zstd release, which includes patches [2] to build cleanly in GRUB. I included the script used to import zstd-1.3.6 below at the bottom of the commit message. Upstream zstd commit hash: 4fa456d7f12f8b27bd3b2f5dfd4f46898cb31c24 Upstream zstd commit name: Merge pull request #1354 from facebook/dev Zstd requires some posix headers, which it gets from posix_wrap. This can be checked by inspecting the .Po files generated by automake, which contain the header dependencies. After building run the command `cat grub-core/lib/zstd/.deps-core/*.Po` to see the dependencies [3]. The only OS dependencies are: - stddef.h, which is already a dependency in posix_wrap, and used for size_t by lzo and xz. - stdarg.h, which comes from the grub/misc.h header, and we don't use in zstd. All the types like uint64_t are typedefed to grub_uint64_t under the hood. The only exception is size_t, which comes from stddef.h. This is already the case for lzo and xz. I don't think there are any cross-compilation concerns, because cross-compilers provide their own system headers (and it would already be broken). [1] https://github.com/facebook/zstd/releases/tag/v1.3.6 [2] facebook/zstd#1344 [3] https://gist.github.com/terrelln/7a16b92f5a1b3aecf980f944b4a966c4 ``` curl -L -O https://github.com/facebook/zstd/releases/download/v1.3.6/zstd-1.3.6.tar.gz curl -L -O https://github.com/facebook/zstd/releases/download/v1.3.6/zstd-1.3.6.tar.gz.sha256 sha256sum --check zstd-1.3.6.tar.gz.sha256 tar xzf zstd-1.3.6.tar.gz SRC_LIB="zstd-1.3.6/lib" DST_LIB="grub-core/lib/zstd" rm -rf $DST_LIB mkdir -p $DST_LIB cp $SRC_LIB/zstd.h $DST_LIB/ cp $SRC_LIB/common/*.[hc] $DST_LIB/ cp $SRC_LIB/decompress/*.[hc] $DST_LIB/ rm $DST_LIB/{pool.[hc],threading.[hc]} rm -rf zstd-1.3.6* echo SUCCESS! ``` Signed-off-by: Nick Terrell <terrelln@fb.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This is a backport of 3861286 to the rhel-8.5 branch. The original commit message follows: - Adds zstd support to the btrfs module. - Adds a test case for btrfs zstd support. - Changes top_srcdir to srcdir in the btrfs module's lzo include following comments from Daniel Kiper about the zstd include. Tested on Ubuntu-18.04 with a btrfs /boot partition with and without zstd compression. A test case was also added to the test suite that fails before the patch, and passes after. Signed-off-by: Davide Cavalca <dcavalca@centosproject.org>
|
The btrfs part looks correct to me, compared with the existing code and then validated it did the right thing WRT zstd. If Nick can double check the zstd decompress part that would be good, but other than that it looks fine. |
|
The zstd decompression looks good to me! |
|
Related bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1972386 |
|
Closing since the corresponding bugzilla has also been closed - see conversation there if this is surprising. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a straight backport of the btrfs zstd support that was originally added in 461f1d8 and 3861286. This will make it possible for grub2 to access btrfs filesystems compressed with zstd, both via grub proper and through
grub2-mount, which is something we'd like to leverage for https://fedoraproject.org/wiki/Changes/FedoraCloudBtrfsByDefault#grub2-mount