From e8f1bda1a5a4fffd6bc6180c1959e622613177f6 Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Sun, 19 Jun 2016 21:35:11 +0800 Subject: [PATCH 1/5] [thin] store the device id in single_mapping_tree_damage_visitor for error reporting 1. fix the damage type for single_mapping_tree_damage_visitor 2. walk_mapping_tree() now requires the device id 3. update metadata_dumper and thin_ls for the new walk_mapping_tree() --- thin-provisioning/mapping_tree.cc | 15 ++++++++++----- thin-provisioning/mapping_tree.h | 2 ++ thin-provisioning/metadata_dumper.cc | 9 +++++---- thin-provisioning/thin_ls.cc | 4 ++-- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/thin-provisioning/mapping_tree.cc b/thin-provisioning/mapping_tree.cc index 454e85c..c1a9358 100644 --- a/thin-provisioning/mapping_tree.cc +++ b/thin-provisioning/mapping_tree.cc @@ -193,14 +193,16 @@ namespace { class single_mapping_tree_damage_visitor { public: - single_mapping_tree_damage_visitor(damage_visitor &v) - : v_(v) { + single_mapping_tree_damage_visitor(damage_visitor &v, + uint64_t dev_id) + : v_(v), + dev_id_(dev_id) { } virtual void visit(btree_path const &path, btree_detail::damage const &d) { switch (path.size()) { case 0: - v_.visit(missing_devices(d.desc_, d.lost_keys_)); + v_.visit(missing_mappings(d.desc_, dev_id_, d.lost_keys_)); break; default: @@ -210,6 +212,7 @@ namespace { private: damage_visitor &v_; + uint64_t dev_id_; }; } @@ -250,19 +253,21 @@ thin_provisioning::check_mapping_tree(mapping_tree const &tree, 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) { - single_mapping_tree_damage_visitor ll_dv(dv); + single_mapping_tree_damage_visitor ll_dv(dv, dev_id); btree_visit_values(tree, mv, ll_dv); } void thin_provisioning::check_mapping_tree(single_mapping_tree const &tree, + uint64_t dev_id, mapping_tree_detail::damage_visitor &visitor) { noop_block_time_visitor mv; - walk_mapping_tree(tree, mv, visitor); + walk_mapping_tree(tree, dev_id, mv, visitor); } //---------------------------------------------------------------- diff --git a/thin-provisioning/mapping_tree.h b/thin-provisioning/mapping_tree.h index d417b47..4e6adda 100644 --- a/thin-provisioning/mapping_tree.h +++ b/thin-provisioning/mapping_tree.h @@ -139,9 +139,11 @@ namespace thin_provisioning { mapping_tree_detail::damage_visitor &visitor); 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); void check_mapping_tree(single_mapping_tree const &tree, + uint64_t dev_id, mapping_tree_detail::damage_visitor &visitor); } diff --git a/thin-provisioning/metadata_dumper.cc b/thin-provisioning/metadata_dumper.cc index ba29bfa..bbb5be8 100644 --- a/thin-provisioning/metadata_dumper.cc +++ b/thin-provisioning/metadata_dumper.cc @@ -196,7 +196,7 @@ namespace { try { if (!opts_.skip_mappings_) - emit_mappings(tree_root); + emit_mappings(dev_id, tree_root); } catch (exception &e) { cerr << e.what(); e_->end_device(); @@ -213,11 +213,11 @@ namespace { } private: - void emit_mappings(block_address subtree_root) { + void emit_mappings(uint64_t dev_id, block_address subtree_root) { mapping_emitter me(e_); single_mapping_tree tree(*md_->tm_, subtree_root, mapping_tree_detail::block_time_ref_counter(md_->data_sm_)); - walk_mapping_tree(tree, static_cast(me), *damage_policy_); + walk_mapping_tree(tree, dev_id, static_cast(me), *damage_policy_); } dump_options const &opts_; @@ -274,7 +274,8 @@ thin_provisioning::metadata_dump_subtree(metadata::ptr md, emitter::ptr e, bool mapping_emitter me(e); single_mapping_tree tree(*md->tm_, subtree_root, mapping_tree_detail::block_time_ref_counter(md->data_sm_)); - walk_mapping_tree(tree, static_cast(me), + // FIXME: pass the current device id instead of zero + walk_mapping_tree(tree, 0, static_cast(me), *mapping_damage_policy(repair)); } diff --git a/thin-provisioning/thin_ls.cc b/thin-provisioning/thin_ls.cc index 4e0b97d..03b8d63 100644 --- a/thin-provisioning/thin_ls.cc +++ b/thin-provisioning/thin_ls.cc @@ -248,7 +248,7 @@ namespace { mapping_pass1 pass1(mappings); fatal_mapping_damage dv; - walk_mapping_tree(dev_mappings, pass1, dv); + walk_mapping_tree(dev_mappings, dev_id, pass1, dv); } @@ -264,7 +264,7 @@ namespace { mapping_pass2 pass2(mappings); fatal_mapping_damage dv; - walk_mapping_tree(dev_mappings, pass2, dv); + walk_mapping_tree(dev_mappings, dev_id, pass2, dv); return pass2.get_exclusives(); } From 60eb60882539e5ba80f69823cca7261d5f8935eb Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Sun, 19 Jun 2016 21:35:58 +0800 Subject: [PATCH 2/5] [counting_visitor] fix the path for ValueCounter --- persistent-data/data-structures/btree_counter.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/persistent-data/data-structures/btree_counter.h b/persistent-data/data-structures/btree_counter.h index ec74e86..6ccf03a 100644 --- a/persistent-data/data-structures/btree_counter.h +++ b/persistent-data/data-structures/btree_counter.h @@ -37,7 +37,7 @@ namespace persistent_data { for (unsigned i = 0; i < nr; i++) { // FIXME: confirm l2 is correct node_location l2(l); - l2.push_key(i); + l2.push_key(n.key_at(i)); vc_.visit(l2, n.value_at(i)); } From 9e7af6b677853e0a5e56e05ffdc4e7c30bef8ff2 Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Sun, 19 Jun 2016 21:37:00 +0800 Subject: [PATCH 3/5] [metadata_counter] remove explicit try/catch when counting data space map Unlike metadata_index_store, the constructor of btree_index_store doesn't throw exceptions. --- thin-provisioning/metadata_counter.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/thin-provisioning/metadata_counter.cc b/thin-provisioning/metadata_counter.cc index e36fde6..f6055f0 100644 --- a/thin-provisioning/metadata_counter.cc +++ b/thin-provisioning/metadata_counter.cc @@ -41,12 +41,10 @@ void thin_provisioning::count_space_maps(transaction_manager::ptr tm, } // Count the data space map (no-throw) - try { + { persistent_space_map::ptr data_sm = open_disk_sm(*tm, static_cast(&sb.data_space_map_root_)); data_sm->count_metadata(bc); - } catch (std::exception &e) { - cerr << e.what() << endl; } } From 3439dbfdfc6b54eb2b064708f951f62e118e2810 Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Sun, 19 Jun 2016 22:05:58 +0800 Subject: [PATCH 4/5] [metadata_counter] hide count_trees() and count_space_maps() --- thin-provisioning/metadata_counter.cc | 72 ++++++++++++++------------- thin-provisioning/metadata_counter.h | 6 --- 2 files changed, 38 insertions(+), 40 deletions(-) diff --git a/thin-provisioning/metadata_counter.cc b/thin-provisioning/metadata_counter.cc index f6055f0..a928056 100644 --- a/thin-provisioning/metadata_counter.cc +++ b/thin-provisioning/metadata_counter.cc @@ -7,46 +7,50 @@ using namespace thin_provisioning; //---------------------------------------------------------------- -void thin_provisioning::count_trees(transaction_manager::ptr tm, - superblock_detail::superblock &sb, - block_counter &bc) { +namespace { + void count_trees(transaction_manager::ptr tm, + superblock_detail::superblock &sb, + block_counter &bc) { - // Count the device tree - { - noop_value_counter vc; - device_tree dtree(*tm, sb.device_details_root_, - device_tree_detail::device_details_traits::ref_counter()); - count_btree_blocks(dtree, bc, vc); + // Count the device tree + { + noop_value_counter vc; + device_tree dtree(*tm, sb.device_details_root_, + device_tree_detail::device_details_traits::ref_counter()); + count_btree_blocks(dtree, bc, vc); + } + + // Count the mapping tree + { + noop_value_counter vc; + mapping_tree mtree(*tm, sb.data_mapping_root_, + mapping_tree_detail::block_traits::ref_counter(space_map::ptr())); + count_btree_blocks(mtree, bc, vc); + } } - // Count the mapping tree - { - noop_value_counter vc; - mapping_tree mtree(*tm, sb.data_mapping_root_, - mapping_tree_detail::block_traits::ref_counter(space_map::ptr())); - count_btree_blocks(mtree, bc, vc); + void count_space_maps(transaction_manager::ptr tm, + superblock_detail::superblock &sb, + block_counter &bc) { + // Count the metadata space map (no-throw) + try { + persistent_space_map::ptr metadata_sm = + open_metadata_sm(*tm, static_cast(&sb.metadata_space_map_root_)); + metadata_sm->count_metadata(bc); + } catch (std::exception &e) { + cerr << e.what() << endl; + } + + // Count the data space map (no-throw) + { + persistent_space_map::ptr data_sm = + open_disk_sm(*tm, static_cast(&sb.data_space_map_root_)); + data_sm->count_metadata(bc); + } } } -void thin_provisioning::count_space_maps(transaction_manager::ptr tm, - superblock_detail::superblock &sb, - block_counter &bc) { - // Count the metadata space map (no-throw) - try { - persistent_space_map::ptr metadata_sm = - open_metadata_sm(*tm, static_cast(&sb.metadata_space_map_root_)); - metadata_sm->count_metadata(bc); - } catch (std::exception &e) { - cerr << e.what() << endl; - } - - // Count the data space map (no-throw) - { - persistent_space_map::ptr data_sm = - open_disk_sm(*tm, static_cast(&sb.data_space_map_root_)); - data_sm->count_metadata(bc); - } -} +//---------------------------------------------------------------- void thin_provisioning::count_metadata(transaction_manager::ptr tm, superblock_detail::superblock &sb, diff --git a/thin-provisioning/metadata_counter.h b/thin-provisioning/metadata_counter.h index 861d72f..67801e8 100644 --- a/thin-provisioning/metadata_counter.h +++ b/thin-provisioning/metadata_counter.h @@ -7,12 +7,6 @@ //---------------------------------------------------------------- namespace thin_provisioning { - void count_trees(transaction_manager::ptr tm, - superblock_detail::superblock &sb, - block_counter &bc); - void count_space_maps(transaction_manager::ptr tm, - superblock_detail::superblock &sb, - block_counter &bc); void count_metadata(transaction_manager::ptr tm, superblock_detail::superblock &sb, block_counter &bc, From 7eac48793caf1146bd2ff9937a99e9fb7c031e18 Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Mon, 20 Jun 2016 00:40:10 +0800 Subject: [PATCH 5/5] [space map disk] tidy up: add const qualifier --- persistent-data/space-maps/disk.cc | 4 ++-- persistent-data/space-maps/disk.h | 4 ++-- thin-provisioning/metadata_counter.cc | 10 +++++----- thin-provisioning/metadata_counter.h | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/persistent-data/space-maps/disk.cc b/persistent-data/space-maps/disk.cc index 9c51921..caeaca3 100644 --- a/persistent-data/space-maps/disk.cc +++ b/persistent-data/space-maps/disk.cc @@ -742,7 +742,7 @@ persistent_data::create_disk_sm(transaction_manager &tm, } checked_space_map::ptr -persistent_data::open_disk_sm(transaction_manager &tm, void *root) +persistent_data::open_disk_sm(transaction_manager &tm, void const *root) { sm_root_disk d; sm_root v; @@ -770,7 +770,7 @@ persistent_data::create_metadata_sm(transaction_manager &tm, block_address nr_bl } checked_space_map::ptr -persistent_data::open_metadata_sm(transaction_manager &tm, void *root) +persistent_data::open_metadata_sm(transaction_manager &tm, void const *root) { sm_root_disk d; sm_root v; diff --git a/persistent-data/space-maps/disk.h b/persistent-data/space-maps/disk.h index 9edef6b..906221e 100644 --- a/persistent-data/space-maps/disk.h +++ b/persistent-data/space-maps/disk.h @@ -29,13 +29,13 @@ namespace persistent_data { create_disk_sm(transaction_manager &tm, block_address nr_blocks); checked_space_map::ptr - open_disk_sm(transaction_manager &tm, void *root); + open_disk_sm(transaction_manager &tm, void const *root); checked_space_map::ptr create_metadata_sm(transaction_manager &tm, block_address nr_blocks); checked_space_map::ptr - open_metadata_sm(transaction_manager &tm, void *root); + open_metadata_sm(transaction_manager &tm, void const *root); bcache::validator::ptr bitmap_validator(); diff --git a/thin-provisioning/metadata_counter.cc b/thin-provisioning/metadata_counter.cc index a928056..95487d2 100644 --- a/thin-provisioning/metadata_counter.cc +++ b/thin-provisioning/metadata_counter.cc @@ -9,7 +9,7 @@ using namespace thin_provisioning; namespace { void count_trees(transaction_manager::ptr tm, - superblock_detail::superblock &sb, + superblock_detail::superblock const &sb, block_counter &bc) { // Count the device tree @@ -30,12 +30,12 @@ namespace { } void count_space_maps(transaction_manager::ptr tm, - superblock_detail::superblock &sb, + superblock_detail::superblock const &sb, block_counter &bc) { // Count the metadata space map (no-throw) try { persistent_space_map::ptr metadata_sm = - open_metadata_sm(*tm, static_cast(&sb.metadata_space_map_root_)); + open_metadata_sm(*tm, static_cast(&sb.metadata_space_map_root_)); metadata_sm->count_metadata(bc); } catch (std::exception &e) { cerr << e.what() << endl; @@ -44,7 +44,7 @@ namespace { // Count the data space map (no-throw) { persistent_space_map::ptr data_sm = - open_disk_sm(*tm, static_cast(&sb.data_space_map_root_)); + open_disk_sm(*tm, static_cast(&sb.data_space_map_root_)); data_sm->count_metadata(bc); } } @@ -53,7 +53,7 @@ namespace { //---------------------------------------------------------------- void thin_provisioning::count_metadata(transaction_manager::ptr tm, - superblock_detail::superblock &sb, + superblock_detail::superblock const &sb, block_counter &bc, bool skip_metadata_snap) { // Count the superblock diff --git a/thin-provisioning/metadata_counter.h b/thin-provisioning/metadata_counter.h index 67801e8..bc65ab9 100644 --- a/thin-provisioning/metadata_counter.h +++ b/thin-provisioning/metadata_counter.h @@ -8,7 +8,7 @@ namespace thin_provisioning { void count_metadata(transaction_manager::ptr tm, - superblock_detail::superblock &sb, + superblock_detail::superblock const &sb, block_counter &bc, bool skip_metadata_snap = false); }