[RFC PATCH 0/3] btrfs-progs: make quiet to overrule verbose

[RFC PATCH 0/3] btrfs-progs: make quiet to overrule verbose

[Anand Jain]

When both the options (--quiet and --verbose) in btrfs send and receive
is specified, we need at least one of it to overrule the other, irrespective
of the chronological order of options.

This patch-set makes quiet overrule verbose.

I don't think this shall break any script as such, because these two options are
of kind of mutually exclusive. But just in case if I am missing something? So
marked it as RFC.

And again patch 3/3 makes quiet option really quiet in receive and removes the
output
 At snapshot <>
I don't expect scripts to specify quiet option and expect some logs.

Anand Jain (3):
  btrfs-progs: send: let option quiet overrule verbose
  btrfs-progs: receive: let option quiet overrule verbose
  btrfs-progs: receive: make quiet really quiet

 cmds/receive.c | 9 +++++++--
 cmds/send.c    | 6 +++++-
 2 files changed, 12 insertions(+), 3 deletions(-)

-- 
2.23.0

[RFC PATCH 1/3] btrfs-progs: send: let option quiet overrule verbose

[Anand Jain]

btrfs send has both -q|--quiet and -v|--verbose options, now the test
here is, which option shall take the precedence if in case both of them
are specified.

Per current implementation it really depends on the order of the options
as shown below

Without fix:
---- btrfs send -q /btrfs/ss2 -f /tmp/t -----
---- btrfs send -q -v /btrfs/ss2 -f /tmp/t -----
At subvol /btrfs/ss2
---- btrfs send -q -vv /btrfs/ss2 -f /tmp/t -----
At subvol /btrfs/ss2
BTRFS_IOC_SEND returned 0
joining genl thread
---- btrfs send -v -q /btrfs/ss2 -f /tmp/t -----
---- btrfs send -vv -q /btrfs/ss2 -f /tmp/t -----
---- btrfs send -v -q -v /btrfs/ss2 -f /tmp/t -----
At subvol /btrfs/ss2

If the effectiveness of the output depends on the chronological order
of the options -q and -v specified in the command line, then its rather
confusing at times.

So fix it by making the option -q|--quiet to overrule the -v|--verbose
option if when both of them are specified.

So with the fix:
---- btrfs send -q /btrfs/ss2 -f /tmp/t -----
---- btrfs send -q -v /btrfs/ss2 -f /tmp/t -----
---- btrfs send -q -vv /btrfs/ss2 -f /tmp/t -----
---- btrfs send -v -q /btrfs/ss2 -f /tmp/t -----
---- btrfs send -vv -q /btrfs/ss2 -f /tmp/t -----
---- btrfs send -v -q -v /btrfs/ss2 -f /tmp/t -----

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

diff --git a/cmds/send.c b/cmds/send.c
index 7ce6c3273857..4bf2be7db4d0 100644
--- a/cmds/send.c
+++ b/cmds/send.c
@@ -477,6 +477,7 @@ static int cmd_send(const struct cmd_struct *cmd, int argc, char **argv)
 	int full_send = 1;
 	int new_end_cmd_semantic = 0;
 	u64 send_flags = 0;
+	bool quiet = false;
 
 	memset(&send, 0, sizeof(send));
 	send.dump_fd = fileno(stdout);
@@ -500,7 +501,7 @@ static int cmd_send(const struct cmd_struct *cmd, int argc, char **argv)
 			g_verbose++;
 			break;
 		case 'q':
-			g_verbose = 0;
+			quiet = true;
 			break;
 		case 'e':
 			new_end_cmd_semantic = 1;
@@ -584,6 +585,9 @@ static int cmd_send(const struct cmd_struct *cmd, int argc, char **argv)
 	if (check_argc_min(argc - optind, 1))
 		return 1;
 
+	if (quiet)
+		g_verbose = 0;
+
 	if (outname[0]) {
 		int tmpfd;
 
-- 
2.23.0

[RFC PATCH 2/3] btrfs-progs: receive: let option quiet overrule verbose

[Anand Jain]

btrfs receive has both -q|--quiet and -v|--verbose options, if when both
the options are specified, the order of the options makes difference in
the output, which is at times causes confusion.

Fix this by letting option --quite to overrule --verbose option.

Without fix:
---- btrfs receive -q -vv -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 receive -v -q -f /tmp/t /btrfs1 -----
At snapshot ss3
---- btrfs receive -vv -q -f /tmp/t /btrfs1 -----
At snapshot ss3

with fix:
---- btrfs receive -q -vv -f /tmp/t /btrfs1 -----
At snapshot ss3
---- btrfs receive -v -q -f /tmp/t /btrfs1 -----
At snapshot ss3
---- btrfs receive -vv -q -f /tmp/t /btrfs1 -----
At snapshot ss3

The output with either of them (-q or -v) remains unaffected
by this patch, as shown below:
---- btrfs receive -q -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
---- btrfs receive -vv -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

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

diff --git a/cmds/receive.c b/cmds/receive.c
index 4b03938ea3eb..d8c934a7c57c 100644
--- a/cmds/receive.c
+++ b/cmds/receive.c
@@ -1291,6 +1291,7 @@ static int cmd_receive(const struct cmd_struct *cmd, int argc, char **argv)
 	u64 max_errors = 1;
 	int dump = 0;
 	int ret = 0;
+	bool quiet = false;
 
 	memset(&rctx, 0, sizeof(rctx));
 	rctx.mnt_fd = -1;
@@ -1321,7 +1322,7 @@ static int cmd_receive(const struct cmd_struct *cmd, int argc, char **argv)
 			g_verbose++;
 			break;
 		case 'q':
-			g_verbose = 0;
+			quiet = true;
 			break;
 		case 'f':
 			if (arg_copy_path(fromfile, optarg, sizeof(fromfile))) {
@@ -1356,6 +1357,9 @@ static int cmd_receive(const struct cmd_struct *cmd, int argc, char **argv)
 		}
 	}
 
+	if (quiet)
+		g_verbose = 0;
+
 	if (dump && check_argc_exact(argc - optind, 0))
 		usage(cmd);
 	if (!dump && check_argc_exact(argc - optind, 1))
-- 
2.23.0

[RFC PATCH 3/3] btrfs-progs: receive: make quiet really quiet

[Anand Jain]

A message is still being printed even when the receive option --quiet
is specified, this patch fixes it.

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

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

Signed-off-by: Anand Jain <anand.jain@oracle.com>

fstests doesn't use --quiet option, so all test cases should remain
unaffected with this change.
---
 cmds/receive.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/cmds/receive.c b/cmds/receive.c
index d8c934a7c57c..cc3df5c35821 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;
-- 
2.23.0

Re: [RFC PATCH 0/3] btrfs-progs: make quiet to overrule verbose

[David Sterba]

On Thu, Oct 24, 2019 at 02:28:22PM +0800, Anand Jain wrote:
> When both the options (--quiet and --verbose) in btrfs send and receive
> is specified, we need at least one of it to overrule the other, irrespective
> of the chronological order of options.

I think the common behaviour is to respect the order of appearance on
the commandline. So 'command -vvv -q' will be the same as 'command -q',
while 'command -q -vvv' will be 'command -vvv'.

Eg. ssh behaves like that, OTOH rsync does not and -q beats -vvv. I
don't know about other commands that accept multiple -v and -q to get
more samples. The usage pattern where order on command line matters is
following the idea where there's a long line and adding -vvv to the end
will make it verbose.

Re: [RFC PATCH 0/3] btrfs-progs: make quiet to overrule verbose

[Anand Jain]

On 24/10/19 11:41 PM, David Sterba wrote:
> On Thu, Oct 24, 2019 at 02:28:22PM +0800, Anand Jain wrote:
>> When both the options (--quiet and --verbose) in btrfs send and receive
>> is specified, we need at least one of it to overrule the other, irrespective
>> of the chronological order of options.
> 
> I think the common behaviour is to respect the order of appearance on
> the commandline.

   I am fine with this. Will fix it as this.

   (IMO generally command -q is used in scripts so it makes sense to keep
   it absolutely quiet when used. Where as -v is used for
   understanding.).

> So 'command -vvv -q' will be the same as 'command -q',

> while 'command -q -vvv' will be 'command -vvv'.

  We need to fix this. As of now command -q -vvv is command -vv.

Thanks, Anand

> Eg. ssh behaves like that, OTOH rsync does not and -q beats -vvv. I
> don't know about other commands that accept multiple -v and -q to get
> more samples. The usage pattern where order on command line matters is
> following the idea where there's a long line and adding -vvv to the end
> will make it verbose.
> 

Re: [RFC PATCH 0/3] btrfs-progs: make quiet to overrule verbose

[Anand Jain]

On 25/10/19 7:51 AM, Anand Jain wrote:
> 
> 
> On 24/10/19 11:41 PM, David Sterba wrote:
>> On Thu, Oct 24, 2019 at 02:28:22PM +0800, Anand Jain wrote:
>>> When both the options (--quiet and --verbose) in btrfs send and receive
>>> is specified, we need at least one of it to overrule the other, 
>>> irrespective
>>> of the chronological order of options.
>>
>> I think the common behaviour is to respect the order of appearance on
>> the commandline.
> 
>    I am fine with this. Will fix it as this.

  Question: command -v -q -v should be equal to command -v, right?

Thanks, Anand


>    (IMO generally command -q is used in scripts so it makes sense to keep
>    it absolutely quiet when used. Where as -v is used for
>    understanding.).
> 
>> So 'command -vvv -q' will be the same as 'command -q',
> 
>> while 'command -q -vvv' will be 'command -vvv'.
> 
>   We need to fix this. As of now command -q -vvv is command -vv.
> 
> Thanks, Anand
> 
>> Eg. ssh behaves like that, OTOH rsync does not and -q beats -vvv. I
>> don't know about other commands that accept multiple -v and -q to get
>> more samples. The usage pattern where order on command line matters is
>> following the idea where there's a long line and adding -vvv to the end
>> will make it verbose.
>>
> 
> 

Re: [RFC PATCH 0/3] btrfs-progs: make quiet to overrule verbose

[David Sterba]

On Fri, Oct 25, 2019 at 09:56:14AM +0800, Anand Jain wrote:
> On 25/10/19 7:51 AM, Anand Jain wrote:
> > 
> > 
> > On 24/10/19 11:41 PM, David Sterba wrote:
> >> On Thu, Oct 24, 2019 at 02:28:22PM +0800, Anand Jain wrote:
> >>> When both the options (--quiet and --verbose) in btrfs send and receive
> >>> is specified, we need at least one of it to overrule the other, 
> >>> irrespective
> >>> of the chronological order of options.
> >>
> >> I think the common behaviour is to respect the order of appearance on
> >> the commandline.
> > 
> >    I am fine with this. Will fix it as this.
> 
>   Question: command -v -q -v should be equal to command -v, right?

No, that would be equivalent to the default level:

verbose starts with 1			()
verbose++				(-v)
verbose = 0				(-q)
verbose++ is now 1, which is not -v	()

Re: [RFC PATCH 0/3] btrfs-progs: make quiet to overrule verbose

[Anand Jain]

On 26/10/19 12:35 AM, David Sterba wrote:
> On Fri, Oct 25, 2019 at 09:56:14AM +0800, Anand Jain wrote:
>> On 25/10/19 7:51 AM, Anand Jain wrote:
>>>
>>>
>>> On 24/10/19 11:41 PM, David Sterba wrote:
>>>> On Thu, Oct 24, 2019 at 02:28:22PM +0800, Anand Jain wrote:
>>>>> When both the options (--quiet and --verbose) in btrfs send and receive
>>>>> is specified, we need at least one of it to overrule the other,
>>>>> irrespective
>>>>> of the chronological order of options.
>>>>
>>>> I think the common behaviour is to respect the order of appearance on
>>>> the commandline.
>>>
>>>     I am fine with this. Will fix it as this.
>>
>>    Question: command -v -q -v should be equal to command -v, right?
> 
> No, that would be equivalent to the default level:
> 
> verbose starts with 1			()
> verbose++				(-v)
> verbose = 0				(-q)
> verbose++ is now 1, which is not -v	()
> 

Oh I was thinking its a bug, and no need to carry forward to the global
verbose. Will make it look like this.

Re: [RFC PATCH 0/3] btrfs-progs: make quiet to overrule verbose

[Anand Jain]

On 26/10/19 9:01 AM, Anand Jain wrote:
> On 26/10/19 12:35 AM, David Sterba wrote:
>> On Fri, Oct 25, 2019 at 09:56:14AM +0800, Anand Jain wrote:
>>> On 25/10/19 7:51 AM, Anand Jain wrote:
>>>>
>>>>
>>>> On 24/10/19 11:41 PM, David Sterba wrote:
>>>>> On Thu, Oct 24, 2019 at 02:28:22PM +0800, Anand Jain wrote:
>>>>>> When both the options (--quiet and --verbose) in btrfs send and 
>>>>>> receive
>>>>>> is specified, we need at least one of it to overrule the other,
>>>>>> irrespective
>>>>>> of the chronological order of options.
>>>>>
>>>>> I think the common behaviour is to respect the order of appearance on
>>>>> the commandline.
>>>>
>>>>     I am fine with this. Will fix it as this.
>>>
>>>    Question: command -v -q -v should be equal to command -v, right?
>>
>> No, that would be equivalent to the default level:
>>
>> verbose starts with 1            ()
>> verbose++                (-v)
>> verbose = 0                (-q)
>> verbose++ is now 1, which is not -v    ()
>>
> 
> Oh I was thinking its a bug, and no need to carry forward to the global
> verbose. Will make it look like this.
> 

What do you think should be the final %verbose value when both
local and global verbose and or quiet options are specified?

For example:
  btrfs -v -q sub-command -v
  btrfs -q sub-command -v
  btrfs -vv sub-command -q
  etc..

Thanks, Anand

Re: [RFC PATCH 0/3] btrfs-progs: make quiet to overrule verbose

[David Sterba]

On Wed, Oct 30, 2019 at 03:42:56AM +0800, Anand Jain wrote:
> >>>    Question: command -v -q -v should be equal to command -v, right?
> >>
> >> No, that would be equivalent to the default level:
> >>
> >> verbose starts with 1            ()
> >> verbose++                (-v)
> >> verbose = 0                (-q)
> >> verbose++ is now 1, which is not -v    ()
> >>
> > 
> > Oh I was thinking its a bug, and no need to carry forward to the global
> > verbose. Will make it look like this.
> 
> What do you think should be the final %verbose value when both
> local and global verbose and or quiet options are specified?
> 
> For example:
>   btrfs -v -q sub-command -v
>   btrfs -q sub-command -v
>   btrfs -vv sub-command -q
>   etc..

Ah that's the conflicting part. I'd say treat all -v and -q equal, so
modify the bconf.verbose variable, and it's straightforward to document.
Some time in the future we should also issue a warning for 'sub-command
-v'.

The order makes it unintuitive so

  btrfs -q command -v

is going to be the default verbosity. We can't ignore the sub-command
part, and making it conditionally work in case there's no global
verbosity setting is kind of complicating it.

So let's take the simple approach, maybe we'll have second thought on
that before release.

Re: [RFC PATCH 0/3] btrfs-progs: make quiet to overrule verbose

[Anand Jain]

On 11/1/19 11:18 PM, David Sterba wrote:
> On Wed, Oct 30, 2019 at 03:42:56AM +0800, Anand Jain wrote:
>>>>>     Question: command -v -q -v should be equal to command -v, right?
>>>>
>>>> No, that would be equivalent to the default level:
>>>>
>>>> verbose starts with 1            ()
>>>> verbose++                (-v)
>>>> verbose = 0                (-q)
>>>> verbose++ is now 1, which is not -v    ()
>>>>
>>>
>>> Oh I was thinking its a bug, and no need to carry forward to the global
>>> verbose. Will make it look like this.
>>
>> What do you think should be the final %verbose value when both
>> local and global verbose and or quiet options are specified?
>>
>> For example:
>>    btrfs -v -q sub-command -v
>>    btrfs -q sub-command -v
>>    btrfs -vv sub-command -q
>>    etc..
> 
> Ah that's the conflicting part.

> I'd say treat all -v and -q equal,

  Umm I don't understand what is treating equal here.
  The sub-command already treats differently under sub-command options.
  As shown below.

       case 'v':
                 bconf.verbose++;
                 break;
       case 'q':
                 bconf.verbose = 0;
		break;


> so
> modify the bconf.verbose variable, and it's straightforward to document.
> Some time in the future we should also issue a warning for 'sub-command
> -v'.

  I am guessing you mean:- Warning option is deprecated ?

> The order makes it unintuitive so
> 
>    btrfs -q command -v
> 
> is going to be the default verbosity.

  default verbosity is 0? 1 ?
  As of now in send/receive default verbosity is 1. And rest
  of the sub-commands its 0.

  And as the -v is last to appear the command will be
  equivalent to 'btrfs sub-command -q -v' which is verbosity level 1.
  I hope this is reasonable.

> We can't ignore the sub-command
> part, and making it conditionally work in case there's no global
> verbosity setting is kind of complicating it.

  Umm. As of now in v1.1, the sub-command continues to operate on the
  global bconf.verbose values, which works very well. Please see v1.1
  in the ML.

> So let's take the simple approach, maybe we'll have second thought on
> that before release.
> 

  Sure. To simplify the discussion, in v1.1 cover-letter I have included
  verbosity implementation code sample, hope this helps.

verbosity code sample as in v1.1

Thanks, Anand