[RFC PATCH 00/14] btrfs-progs: global-verbose option

[RFC PATCH 00/14] btrfs-progs: global-verbose option

[Anand Jain]

This patch set brings --verbose option to the top level btrfs command,
such as 'btrfs --verbose'. With this we don't have to add or remember
verbose option at the sub-commands level.

As there are already verbose options to 11 sub-commands as listed
below [1][2]. So the top level --verbose option here takes care to transpire
verbose request from the top level to the sub-command level for 9 (not 11)
sub-commands as in [1] as of now.

This patch is RFC still for the following two reasons (comments appreciated).

1.
The sub-commands as in [2] uses multi-level compile time verbose option,
such as %g_verbose = 0 (quite), %g_verbose = 1 (default), %g_verbose > 1
(real-verbose). And verbose at default is also part the .out files in
fstests. So it needs further discussions on how to handle the multi-
level verbose option using the global verbose option, and so sub-
commands in [2] are untouched.

2.
These patch has been unit-tested individually.
These patches does not alter the verbose output.
But it fixes the indentation in the command's help output, which may be
used in fstests and btrfs-progs/tests and their verification is pending
still, which I am planning to do it before v1.

[1]
btrfs subvolume delete --help
        -v|--verbose           verbose output of operations
btrfs filesystem defragment --help
        -v                  be verbose
btrfs balance start --help
        -v|--verbose        be verbose
btrfs balance status --help
        -v|--verbose        be verbose
btrfs rescue chunk-recover --help
        -v      Verbose mode
btrfs rescue super-recover --help
        -v      Verbose mode
btrfs restore --help
        -v|--verbose         verbose
btrfs inspect-internal inode-resolve --help
        -v   verbose mode
btrfs inspect-internal logical-resolve --help
        -v          verbose mode

[2]
btrfs send --help
        -v|--verbose     enable verbose output to stderr, each occurrence of
btrfs receive --help
        -v               increase verbosity about performed action

Anand Jain (14):
  btrfs-progs: add global verbose helper functions
  btrfs-progs: migrate subvolume delete to global verbose
  btrfs-progs: migrate filesystem defragment to global verbose
  btrfs-progs: migrate btrfs balance start to global verbose
  btrfs-progs: migrate balance status to global verbose
  btrfs-progs: fix help, show long option in balance start and status
  btrfs-progs: migrate rescue chunk-recover to global verbose
  btrfs-progs: migrate rescue super-recover to global verbose
  btrfs-progs: restore: delete unreachable code
  btrfs-progs: migrate restore to global verbose
  btrfs-progs: migrate inspect-internal inode-resolve to global verbose
  btrfs-progs: migrate inspect-internal logical-resolve to global
    verbose
  btrfs-progs: refactor btrfs_scan_devices() to accept verbose argument
  btrfs-progs: enable verbose for btrfs device scan

 btrfs.c              | 12 ++++++--
 cmds/balance.c       | 29 +++++++++---------
 cmds/device.c        |  2 +-
 cmds/filesystem.c    | 27 ++++++++---------
 cmds/inspect.c       | 36 +++++++++++-----------
 cmds/rescue.c        | 22 +++++++-------
 cmds/restore.c       | 86 +++++++++++++++++++++-------------------------------
 cmds/subvolume.c     | 35 +++++++++++----------
 common/device-scan.c |  4 ++-
 common/device-scan.h |  2 +-
 common/help.h        |  8 +++++
 common/messages.c    | 19 ++++++++++++
 common/messages.h    |  5 +++
 common/utils.c       |  2 +-
 disk-io.c            |  2 +-
 15 files changed, 155 insertions(+), 136 deletions(-)

-- 
1.8.3.1

[RFC PATCH 01/14] btrfs-progs: add global verbose helper functions

[Anand Jain]

The idea is to use the global --verbose command option to show
verbose output from the sub-commands. This patch adds a global
bool variable, %global_verbose, to transpire the verbose requisites
to the sub-command level. And provides pr_verbose() helper
function to log the verbose messages.

Suggested-by: David Sterba <dsterba@suse.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 btrfs.c           | 12 ++++++++++--
 common/help.h     |  8 ++++++++
 common/messages.c | 19 +++++++++++++++++++
 common/messages.h |  5 +++++
 4 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/btrfs.c b/btrfs.c
index 72dad6fb3983..ac10d8110495 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -26,8 +26,10 @@
 #include "common/help.h"
 #include "common/box.h"
 
+extern bool global_verbose;
+
 static const char * const btrfs_cmd_group_usage[] = {
-	"btrfs [--help] [--version] [--format <format>] <group> [<group>...] <command> [<args>]",
+	"btrfs [--help] [--version] [--format <format>] [--verbose] <group> [<group>...] <command> [<args>]",
 	NULL
 };
 
@@ -242,12 +244,13 @@ static void handle_output_format(const char *format)
  */
 static int handle_global_options(int argc, char **argv)
 {
-	enum { OPT_HELP = 256, OPT_VERSION, OPT_FULL, OPT_FORMAT };
+	enum { OPT_HELP = 256, OPT_VERSION, OPT_FULL, OPT_FORMAT, OPT_VERBOSE };
 	static const struct option long_options[] = {
 		{ "help", no_argument, NULL, OPT_HELP },
 		{ "version", no_argument, NULL, OPT_VERSION },
 		{ "format", required_argument, NULL, OPT_FORMAT },
 		{ "full", no_argument, NULL, OPT_FULL },
+		{ "verbose", no_argument, NULL, OPT_VERBOSE },
 		{ NULL, 0, NULL, 0}
 	};
 	int shift;
@@ -270,6 +273,7 @@ static int handle_global_options(int argc, char **argv)
 		case OPT_FORMAT:
 			handle_output_format(optarg);
 			break;
+		case OPT_VERBOSE: break;
 		default:
 			fprintf(stderr, "Unknown global option: %s\n",
 					argv[optind - 1]);
@@ -310,6 +314,10 @@ static void handle_special_globals(int shift, int argc, char **argv)
 			cmd_execute(&cmd_struct_version, argc, argv);
 			exit(0);
 		}
+
+	for (i = 0; i < shift; i++)
+		if (strcmp(argv[i], "--verbose") == 0)
+			global_verbose = true;
 }
 
 static const struct cmd_group btrfs_cmd_group = {
diff --git a/common/help.h b/common/help.h
index 01dfc68a7c8d..7bb3074b0be6 100644
--- a/common/help.h
+++ b/common/help.h
@@ -53,6 +53,14 @@
 	"-t|--tbytes        show sizes in TiB, or TB with --si"
 
 /*
+ * Global verbose option for the sub-commands
+ */
+#define HELPINFO_INSERT_VERBOSE							\
+	"-v|--verbose       show verbose output"
+#define HELPINFO_INSERT_VERBOSE_SHORT						\
+	"-v                 show verbose output"
+
+/*
  * Special marker in the help strings that will preemptively insert the global
  * options and then continue with the following text that possibly follows
  * after the regular options
diff --git a/common/messages.c b/common/messages.c
index 0e5694ecd467..e14c112ebbbf 100644
--- a/common/messages.c
+++ b/common/messages.c
@@ -16,6 +16,7 @@
 
 #include <stdio.h>
 #include <stdarg.h>
+#include <stdbool.h>
 #include "common/messages.h"
 
 __attribute__ ((format (printf, 1, 2)))
@@ -75,3 +76,21 @@ int __btrfs_error_on(int condition, const char *fmt, ...)
 
 	return 1;
 }
+
+bool global_verbose = false;
+
+__attribute__ ((format (printf, 2, 3)))
+void pr_verbose(bool condition, const char *fmt, ...)
+{
+	va_list args;
+
+	if (condition == false)
+		return;
+
+	if (global_verbose == false)
+		return;
+
+	va_start(args, fmt);
+	vfprintf(stdout, fmt, args);
+	va_end(args);
+}
diff --git a/common/messages.h b/common/messages.h
index 596047948fef..a14e2d62f3a0 100644
--- a/common/messages.h
+++ b/common/messages.h
@@ -14,6 +14,8 @@
  * Boston, MA 021110-1307, USA.
  */
 
+#include <stdbool.h>
+
 #ifndef __BTRFS_MESSAGES_H__
 #define __BTRFS_MESSAGES_H__
 
@@ -96,3 +98,6 @@ __attribute__ ((format (printf, 2, 3)))
 int __btrfs_error_on(int condition, const char *fmt, ...);
 
 #endif
+
+__attribute__ ((format (printf, 2, 3)))
+void pr_verbose(bool condition, const char *fmt, ...);
-- 
1.8.3.1

[RFC PATCH 02/14] btrfs-progs: migrate subvolume delete to global verbose

[Anand Jain]

btrfs subvolume delete already supports verbose at the sub-command
level, this patch restores same verbose which can be either enabled
by the sub-command or from the top level command.

Suggested-by: David Sterba <dsterba@suse.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds/subvolume.c | 35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/cmds/subvolume.c b/cmds/subvolume.c
index 7a5fd79bb1f3..18efd0cf6e4a 100644
--- a/cmds/subvolume.c
+++ b/cmds/subvolume.c
@@ -43,6 +43,8 @@
 #include "common/path-utils.h"
 #include "common/device-scan.h"
 
+extern bool global_verbose;
+
 static int wait_for_subvolume_cleaning(int fd, size_t count, uint64_t *ids,
 				       int sleep_interval)
 {
@@ -231,9 +233,9 @@ static const char * const cmd_subvol_delete_usage[] = {
 	"after a crash). Use one of the --commit options to wait until the",
 	"operation is safely stored on the media.",
 	"",
-	"-c|--commit-after      wait for transaction commit at the end of the operation",
-	"-C|--commit-each       wait for transaction commit after deleting each subvolume",
-	"-v|--verbose           verbose output of operations",
+	"-c|--commit-after  wait for transaction commit at the end of the operation",
+	"-C|--commit-each   wait for transaction commit after deleting each subvolume",
+	HELPINFO_INSERT_VERBOSE,
 	NULL
 };
 
@@ -248,7 +250,6 @@ static int cmd_subvol_delete(const struct cmd_struct *cmd,
 	char	*dupvname = NULL;
 	char	*path;
 	DIR	*dirstream = NULL;
-	int verbose = 0;
 	int commit_mode = 0;
 	u8 fsid[BTRFS_FSID_SIZE];
 	char uuidbuf[BTRFS_UUID_UNPARSED_SIZE];
@@ -278,7 +279,7 @@ static int cmd_subvol_delete(const struct cmd_struct *cmd,
 			commit_mode = COMMIT_EACH;
 			break;
 		case 'v':
-			verbose++;
+			global_verbose = true;
 			break;
 		default:
 			usage_unknown_option(cmd, argv);
@@ -288,11 +289,9 @@ static int cmd_subvol_delete(const struct cmd_struct *cmd,
 	if (check_argc_min(argc - optind, 1))
 		return 1;
 
-	if (verbose > 0) {
-		printf("Transaction commit: %s\n",
-			!commit_mode ? "none (default)" :
-			commit_mode == COMMIT_AFTER ? "at the end" : "after each");
-	}
+	pr_verbose(global_verbose, "Transaction commit: %s\n",
+		   !commit_mode ? "none (default)" :
+		   commit_mode == COMMIT_AFTER ? "at the end" : "after each");
 
 	cnt = optind;
 
@@ -353,11 +352,10 @@ again:
 		}
 
 		if (add_seen_fsid(fsid, seen_fsid_hash, fd, dirstream) == 0) {
-			if (verbose > 0) {
-				uuid_unparse(fsid, uuidbuf);
-				printf("  new fs is found for '%s', fsid: %s\n",
-						path, uuidbuf);
-			}
+			uuid_unparse(fsid, uuidbuf);
+			pr_verbose(global_verbose,
+				   "  new fs is found for '%s', fsid: %s\n",
+				   path, uuidbuf);
 			/*
 			 * This is the first time a subvolume on this
 			 * filesystem is deleted, keep fd in order to issue
@@ -398,10 +396,11 @@ keep_fd:
 			"unable to do final sync after deletion: %m, fsid: %s",
 						uuidbuf);
 					ret = 1;
-				} else if (verbose > 0) {
+				} else {
 					uuid_unparse(seen->fsid, uuidbuf);
-					printf("final sync is done for fsid: %s\n",
-						uuidbuf);
+					pr_verbose(global_verbose,
+						   "final sync is done for fsid: %s\n",
+						   uuidbuf);
 				}
 				seen = seen->next;
 			}
-- 
1.8.3.1

[RFC PATCH 03/14] btrfs-progs: migrate filesystem defragment to global verbose

[Anand Jain]

btrfs filesystem deframent already supports local sub-command
verbose option, enable the same when the global verbose option is set.
And as well make sure the same remains enabled at the local level.

Suggested-by: David Sterba <dsterba@suse.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds/filesystem.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/cmds/filesystem.c b/cmds/filesystem.c
index 4f22089abeaa..ee4d366fbf64 100644
--- a/cmds/filesystem.c
+++ b/cmds/filesystem.c
@@ -832,13 +832,13 @@ static const char * const cmd_filesystem_defrag_usage[] = {
 	"btrfs filesystem defragment [options] <file>|<dir> [<file>|<dir>...]",
 	"Defragment a file or a directory",
 	"",
-	"-v                  be verbose",
-	"-r                  defragment files recursively",
-	"-c[zlib,lzo,zstd]   compress the file while defragmenting",
-	"-f                  flush data to disk immediately after defragmenting",
-	"-s start            defragment only from byte onward",
-	"-l len              defragment only up to len bytes",
-	"-t size             target extent size hint (default: 32M)",
+	HELPINFO_INSERT_VERBOSE_SHORT,
+	"-r                 defragment files recursively",
+	"-c[zlib,lzo,zstd]  compress the file while defragmenting",
+	"-f                 flush data to disk immediately after defragmenting",
+	"-s start           defragment only from byte onward",
+	"-l len             defragment only up to len bytes",
+	"-t size            target extent size hint (default: 32M)",
 	"",
 	"Warning: most Linux kernels will break up the ref-links of COW data",
 	"(e.g., files copied with 'cp --reflink', snapshots) which may cause",
@@ -848,7 +848,7 @@ static const char * const cmd_filesystem_defrag_usage[] = {
 };
 
 static struct btrfs_ioctl_defrag_range_args defrag_global_range;
-static int defrag_global_verbose;
+extern bool global_verbose;
 static int defrag_global_errors;
 static int defrag_callback(const char *fpath, const struct stat *sb,
 		int typeflag, struct FTW *ftwbuf)
@@ -857,8 +857,7 @@ static int defrag_callback(const char *fpath, const struct stat *sb,
 	int fd = 0;
 
 	if ((typeflag == FTW_F) && S_ISREG(sb->st_mode)) {
-		if (defrag_global_verbose)
-			printf("%s\n", fpath);
+		pr_verbose(global_verbose, "%s\n", fpath);
 		fd = open(fpath, defrag_open_mode);
 		if (fd < 0) {
 			goto error;
@@ -913,7 +912,6 @@ static int cmd_filesystem_defrag(const struct cmd_struct *cmd,
 	thresh = SZ_32M;
 
 	defrag_global_errors = 0;
-	defrag_global_verbose = 0;
 	defrag_global_errors = 0;
 	optind = 0;
 	while(1) {
@@ -931,7 +929,7 @@ static int cmd_filesystem_defrag(const struct cmd_struct *cmd,
 			flush = 1;
 			break;
 		case 'v':
-			defrag_global_verbose = 1;
+			global_verbose = true;
 			break;
 		case 's':
 			start = parse_size(optarg);
@@ -1031,8 +1029,7 @@ static int cmd_filesystem_defrag(const struct cmd_struct *cmd,
 			/* errors are handled in the callback */
 			ret = 0;
 		} else {
-			if (defrag_global_verbose)
-				printf("%s\n", argv[i]);
+			pr_verbose(global_verbose, "%s\n", argv[i]);
 			ret = ioctl(fd, BTRFS_IOC_DEFRAG_RANGE,
 					&defrag_global_range);
 			defrag_err = errno;
-- 
1.8.3.1

[RFC PATCH 04/14] btrfs-progs: migrate btrfs balance start to global verbose

[Anand Jain]

Make sure the sub command balance start calls verbose when the global
verbose is set and vise versa.

Suggested-by: David Sterba <dsterba@suse.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds/balance.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/cmds/balance.c b/cmds/balance.c
index 32830002f3a0..7e84efd6a80d 100644
--- a/cmds/balance.c
+++ b/cmds/balance.c
@@ -34,6 +34,8 @@
 #include "common/utils.h"
 #include "common/help.h"
 
+extern bool global_verbose;
+
 static const char * const balance_cmd_group_usage[] = {
 	"btrfs balance <command> [options] <path>",
 	"btrfs balance <path>",
@@ -487,14 +489,13 @@ static const char * const cmd_balance_start_usage[] = {
 	"long operation and the user is warned before this start, with",
 	"a delay to stop it.",
 	"",
-	"-d[filters]    act on data chunks",
-	"-m[filters]    act on metadata chunks",
-	"-s[filters]    act on system chunks (only under -f)",
-	"-v             be verbose",
-	"-f             force a reduction of metadata integrity",
-	"--full-balance do not print warning and do not delay start",
-	"--background|--bg",
-	"               run the balance as a background process",
+	"-d[filters]        act on data chunks",
+	"-m[filters]        act on metadata chunks",
+	"-s[filters]        act on system chunks (only under -f)",
+	HELPINFO_INSERT_VERBOSE_SHORT,
+	"-f                 force a reduction of metadata integrity",
+	"--full-balance     do not print warning and do not delay start",
+	"--background|--bg  run the balance as a background process",
 	NULL
 };
 
@@ -505,7 +506,6 @@ static int cmd_balance_start(const struct cmd_struct *cmd,
 	struct btrfs_balance_args *ptrs[] = { &args.data, &args.sys,
 						&args.meta, NULL };
 	int force = 0;
-	int verbose = 0;
 	int background = 0;
 	unsigned start_flags = 0;
 	int i;
@@ -560,7 +560,7 @@ static int cmd_balance_start(const struct cmd_struct *cmd,
 			force = 1;
 			break;
 		case 'v':
-			verbose = 1;
+			global_verbose = true;
 			break;
 		case GETOPT_VAL_FULL_BALANCE:
 			start_flags |= BALANCE_START_NOWARN;
@@ -636,7 +636,7 @@ static int cmd_balance_start(const struct cmd_struct *cmd,
 
 	if (force)
 		args.flags |= BTRFS_BALANCE_FORCE;
-	if (verbose)
+	if (global_verbose)
 		dump_ioctl_balance_args(&args);
 	if (background) {
 		switch (fork()) {
-- 
1.8.3.1

[RFC PATCH 05/14] btrfs-progs: migrate balance status to global verbose

[Anand Jain]

Make sure top level verbose option can enable the blalance status
subcommand's verbose option.

Suggested-by: David Sterba <dsterba@suse.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds/balance.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/cmds/balance.c b/cmds/balance.c
index 7e84efd6a80d..d4916c5fb34e 100644
--- a/cmds/balance.c
+++ b/cmds/balance.c
@@ -822,7 +822,7 @@ static const char * const cmd_balance_status_usage[] = {
 	"btrfs balance status [-v] <path>",
 	"Show status of running or paused balance",
 	"",
-	"-v     be verbose",
+	HELPINFO_INSERT_VERBOSE_SHORT,
 	NULL
 };
 
@@ -839,7 +839,6 @@ static int cmd_balance_status(const struct cmd_struct *cmd,
 	const char *path;
 	DIR *dirstream = NULL;
 	int fd;
-	int verbose = 0;
 	int ret;
 
 	optind = 0;
@@ -856,7 +855,7 @@ static int cmd_balance_status(const struct cmd_struct *cmd,
 
 		switch (opt) {
 		case 'v':
-			verbose = 1;
+			global_verbose = true;
 			break;
 		default:
 			usage_unknown_option(cmd, argv);
@@ -902,7 +901,7 @@ static int cmd_balance_status(const struct cmd_struct *cmd,
 	       (unsigned long long)args.stat.considered,
 	       100 * (1 - (float)args.stat.completed/args.stat.expected));
 
-	if (verbose)
+	if (global_verbose)
 		dump_ioctl_balance_args(&args);
 
 	ret = 1;
-- 
1.8.3.1

[RFC PATCH 06/14] btrfs-progs: fix help, show long option in balance start and status

[Anand Jain]

btrfs balance start|status support both short and long option
-v|--verbose however failed to show it in its --help. This patch fixes
the --help.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds/balance.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cmds/balance.c b/cmds/balance.c
index d4916c5fb34e..06bab9f7f96f 100644
--- a/cmds/balance.c
+++ b/cmds/balance.c
@@ -492,7 +492,7 @@ static const char * const cmd_balance_start_usage[] = {
 	"-d[filters]        act on data chunks",
 	"-m[filters]        act on metadata chunks",
 	"-s[filters]        act on system chunks (only under -f)",
-	HELPINFO_INSERT_VERBOSE_SHORT,
+	HELPINFO_INSERT_VERBOSE,
 	"-f                 force a reduction of metadata integrity",
 	"--full-balance     do not print warning and do not delay start",
 	"--background|--bg  run the balance as a background process",
@@ -822,7 +822,7 @@ static const char * const cmd_balance_status_usage[] = {
 	"btrfs balance status [-v] <path>",
 	"Show status of running or paused balance",
 	"",
-	HELPINFO_INSERT_VERBOSE_SHORT,
+	HELPINFO_INSERT_VERBOSE,
 	NULL
 };
 
-- 
1.8.3.1

[RFC PATCH 07/14] btrfs-progs: migrate rescue chunk-recover to global verbose

[Anand Jain]

Now with this patch the btrfs rescue chunk-recover can display verbose
output either at the sub-command level or at the top level. For example
'btrfs --verbose rescue chunk-recover <>' or 'btrfs rescue chunk-recover
-v <>'.

Suggested-by: David Sterba <dsterba@suse.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds/rescue.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/cmds/rescue.c b/cmds/rescue.c
index e8eab6808bc3..1785bc164264 100644
--- a/cmds/rescue.c
+++ b/cmds/rescue.c
@@ -28,6 +28,8 @@
 #include "common/help.h"
 #include "cmds/rescue.h"
 
+extern bool global_verbose;
+
 static const char * const rescue_cmd_group_usage[] = {
 	"btrfs rescue <command> [options] <path>",
 	NULL
@@ -37,9 +39,9 @@ static const char * const cmd_rescue_chunk_recover_usage[] = {
 	"btrfs rescue chunk-recover [options] <device>",
 	"Recover the chunk tree by scanning the devices one by one.",
 	"",
-	"-y	Assume an answer of `yes' to all questions",
-	"-v	Verbose mode",
-	"-h	Help",
+	"-y	               Assume an answer of `yes' to all questions",
+	HELPINFO_INSERT_VERBOSE_SHORT,
+	"-h	               Help",
 	NULL
 };
 
@@ -49,7 +51,6 @@ static int cmd_rescue_chunk_recover(const struct cmd_struct *cmd,
 	int ret = 0;
 	char *file;
 	int yes = 0;
-	int verbose = 0;
 
 	optind = 0;
 	while (1) {
@@ -61,7 +62,7 @@ static int cmd_rescue_chunk_recover(const struct cmd_struct *cmd,
 			yes = 1;
 			break;
 		case 'v':
-			verbose = 1;
+			global_verbose = true;
 			break;
 		default:
 			usage_unknown_option(cmd, argv);
@@ -83,7 +84,7 @@ static int cmd_rescue_chunk_recover(const struct cmd_struct *cmd,
 		return 1;
 	}
 
-	ret = btrfs_recover_chunk_tree(file, verbose, yes);
+	ret = btrfs_recover_chunk_tree(file, global_verbose, yes);
 	if (!ret) {
 		fprintf(stdout, "Chunk tree recovered successfully\n");
 	} else if (ret > 0) {
-- 
1.8.3.1

[RFC PATCH 08/14] btrfs-progs: migrate rescue super-recover to global verbose

[Anand Jain]

Now with this patch 'btrfs rescue super-recover' can show verbose output
either by the top level --verbose option or by the sub-command -v
option.

Suggested-by: David Sterba <dsterba@suse.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds/rescue.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/cmds/rescue.c b/cmds/rescue.c
index 1785bc164264..bd11241478a8 100644
--- a/cmds/rescue.c
+++ b/cmds/rescue.c
@@ -101,8 +101,8 @@ static const char * const cmd_rescue_super_recover_usage[] = {
 	"btrfs rescue super-recover [options] <device>",
 	"Recover bad superblocks from good copies",
 	"",
-	"-y	Assume an answer of `yes' to all questions",
-	"-v	Verbose mode",
+	"-y                 Assume an answer of `yes' to all questions",
+	HELPINFO_INSERT_VERBOSE_SHORT,
 	NULL
 };
 
@@ -118,7 +118,6 @@ static int cmd_rescue_super_recover(const struct cmd_struct *cmd,
 				    int argc, char **argv)
 {
 	int ret;
-	int verbose = 0;
 	int yes = 0;
 	char *dname;
 
@@ -129,7 +128,7 @@ static int cmd_rescue_super_recover(const struct cmd_struct *cmd,
 			break;
 		switch (c) {
 		case 'v':
-			verbose = 1;
+			global_verbose = true;
 			break;
 		case 'y':
 			yes = 1;
@@ -151,7 +150,7 @@ static int cmd_rescue_super_recover(const struct cmd_struct *cmd,
 		error("the device is busy");
 		return 1;
 	}
-	ret = btrfs_recover_superblocks(dname, verbose, yes);
+	ret = btrfs_recover_superblocks(dname, global_verbose, yes);
 	return ret;
 }
 static DEFINE_SIMPLE_COMMAND(rescue_super_recover, "super-recover");
-- 
1.8.3.1

[RFC PATCH 09/14] btrfs-progs: restore: delete unreachable code

[Anand Jain]

Maximum value of %verbose is 1 when %verbose is enabled using
'btrfs restore -v <dev> <dir>', and the code under the condition
%verbose > 1 is never reached. So delete them.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds/restore.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/cmds/restore.c b/cmds/restore.c
index c104b01aef69..79caf6734e76 100644
--- a/cmds/restore.c
+++ b/cmds/restore.c
@@ -987,9 +987,6 @@ static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
 
 	leaf = path.nodes[0];
 	while (!leaf) {
-		if (verbose > 1)
-			printf("No leaf after search, looking for the next "
-			       "leaf\n");
 		ret = next_leaf(root, &path);
 		if (ret < 0) {
 			fprintf(stderr, "Error getting next leaf %d\n",
@@ -1035,18 +1032,12 @@ static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
 			continue;
 		}
 		btrfs_item_key_to_cpu(leaf, &found_key, path.slots[0]);
-		if (found_key.objectid != key->objectid) {
-			if (verbose > 1)
-				printf("Found objectid=%Lu, key=%Lu\n",
-				       found_key.objectid, key->objectid);
+		if (found_key.objectid != key->objectid)
 			break;
-		}
-		if (found_key.type != key->type) {
-			if (verbose > 1)
-				printf("Found type=%u, want=%u\n",
-				       found_key.type, key->type);
+
+		if (found_key.type != key->type)
 			break;
-		}
+
 		dir_item = btrfs_item_ptr(leaf, path.slots[0],
 					  struct btrfs_dir_item);
 		name_ptr = (unsigned long)(dir_item + 1);
-- 
1.8.3.1

[RFC PATCH 10/14] btrfs-progs: migrate restore to global verbose

[Anand Jain]

Command btrfs restore provides local verbose option, this patch makes it
enable-able by using the global --verbose option as well.

Suggested-by: David Sterba <dsterba@suse.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds/restore.c | 69 ++++++++++++++++++++++++++--------------------------------
 1 file changed, 31 insertions(+), 38 deletions(-)

diff --git a/cmds/restore.c b/cmds/restore.c
index 79caf6734e76..3592faeb6bca 100644
--- a/cmds/restore.c
+++ b/cmds/restore.c
@@ -51,7 +51,7 @@ static char fs_name[PATH_MAX];
 static char path_name[PATH_MAX];
 static char symlink_target[PATH_MAX];
 static int get_snaps = 0;
-static int verbose = 0;
+extern bool global_verbose;
 static int restore_metadata = 0;
 static int restore_symlinks = 0;
 static int ignore_errors = 0;
@@ -375,8 +375,7 @@ static int copy_one_extent(struct btrfs_root *root, int fd,
 	if (compress == BTRFS_COMPRESS_NONE)
 		bytenr += offset;
 
-	if (verbose && offset)
-		printf("offset is %Lu\n", offset);
+	pr_verbose(global_verbose && offset, "offset is %Lu\n", offset);
 	/* we found a hole */
 	if (disk_size == 0)
 		return 0;
@@ -832,9 +831,8 @@ static int overwrite_ok(const char * path)
 		if (overwrite)
 			return 2;
 
-		if (verbose || !warn)
-			printf("Skipping existing file"
-				   " %s\n", path);
+		pr_verbose(global_verbose || !warn,
+			   "Skipping existing file %s\n", path);
 		if (!warn)
 			printf("If you wish to overwrite use -o\n");
 		warn = 1;
@@ -994,9 +992,8 @@ static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
 			goto out;
 		} else if (ret > 0) {
 			/* No more leaves to search */
-			if (verbose)
-				printf("Reached the end of the tree looking "
-				       "for the directory\n");
+			pr_verbose(global_verbose,
+		   "Reached the end of the tree looking for the directory\n");
 			ret = 0;
 			goto out;
 		}
@@ -1020,10 +1017,8 @@ static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
 					goto out;
 				} else if (ret > 0) {
 					/* No more leaves to search */
-					if (verbose)
-						printf("Reached the end of "
-						       "the tree searching the"
-						       " directory\n");
+					pr_verbose(global_verbose,
+		"Reached the end of the tree searching the directory\n");
 					ret = 0;
 					goto out;
 				}
@@ -1063,8 +1058,7 @@ static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
 			if (!overwrite_ok(path_name))
 				goto next;
 
-			if (verbose)
-				printf("Restoring %s\n", path_name);
+			pr_verbose(global_verbose, "Restoring %s\n", path_name);
 			if (dry_run)
 				goto next;
 			fd = open(path_name, O_CREAT|O_WRONLY, 0644);
@@ -1136,8 +1130,7 @@ static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
 				location.objectid = BTRFS_FIRST_FREE_OBJECTID;
 			}
 
-			if (verbose)
-				printf("Restoring %s\n", path_name);
+			pr_verbose(global_verbose, "Restoring %s\n", path_name);
 
 			errno = 0;
 			if (dry_run)
@@ -1200,8 +1193,7 @@ next:
 		}
 	}
 
-	if (verbose)
-		printf("Done searching %s\n", in_dir);
+	pr_verbose(global_verbose, "Done searching %s\n", in_dir);
 out:
 	btrfs_release_path(&path);
 	return ret;
@@ -1392,25 +1384,26 @@ static const char * const cmd_restore_usage[] = {
 	"btrfs restore [options] <device> <path> | -l <device>",
 	"Try to restore files from a damaged filesystem (unmounted)",
 	"",
-	"-s|--snapshots       get snapshots",
-	"-x|--xattr           restore extended attributes",
-	"-m|--metadata        restore owner, mode and times",
-	"-S|--symlink         restore symbolic links",
-	"-v|--verbose         verbose",
-	"-i|--ignore-errors   ignore errors",
-	"-o|--overwrite       overwrite",
-	"-t <bytenr>          tree location",
-	"-f <bytenr>          filesystem location",
-	"-u|--super <mirror>  super mirror",
-	"-r|--root <rootid>   root objectid",
-	"-d                   find dir",
-	"-l|--list-roots      list tree roots",
-	"-D|--dry-run         dry run (only list files that would be recovered)",
+	"-s|--snapshots     get snapshots",
+	"-x|--xattr         restore extended attributes",
+	"-m|--metadata      restore owner, mode and times",
+	"-S|--symlink       restore symbolic links",
+	HELPINFO_INSERT_VERBOSE,
+	"-i|--ignore-errors ignore errors",
+	"-o|--overwrite     overwrite",
+	"-t <bytenr>        tree location",
+	"-f <bytenr>        filesystem location",
+	"-u|--super <mirror>",
+	"                   super mirror",
+	"-r|--root <rootid> root objectid",
+	"-d                 find dir",
+	"-l|--list-roots    list tree roots",
+	"-D|--dry-run       dry run (only list files that would be recovered)",
 	"--path-regex <regex>",
-	"                     restore only filenames matching regex,",
-	"                     you have to use following syntax (possibly quoted):",
-	"                     ^/(|home(|/username(|/Desktop(|/.*))))$",
-	"-c                   ignore case (--path-regex only)",
+	"                   restore only filenames matching regex,",
+	"                   you have to use following syntax (possibly quoted):",
+	"                   ^/(|home(|/username(|/Desktop(|/.*))))$",
+	"-c                 ignore case (--path-regex only)",
 	NULL
 };
 
@@ -1463,7 +1456,7 @@ static int cmd_restore(const struct cmd_struct *cmd, int argc, char **argv)
 				get_snaps = 1;
 				break;
 			case 'v':
-				verbose++;
+				global_verbose = true;
 				break;
 			case 'i':
 				ignore_errors = 1;
-- 
1.8.3.1

[RFC PATCH 11/14] btrfs-progs: migrate inspect-internal inode-resolve to global verbose

[Anand Jain]

Command btrfs inspect-internal inode-resolve provides verbose option at
the sub-command level, this patch makes it enable-able by using the
global --verbose option.

Suggested-by: David Sterba <dsterba@suse.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds/inspect.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/cmds/inspect.c b/cmds/inspect.c
index 758b6e60c591..f872b471b420 100644
--- a/cmds/inspect.c
+++ b/cmds/inspect.c
@@ -33,6 +33,8 @@
 #include "btrfs-list.h"
 #include "common/help.h"
 
+extern bool global_verbose;
+
 static const char * const inspect_cmd_group_usage[] = {
 	"btrfs inspect-internal <command> <args>",
 	NULL
@@ -56,8 +58,8 @@ static int __ino_to_path_fd(u64 inum, int fd, int verbose, const char *prepend)
 		goto out;
 	}
 
-	if (verbose)
-		printf("ioctl ret=%d, bytes_left=%lu, bytes_missing=%lu, "
+	pr_verbose(global_verbose,
+			"ioctl ret=%d, bytes_left=%lu, bytes_missing=%lu, "
 			"cnt=%d, missed=%d\n", ret,
 			(unsigned long)fspath->bytes_left,
 			(unsigned long)fspath->bytes_missing,
@@ -83,7 +85,7 @@ static const char * const cmd_inspect_inode_resolve_usage[] = {
 	"btrfs inspect-internal inode-resolve [-v] <inode> <path>",
 	"Get file system paths for the given inode",
 	"",
-	"-v   verbose mode",
+	HELPINFO_INSERT_VERBOSE_SHORT,
 	NULL
 };
 
@@ -91,7 +93,6 @@ static int cmd_inspect_inode_resolve(const struct cmd_struct *cmd,
 				     int argc, char **argv)
 {
 	int fd;
-	int verbose = 0;
 	int ret;
 	DIR *dirstream = NULL;
 
@@ -103,7 +104,7 @@ static int cmd_inspect_inode_resolve(const struct cmd_struct *cmd,
 
 		switch (c) {
 		case 'v':
-			verbose = 1;
+			global_verbose = true;
 			break;
 		default:
 			usage_unknown_option(cmd, argv);
@@ -117,8 +118,8 @@ static int cmd_inspect_inode_resolve(const struct cmd_struct *cmd,
 	if (fd < 0)
 		return 1;
 
-	ret = __ino_to_path_fd(arg_strtou64(argv[optind]), fd, verbose,
-			       argv[optind+1]);
+	ret = __ino_to_path_fd(arg_strtou64(argv[optind]), fd,
+			       global_verbose ? 1 : 0, argv[optind+1]);
 	close_file_or_dir(fd, dirstream);
 	return !!ret;
 
-- 
1.8.3.1

[RFC PATCH 12/14] btrfs-progs: migrate inspect-internal logical-resolve to global verbose

[Anand Jain]

Command btrfs inspect-internal logical-resolve provides local verbose
option this patch makes it enable-able by using the global --verbose
option.

Suggested-by: David Sterba <dsterba@suse.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds/inspect.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/cmds/inspect.c b/cmds/inspect.c
index f872b471b420..3f35cdea56b5 100644
--- a/cmds/inspect.c
+++ b/cmds/inspect.c
@@ -40,7 +40,7 @@ static const char * const inspect_cmd_group_usage[] = {
 	NULL
 };
 
-static int __ino_to_path_fd(u64 inum, int fd, int verbose, const char *prepend)
+static int __ino_to_path_fd(u64 inum, int fd, const char *prepend)
 {
 	int ret;
 	int i;
@@ -118,8 +118,7 @@ static int cmd_inspect_inode_resolve(const struct cmd_struct *cmd,
 	if (fd < 0)
 		return 1;
 
-	ret = __ino_to_path_fd(arg_strtou64(argv[optind]), fd,
-			       global_verbose ? 1 : 0, argv[optind+1]);
+	ret = __ino_to_path_fd(arg_strtou64(argv[optind]), fd, argv[optind+1]);
 	close_file_or_dir(fd, dirstream);
 	return !!ret;
 
@@ -130,11 +129,11 @@ static const char * const cmd_inspect_logical_resolve_usage[] = {
 	"btrfs inspect-internal logical-resolve [-Pv] [-s bufsize] <logical> <path>",
 	"Get file system paths for the given logical address",
 	"",
-	"-P          skip the path resolving and print the inodes instead",
-	"-v          verbose mode",
-	"-s bufsize  set inode container's size. This is used to increase inode",
-	"            container's size in case it is not enough to read all the ",
-	"            resolved results. The max value one can set is 64k",
+	"-P                 skip the path resolving and print the inodes instead",
+	HELPINFO_INSERT_VERBOSE_SHORT,
+	"-s bufsize         set inode container's size. This is used to increase inode",
+	"                   container's size in case it is not enough to read all the ",
+	"                   resolved results. The max value one can set is 64k",
 	NULL
 };
 
@@ -144,7 +143,6 @@ static int cmd_inspect_logical_resolve(const struct cmd_struct *cmd,
 	int ret;
 	int fd;
 	int i;
-	int verbose = 0;
 	int getpath = 1;
 	int bytes_left;
 	struct btrfs_ioctl_logical_ino_args loi;
@@ -165,7 +163,7 @@ static int cmd_inspect_logical_resolve(const struct cmd_struct *cmd,
 			getpath = 0;
 			break;
 		case 'v':
-			verbose = 1;
+			global_verbose = true;
 			break;
 		case 's':
 			size = arg_strtou64(optarg);
@@ -200,8 +198,8 @@ static int cmd_inspect_logical_resolve(const struct cmd_struct *cmd,
 		goto out;
 	}
 
-	if (verbose)
-		printf("ioctl ret=%d, total_size=%llu, bytes_left=%lu, "
+	pr_verbose(global_verbose,
+			"ioctl ret=%d, total_size=%llu, bytes_left=%lu, "
 			"bytes_missing=%lu, cnt=%d, missed=%d\n",
 			ret, size,
 			(unsigned long)inodes->bytes_left,
@@ -251,8 +249,7 @@ static int cmd_inspect_logical_resolve(const struct cmd_struct *cmd,
 					goto out;
 				}
 			}
-			ret = __ino_to_path_fd(inum, path_fd, verbose,
-						full_path);
+			ret = __ino_to_path_fd(inum, path_fd, full_path);
 			if (path_fd != fd)
 				close_file_or_dir(path_fd, dirs);
 		} else {
-- 
1.8.3.1

[RFC PATCH 13/14] btrfs-progs: refactor btrfs_scan_devices() to accept verbose argument

[Anand Jain]

Function btrfs_scan_devices() is being used by commands such as
'btrfs filesystem' and 'btrfs device', by having the verbose argument in
the btrfs_scan_devices() we can control which threads to show the
verbose when verbose is enabled by the global verbose option.

So add an option %verbose to btrfs_scan_devices().

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds/device.c        | 2 +-
 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, 8 insertions(+), 6 deletions(-)

diff --git a/cmds/device.c b/cmds/device.c
index 24158308a41b..b429a169cd5d 100644
--- a/cmds/device.c
+++ b/cmds/device.c
@@ -354,7 +354,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(false);
 			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 ee4d366fbf64..fb6e2e998dcf 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..a5963d789f49 100644
--- a/common/device-scan.c
+++ b/common/device-scan.c
@@ -26,6 +26,7 @@
 #include <linux/limits.h>
 #include <blkid/blkid.h>
 #include <uuid/uuid.h>
+#include <stdbool.h>
 #include "kernel-lib/overflow.h"
 #include "common/path-utils.h"
 #include "common/device-scan.h"
@@ -360,7 +361,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;
@@ -404,6 +405,7 @@ int btrfs_scan_devices(void)
 			close (fd);
 			continue;
 		}
+		pr_verbose(verbose, "registered: %s\n", path);
 
 		close(fd);
 	}
diff --git a/common/device-scan.h b/common/device-scan.h
index eda2bae5c6c4..c50fe2fbf91f 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(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 6617b3ef38b1..9027de596f5d 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 a9744af90a43..33bd003167fe 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -1100,7 +1100,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;
 	}
-- 
1.8.3.1

[RFC PATCH 14/14] btrfs-progs: enable verbose for btrfs device scan

[Anand Jain]

Enable verbose output for the device scan only through the global
verbose option.

For example:
./btrfs --verbose device scan
Scanning for Btrfs filesystems
registered: /dev/sda1
registered: /dev/sda2
registered: /dev/sda3
registered: /dev/sda5
registered: /dev/sda6

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds/device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmds/device.c b/cmds/device.c
index b429a169cd5d..b38bf78a798e 100644
--- a/cmds/device.c
+++ b/cmds/device.c
@@ -354,7 +354,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(false);
+			ret = btrfs_scan_devices(true);
 			error_on(ret, "error %d while scanning", ret);
 			ret = btrfs_register_all_devices();
 			error_on(ret,
-- 
1.8.3.1

Re: [RFC PATCH 00/14] btrfs-progs: global-verbose option

[David Sterba]

On Mon, Oct 21, 2019 at 06:01:08PM +0800, Anand Jain wrote:
> This patch set brings --verbose option to the top level btrfs command,
> such as 'btrfs --verbose'. With this we don't have to add or remember
> verbose option at the sub-commands level.
> 
> As there are already verbose options to 11 sub-commands as listed
> below [1][2]. So the top level --verbose option here takes care to transpire
> verbose request from the top level to the sub-command level for 9 (not 11)
> sub-commands as in [1] as of now.
> 
> This patch is RFC still for the following two reasons (comments appreciated).
> 
> 1.
> The sub-commands as in [2] uses multi-level compile time verbose option,
> such as %g_verbose = 0 (quite), %g_verbose = 1 (default), %g_verbose > 1
> (real-verbose). And verbose at default is also part the .out files in
> fstests. So it needs further discussions on how to handle the multi-
> level verbose option using the global verbose option, and so sub-
> commands in [2] are untouched.

The idea is to unify all verbosity options. Default is 1, 0 is for quiet
(only errors are printed), the rest is up to the commands what to print
on the higher levels.

> 2.
> These patch has been unit-tested individually.
> These patches does not alter the verbose output.
> But it fixes the indentation in the command's help output, which may be
> used in fstests and btrfs-progs/tests and their verification is pending
> still, which I am planning to do it before v1.

The indentation does not need to be changed if the glboal options are
split from the per-command, like

---
usage: btrfs subvolume delete [options] <subvolume> [<subvolume>...]

    Delete subvolume(s)

    Delete subvolumes from the filesystem. The corresponding directory
    is removed instantly but the data blocks are removed later.
    The deletion does not involve full commit by default due to
    performance reasons (as a consequence, the subvolume may appear again
    after a crash). Use one of the --commit options to wait until the
    operation is safely stored on the media.

    -c|--commit-after  wait for transaction commit at the end of the operation
    -C|--commit-each   wait for transaction commit after deleting each subvolume

    Global options:
    -v|--verbose     show verbose output
---

Some commands can have long option names or the argument names make it
long in some cases, the global options could stay indented. I think
visually it'll be ok. We can introduce some way to automatically format
the options and help texts so we don't have to adjust them manually each
time, but this would be more intrusive and can be done later.

With the global verbose option there shouldbe also -q|--quiet. Both
short and long versions should be available for all commands. So the
help would look like:

---
    Global options:
    -v|--verbose     verbose output, repeat for more verbosity
    -q|--quiet       print only errors
---

In code this looks like:

          "",
          "-c|--commit-after  wait for transaction commit at the end of the operation",
          "-C|--commit-each   wait for transaction commit after deleting each subvolume",
          HELPINFO_GLOBAL_OPTIONS_HEADER,
          HELPINFO_INSERT_VERBOSE,
          NULL

#define HELPINFO_GLOBAL_OPTIONS_HEADER					\
	"",								\
	"Global options:"

and HELPINFO_INSERT_VERBOSE also contains the quiet option.

The global option value is stored in 'btrfs_config_init bconf', so
everything can access it directly.

Thanks for working on this, I'll have more comments on v2 as I probably
forgot a few more things to do, the above is the base for all further
changes.

Re: [RFC PATCH 00/14] btrfs-progs: global-verbose option

[Anand Jain]

On 10/22/19 12:12 AM, David Sterba wrote:
> On Mon, Oct 21, 2019 at 06:01:08PM +0800, Anand Jain wrote:
>> This patch set brings --verbose option to the top level btrfs command,
>> such as 'btrfs --verbose'. With this we don't have to add or remember
>> verbose option at the sub-commands level.
>>
>> As there are already verbose options to 11 sub-commands as listed
>> below [1][2]. So the top level --verbose option here takes care to transpire
>> verbose request from the top level to the sub-command level for 9 (not 11)
>> sub-commands as in [1] as of now.
>>
>> This patch is RFC still for the following two reasons (comments appreciated).
>>
>> 1.
>> The sub-commands as in [2] uses multi-level compile time verbose option,
>> such as %g_verbose = 0 (quite), %g_verbose = 1 (default), %g_verbose > 1
>> (real-verbose). And verbose at default is also part the .out files in
>> fstests. So it needs further discussions on how to handle the multi-
>> level verbose option using the global verbose option, and so sub-
>> commands in [2] are untouched.
> 
> The idea is to unify all verbosity options. Default is 1, 0 is for quiet
> (only errors are printed), the rest is up to the commands what to print
> on the higher levels.

As of now verbosity level is a compile time option. [3]

[3]
-------
cmds/send.c

  51 /*
  52  * Default is 1 for historical reasons, changing may break scripts 
that expect
  53  * the 'At subvol' message.
  54  */
  55 static int g_verbose = 1;
--------



>> 2.
>> These patch has been unit-tested individually.
>> These patches does not alter the verbose output.
>> But it fixes the indentation in the command's help output, which may be
>> used in fstests and btrfs-progs/tests and their verification is pending
>> still, which I am planning to do it before v1.
> 
> The indentation does not need to be changed if the glboal options are
> split from the per-command, like
 >
> ---
> usage: btrfs subvolume delete [options] <subvolume> [<subvolume>...]
> 
>      Delete subvolume(s)
> 
>      Delete subvolumes from the filesystem. The corresponding directory
>      is removed instantly but the data blocks are removed later.
>      The deletion does not involve full commit by default due to
>      performance reasons (as a consequence, the subvolume may appear again
>      after a crash). Use one of the --commit options to wait until the
>      operation is safely stored on the media.
> 
>      -c|--commit-after  wait for transaction commit at the end of the operation
>      -C|--commit-each   wait for transaction commit after deleting each subvolume
> 
>      Global options:
>      -v|--verbose     show verbose output
> ---


  Oh split it. ok. Makes sense.

> Some commands can have long option names or the argument names make it
> long in some cases, the global options could stay indented. I think
> visually it'll be ok. We can introduce some way to automatically format
> the options and help texts so we don't have to adjust them manually each
> time, but this would be more intrusive and can be done later.

  ok. But my pertaining question is if the sub-command verbose option
  should still remain? if no I will be happy to take it out as the same
  verbose will anyway be activated using the global verbose option.

> With the global verbose option there shouldbe also -q|--quiet. Both
> short and long versions should be available for all commands. So the
> help would look like:
> 
> ---
>      Global options:
>      -v|--verbose     verbose output, repeat for more verbosity
>      -q|--quiet       print only errors
> ---
> 
> In code this looks like:
> 
>            "",
>            "-c|--commit-after  wait for transaction commit at the end of the operation",
>            "-C|--commit-each   wait for transaction commit after deleting each subvolume",
>            HELPINFO_GLOBAL_OPTIONS_HEADER,
>            HELPINFO_INSERT_VERBOSE,
>            NULL
> 
> #define HELPINFO_GLOBAL_OPTIONS_HEADER					\
> 	"",								\
> 	"Global options:"
> 
> and HELPINFO_INSERT_VERBOSE also contains the quiet option.
> 
> The global option value is stored in 'btrfs_config_init bconf', so
> everything can access it directly.

  Oh ok.

  In the above code-snap [3].

  g_verbose = 0 and g_verbose = 1 can be mapped to the global
  -q|--quite and --verbose respectively.


  But any idea what to do with g_verbose > 1? which we support
  in send.c and receive.c. And in defrag which the patch [4] removed it.
   [4]
   [RFC PATCH 09/14] btrfs-progs: restore: delete unreachable code

  Another way is

   btrfs [--quite] [--verbose[=n]]

n=1 default
n=2 verbose

Can't imagine anything better.

Thanks for helping to shape this.

Anand

> Thanks for working on this, I'll have more comments on v2 as I probably
> forgot a few more things to do, the above is the base for all further
> changes.
> 

Re: [RFC PATCH 00/14] btrfs-progs: global-verbose option

[David Sterba]

On Tue, Oct 22, 2019 at 09:54:41AM +0800, Anand Jain wrote:
> >> 1.
> >> The sub-commands as in [2] uses multi-level compile time verbose option,
> >> such as %g_verbose = 0 (quite), %g_verbose = 1 (default), %g_verbose > 1
> >> (real-verbose). And verbose at default is also part the .out files in
> >> fstests. So it needs further discussions on how to handle the multi-
> >> level verbose option using the global verbose option, and so sub-
> >> commands in [2] are untouched.
> > 
> > The idea is to unify all verbosity options. Default is 1, 0 is for quiet
> > (only errors are printed), the rest is up to the commands what to print
> > on the higher levels.
> 
> As of now verbosity level is a compile time option. [3]
> 
> [3]
> -------
> cmds/send.c
> 
>   51 /*
>   52  * Default is 1 for historical reasons, changing may break scripts 
> that expect
>   53  * the 'At subvol' message.
>   54  */
>   55 static int g_verbose = 1;
> --------

All the specific options would need to be unified while also maintaining
backward compatibility, like the above comment. Fortunatelly, if we set
default verbosity to 1, the only thing to do here will be to convert it
to the global config.

> > Some commands can have long option names or the argument names make it
> > long in some cases, the global options could stay indented. I think
> > visually it'll be ok. We can introduce some way to automatically format
> > the options and help texts so we don't have to adjust them manually each
> > time, but this would be more intrusive and can be done later.
> 
>   ok. But my pertaining question is if the sub-command verbose option
>   should still remain? if no I will be happy to take it out as the same
>   verbose will anyway be activated using the global verbose option.

For backward compatibility, where the per-command verbosity option
exists, it must continue, but will be an alias to the global option.
This is the awkward part but that' the cost of compatibility.

> > With the global verbose option there shouldbe also -q|--quiet. Both
> > short and long versions should be available for all commands. So the
> > help would look like:
> > 
> > ---
> >      Global options:
> >      -v|--verbose     verbose output, repeat for more verbosity
> >      -q|--quiet       print only errors
> > ---
> > 
> > In code this looks like:
> > 
> >            "",
> >            "-c|--commit-after  wait for transaction commit at the end of the operation",
> >            "-C|--commit-each   wait for transaction commit after deleting each subvolume",
> >            HELPINFO_GLOBAL_OPTIONS_HEADER,
> >            HELPINFO_INSERT_VERBOSE,
> >            NULL
> > 
> > #define HELPINFO_GLOBAL_OPTIONS_HEADER					\
> > 	"",								\
> > 	"Global options:"
> > 
> > and HELPINFO_INSERT_VERBOSE also contains the quiet option.
> > 
> > The global option value is stored in 'btrfs_config_init bconf', so
> > everything can access it directly.
> 
>   Oh ok.
> 
>   In the above code-snap [3].
> 
>   g_verbose = 0 and g_verbose = 1 can be mapped to the global
>   -q|--quite and --verbose respectively.

Yes, that's right

>   But any idea what to do with g_verbose > 1? which we support
>   in send.c and receive.c. And in defrag which the patch [4] removed it.

The verbosity option can be usually repeated and increasing the level,
so 

 $ btrfs -vvv send subvol

would be the same as

 $ btrfs send -vvv subvol

>    [4]
>    [RFC PATCH 09/14] btrfs-progs: restore: delete unreachable code
> 
>   Another way is
> 
>    btrfs [--quite] [--verbose[=n]]
> 
> n=1 default
> n=2 verbose

I'm not sure I've seen the '-v=n' way of specifying verbosity and would
rather avoid optional arugment.

The code hadling -v would do

	case OPT_VERBOSE:
		bconf.verbose++;
		break;
	case OPT_QUIET:
		bconf.verbose = 0;
		break;