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

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

在 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.

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;
>   	}
>