On 10/22/19 12:12 AM, David Sterba wrote:
On Mon, Oct 21, 2019 at 06:01:08PM +0800, Anand Jain wrote:
This patch set brings --verbose option to the top level btrfs command,
such as 'btrfs --verbose'. With this we don't have to add or remember
verbose option at the sub-commands level.
As there are already verbose options to 11 sub-commands as listed
below [1][2]. So the top level --verbose option here takes care to transpire
verbose request from the top level to the sub-command level for 9 (not 11)
sub-commands as in [1] as of now.
This patch is RFC still for the following two reasons (comments appreciated).
1.
The sub-commands as in [2] uses multi-level compile time verbose option,
such as %g_verbose = 0 (quite), %g_verbose = 1 (default), %g_verbose > 1
(real-verbose). And verbose at default is also part the .out files in
fstests. So it needs further discussions on how to handle the multi-
level verbose option using the global verbose option, and so sub-
commands in [2] are untouched.
The idea is to unify all verbosity options. Default is 1, 0 is for quiet
(only errors are printed), the rest is up to the commands what to print
on the higher levels.
As of now verbosity level is a compile time option. [3]
[3]
-------
cmds/send.c
51 /*
52 * Default is 1 for historical reasons, changing may break scripts
that expect
53 * the 'At subvol' message.
54 */
55 static int g_verbose = 1;
--------
2.
These patch has been unit-tested individually.
These patches does not alter the verbose output.
But it fixes the indentation in the command's help output, which may be
used in fstests and btrfs-progs/tests and their verification is pending
still, which I am planning to do it before v1.
The indentation does not need to be changed if the glboal options are
split from the per-command, like
>
---
usage: btrfs subvolume delete [options] <subvolume> [<subvolume>...]
Delete subvolume(s)
Delete subvolumes from the filesystem. The corresponding directory
is removed instantly but the data blocks are removed later.
The deletion does not involve full commit by default due to
performance reasons (as a consequence, the subvolume may appear again
after a crash). Use one of the --commit options to wait until the
operation is safely stored on the media.
-c|--commit-after wait for transaction commit at the end of the operation
-C|--commit-each wait for transaction commit after deleting each subvolume
Global options:
-v|--verbose show verbose output
---
Oh split it. ok. Makes sense.
Some commands can have long option names or the argument names make it
long in some cases, the global options could stay indented. I think
visually it'll be ok. We can introduce some way to automatically format
the options and help texts so we don't have to adjust them manually each
time, but this would be more intrusive and can be done later.
ok. But my pertaining question is if the sub-command verbose option
should still remain? if no I will be happy to take it out as the same
verbose will anyway be activated using the global verbose option.
With the global verbose option there shouldbe also -q|--quiet. Both
short and long versions should be available for all commands. So the
help would look like:
---
Global options:
-v|--verbose verbose output, repeat for more verbosity
-q|--quiet print only errors
---
In code this looks like:
"",
"-c|--commit-after wait for transaction commit at the end of the operation",
"-C|--commit-each wait for transaction commit after deleting each subvolume",
HELPINFO_GLOBAL_OPTIONS_HEADER,
HELPINFO_INSERT_VERBOSE,
NULL
#define HELPINFO_GLOBAL_OPTIONS_HEADER \
"", \
"Global options:"
and HELPINFO_INSERT_VERBOSE also contains the quiet option.
The global option value is stored in 'btrfs_config_init bconf', so
everything can access it directly.
Oh ok.
In the above code-snap [3].
g_verbose = 0 and g_verbose = 1 can be mapped to the global
-q|--quite and --verbose respectively.
But any idea what to do with g_verbose > 1? which we support
in send.c and receive.c. And in defrag which the patch [4] removed it.
[4]
[RFC PATCH 09/14] btrfs-progs: restore: delete unreachable code
Another way is
btrfs [--quite] [--verbose[=n]]
n=1 default
n=2 verbose
Can't imagine anything better.
Thanks for helping to shape this.
Anand
Thanks for working on this, I'll have more comments on v2 as I probably
forgot a few more things to do, the above is the base for all further
changes.
