Re: [PATCH] btrfs-progs: add verbose option to btrfs device scan

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

On 2019/7/15 下午11:09, Nikolay Borisov wrote:
>
>
> On 15.07.19 г. 17:42 ч., Anand Jain wrote:
>> To help debug device scan issues, add verbose option to btrfs device scan.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>
> I fail to see what this patch helps for. We get the path in case of
> errors, in case of success what good could the path be ?

AFAIK it would provide an easy way to debug blkid related bug.

E.g. scan only works on some devices and misses some devices.

So it makes sense to me, although "debug" would be more suitable in this
case.

Thanks,
Qu
>
>
>> ---
>>  cmds/device.c        | 8 ++++++--
>>  cmds/filesystem.c    | 2 +-
>>  common/device-scan.c | 4 +++-
>>  common/device-scan.h | 2 +-
>>  common/utils.c       | 2 +-
>>  disk-io.c            | 2 +-
>>  6 files changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/cmds/device.c b/cmds/device.c
>> index 24158308a41b..2fa13e61f806 100644
>> --- a/cmds/device.c
>> +++ b/cmds/device.c
>> @@ -313,6 +313,7 @@ static int cmd_device_scan(const struct cmd_struct *cmd, int argc, char **argv)
>>  	int all = 0;
>>  	int ret = 0;
>>  	int forget = 0;
>> +	int verbose = 0;
>
> nit: make it a bool.
>
>>
>>  	optind = 0;
>>  	while (1) {
>> @@ -323,7 +324,7 @@ static int cmd_device_scan(const struct cmd_struct *cmd, int argc, char **argv)
>>  			{ NULL, 0, NULL, 0}
>>  		};
>>
>> -		c = getopt_long(argc, argv, "du", long_options, NULL);
>> +		c = getopt_long(argc, argv, "duv", long_options, NULL);
>>  		if (c < 0)
>>  			break;
>>  		switch (c) {
>> @@ -333,6 +334,9 @@ static int cmd_device_scan(const struct cmd_struct *cmd, int argc, char **argv)
>>  		case 'u':
>>  			forget = 1;
>>  			break;
>> +		case 'v':
>> +			verbose = 1;
>> +			break;
>>  		default:
>>  			usage_unknown_option(cmd, argv);
>>  		}
>> @@ -354,7 +358,7 @@ static int cmd_device_scan(const struct cmd_struct *cmd, int argc, char **argv)
>>  			}
>>  		} else {
>>  			printf("Scanning for Btrfs filesystems\n");
>> -			ret = btrfs_scan_devices();
>> +			ret = btrfs_scan_devices(verbose);
>>  			error_on(ret, "error %d while scanning", ret);
>>  			ret = btrfs_register_all_devices();
>>  			error_on(ret,
>> diff --git a/cmds/filesystem.c b/cmds/filesystem.c
>> index 4f22089abeaa..37b23af36847 100644
>> --- a/cmds/filesystem.c
>> +++ b/cmds/filesystem.c
>> @@ -746,7 +746,7 @@ devs_only:
>>  		else
>>  			ret = 1;
>>  	} else {
>> -		ret = btrfs_scan_devices();
>> +		ret = btrfs_scan_devices(0);
>>  	}
>>
>>  	if (ret) {
>> diff --git a/common/device-scan.c b/common/device-scan.c
>> index 2c5ae225f710..bea201b351f0 100644
>> --- a/common/device-scan.c
>> +++ b/common/device-scan.c
>> @@ -351,7 +351,7 @@ void free_seen_fsid(struct seen_fsid *seen_fsid_hash[])
>>  	}
>>  }
>>
>> -int btrfs_scan_devices(void)
>> +int btrfs_scan_devices(int verbose)
>>  {
>>  	int fd = -1;
>>  	int ret;
>> @@ -380,6 +380,8 @@ int btrfs_scan_devices(void)
>>  			continue;
>>  		/* if we are here its definitely a btrfs disk*/
>>  		strncpy_null(path, blkid_dev_devname(dev));
>> +		if (verbose)
>> +			printf("blkid: btrfs device: %s\n", path);
>>
>>  		fd = open(path, O_RDONLY);
>>  		if (fd < 0) {
>> diff --git a/common/device-scan.h b/common/device-scan.h
>> index eda2bae5c6c4..8017a27511b9 100644
>> --- a/common/device-scan.h
>> +++ b/common/device-scan.h
>> @@ -29,7 +29,7 @@ struct seen_fsid {
>>  	int fd;
>>  };
>>
>> -int btrfs_scan_devices(void);
>> +int btrfs_scan_devices(int verbose);
>>  int btrfs_register_one_device(const char *fname);
>>  int btrfs_register_all_devices(void);
>>  int btrfs_add_to_fsid(struct btrfs_trans_handle *trans,
>> diff --git a/common/utils.c b/common/utils.c
>> index ad938409a94f..36ce89a025f1 100644
>> --- a/common/utils.c
>> +++ b/common/utils.c
>> @@ -277,7 +277,7 @@ int check_mounted_where(int fd, const char *file, char *where, int size,
>>
>>  	/* scan other devices */
>>  	if (is_btrfs && total_devs > 1) {
>> -		ret = btrfs_scan_devices();
>> +		ret = btrfs_scan_devices(0);
>>  		if (ret)
>>  			return ret;
>>  	}
>> diff --git a/disk-io.c b/disk-io.c
>> index be44eead5cef..4f52a29700ab 100644
>> --- a/disk-io.c
>> +++ b/disk-io.c
>> @@ -1085,7 +1085,7 @@ int btrfs_scan_fs_devices(int fd, const char *path,
>>  	}
>>
>>  	if (!skip_devices && total_devs != 1) {
>> -		ret = btrfs_scan_devices();
>> +		ret = btrfs_scan_devices(0);
>>  		if (ret)
>>  			return ret;
>>  	}
>>