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

[Qu Wenruo]

在 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@xxxxxxxxxxxxxxx # 7.2+
> > Signed-off-by: Hengyu Liang <hengyul@xxxxxxxxxx>
> > ---
> >   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)