* [PATCH 0/3] btrfs-progs: check: Introduce optional argument for -b|--backup
@ 2019-10-21 9:37 Qu Wenruo
2019-10-21 9:37 ` [PATCH 1/3] btrfs-progs: utils-lib: Use error() to replace fprintf(stderr, "ERROR: ") Qu Wenruo
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Qu Wenruo @ 2019-10-21 9:37 UTC (permalink / raw)
To: linux-btrfs
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(-)
--
2.23.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/3] btrfs-progs: utils-lib: Use error() to replace fprintf(stderr, "ERROR: ") 2019-10-21 9:37 [PATCH 0/3] btrfs-progs: check: Introduce optional argument for -b|--backup Qu Wenruo @ 2019-10-21 9:37 ` Qu Wenruo 2019-10-21 9:48 ` Johannes Thumshirn 2019-10-21 9:37 ` [PATCH 2/3] btrfs-progs: disk-io: Handle backup root more correctly Qu Wenruo ` (2 subsequent siblings) 3 siblings, 1 reply; 7+ messages in thread From: Qu Wenruo @ 2019-10-21 9:37 UTC (permalink / raw) To: linux-btrfs This saves several lines of code. Signed-off-by: Qu Wenruo <wqu@suse.com> --- utils-lib.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/utils-lib.c b/utils-lib.c index c2b6097f5df9..0202dd7677f0 100644 --- a/utils-lib.c +++ b/utils-lib.c @@ -23,8 +23,7 @@ u64 arg_strtou64(const char *str) value = strtoull(str, &ptr_parse_end, 0); if (ptr_parse_end && *ptr_parse_end != '\0') { - fprintf(stderr, "ERROR: %s is not a valid numeric value.\n", - str); + error("%s is not a valid numeric value.", str); exit(1); } @@ -33,12 +32,11 @@ u64 arg_strtou64(const char *str) * unexpected number to us, so let's do the check ourselves. */ if (str[0] == '-') { - fprintf(stderr, "ERROR: %s: negative value is invalid.\n", - str); + error("%s: negative value is invalid.", str); exit(1); } if (value == ULLONG_MAX) { - fprintf(stderr, "ERROR: %s is too large.\n", str); + error("%s is too large.", str); exit(1); } return value; -- 2.23.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] btrfs-progs: utils-lib: Use error() to replace fprintf(stderr, "ERROR: ") 2019-10-21 9:37 ` [PATCH 1/3] btrfs-progs: utils-lib: Use error() to replace fprintf(stderr, "ERROR: ") Qu Wenruo @ 2019-10-21 9:48 ` Johannes Thumshirn 0 siblings, 0 replies; 7+ messages in thread From: Johannes Thumshirn @ 2019-10-21 9:48 UTC (permalink / raw) To: Qu Wenruo, linux-btrfs Looks good, Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de> -- Johannes Thumshirn SUSE Labs Filesystems jthumshirn@suse.de +49 911 74053 689 SUSE Software Solutions Germany GmbH Maxfeldstr. 5 90409 Nürnberg Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/3] btrfs-progs: disk-io: Handle backup root more correctly 2019-10-21 9:37 [PATCH 0/3] btrfs-progs: check: Introduce optional argument for -b|--backup Qu Wenruo 2019-10-21 9:37 ` [PATCH 1/3] btrfs-progs: utils-lib: Use error() to replace fprintf(stderr, "ERROR: ") Qu Wenruo @ 2019-10-21 9:37 ` Qu Wenruo 2019-10-21 9:37 ` [PATCH 3/3] btrfs-progs: check: Introduce optional argument for -b|--backup Qu Wenruo 2019-11-15 12:32 ` [PATCH 0/3] " David Sterba 3 siblings, 0 replies; 7+ messages in thread From: Qu Wenruo @ 2019-10-21 9:37 UTC (permalink / raw) To: linux-btrfs Current backup root handling has extra check on super generation: static int find_best_backup_root(struct btrfs_super_block *super) { u64 orig_gen = btrfs_super_generation(super); ... if (btrfs_backup_tree_root_gen(backup) != orig_gen && btrfs_backup_tree_root_gen(backup) > gen) { best_index = i; gen = btrfs_backup_tree_root_gen(backup); This check is to ensure we don't get backup root with current generation, but it can still return backup root newer than current root. So for the following super: generation: 10 backup[0] generation: 8 backup[1] generation: 9 backup[2] generation: 10 backup[3] generation: 11 If we're calling find_best_backup_root() then we can pick up slot 3 which is newer than current generation. This patch introduce a new parameter for find_best_backup_root() to specify the max generation. So with above superblock, calling find_best_backup_root(sb, sb_gen - 1) will ensure we get slot 1, other than slot 3. This also affects how we update backup roots. Furthermore, due to the change in the return value, find_best_backup_root() can now return -1 to indicates error (no valid backup found), so change callers to co-operate. Signed-off-by: Qu Wenruo <wqu@suse.com> --- disk-io.c | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/disk-io.c b/disk-io.c index be44eead5cef..36db1be264cd 100644 --- a/disk-io.c +++ b/disk-io.c @@ -845,17 +845,22 @@ int btrfs_check_fs_compatibility(struct btrfs_super_block *sb, return 0; } -static int find_best_backup_root(struct btrfs_super_block *super) +/* + * Find the newest backup slot whose generation <= @max_gen + * + * Can return <0 for error, indicating no valid backup slot for @max_gen. + */ +static int find_best_backup_root(struct btrfs_super_block *super, + u64 max_gen) { struct btrfs_root_backup *backup; - u64 orig_gen = btrfs_super_generation(super); u64 gen = 0; - int best_index = 0; + int best_index = -1; int i; for (i = 0; i < BTRFS_NUM_BACKUP_ROOTS; i++) { backup = super->super_roots + i; - if (btrfs_backup_tree_root_gen(backup) != orig_gen && + if (btrfs_backup_tree_root_gen(backup) <= max_gen && btrfs_backup_tree_root_gen(backup) > gen) { best_index = i; gen = btrfs_backup_tree_root_gen(backup); @@ -908,9 +913,10 @@ int btrfs_setup_all_roots(struct btrfs_fs_info *fs_info, u64 root_tree_bytenr, root_tree_bytenr = btrfs_super_root(sb); } else if (flags & OPEN_CTREE_BACKUP_ROOT) { struct btrfs_root_backup *backup; - int index = find_best_backup_root(sb); - if (index >= BTRFS_NUM_BACKUP_ROOTS) { - fprintf(stderr, "Invalid backup root number\n"); + int index = find_best_backup_root(sb, + btrfs_super_generation(sb) - 1); + if (index < 0) { + error("can't find any valid backup root"); return -EIO; } backup = fs_info->super_copy->super_roots + index; @@ -1707,10 +1713,22 @@ static int write_dev_supers(struct btrfs_fs_info *fs_info, static void backup_super_roots(struct btrfs_fs_info *info) { struct btrfs_root_backup *root_backup; + u64 current_gen = btrfs_super_generation(info->super_copy); int next_backup; int last_backup; - last_backup = find_best_backup_root(info->super_copy); + last_backup = find_best_backup_root(info->super_copy, current_gen - 1); + /* No older backups, retry current gen */ + if (last_backup < 0) { + last_backup = find_best_backup_root(info->super_copy, + current_gen); + /* + * Still failed, means no valid backup root at all, restart + * from slot 0. + */ + if (last_backup < 0) + last_backup = 0; + } next_backup = (last_backup + 1) % BTRFS_NUM_BACKUP_ROOTS; /* just overwrite the last backup if we're at the same generation */ -- 2.23.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] btrfs-progs: check: Introduce optional argument for -b|--backup 2019-10-21 9:37 [PATCH 0/3] btrfs-progs: check: Introduce optional argument for -b|--backup Qu Wenruo 2019-10-21 9:37 ` [PATCH 1/3] btrfs-progs: utils-lib: Use error() to replace fprintf(stderr, "ERROR: ") Qu Wenruo 2019-10-21 9:37 ` [PATCH 2/3] btrfs-progs: disk-io: Handle backup root more correctly Qu Wenruo @ 2019-10-21 9:37 ` Qu Wenruo 2019-11-15 12:32 ` [PATCH 0/3] " David Sterba 3 siblings, 0 replies; 7+ messages in thread From: Qu Wenruo @ 2019-10-21 9:37 UTC (permalink / raw) To: linux-btrfs Add an optional argument, <gen_diff> for -b|--backup. This optional argument allow user to specify the generation difference to search for the best backup root. The value values are: -3, -2, -1. To co-operate with this change, the following modifications are made: - Man page and help string update - New OPEN_CTREE flags and helpers are introduced OPEN_CTREE_BACKUP_GEN_DIFF[123] are introduced to replace old single bit OPEN_CTREE_BACKUP_ROOT. New helpers backup_gen_diff_to_cflags() and cflags_to_backup_gen_diff() are introduced to do the convert. So that we can keep the old open_ctree() interface without introducing new parameters. - New arg_strol() helper to get negative int - New btrfs_fs_info::backup_gen_diff member Now btrfs_setup_all_roots() uses that member to search backup roots. Signed-off-by: Qu Wenruo <wqu@suse.com> --- Documentation/btrfs-check.asciidoc | 6 ++++-- check/main.c | 33 ++++++++++++++++++++++++++---- common/utils.h | 1 + ctree.h | 8 ++++++++ disk-io.c | 25 ++++++++++++++++++---- disk-io.h | 33 ++++++++++++++++++------------ utils-lib.c | 17 +++++++++++++++ 7 files changed, 100 insertions(+), 23 deletions(-) diff --git a/Documentation/btrfs-check.asciidoc b/Documentation/btrfs-check.asciidoc index b963eae5cdce..84a4241069cf 100644 --- a/Documentation/btrfs-check.asciidoc +++ b/Documentation/btrfs-check.asciidoc @@ -41,8 +41,10 @@ filesystem, similarly the run time. SAFE OR ADVISORY OPTIONS ------------------------ --b|--backup:: -use the first valid set of backup roots stored in the superblock +-b[<gen_diff>] | --backup[=<gen_diff>]:: +use the newest valid set of backup roots which is no older than 'generation + <gen_diff>' ++ +'<gen_diff>' is optional, if not given, default value is -1. Valid values are -3, -2 ,-1. + This can be combined with '--super' if some of the superblocks are damaged. diff --git a/check/main.c b/check/main.c index c2d0f3949c5e..46f3e3d4c5b5 100644 --- a/check/main.c +++ b/check/main.c @@ -9773,7 +9773,9 @@ static const char * const cmd_check_usage[] = { "Options:", " starting point selection:", " -s|--super <superblock> use this superblock copy", - " -b|--backup use the first valid backup root copy", + " -b|--backup[=<gen_diff>] use the backup root with <gen_diff>", + " <gen_diff> is an optional parameter,", + " valid values are -3, -2, -1 (default).", " -r|--tree-root <bytenr> use the given bytenr for the tree root", " --chunk-root <bytenr> use the given bytenr for the chunk tree root", " operation modes:", @@ -9799,6 +9801,17 @@ static const char * const cmd_check_usage[] = { NULL }; +static unsigned backup_gen_diff_to_cflags(int backup_gen_diff) +{ + ASSERT(-3 <= backup_gen_diff && backup_gen_diff <= -1); + + if (backup_gen_diff == -3) + return OPEN_CTREE_BACKUP_GEN_DIFF3; + if (backup_gen_diff == -2) + return OPEN_CTREE_BACKUP_GEN_DIFF2; + return OPEN_CTREE_BACKUP_GEN_DIFF1; +} + static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv) { struct cache_tree root_cache; @@ -9818,6 +9831,7 @@ static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv) int qgroup_report = 0; int qgroups_repaired = 0; int qgroup_report_ret; + int backup_gen_diff; unsigned ctree_flags = OPEN_CTREE_EXCLUSIVE; int force = 0; @@ -9838,7 +9852,7 @@ static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv) GETOPT_VAL_INIT_EXTENT }, { "check-data-csum", no_argument, NULL, GETOPT_VAL_CHECK_CSUM }, - { "backup", no_argument, NULL, 'b' }, + { "backup", optional_argument, NULL, 'b' }, { "subvol-extents", required_argument, NULL, 'E' }, { "qgroup-report", no_argument, NULL, 'Q' }, { "tree-root", required_argument, NULL, 'r' }, @@ -9853,13 +9867,24 @@ static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv) { NULL, 0, NULL, 0} }; - c = getopt_long(argc, argv, "as:br:pE:Q", long_options, NULL); + c = getopt_long(argc, argv, "as:b::r:pE:Q", long_options, NULL); if (c < 0) break; switch(c) { case 'a': /* ignored */ break; case 'b': - ctree_flags |= OPEN_CTREE_BACKUP_ROOT; + if (optarg) { + backup_gen_diff = arg_strtol(optarg); + } else + backup_gen_diff = -1; + if (!(-3 <= backup_gen_diff && + backup_gen_diff <= -1)) { + error( + "backup_gen_diff must be in [-3, -1]"); + exit(1); + } + ctree_flags |= backup_gen_diff_to_cflags( + backup_gen_diff); break; case 's': num = arg_strtou64(optarg); diff --git a/common/utils.h b/common/utils.h index 7867fb0a35d9..26bc2b081356 100644 --- a/common/utils.h +++ b/common/utils.h @@ -68,6 +68,7 @@ const char *pretty_size_mode(u64 size, unsigned mode); u64 parse_size(char *s); u64 parse_qgroupid(const char *p); u64 arg_strtou64(const char *str); +int long arg_strtol(const char *str); int open_file_or_dir(const char *fname, DIR **dirstream); int open_file_or_dir3(const char *fname, DIR **dirstream, int open_flags); void close_file_or_dir(int fd, DIR *dirstream); diff --git a/ctree.h b/ctree.h index 0d12563b7261..5f83d9631678 100644 --- a/ctree.h +++ b/ctree.h @@ -1176,6 +1176,14 @@ struct btrfs_fs_info { unsigned int finalize_on_close:1; int transaction_aborted; + /* + * The generation diff for backup root use. + * Valid values are [-3, 0] + * [-3, -1]: Use backup root whose generation is no newer than + * current_gen + diff + * 0: Don't use backup root at all. + */ + int backup_gen_diff; int (*free_extent_hook)(struct btrfs_fs_info *fs_info, u64 bytenr, u64 num_bytes, u64 parent, diff --git a/disk-io.c b/disk-io.c index 36db1be264cd..4186a6544dce 100644 --- a/disk-io.c +++ b/disk-io.c @@ -909,14 +909,18 @@ int btrfs_setup_all_roots(struct btrfs_fs_info *fs_info, u64 root_tree_bytenr, btrfs_setup_root(root, fs_info, BTRFS_ROOT_TREE_OBJECTID); generation = btrfs_super_generation(sb); - if (!root_tree_bytenr && !(flags & OPEN_CTREE_BACKUP_ROOT)) { + if (!root_tree_bytenr && !fs_info->backup_gen_diff) { root_tree_bytenr = btrfs_super_root(sb); - } else if (flags & OPEN_CTREE_BACKUP_ROOT) { + } else if (fs_info->backup_gen_diff) { struct btrfs_root_backup *backup; int index = find_best_backup_root(sb, - btrfs_super_generation(sb) - 1); + btrfs_super_generation(sb) + + fs_info->backup_gen_diff); if (index < 0) { - error("can't find any valid backup root"); + error( + "can't find valid backup root with no newer than gen %llu", + btrfs_super_generation(sb) + + fs_info->backup_gen_diff); return -EIO; } backup = fs_info->super_copy->super_roots + index; @@ -1150,6 +1154,17 @@ int btrfs_setup_chunk_tree_and_device_map(struct btrfs_fs_info *fs_info, return 0; } +static int cflags_to_backup_gen_diff(unsigned flags) +{ + if (flags & OPEN_CTREE_BACKUP_GEN_DIFF3) + return -3; + if (flags & OPEN_CTREE_BACKUP_GEN_DIFF2) + return -2; + if (flags & OPEN_CTREE_BACKUP_GEN_DIFF1) + return -1; + return 0; +} + static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path, u64 sb_bytenr, u64 root_tree_bytenr, @@ -1261,6 +1276,8 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path, btrfs_header_chunk_tree_uuid(eb), BTRFS_UUID_SIZE); + fs_info->backup_gen_diff = cflags_to_backup_gen_diff(flags); + ret = btrfs_setup_all_roots(fs_info, root_tree_bytenr, flags); if (ret && !(flags & __OPEN_CTREE_RETURN_CHUNK_ROOT) && !fs_info->ignore_chunk_tree_error) diff --git a/disk-io.h b/disk-io.h index 7b5c3806ba98..cebedf8ff408 100644 --- a/disk-io.h +++ b/disk-io.h @@ -34,25 +34,23 @@ enum btrfs_open_ctree_flags { OPEN_CTREE_WRITES = (1U << 0), /* Allow to open filesystem with some broken tree roots (eg log root) */ OPEN_CTREE_PARTIAL = (1U << 1), - /* If primary root pinters are invalid, try backup copies */ - OPEN_CTREE_BACKUP_ROOT = (1U << 2), /* Allow reading all superblock copies if the primary is damaged */ - OPEN_CTREE_RECOVER_SUPER = (1U << 3), + OPEN_CTREE_RECOVER_SUPER = (1U << 2), /* Restoring filesystem image */ - OPEN_CTREE_RESTORE = (1U << 4), + OPEN_CTREE_RESTORE = (1U << 3), /* Do not read block groups (extent tree) */ - OPEN_CTREE_NO_BLOCK_GROUPS = (1U << 5), + OPEN_CTREE_NO_BLOCK_GROUPS = (1U << 4), /* Open all devices in O_EXCL mode */ - OPEN_CTREE_EXCLUSIVE = (1U << 6), + OPEN_CTREE_EXCLUSIVE = (1U << 5), /* Do not scan devices */ - OPEN_CTREE_NO_DEVICES = (1U << 7), + OPEN_CTREE_NO_DEVICES = (1U << 6), /* * Don't print error messages if bytenr or checksums do not match in * tree block headers. Turn on by OPEN_CTREE_SUPPRESS_ERROR */ - OPEN_CTREE_SUPPRESS_CHECK_BLOCK_ERRORS = (1U << 8), + OPEN_CTREE_SUPPRESS_CHECK_BLOCK_ERRORS = (1U << 7), /* Return the chunk root */ - __OPEN_CTREE_RETURN_CHUNK_ROOT = (1U << 9), + __OPEN_CTREE_RETURN_CHUNK_ROOT = (1U << 8), OPEN_CTREE_CHUNK_ROOT_ONLY = OPEN_CTREE_PARTIAL + OPEN_CTREE_SUPPRESS_CHECK_BLOCK_ERRORS + __OPEN_CTREE_RETURN_CHUNK_ROOT, @@ -63,7 +61,7 @@ enum btrfs_open_ctree_flags { */ /* Ignore UUID mismatches */ - OPEN_CTREE_IGNORE_FSID_MISMATCH = (1U << 10), + OPEN_CTREE_IGNORE_FSID_MISMATCH = (1U << 9), /* * Allow open_ctree_fs_info() to return an incomplete fs_info with @@ -71,20 +69,29 @@ enum btrfs_open_ctree_flags { * It's useful when chunks are corrupted. * Makes no sense for open_ctree variants returning btrfs_root. */ - OPEN_CTREE_IGNORE_CHUNK_TREE_ERROR = (1U << 11), + OPEN_CTREE_IGNORE_CHUNK_TREE_ERROR = (1U << 10), /* * Allow to open fs with temporary superblock (BTRFS_MAGIC_PARTIAL), * such fs contains very basic tree layout, just able to be opened. * Such temporary super is used for mkfs or convert. */ - OPEN_CTREE_TEMPORARY_SUPER = (1U << 12), + OPEN_CTREE_TEMPORARY_SUPER = (1U << 11), /* * Invalidate the free space tree (i.e., clear the FREE_SPACE_TREE_VALID * compat_ro bit). */ - OPEN_CTREE_INVALIDATE_FST = (1U << 13), + OPEN_CTREE_INVALIDATE_FST = (1U << 12), + + /* + * If primary root pinters are invalid, try backup copies + * The tailing number is the generation diff, for better + * backup root control. + */ + OPEN_CTREE_BACKUP_GEN_DIFF1 = (1U << 13), + OPEN_CTREE_BACKUP_GEN_DIFF2 = (1U << 14), + OPEN_CTREE_BACKUP_GEN_DIFF3 = (1U << 15), }; /* diff --git a/utils-lib.c b/utils-lib.c index 0202dd7677f0..2eb2bc8bbd0d 100644 --- a/utils-lib.c +++ b/utils-lib.c @@ -42,6 +42,23 @@ u64 arg_strtou64(const char *str) return value; } +int long arg_strtol(const char *str) +{ + int long value; + char *ptr_parse_end = NULL; + + value = strtol(str, &ptr_parse_end, 0); + if (ptr_parse_end && *ptr_parse_end != '\0') { + error("%s is not a valid numeric value.", str); + exit(1); + } + if (errno == ERANGE) { + error("%s is out of valid range.", str); + exit(1); + } + return value; +} + /* * For a given: * - file or directory return the containing tree root id -- 2.23.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 0/3] btrfs-progs: check: Introduce optional argument for -b|--backup 2019-10-21 9:37 [PATCH 0/3] btrfs-progs: check: Introduce optional argument for -b|--backup Qu Wenruo ` (2 preceding siblings ...) 2019-10-21 9:37 ` [PATCH 3/3] btrfs-progs: check: Introduce optional argument for -b|--backup Qu Wenruo @ 2019-11-15 12:32 ` David Sterba 2019-11-15 12:36 ` Qu Wenruo 3 siblings, 1 reply; 7+ messages in thread From: David Sterba @ 2019-11-15 12:32 UTC (permalink / raw) To: Qu Wenruo; +Cc: linux-btrfs 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/3] btrfs-progs: check: Introduce optional argument for -b|--backup 2019-11-15 12:32 ` [PATCH 0/3] " David Sterba @ 2019-11-15 12:36 ` Qu Wenruo 0 siblings, 0 replies; 7+ messages in thread From: Qu Wenruo @ 2019-11-15 12:36 UTC (permalink / raw) To: dsterba, Qu Wenruo, linux-btrfs [-- Attachment #1.1: Type: text/plain, Size: 1015 bytes --] 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 [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-11-15 12:37 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-10-21 9:37 [PATCH 0/3] btrfs-progs: check: Introduce optional argument for -b|--backup Qu Wenruo 2019-10-21 9:37 ` [PATCH 1/3] btrfs-progs: utils-lib: Use error() to replace fprintf(stderr, "ERROR: ") Qu Wenruo 2019-10-21 9:48 ` Johannes Thumshirn 2019-10-21 9:37 ` [PATCH 2/3] btrfs-progs: disk-io: Handle backup root more correctly Qu Wenruo 2019-10-21 9:37 ` [PATCH 3/3] btrfs-progs: check: Introduce optional argument for -b|--backup Qu Wenruo 2019-11-15 12:32 ` [PATCH 0/3] " David Sterba 2019-11-15 12:36 ` Qu Wenruo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox