Re: [PATCH] btrfs: Accept and ignore compression level for lzo

[Filipe Manana <fdmanana@kernel.org>]

On Mon, Aug 25, 2025 at 10:03 AM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>
>
>
> 在 2025/8/25 18:21, Daniel Vacek 写道:
> > On Sun, 24 Aug 2025 at 17:58, Calvin Owens <calvin@wbinvd.org> wrote:
> >> From: Calvin Owens <calvin@wbinvd.org>
> >> Subject: [PATCH v3] btrfs: Accept and ignore compression level for lzo
> >>
> >> The compression level is meaningless for lzo, but before commit
> >> 3f093ccb95f30 ("btrfs: harden parsing of compression mount options"),
> >> it was silently ignored if passed.
> >>
> >> After that commit, passing a level with lzo fails to mount:
> >>
> >>      BTRFS error: unrecognized compression value lzo:1
> >>
> >> It seems reasonable for users to expect that lzo would permit a numeric
> >> level option, as all the other algos do, even though the kernel's
> >> implementation of LZO currently only supports a single level. Because it
> >> has always worked to pass a level, it seems likely to me that users in
> >> the real world are relying on doing so.
> >>
> >> This patch restores the old behavior, giving "lzo:N" the same semantics
> >> as all of the other compression algos.
> >>
> >> To be clear, silly variants like "lzo:one", "lzo:the_first_option", or
> >> "lzo:armageddon" also used to work. This isn't meant to suggest that
> >> any possible mis-interpretation of mount options that once worked must
> >> continue to work forever. This is an exceptional case where it makes
> >> sense to preserve compatibility, both because the mis-interpretation is
> >> reasonable, and because nothing tangible is sacrificed.
> >>
> >> Fixes: 3f093ccb95f30 ("btrfs: harden parsing of compression mount options")
> >> Signed-off-by: Calvin Owens <calvin@wbinvd.org>
> >> ---
> >>   fs/btrfs/super.c | 7 +++++--
> >>   1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > v3 looks good to me. The original hardening was meant to gate complete
> > nonsense like "compress=lzoutput", etc...
> >
> > Reviewed-by: Daniel Vacek <neelx@suse.com>
>
> Now merged and pushed to for-next branch with the latest reviewed-by tags.

Btw, don't forget a couple things:

1) In the subject, after the prefix "btrfs: " the first word should
not be capitalized;

2) In the log message (btrfs_warn() call), the first word should also
not be capitalized.

These are the styles we follow, so we should be consistent.

>
> Thanks,
> Qu
> >
> > Thank you.
> >
> >> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> >> index a262b494a89f..18eb00b3639b 100644
> >> --- a/fs/btrfs/super.c
> >> +++ b/fs/btrfs/super.c
> >> @@ -299,9 +299,12 @@ static int btrfs_parse_compress(struct btrfs_fs_context *ctx,
> >>                  btrfs_set_opt(ctx->mount_opt, COMPRESS);
> >>                  btrfs_clear_opt(ctx->mount_opt, NODATACOW);
> >>                  btrfs_clear_opt(ctx->mount_opt, NODATASUM);
> >> -       } else if (btrfs_match_compress_type(string, "lzo", false)) {
> >> +       } else if (btrfs_match_compress_type(string, "lzo", true)) {
> >>                  ctx->compress_type = BTRFS_COMPRESS_LZO;
> >> -               ctx->compress_level = 0;
> >> +               ctx->compress_level = btrfs_compress_str2level(BTRFS_COMPRESS_LZO,
> >> +                                                              string + 3);
> >> +               if (string[3] == ':' && string[4])
> >> +                       btrfs_warn(NULL, "Compression level ignored for LZO");
> >>                  btrfs_set_opt(ctx->mount_opt, COMPRESS);
> >>                  btrfs_clear_opt(ctx->mount_opt, NODATACOW);
> >>                  btrfs_clear_opt(ctx->mount_opt, NODATASUM);
> >> --
> >> 2.49.1
> >>
>
>