Some btree visitor tidying.

Introduce node_location to replace the long list of parameters.  Also
add a depth field to keep track of the depth from root.

persistent-data/data-structures/btree.h

@@ -263,6 +263,14 @@ namespace persistent_data {
 			std::list<block_manager<>::write_ref> spine_;
 			block_address root_;
 		};
+
+		// Used when visiting the nodes that make up a btree.
+		struct node_location {
+			unsigned level;
+			unsigned depth;
+			bool sub_root;
+			boost::optional<uint64_t> key;
+		};
 	}
 
 	template <unsigned Levels, typename ValueTraits>
@@ -308,23 +316,25 @@ namespace persistent_data {
 		// inspect the individual nodes that make up a btree.
 		class visitor {
 		public:
-			virtual ~visitor() {}
 			typedef boost::shared_ptr<visitor> ptr;
+			typedef btree_detail::node_location node_location;
+
+			virtual ~visitor() {}
 
 			// The bool return values indicate whether the walk
 			// should be continued into sub trees of the node (true == continue).
-			virtual bool visit_internal(unsigned level, bool sub_root, boost::optional<uint64_t> key,
+			virtual bool visit_internal(node_location const &l,
 						    internal_node const &n) = 0;
-			virtual bool visit_internal_leaf(unsigned level, bool sub_root, boost::optional<uint64_t> key,
+			virtual bool visit_internal_leaf(node_location const &l,
 							 internal_node const &n) = 0;
-			virtual bool visit_leaf(unsigned level, bool sub_root, boost::optional<uint64_t> key,
+			virtual bool visit_leaf(node_location const &l,
 						leaf_node const &n) = 0;
 
 			virtual void visit_complete() {}
 		};
 
 		// Walks the tree in depth first order
-		void visit(typename visitor::ptr visitor) const;
+		void visit_depth_first(typename visitor::ptr visitor) const;
 
 	private:
 		template <typename ValueTraits2, typename Search>
@@ -353,7 +363,7 @@ namespace persistent_data {
 				int *index);
 
 		void walk_tree(typename visitor::ptr visitor,
-			       unsigned level, bool root, boost::optional<uint64_t> key,
+			       btree_detail::node_location const &loc,
 			       block_address b) const;
 
 		typename persistent_data::transaction_manager::ptr tm_;

persistent-data/data-structures/btree.tcc

@@ -742,37 +742,57 @@ insert_location(btree_detail::shadow_spine &spine,
 
 template <unsigned Levels, typename ValueTraits>
 void
-btree<Levels, ValueTraits>::visit(typename visitor::ptr visitor) const
+btree<Levels, ValueTraits>::visit_depth_first(typename visitor::ptr visitor) const
 {
-	walk_tree(visitor, 0, true, boost::optional<uint64_t>(), root_);
+	node_location loc;
+
+	loc.level = 0;
+	loc.depth = 0;
+	loc.sub_root = true;
+	loc.key = boost::optional<uint64_t>();
+
+	walk_tree(visitor, loc, root_);
 	visitor->visit_complete();
 }
 
 template <unsigned Levels, typename ValueTraits>
 void
-btree<Levels, ValueTraits>::
+btree<Levels, ValueTraits>::walk_tree(typename visitor::ptr visitor,
-walk_tree(typename visitor::ptr visitor,
+				      node_location const &loc,
-	  unsigned level, bool sub_root,
+				      block_address b) const
-	  boost::optional<uint64_t> key,
-	  block_address b) const
 {
 	using namespace btree_detail;
 
 	read_ref blk = tm_->read_lock(b);
 	internal_node o = to_node<uint64_traits>(blk);
 	if (o.get_type() == INTERNAL) {
-		if (visitor->visit_internal(level, sub_root, key, o))
+		if (visitor->visit_internal(loc, o))
-			for (unsigned i = 0; i < o.get_nr_entries(); i++)
+			for (unsigned i = 0; i < o.get_nr_entries(); i++) {
-				walk_tree(visitor, level, false, o.key_at(i), o.value_at(i));
+				node_location loc2(loc);
 
-	} else if (level < Levels - 1) {
+				loc2.depth = loc.depth + 1;
-		if (visitor->visit_internal_leaf(level, sub_root, key, o))
+				loc2.sub_root = false;
-			for (unsigned i = 0; i < o.get_nr_entries(); i++)
+				loc2.key = boost::optional<uint64_t>(o.key_at(i));
-				walk_tree(visitor, level + 1, true, boost::optional<uint64_t>(o.key_at(i)), o.value_at(i));
+
+				walk_tree(visitor, loc2, o.value_at(i));
+			}
+
+	} else if (loc.level < Levels - 1) {
+		if (visitor->visit_internal_leaf(loc, o))
+			for (unsigned i = 0; i < o.get_nr_entries(); i++) {
+				node_location loc2(loc);
+
+				loc2.level = loc.level + 1;
+				loc2.depth = loc.depth + 1;
+				loc2.sub_root = true;
+				loc2.key = boost::optional<uint64_t>(o.key_at(i));
+
+				walk_tree(visitor, loc, o.value_at(i));
+			}
 
 	} else {
 		leaf_node ov = to_node<ValueTraits>(blk);
-		visitor->visit_leaf(level, sub_root, key, ov);
+		visitor->visit_leaf(loc, ov);
 	}
 }
 

persistent-data/data-structures/btree_checker.h

@@ -57,31 +57,27 @@ namespace persistent_data {
 	template <uint32_t Levels, typename ValueTraits>
 	class btree_checker : public btree<Levels, ValueTraits>::visitor {
 	public:
+		typedef btree_detail::node_location node_location;
+
 		btree_checker(block_counter &counter, bool avoid_repeated_visits = true)
 			: counter_(counter),
 			  errs_(new error_set("btree errors")),
 			  avoid_repeated_visits_(avoid_repeated_visits) {
 		}
 
-		bool visit_internal(unsigned level,
+		bool visit_internal(node_location const &loc,
-				    bool sub_root,
-				    optional<uint64_t> key,
 				    btree_detail::node_ref<uint64_traits> const &n) {
-			return check_internal(level, sub_root, key, n);
+			return check_internal(loc, n);
 		}
 
-		bool visit_internal_leaf(unsigned level,
+		bool visit_internal_leaf(node_location const &loc,
-					 bool sub_root,
-					 optional<uint64_t> key,
 					 btree_detail::node_ref<uint64_traits> const &n) {
-			return check_leaf(level, sub_root, key, n);
+			return check_leaf(loc, n);
 		}
 
-		bool visit_leaf(unsigned level,
+		bool visit_leaf(node_location const &loc,
-				bool sub_root,
-				optional<uint64_t> key,
 				btree_detail::node_ref<ValueTraits> const &n) {
-			return check_leaf(level, sub_root, key, n);
+			return check_leaf(loc, n);
 		}
 
 		error_set::ptr get_errors() const {
@@ -94,19 +90,17 @@ namespace persistent_data {
 		}
 
 	private:
-		bool check_internal(unsigned level,
+		bool check_internal(node_location const &loc,
-				    bool sub_root,
-				    optional<uint64_t> key,
 				    btree_detail::node_ref<uint64_traits> const &n) {
 			if (!already_visited(n) &&
 			    check_sum(n) &&
 			    check_block_nr(n) &&
 			    check_max_entries(n) &&
-			    check_nr_entries(n, sub_root) &&
+			    check_nr_entries(n, loc.sub_root) &&
 			    check_ordered_keys(n) &&
-			    check_parent_key(sub_root ? optional<uint64_t>() : key, n)) {
+			    check_parent_key(loc.sub_root ? optional<uint64_t>() : loc.key, n)) {
-				if (sub_root)
+				if (loc.sub_root)
-					new_root(level);
+					new_root(loc.level);
 
 				return true;
 			}
@@ -115,21 +109,19 @@ namespace persistent_data {
 		}
 
 		template <typename ValueTraits2>
-		bool check_leaf(unsigned level,
+		bool check_leaf(node_location const &loc,
-			      bool sub_root,
+				btree_detail::node_ref<ValueTraits2> const &n) {
-			      optional<uint64_t> key,
-			      btree_detail::node_ref<ValueTraits2> const &n) {
 			if (!already_visited(n) &&
 			    check_sum(n) &&
 			    check_block_nr(n) &&
 			    check_max_entries(n) &&
-			    check_nr_entries(n, sub_root) &&
+			    check_nr_entries(n, loc.sub_root) &&
 			    check_ordered_keys(n) &&
-			    check_parent_key(sub_root ? optional<uint64_t>() : key, n)) {
+			    check_parent_key(loc.sub_root ? optional<uint64_t>() : loc.key, n)) {
-				if (sub_root)
+				if (loc.sub_root)
-					new_root(level);
+					new_root(loc.level);
 
-				return check_leaf_key(level, n);
+				return check_leaf_key(loc.level, n);
 			}
 
 			return false;

persistent-data/space-maps/disk.cc

@@ -361,7 +361,7 @@ namespace {
 
 		virtual void check(block_counter &counter) const {
 			ref_count_checker::ptr v(new ref_count_checker(counter));
-			ref_counts_.visit(v);
+			ref_counts_.visit_depth_first(v);
 
 			block_address nr_entries = div_up<block_address>(get_nr_blocks(), ENTRIES_PER_BLOCK);
 			indexes_->check(counter, nr_entries);
@@ -488,17 +488,16 @@ namespace {
 
 	class bitmap_tree_validator : public btree_checker<1, index_entry_traits> {
 	public:
+		typedef typename btree_checker<1, index_entry_traits>::visitor::node_location node_location;
 		typedef boost::shared_ptr<bitmap_tree_validator> ptr;
 
 		bitmap_tree_validator(block_counter &counter)
 			: btree_checker<1, index_entry_traits>(counter) {
 		}
 
-		bool visit_leaf(unsigned level,
+		bool visit_leaf(node_location const &loc,
-				bool sub_root,
-				optional<uint64_t> key,
 				btree_detail::node_ref<index_entry_traits> const &n) {
-			bool r = btree_checker<1, index_entry_traits>::visit_leaf(level, sub_root, key, n);
+			bool r = btree_checker<1, index_entry_traits>::visit_leaf(loc, n);
 			if (!r)
 				return r;
 
@@ -586,7 +585,7 @@ namespace {
 
 		virtual void check(block_counter &counter, block_address nr_index_entries) const {
 			bitmap_tree_validator::ptr v(new bitmap_tree_validator(counter));
-			bitmaps_.visit(v);
+			bitmaps_.visit_depth_first(v);
 			v->check_all_index_entries_present(nr_index_entries);
 		}
 

thin-provisioning/device_checker.cc

@@ -14,14 +14,13 @@ using namespace thin_provisioning;
 namespace {
 	// FIXME: duplication with metadata.cc
 	transaction_manager::ptr
-	open_tm(block_manager<>::ptr bm) {
+	open_core_tm(block_manager<>::ptr bm) {
 		space_map::ptr sm(new core_map(bm->get_nr_blocks()));
 		sm->inc(SUPERBLOCK_LOCATION);
 		transaction_manager::ptr tm(new transaction_manager(bm, sm));
 		return tm;
 	}
 
-
 	class device_visitor : public btree<1, device_details_traits>::visitor {
 	public:
 		typedef boost::shared_ptr<device_visitor> ptr;
@@ -31,26 +30,20 @@ namespace {
 			: checker_(counter) {
 		}
 
-		bool visit_internal(unsigned level,
+		bool visit_internal(node_location const &loc,
-				    bool sub_root,
-				    optional<uint64_t> key,
 				    btree_detail::node_ref<uint64_traits> const &n) {
-			return checker_.visit_internal(level, sub_root, key, n);
+			return checker_.visit_internal(loc, n);
 		}
 
-		bool visit_internal_leaf(unsigned level,
+		bool visit_internal_leaf(node_location const &loc,
-					 bool sub_root,
-					 optional<uint64_t> key,
 					 btree_detail::node_ref<uint64_traits> const &n) {
-			return checker_.visit_internal_leaf(level, sub_root, key, n);
+			return checker_.visit_internal_leaf(loc, n);
 		}
 
-		bool visit_leaf(unsigned level,
+		bool visit_leaf(node_location const &loc,
-				bool sub_root,
-				optional<uint64_t> key,
 				btree_detail::node_ref<device_details_traits> const &n) {
 
-			if (!checker_.visit_leaf(level, sub_root, key, n))
+			if (!checker_.visit_leaf(loc, n))
 				return false;
 
 			for (unsigned i = 0; i < n.get_nr_entries(); i++)
@@ -83,13 +76,13 @@ device_checker::check()
 {
 	block_counter counter;
 	device_visitor::ptr v(new device_visitor(counter));
-	transaction_manager::ptr tm(open_tm(bm_));
+	transaction_manager::ptr tm(open_core_tm(bm_));
 	detail_tree::ptr details(new detail_tree(tm, root_,
 						 device_details_traits::ref_counter()));
 	damage_list_ptr damage(new damage_list);
 
 	try {
-		details->visit(v);
+		details->visit_depth_first(v);
 
 	} catch (std::exception const &e) {
 

thin-provisioning/metadata_dumper.cc

@@ -27,6 +27,7 @@ namespace {
 	class mappings_extractor : public btree<2, block_traits>::visitor {
 	public:
 		typedef boost::shared_ptr<mappings_extractor> ptr;
+		typedef btree_detail::node_location node_location;
 		typedef btree_checker<2, block_traits> checker;
 
 		mappings_extractor(uint64_t dev_id, emitter::ptr e,
@@ -41,20 +42,20 @@ namespace {
 			  found_errors_(false) {
 		}
 
-		bool visit_internal(unsigned level, bool sub_root, boost::optional<uint64_t> key,
+		bool visit_internal(node_location const &loc,
 				    btree_detail::node_ref<uint64_traits> const &n) {
 
-			if (!checker_.visit_internal(level, sub_root, key, n)) {
+			if (!checker_.visit_internal(loc, n)) {
 				found_errors_ = true;
 				return false;
 			}
 
-			return (sub_root && key) ? (*key == dev_id_) : true;
+			return (loc.sub_root && loc.key) ? (*loc.key == dev_id_) : true;
 		}
 
-		bool visit_internal_leaf(unsigned level, bool sub_root, boost::optional<uint64_t> key,
+		bool visit_internal_leaf(node_location const &loc,
 					 btree_detail::node_ref<uint64_traits> const &n) {
-			if (!checker_.visit_internal_leaf(level, sub_root, key, n)) {
+			if (!checker_.visit_internal_leaf(loc, n)) {
 				found_errors_ = true;
 				return false;
 			}
@@ -62,9 +63,9 @@ namespace {
 			return true;
 		}
 
-		bool visit_leaf(unsigned level, bool sub_root, boost::optional<uint64_t> maybe_key,
+		bool visit_leaf(node_location const &loc,
 				btree_detail::node_ref<block_traits> const &n) {
-			if (!checker_.visit_leaf(level, sub_root, maybe_key, n)) {
+			if (!checker_.visit_leaf(loc, n)) {
 				found_errors_ = true;
 				return false;
 			}
@@ -136,6 +137,7 @@ namespace {
 
 	class details_extractor : public btree<1, device_details_traits>::visitor {
 	public:
+		typedef typename btree<1, device_details_traits>::visitor::node_location node_location;
 		typedef boost::shared_ptr<details_extractor> ptr;
 		typedef btree_checker<1, device_details_traits> checker;
 
@@ -144,19 +146,19 @@ namespace {
 			  checker_(counter_, false) {
 		}
 
-		bool visit_internal(unsigned level, bool sub_root, boost::optional<uint64_t> key,
+		bool visit_internal(node_location const &loc,
 				    btree_detail::node_ref<uint64_traits> const &n) {
-			return checker_.visit_internal(level, sub_root, key, n);
+			return checker_.visit_internal(loc, n);
 		}
 
-		bool visit_internal_leaf(unsigned level, bool sub_root, boost::optional<uint64_t> key,
+		bool visit_internal_leaf(node_location const &loc,
 					 btree_detail::node_ref<uint64_traits> const &n) {
-			return checker_.visit_internal_leaf(level, sub_root, key, n);
+			return checker_.visit_internal_leaf(loc, n);
 		}
 
-		bool visit_leaf(unsigned level, bool sub_root, boost::optional<uint64_t> maybe_key,
+		bool visit_leaf(node_location const &loc,
 				btree_detail::node_ref<device_details_traits> const &n) {
-			if (!checker_.visit_leaf(level, sub_root, maybe_key, n))
+			if (!checker_.visit_leaf(loc, n))
 				return false;
 
 			for (unsigned i = 0; i < n.get_nr_entries(); i++)
@@ -198,7 +200,7 @@ thin_provisioning::metadata_dump(metadata::ptr md, emitter::ptr e, bool repair)
 
 	details_extractor::ptr de(new details_extractor);
 
-	md->details_->visit(de);
+	md->details_->visit_depth_first(de);
 	if (de->corruption() && !repair)
 		throw runtime_error("corruption in device details tree");
 
@@ -216,7 +218,7 @@ thin_provisioning::metadata_dump(metadata::ptr md, emitter::ptr e, bool repair)
 				dd.snapshotted_time_);
 
 		mappings_extractor::ptr me(new mappings_extractor(dev_id, e, md->metadata_sm_, md->data_sm_));
-		md->mappings_->visit(me);
+		md->mappings_->visit_depth_first(me);
 
 		if (me->corruption() && !repair) {
 			ostringstream out;

unit-tests/btree_t.cc

@@ -52,26 +52,24 @@ namespace {
 	//
 	class constraint_visitor : public btree<1, uint64_traits>::visitor {
 	public:
+		typedef btree_detail::node_location node_location;
 		typedef btree_detail::node_ref<uint64_traits> internal_node;
 		typedef btree_detail::node_ref<uint64_traits> leaf_node;
 
-		bool visit_internal(unsigned level, bool sub_root,
+		bool visit_internal(node_location const &loc,
-				    boost::optional<uint64_t> key,
 				    internal_node const &n) {
 			check_duplicate_block(n.get_location());
 			return true;
 		}
 
-	  bool visit_internal_leaf(unsigned level, bool sub_root,
+		bool visit_internal_leaf(node_location const &loc,
-				   boost::optional<uint64_t> key,
+					 internal_node const &n) {
-				   internal_node const &n) {
 			check_duplicate_block(n.get_location());
 			return true;
 		}
 
-	  bool visit_leaf(unsigned level, bool sub_root,
+		bool visit_leaf(node_location const &loc,
-			  boost::optional<uint64_t> key,
+				leaf_node const &n) {
-			  leaf_node const &n) {
 			check_duplicate_block(n.get_location());
 			return true;
 		}
@@ -94,7 +92,7 @@ namespace {
 		typedef btree<1, uint64_traits> tree_type;
 
 		tree_type::visitor::ptr v(new constraint_visitor);
-		tree->visit(v);
+		tree->visit_depth_first(v);
 	}
 }