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

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

[Anand Jain]

To help debug device scan issues, add verbose option to btrfs device scan.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 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;
 
 	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;
 	}
-- 
2.21.0 (Apple Git-120)

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

[Nikolay Borisov]

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 ?


> ---
>  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;
>  	}
> 

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

[Anand Jain]

> On 15 Jul 2019, at 11:09 PM, Nikolay Borisov <nborisov@suse.com> 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.
> 

To know what are the devices path scanned.

>  We get the path in case of errors,

No. We get the devices path when we use -v option and the cli [1] can be success / fail.
[1] 'btrfs device scan -v'

> in case of success what good could the path be ?



> 
>> ---
>> 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.

yep. Will do.

Thanks, Anand

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

[Qu Wenruo]

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;
>>  	}
>>

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

[Anand Jain]

> On 16 Jul 2019, at 9:26 AM, Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
> 
> 
> 
> 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.

 As the objective is to show the devices scanned with
 its device path, so IMO verbose would be suitable?  Its a small change,
 can be modified at the time of integration. Will send V2 with a nit fix
 to use bool instead of int.

Thanks, Anand

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

[Anand Jain]

To help debug device scan issues, add verbose option to btrfs device scan.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v2: Use bool instead of int as a btrfs_scan_device() argument.

 cmds/device.c        | 8 ++++++--
 cmds/filesystem.c    | 2 +-
 common/device-scan.c | 4 +++-
 common/device-scan.h | 3 ++-
 common/utils.c       | 2 +-
 disk-io.c            | 2 +-
 6 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/cmds/device.c b/cmds/device.c
index 24158308a41b..9b715ffc42a3 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;
+	bool verbose = false;
 
 	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 = true;
+			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..02d47a43a792 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(false);
 	}
 
 	if (ret) {
diff --git a/common/device-scan.c b/common/device-scan.c
index 2c5ae225f710..71c91dee6a86 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(bool 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..3e473c48d1af 100644
--- a/common/device-scan.h
+++ b/common/device-scan.h
@@ -1,6 +1,7 @@
 #ifndef __DEVICE_SCAN_H__
 #define __DEVICE_SCAN_H__
 
+#include <stdbool.h>
 #include "kerncompat.h"
 #include "ioctl.h"
 
@@ -29,7 +30,7 @@ struct seen_fsid {
 	int fd;
 };
 
-int btrfs_scan_devices(void);
+int btrfs_scan_devices(bool 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..07648f07fbd4 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(false);
 		if (ret)
 			return ret;
 	}
diff --git a/disk-io.c b/disk-io.c
index be44eead5cef..71ed78b671da 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(false);
 		if (ret)
 			return ret;
 	}
-- 
2.21.0 (Apple Git-120)

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

[Nikolay Borisov]

On 16.07.19 г. 4:26 ч., Qu Wenruo wrote:
> 
> 
> 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.

In this case (and I already debugged one such case) it's invaluable to
use LIBBLKID_DEBUG environment variable, the debug string added in this
patch won't help in this particular case.

<snip>

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

[Anand Jain]

> On 16 Jul 2019, at 2:46 PM, Nikolay Borisov <nborisov@suse.com> wrote:
> 
> 
> 
> On 16.07.19 г. 4:26 ч., Qu Wenruo wrote:
>> 
>> 
>> 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.
> 
> In this case (and I already debugged one such case) it's invaluable to
> use LIBBLKID_DEBUG environment variable, the debug string added in this
> patch won't help in this particular case.
> 
> <snip>

Export LIBBLKID_DEBUG=all is good for debugging libblkid itself,
btrfs dev scan -v provides a confirmation on which devices
were scanned and their paths.

Thanks, Anand

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

[Anand Jain]

Ping?

Thanks, Anand


On 16/7/19 11:05 AM, 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>
> ---
> v2: Use bool instead of int as a btrfs_scan_device() argument.
> 
>   cmds/device.c        | 8 ++++++--
>   cmds/filesystem.c    | 2 +-
>   common/device-scan.c | 4 +++-
>   common/device-scan.h | 3 ++-
>   common/utils.c       | 2 +-
>   disk-io.c            | 2 +-
>   6 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/cmds/device.c b/cmds/device.c
> index 24158308a41b..9b715ffc42a3 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;
> +	bool verbose = false;
>   
>   	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 = true;
> +			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..02d47a43a792 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(false);
>   	}
>   
>   	if (ret) {
> diff --git a/common/device-scan.c b/common/device-scan.c
> index 2c5ae225f710..71c91dee6a86 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(bool 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..3e473c48d1af 100644
> --- a/common/device-scan.h
> +++ b/common/device-scan.h
> @@ -1,6 +1,7 @@
>   #ifndef __DEVICE_SCAN_H__
>   #define __DEVICE_SCAN_H__
>   
> +#include <stdbool.h>
>   #include "kerncompat.h"
>   #include "ioctl.h"
>   
> @@ -29,7 +30,7 @@ struct seen_fsid {
>   	int fd;
>   };
>   
> -int btrfs_scan_devices(void);
> +int btrfs_scan_devices(bool 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..07648f07fbd4 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(false);
>   		if (ret)
>   			return ret;
>   	}
> diff --git a/disk-io.c b/disk-io.c
> index be44eead5cef..71ed78b671da 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(false);
>   		if (ret)
>   			return ret;
>   	}
>