linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option
@ 2019-11-04  6:32 Anand Jain
  2019-11-04  6:32 ` [PATCH v1 01/18] btrfs-progs: receive: fix option quiet Anand Jain
                   ` (18 more replies)
  0 siblings, 19 replies; 31+ messages in thread
From: Anand Jain @ 2019-11-04  6:32 UTC (permalink / raw)
  To: linux-btrfs

v1->v1.1:
 . Fix typo in HELPINFO_INSERT_QUIET.
 . Remove #include <stdbool.h> where its no more required.
   (was needed when %bconf.verbose was declared as bool).
 . Use pr_verbose(-1,..) instead of all conditions printf()
 . Use pr_verbose(1,..) instead of pr_verbose(true,..)

verbosity sample code as in v1.1

global init
-----------
        bconf.verbose = -1; //-1:default, 0:quiet, >1:verbose

at global options
-----------------
	case 'v':
		bconf.verbose < 0 ? bconf.verbose = 1 : bconf.verbose++;
		break;
	case 'q':
		bconf.verbose = 0;
		break;

sub-command init
----------------
For send/receive only (special cases, default verbosity is 1):

	if (bconf.verbose < 0)
		bconf.verbose = 1;
	else if (bconf.verbose > 0)
		bconf.verbose++;

Non-send/receive:
default verbosity is 0 (ref: cmds/rescue.c)
	if (bconf.verbose < 0)
		bconf.verbose = 0;

at sub-command options
----------------------
	case 'v':
		bconf.verbose++;
		break;
	case 'q':
		bconf.verbose = 0;
		break;


pr_verbose()
------------
/*
 * level -1: prints message unless bconf.verbose == 0;
 * level  0: quiet
 * level >0: prints message only if <= bconf.verbose
 */
void pr_verbose(int level, const char *fmt, ...)
{
        va_list args;

        if (level == 0 || bconf.verbose == 0)
                return;

        if (level > bconf.verbose)
                return;

        va_start(args, fmt);
        vfprintf(stdout, fmt, args);
        va_end(args);
}


RFC->v1:
. Adds --quiet option to the global btrfs(8) command.
. Used global struct bconf.
. Refactored pr_verbose(), accepts level (int) as argument, so now the
sub-command can specify the verbose level at which the particular
verbose messages has to be printed.
. Added global help defines.

Kindly note the following:-

1.
 There are certain sub-commands which does not have any verbose output
 or quiet output. However if the global options were used with those
 sub-commands then the command shall not report any usage error. Or
 my question is should it error out.? For example:
  (with the patch) btrfs --verbose device ready /dev/sdb
 actually there isn't any verbose output but we won't error out.
 Similarly,
  (without the patch) btrfs send -vvvvv will not show usage error
  as well.
  So I believe this is fine. IMO.

2.
  There is slight difference in output when global options are used
  as compared to the output using the same sequence of options at the
  sub-command level. For example:

   btrfs send -v -q -v  is-equal-to  btrfs send
   But same sequence in the global option
   btrfs -v -q -v send is-not-equal-to btrfs send
   but is-equal-to btrfs -v send or btrfs send -v.
   (similarly applies to receive as well).

  which IMO is fair expectation as -v is ending last.


RFC:
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 (18):
  btrfs-progs: receive: fix option quiet
  btrfs-progs: balance status: fix usage show long verbose
  btrfs-progs: balance start: fix usage add long verbose
  btrfs-progs: add global verbose and quiet options and helper functions
  btrfs-progs: send: use global verbose and quiet options
  btrfs-progs: receive: use global verbose and quiet options
  btrfs-progs: subvolume delete: use global verbose option
  btrfs-progs: filesystem defragment: use global verbose option
  btrfs-progs: balance start: use global verbose option
  btrfs-progs: balance status: use global verbose option
  btrfs-progs: rescue chunk-recover: use global verbose option
  btrfs-progs: rescue super-recover: use global verbose option
  btrfs-progs: restore: use global verbose option
  btrfs-progs: inspect-internal inode-resolve: use global verbose
  btrfs-progs: inspect-internal logical-resolve: use global verbose
    option
  btrfs-progs: refactor btrfs_scan_devices() to accept verbose argument
  btrfs-progs: device scan: add verbose option
  btrfs-progs: device scan: add quiet option

 btrfs.c              | 20 +++++++++++--
 cmds/balance.c       | 26 +++++++++++------
 cmds/device.c        |  7 +++--
 cmds/filesystem.c    | 18 ++++++------
 cmds/inspect.c       | 45 +++++++++++++++---------------
 cmds/receive.c       | 79 +++++++++++++++++++++++++++++-----------------------
 cmds/rescue.c        | 22 +++++++++++----
 cmds/restore.c       | 57 ++++++++++++++++++-------------------
 cmds/send.c          | 33 ++++++++++++++--------
 cmds/subvolume.c     | 32 +++++++++++----------
 common/device-scan.c |  3 +-
 common/device-scan.h |  2 +-
 common/help.h        | 11 ++++++++
 common/messages.c    | 17 +++++++++++
 common/messages.h    |  3 ++
 common/utils.c       |  3 +-
 common/utils.h       |  3 ++
 disk-io.c            |  2 +-
 18 files changed, 239 insertions(+), 144 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v1 01/18] btrfs-progs: receive: fix option quiet
  2019-11-04  6:32 [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option Anand Jain
@ 2019-11-04  6:32 ` Anand Jain
  2019-11-04  6:33 ` [PATCH v1 02/18] btrfs-progs: balance status: fix usage show long verbose Anand Jain
                   ` (17 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Anand Jain @ 2019-11-04  6:32 UTC (permalink / raw)
  To: linux-btrfs

Even when -q option specified, the receive sub-command is not quiet as
show below.

 btrfs receive -q -f /tmp/t /btrfs1
 At snapshot ss3

It must be quiet atlest when its been asked to be quiet.

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

diff --git a/cmds/receive.c b/cmds/receive.c
index 4b03938ea3eb..c4827c1dd999 100644
--- a/cmds/receive.c
+++ b/cmds/receive.c
@@ -269,7 +269,8 @@ static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid,
 		goto out;
 	}
 
-	fprintf(stdout, "At snapshot %s\n", path);
+	if (g_verbose)
+		fprintf(stdout, "At snapshot %s\n", path);
 
 	memcpy(rctx->cur_subvol.received_uuid, uuid, BTRFS_UUID_SIZE);
 	rctx->cur_subvol.stransid = ctransid;
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1 02/18] btrfs-progs: balance status: fix usage show long verbose
  2019-11-04  6:32 [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option Anand Jain
  2019-11-04  6:32 ` [PATCH v1 01/18] btrfs-progs: receive: fix option quiet Anand Jain
@ 2019-11-04  6:33 ` Anand Jain
  2019-11-04  6:33 ` [PATCH v1 03/18] btrfs-progs: balance start: fix usage add " Anand Jain
                   ` (16 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Anand Jain @ 2019-11-04  6:33 UTC (permalink / raw)
  To: linux-btrfs

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

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

diff --git a/cmds/balance.c b/cmds/balance.c
index 32830002f3a0..b236fabbe236 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",
+	"-v|--verbose     be verbose",
 	NULL
 };
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1 03/18] btrfs-progs: balance start: fix usage add long verbose
  2019-11-04  6:32 [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option Anand Jain
  2019-11-04  6:32 ` [PATCH v1 01/18] btrfs-progs: receive: fix option quiet Anand Jain
  2019-11-04  6:33 ` [PATCH v1 02/18] btrfs-progs: balance status: fix usage show long verbose Anand Jain
@ 2019-11-04  6:33 ` Anand Jain
  2019-11-04  6:33 ` [PATCH v1.1 04/18] btrfs-progs: add global verbose and quiet options and helper functions Anand Jain
                   ` (15 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Anand Jain @ 2019-11-04  6:33 UTC (permalink / raw)
  To: linux-btrfs

btrfs balance start supports both short and long option -v|--verbose
however usage failed to show the long option. This patch fixes the --help.

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

diff --git a/cmds/balance.c b/cmds/balance.c
index b236fabbe236..a98cd8d6200f 100644
--- a/cmds/balance.c
+++ b/cmds/balance.c
@@ -490,7 +490,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)",
-	"-v             be verbose",
+	"-v|--verbose   be verbose",
 	"-f             force a reduction of metadata integrity",
 	"--full-balance do not print warning and do not delay start",
 	"--background|--bg",
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1.1 04/18] btrfs-progs: add global verbose and quiet options and helper functions
  2019-11-04  6:32 [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option Anand Jain
                   ` (2 preceding siblings ...)
  2019-11-04  6:33 ` [PATCH v1 03/18] btrfs-progs: balance start: fix usage add " Anand Jain
@ 2019-11-04  6:33 ` Anand Jain
  2019-11-14 16:08   ` David Sterba
  2019-11-15 15:58   ` David Sterba
  2019-11-04  6:33 ` [PATCH v1.1 05/18] btrfs-progs: send: use global verbose and quiet options Anand Jain
                   ` (14 subsequent siblings)
  18 siblings, 2 replies; 31+ messages in thread
From: Anand Jain @ 2019-11-04  6:33 UTC (permalink / raw)
  To: linux-btrfs

Add btrfs(8) global --verbose and --quiet command options to show
verbose or no output from the sub-commands.
By introducing global a %bconf::verbose memeber to transpire the same
down to the sub-command.
Further the added helper function pr_verbose() helps to logs the verbose
messages, based on the state of the %bconf::verbose. And further HELPINFO_
defines are provides for the usage.

Suggested-by: David Sterba <dsterba@suse.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v1.1: Fix typo in HELPINFO_INSERT_QUIET
      Drop stale #include <stdbool.h> we don't need it because
      %bconf.verbose is now declared as int.

 btrfs.c           | 20 ++++++++++++++++++--
 common/help.h     | 11 +++++++++++
 common/messages.c | 17 +++++++++++++++++
 common/messages.h |  3 +++
 common/utils.c    |  1 +
 common/utils.h    |  3 +++
 6 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/btrfs.c b/btrfs.c
index 6c8aabe24dc8..a97bc1858390 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -27,7 +27,7 @@
 #include "common/box.h"
 
 static const char * const btrfs_cmd_group_usage[] = {
-	"btrfs [--help] [--version] [--format <format>] <group> [<group>...] <command> [<args>]",
+	"btrfs [--help] [--version] [--format <format>] [-v|--verbose] [--quiet] <group> [<group>...] <command> [<args>]",
 	NULL
 };
 
@@ -248,6 +248,8 @@ static int handle_global_options(int argc, char **argv)
 		{ "version", no_argument, NULL, OPT_VERSION },
 		{ "format", required_argument, NULL, OPT_FORMAT },
 		{ "full", no_argument, NULL, OPT_FULL },
+		{ "verbose", no_argument, NULL, 'v' },
+		{ "quiet", no_argument, NULL, 'q' },
 		{ NULL, 0, NULL, 0}
 	};
 	int shift;
@@ -259,7 +261,7 @@ static int handle_global_options(int argc, char **argv)
 	while (1) {
 		int c;
 
-		c = getopt_long(argc, argv, "+", long_options, NULL);
+		c = getopt_long(argc, argv, "+vq", long_options, NULL);
 		if (c < 0)
 			break;
 
@@ -270,6 +272,12 @@ static int handle_global_options(int argc, char **argv)
 		case OPT_FORMAT:
 			handle_output_format(optarg);
 			break;
+		case 'v':
+			bconf.verbose < 0 ? bconf.verbose = 1 : bconf.verbose++;
+			break;
+		case 'q':
+			bconf.verbose = 0;
+			break;
 		default:
 			fprintf(stderr, "Unknown global option: %s\n",
 					argv[optind - 1]);
@@ -310,6 +318,14 @@ 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)
+			bconf.verbose < 0 ? bconf.verbose = 1 : bconf.verbose++;
+
+	for (i = 0; i < shift; i++)
+		if (strcmp(argv[i], "--quiet") == 0)
+			bconf.verbose = 0;
 }
 
 static const struct cmd_group btrfs_cmd_group = {
diff --git a/common/help.h b/common/help.h
index 01dfc68a7c8d..2618f31de712 100644
--- a/common/help.h
+++ b/common/help.h
@@ -53,6 +53,17 @@
 	"-t|--tbytes        show sizes in TiB, or TB with --si"
 
 /*
+ * Global verbose option for the sub-commands
+ */
+#define HELPINFO_GLOBAL_OPTIONS_HEADER						\
+	"",									\
+	"Global options:"
+#define HELPINFO_INSERT_VERBOSE							\
+	"-v|--verbose       show verbose output"
+#define HELPINFO_INSERT_QUIET							\
+	"-q|--quiet         run the command quietly"
+
+/*
  * 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..410630ffdd6d 100644
--- a/common/messages.c
+++ b/common/messages.c
@@ -17,6 +17,7 @@
 #include <stdio.h>
 #include <stdarg.h>
 #include "common/messages.h"
+#include "common/utils.h"
 
 __attribute__ ((format (printf, 1, 2)))
 void __btrfs_warning(const char *fmt, ...)
@@ -75,3 +76,19 @@ int __btrfs_error_on(int condition, const char *fmt, ...)
 
 	return 1;
 }
+
+__attribute__ ((format (printf, 2, 3)))
+void pr_verbose(int level, const char *fmt, ...)
+{
+	va_list args;
+
+	if (level == 0 || bconf.verbose == 0)
+		return;
+
+	if (level > bconf.verbose)
+		return;
+
+	va_start(args, fmt);
+	vfprintf(stdout, fmt, args);
+	va_end(args);
+}
diff --git a/common/messages.h b/common/messages.h
index 596047948fef..f802a9413265 100644
--- a/common/messages.h
+++ b/common/messages.h
@@ -96,3 +96,6 @@ __attribute__ ((format (printf, 2, 3)))
 int __btrfs_error_on(int condition, const char *fmt, ...);
 
 #endif
+
+__attribute__ ((format (printf, 2, 3)))
+void pr_verbose(int level, const char *fmt, ...);
diff --git a/common/utils.c b/common/utils.c
index 2cf15c333f6b..c2c6d0af0efc 100644
--- a/common/utils.c
+++ b/common/utils.c
@@ -1649,6 +1649,7 @@ u8 rand_u8(void)
 void btrfs_config_init(void)
 {
 	bconf.output_format = CMD_FORMAT_TEXT;
+	bconf.verbose = -1;
 }
 
 /* Returns total size of main memory in bytes, -1UL if error. */
diff --git a/common/utils.h b/common/utils.h
index 0ef1d6e89c2b..8774194f4e9d 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -122,6 +122,9 @@ void print_all_devices(struct list_head *devices);
  */
 struct btrfs_config {
 	unsigned int output_format;
+
+	/* -1:unset 0:quiet >0:verbose */
+	int verbose;
 };
 extern struct btrfs_config bconf;
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1.1 05/18] btrfs-progs: send: use global verbose and quiet options
  2019-11-04  6:32 [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option Anand Jain
                   ` (3 preceding siblings ...)
  2019-11-04  6:33 ` [PATCH v1.1 04/18] btrfs-progs: add global verbose and quiet options and helper functions Anand Jain
@ 2019-11-04  6:33 ` Anand Jain
  2019-11-04  6:33 ` [PATCH v1.1 06/18] btrfs-progs: receive: " Anand Jain
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Anand Jain @ 2019-11-04  6:33 UTC (permalink / raw)
  To: linux-btrfs

Transpire global --verbose and --quiet options down to the btrfs send
sub-command.

Suggested-by: David Sterba <dsterba@suse.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v1.1: Fix typo in HELPINFO_INSERT_QUIET

Global verbose-and-quiet option sequences behave differently from the
same sequence used in the local command, as show below.

Before:
btrfs.old send -q -v -f /tmp/f -p /btrfs/ss2 /btrfs/ss3
At subvol /btrfs/ss3

btrfs.old send -v -f /tmp/f -p /btrfs/ss2 /btrfs/ss3
At subvol /btrfs/ss3
BTRFS_IOC_SEND returned 0
joining genl thread

After:
btrfs.new send -q -v -f /tmp/f -p /btrfs/ss2 /btrfs/ss3
At subvol /btrfs/ss3

btrfs.new -q -v send -f /tmp/f -p /btrfs/ss2 /btrfs/ss3
At subvol /btrfs/ss3
BTRFS_IOC_SEND returned 0
joining genl thread

The (btrfs.new -q -v send) output is same as (btrfs.old send -v) which IMO
is acceptable and not a regression.

 cmds/send.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/cmds/send.c b/cmds/send.c
index 7ce6c3273857..a5b9c7698dba 100644
--- a/cmds/send.c
+++ b/cmds/send.c
@@ -48,11 +48,6 @@
 
 #define SEND_BUFFER_SIZE	SZ_64K
 
-/*
- * Default is 1 for historical reasons, changing may break scripts that expect
- * the 'At subvol' message.
- */
-static int g_verbose = 1;
 
 struct btrfs_send {
 	int send_fd;
@@ -292,10 +287,10 @@ static int do_send(struct btrfs_send *send, u64 parent_root_id,
 				"Try upgrading your kernel or don't use -e.\n");
 		goto out;
 	}
-	if (g_verbose > 1)
+	if (bconf.verbose > 1)
 		fprintf(stderr, "BTRFS_IOC_SEND returned %d\n", ret);
 
-	if (g_verbose > 1)
+	if (bconf.verbose > 1)
 		fprintf(stderr, "joining genl thread\n");
 
 	close(pipefd[1]);
@@ -460,6 +455,9 @@ static const char * const cmd_send_usage[] = {
 	"-v|--verbose     enable verbose output to stderr, each occurrence of",
 	"                 this option increases verbosity",
 	"-q|--quiet       suppress all messages, except errors",
+	HELPINFO_GLOBAL_OPTIONS_HEADER,
+	HELPINFO_INSERT_VERBOSE,
+	HELPINFO_INSERT_QUIET,
 	NULL
 };
 
@@ -482,6 +480,17 @@ static int cmd_send(const struct cmd_struct *cmd, int argc, char **argv)
 	send.dump_fd = fileno(stdout);
 	outname[0] = 0;
 
+	/*
+	 * For send, verbose default is 1 (insteasd of 0) for historical reasons,
+	 * changing may break scripts that expect the 'At subvol' message. But do
+	 * it only when bconf.verbose is unset (-1) and also adjust the value,
+	 * if global verbose is already set.
+	 */
+	if (bconf.verbose < 0)
+		bconf.verbose = 1;
+	else if (bconf.verbose > 0)
+		bconf.verbose++;
+
 	optind = 0;
 	while (1) {
 		enum { GETOPT_VAL_SEND_NO_DATA = 256 };
@@ -497,10 +506,10 @@ static int cmd_send(const struct cmd_struct *cmd, int argc, char **argv)
 
 		switch (c) {
 		case 'v':
-			g_verbose++;
+			bconf.verbose++;
 			break;
 		case 'q':
-			g_verbose = 0;
+			bconf.verbose = 0;
 			break;
 		case 'e':
 			new_end_cmd_semantic = 1;
@@ -680,8 +689,8 @@ static int cmd_send(const struct cmd_struct *cmd, int argc, char **argv)
 		}
 	}
 
-	if ((send_flags & BTRFS_SEND_FLAG_NO_FILE_DATA) && g_verbose > 1)
-		if (g_verbose > 1)
+	if ((send_flags & BTRFS_SEND_FLAG_NO_FILE_DATA) && bconf.verbose > 1)
+		if (bconf.verbose > 1)
 			fprintf(stderr, "Mode NO_FILE_DATA enabled\n");
 
 	for (i = optind; i < argc; i++) {
@@ -691,7 +700,7 @@ static int cmd_send(const struct cmd_struct *cmd, int argc, char **argv)
 		free(subvol);
 		subvol = argv[i];
 
-		if (g_verbose > 0)
+		if (bconf.verbose > 0)
 			fprintf(stderr, "At subvol %s\n", subvol);
 
 		subvol = realpath(subvol, NULL);
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1.1 06/18] btrfs-progs: receive: use global verbose and quiet options
  2019-11-04  6:32 [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option Anand Jain
                   ` (4 preceding siblings ...)
  2019-11-04  6:33 ` [PATCH v1.1 05/18] btrfs-progs: send: use global verbose and quiet options Anand Jain
@ 2019-11-04  6:33 ` Anand Jain
  2019-11-04  6:33 ` [PATCH v1.1 07/18] btrfs-progs: subvolume delete: use global verbose option Anand Jain
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Anand Jain @ 2019-11-04  6:33 UTC (permalink / raw)
  To: linux-btrfs

Transpire global --verbose and --quiet options down to the btrfs receive
sub-command.

Suggested-by: David Sterba <dsterba@suse.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v1.1: Fix typo in HELPINFO_INSERT_QUIET

Global verbose and quite option sequences behave differently from the
same sequence in the local command, as show below.

Before:
  btrfs receive -q -v -f /tmp/t /btrfs1
  At snapshot ss3

  btrfs receive -v -f /tmp/t /btrfs1
  At snapshot ss3
  receiving snapshot ss3 uuid=9d0001ec-29e4-194a-a13e-42d9f428d745, ctransid=11 parent_uuid=a6b75134-8865-f045-89d2-c2afcf794475, parent_ctransid=11
  BTRFS_IOC_SET_RECEIVED_SUBVOL uuid=9d0001ec-29e4-194a-a13e-42d9f428d745, stransid=11

After:
In the sub-command level the output remains same as before.
  btrfs receive -q -v -f /tmp/t /btrfs1
  At snapshot ss3

In the global verbose level it does it correctly
which is same as 'before: btrfs receive -v'
  btrfs -q -v receive -f /tmp/t /btrfs1
  At snapshot ss3
  receiving snapshot ss3 uuid=9d0001ec-29e4-194a-a13e-42d9f428d745, ctransid=11 parent_uuid=a6b75134-8865-f045-89d2-c2afcf794475, parent_ctransid=11
  BTRFS_IOC_SET_RECEIVED_SUBVOL uuid=9d0001ec-29e4-194a-a13e-42d9f428d745, stransid=11

btrfs -q -v receive is same as btrfs -v receive, I belive this isn't regression.

 cmds/receive.c | 80 ++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 44 insertions(+), 36 deletions(-)

diff --git a/cmds/receive.c b/cmds/receive.c
index c4827c1dd999..05718ecb0287 100644
--- a/cmds/receive.c
+++ b/cmds/receive.c
@@ -53,12 +53,6 @@
 #include "common/help.h"
 #include "common/path-utils.h"
 
-/*
- * Default is 1 for historical reasons, changing may break scripts that expect
- * the 'At subvol' message.
- */
-static int g_verbose = 1;
-
 struct btrfs_receive
 {
 	int mnt_fd;
@@ -116,7 +110,7 @@ static int finish_subvol(struct btrfs_receive *rctx)
 	memcpy(rs_args.uuid, rctx->cur_subvol.received_uuid, BTRFS_UUID_SIZE);
 	rs_args.stransid = rctx->cur_subvol.stransid;
 
-	if (g_verbose >= 2) {
+	if (bconf.verbose >= 2) {
 		uuid_unparse((u8*)rs_args.uuid, uuid_str);
 		fprintf(stderr, "BTRFS_IOC_SET_RECEIVED_SUBVOL uuid=%s, "
 				"stransid=%llu\n", uuid_str, rs_args.stransid);
@@ -199,13 +193,13 @@ static int process_subvol(const char *path, const u8 *uuid, u64 ctransid,
 		goto out;
 	}
 
-	if (g_verbose)
+	if (bconf.verbose >= 1)
 		fprintf(stderr, "At subvol %s\n", path);
 
 	memcpy(rctx->cur_subvol.received_uuid, uuid, BTRFS_UUID_SIZE);
 	rctx->cur_subvol.stransid = ctransid;
 
-	if (g_verbose >= 2) {
+	if (bconf.verbose >= 2) {
 		uuid_unparse((u8*)rctx->cur_subvol.received_uuid, uuid_str);
 		fprintf(stderr, "receiving subvol %s uuid=%s, stransid=%llu\n",
 				path, uuid_str,
@@ -269,13 +263,12 @@ static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid,
 		goto out;
 	}
 
-	if (g_verbose)
-		fprintf(stdout, "At snapshot %s\n", path);
+	pr_verbose(1, "At snapshot %s\n", path);
 
 	memcpy(rctx->cur_subvol.received_uuid, uuid, BTRFS_UUID_SIZE);
 	rctx->cur_subvol.stransid = ctransid;
 
-	if (g_verbose >= 2) {
+	if (bconf.verbose >= 2) {
 		uuid_unparse((u8*)rctx->cur_subvol.received_uuid, uuid_str);
 		fprintf(stderr, "receiving snapshot %s uuid=%s, "
 				"ctransid=%llu ", path, uuid_str,
@@ -402,7 +395,7 @@ static int process_mkfile(const char *path, void *user)
 		goto out;
 	}
 
-	if (g_verbose >= 3)
+	if (bconf.verbose >= 3)
 		fprintf(stderr, "mkfile %s\n", path);
 
 	ret = creat(full_path, 0600);
@@ -430,7 +423,7 @@ static int process_mkdir(const char *path, void *user)
 		goto out;
 	}
 
-	if (g_verbose >= 3)
+	if (bconf.verbose >= 3)
 		fprintf(stderr, "mkdir %s\n", path);
 
 	ret = mkdir(full_path, 0700);
@@ -455,7 +448,7 @@ static int process_mknod(const char *path, u64 mode, u64 dev, void *user)
 		goto out;
 	}
 
-	if (g_verbose >= 3)
+	if (bconf.verbose >= 3)
 		fprintf(stderr, "mknod %s mode=%llu, dev=%llu\n",
 				path, mode, dev);
 
@@ -481,7 +474,7 @@ static int process_mkfifo(const char *path, void *user)
 		goto out;
 	}
 
-	if (g_verbose >= 3)
+	if (bconf.verbose >= 3)
 		fprintf(stderr, "mkfifo %s\n", path);
 
 	ret = mkfifo(full_path, 0600);
@@ -506,7 +499,7 @@ static int process_mksock(const char *path, void *user)
 		goto out;
 	}
 
-	if (g_verbose >= 3)
+	if (bconf.verbose >= 3)
 		fprintf(stderr, "mksock %s\n", path);
 
 	ret = mknod(full_path, 0600 | S_IFSOCK, 0);
@@ -531,7 +524,7 @@ static int process_symlink(const char *path, const char *lnk, void *user)
 		goto out;
 	}
 
-	if (g_verbose >= 3)
+	if (bconf.verbose >= 3)
 		fprintf(stderr, "symlink %s -> %s\n", path, lnk);
 
 	ret = symlink(lnk, full_path);
@@ -563,7 +556,7 @@ static int process_rename(const char *from, const char *to, void *user)
 		goto out;
 	}
 
-	if (g_verbose >= 3)
+	if (bconf.verbose >= 3)
 		fprintf(stderr, "rename %s -> %s\n", from, to);
 
 	ret = rename(full_from, full_to);
@@ -595,7 +588,7 @@ static int process_link(const char *path, const char *lnk, void *user)
 		goto out;
 	}
 
-	if (g_verbose >= 3)
+	if (bconf.verbose >= 3)
 		fprintf(stderr, "link %s -> %s\n", path, lnk);
 
 	ret = link(full_link_path, full_path);
@@ -621,7 +614,7 @@ static int process_unlink(const char *path, void *user)
 		goto out;
 	}
 
-	if (g_verbose >= 3)
+	if (bconf.verbose >= 3)
 		fprintf(stderr, "unlink %s\n", path);
 
 	ret = unlink(full_path);
@@ -646,7 +639,7 @@ static int process_rmdir(const char *path, void *user)
 		goto out;
 	}
 
-	if (g_verbose >= 3)
+	if (bconf.verbose >= 3)
 		fprintf(stderr, "rmdir %s\n", path);
 
 	ret = rmdir(full_path);
@@ -711,7 +704,7 @@ static int process_write(const char *path, const void *data, u64 offset,
 	if (ret < 0)
 		goto out;
 
-	if (g_verbose >= 2)
+	if (bconf.verbose >= 2)
 		fprintf(stderr, "write %s - offset=%llu length=%llu\n",
 			path, offset, len);
 
@@ -819,7 +812,7 @@ static int process_clone(const char *path, u64 offset, u64 len,
 		goto out;
 	}
 
-	if (g_verbose >= 2)
+	if (bconf.verbose >= 2)
 		fprintf(stderr,
 			"clone %s - source=%s source offset=%llu offset=%llu length=%llu\n",
 			path, clone_path, clone_offset, offset, len);
@@ -860,7 +853,7 @@ static int process_set_xattr(const char *path, const char *name,
 	}
 
 	if (strcmp("security.capability", name) == 0) {
-		if (g_verbose >= 4)
+		if (bconf.verbose >= 4)
 			fprintf(stderr, "set_xattr: cache capabilities\n");
 		if (rctx->cached_capabilities_len)
 			warning("capabilities set multiple times per file: %s",
@@ -875,7 +868,7 @@ static int process_set_xattr(const char *path, const char *name,
 		memcpy(rctx->cached_capabilities, data, len);
 	}
 
-	if (g_verbose >= 3) {
+	if (bconf.verbose >= 3) {
 		fprintf(stderr, "set_xattr %s - name=%s data_len=%d "
 				"data=%.*s\n", path, name, len,
 				len, (char*)data);
@@ -905,7 +898,7 @@ static int process_remove_xattr(const char *path, const char *name, void *user)
 		goto out;
 	}
 
-	if (g_verbose >= 3) {
+	if (bconf.verbose >= 3) {
 		fprintf(stderr, "remove_xattr %s - name=%s\n",
 				path, name);
 	}
@@ -933,7 +926,7 @@ static int process_truncate(const char *path, u64 size, void *user)
 		goto out;
 	}
 
-	if (g_verbose >= 3)
+	if (bconf.verbose >= 3)
 		fprintf(stderr, "truncate %s size=%llu\n", path, size);
 
 	ret = truncate(full_path, size);
@@ -959,7 +952,7 @@ static int process_chmod(const char *path, u64 mode, void *user)
 		goto out;
 	}
 
-	if (g_verbose >= 3)
+	if (bconf.verbose >= 3)
 		fprintf(stderr, "chmod %s - mode=0%o\n", path, (int)mode);
 
 	ret = chmod(full_path, mode);
@@ -985,7 +978,7 @@ static int process_chown(const char *path, u64 uid, u64 gid, void *user)
 		goto out;
 	}
 
-	if (g_verbose >= 3)
+	if (bconf.verbose >= 3)
 		fprintf(stderr, "chown %s - uid=%llu, gid=%llu\n", path,
 				uid, gid);
 
@@ -997,7 +990,7 @@ static int process_chown(const char *path, u64 uid, u64 gid, void *user)
 	}
 
 	if (rctx->cached_capabilities_len) {
-		if (g_verbose >= 3)
+		if (bconf.verbose >= 3)
 			fprintf(stderr, "chown: restore capabilities\n");
 		ret = lsetxattr(full_path, "security.capability",
 				rctx->cached_capabilities,
@@ -1031,7 +1024,7 @@ static int process_utimes(const char *path, struct timespec *at,
 		goto out;
 	}
 
-	if (g_verbose >= 3)
+	if (bconf.verbose >= 3)
 		fprintf(stderr, "utimes %s\n", path);
 
 	tv[0] = *at;
@@ -1050,7 +1043,7 @@ out:
 static int process_update_extent(const char *path, u64 offset, u64 len,
 		void *user)
 {
-	if (g_verbose >= 3)
+	if (bconf.verbose >= 3)
 		fprintf(stderr, "update_extent %s: offset=%llu, len=%llu\n",
 				path, (unsigned long long)offset,
 				(unsigned long long)len);
@@ -1190,7 +1183,7 @@ static int do_receive(struct btrfs_receive *rctx, const char *tomnt,
 
 	while (!end) {
 		if (rctx->cached_capabilities_len) {
-			if (g_verbose >= 4)
+			if (bconf.verbose >= 4)
 				fprintf(stderr, "clear cached capabilities\n");
 			memset(rctx->cached_capabilities, 0,
 					sizeof(rctx->cached_capabilities));
@@ -1279,6 +1272,9 @@ static const char * const cmd_receive_usage[] = {
 	"                 this file system is mounted.",
 	"--dump           dump stream metadata, one line per operation,",
 	"                 does not require the MOUNT parameter",
+	HELPINFO_GLOBAL_OPTIONS_HEADER,
+	HELPINFO_INSERT_VERBOSE,
+	HELPINFO_INSERT_QUIET,
 	NULL
 };
 
@@ -1301,6 +1297,18 @@ static int cmd_receive(const struct cmd_struct *cmd, int argc, char **argv)
 	realmnt[0] = 0;
 	fromfile[0] = 0;
 
+	/*
+	 * Init global verbose to default, if it is unset.
+	 * Default is 1 for historical reasons, changing may break scripts that
+	 * expect the 'At subvol' message.
+	 * As default is 1, which means the effective verbose for receive is 2
+	 * which global verbose is unaware. So adjust global verbose value here.
+	 */
+	if (bconf.verbose < 0)
+		bconf.verbose = 1;
+	else if (bconf.verbose > 0)
+		bconf.verbose++;
+
 	optind = 0;
 	while (1) {
 		int c;
@@ -1319,10 +1327,10 @@ static int cmd_receive(const struct cmd_struct *cmd, int argc, char **argv)
 
 		switch (c) {
 		case 'v':
-			g_verbose++;
+			bconf.verbose++;
 			break;
 		case 'q':
-			g_verbose = 0;
+			bconf.verbose = 0;
 			break;
 		case 'f':
 			if (arg_copy_path(fromfile, optarg, sizeof(fromfile))) {
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1.1 07/18] btrfs-progs: subvolume delete: use global verbose option
  2019-11-04  6:32 [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option Anand Jain
                   ` (5 preceding siblings ...)
  2019-11-04  6:33 ` [PATCH v1.1 06/18] btrfs-progs: receive: " Anand Jain
@ 2019-11-04  6:33 ` Anand Jain
  2019-11-04  6:33 ` [PATCH v1 08/18] btrfs-progs: filesystem defragment: " Anand Jain
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Anand Jain @ 2019-11-04  6:33 UTC (permalink / raw)
  To: linux-btrfs

Transpire global --verbose option down to the btrfs subvolume delete
sub-command.

Suggested-by: David Sterba <dsterba@suse.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v1.1: fix stale error use pr_verbose(1,..) instead of pr_verbose(true,..)

 cmds/subvolume.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/cmds/subvolume.c b/cmds/subvolume.c
index 7a5fd79bb1f3..7cc215e1b0d5 100644
--- a/cmds/subvolume.c
+++ b/cmds/subvolume.c
@@ -234,6 +234,8 @@ static const char * const cmd_subvol_delete_usage[] = {
 	"-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",
+	HELPINFO_GLOBAL_OPTIONS_HEADER,
+	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];
@@ -256,6 +257,10 @@ static int cmd_subvol_delete(const struct cmd_struct *cmd,
 	enum { COMMIT_AFTER = 1, COMMIT_EACH = 2 };
 	enum btrfs_util_error err;
 
+	/* init global verbose if unset */
+	if (bconf.verbose < 0)
+		bconf.verbose = 0;
+
 	optind = 0;
 	while (1) {
 		int c;
@@ -278,7 +283,7 @@ static int cmd_subvol_delete(const struct cmd_struct *cmd,
 			commit_mode = COMMIT_EACH;
 			break;
 		case 'v':
-			verbose++;
+			bconf.verbose++;
 			break;
 		default:
 			usage_unknown_option(cmd, argv);
@@ -288,11 +293,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(1, "Transaction commit: %s\n",
+		   !commit_mode ? "none (default)" :
+		   commit_mode == COMMIT_AFTER ? "at the end" : "after each");
 
 	cnt = optind;
 
@@ -353,11 +356,9 @@ 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(1, "  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 +399,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(1,
+					   "final sync is done for fsid: %s\n",
+						   uuidbuf);
 				}
 				seen = seen->next;
 			}
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1 08/18] btrfs-progs: filesystem defragment: use global verbose option
  2019-11-04  6:32 [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option Anand Jain
                   ` (6 preceding siblings ...)
  2019-11-04  6:33 ` [PATCH v1.1 07/18] btrfs-progs: subvolume delete: use global verbose option Anand Jain
@ 2019-11-04  6:33 ` Anand Jain
  2019-11-14 16:16   ` David Sterba
  2019-11-04  6:33 ` [PATCH v1 09/18] btrfs-progs: balance start: " Anand Jain
                   ` (10 subsequent siblings)
  18 siblings, 1 reply; 31+ messages in thread
From: Anand Jain @ 2019-11-04  6:33 UTC (permalink / raw)
  To: linux-btrfs

Transpire global --verbose option down to the btrfs receive sub-command.

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

diff --git a/cmds/filesystem.c b/cmds/filesystem.c
index 4f22089abeaa..819b9fd1fcc5 100644
--- a/cmds/filesystem.c
+++ b/cmds/filesystem.c
@@ -844,11 +844,12 @@ static const char * const cmd_filesystem_defrag_usage[] = {
 	"(e.g., files copied with 'cp --reflink', snapshots) which may cause",
 	"considerable increase of space usage. See btrfs-filesystem(8) for",
 	"more information.",
+	HELPINFO_GLOBAL_OPTIONS_HEADER,
+	HELPINFO_INSERT_VERBOSE,
 	NULL
 };
 
 static struct btrfs_ioctl_defrag_range_args defrag_global_range;
-static int defrag_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 +858,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(1, "%s\n", fpath);
 		fd = open(fpath, defrag_open_mode);
 		if (fd < 0) {
 			goto error;
@@ -897,6 +897,10 @@ static int cmd_filesystem_defrag(const struct cmd_struct *cmd,
 	int compress_type = BTRFS_COMPRESS_NONE;
 	DIR *dirstream;
 
+	/* init global verbose if unset */
+	if (bconf.verbose < 0)
+		bconf.verbose = 0;
+
 	/*
 	 * Kernel 4.19+ supports defragmention of files open read-only,
 	 * otherwise it's an ETXTBSY error
@@ -913,7 +917,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 +934,7 @@ static int cmd_filesystem_defrag(const struct cmd_struct *cmd,
 			flush = 1;
 			break;
 		case 'v':
-			defrag_global_verbose = 1;
+			bconf.verbose++;
 			break;
 		case 's':
 			start = parse_size(optarg);
@@ -1031,8 +1034,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(1, "%s\n", argv[i]);
 			ret = ioctl(fd, BTRFS_IOC_DEFRAG_RANGE,
 					&defrag_global_range);
 			defrag_err = errno;
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1 09/18] btrfs-progs: balance start: use global verbose option
  2019-11-04  6:32 [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option Anand Jain
                   ` (7 preceding siblings ...)
  2019-11-04  6:33 ` [PATCH v1 08/18] btrfs-progs: filesystem defragment: " Anand Jain
@ 2019-11-04  6:33 ` Anand Jain
  2019-11-04  6:33 ` [PATCH v1 10/18] btrfs-progs: balance status: " Anand Jain
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Anand Jain @ 2019-11-04  6:33 UTC (permalink / raw)
  To: linux-btrfs

Transpire global --verbose option down to the btrfs balance start
sub-command.

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

diff --git a/cmds/balance.c b/cmds/balance.c
index a98cd8d6200f..0170fcb7f386 100644
--- a/cmds/balance.c
+++ b/cmds/balance.c
@@ -495,6 +495,8 @@ static const char * const cmd_balance_start_usage[] = {
 	"--full-balance do not print warning and do not delay start",
 	"--background|--bg",
 	"               run the balance as a background process",
+	HELPINFO_GLOBAL_OPTIONS_HEADER,
+	HELPINFO_INSERT_VERBOSE,
 	NULL
 };
 
@@ -505,11 +507,14 @@ 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;
 
+	/* If global verbose is unset, set it to 0 */
+	if (bconf.verbose < 0)
+		bconf.verbose = 0;
+
 	memset(&args, 0, sizeof(args));
 
 	optind = 0;
@@ -560,7 +565,7 @@ static int cmd_balance_start(const struct cmd_struct *cmd,
 			force = 1;
 			break;
 		case 'v':
-			verbose = 1;
+			bconf.verbose++;
 			break;
 		case GETOPT_VAL_FULL_BALANCE:
 			start_flags |= BALANCE_START_NOWARN;
@@ -636,7 +641,7 @@ static int cmd_balance_start(const struct cmd_struct *cmd,
 
 	if (force)
 		args.flags |= BTRFS_BALANCE_FORCE;
-	if (verbose)
+	if (bconf.verbose)
 		dump_ioctl_balance_args(&args);
 	if (background) {
 		switch (fork()) {
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1 10/18] btrfs-progs: balance status: use global verbose option
  2019-11-04  6:32 [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option Anand Jain
                   ` (8 preceding siblings ...)
  2019-11-04  6:33 ` [PATCH v1 09/18] btrfs-progs: balance start: " Anand Jain
@ 2019-11-04  6:33 ` Anand Jain
  2019-11-04  6:33 ` [PATCH v1 11/18] btrfs-progs: rescue chunk-recover: " Anand Jain
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Anand Jain @ 2019-11-04  6:33 UTC (permalink / raw)
  To: linux-btrfs

Transpire global --verbose option down to the btrfs balance status
sub-command.

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

diff --git a/cmds/balance.c b/cmds/balance.c
index 0170fcb7f386..54784c76c1de 100644
--- a/cmds/balance.c
+++ b/cmds/balance.c
@@ -828,6 +828,8 @@ static const char * const cmd_balance_status_usage[] = {
 	"Show status of running or paused balance",
 	"",
 	"-v|--verbose     be verbose",
+	HELPINFO_GLOBAL_OPTIONS_HEADER,
+	HELPINFO_INSERT_VERBOSE,
 	NULL
 };
 
@@ -844,9 +846,12 @@ static int cmd_balance_status(const struct cmd_struct *cmd,
 	const char *path;
 	DIR *dirstream = NULL;
 	int fd;
-	int verbose = 0;
 	int ret;
 
+	/* init global verbose if unset */
+	if (bconf.verbose < 0)
+		bconf.verbose = 0;
+
 	optind = 0;
 	while (1) {
 		int opt;
@@ -861,7 +866,7 @@ static int cmd_balance_status(const struct cmd_struct *cmd,
 
 		switch (opt) {
 		case 'v':
-			verbose = 1;
+			bconf.verbose++;
 			break;
 		default:
 			usage_unknown_option(cmd, argv);
@@ -907,7 +912,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 (bconf.verbose)
 		dump_ioctl_balance_args(&args);
 
 	ret = 1;
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1 11/18] btrfs-progs: rescue chunk-recover: use global verbose option
  2019-11-04  6:32 [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option Anand Jain
                   ` (9 preceding siblings ...)
  2019-11-04  6:33 ` [PATCH v1 10/18] btrfs-progs: balance status: " Anand Jain
@ 2019-11-04  6:33 ` Anand Jain
  2019-11-04  6:33 ` [PATCH v1 12/18] btrfs-progs: rescue super-recover: " Anand Jain
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Anand Jain @ 2019-11-04  6:33 UTC (permalink / raw)
  To: linux-btrfs

Transpire global --verbose option down to the btrfs rescue chunk-recover
sub-command.

For example: Both global and local verbose options are now supported.
 btrfs -v|--verbose rescue chunk-recover
 btrfs rescue chunk-recover -v

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

diff --git a/cmds/rescue.c b/cmds/rescue.c
index e8eab6808bc3..649f612aa051 100644
--- a/cmds/rescue.c
+++ b/cmds/rescue.c
@@ -40,6 +40,8 @@ static const char * const cmd_rescue_chunk_recover_usage[] = {
 	"-y	Assume an answer of `yes' to all questions",
 	"-v	Verbose mode",
 	"-h	Help",
+	HELPINFO_GLOBAL_OPTIONS_HEADER,
+	HELPINFO_INSERT_VERBOSE,
 	NULL
 };
 
@@ -49,7 +51,10 @@ static int cmd_rescue_chunk_recover(const struct cmd_struct *cmd,
 	int ret = 0;
 	char *file;
 	int yes = 0;
-	int verbose = 0;
+
+	/* If verbose is unset, set it to 0 */
+	if (bconf.verbose < 0)
+		bconf.verbose = 0;
 
 	optind = 0;
 	while (1) {
@@ -61,7 +66,7 @@ static int cmd_rescue_chunk_recover(const struct cmd_struct *cmd,
 			yes = 1;
 			break;
 		case 'v':
-			verbose = 1;
+			bconf.verbose++;
 			break;
 		default:
 			usage_unknown_option(cmd, argv);
@@ -83,7 +88,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, bconf.verbose, yes);
 	if (!ret) {
 		fprintf(stdout, "Chunk tree recovered successfully\n");
 	} else if (ret > 0) {
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1 12/18] btrfs-progs: rescue super-recover: use global verbose option
  2019-11-04  6:32 [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option Anand Jain
                   ` (10 preceding siblings ...)
  2019-11-04  6:33 ` [PATCH v1 11/18] btrfs-progs: rescue chunk-recover: " Anand Jain
@ 2019-11-04  6:33 ` Anand Jain
  2019-11-04  6:33 ` [PATCH v1.1 13/18] btrfs-progs: restore: " Anand Jain
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Anand Jain @ 2019-11-04  6:33 UTC (permalink / raw)
  To: linux-btrfs

Transpire global --verbose option down to the btrfs rescue super-recover
sub-command.

For example: Both global and local verbose options are now supported.
btrfs -v|--verbose rescue super-recover
btrfs rescue super-recover -v

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

diff --git a/cmds/rescue.c b/cmds/rescue.c
index 649f612aa051..195536457f7b 100644
--- a/cmds/rescue.c
+++ b/cmds/rescue.c
@@ -107,6 +107,8 @@ static const char * const cmd_rescue_super_recover_usage[] = {
 	"",
 	"-y	Assume an answer of `yes' to all questions",
 	"-v	Verbose mode",
+	HELPINFO_GLOBAL_OPTIONS_HEADER,
+	HELPINFO_INSERT_VERBOSE,
 	NULL
 };
 
@@ -122,10 +124,13 @@ 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;
 
+	/* Init global verbose if unset */
+	if (bconf.verbose < 0)
+		bconf.verbose = 0;
+
 	optind = 0;
 	while (1) {
 		int c = getopt(argc, argv, "vy");
@@ -133,7 +138,7 @@ static int cmd_rescue_super_recover(const struct cmd_struct *cmd,
 			break;
 		switch (c) {
 		case 'v':
-			verbose = 1;
+			bconf.verbose++;
 			break;
 		case 'y':
 			yes = 1;
@@ -155,7 +160,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, bconf.verbose, yes);
 	return ret;
 }
 static DEFINE_SIMPLE_COMMAND(rescue_super_recover, "super-recover");
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1.1 13/18] btrfs-progs: restore: use global verbose option
  2019-11-04  6:32 [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option Anand Jain
                   ` (11 preceding siblings ...)
  2019-11-04  6:33 ` [PATCH v1 12/18] btrfs-progs: rescue super-recover: " Anand Jain
@ 2019-11-04  6:33 ` Anand Jain
  2019-11-04  6:33 ` [PATCH v1 14/18] btrfs-progs: inspect-internal inode-resolve: use global verbose Anand Jain
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Anand Jain @ 2019-11-04  6:33 UTC (permalink / raw)
  To: linux-btrfs

Transpire global --verbose option down to the btrfs restore sub-command.

Suggested-by: David Sterba <dsterba@suse.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v1.1: use pr_verbose(-1,..) instead of all conditions printf();

 cmds/restore.c | 57 +++++++++++++++++++++++++++------------------------------
 1 file changed, 27 insertions(+), 30 deletions(-)

diff --git a/cmds/restore.c b/cmds/restore.c
index c104b01aef69..5afa1c7c40ff 100644
--- a/cmds/restore.c
+++ b/cmds/restore.c
@@ -51,7 +51,6 @@ 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;
 static int restore_metadata = 0;
 static int restore_symlinks = 0;
 static int ignore_errors = 0;
@@ -375,8 +374,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(offset ? 1 : 0, "offset is %Lu\n", offset);
 	/* we found a hole */
 	if (disk_size == 0)
 		return 0;
@@ -832,11 +830,13 @@ static int overwrite_ok(const char * path)
 		if (overwrite)
 			return 2;
 
-		if (verbose || !warn)
-			printf("Skipping existing file"
-				   " %s\n", path);
-		if (!warn)
-			printf("If you wish to overwrite use -o\n");
+		if (!warn) {
+			pr_verbose(-1, "Skipping existing file %s\n", path);
+			pr_verbose(-1, "If you wish to overwrite use -o\n");
+		} else {
+			pr_verbose(1, "Skipping existing file %s\n", path);
+		}
+
 		warn = 1;
 		return 0;
 	}
@@ -987,9 +987,8 @@ 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");
+		pr_verbose(2,
+			   "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",
@@ -997,9 +996,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(1,
+		   "Reached the end of the tree looking for the directory\n");
 			ret = 0;
 			goto out;
 		}
@@ -1023,10 +1021,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(1,
+		"Reached the end of the tree searching the directory\n");
 					ret = 0;
 					goto out;
 				}
@@ -1036,14 +1032,12 @@ static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
 		}
 		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);
+			pr_verbose(2, "Found objectid=%Lu, key=%Lu\n",
+				   found_key.objectid, key->objectid);
 			break;
 		}
 		if (found_key.type != key->type) {
-			if (verbose > 1)
-				printf("Found type=%u, want=%u\n",
+			pr_verbose(2, "Found type=%u, want=%u\n",
 				       found_key.type, key->type);
 			break;
 		}
@@ -1072,8 +1066,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(1, "Restoring %s\n", path_name);
 			if (dry_run)
 				goto next;
 			fd = open(path_name, O_CREAT|O_WRONLY, 0644);
@@ -1145,8 +1138,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(1, "Restoring %s\n", path_name);
 
 			errno = 0;
 			if (dry_run)
@@ -1209,8 +1201,7 @@ next:
 		}
 	}
 
-	if (verbose)
-		printf("Done searching %s\n", in_dir);
+	pr_verbose(1, "Done searching %s\n", in_dir);
 out:
 	btrfs_release_path(&path);
 	return ret;
@@ -1420,6 +1411,8 @@ static const char * const cmd_restore_usage[] = {
 	"                     you have to use following syntax (possibly quoted):",
 	"                     ^/(|home(|/username(|/Desktop(|/.*))))$",
 	"-c                   ignore case (--path-regex only)",
+	HELPINFO_GLOBAL_OPTIONS_HEADER,
+	HELPINFO_INSERT_VERBOSE,
 	NULL
 };
 
@@ -1441,6 +1434,10 @@ static int cmd_restore(const struct cmd_struct *cmd, int argc, char **argv)
 	regex_t match_reg, *mreg = NULL;
 	char reg_err[256];
 
+	/* Init global verbose if unset */
+	if (bconf.verbose < 0)
+		bconf.verbose = 0;
+
 	optind = 0;
 	while (1) {
 		int opt;
@@ -1472,7 +1469,7 @@ static int cmd_restore(const struct cmd_struct *cmd, int argc, char **argv)
 				get_snaps = 1;
 				break;
 			case 'v':
-				verbose++;
+				bconf.verbose++;
 				break;
 			case 'i':
 				ignore_errors = 1;
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1 14/18] btrfs-progs: inspect-internal inode-resolve: use global verbose
  2019-11-04  6:32 [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option Anand Jain
                   ` (12 preceding siblings ...)
  2019-11-04  6:33 ` [PATCH v1.1 13/18] btrfs-progs: restore: " Anand Jain
@ 2019-11-04  6:33 ` Anand Jain
  2019-11-04  6:33 ` [PATCH v1 15/18] btrfs-progs: inspect-internal logical-resolve: use global verbose option Anand Jain
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Anand Jain @ 2019-11-04  6:33 UTC (permalink / raw)
  To: linux-btrfs

Transpire global --verbose option down to the
btrfs inspect-internal inode-resolve sub-command.

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

diff --git a/cmds/inspect.c b/cmds/inspect.c
index 758b6e60c591..f36fee395159 100644
--- a/cmds/inspect.c
+++ b/cmds/inspect.c
@@ -56,12 +56,11 @@ 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, "
-			"cnt=%d, missed=%d\n", ret,
-			(unsigned long)fspath->bytes_left,
-			(unsigned long)fspath->bytes_missing,
-			fspath->elem_cnt, fspath->elem_missed);
+	pr_verbose(1,
+	"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, fspath->elem_cnt,
+		   fspath->elem_missed);
 
 	for (i = 0; i < fspath->elem_cnt; ++i) {
 		u64 ptr;
@@ -84,6 +83,8 @@ static const char * const cmd_inspect_inode_resolve_usage[] = {
 	"Get file system paths for the given inode",
 	"",
 	"-v   verbose mode",
+	HELPINFO_GLOBAL_OPTIONS_HEADER,
+	HELPINFO_INSERT_VERBOSE,
 	NULL
 };
 
@@ -91,10 +92,13 @@ 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;
 
+	/* set global verbose if unset */
+	if (bconf.verbose < 0)
+		bconf.verbose = 0;
+
 	optind = 0;
 	while (1) {
 		int c = getopt(argc, argv, "v");
@@ -103,7 +107,7 @@ static int cmd_inspect_inode_resolve(const struct cmd_struct *cmd,
 
 		switch (c) {
 		case 'v':
-			verbose = 1;
+			bconf.verbose++;
 			break;
 		default:
 			usage_unknown_option(cmd, argv);
@@ -117,8 +121,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,
+			       bconf.verbose, argv[optind+1]);
 	close_file_or_dir(fd, dirstream);
 	return !!ret;
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1 15/18] btrfs-progs: inspect-internal logical-resolve: use global verbose option
  2019-11-04  6:32 [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option Anand Jain
                   ` (13 preceding siblings ...)
  2019-11-04  6:33 ` [PATCH v1 14/18] btrfs-progs: inspect-internal inode-resolve: use global verbose Anand Jain
@ 2019-11-04  6:33 ` Anand Jain
  2019-11-04  6:33 ` [PATCH v1.1 16/18] btrfs-progs: refactor btrfs_scan_devices() to accept verbose argument Anand Jain
                   ` (3 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Anand Jain @ 2019-11-04  6:33 UTC (permalink / raw)
  To: linux-btrfs

Transpire global --verbose option down to the
btrfs inspect-internal logical-resolve sub-command.

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 f36fee395159..443a287fd540 100644
--- a/cmds/inspect.c
+++ b/cmds/inspect.c
@@ -38,7 +38,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;
@@ -121,8 +121,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,
-			       bconf.verbose, argv[optind+1]);
+	ret = __ino_to_path_fd(arg_strtou64(argv[optind]), fd, argv[optind+1]);
 	close_file_or_dir(fd, dirstream);
 	return !!ret;
 
@@ -138,6 +137,8 @@ static const char * const cmd_inspect_logical_resolve_usage[] = {
 	"-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",
+	HELPINFO_GLOBAL_OPTIONS_HEADER,
+	HELPINFO_INSERT_VERBOSE,
 	NULL
 };
 
@@ -147,7 +148,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;
@@ -168,7 +168,7 @@ static int cmd_inspect_logical_resolve(const struct cmd_struct *cmd,
 			getpath = 0;
 			break;
 		case 'v':
-			verbose = 1;
+			bconf.verbose++;
 			break;
 		case 's':
 			size = arg_strtou64(optarg);
@@ -203,13 +203,11 @@ 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, "
-			"bytes_missing=%lu, cnt=%d, missed=%d\n",
-			ret, size,
-			(unsigned long)inodes->bytes_left,
-			(unsigned long)inodes->bytes_missing,
-			inodes->elem_cnt, inodes->elem_missed);
+	pr_verbose(1,
+"ioctl ret=%d, total_size=%llu, bytes_left=%lu, bytes_missing=%lu, cnt=%d, missed=%d\n",
+		   ret, size, (unsigned long)inodes->bytes_left,
+		   (unsigned long)inodes->bytes_missing, inodes->elem_cnt,
+		   inodes->elem_missed);
 
 	bytes_left = sizeof(full_path);
 	ret = snprintf(full_path, bytes_left, "%s/", argv[optind+1]);
@@ -254,8 +252,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


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1.1 16/18] btrfs-progs: refactor btrfs_scan_devices() to accept verbose argument
  2019-11-04  6:32 [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option Anand Jain
                   ` (14 preceding siblings ...)
  2019-11-04  6:33 ` [PATCH v1 15/18] btrfs-progs: inspect-internal logical-resolve: use global verbose option Anand Jain
@ 2019-11-04  6:33 ` Anand Jain
  2019-11-04  6:33 ` [PATCH v1 17/18] btrfs-progs: device scan: add verbose option Anand Jain
                   ` (2 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Anand Jain @ 2019-11-04  6:33 UTC (permalink / raw)
  To: linux-btrfs

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>
---
v1.1: drop stale #include <stdbool.h> as %bconf.verbose is now int

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

diff --git a/cmds/device.c b/cmds/device.c
index 24158308a41b..f96d71e0477a 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(0);
 			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 819b9fd1fcc5..c5332335e417 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 48dbd9e19715..b6b6b30cfb23 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(int verbose)
 {
 	int fd = -1;
 	int ret;
@@ -404,6 +404,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..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 c2c6d0af0efc..81c319f8ff52 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 c3e85c0ee06e..beec7b162531 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -1097,7 +1097,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;
 	}
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1 17/18] btrfs-progs: device scan: add verbose option
  2019-11-04  6:32 [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option Anand Jain
                   ` (15 preceding siblings ...)
  2019-11-04  6:33 ` [PATCH v1.1 16/18] btrfs-progs: refactor btrfs_scan_devices() to accept verbose argument Anand Jain
@ 2019-11-04  6:33 ` Anand Jain
  2019-11-04  6:33 ` [PATCH v1.1 18/18] btrfs-progs: device scan: add quiet option Anand Jain
  2019-11-15 16:11 ` [PATCH v1.1 00/18] btrfs-progs: global verbose and " David Sterba
  18 siblings, 0 replies; 31+ messages in thread
From: Anand Jain @ 2019-11-04  6:33 UTC (permalink / raw)
  To: linux-btrfs

Enable verbose output for the device scanned, this uses 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 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/cmds/device.c b/cmds/device.c
index f96d71e0477a..d268fb2de126 100644
--- a/cmds/device.c
+++ b/cmds/device.c
@@ -303,6 +303,8 @@ static const char * const cmd_device_scan_usage[] = {
 	" -d|--all-devices            enumerate and register all devices, use as a fallback",
 	"                             if blkid is not available",
 	" -u|--forget [<device>...]   unregister a given device or all stale devices if no path ",
+	HELPINFO_GLOBAL_OPTIONS_HEADER,
+	HELPINFO_INSERT_VERBOSE,
 	NULL
 };
 
@@ -354,7 +356,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(0);
+			ret = btrfs_scan_devices(1);
 			error_on(ret, "error %d while scanning", ret);
 			ret = btrfs_register_all_devices();
 			error_on(ret,
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1.1 18/18] btrfs-progs: device scan: add quiet option
  2019-11-04  6:32 [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option Anand Jain
                   ` (16 preceding siblings ...)
  2019-11-04  6:33 ` [PATCH v1 17/18] btrfs-progs: device scan: add verbose option Anand Jain
@ 2019-11-04  6:33 ` Anand Jain
  2019-11-15 16:11 ` [PATCH v1.1 00/18] btrfs-progs: global verbose and " David Sterba
  18 siblings, 0 replies; 31+ messages in thread
From: Anand Jain @ 2019-11-04  6:33 UTC (permalink / raw)
  To: linux-btrfs

Enable the quiet option to the btrfs(8) device scan command.
Does the job quietly. For example:
 btrfs --quiet device scan

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v1.1: fix typo in HELPINFO_INSERT_QUIET

 cmds/device.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/cmds/device.c b/cmds/device.c
index d268fb2de126..8fef0695990e 100644
--- a/cmds/device.c
+++ b/cmds/device.c
@@ -305,6 +305,7 @@ static const char * const cmd_device_scan_usage[] = {
 	" -u|--forget [<device>...]   unregister a given device or all stale devices if no path ",
 	HELPINFO_GLOBAL_OPTIONS_HEADER,
 	HELPINFO_INSERT_VERBOSE,
+	HELPINFO_INSERT_QUIET,
 	NULL
 };
 
@@ -355,7 +356,7 @@ static int cmd_device_scan(const struct cmd_struct *cmd, int argc, char **argv)
 				error("cannot unregister devices: %m");
 			}
 		} else {
-			printf("Scanning for Btrfs filesystems\n");
+			pr_verbose(-1, "Scanning for Btrfs filesystems\n");
 			ret = btrfs_scan_devices(1);
 			error_on(ret, "error %d while scanning", ret);
 			ret = btrfs_register_all_devices();
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH v1.1 04/18] btrfs-progs: add global verbose and quiet options and helper functions
  2019-11-04  6:33 ` [PATCH v1.1 04/18] btrfs-progs: add global verbose and quiet options and helper functions Anand Jain
@ 2019-11-14 16:08   ` David Sterba
  2019-11-19  2:44     ` Anand Jain
  2019-11-19  3:36     ` Anand Jain
  2019-11-15 15:58   ` David Sterba
  1 sibling, 2 replies; 31+ messages in thread
From: David Sterba @ 2019-11-14 16:08 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

On Mon, Nov 04, 2019 at 02:33:02PM +0800, Anand Jain wrote:
>  /*
> + * Global verbose option for the sub-commands
> + */
> +#define HELPINFO_GLOBAL_OPTIONS_HEADER						\
> +	"",									\
> +	"Global options:"
> +#define HELPINFO_INSERT_VERBOSE							\
> +	"-v|--verbose       show verbose output"
> +#define HELPINFO_INSERT_QUIET							\
> +	"-q|--quiet         run the command quietly"
> +
> +/*
>   * 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

I've realized there's more magic around the global options, because
currently the --format option depends on the subcommand definition thus
it can't be a static text like you do with the definition of
HELPINFO_GLOBAL_OPTIONS_HEADER.

There's a special keyword that gets replaced, the verbose/quite options
don't need that so it's just the plain textual definition/description.

As this is a simple fixup
s/HELPINFO_GLOBAL_OPTIONS_HEADER/HELPINFO_INSERT_GLOBALS/, hold on with
resending I might find more things or fix it myself.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v1 08/18] btrfs-progs: filesystem defragment: use global verbose option
  2019-11-04  6:33 ` [PATCH v1 08/18] btrfs-progs: filesystem defragment: " Anand Jain
@ 2019-11-14 16:16   ` David Sterba
  2019-11-25 10:35     ` Anand Jain
  0 siblings, 1 reply; 31+ messages in thread
From: David Sterba @ 2019-11-14 16:16 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

On Mon, Nov 04, 2019 at 02:33:06PM +0800, Anand Jain wrote:
> Transpire global --verbose option down to the btrfs receive sub-command.
> 
> Suggested-by: David Sterba <dsterba@suse.com>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  cmds/filesystem.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/cmds/filesystem.c b/cmds/filesystem.c
> index 4f22089abeaa..819b9fd1fcc5 100644
> --- a/cmds/filesystem.c
> +++ b/cmds/filesystem.c
> @@ -844,11 +844,12 @@ static const char * const cmd_filesystem_defrag_usage[] = {
>  	"(e.g., files copied with 'cp --reflink', snapshots) which may cause",
>  	"considerable increase of space usage. See btrfs-filesystem(8) for",
>  	"more information.",
> +	HELPINFO_GLOBAL_OPTIONS_HEADER,
> +	HELPINFO_INSERT_VERBOSE,

Please not that this needs to be put right after the command options,
ie. before the paragraph.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v1.1 04/18] btrfs-progs: add global verbose and quiet options and helper functions
  2019-11-04  6:33 ` [PATCH v1.1 04/18] btrfs-progs: add global verbose and quiet options and helper functions Anand Jain
  2019-11-14 16:08   ` David Sterba
@ 2019-11-15 15:58   ` David Sterba
  2019-11-19  5:07     ` Anand Jain
  1 sibling, 1 reply; 31+ messages in thread
From: David Sterba @ 2019-11-15 15:58 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

On Mon, Nov 04, 2019 at 02:33:02PM +0800, Anand Jain wrote:
> +		case 'v':
> +			bconf.verbose < 0 ? bconf.verbose = 1 : bconf.verbose++;

This code gets repeated and it's not IMO simple enough to be copy-pasted
around. Eg. bconf_be_verbose() and eventually bconf_be_quiet().

> +			break;
> +		case 'q':
> +			bconf.verbose = 0;
> +			break;
>  		default:
>  			fprintf(stderr, "Unknown global option: %s\n",
>  					argv[optind - 1]);
> --- a/common/help.h
> +++ b/common/help.h
> @@ -53,6 +53,17 @@
>  	"-t|--tbytes        show sizes in TiB, or TB with --si"
>  
>  /*
> + * Global verbose option for the sub-commands
> + */
> +#define HELPINFO_GLOBAL_OPTIONS_HEADER						\
> +	"",									\
> +	"Global options:"
> +#define HELPINFO_INSERT_VERBOSE							\
> +	"-v|--verbose       show verbose output"

                            increase output verbosity

> +#define HELPINFO_INSERT_QUIET							\
> +	"-q|--quiet         run the command quietly"

			    print only errors
> +
> +/*
>   * 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
> --- a/common/utils.h
> +++ b/common/utils.h
> @@ -122,6 +122,9 @@ void print_all_devices(struct list_head *devices);
>   */
>  struct btrfs_config {
>  	unsigned int output_format;
> +
> +	/* -1:unset 0:quiet >0:verbose */

Instead of the constants, please add some defines for the unset and
default states. Maybe also for quiet.

> +	int verbose;
>  };
>  extern struct btrfs_config bconf;

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option
  2019-11-04  6:32 [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option Anand Jain
                   ` (17 preceding siblings ...)
  2019-11-04  6:33 ` [PATCH v1.1 18/18] btrfs-progs: device scan: add quiet option Anand Jain
@ 2019-11-15 16:11 ` David Sterba
  2019-11-19  3:50   ` Anand Jain
  18 siblings, 1 reply; 31+ messages in thread
From: David Sterba @ 2019-11-15 16:11 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

On Mon, Nov 04, 2019 at 02:32:58PM +0800, Anand Jain wrote:
> v1->v1.1:
>  . Fix typo in HELPINFO_INSERT_QUIET.
>  . Remove #include <stdbool.h> where its no more required.
>    (was needed when %bconf.verbose was declared as bool).
>  . Use pr_verbose(-1,..) instead of all conditions printf()
>  . Use pr_verbose(1,..) instead of pr_verbose(true,..)
> 
> verbosity sample code as in v1.1

Please user integer numbers for patch revisions, no need to mark patches
that haven't changed, the overall summary of changes can mention what
changed where if needed.

> pr_verbose()
> ------------
> /*
>  * level -1: prints message unless bconf.verbose == 0;
>  * level  0: quiet

Are we ever going to call the function with level == 0?

>  * level >0: prints message only if <= bconf.verbose
>  */
> void pr_verbose(int level, const char *fmt, ...)
> {
>         va_list args;
> 
>         if (level == 0 || bconf.verbose == 0)
>                 return;
> 
>         if (level > bconf.verbose)
>                 return;
> 
>         va_start(args, fmt);
>         vfprintf(stdout, fmt, args);
>         va_end(args);
> }
> 
> 1.
>  There are certain sub-commands which does not have any verbose output
>  or quiet output. However if the global options were used with those
>  sub-commands then the command shall not report any usage error. Or
>  my question is should it error out.? For example:
>   (with the patch) btrfs --verbose device ready /dev/sdb
>  actually there isn't any verbose output but we won't error out.
>  Similarly,
>   (without the patch) btrfs send -vvvvv will not show usage error
>   as well.
>   So I believe this is fine. IMO.

Yes this is fine.

> 2.
>   There is slight difference in output when global options are used
>   as compared to the output using the same sequence of options at the
>   sub-command level. For example:
> 
>    btrfs send -v -q -v  is-equal-to  btrfs send
>    But same sequence in the global option
>    btrfs -v -q -v send is-not-equal-to btrfs send
>    but is-equal-to btrfs -v send or btrfs send -v.
>    (similarly applies to receive as well).
> 
>   which IMO is fair expectation as -v is ending last.

Agreed, hopefully the wild combinations of -v and -q are not too common.

The patchset looks good, though it needs the small fixups like the
global header or the helper macros, the core of the changes is there.
That should be good for 5.4.

The first version merged should bring the support for global verbosity
options, then we can gradually convert all fprintf/printf to the helpers
and add new printfs with higher verbosity levels.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v1.1 04/18] btrfs-progs: add global verbose and quiet options and helper functions
  2019-11-14 16:08   ` David Sterba
@ 2019-11-19  2:44     ` Anand Jain
  2019-11-19  3:36     ` Anand Jain
  1 sibling, 0 replies; 31+ messages in thread
From: Anand Jain @ 2019-11-19  2:44 UTC (permalink / raw)
  To: dsterba, linux-btrfs



On 11/15/19 12:08 AM, David Sterba wrote:
> On Mon, Nov 04, 2019 at 02:33:02PM +0800, Anand Jain wrote:
>>   /*
>> + * Global verbose option for the sub-commands
>> + */
>> +#define HELPINFO_GLOBAL_OPTIONS_HEADER						\
>> +	"",									\
>> +	"Global options:"
>> +#define HELPINFO_INSERT_VERBOSE							\
>> +	"-v|--verbose       show verbose output"
>> +#define HELPINFO_INSERT_QUIET							\
>> +	"-q|--quiet         run the command quietly"
>> +
>> +/*
>>    * 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
> 
> I've realized there's more magic around the global options, because
> currently the --format option depends on the subcommand definition thus
> it can't be a static text like you do with the definition of
> HELPINFO_GLOBAL_OPTIONS_HEADER.
 >
> There's a special keyword that gets replaced, the verbose/quite options
> don't need that so it's just the plain textual definition/description.
> 
> As this is a simple fixup
> s/HELPINFO_GLOBAL_OPTIONS_HEADER/HELPINFO_INSERT_GLOBALS/, hold on with
> resending I might find more things or fix it myself.
> 
  As v2 is in progress I shall fix this.

Thanks, Anand

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v1.1 04/18] btrfs-progs: add global verbose and quiet options and helper functions
  2019-11-14 16:08   ` David Sterba
  2019-11-19  2:44     ` Anand Jain
@ 2019-11-19  3:36     ` Anand Jain
  2019-11-19 16:51       ` David Sterba
  1 sibling, 1 reply; 31+ messages in thread
From: Anand Jain @ 2019-11-19  3:36 UTC (permalink / raw)
  To: dsterba, linux-btrfs



On 11/15/19 12:08 AM, David Sterba wrote:
> On Mon, Nov 04, 2019 at 02:33:02PM +0800, Anand Jain wrote:
>>   /*
>> + * Global verbose option for the sub-commands
>> + */
>> +#define HELPINFO_GLOBAL_OPTIONS_HEADER						\
>> +	"",									\
>> +	"Global options:"
>> +#define HELPINFO_INSERT_VERBOSE							\
>> +	"-v|--verbose       show verbose output"
>> +#define HELPINFO_INSERT_QUIET							\
>> +	"-q|--quiet         run the command quietly"
>> +
>> +/*
>>    * 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
> 
> I've realized there's more magic around the global options, because
> currently the --format option depends on the subcommand definition thus
> it can't be a static text like you do with the definition of
> HELPINFO_GLOBAL_OPTIONS_HEADER.
> 
> There's a special keyword that gets replaced, the verbose/quite options
> don't need that so it's just the plain textual definition/description.
> 
> As this is a simple fixup
> s/HELPINFO_GLOBAL_OPTIONS_HEADER/HELPINFO_INSERT_GLOBALS/, hold on with
> resending I might find more things or fix it myself.
> 

But there is one problem,  HELPINFO_INSERT_GLOBALS is already defined
as..
      Global options:
     --format TYPE      where TYPE is: text

(ref: common/help.c   do_usage_one_command())

Albeit all commands support --format text by default.

But no sub-command is using the HELPINFO_INSERT_GLOBALS in its usage.

Looks like its a good idea to separate title and the options, like
#define HELPINFO_INSERT_GLOBALS		"Global options:"
#define HELPINGO_INSERT_FORMAT		"--format TYPE"

As at the moment I am not sure if its safe to declare that all
sub-commands will support --format json (whenever we do that).

So with the defines split as above, in each sub-command-usage
we shall do..

-----------------------------------------
diff --git a/cmds/filesystem.c b/cmds/filesystem.c
index 4f22089abeaa..f4dba38b4c17 100644
--- a/cmds/filesystem.c
+++ b/cmds/filesystem.c
@@ -631,6 +631,10 @@ static const char * const 
cmd_filesystem_show_usage[] = {
         "-m|--mounted       show only mounted btrfs",
         HELPINFO_UNITS_LONG,
         "If no argument is given, structure of all present filesystems 
is shown.",
+       HELPINFO_INSERT_GLOBALS,
+       HELPINFO_INSERT_FORMAT,
+       HELPINFO_INSERT_VERBOSE,
+       HELPINFO_INSERT_QUIET,
         NULL
-----------------------------------------


Thanks, Anand

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option
  2019-11-15 16:11 ` [PATCH v1.1 00/18] btrfs-progs: global verbose and " David Sterba
@ 2019-11-19  3:50   ` Anand Jain
  0 siblings, 0 replies; 31+ messages in thread
From: Anand Jain @ 2019-11-19  3:50 UTC (permalink / raw)
  To: dsterba, linux-btrfs



On 11/16/19 12:11 AM, David Sterba wrote:
> On Mon, Nov 04, 2019 at 02:32:58PM +0800, Anand Jain wrote:
>> v1->v1.1:
>>   . Fix typo in HELPINFO_INSERT_QUIET.
>>   . Remove #include <stdbool.h> where its no more required.
>>     (was needed when %bconf.verbose was declared as bool).
>>   . Use pr_verbose(-1,..) instead of all conditions printf()
>>   . Use pr_verbose(1,..) instead of pr_verbose(true,..)
>>
>> verbosity sample code as in v1.1
> 
> Please user integer numbers for patch revisions, no need to mark patches
> that haven't changed, the overall summary of changes can mention what
> changed where if needed.

  Ok got it.
  (I kind of didn't want to integer++ unless it fixes review comments).

>> pr_verbose()
>> ------------
>> /*
>>   * level -1: prints message unless bconf.verbose == 0;
>>   * level  0: quiet
> 
> Are we ever going to call the function with level == 0?

   Yes. For example in the patch

    [PATCH v1.1 13/18] btrfs-progs: restore: use global verbose option

--------------
@@ -375,8 +374,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(offset ? 1 : 0, "offset is %Lu\n", offset);
--------------



>>   * level >0: prints message only if <= bconf.verbose
>>   */
>> void pr_verbose(int level, const char *fmt, ...)
>> {
>>          va_list args;
>>
>>          if (level == 0 || bconf.verbose == 0)
>>                  return;
>>
>>          if (level > bconf.verbose)
>>                  return;
>>
>>          va_start(args, fmt);
>>          vfprintf(stdout, fmt, args);
>>          va_end(args);
>> }
>>
>> 1.
>>   There are certain sub-commands which does not have any verbose output
>>   or quiet output. However if the global options were used with those
>>   sub-commands then the command shall not report any usage error. Or
>>   my question is should it error out.? For example:
>>    (with the patch) btrfs --verbose device ready /dev/sdb
>>   actually there isn't any verbose output but we won't error out.
>>   Similarly,
>>    (without the patch) btrfs send -vvvvv will not show usage error
>>    as well.
>>    So I believe this is fine. IMO.
> 
> Yes this is fine.
> 
>> 2.
>>    There is slight difference in output when global options are used
>>    as compared to the output using the same sequence of options at the
>>    sub-command level. For example:
>>
>>     btrfs send -v -q -v  is-equal-to  btrfs send
>>     But same sequence in the global option
>>     btrfs -v -q -v send is-not-equal-to btrfs send
>>     but is-equal-to btrfs -v send or btrfs send -v.
>>     (similarly applies to receive as well).
>>
>>    which IMO is fair expectation as -v is ending last.
> 
> Agreed, hopefully the wild combinations of -v and -q are not too common.
> 
> The patchset looks good, though it needs the small fixups like the
> global header or the helper macros, the core of the changes is there.
> That should be good for 5.4.
> 
> The first version merged should bring the support for global verbosity
> options, then we can gradually convert all fprintf/printf to the helpers
> and add new printfs with higher verbosity levels.
> 

Yes. Will do.

Thanks, Anand

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v1.1 04/18] btrfs-progs: add global verbose and quiet options and helper functions
  2019-11-15 15:58   ` David Sterba
@ 2019-11-19  5:07     ` Anand Jain
  2019-11-19 17:02       ` David Sterba
  0 siblings, 1 reply; 31+ messages in thread
From: Anand Jain @ 2019-11-19  5:07 UTC (permalink / raw)
  To: dsterba, linux-btrfs



On 11/15/19 11:58 PM, David Sterba wrote:
> On Mon, Nov 04, 2019 at 02:33:02PM +0800, Anand Jain wrote:
>> +		case 'v':
>> +			bconf.verbose < 0 ? bconf.verbose = 1 : bconf.verbose++;
> 
> This code gets repeated and it's not IMO simple enough to be copy-pasted
> around. Eg. bconf_be_verbose() and eventually bconf_be_quiet().

  I was just concerned- it will make life of other developers difficult,
  IMO too much abstraction in the code is almost like learning a new
  programming language for the new person looking at the code.
  For example fstests. To write patch for fstests you need to
  learn about a lot of helpers, defines and functions and filters
  specific to fstests but you wouldn't have had this problem if the
  fstests abstractions were limited and if it embraced open-code style.
  Just my 1c.

  For now I have added bconf_be_verbose() and bconf_be_quiet() it looks
  neat as below,

+               case 'v':
+                       bconf_be_verbose();
+                       break;
+               case 'q':
+                       bconf_be_quiet();
+                       break;


>> +			break;
>> +		case 'q':
>> +			bconf.verbose = 0;
>> +			break;
>>   		default:
>>   			fprintf(stderr, "Unknown global option: %s\n",
>>   					argv[optind - 1]);
>> --- a/common/help.h
>> +++ b/common/help.h
>> @@ -53,6 +53,17 @@
>>   	"-t|--tbytes        show sizes in TiB, or TB with --si"
>>   
>>   /*
>> + * Global verbose option for the sub-commands
>> + */
>> +#define HELPINFO_GLOBAL_OPTIONS_HEADER						\
>> +	"",									\
>> +	"Global options:"
>> +#define HELPINFO_INSERT_VERBOSE							\
>> +	"-v|--verbose       show verbose output"
> 
>                              increase output verbosity

fixed.

>> +#define HELPINFO_INSERT_QUIET							\
>> +	"-q|--quiet         run the command quietly"
>   			    print only errors
fixed.

>> +
>> +/*
>>    * 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
>> --- a/common/utils.h
>> +++ b/common/utils.h
>> @@ -122,6 +122,9 @@ void print_all_devices(struct list_head *devices);
>>    */
>>   struct btrfs_config {
>>   	unsigned int output_format;
>> +
>> +	/* -1:unset 0:quiet >0:verbose */
> 
> Instead of the constants, please add some defines for the unset and
> default states. Maybe also for quiet.

Fixed.

+#define BTRFS_BCONF_QUIET       0
+#define BTRFS_BCONF_UNSET      -1


Thanks for the valuable comments.
Anand


>> +	int verbose;
>>   };
>>   extern struct btrfs_config bconf;

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v1.1 04/18] btrfs-progs: add global verbose and quiet options and helper functions
  2019-11-19  3:36     ` Anand Jain
@ 2019-11-19 16:51       ` David Sterba
  2019-11-25 10:36         ` Anand Jain
  0 siblings, 1 reply; 31+ messages in thread
From: David Sterba @ 2019-11-19 16:51 UTC (permalink / raw)
  To: Anand Jain; +Cc: dsterba, linux-btrfs

On Tue, Nov 19, 2019 at 11:36:59AM +0800, Anand Jain wrote:
> > As this is a simple fixup
> > s/HELPINFO_GLOBAL_OPTIONS_HEADER/HELPINFO_INSERT_GLOBALS/, hold on with
> > resending I might find more things or fix it myself.
> > 
> 
> But there is one problem,  HELPINFO_INSERT_GLOBALS is already defined
> as..
>       Global options:
>      --format TYPE      where TYPE is: text
> 
> (ref: common/help.c   do_usage_one_command())
> 
> Albeit all commands support --format text by default.
> 
> But no sub-command is using the HELPINFO_INSERT_GLOBALS in its usage.
> 
> Looks like its a good idea to separate title and the options, like
> #define HELPINFO_INSERT_GLOBALS		"Global options:"
> #define HELPINGO_INSERT_FORMAT		"--format TYPE"

Yeah, makes sense to split it, right now the format does not bring
anything so that will be better done in a major release some time in the
future when more commands have json output.

> As at the moment I am not sure if its safe to declare that all
> sub-commands will support --format json (whenever we do that).

No, right now json output should not be declared anywhere.

> So with the defines split as above, in each sub-command-usage
> we shall do..
> 
> -----------------------------------------
> diff --git a/cmds/filesystem.c b/cmds/filesystem.c
> index 4f22089abeaa..f4dba38b4c17 100644
> --- a/cmds/filesystem.c
> +++ b/cmds/filesystem.c
> @@ -631,6 +631,10 @@ static const char * const 
> cmd_filesystem_show_usage[] = {
>          "-m|--mounted       show only mounted btrfs",
>          HELPINFO_UNITS_LONG,
>          "If no argument is given, structure of all present filesystems 
> is shown.",
> +       HELPINFO_INSERT_GLOBALS,
> +       HELPINFO_INSERT_FORMAT,
> +       HELPINFO_INSERT_VERBOSE,
> +       HELPINFO_INSERT_QUIET,

Sounds ok.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v1.1 04/18] btrfs-progs: add global verbose and quiet options and helper functions
  2019-11-19  5:07     ` Anand Jain
@ 2019-11-19 17:02       ` David Sterba
  0 siblings, 0 replies; 31+ messages in thread
From: David Sterba @ 2019-11-19 17:02 UTC (permalink / raw)
  To: Anand Jain; +Cc: dsterba, linux-btrfs

On Tue, Nov 19, 2019 at 01:07:05PM +0800, Anand Jain wrote:
> On 11/15/19 11:58 PM, David Sterba wrote:
> > On Mon, Nov 04, 2019 at 02:33:02PM +0800, Anand Jain wrote:
> >> +		case 'v':
> >> +			bconf.verbose < 0 ? bconf.verbose = 1 : bconf.verbose++;
> > 
> > This code gets repeated and it's not IMO simple enough to be copy-pasted
> > around. Eg. bconf_be_verbose() and eventually bconf_be_quiet().
> 
>   I was just concerned- it will make life of other developers difficult,
>   IMO too much abstraction in the code is almost like learning a new
>   programming language for the new person looking at the code.
>   For example fstests. To write patch for fstests you need to
>   learn about a lot of helpers, defines and functions and filters
>   specific to fstests but you wouldn't have had this problem if the
>   fstests abstractions were limited and if it embraced open-code style.
>   Just my 1c.

Yes it takes time to learn the abstractions but then it makes a lot of
things easier and allows to think about what to do and not necessarily
how. In a clean codebase there are enough examples to follow or copy &
adapt, I don't think it's a big deal and that it's normal and expected
when one works on various code bases.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v1 08/18] btrfs-progs: filesystem defragment: use global verbose option
  2019-11-14 16:16   ` David Sterba
@ 2019-11-25 10:35     ` Anand Jain
  0 siblings, 0 replies; 31+ messages in thread
From: Anand Jain @ 2019-11-25 10:35 UTC (permalink / raw)
  To: dsterba, linux-btrfs



On 11/15/19 12:16 AM, David Sterba wrote:
> On Mon, Nov 04, 2019 at 02:33:06PM +0800, Anand Jain wrote:
>> Transpire global --verbose option down to the btrfs receive sub-command.
>>
>> Suggested-by: David Sterba <dsterba@suse.com>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   cmds/filesystem.c | 16 +++++++++-------
>>   1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/cmds/filesystem.c b/cmds/filesystem.c
>> index 4f22089abeaa..819b9fd1fcc5 100644
>> --- a/cmds/filesystem.c
>> +++ b/cmds/filesystem.c
>> @@ -844,11 +844,12 @@ static const char * const cmd_filesystem_defrag_usage[] = {
>>   	"(e.g., files copied with 'cp --reflink', snapshots) which may cause",
>>   	"considerable increase of space usage. See btrfs-filesystem(8) for",
>>   	"more information.",
>> +	HELPINFO_GLOBAL_OPTIONS_HEADER,
>> +	HELPINFO_INSERT_VERBOSE,
> 
> Please not that this needs to be put right after the command options,
> ie. before the paragraph.
> 

This is fixed in v2.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v1.1 04/18] btrfs-progs: add global verbose and quiet options and helper functions
  2019-11-19 16:51       ` David Sterba
@ 2019-11-25 10:36         ` Anand Jain
  0 siblings, 0 replies; 31+ messages in thread
From: Anand Jain @ 2019-11-25 10:36 UTC (permalink / raw)
  To: dsterba, linux-btrfs



On 11/20/19 12:51 AM, David Sterba wrote:
> On Tue, Nov 19, 2019 at 11:36:59AM +0800, Anand Jain wrote:
>>> As this is a simple fixup
>>> s/HELPINFO_GLOBAL_OPTIONS_HEADER/HELPINFO_INSERT_GLOBALS/, hold on with
>>> resending I might find more things or fix it myself.
>>>
>>
>> But there is one problem,  HELPINFO_INSERT_GLOBALS is already defined
>> as..
>>        Global options:
>>       --format TYPE      where TYPE is: text
>>
>> (ref: common/help.c   do_usage_one_command())
>>
>> Albeit all commands support --format text by default.
>>
>> But no sub-command is using the HELPINFO_INSERT_GLOBALS in its usage.
>>
>> Looks like its a good idea to separate title and the options, like
>> #define HELPINFO_INSERT_GLOBALS		"Global options:"
>> #define HELPINGO_INSERT_FORMAT		"--format TYPE"
> 
> Yeah, makes sense to split it, right now the format does not bring
> anything so that will be better done in a major release some time in the
> future when more commands have json output.

  Ok. In v2 I have split
   HELPINFO_GLOBAL_OPTIONS_HEADER into HELPINFO_INSERT_GLOBALS and
   HELPINGO_INSERT_FORMAT as above.

Thanks, Anand


>> As at the moment I am not sure if its safe to declare that all
>> sub-commands will support --format json (whenever we do that).
> 
> No, right now json output should not be declared anywhere.
> 
>> So with the defines split as above, in each sub-command-usage
>> we shall do..
>> 
>> -----------------------------------------
>> diff --git a/cmds/filesystem.c b/cmds/filesystem.c
>> index 4f22089abeaa..f4dba38b4c17 100644
>> --- a/cmds/filesystem.c
>> +++ b/cmds/filesystem.c
>> @@ -631,6 +631,10 @@ static const char * const
>> cmd_filesystem_show_usage[] = {
>>           "-m|--mounted       show only mounted btrfs",
>>           HELPINFO_UNITS_LONG,
>>           "If no argument is given, structure of all present filesystems
>> is shown.",
>> +       HELPINFO_INSERT_GLOBALS,
>> +       HELPINFO_INSERT_FORMAT,
>> +       HELPINFO_INSERT_VERBOSE,
>> +       HELPINFO_INSERT_QUIET,
> 
> Sounds ok.
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2019-11-25 10:37 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-04  6:32 [PATCH v1.1 00/18] btrfs-progs: global verbose and quiet option Anand Jain
2019-11-04  6:32 ` [PATCH v1 01/18] btrfs-progs: receive: fix option quiet Anand Jain
2019-11-04  6:33 ` [PATCH v1 02/18] btrfs-progs: balance status: fix usage show long verbose Anand Jain
2019-11-04  6:33 ` [PATCH v1 03/18] btrfs-progs: balance start: fix usage add " Anand Jain
2019-11-04  6:33 ` [PATCH v1.1 04/18] btrfs-progs: add global verbose and quiet options and helper functions Anand Jain
2019-11-14 16:08   ` David Sterba
2019-11-19  2:44     ` Anand Jain
2019-11-19  3:36     ` Anand Jain
2019-11-19 16:51       ` David Sterba
2019-11-25 10:36         ` Anand Jain
2019-11-15 15:58   ` David Sterba
2019-11-19  5:07     ` Anand Jain
2019-11-19 17:02       ` David Sterba
2019-11-04  6:33 ` [PATCH v1.1 05/18] btrfs-progs: send: use global verbose and quiet options Anand Jain
2019-11-04  6:33 ` [PATCH v1.1 06/18] btrfs-progs: receive: " Anand Jain
2019-11-04  6:33 ` [PATCH v1.1 07/18] btrfs-progs: subvolume delete: use global verbose option Anand Jain
2019-11-04  6:33 ` [PATCH v1 08/18] btrfs-progs: filesystem defragment: " Anand Jain
2019-11-14 16:16   ` David Sterba
2019-11-25 10:35     ` Anand Jain
2019-11-04  6:33 ` [PATCH v1 09/18] btrfs-progs: balance start: " Anand Jain
2019-11-04  6:33 ` [PATCH v1 10/18] btrfs-progs: balance status: " Anand Jain
2019-11-04  6:33 ` [PATCH v1 11/18] btrfs-progs: rescue chunk-recover: " Anand Jain
2019-11-04  6:33 ` [PATCH v1 12/18] btrfs-progs: rescue super-recover: " Anand Jain
2019-11-04  6:33 ` [PATCH v1.1 13/18] btrfs-progs: restore: " Anand Jain
2019-11-04  6:33 ` [PATCH v1 14/18] btrfs-progs: inspect-internal inode-resolve: use global verbose Anand Jain
2019-11-04  6:33 ` [PATCH v1 15/18] btrfs-progs: inspect-internal logical-resolve: use global verbose option Anand Jain
2019-11-04  6:33 ` [PATCH v1.1 16/18] btrfs-progs: refactor btrfs_scan_devices() to accept verbose argument Anand Jain
2019-11-04  6:33 ` [PATCH v1 17/18] btrfs-progs: device scan: add verbose option Anand Jain
2019-11-04  6:33 ` [PATCH v1.1 18/18] btrfs-progs: device scan: add quiet option Anand Jain
2019-11-15 16:11 ` [PATCH v1.1 00/18] btrfs-progs: global verbose and " David Sterba
2019-11-19  3:50   ` Anand Jain

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).