Re: [PATCH] btrfs: fix chattr failing on nodatasum files that have the compress flag

[Qu Wenruo <wqu@suse.com>]

在 2026/10/11 07:47, Qu Wenruo 写道:
> 
> 
> 在 2026/10/10 21:24, Hengyu Liang 写道:
>> Commit cf1afec09e9f ("btrfs: validate properties before setting them")
>> made btrfs_set_prop() validate a property before setting it. For the
>> compression property, the validation fails with -EINVAL when the inode
>> has NODATASUM.
>>
>> However, a file created on a filesystem mounted with nodatasum has
>> NODATASUM and still inherits the compress flag from its directory, and
>> btrfs_fileattr_set() sets the compression property whenever the new
>> flags include FS_COMPR_FL. As of now, chattr fails on such a file, even
>> when it only changes another flag.
>>
>> The issue can be reproduced with:
>>
>>      mkfs.btrfs -f /dev/sdb
>>      mount -o nodatasum /dev/sdb /mnt
>>      mkdir /mnt/dir
>>      chattr +c /mnt/dir
>>      touch /mnt/dir/file
>>      chattr +A /mnt/dir/file
>>
>> Before commit cf1afec09e9f ("btrfs: validate properties before setting
>> them"), the last command succeeds.
>>
>> After that commit, it fails with EINVAL.
> 
> Explain the reason why it fails.
> 
>>
>> This patch will make btrfs_fileattr_set() set the compression property
>> only when the inode can be compressed.
>>
>> Fixes: cf1afec09e9f ("btrfs: validate properties before setting them")
>> Cc: stable@vger.kernel.org # 7.2+
>> Signed-off-by: Hengyu Liang <hengyul@cs.unc.edu>
>> ---
>>   fs/btrfs/ioctl.c | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index 52aab510aea0..347509043f2e 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -421,7 +421,12 @@ int btrfs_fileattr_set(struct mnt_idmap *idmap,
>>       if (IS_ERR(trans))
>>           return PTR_ERR(trans);
>> -    if (comp) {
>> +    /*
>> +     * An inode with NODATASUM can not be compressed but can still 
>> have the
>> +     * compress flag, inherited from its parent directory. Setting the
>> +     * property fails validation for such an inode, so leave it unset.
>> +     */
> 
> This is very LLM-like, and very LLM's objective-oriented behavior.
> Disclose it first.
> 
> Secondly it just slightly do nothing, which is not the expected behavior.

s/slightly/silently/

> 
> Furthermore, this introduced another layer of exceptions.
> 
> For regular mount options, -o nodatasum,compress, the later compress 
> will override the nodatasum.
> 
> So when setting compress in the first place, the NODATASUM/NODATACOW 
> flag should be cleared.
> 
>> +    if (comp && btrfs_inode_can_compress(inode)) {
>>           ret = btrfs_set_prop(trans, inode, "btrfs.compression",
>>                        comp, strlen(comp), 0);
>>           if (ret)
>