Re: [PATCH 1/2] btrfs-progs: fi show: Print missing device for a mounted file system

[Nikolay Borisov <nborisov@suse.com>]

On 2.09.21 г. 13:27, Qu Wenruo wrote:
> 
> 
> On 2021/9/2 下午6:06, Nikolay Borisov wrote:
>> Currently when a device is missing for a mounted filesystem the output
>> that is produced is unhelpful:
>>
>> Label: none  uuid: 139ef309-021f-4b98-a3a8-ce230a83b1e2
>>     Total devices 2 FS bytes used 128.00KiB
>>     devid    1 size 5.00GiB used 1.26GiB path /dev/loop0
>>     *** Some devices missing
>>
>> While the context which prints this is perfectly capable of showing
>> which device exactly is missing, like so:
>>
>> Label: none  uuid: 4a85a40b-9b79-4bde-8e52-c65a550a176b
>>     Total devices 2 FS bytes used 128.00KiB
>>     devid    1 size 5.00GiB used 1.26GiB path /dev/loop0
>>     devid    2 size 0 used 0 path /dev/loop1 ***MISSING***
>>
>> This is a lot more usable output as it presents the user with the id
>> of the missing device and its path.
> 
> The idea is pretty awesome.
> 
> Just one question, if one device is missing, how could we know its path?
> Thus does the device path output make any sense?

The path is not canonicalized but otherwise the paths comes from
btrfs_ioctl_dev_info_args which is filled by a call to get_fs_info where
we call get_device_info for every device in the fs_info.

So the path is really dev->name from kernel space or if we don't have a
dev->name it will be 0. In either case it's useful that we get the devid
so that the user can do :

btrfs device remove 2 (if we take the above example), alternatively the
path would be a NULL-terminated string which aka empty. I guess that's
still better than simply saying *some devices are missing*

> 
> Thanks,
> Qu
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>>   cmds/filesystem.c | 7 +++----
>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/cmds/filesystem.c b/cmds/filesystem.c
>> index db8433ba3542..ff13de6ac990 100644
>> --- a/cmds/filesystem.c
>> +++ b/cmds/filesystem.c
>> @@ -295,7 +295,6 @@ static int print_one_fs(struct
>> btrfs_ioctl_fs_info_args *fs_info,
>>   {
>>       int i;
>>       int fd;
>> -    int missing = 0;
>>       char uuidbuf[BTRFS_UUID_UNPARSED_SIZE];
>>       struct btrfs_ioctl_dev_info_args *tmp_dev_info;
>>       int ret;
>> @@ -325,8 +324,10 @@ static int print_one_fs(struct
>> btrfs_ioctl_fs_info_args *fs_info,
>>           /* Add check for missing devices even mounted */
>>           fd = open((char *)tmp_dev_info->path, O_RDONLY);
>>           if (fd < 0) {
>> -            missing = 1;
>> +            printf("\tdevid %4llu size 0 used 0 path %s
>> ***MISSING***\n",
>> +                    tmp_dev_info->devid,tmp_dev_info->path);
>>               continue;
>> +
>>           }
>>           close(fd);
>>           canonical_path = path_canonicalize((char *)tmp_dev_info->path);
>> @@ -339,8 +340,6 @@ static int print_one_fs(struct
>> btrfs_ioctl_fs_info_args *fs_info,
>>           free(canonical_path);
>>       }
>>
>> -    if (missing)
>> -        printf("\t*** Some devices missing\n");
>>       printf("\n");
>>       return 0;
>>   }
>>
>