diff --git a/functional-tests/command-line/thin-check.scm b/functional-tests/command-line/thin-check.scm index 1f93122..143bef8 100644 --- a/functional-tests/command-line/thin-check.scm +++ b/functional-tests/command-line/thin-check.scm @@ -12,6 +12,8 @@ " {-q|--quiet}\n" " {-h|--help}\n" " {-V|--version}\n" + " {--fix-metadata-leaks}\n" + " {--override-mapping-root}\n" " {--clear-needs-check-flag}\n" " {--ignore-non-fatal-errors}\n" " {--skip-mappings}\n" diff --git a/functional-tests/scenario-string-constants.scm b/functional-tests/scenario-string-constants.scm index a1779e8..09cb7dd 100644 --- a/functional-tests/scenario-string-constants.scm +++ b/functional-tests/scenario-string-constants.scm @@ -30,6 +30,7 @@ Options: {-h|--help} {-V|--version} {-m|--metadata-snap} + {--fix-metadata-leaks} {--override-mapping-root} {--clear-needs-check-flag} {--ignore-non-fatal-errors} diff --git a/functional-tests/thin-functional-tests.scm b/functional-tests/thin-functional-tests.scm index d5e5d00..07fd62c 100644 --- a/functional-tests/thin-functional-tests.scm +++ b/functional-tests/thin-functional-tests.scm @@ -95,6 +95,24 @@ "Unrecognised option should cause failure" (run-fail (thin-check "--hedgehogs-only"))) + (define-scenario (thin-check incompatible-options fix-metadata-leaks) + "Incompatible options should cause failure" + (with-valid-metadata (md) + (run-fail (thin-check "--fix-metadata-leaks" "-m" md)) + (run-fail (thin-check "--fix-metadata-leaks" "--override-mapping-root 123" md)) + (run-fail (thin-check "--fix-metadata-leaks" "--super-block-only" md)) + (run-fail (thin-check "--fix-metadata-leaks" "--skip-mappings" md)) + (run-fail (thin-check "--fix-metadata-leaks" "--ignore-non-fatal-errors" md)))) + + (define-scenario (thin-check incompatible-options clear-needs-check-flag) + "Incompatible options should cause failure" + (with-valid-metadata (md) + (run-fail (thin-check "--clear-needs-check-flag" "-m" md)) + (run-fail (thin-check "--clear-needs-check-flag" "--override-mapping-root 123" md)) + (run-fail (thin-check "--clear-needs-check-flag" "--super-block-only" md)) + (run-fail (thin-check "--clear-needs-check-flag" "--skip-mappings" md)) + (run-fail (thin-check "--clear-needs-check-flag" "--ignore-non-fatal-errors" md)))) + (define-scenario (thin-check superblock-only-valid) "--super-block-only check passes on valid metadata" (with-valid-metadata (md) @@ -127,6 +145,11 @@ (with-valid-metadata (md) (run-ok (thin-check "--clear-needs-check-flag" md)))) + (define-scenario (thin-check fix-metadata-leaks) + "Accepts --fix-metadata-leaks" + (with-valid-metadata (md) + (run-ok (thin-check "--fix-metadata-leaks" md)))) + (define-scenario (thin-check tiny-metadata) "Prints helpful message in case tiny metadata given" (with-temp-file-sized ((md "thin.bin" 1024)) diff --git a/thin-provisioning/mapping_tree.cc b/thin-provisioning/mapping_tree.cc index a73dc3a..4d7d998 100644 --- a/thin-provisioning/mapping_tree.cc +++ b/thin-provisioning/mapping_tree.cc @@ -229,7 +229,7 @@ void thin_provisioning::walk_mapping_tree(dev_tree const &tree, mapping_tree_detail::device_visitor &dev_v, mapping_tree_detail::damage_visitor &dv, - bool ignore_non_fatal) + bool ignore_non_fatal) { dev_tree_damage_visitor ll_dv(dv); btree_visit_values(tree, dev_v, ll_dv, ignore_non_fatal); @@ -238,7 +238,7 @@ thin_provisioning::walk_mapping_tree(dev_tree const &tree, void thin_provisioning::check_mapping_tree(dev_tree const &tree, mapping_tree_detail::damage_visitor &visitor, - bool ignore_non_fatal) + bool ignore_non_fatal) { noop_block_visitor dev_v; walk_mapping_tree(tree, dev_v, visitor, ignore_non_fatal); @@ -248,7 +248,7 @@ void thin_provisioning::walk_mapping_tree(mapping_tree const &tree, mapping_tree_detail::mapping_visitor &mv, mapping_tree_detail::damage_visitor &dv, - bool ignore_non_fatal) + bool ignore_non_fatal) { mapping_tree_damage_visitor ll_dv(dv); btree_visit_values(tree, mv, ll_dv, ignore_non_fatal); @@ -257,7 +257,7 @@ thin_provisioning::walk_mapping_tree(mapping_tree const &tree, void thin_provisioning::check_mapping_tree(mapping_tree const &tree, mapping_tree_detail::damage_visitor &visitor, - bool ignore_non_fatal) + bool ignore_non_fatal) { noop_block_time_visitor mv; walk_mapping_tree(tree, mv, visitor, ignore_non_fatal); @@ -268,7 +268,7 @@ thin_provisioning::walk_mapping_tree(single_mapping_tree const &tree, uint64_t dev_id, mapping_tree_detail::mapping_visitor &mv, mapping_tree_detail::damage_visitor &dv, - bool ignore_non_fatal) + bool ignore_non_fatal) { single_mapping_tree_damage_visitor ll_dv(dv, dev_id); btree_visit_values(tree, mv, ll_dv, ignore_non_fatal); @@ -278,7 +278,7 @@ void thin_provisioning::check_mapping_tree(single_mapping_tree const &tree, uint64_t dev_id, mapping_tree_detail::damage_visitor &visitor, - bool ignore_non_fatal) + bool ignore_non_fatal) { noop_block_time_visitor mv; walk_mapping_tree(tree, dev_id, mv, visitor, ignore_non_fatal); diff --git a/thin-provisioning/mapping_tree.h b/thin-provisioning/mapping_tree.h index 870d614..24207ed 100644 --- a/thin-provisioning/mapping_tree.h +++ b/thin-provisioning/mapping_tree.h @@ -130,31 +130,31 @@ namespace thin_provisioning { void walk_mapping_tree(dev_tree const &tree, mapping_tree_detail::device_visitor &dev_v, mapping_tree_detail::damage_visitor &dv, - bool ignore_non_fatal = false); + bool ignore_non_fatal = false); void walk_mapping_tree(mapping_tree const &tree, mapping_tree_detail::mapping_visitor &mv, mapping_tree_detail::damage_visitor &dv, - bool ignore_non_fatal = false); + bool ignore_non_fatal = false); void walk_mapping_tree(single_mapping_tree const &tree, uint64_t dev_id, mapping_tree_detail::mapping_visitor &mv, mapping_tree_detail::damage_visitor &dv, - bool ignore_non_fatal = false); - + bool ignore_non_fatal = false); + void check_mapping_tree(single_mapping_tree const &tree, uint64_t dev_id, mapping_tree_detail::damage_visitor &visitor, - bool ignore_non_fatal = false); + bool ignore_non_fatal = false); void check_mapping_tree(dev_tree const &tree, mapping_tree_detail::damage_visitor &visitor, - bool ignore_non_fatal = false); + bool ignore_non_fatal = false); void check_mapping_tree(mapping_tree const &tree, mapping_tree_detail::damage_visitor &visitor, - bool ignore_non_fatal = false); + bool ignore_non_fatal = false); } //---------------------------------------------------------------- diff --git a/thin-provisioning/metadata.cc b/thin-provisioning/metadata.cc index 3c5446a..f6adcb9 100644 --- a/thin-provisioning/metadata.cc +++ b/thin-provisioning/metadata.cc @@ -124,7 +124,7 @@ metadata::metadata(block_manager::ptr bm, throw runtime_error("no current metadata snap"); if (metadata_snap && *metadata_snap != actual_sb.metadata_snap_) - throw runtime_error("metadata snapshot does not match that in superblock"); + throw runtime_error("metadata snapshot does not match that in superblock"); sb_ = read_superblock(bm, actual_sb.metadata_snap_); diff --git a/thin-provisioning/metadata_checker.cc b/thin-provisioning/metadata_checker.cc index 7a619c9..b3e05c1 100644 --- a/thin-provisioning/metadata_checker.cc +++ b/thin-provisioning/metadata_checker.cc @@ -16,6 +16,7 @@ // with thin-provisioning-tools. If not, see // . +#include "base/error_state.h" #include "base/nested_output.h" #include "persistent-data/file_utils.h" #include "persistent-data/space-maps/core.h" @@ -151,7 +152,7 @@ namespace { error_state examine_devices_tree_(transaction_manager::ptr tm, superblock_detail::superblock const &sb, nested_output &out, - bool ignore_non_fatal) { + bool ignore_non_fatal) { out << "examining devices tree" << end_message(); nested_output::nest _ = out.push(); @@ -166,7 +167,7 @@ namespace { error_state examine_top_level_mapping_tree_(transaction_manager::ptr tm, superblock_detail::superblock const &sb, nested_output &out, - bool ignore_non_fatal) { + bool ignore_non_fatal) { out << "examining top level of mapping tree" << end_message(); nested_output::nest _ = out.push(); @@ -182,7 +183,7 @@ namespace { superblock_detail::superblock const &sb, nested_output &out, optional data_sm, - bool ignore_non_fatal) { + bool ignore_non_fatal) { out << "examining mapping tree" << end_message(); nested_output::nest _ = out.push(); @@ -202,7 +203,7 @@ namespace { error_state examine_top_level_mapping_tree(transaction_manager::ptr tm, superblock_detail::superblock const &sb, nested_output &out, - bool ignore_non_fatal) { + bool ignore_non_fatal) { error_state err = examine_devices_tree_(tm, sb, out, ignore_non_fatal); err << examine_top_level_mapping_tree_(tm, sb, out, ignore_non_fatal); @@ -212,31 +213,23 @@ namespace { error_state examine_mapping_tree(transaction_manager::ptr tm, superblock_detail::superblock const &sb, nested_output &out, - optional data_sm, - bool ignore_non_fatal) { + optional data_sm, + bool ignore_non_fatal) { error_state err = examine_devices_tree_(tm, sb, out, ignore_non_fatal); err << examine_mapping_tree_(tm, sb, out, data_sm, ignore_non_fatal); return err; } - error_state check_space_map_counts(transaction_manager::ptr tm, - superblock_detail::superblock const &sb, - nested_output &out) { - out << "checking space map counts" << end_message(); - nested_output::nest _ = out.push(); - - block_counter bc; - count_metadata(tm, sb, bc); - - // Finally we need to check the metadata space map agrees - // with the counts we've just calculated. + error_state compare_metadata_space_maps(space_map::ptr actual, + block_counter const &expected, + nested_output &out) { error_state err = NO_ERROR; - persistent_space_map::ptr metadata_sm = - open_metadata_sm(*tm, static_cast(&sb.metadata_space_map_root_)); - for (unsigned b = 0; b < metadata_sm->get_nr_blocks(); b++) { - ref_t c_actual = metadata_sm->get_count(b); - ref_t c_expected = bc.get_count(b); + block_address nr_blocks = actual->get_nr_blocks(); + + for (block_address b = 0; b < nr_blocks; b++) { + ref_t c_actual = actual->get_count(b); + ref_t c_expected = expected.get_count(b); if (c_actual != c_expected) { out << "metadata reference counts differ for block " << b @@ -251,6 +244,67 @@ namespace { return err; } + error_state collect_leaked_blocks(space_map::ptr actual, + block_counter const &expected, + std::set &leaked) { + error_state err = NO_ERROR; + block_address nr_blocks = actual->get_nr_blocks(); + + for (block_address b = 0; b < nr_blocks; b++) { + ref_t c_actual = actual->get_count(b); + ref_t c_expected = expected.get_count(b); + + if (c_actual == c_expected) + continue; + + if (c_actual < c_expected) { + err << FATAL; + break; + } + + // Theoretically, the ref-count of a leaked block + // should be only one. Here a leaked ref-count of two + // is allowed. + if (c_expected || c_actual >= 3) + err << NON_FATAL; + else if (c_actual > 0) + leaked.insert(b); + } + + return err; + } + + error_state clear_leaked_blocks(space_map::ptr actual, + block_counter const &expected) { + error_state err = NO_ERROR; + std::set leaked; + + err << collect_leaked_blocks(actual, expected, leaked); + if (err != NO_ERROR) + return err; + + for (auto const &b : leaked) + actual->set_count(b, 0); + + return err; + } + + error_state check_metadata_space_map_counts(transaction_manager::ptr tm, + superblock_detail::superblock const &sb, + block_counter &bc, + nested_output &out) { + out << "checking space map counts" << end_message(); + nested_output::nest _ = out.push(); + + count_metadata(tm, sb, bc); + + // Finally we need to check the metadata space map agrees + // with the counts we've just calculated. + space_map::ptr metadata_sm = + open_metadata_sm(*tm, static_cast(&sb.metadata_space_map_root_)); + return compare_metadata_space_maps(metadata_sm, bc, out); + } + error_state compare_space_maps(space_map::ptr actual, space_map::ptr expected, nested_output &out) { @@ -265,7 +319,7 @@ namespace { for (block_address b = 0; b < nr_blocks; b++) { auto a_count = actual->get_count(b); auto e_count = actual->get_count(b); - + if (a_count != e_count) { out << "data reference counts differ for block " << b << ", expected " << e_count @@ -297,40 +351,36 @@ namespace { class metadata_checker { public: - metadata_checker(block_manager::ptr bm, + metadata_checker(std::string const &path, check_options check_opts, output_options output_opts) - : bm_(bm), + : path_(path), options_(check_opts), out_(cerr, 2), - info_out_(cout, 0) { + info_out_(cout, 0), + err_(NO_ERROR) { if (output_opts == OUTPUT_QUIET) { out_.disable(); info_out_.disable(); } + + sb_location_ = get_superblock_location(); } - error_state check() { - error_state err = NO_ERROR; - auto sb_location = superblock_detail::SUPERBLOCK_LOCATION; + void check() { + block_manager::ptr bm = open_bm(path_, block_manager::READ_ONLY, + !options_.use_metadata_snap_); - if (options_.use_metadata_snap_) { - superblock_detail::superblock sb = read_superblock(bm_, sb_location); - sb_location = sb.metadata_snap_; - if (sb_location == superblock_detail::SUPERBLOCK_LOCATION) - throw runtime_error("No metadata snapshot found."); - } - - err << examine_superblock(bm_, sb_location, out_); - if (err == FATAL) { - if (check_for_xml(bm_)) + err_ = examine_superblock(bm, sb_location_, out_); + if (err_ == FATAL) { + if (check_for_xml(bm)) out_ << "This looks like XML. thin_check only checks the binary metadata format." << end_message(); - return err; + return; } - superblock_detail::superblock sb = read_superblock(bm_, sb_location); - transaction_manager::ptr tm = open_tm(bm_, sb_location); + transaction_manager::ptr tm = open_tm(bm, sb_location_); + superblock_detail::superblock sb = read_superblock(bm, sb_location_); sb.data_mapping_root_ = mapping_root(sb, options_); print_info(tm, sb, info_out_); @@ -338,24 +388,99 @@ namespace { if (options_.sm_opts_ == check_options::SPACE_MAP_FULL) { space_map::ptr data_sm{open_disk_sm(*tm, &sb.data_space_map_root_)}; optional core_sm{create_core_map(data_sm->get_nr_blocks())}; - err << examine_data_mappings(tm, sb, options_.check_data_mappings_, out_, core_sm); + err_ << examine_data_mappings(tm, sb, options_.data_mapping_opts_, out_, core_sm); + + if (err_ == FATAL) + return; // if we're checking everything, and there were no errors, // then we should check the space maps too. - if (err != FATAL) { - err << examine_metadata_space_map(tm, sb, options_.sm_opts_, out_); + err_ << examine_metadata_space_map(tm, sb, options_.sm_opts_, out_, expected_rc_); - if (core_sm) - err << compare_space_maps(data_sm, *core_sm, out_); - } + // check the data space map + if (core_sm) + err_ << compare_space_maps(data_sm, *core_sm, out_); } else - err << examine_data_mappings(tm, sb, options_.check_data_mappings_, out_, - optional()); + err_ << examine_data_mappings(tm, sb, options_.data_mapping_opts_, out_, + optional()); + } - return err; + bool fix_metadata_leaks() { + if (!verify_preconditions_before_fixing()) { + out_ << "metadata has not been fully examined" << end_message(); + return false; + } + + // skip if the metadata cannot be fixed, or there's no leaked blocks + if (err_ == FATAL) + return false; + else if (err_ == NO_ERROR) + return true; + + block_manager::ptr bm = open_bm(path_, block_manager::READ_WRITE); + superblock_detail::superblock sb = read_superblock(bm, sb_location_); + transaction_manager::ptr tm = open_tm(bm, sb_location_); + persistent_space_map::ptr metadata_sm = + open_metadata_sm(*tm, static_cast(&sb.metadata_space_map_root_)); + tm->set_sm(metadata_sm); + + err_ = clear_leaked_blocks(metadata_sm, expected_rc_); + + if (err_ != NO_ERROR) + return false; + + metadata_sm->commit(); + metadata_sm->copy_root(&sb.metadata_space_map_root_, sizeof(sb.metadata_space_map_root_)); + write_superblock(bm, sb); + + out_ << "fixed metadata leaks" << end_message(); + + return true; + } + + bool clear_needs_check_flag() { + if (!verify_preconditions_before_fixing()) { + out_ << "metadata has not been fully examined" << end_message(); + return false; + } + + if (err_ != NO_ERROR) + return false; + + block_manager::ptr bm = open_bm(path_, block_manager::READ_WRITE); + superblock_detail::superblock sb = read_superblock(bm); + sb.set_needs_check_flag(false); + write_superblock(bm, sb); + + out_ << "cleared needs_check flag" << end_message(); + + return true; + } + + bool get_status() const { + if (options_.ignore_non_fatal_) + return (err_ == FATAL) ? false : true; + + return (err_ == NO_ERROR) ? true : false; } private: + block_address + get_superblock_location() { + block_address sb_location = superblock_detail::SUPERBLOCK_LOCATION; + + if (options_.use_metadata_snap_) { + block_manager::ptr bm = open_bm(path_, block_manager::READ_ONLY, + !options_.use_metadata_snap_); + superblock_detail::superblock sb = read_superblock(bm, sb_location); + sb_location = sb.metadata_snap_; + if (sb_location == superblock_detail::SUPERBLOCK_LOCATION) + throw runtime_error("No metadata snapshot found."); + } + + return sb_location; + } + error_state examine_data_mappings(transaction_manager::ptr tm, superblock_detail::superblock const &sb, @@ -382,12 +507,13 @@ namespace { examine_metadata_space_map(transaction_manager::ptr tm, superblock_detail::superblock const &sb, check_options::space_map_options option, - nested_output &out) { + nested_output &out, + block_counter &bc) { error_state err = NO_ERROR; switch (option) { case check_options::SPACE_MAP_FULL: - err << check_space_map_counts(tm, sb, out); + err << check_metadata_space_map_counts(tm, sb, bc, out); break; default: break; // do nothing @@ -396,10 +522,26 @@ namespace { return err; } - block_manager::ptr bm_; + bool verify_preconditions_before_fixing() const { + if (options_.use_metadata_snap_ || + !!options_.override_mapping_root_ || + options_.sm_opts_ != check_options::SPACE_MAP_FULL || + options_.data_mapping_opts_ != check_options::DATA_MAPPING_LEVEL2) + return false; + + if (!expected_rc_.get_counts().size()) + return false; + + return true; + } + + std::string const &path_; check_options options_; nested_output out_; nested_output info_out_; + block_address sb_location_; + block_counter expected_rc_; + base::error_state err_; // metadata state }; } @@ -407,17 +549,20 @@ namespace { check_options::check_options() : use_metadata_snap_(false), - check_data_mappings_(DATA_MAPPING_LEVEL2), - sm_opts_(SPACE_MAP_FULL) { + data_mapping_opts_(DATA_MAPPING_LEVEL2), + sm_opts_(SPACE_MAP_FULL), + ignore_non_fatal_(false), + fix_metadata_leaks_(false), + clear_needs_check_(false) { } void check_options::set_superblock_only() { - check_data_mappings_ = DATA_MAPPING_NONE; + data_mapping_opts_ = DATA_MAPPING_NONE; sm_opts_ = SPACE_MAP_NONE; } void check_options::set_skip_mappings() { - check_data_mappings_ = DATA_MAPPING_LEVEL1; + data_mapping_opts_ = DATA_MAPPING_LEVEL1; sm_opts_ = SPACE_MAP_NONE; } @@ -433,14 +578,59 @@ void check_options::set_metadata_snap() { void check_options::set_ignore_non_fatal() { ignore_non_fatal_ = true; } - -base::error_state -thin_provisioning::check_metadata(block_manager::ptr bm, - check_options const &check_opts, - output_options output_opts) -{ - metadata_checker checker(bm, check_opts, output_opts); - return checker.check(); + +void check_options::set_fix_metadata_leaks() { + fix_metadata_leaks_ = true; +} + +void check_options::set_clear_needs_check() { + clear_needs_check_ = true; +} + +bool check_options::check_conformance() { + if (fix_metadata_leaks_ || clear_needs_check_) { + if (ignore_non_fatal_) { + cerr << "cannot perform fix by ignoring non-fatal errors" << endl; + return false; + } + + if (use_metadata_snap_) { + cerr << "cannot perform fix within metadata snap" << endl; + return false; + } + + if (!!override_mapping_root_) { + cerr << "cannot perform fix with an overridden mapping root" << endl; + return false; + } + + if (data_mapping_opts_ != DATA_MAPPING_LEVEL2 || + sm_opts_ != SPACE_MAP_FULL) { + cerr << "cannot perform fix without a full examination" << endl; + return false; + } + } + + return true; +} + +//---------------------------------------------------------------- + +bool +thin_provisioning::check_metadata(std::string const &path, + check_options const &check_opts, + output_options output_opts) +{ + metadata_checker checker(path, check_opts, output_opts); + + checker.check(); + if (check_opts.fix_metadata_leaks_) + checker.fix_metadata_leaks(); + if (check_opts.fix_metadata_leaks_ || + check_opts.clear_needs_check_) + checker.clear_needs_check_flag(); + + return checker.get_status(); } //---------------------------------------------------------------- diff --git a/thin-provisioning/metadata_checker.h b/thin-provisioning/metadata_checker.h index 05f36cc..2871932 100644 --- a/thin-provisioning/metadata_checker.h +++ b/thin-provisioning/metadata_checker.h @@ -19,7 +19,6 @@ #ifndef METADATA_CHECKER_H #define METADATA_CHECKER_H -#include "base/error_state.h" #include "block-cache/block_cache.h" #include "persistent-data/block.h" @@ -40,17 +39,22 @@ namespace thin_provisioning { check_options(); + bool check_conformance(); void set_superblock_only(); void set_skip_mappings(); void set_override_mapping_root(bcache::block_address b); void set_metadata_snap(); void set_ignore_non_fatal(); + void set_fix_metadata_leaks(); + void set_clear_needs_check(); bool use_metadata_snap_; - data_mapping_options check_data_mappings_; + data_mapping_options data_mapping_opts_; space_map_options sm_opts_; boost::optional override_mapping_root_; bool ignore_non_fatal_; + bool fix_metadata_leaks_; + bool clear_needs_check_; }; enum output_options { @@ -58,8 +62,8 @@ namespace thin_provisioning { OUTPUT_QUIET, }; - base::error_state - check_metadata(persistent_data::block_manager::ptr bm, + bool + check_metadata(std::string const &path, check_options const &check_opts, output_options output_opts); } diff --git a/thin-provisioning/thin_check.cc b/thin-provisioning/thin_check.cc index 26599c6..3a4ef4b 100644 --- a/thin-provisioning/thin_check.cc +++ b/thin-provisioning/thin_check.cc @@ -43,27 +43,13 @@ using namespace thin_provisioning; namespace { struct flags { flags() - : ignore_non_fatal_errors(false), - quiet(false), - clear_needs_check_flag_on_success(false) { + : quiet(false) { } check_options check_opts; - - bool ignore_non_fatal_errors; - bool quiet; - bool clear_needs_check_flag_on_success; }; - void clear_needs_check(string const &path) { - block_manager::ptr bm = open_bm(path, block_manager::READ_WRITE); - - superblock_detail::superblock sb = read_superblock(bm); - sb.set_needs_check_flag(false); - write_superblock(bm, sb); - } - // Returns 0 on success, 1 on failure (this gets returned directly // by main). int check(string const &path, flags fs) { @@ -76,18 +62,8 @@ namespace { return 1; } - block_manager::ptr bm = open_bm(path, block_manager::READ_ONLY, - !fs.check_opts.use_metadata_snap_); output_options output_opts = !fs.quiet ? OUTPUT_NORMAL : OUTPUT_QUIET; - error_state err = check_metadata(bm, fs.check_opts, output_opts); - - if (fs.ignore_non_fatal_errors) - success = (err == FATAL) ? false : true; - else - success = (err == NO_ERROR) ? true : false; - - if (success && fs.clear_needs_check_flag_on_success) - clear_needs_check(path); + success = check_metadata(path, fs.check_opts, output_opts); } catch (std::exception &e) { if (!fs.quiet) @@ -116,6 +92,7 @@ thin_check_cmd::usage(std::ostream &out) const << " {-h|--help}\n" << " {-V|--version}\n" << " {-m|--metadata-snap}\n" + << " {--fix-metadata-leaks}\n" << " {--override-mapping-root}\n" << " {--clear-needs-check-flag}\n" << " {--ignore-non-fatal-errors}\n" @@ -140,6 +117,7 @@ thin_check_cmd::run(int argc, char **argv) { "ignore-non-fatal-errors", no_argument, NULL, 3}, { "clear-needs-check-flag", no_argument, NULL, 4 }, { "override-mapping-root", required_argument, NULL, 5}, + { "fix-metadata-leaks", no_argument, NULL, 6}, { NULL, no_argument, NULL, 0 } }; @@ -173,13 +151,12 @@ thin_check_cmd::run(int argc, char **argv) case 3: // ignore-non-fatal-errors - fs.ignore_non_fatal_errors = true; fs.check_opts.set_ignore_non_fatal(); break; case 4: // clear needs-check flag - fs.clear_needs_check_flag_on_success = true; + fs.check_opts.set_clear_needs_check(); break; case 5: @@ -187,14 +164,18 @@ thin_check_cmd::run(int argc, char **argv) fs.check_opts.set_override_mapping_root(boost::lexical_cast(optarg)); break; + case 6: + // fix-metadata-leaks + fs.check_opts.set_fix_metadata_leaks(); + break; + default: usage(cerr); return 1; } } - if (fs.clear_needs_check_flag_on_success && fs.check_opts.use_metadata_snap_) { - cerr << "--metadata-snap cannot be combined with --clear-needs-check-flag."; + if (!fs.check_opts.check_conformance()) { usage(cerr); exit(1); } diff --git a/thin-provisioning/thin_generate_damage.cc b/thin-provisioning/thin_generate_damage.cc index 1863d96..9b59793 100644 --- a/thin-provisioning/thin_generate_damage.cc +++ b/thin-provisioning/thin_generate_damage.cc @@ -91,6 +91,15 @@ thin_generate_damage_cmd::thin_generate_damage_cmd() void thin_generate_damage_cmd::usage(std::ostream &out) const { + out << "Usage: " << get_name() << " [options]\n" + << "Options:\n" + << " {-h|--help}\n" + << " {-o|--output} \n" + << " {--create-metadata-leaks}\n" + << " {--nr-blocks} \n" + << " {--expected} \n" + << " {--actual} \n" + << " {-V|--version}" << endl; } int @@ -114,7 +123,7 @@ thin_generate_damage_cmd::run(int argc, char **argv) switch(c) { case 'h': usage(cout); - break; + return 0; case 'o': fs.output = optarg; break;