From fec11289b09f57fe539d02b2d27fcecbbfaf887e Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 2 Jul 2020 16:03:23 +0100 Subject: [PATCH] [thin_check/dump] Under populated nodes are now 'non fatal errors' thin_dump always ignores non-fatal-errors. --- .../data-structures/btree_damage_visitor.h | 22 +++++++--- thin-provisioning/device_tree.cc | 11 +++-- thin-provisioning/device_tree.h | 6 ++- thin-provisioning/mapping_tree.cc | 30 ++++++++------ thin-provisioning/mapping_tree.h | 25 +++++++---- thin-provisioning/metadata_checker.cc | 41 +++++++++++-------- thin-provisioning/metadata_checker.h | 2 + thin-provisioning/metadata_dumper.cc | 16 ++++---- thin-provisioning/thin_check.cc | 1 + 9 files changed, 98 insertions(+), 56 deletions(-) diff --git a/persistent-data/data-structures/btree_damage_visitor.h b/persistent-data/data-structures/btree_damage_visitor.h index b57c553..7eaa8d2 100644 --- a/persistent-data/data-structures/btree_damage_visitor.h +++ b/persistent-data/data-structures/btree_damage_visitor.h @@ -106,10 +106,12 @@ namespace persistent_data { typedef boost::optional maybe_run64; btree_damage_visitor(ValueVisitor &value_visitor, - DamageVisitor &damage_visitor) + DamageVisitor &damage_visitor, + bool ignore_non_fatal) : avoid_repeated_visits_(true), value_visitor_(value_visitor), - damage_visitor_(damage_visitor) { + damage_visitor_(damage_visitor), + ignore_non_fatal_(ignore_non_fatal) { } bool visit_internal(node_location const &loc, @@ -178,7 +180,7 @@ namespace persistent_data { else if (!checker_.check_block_nr(n) || !checker_.check_value_size(n) || !checker_.check_max_entries(n) || - !checker_.check_nr_entries(n, loc.is_sub_root()) || + !check_nr_entries(loc, n) || !checker_.check_ordered_keys(n) || !checker_.check_parent_key(n, loc.is_sub_root() ? boost::optional() : loc.key)) { report_damage(checker_.get_last_error_string()); @@ -202,7 +204,7 @@ namespace persistent_data { else if (!checker_.check_block_nr(n) || !checker_.check_value_size(n) || !checker_.check_max_entries(n) || - !checker_.check_nr_entries(n, loc.is_sub_root()) || + !check_nr_entries(loc, n) || !checker_.check_ordered_keys(n) || !checker_.check_parent_key(n, loc.is_sub_root() ? boost::optional() : loc.key) || !checker_.check_leaf_key(n, last_leaf_key_[loc.level()])) { @@ -300,6 +302,12 @@ namespace persistent_data { maybe_issue_damage(*old_path); } + template + bool check_nr_entries(node_location const &loc, + btree_detail::node_ref const &n) { + return ignore_non_fatal_ || checker_.check_nr_entries(n, loc.is_sub_root()); + } + //-------------------------------- bool avoid_repeated_visits_; @@ -314,15 +322,17 @@ namespace persistent_data { path_tracker path_tracker_; damage_tracker dt_; std::list damage_reasons_; + bool ignore_non_fatal_; }; } template void btree_visit_values(btree const &tree, ValueVisitor &value_visitor, - DamageVisitor &damage_visitor) { + DamageVisitor &damage_visitor, + bool ignore_non_fatal = false) { btree_detail::btree_damage_visitor - v(value_visitor, damage_visitor); + v(value_visitor, damage_visitor, ignore_non_fatal); tree.visit_depth_first(v); } } diff --git a/thin-provisioning/device_tree.cc b/thin-provisioning/device_tree.cc index 4b4e0dd..f0d92a7 100644 --- a/thin-provisioning/device_tree.cc +++ b/thin-provisioning/device_tree.cc @@ -96,18 +96,21 @@ namespace thin_provisioning { void thin_provisioning::walk_device_tree(device_tree const &tree, device_tree_detail::device_visitor &vv, - device_tree_detail::damage_visitor &dv) + device_tree_detail::damage_visitor &dv, + bool ignore_non_fatal) { visitor_adapter av(vv); ll_damage_visitor ll_dv(dv); - btree_visit_values(tree, av, ll_dv); + btree_visit_values(tree, av, ll_dv, ignore_non_fatal); } void -thin_provisioning::check_device_tree(device_tree const &tree, damage_visitor &visitor) +thin_provisioning::check_device_tree(device_tree const &tree, + damage_visitor &visitor, + bool ignore_non_fatal) { noop_visitor vv; - walk_device_tree(tree, vv, visitor); + walk_device_tree(tree, vv, visitor, ignore_non_fatal); } //---------------------------------------------------------------- diff --git a/thin-provisioning/device_tree.h b/thin-provisioning/device_tree.h index d7178cd..5bfefda 100644 --- a/thin-provisioning/device_tree.h +++ b/thin-provisioning/device_tree.h @@ -75,9 +75,11 @@ namespace thin_provisioning { void walk_device_tree(device_tree const &tree, device_tree_detail::device_visitor &dev_v, - device_tree_detail::damage_visitor &dv); + device_tree_detail::damage_visitor &dv, + bool ignore_non_fatal = false); void check_device_tree(device_tree const &tree, - device_tree_detail::damage_visitor &visitor); + device_tree_detail::damage_visitor &visitor, + bool ignore_non_fatal = false); } //---------------------------------------------------------------- diff --git a/thin-provisioning/mapping_tree.cc b/thin-provisioning/mapping_tree.cc index f23ec70..a73dc3a 100644 --- a/thin-provisioning/mapping_tree.cc +++ b/thin-provisioning/mapping_tree.cc @@ -228,54 +228,60 @@ namespace { void thin_provisioning::walk_mapping_tree(dev_tree const &tree, mapping_tree_detail::device_visitor &dev_v, - mapping_tree_detail::damage_visitor &dv) + mapping_tree_detail::damage_visitor &dv, + bool ignore_non_fatal) { dev_tree_damage_visitor ll_dv(dv); - btree_visit_values(tree, dev_v, ll_dv); + btree_visit_values(tree, dev_v, ll_dv, ignore_non_fatal); } void thin_provisioning::check_mapping_tree(dev_tree const &tree, - mapping_tree_detail::damage_visitor &visitor) + mapping_tree_detail::damage_visitor &visitor, + bool ignore_non_fatal) { noop_block_visitor dev_v; - walk_mapping_tree(tree, dev_v, visitor); + walk_mapping_tree(tree, dev_v, visitor, ignore_non_fatal); } void thin_provisioning::walk_mapping_tree(mapping_tree const &tree, mapping_tree_detail::mapping_visitor &mv, - mapping_tree_detail::damage_visitor &dv) + mapping_tree_detail::damage_visitor &dv, + bool ignore_non_fatal) { mapping_tree_damage_visitor ll_dv(dv); - btree_visit_values(tree, mv, ll_dv); + btree_visit_values(tree, mv, ll_dv, ignore_non_fatal); } void thin_provisioning::check_mapping_tree(mapping_tree const &tree, - mapping_tree_detail::damage_visitor &visitor) + mapping_tree_detail::damage_visitor &visitor, + bool ignore_non_fatal) { noop_block_time_visitor mv; - walk_mapping_tree(tree, mv, visitor); + walk_mapping_tree(tree, mv, visitor, ignore_non_fatal); } void 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) + mapping_tree_detail::damage_visitor &dv, + bool ignore_non_fatal) { single_mapping_tree_damage_visitor ll_dv(dv, dev_id); - btree_visit_values(tree, mv, ll_dv); + btree_visit_values(tree, mv, ll_dv, ignore_non_fatal); } void thin_provisioning::check_mapping_tree(single_mapping_tree const &tree, uint64_t dev_id, - mapping_tree_detail::damage_visitor &visitor) + mapping_tree_detail::damage_visitor &visitor, + bool ignore_non_fatal) { noop_block_time_visitor mv; - walk_mapping_tree(tree, dev_id, mv, visitor); + 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 22ad514..7545b74 100644 --- a/thin-provisioning/mapping_tree.h +++ b/thin-provisioning/mapping_tree.h @@ -129,23 +129,32 @@ namespace thin_provisioning { void walk_mapping_tree(dev_tree const &tree, mapping_tree_detail::device_visitor &dev_v, - mapping_tree_detail::damage_visitor &dv); - void check_mapping_tree(dev_tree const &tree, - mapping_tree_detail::damage_visitor &visitor); + mapping_tree_detail::damage_visitor &dv, + 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); - void check_mapping_tree(mapping_tree const &tree, - mapping_tree_detail::damage_visitor &visitor); + mapping_tree_detail::damage_visitor &dv, + 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); + mapping_tree_detail::damage_visitor &dv, + bool ignore_non_fatal = false); + void check_mapping_tree(single_mapping_tree const &tree, uint64_t dev_id, - mapping_tree_detail::damage_visitor &visitor); + mapping_tree_detail::damage_visitor &visitor, + bool ignore_non_fatal = false); + + void check_mapping_tree(dev_tree const &tree, + mapping_tree_detail::damage_visitor &visitor, + bool ignore_non_fatal = false); + + void check_mapping_tree(mapping_tree const &tree, + mapping_tree_detail::damage_visitor &visitor, + bool ignore_non_fatal = false); } //---------------------------------------------------------------- diff --git a/thin-provisioning/metadata_checker.cc b/thin-provisioning/metadata_checker.cc index ed3d20d..7a619c9 100644 --- a/thin-provisioning/metadata_checker.cc +++ b/thin-provisioning/metadata_checker.cc @@ -150,28 +150,30 @@ namespace { error_state examine_devices_tree_(transaction_manager::ptr tm, superblock_detail::superblock const &sb, - nested_output &out) { + nested_output &out, + bool ignore_non_fatal) { out << "examining devices tree" << end_message(); nested_output::nest _ = out.push(); devices_reporter dev_rep(out); device_tree dtree(*tm, sb.device_details_root_, device_tree_detail::device_details_traits::ref_counter()); - check_device_tree(dtree, dev_rep); + check_device_tree(dtree, dev_rep, ignore_non_fatal); return dev_rep.get_error(); } error_state examine_top_level_mapping_tree_(transaction_manager::ptr tm, superblock_detail::superblock const &sb, - nested_output &out) { + nested_output &out, + bool ignore_non_fatal) { out << "examining top level of mapping tree" << end_message(); nested_output::nest _ = out.push(); mapping_reporter mapping_rep(out); dev_tree dtree(*tm, sb.data_mapping_root_, mapping_tree_detail::mtree_traits::ref_counter(*tm)); - check_mapping_tree(dtree, mapping_rep); + check_mapping_tree(dtree, mapping_rep, ignore_non_fatal); return mapping_rep.get_error(); } @@ -179,7 +181,8 @@ namespace { error_state examine_mapping_tree_(transaction_manager::ptr tm, superblock_detail::superblock const &sb, nested_output &out, - optional data_sm) { + optional data_sm, + bool ignore_non_fatal) { out << "examining mapping tree" << end_message(); nested_output::nest _ = out.push(); @@ -189,18 +192,19 @@ namespace { if (data_sm) { data_ref_counter dcounter(*data_sm); - walk_mapping_tree(mtree, dcounter, mapping_rep); + walk_mapping_tree(mtree, dcounter, mapping_rep, ignore_non_fatal); } else - check_mapping_tree(mtree, mapping_rep); + check_mapping_tree(mtree, mapping_rep, ignore_non_fatal); return mapping_rep.get_error(); } error_state examine_top_level_mapping_tree(transaction_manager::ptr tm, superblock_detail::superblock const &sb, - nested_output &out) { - error_state err = examine_devices_tree_(tm, sb, out); - err << examine_top_level_mapping_tree_(tm, sb, out); + nested_output &out, + 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); return err; } @@ -208,9 +212,10 @@ namespace { error_state examine_mapping_tree(transaction_manager::ptr tm, superblock_detail::superblock const &sb, nested_output &out, - optional data_sm) { - error_state err = examine_devices_tree_(tm, sb, out); - err << examine_mapping_tree_(tm, sb, out, data_sm); + 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; } @@ -351,7 +356,7 @@ namespace { } private: - static error_state + error_state examine_data_mappings(transaction_manager::ptr tm, superblock_detail::superblock const &sb, check_options::data_mapping_options option, @@ -361,10 +366,10 @@ namespace { switch (option) { case check_options::DATA_MAPPING_LEVEL1: - err << examine_top_level_mapping_tree(tm, sb, out); + err << examine_top_level_mapping_tree(tm, sb, out, options_.ignore_non_fatal_); break; case check_options::DATA_MAPPING_LEVEL2: - err << examine_mapping_tree(tm, sb, out, data_sm); + err << examine_mapping_tree(tm, sb, out, data_sm, options_.ignore_non_fatal_); break; default: break; // do nothing @@ -424,6 +429,10 @@ void check_options::set_metadata_snap() { use_metadata_snap_ = true; sm_opts_ = SPACE_MAP_NONE; } + +void check_options::set_ignore_non_fatal() { + ignore_non_fatal_ = true; +} base::error_state thin_provisioning::check_metadata(block_manager::ptr bm, diff --git a/thin-provisioning/metadata_checker.h b/thin-provisioning/metadata_checker.h index 1b94d7e..05f36cc 100644 --- a/thin-provisioning/metadata_checker.h +++ b/thin-provisioning/metadata_checker.h @@ -44,11 +44,13 @@ namespace thin_provisioning { void set_skip_mappings(); void set_override_mapping_root(bcache::block_address b); void set_metadata_snap(); + void set_ignore_non_fatal(); bool use_metadata_snap_; data_mapping_options check_data_mappings_; space_map_options sm_opts_; boost::optional override_mapping_root_; + bool ignore_non_fatal_; }; enum output_options { diff --git a/thin-provisioning/metadata_dumper.cc b/thin-provisioning/metadata_dumper.cc index 2ceffcc..4feb40f 100644 --- a/thin-provisioning/metadata_dumper.cc +++ b/thin-provisioning/metadata_dumper.cc @@ -132,7 +132,7 @@ namespace { auto tree = device_tree(tm, root, device_tree_detail::device_details_traits::ref_counter()); try { - walk_device_tree(tree, de, dv); + walk_device_tree(tree, de, dv, true); } catch (...) { return optional>(); } @@ -157,7 +157,7 @@ namespace { auto tree = dev_tree(tm, root, mapping_tree_detail::mtree_traits::ref_counter(tm)); try { - walk_mapping_tree(tree, me, mv); + walk_mapping_tree(tree, me, mv, true); } catch (...) { return optional>(); } @@ -754,7 +754,7 @@ namespace { // Since we're not mutating the btrees we don't need a real space map noop_map::ptr sm(new noop_map); single_mapping_tree tree(tm_, subtree_root, mapping_tree_detail::block_time_ref_counter(sm)); - walk_mapping_tree(tree, dev_id, static_cast(me), *damage_policy_); + walk_mapping_tree(tree, dev_id, static_cast(me), *damage_policy_, true); } dump_options const &opts_; @@ -785,7 +785,7 @@ namespace { dump_options opts; details_extractor de(opts); device_tree_detail::damage_visitor::ptr dd_policy(details_damage_policy(true)); - walk_device_tree(*md.details_, de, *dd_policy); + walk_device_tree(*md.details_, de, *dd_policy, true); e->begin_superblock("", sb.time_, sb.trans_id_, @@ -798,7 +798,7 @@ namespace { { mapping_tree_detail::damage_visitor::ptr md_policy(mapping_damage_policy(true)); mapping_tree_emit_visitor mte(opts, *md.tm_, e, de.get_details(), mapping_damage_policy(true)); - walk_mapping_tree(*md.mappings_top_level_, mte, *md_policy); + walk_mapping_tree(*md.mappings_top_level_, mte, *md_policy, true); } e->end_superblock(); @@ -882,7 +882,7 @@ thin_provisioning::metadata_dump(metadata::ptr md, emitter::ptr e, dump_options { details_extractor de(opts); device_tree_detail::damage_visitor::ptr dd_policy(details_damage_policy(false)); - walk_device_tree(*md->details_, de, *dd_policy); + walk_device_tree(*md->details_, de, *dd_policy, true); e->begin_superblock("", md->sb_.time_, md->sb_.trans_id_, @@ -895,7 +895,7 @@ thin_provisioning::metadata_dump(metadata::ptr md, emitter::ptr e, dump_options { mapping_tree_detail::damage_visitor::ptr md_policy(mapping_damage_policy(false)); mapping_tree_emit_visitor mte(opts, *md->tm_, e, de.get_details(), mapping_damage_policy(false)); - walk_mapping_tree(*md->mappings_top_level_, mte, *md_policy); + walk_mapping_tree(*md->mappings_top_level_, mte, *md_policy, true); } e->end_superblock(); @@ -924,7 +924,7 @@ thin_provisioning::metadata_dump_subtree(metadata::ptr md, emitter::ptr e, bool mapping_tree_detail::block_time_ref_counter(md->data_sm_)); // FIXME: pass the current device id instead of zero walk_mapping_tree(tree, 0, static_cast(me), - *mapping_damage_policy(repair)); + *mapping_damage_policy(repair), true); } //---------------------------------------------------------------- diff --git a/thin-provisioning/thin_check.cc b/thin-provisioning/thin_check.cc index 05f5582..26599c6 100644 --- a/thin-provisioning/thin_check.cc +++ b/thin-provisioning/thin_check.cc @@ -174,6 +174,7 @@ 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: