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

[Hugo Mills <hugo@carfax.org.uk>]

[-- Attachment #1: Type: text/plain, Size: 7599 bytes --]

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

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

-- 
Hugo Mills             | O tempura! O moresushi!
hugo@... carfax.org.uk |
http://carfax.org.uk/  |
PGP: E2AB1DE4          |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]