Re: [PATCH 1/2] btrfs-progs: Correctly open filesystem on image file

Nikolay Borisov
2019-05-16T13:45:17Z

On 16.05.19 г. 16:41 ч., Qu Wenruo wrote:
>
>
> On 2019/5/16 下午9:12, Nikolay Borisov wrote:
>> When btrfs' 'filesystem' subcommand is passed path to an image file it
>> currently fails since the code expects the image file is going to be
>> recognised by libblkid (called from btrfs_scan_devices()). This is not
>> the case since libblkid only scan well-known locations under /dev.
>>
>> Fix this by explicitly calling open_ctree which will correctly open
>> the image and add it to the correct btrfs_fs_devices struct. This allows
>> subsequent cmd_filesystem_show logic to correctly show requested
>> information.
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>> cmds-filesystem.c | 13 ++++++++++++-
>> 1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/cmds-filesystem.c b/cmds-filesystem.c
>> index b8beec13f0e5..f55ce9b4ab85 100644
>> --- a/cmds-filesystem.c
>> +++ b/cmds-filesystem.c
>> @@ -771,7 +771,18 @@ static int cmd_filesystem_show(int argc, char **argv)
>> goto out;
>>
>> devs_only:
>> - ret = btrfs_scan_devices();
>> + if (type == BTRFS_ARG_REG) {
>> + /*
>> + * We don't close the fs_info because it will free the device,
>> + * this is not a long-running process so it's fine
>> + */
>
> The comment makes sense, but I'm pretty sure we still prefer to clean it up.
>
> Just something like:
>
> struct btrfs_root *root = NULL;
>
> root = open_ctree();
> if (root)
> ret = 0;
> else
> ret = 1;
> close_ctree(root);

Tested that and it doesn't work, because close_ctree will call
btrfs_close_devices which frees existing devices so the test fails.

>
> Despite that, I think the patch looks good to me.
>
> Thanks,
> Qu
>
>> + if (open_ctree(search, btrfs_sb_offset(0), 0))
>> + ret = 0;
>> + else
>> + ret = 1;
>> + } else {
>> + ret = btrfs_scan_devices();
>> + }
>>
>> if (ret) {
>> error("blkid device scan returned %d", ret);
>>
>