Re: [PATCH v5] btrfs-progs: dump-tree: add noscan option

[Nikolay Borisov <nborisov@suse.com>]

On 12.03.19 г. 16:19 ч., Hugo Mills wrote:
> On Tue, Mar 12, 2019 at 04:16:29PM +0200, Nikolay Borisov wrote:
>>
>>
>> On 12.03.19 г. 15:22 ч., Anand Jain wrote:
>>> From: Anand Jain <Anand.Jain@oracle.com>
>>>
>>> The cli 'btrfs inspect dump-tree <dev>' will scan for the partner devices
>>> if any by default.
>>>
>>> So as of now you can not inspect each mirrored device independently.
>>>
>>> This patch adds noscan option, which when used won't scan the system for
>>> the partner devices, instead it just uses the devices provided in the
>>> argument.
>>>
>>> For example:
>>>   btrfs inspect dump-tree --noscan <dev> [<dev>..]
>>>
>>> This helps to debug degraded raid1 and raid10.
>>
>> This is a much more clear and concise description of what you want to
>> achieve. However, I don't think noscan is a good name, how about
>> --use-dev or --force-dev or something like that ? I'll also like David
>> to chime in on a particular naming. Don't be quick to send another version.
> 
>    How about --one-device, or just --device <dev1> --device <dev2> ..

I like the --device option
.
> 
>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>> ---
>>> v4->v5: nit: use %m to print error string.
>>> 	changelog update.
>>> v3->v4: change the patch title.
>>> 	collapse scan_args() to its only parent cmd_inspect_dump_tree()
>>> 	(it was bit confusing).
>>> 	update the change log.
>>> 	update usage.
>>> 	update man page.
>>> v2->v3: make it scalable for more than two disks in noscan mode
>>> v1->v2: rename --degraded to --noscan
>>>
>>>  Documentation/btrfs-inspect-internal.asciidoc |  5 ++-
>>>  cmds-inspect-dump-tree.c                      | 53 +++++++++++++++++++++------
>>>  2 files changed, 45 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/Documentation/btrfs-inspect-internal.asciidoc b/Documentation/btrfs-inspect-internal.asciidoc
>>> index 381497d284b8..f9d7f1c58f00 100644
>>> --- a/Documentation/btrfs-inspect-internal.asciidoc
>>> +++ b/Documentation/btrfs-inspect-internal.asciidoc
>>> @@ -61,7 +61,7 @@ specify which mirror to print, valid values are 0, 1 and 2 and the superblock
>>>  must be present on the device with a valid signature, can be used together with
>>>  '--force'
>>>  
>>> -*dump-tree* [options] <device>::
>>> +*dump-tree* [options] <device> [device...]::
>>>  (replaces the standalone tool *btrfs-debug-tree*)
>>>  +
>>>  Dump tree structures from a given device in textual form, expand keys to human
>>> @@ -95,6 +95,9 @@ intermixed in the output
>>>  --bfs::::
>>>  use breadth-first search to print trees. the nodes are printed before all
>>>  leaves
>>> +--noscan::::
>>> +do not scan the system for other partner device(s), only use the device(s)
>>> +provided in the argument
>>>  -t <tree_id>::::
>>>  print only the tree with the specified ID, where the ID can be numerical or
>>>  common name in a flexible human readable form
>>> diff --git a/cmds-inspect-dump-tree.c b/cmds-inspect-dump-tree.c
>>> index ad5345b4f1db..37d9b29fa824 100644
>>> --- a/cmds-inspect-dump-tree.c
>>> +++ b/cmds-inspect-dump-tree.c
>>> @@ -21,6 +21,7 @@
>>>  #include <unistd.h>
>>>  #include <uuid/uuid.h>
>>>  #include <getopt.h>
>>> +#include <fcntl.h>
>>>  
>>>  #include "kerncompat.h"
>>>  #include "radix-tree.h"
>>> @@ -185,7 +186,7 @@ static u64 treeid_from_string(const char *str, const char **end)
>>>  }
>>>  
>>>  const char * const cmd_inspect_dump_tree_usage[] = {
>>> -	"btrfs inspect-internal dump-tree [options] device",
>>> +	"btrfs inspect-internal dump-tree [options] <device> [<device> ..]",
>>>  	"Dump tree structures from a given device",
>>>  	"Dump tree structures from a given device in textual form, expand keys to human",
>>>  	"readable equivalents where possible.",
>>> @@ -200,6 +201,7 @@ const char * const cmd_inspect_dump_tree_usage[] = {
>>>  	"-b|--block <block_num> print info from the specified block only",
>>>  	"-t|--tree <tree_id>    print only tree with the given id (string or number)",
>>>  	"--follow               use with -b, to show all children tree blocks of <block_num>",
>>> +	"--noscan               do not scan for the partner device(s)",
>>>  	NULL
>>>  };
>>>  
>>> @@ -214,7 +216,7 @@ int cmd_inspect_dump_tree(int argc, char **argv)
>>>  	struct btrfs_disk_key disk_key;
>>>  	struct btrfs_key found_key;
>>>  	char uuidbuf[BTRFS_UUID_UNPARSED_SIZE];
>>> -	int ret;
>>> +	int ret = 0;
>>>  	int slot;
>>>  	int extent_only = 0;
>>>  	int device_only = 0;
>>> @@ -222,6 +224,7 @@ int cmd_inspect_dump_tree(int argc, char **argv)
>>>  	int roots_only = 0;
>>>  	int root_backups = 0;
>>>  	int traverse = BTRFS_PRINT_TREE_DEFAULT;
>>> +	int dev_optind;
>>>  	unsigned open_ctree_flags;
>>>  	u64 block_only = 0;
>>>  	struct btrfs_root *tree_root_scan;
>>> @@ -239,8 +242,8 @@ int cmd_inspect_dump_tree(int argc, char **argv)
>>>  	optind = 0;
>>>  	while (1) {
>>>  		int c;
>>> -		enum { GETOPT_VAL_FOLLOW = 256, GETOPT_VAL_DFS,
>>> -		       GETOPT_VAL_BFS };
>>> +		enum { GETOPT_VAL_FOLLOW = 256, GETOPT_VAL_DFS, GETOPT_VAL_BFS,
>>> +		       GETOPT_VAL_NOSCAN};
>>>  		static const struct option long_options[] = {
>>>  			{ "extents", no_argument, NULL, 'e'},
>>>  			{ "device", no_argument, NULL, 'd'},
>>> @@ -252,6 +255,7 @@ int cmd_inspect_dump_tree(int argc, char **argv)
>>>  			{ "follow", no_argument, NULL, GETOPT_VAL_FOLLOW },
>>>  			{ "bfs", no_argument, NULL, GETOPT_VAL_BFS },
>>>  			{ "dfs", no_argument, NULL, GETOPT_VAL_DFS },
>>> +			{ "noscan", no_argument, NULL, GETOPT_VAL_NOSCAN },
>>>  			{ NULL, 0, NULL, 0 }
>>>  		};
>>>  
>>> @@ -313,24 +317,49 @@ int cmd_inspect_dump_tree(int argc, char **argv)
>>>  		case GETOPT_VAL_BFS:
>>>  			traverse = BTRFS_PRINT_TREE_BFS;
>>>  			break;
>>> +		case GETOPT_VAL_NOSCAN:
>>> +			open_ctree_flags |= OPEN_CTREE_NO_DEVICES;
>>> +			break;
>>>  		default:
>>>  			usage(cmd_inspect_dump_tree_usage);
>>>  		}
>>>  	}
>>>  
>>> -	if (check_argc_exact(argc - optind, 1))
>>> +	if (check_argc_min(argc - optind, 1))
>>>  		usage(cmd_inspect_dump_tree_usage);
>>>  
>>> -	ret = check_arg_type(argv[optind]);
>>> -	if (ret != BTRFS_ARG_BLKDEV && ret != BTRFS_ARG_REG) {
>>> +	dev_optind = optind;
>>> +	while (dev_optind < argc) {
>>> +		int fd;
>>> +		struct btrfs_fs_devices *fs_devices;
>>> +		u64 num_devices;
>>> +
>>> +		ret = check_arg_type(argv[optind]);
>>> +		if (ret != BTRFS_ARG_BLKDEV && ret != BTRFS_ARG_REG) {
>>> +			if (ret < 0) {
>>> +				errno = -ret;
>>> +				error("invalid argument %s: %m", argv[dev_optind]);
>>> +			} else {
>>> +				error("not a block device or regular file: %s",
>>> +				       argv[dev_optind]);
>>> +			}
>>> +		}
>>> +		fd = open(argv[dev_optind], O_RDONLY);
>>> +		if (fd < 0) {
>>> +			error("cannot open %s: %m", argv[dev_optind]);
>>> +			return -EINVAL;
>>> +		}
>>> +		ret = btrfs_scan_one_device(fd, argv[dev_optind], &fs_devices,
>>> +					    &num_devices,
>>> +					    BTRFS_SUPER_INFO_OFFSET,
>>> +					    SBREAD_DEFAULT);
>>> +		close(fd);
>>>  		if (ret < 0) {
>>>  			errno = -ret;
>>> -			error("invalid argument %s: %m", argv[optind]);
>>> -		} else {
>>> -			error("not a block device or regular file: %s",
>>> -			      argv[optind]);
>>> +			error("device scan %s: %m", argv[dev_optind]);
>>> +			return ret;
>>>  		}
>>> -		goto out;
>>> +		dev_optind++;
>>>  	}
>>>  
>>>  	printf("%s\n", PACKAGE_STRING);
>>>
>