include sm_disk bitmaps in metadata counts

btree.h

@@ -325,9 +325,11 @@ namespace persistent_data {
 			virtual ~visitor() {}
 			typedef boost::shared_ptr<visitor> ptr;
 
-			virtual void visit_internal(unsigned level, bool is_root, internal_node const &n) = 0;
+			// The bool return values indicate whether the walk
-			virtual void visit_internal_leaf(unsigned level, bool is_root, internal_node const &n) = 0;
+			// should be continued into sub trees of the node (true == continue).
-			virtual void visit_leaf(unsigned level, bool is_root, leaf_node const &n) = 0;
+			virtual bool visit_internal(unsigned level, bool is_root, internal_node const &n) = 0;
+			virtual bool visit_internal_leaf(unsigned level, bool is_root, internal_node const &n) = 0;
+			virtual bool visit_leaf(unsigned level, bool is_root, leaf_node const &n) = 0;
 		};
 
 		// Walks the tree in depth first order

btree.tcc

@@ -627,25 +627,21 @@ walk_tree(typename visitor::ptr visitor,
 {
 	using namespace btree_detail;
 
-	try {
+	read_ref blk = tm_->read_lock(b);
-		read_ref blk = tm_->read_lock(b);
+	internal_node o = to_node<uint64_traits, BlockSize>(blk);
-		internal_node o = to_node<uint64_traits, BlockSize>(blk);
+	if (o.get_type() == INTERNAL) {
-		if (o.get_type() == INTERNAL) {
+		if (visitor->visit_internal(level, is_root, o))
-			visitor->visit_internal(level, is_root, o);
 			for (unsigned i = 0; i < o.get_nr_entries(); i++)
 				walk_tree(visitor, level, false, o.value_at(i));
 
-		} else if (level < Levels - 1) {
+	} else if (level < Levels - 1) {
-			visitor->visit_internal_leaf(level, is_root, o);
+		if (visitor->visit_internal_leaf(level, is_root, o))
 			for (unsigned i = 0; i < o.get_nr_entries(); i++)
 				walk_tree(visitor, level + 1, true, o.value_at(i));
 
-		} else {
+	} else {
-			leaf_node ov = to_node<ValueTraits, BlockSize>(blk);
+		leaf_node ov = to_node<ValueTraits, BlockSize>(blk);
-			visitor->visit_leaf(level, is_root, ov);
+		visitor->visit_leaf(level, is_root, ov);
-		}
-	} catch (...) {		// FIXME: we should only catch terminate_walk type exceptions
-
 	}
 }
 

btree_validator.h

@@ -74,51 +74,60 @@ namespace persistent_data {
 			  errs_(new error_set("btree errors")) {
 		}
 
-		void visit_internal(unsigned level, bool is_root,
+		bool visit_internal(unsigned level, bool is_root,
 				    btree_detail::node_ref<uint64_traits, BlockSize> const &n) {
-			check_duplicate_block(n.get_location());
+			if (already_visited(n))
+				return false;
+
 			check_block_nr(n);
 			check_max_entries(n);
 			check_nr_entries(n, is_root);
-
+			return true;
-			for (unsigned i = 0; i < n.get_nr_entries(); i++)
-				counter_.inc(n.value_at(i));
 		}
 
-		void visit_internal_leaf(unsigned level, bool is_root,
+		bool visit_internal_leaf(unsigned level, bool is_root,
 					 btree_detail::node_ref<uint64_traits, BlockSize> const &n) {
-			check_duplicate_block(n.get_location());
+			if (already_visited(n))
+				return false;
+
 			check_block_nr(n);
 			check_max_entries(n);
 			check_nr_entries(n, is_root);
-
+			return true;
-			for (unsigned i = 0; i < n.get_nr_entries(); i++)
-				counter_.inc(n.value_at(i));
 		}
 
-		void visit_leaf(unsigned level, bool is_root,
+		bool visit_leaf(unsigned level, bool is_root,
 				btree_detail::node_ref<ValueTraits, BlockSize> const &n) {
-			counter_.inc(n.get_location());
+			if (already_visited(n))
-			check_duplicate_block(n.get_location());
+				return false;
+
 			check_block_nr(n);
 			check_max_entries(n);
 			check_nr_entries(n, is_root);
+			return true;
 		}
 
 		boost::optional<error_set::ptr> get_errors() const {
 			return errs_;
 		}
 
+	protected:
+		block_counter &get_counter() {
+			return counter_;
+		}
+
 	private:
-		void check_duplicate_block(block_address b) {
+		template <typename node>
-			if (seen_.count(b)) {
+		bool already_visited(node const &n) {
-				std::ostringstream out;
+			block_address b = n.get_location();
-				out << "duplicate block in btree: " << b;
+
-				errs_->add_child(out.str());
+			counter_.inc(b);
-				throw runtime_error(out.str());
+
-			}
+			if (seen_.count(b) > 0)
+				return true;
 
 			seen_.insert(b);
+			return false;
 		}
 
 		template <typename node>

metadata.cc

@@ -54,20 +54,31 @@ namespace {
 			  data_counter_(data_counter) {
 		}
 
-		void visit_internal_leaf(unsigned level, bool is_root,
+		// Sharing can only occur in level 1 nodes.
+		// FIXME: not true once we start having held roots.
+		bool visit_internal_leaf(unsigned level, bool is_root,
 					 btree_detail::node_ref<uint64_traits, MD_BLOCK_SIZE> const &n) {
-			btree_validator<2, block_traits, MD_BLOCK_SIZE>::visit_internal_leaf(level, is_root, n);
+
+			bool r = btree_validator<2, block_traits, MD_BLOCK_SIZE>::visit_internal_leaf(level, is_root, n);
+			if (!r && level == 0) {
+				throw runtime_error("unexpected sharing in level 0 of mapping tree.");
+			}
 
 			for (unsigned i = 0; i < n.get_nr_entries(); i++)
 				devices_.insert(n.key_at(i));
+
+			return r;
 		}
 
-		void visit_leaf(unsigned level, bool is_root,
+		bool visit_leaf(unsigned level, bool is_root,
 				btree_detail::node_ref<block_traits, MD_BLOCK_SIZE> const &n) {
-			btree_validator<2, block_traits, MD_BLOCK_SIZE>::visit_leaf(level, is_root, n);
+			bool r = btree_validator<2, block_traits, MD_BLOCK_SIZE>::visit_leaf(level, is_root, n);
 
-			for (unsigned i = 0; i < n.get_nr_entries(); i++)
+			if (r)
-				data_counter_.inc(n.value_at(i).block_);
+				for (unsigned i = 0; i < n.get_nr_entries(); i++)
+					data_counter_.inc(n.value_at(i).block_);
+
+			return r;
 		}
 
 		set<uint64_t> const &get_devices() const {
@@ -87,12 +98,15 @@ namespace {
 			: btree_validator<1, device_details_traits, MD_BLOCK_SIZE>(counter) {
 		}
 
-		void visit_leaf(unsigned level, bool is_root,
+		bool visit_leaf(unsigned level, bool is_root,
 				btree_detail::node_ref<device_details_traits, MD_BLOCK_SIZE> const &n) {
-			btree_validator<1, device_details_traits, MD_BLOCK_SIZE>::visit_leaf(level, is_root, n);
+			bool r = btree_validator<1, device_details_traits, MD_BLOCK_SIZE>::visit_leaf(level, is_root, n);
 
-			for (unsigned i = 0; i < n.get_nr_entries(); i++)
+			if (r)
-				devices_.insert(n.key_at(i));
+				for (unsigned i = 0; i < n.get_nr_entries(); i++)
+					devices_.insert(n.key_at(i));
+
+			return r;
 		}
 
 		set<uint64_t> const &get_devices() const {

space_map_disk.h

@@ -321,6 +321,17 @@ namespace persistent_data {
 			bitmap_tree_validator(block_counter &counter)
 				: btree_validator<1, index_entry_traits, BlockSize>(counter) {
 			}
+
+			bool visit_leaf(unsigned level, bool is_root,
+					btree_detail::node_ref<index_entry_traits, BlockSize> const &n) {
+				bool r = btree_validator<1, index_entry_traits, BlockSize>::visit_leaf(level, is_root, n);
+
+				if (r)
+					for (unsigned i = 0; i < n.get_nr_entries(); i++)
+						btree_validator<1, index_entry_traits, BlockSize>::get_counter().inc(n.value_at(i).blocknr_);
+
+				return r;
+			}
 		};
 
 		template <uint32_t BlockSize>
@@ -362,7 +373,6 @@ namespace persistent_data {
 				sm_disk_base<BlockSize>::check(counter);
 
 				typename bitmap_tree_validator<BlockSize>::ptr v(new bitmap_tree_validator<BlockSize>(counter));
-				counter.inc(bitmaps_.get_root());
 				bitmaps_.visit(v);
 			}