Re: [PATCH v2 4/4] btrfs: support inheritance for per-inode compression levels

[Qu Wenruo <wqu@suse.com>]

在 2026/8/9 11:20, koraynilay 写道:
> Change prop_handler's extract() signature to take an output buffer and
> its length. This allows for the algo:level string to be generated
> dynamically, but now the caller is in charge of managing that buffer
> memory. Before this patch prop_compression_extract() would return only
> the address of statically compiled string without being able to add the
> level, now it concatenates the level to that statically compiled string.
> 
> This signature change is fine because the only currently supported prop
> is compression, so there is only 1 extract() function.
> 
> Assisted-by: Gemini:3.1-pro antigravity-cli-1.1.5
> Signed-off-by: koraynilay <koray.fra@gmail.com>
> ---
>   fs/btrfs/props.c | 14 +++++++++++---
>   1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
> index f79a61a2759f..9c97792800fb 100644
> --- a/fs/btrfs/props.c
> +++ b/fs/btrfs/props.c
> @@ -27,7 +27,7 @@ struct prop_handler {
>   	int (*validate)(const struct btrfs_inode *inode, const char *value,
>   			size_t len);
>   	int (*apply)(struct btrfs_inode *inode, const char *value, size_t len);
> -	const char *(*extract)(const struct btrfs_inode *inode);
> +	const char *(*extract)(const struct btrfs_inode *inode, char *buf, size_t len);
>   	bool (*ignore)(const struct btrfs_inode *inode);
>   	int inheritable;
>   };
> @@ -395,12 +395,19 @@ static bool prop_compression_ignore(const struct btrfs_inode *inode)
>   	return false;
>   }
>   
> -static const char *prop_compression_extract(const struct btrfs_inode *inode)
> +static const char *prop_compression_extract(const struct btrfs_inode *inode,
> +					    char *buf, size_t len)
>   {
>   	switch (inode->prop_compress) {
>   	case BTRFS_COMPRESS_ZLIB:
>   	case BTRFS_COMPRESS_LZO:
>   	case BTRFS_COMPRESS_ZSTD:
> +		if (inode->prop_compress_level) {
> +			snprintf(buf, len, "%s:%d",
> +				 btrfs_compress_type2str(inode->prop_compress),
> +				 inode->prop_compress_level);
> +			return buf;
> +		}
>   		return btrfs_compress_type2str(inode->prop_compress);
>   	default:
>   		break;
> @@ -437,6 +444,7 @@ int btrfs_inode_inherit_props(struct btrfs_trans_handle *trans,
>   		const struct prop_handler *h = &prop_handlers[i];
>   		const char *value;
>   		u64 num_bytes = 0;
> +		char buf[16];

Please use a macro to define the value.

In a perfect world, we want some way to determine the max string size at 
compile time, comparing the max length of "zlib", "zstd", "lzo", "none" 
with extra ":" and possible level values, but we do not have an easy way 
to do that.

So a comment on the how the size is calculated would be enough, e.g. the 
maximum length is "zstd:-15" with terminating 0, and round it up to 
power of 2 for future expansion.

Thanks,
Qu
>   
>   		if (!h->inheritable)
>   			continue;
> @@ -444,7 +452,7 @@ int btrfs_inode_inherit_props(struct btrfs_trans_handle *trans,
>   		if (h->ignore(inode))
>   			continue;
>   
> -		value = h->extract(parent);
> +		value = h->extract(parent, buf, sizeof(buf));
>   		if (!value)
>   			continue;
>