Re: [PATCH] btrfs: don't force the filesystem read-only on EDQUOT/ENOSPC verity rollback

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2026/9/14 16:02, Qu Wenruo 写道:
> 
> 
> 在 2026/9/14 15:54, Daniel Linjama 写道:
>> When enable_verity() hits the qgroup limit, rollback_verity() needs its
>> own metadata reservation. When the qgroup limit refuses the rollback,
>> the whole filesystem is forced read-only even though the qgroup limit
>> was for one subvolume only.
>>
>> Skip btrfs_handle_fs_error() for -EDQUOT/-ENOSPC and just return the
>> error. The verity orphan item is left in place but the orphan cleanup
>> from commit 705242538ff3 ("btrfs: verity metadata orphan items") will
>> remove it at the next mount. fsverity enable still correctly fails but
>> the filesystem is not forced read-only.
>>
>> Fixes: 146054090b08 ("btrfs: initial fsverity support")
>> Signed-off-by: Daniel Linjama <daniel@dev.linjama.com>
>> ---
>> Verified on a virtual machine with a mainline kernel (08df884136f1) and
>> a loop-mounted btrfs, with either a qgroup limit on a subvolume or a
>> full filesystem. On the unpatched kernel the whole filesystem was forced
>> read-only. On the patched kernel fsverity enable returns -EDQUOT/-ENOSPC
>> and the filesystem stays read-write.
> 
> Please also submit a fstests case for it.
> 
>>
>>   fs/btrfs/verity.c | 22 ++++++++++++++++++----
>>   1 file changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c
>> index 4e0ab5842274..8d0f9eea4c39 100644
>> --- a/fs/btrfs/verity.c
>> +++ b/fs/btrfs/verity.c
>> @@ -441,7 +441,9 @@ static int del_orphan(struct btrfs_trans_handle 
>> *trans, struct btrfs_inode *inod
>>    *
>>    * We try to handle recoverable errors while enabling verity by 
>> rolling it back
>>    * and just failing the operation, rather than having an fs level 
>> error no
>> - * matter what. However, any error in rollback is unrecoverable.
>> + * matter what. Failing to delete the verity items for lack of space is
>> + * tolerated, the orphan item ensures they are removed on the next 
>> mount.
>> + * Any other error in rollback is unrecoverable.
>>    *
>>    * Returns 0 on success, negative error code on failure.
>>    */
>> @@ -456,9 +458,21 @@ static int rollback_verity(struct btrfs_inode 
>> *inode)
>>       clear_bit(BTRFS_INODE_VERITY_IN_PROGRESS, &inode->runtime_flags);
>>       ret = btrfs_drop_verity_items(inode);
>>       if (ret) {
>> -        btrfs_handle_fs_error(root->fs_info, ret,
>> -                "failed to drop verity items in rollback %llu",
>> -                inode->vfs_inode.i_ino);
>> +        /*
>> +         * -EDQUOT and -ENOSPC mean we could not reserve metadata to
>> +         * delete the verity items. That is not a consistency problem,
>> +         * so don't turn the filesystem read-only. Leave the items and
>> +         * the orphan in place, orphan cleanup on the next mount will
>> +         * remove them.
>> +         */
> 
> EDQUOT is fine, but I'm not sure if ENOSPC is really recoverable.
> 
> Even if we ignore ENOSPC for now, we may have really exhausted the 
> metadata, and the next transaction may be aborted anyway.

Sashiko also mentioned even if we ignored EDQUOT for now, the next mount 
we still fallback to call btrfs_drop_verity_items() on the inode cleanup.

And it will return -EDQUOT again, so the inode will not be cleanred up.

Considering we're only delete items, I think the better solution would 
be calling btrfs_start_transaction_fallback_global_rsv(), which will not 
enforce qgroup, thus should not return -EDQUOT in the first place.

> 
> I believe your test case is only utilizing qgroup limit, or did you hit 
> a case where btrfs_drop_verity_items() returns -ENOSPC and the fs still 
> works fine?
> 
> Otherwise the idea looks good to me.
> 
> Thanks,
> Qu
>> +        if (ret != -EDQUOT && ret != -ENOSPC)
>> +            btrfs_handle_fs_error(root->fs_info, ret,
>> +                          "failed to drop verity items in rollback 
>> %llu",
>> +                          inode->vfs_inode.i_ino);
>> +        else
>> +            btrfs_warn(root->fs_info,
>> +                   "failed to drop verity items in rollback %llu: %pe",
>> +                   inode->vfs_inode.i_ino, ERR_PTR(ret));
>>           goto out;
>>       }
>