Re: [PATCH 1/4] btrfs: set fs_root = NULL on error

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

[-- Attachment #1.1: Type: text/plain, Size: 1228 bytes --]



On 2020/2/12 上午5:40, Josef Bacik wrote:
> While running my error injection script I hit a panic when we tried to
> clean up the fs_root when free'ing the fs_root.  This is because
> fs_info->fs_root == PTR_ERR(-EIO), which isn't great.  Fix this by
> setting fs_info->fs_root = NULL; if we fail to read the root.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Just one off-topic idea, can we have test cases in fstests to do
specific error injection test?

For your fix, we can inject ENOMEM error with call chain
btrfs_read_fs_root_no_name()->open_ctree() to get a 100% reproducible
test, which looks to be a solid test case.

Thanks,
Qu

> ---
>  fs/btrfs/disk-io.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index eb441fa3711b..5b6140482cef 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3260,6 +3260,7 @@ int __cold open_ctree(struct super_block *sb,
>  	if (IS_ERR(fs_info->fs_root)) {
>  		err = PTR_ERR(fs_info->fs_root);
>  		btrfs_warn(fs_info, "failed to read fs tree: %d", err);
> +		fs_info->fs_root = NULL;
>  		goto fail_qgroup;
>  	}
>  
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]