[v2,RESEND] btrfs-progs: add verbose option to btrfs device scan
diff mbox series

Message ID 1569398832-16277-1-git-send-email-anand.jain@oracle.com
State New
Headers show
Series
  • [v2,RESEND] btrfs-progs: add verbose option to btrfs device scan
Related show

Commit Message

Anand Jain Sept. 25, 2019, 8:07 a.m. UTC
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(-)

Comments

Anand Jain Oct. 1, 2019, 7:52 a.m. UTC | #1
Ping?


On 9/25/19 4:07 PM, 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 48dbd9e19715..a500edf0f7d7 100644
> --- a/common/device-scan.c
> +++ b/common/device-scan.c
> @@ -360,7 +360,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;
> @@ -389,6 +389,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 f2a10cccca86..9a02a80492cb 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 01314504a50a..d5b3e4f793e4 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;
>   	}
>
Graham Cobb Oct. 1, 2019, 9:41 a.m. UTC | #2
On 01/10/2019 08:52, Anand Jain wrote:
> Ping?
> 
> 
> On 9/25/19 4:07 PM, 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,

Shouldn't "--verbose" be accepted as a long version of the option? That
would mean adding it to long_options.

The usage message cmd_device_scan_usage needs to be updated to include
the new option(s).

I have tested this on my systems (4.19 kernel) and it not only works
well, it is useful to get the list of devices it finds. If you wish,
feel free to add:

Tested-by: Graham Cobb <g.btrfs@cobb.uk.net>

Graham

>> 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 48dbd9e19715..a500edf0f7d7 100644
>> --- a/common/device-scan.c
>> +++ b/common/device-scan.c
>> @@ -360,7 +360,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;
>> @@ -389,6 +389,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 f2a10cccca86..9a02a80492cb 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 01314504a50a..d5b3e4f793e4 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;
>>       }
>>
>
Anand Jain Oct. 1, 2019, 10:37 a.m. UTC | #3
> Shouldn't "--verbose" be accepted as a long version of the option? That
> would mean adding it to long_options.

For verbose we provide -v in
    btrfs balance
    btrfs convert
    btrfs filesystem
    btrfs inspect-internal
    btrfs rescue

and -v|--verbose in
    btrfs send
    btrfs restore
    btrfs subvolume

we kind of lost the consistency. But most of it is -v.

> The usage message cmd_device_scan_usage needs to be updated to include
> the new option(s).

  Will do.

> I have tested this on my systems (4.19 kernel) and it not only works
> well, it is useful to get the list of devices it finds. If you wish,
> feel free to add:

Thanks.

> Tested-by: Graham Cobb <g.btrfs@cobb.uk.net>

Thanks, Anand

> Graham
Anand Jain Oct. 2, 2019, 4:11 a.m. UTC | #4
On 10/1/19 6:37 PM, Anand Jain wrote:
> 
>> Shouldn't "--verbose" be accepted as a long version of the option? That
>> would mean adding it to long_options.
> 
> For verbose we provide -v in
>     btrfs balance
>     btrfs convert
>     btrfs filesystem
>     btrfs inspect-internal
>     btrfs rescue
> 
> and -v|--verbose in
>     btrfs send
>     btrfs restore
>     btrfs subvolume
> 
> we kind of lost the consistency. But most of it is -v.

  Added --verbose in v3. It is now consistent within
  btrfs dev scan sub-command. Thanks.

-Anand

> 
>> The usage message cmd_device_scan_usage needs to be updated to include
>> the new option(s).
> 
>   Will do.
> 
>> I have tested this on my systems (4.19 kernel) and it not only works
>> well, it is useful to get the list of devices it finds. If you wish,
>> feel free to add:
> 
> Thanks.
> 
>> Tested-by: Graham Cobb <g.btrfs@cobb.uk.net>
> 
> Thanks, Anand
> 
>> Graham

Patch
diff mbox series

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 48dbd9e19715..a500edf0f7d7 100644
--- a/common/device-scan.c
+++ b/common/device-scan.c
@@ -360,7 +360,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;
@@ -389,6 +389,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 f2a10cccca86..9a02a80492cb 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 01314504a50a..d5b3e4f793e4 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;
 	}