mbox series

[0/3] btrfs-progs: check: Introduce optional argument for -b|--backup

Message ID 20191021093755.56835-1-wqu@suse.com (mailing list archive)
Headers show
Series btrfs-progs: check: Introduce optional argument for -b|--backup | expand

Message

Qu Wenruo Oct. 21, 2019, 9:37 a.m. UTC
Before this patchset, if we want to use backup roots, it's only possible
to let btrfs-check to automatically choose the backup.

If user want to use a specified backup, it can only use -r|--tree-root
option along with backup roots dump from "btrfs ins dump-super".

This patchset will introduce optional argument for -b|--backup, so user
can specify which backup to use by providing the generation difference
(-3, -2, -1).

If the optional argument is not provided, the default value is -1, and
the behavior should be pretty much the same.

Qu Wenruo (3):
  btrfs-progs: utils-lib: Use error() to replace fprintf(stderr, "ERROR:
    ")
  btrfs-progs: disk-io: Handle backup root more correctly
  btrfs-progs: check: Introduce optional argument for -b|--backup

 Documentation/btrfs-check.asciidoc |  6 ++--
 check/main.c                       | 33 +++++++++++++++---
 common/utils.h                     |  1 +
 ctree.h                            |  8 +++++
 disk-io.c                          | 55 ++++++++++++++++++++++++------
 disk-io.h                          | 33 +++++++++++-------
 utils-lib.c                        | 25 +++++++++++---
 7 files changed, 127 insertions(+), 34 deletions(-)

Comments

David Sterba Nov. 15, 2019, 12:32 p.m. UTC | #1
On Mon, Oct 21, 2019 at 05:37:52PM +0800, Qu Wenruo wrote:
> Before this patchset, if we want to use backup roots, it's only possible
> to let btrfs-check to automatically choose the backup.
> 
> If user want to use a specified backup, it can only use -r|--tree-root
> option along with backup roots dump from "btrfs ins dump-super".
> 
> This patchset will introduce optional argument for -b|--backup, so user
> can specify which backup to use by providing the generation difference
> (-3, -2, -1).

Please don't introduce the optional arguments. I think we've learned the
lesson with 'defrag -c' or balance -d/-m arguments. In this case a long
option would be fine as the backup roots is not something that's used
often. We can keep the --backup as "use first good" and add the more
specific selection.
Qu Wenruo Nov. 15, 2019, 12:36 p.m. UTC | #2
On 2019/11/15 下午8:32, David Sterba wrote:
> On Mon, Oct 21, 2019 at 05:37:52PM +0800, Qu Wenruo wrote:
>> Before this patchset, if we want to use backup roots, it's only possible
>> to let btrfs-check to automatically choose the backup.
>>
>> If user want to use a specified backup, it can only use -r|--tree-root
>> option along with backup roots dump from "btrfs ins dump-super".
>>
>> This patchset will introduce optional argument for -b|--backup, so user
>> can specify which backup to use by providing the generation difference
>> (-3, -2, -1).
> 
> Please don't introduce the optional arguments. I think we've learned the
> lesson with 'defrag -c' or balance -d/-m arguments. In this case a long
> option would be fine as the backup roots is not something that's used
> often. We can keep the --backup as "use first good" and add the more
> specific selection.
>

OK, I'll rename it to something like --backup-gen-diff, and get rid of
the minus geneartion part.

Thanks,
Qu