Re: [PATCH v2] btrfs: preserve the compression property when other inode flags change

在 2026/8/14 22:31, Sam Ho 写道:
> Setting the compression property on an inode also sets BTRFS_INODE_COMPRESS
> on it, and btrfs_inode_flags_to_fsflags() reports that back as FS_COMPR_FL
> to FS_IOC_GETFLAGS. chattr(1), like any other FS_IOC_SETFLAGS caller, reads
> the current flags, flips only the bit the user asked for and writes the
> whole set back, so a request as unrelated as "chattr +i" reaches
> btrfs_fileattr_set() with FS_COMPR_FL set.
> 
> btrfs_fileattr_set() takes that as a request to enable compression and
> overwrites the compression property with the algorithm from the mount
> options, falling back to zlib when the filesystem was not mounted with
> -o compress. The algorithm the user selected is silently replaced:
> 
>    # btrfs property set /mnt/foo compression zstd
>    # btrfs property get /mnt/foo compression
>    compression=zstd
>    # chattr +i /mnt/foo
>    # btrfs property get /mnt/foo compression
>    compression=zlib
> 
> Every chattr operation triggers this, not just +i, and directories are
> affected as well, so files created afterwards inherit the wrong algorithm
> too. On a filesystem mounted with -o compress=lzo the property is replaced
> with lzo instead. Recovering needs a chattr -i first, because the immutable
> flag rejects the setxattr that "btrfs property set" issues.
> 
> Prefer the algorithm recorded in the compression property and only fall
> back to the mount default when there is no property, so that unrelated
> flag changes no longer overwrite the user's choice. Inodes that have the
> compress flag set but no property still get the default, so they behave
> as before.
> 
> Signed-off-by: Sam Ho <[email protected]>

Reviewed-by: Qu Wenruo <[email protected]>

Thanks,
Qu

> ---
> v2:
>   - Pick the compression type first and convert it to a string only once at
>     the end, instead of going through btrfs_compress_type2str() for every
>     candidate, and check inode->prop_compress directly rather than testing
>     old_fsflags for FS_COMPR_FL. Suggested by Qu Wenruo; the two are
>     equivalent, since prop_compression_apply() only leaves prop_compress
>     set while BTRFS_INODE_COMPRESS is set.
>   - Reword the last changelog paragraph and the comment to match.
> 
> v1: https://lore.kernel.org/linux-btrfs/[email protected]/
> 
>   fs/btrfs/ioctl.c | 21 ++++++++++++++++++---
>   1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index baa645e98812..343d089aa5c3 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -377,6 +377,7 @@ int btrfs_fileattr_set(struct mnt_idmap *idmap,
>   		inode_flags &= ~BTRFS_INODE_COMPRESS;
>   		inode_flags |= BTRFS_INODE_NOCOMPRESS;
>   	} else if (fsflags & FS_COMPR_FL) {
> +		enum btrfs_compression_type comp_type;
>   
>   		if (IS_SWAPFILE(&inode->vfs_inode))
>   			return -ETXTBSY;
> @@ -384,9 +385,23 @@ int btrfs_fileattr_set(struct mnt_idmap *idmap,
>   		inode_flags |= BTRFS_INODE_COMPRESS;
>   		inode_flags &= ~BTRFS_INODE_NOCOMPRESS;
>   
> -		comp = btrfs_compress_type2str(fs_info->compress_type);
> -		if (!comp || comp[0] == 0)
> -			comp = btrfs_compress_type2str(BTRFS_COMPRESS_ZLIB);
> +		/*
> +		 * Keep the algorithm recorded in the compression property,
> +		 * otherwise changing an unrelated attribute would reset it to
> +		 * the mount default, since FS_IOC_SETFLAGS callers write back
> +		 * the whole flag set they got from FS_IOC_GETFLAGS and that
> +		 * includes FS_COMPR_FL for any inode carrying the property.
> +		 *
> +		 * Inodes with the compress flag set but no property keep using
> +		 * the mount default, so they behave as before.
> +		 */
> +		if (inode->prop_compress)
> +			comp_type = inode->prop_compress;
> +		else if (fs_info->compress_type)
> +			comp_type = fs_info->compress_type;
> +		else
> +			comp_type = BTRFS_COMPRESS_ZLIB;
> +		comp = btrfs_compress_type2str(comp_type);
>   	} else {
>   		inode_flags &= ~(BTRFS_INODE_COMPRESS | BTRFS_INODE_NOCOMPRESS);
>   	}