On Tue, Oct 09, 2018 at 04:21:37PM -0700, Nick Terrell wrote:
> Adds zstd support to the btrfs module.
>
> 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: Nick Terrell <terre...@fb.com>
> ---
> v1 -> v2:
> - Fix comments from Daniel Kiper.
>
> v2 -> v3:
> - Use grub_error() to set grub_errno in grub_btrfs_zstd_decompress().
> - Fix style and formatting comments.
>
> Makefile.util.def | 10 +++-
> grub-core/Makefile.core.def | 10 +++-
> grub-core/fs/btrfs.c | 112 ++++++++++++++++++++++++++++++++++-
> tests/btrfs_test.in | 1 +
> tests/util/grub-fs-tester.in | 2 +-
> 5 files changed, 131 insertions(+), 4 deletions(-)
>
> diff --git a/Makefile.util.def b/Makefile.util.def
> index f9caccb97..27c5e9903 100644
> --- a/Makefile.util.def
> +++ b/Makefile.util.def
> @@ -54,7 +54,7 @@ library = {
> library = {
> name = libgrubmods.a;
> cflags = '-fno-builtin -Wno-undef';
> - cppflags = '-I$(top_srcdir)/grub-core/lib/minilzo
> -I$(srcdir)/grub-core/lib/xzembed -DMINILZO_HAVE_CONFIG_H';
> + cppflags = '-I$(srcdir)/grub-core/lib/minilzo
> -I$(srcdir)/grub-core/lib/xzembed -I$(srcdir)/grub-core/lib/zstd
> -DMINILZO_HAVE_CONFIG_H';
>
> common_nodist = grub_script.tab.c;
> common_nodist = grub_script.yy.c;
> @@ -164,6 +164,14 @@ library = {
> common = grub-core/lib/xzembed/xz_dec_bcj.c;
> common = grub-core/lib/xzembed/xz_dec_lzma2.c;
> common = grub-core/lib/xzembed/xz_dec_stream.c;
> + common = grub-core/lib/zstd/debug.c;
> + common = grub-core/lib/zstd/entropy_common.c;
> + common = grub-core/lib/zstd/error_private.c;
> + common = grub-core/lib/zstd/fse_decompress.c;
> + common = grub-core/lib/zstd/huf_decompress.c;
> + common = grub-core/lib/zstd/xxhash.c;
> + common = grub-core/lib/zstd/zstd_common.c;
> + common = grub-core/lib/zstd/zstd_decompress.c;
> };
>
> program = {
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 2c1d62cee..f818f58bc 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1265,8 +1265,16 @@ module = {
> name = btrfs;
> common = fs/btrfs.c;
> common = lib/crc.c;
> + common = lib/zstd/debug.c;
> + common = lib/zstd/entropy_common.c;
> + common = lib/zstd/error_private.c;
> + common = lib/zstd/fse_decompress.c;
> + common = lib/zstd/huf_decompress.c;
> + common = lib/zstd/xxhash.c;
> + common = lib/zstd/zstd_common.c;
> + common = lib/zstd/zstd_decompress.c;
> cflags = '$(CFLAGS_POSIX) -Wno-undef';
> - cppflags = '-I$(srcdir)/lib/posix_wrap -I$(srcdir)/lib/minilzo
> -DMINILZO_HAVE_CONFIG_H';
> + cppflags = '-I$(srcdir)/lib/posix_wrap -I$(srcdir)/lib/minilzo
> -I$(srcdir)/lib/zstd -DMINILZO_HAVE_CONFIG_H';
> };
Please do not embed zstd lib into btrfs module here.
I would like to see zstd lib as separate module if possible.
> module = {
> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> index 4849c1ceb..c44fe8029 100644
> --- a/grub-core/fs/btrfs.c
> +++ b/grub-core/fs/btrfs.c
> @@ -17,6 +17,8 @@
> * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
> */
>
> +#define ZSTD_STATIC_LINKING_ONLY
> +
> #include <grub/err.h>
> #include <grub/file.h>
> #include <grub/mm.h>
> @@ -27,6 +29,7 @@
> #include <grub/lib/crc.h>
> #include <grub/deflate.h>
> #include <minilzo.h>
> +#include <zstd.h>
> #include <grub/i18n.h>
> #include <grub/btrfs.h>
>
> @@ -45,6 +48,9 @@ GRUB_MOD_LICENSE ("GPLv3+");
> #define GRUB_BTRFS_LZO_BLOCK_MAX_CSIZE (GRUB_BTRFS_LZO_BLOCK_SIZE + \
> (GRUB_BTRFS_LZO_BLOCK_SIZE / 16) + 64 + 3)
>
> +#define ZSTD_BTRFS_MAX_WINDOWLOG 17
> +#define ZSTD_BTRFS_MAX_INPUT (1 << ZSTD_BTRFS_MAX_WINDOWLOG)
> +
> typedef grub_uint8_t grub_btrfs_checksum_t[0x20];
> typedef grub_uint16_t grub_btrfs_uuid_t[8];
>
> @@ -212,6 +218,7 @@ struct grub_btrfs_extent_data
> #define GRUB_BTRFS_COMPRESSION_NONE 0
> #define GRUB_BTRFS_COMPRESSION_ZLIB 1
> #define GRUB_BTRFS_COMPRESSION_LZO 2
> +#define GRUB_BTRFS_COMPRESSION_ZSTD 3
>
> #define GRUB_BTRFS_OBJECT_ID_CHUNK 0x100
>
> @@ -912,6 +919,95 @@ grub_btrfs_read_inode (struct grub_btrfs_data *data,
> return grub_btrfs_read_logical (data, elemaddr, inode, sizeof (*inode), 0);
> }
>
> +static void* grub_zstd_malloc (void* state, size_t size)
> +{
> + (void)state;
Please drop this and use "__attribute__ ((unused))" in function prototype.
> + return grub_malloc (size);
> +}
> +
> +static void grub_zstd_free (void* state, void* address)
> +{
> + (void)state;
Ditto.
> + return grub_free (address);
> +}
> +
> +static ZSTD_customMem grub_zstd_allocator (void)
> +{
> + ZSTD_customMem allocator;
> +
> + allocator.customAlloc = &grub_zstd_malloc;
> + allocator.customFree = &grub_zstd_free;
> + allocator.opaque = NULL;
> +
> + return allocator;
> +}
> +
> +static grub_ssize_t
> +grub_btrfs_zstd_decompress (char *ibuf, grub_size_t isize, grub_off_t off,
> + char *obuf, grub_size_t osize)
> +{
> + void *allocated = NULL;
> + char *otmpbuf = obuf;
> + grub_size_t otmpsize = osize;
> + ZSTD_DCtx *dctx = NULL;
> + grub_size_t zstd_ret;
> + grub_ssize_t ret = -1;
> +
> + /*
> + * Zstd will fail if it can't fit the entire output in the destination
> + * buffer, so if osize isn't large enough, allocate a temporary buffer.
> + */
> + if (otmpsize < ZSTD_BTRFS_MAX_INPUT) {
> + allocated = grub_malloc (ZSTD_BTRFS_MAX_INPUT);
> + if (!allocated) {
Hmmm... Should not we say something here? Like grub_error() call below?
It seems to me that failing silently is bad idea here.
> + goto err;
> + }
> + otmpbuf = (char*)allocated;
> + otmpsize = ZSTD_BTRFS_MAX_INPUT;
> + }
> +
> + /* Create the ZSTD_DCtx. */
> + dctx = ZSTD_createDCtx_advanced (grub_zstd_allocator ());
> + if (!dctx) {
> + grub_error (GRUB_ERR_OUT_OF_MEMORY, "zstd out of memory");
> + goto err;
> + }
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
