On Wed, Mar 28, 2018 at 11:24 PM, Nikolay Borisov <nborisov@xxxxxxxx> wrote:
>
>
> On 29.03.2018 01:11, Liu Bo wrote:
>> When mount fails to read trees like fs tree, checksum tree, extent
>> tree, etc, there is not enough information about where went wrong.
>>
>> With this, messages like
>>
>> "BTRFS warning (device sdf): failed to read root (objectid=7): -5"
>>
>> would help us a bit.
>>
>> Signed-off-by: Liu Bo <bo.liu@xxxxxxxxxxxxxxxxx>
>> ---
>> fs/btrfs/disk-io.c | 31 ++++++++++++++++++++++---------
>> 1 file changed, 22 insertions(+), 9 deletions(-)
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index 21f34ad..954616c 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -2334,23 +2334,29 @@ static int btrfs_read_roots(struct btrfs_fs_info *fs_info)
>> location.offset = 0;
>>
>> root = btrfs_read_tree_root(tree_root, &location);
>> - if (IS_ERR(root))
>> - return PTR_ERR(root);
>> + if (IS_ERR(root)) {
>> + ret = PTR_ERR(root);
>> + goto out;
>> + }
>> set_bit(BTRFS_ROOT_TRACK_DIRTY, &root->state);
>> fs_info->extent_root = root;
>>
>> location.objectid = BTRFS_DEV_TREE_OBJECTID;
>> root = btrfs_read_tree_root(tree_root, &location);
>> - if (IS_ERR(root))
>> - return PTR_ERR(root);
>> + if (IS_ERR(root)) {
>> + ret = PTR_ERR(root);
>> + goto out;
>> + }
>> set_bit(BTRFS_ROOT_TRACK_DIRTY, &root->state);
>> fs_info->dev_root = root;
>> btrfs_init_devices_late(fs_info);
>>
>> location.objectid = BTRFS_CSUM_TREE_OBJECTID;
>> root = btrfs_read_tree_root(tree_root, &location);
>> - if (IS_ERR(root))
>> - return PTR_ERR(root);
>> + if (IS_ERR(root)) {
>> + ret = PTR_ERR(root);
>> + goto out;
>> + }
>> set_bit(BTRFS_ROOT_TRACK_DIRTY, &root->state);
>> fs_info->csum_root = root;
>>
>> @@ -2367,7 +2373,7 @@ static int btrfs_read_roots(struct btrfs_fs_info *fs_info)
>> if (IS_ERR(root)) {
>> ret = PTR_ERR(root);
>> if (ret != -ENOENT)
>> - return ret;
>> + goto out;
>> } else {
>> set_bit(BTRFS_ROOT_TRACK_DIRTY, &root->state);
>> fs_info->uuid_root = root;
>> @@ -2376,13 +2382,19 @@ static int btrfs_read_roots(struct btrfs_fs_info *fs_info)
>> if (btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE)) {
>> location.objectid = BTRFS_FREE_SPACE_TREE_OBJECTID;
>> root = btrfs_read_tree_root(tree_root, &location);
>> - if (IS_ERR(root))
>> - return PTR_ERR(root);
>> + if (IS_ERR(root)) {
>> + ret = PTR_ERR(root);
>> + goto out;
>> + }
>> set_bit(BTRFS_ROOT_TRACK_DIRTY, &root->state);
>> fs_info->free_space_root = root;
>> }
>>
>> return 0;
>> +out:
>> + btrfs_warn(fs_info, "failed to read root (objectid=%llu): %d",
>> + location.objectid, ret);
>
> Since those are critical errors don't we want btrfs_err here?
>
Not much difference to me, I followed the convention, after failing to
read tree roots and falling back to recovery tree roots array,
btrfs_warn is used.
tree_root->node = read_tree_block();
if () {
btrfs_warn(...);
goto recovery_tree_roots;
}
thanks,
liubo
>> + return ret;
>> }
>>
>> int open_ctree(struct super_block *sb,
>> @@ -2953,6 +2965,7 @@ int open_ctree(struct super_block *sb,
>> fs_info->fs_root = btrfs_read_fs_root_no_name(fs_info, &location);
>> 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);
>
> ditto
>
>> goto fail_qgroup;
>> }
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
