On 2019/5/16 下午9:45, Nikolay Borisov wrote:
>
>
> 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@xxxxxxxx>
>>> ---
>>> 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.
Sorry for the confusion, I missed "..." line before close_ctree(root);
I mean something like:
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index c4e43f8446dd..d20cbd49c201 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -677,6 +677,7 @@ static int cmd_filesystem_show(int argc, char **argv)
{
LIST_HEAD(all_uuids);
struct btrfs_fs_devices *fs_devices;
+ struct btrfs_root *root = NULL;
char *search = NULL;
int ret;
/* default, search both kernel and udev */
@@ -779,7 +780,8 @@ devs_only:
* We don't close the fs_info because it will free the
device,
* this is not a long-running process so it's fine
*/
- if (open_ctree(search, btrfs_sb_offset(0), 0))
+ root = open_ctree(search, 0, 0);
+ if (root)
ret = 0;
else
ret = 1;
@@ -821,6 +823,7 @@ devs_only:
free_fs_devices(fs_devices);
}
out:
+ close_ctree(root);
free_seen_fsid(seen_fsid_hash);
return ret;
}
Thanks,
Qu
>
>>
>> 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);
>>>
>>
