Merge pull request #150 from mingnus/thin-check-fix-metadata-leaks

Add space-map counting tests

persistent-data/block_counter.h

@@ -62,6 +62,10 @@ namespace persistent_data {
 			return stop_on_error_;
 		}
 
+		virtual void clear() {
+			counts_.clear();
+		}
+
 	private:
 		count_map counts_;
 		bool stop_on_error_;
@@ -86,6 +90,11 @@ namespace persistent_data {
 		base::run_set<block_address> const& get_visited() const {
 			return visited_;
 		}
+
+		virtual void clear() {
+			visited_.clear();
+		}
+
 	private:
 		base::run_set<block_address> visited_;
 	};

persistent-data/space-maps/disk.cc

@@ -666,9 +666,14 @@ namespace {
 				if (!ie.blocknr_)
 					return;
 
+				try {
 					block_manager::read_ref rr = tm_.read_lock(ie.blocknr_, bitmap_validator_);
 					if (rr.data())
 						bc_.inc(ie.blocknr_);
+				} catch (std::exception &e) {
+					if (bc_.stop_on_error())
+						throw;
+				}
 			}
 
 		private:

persistent-data/transaction_manager.cc

@@ -37,6 +37,13 @@ transaction_manager::~transaction_manager()
 {
 }
 
+void
+transaction_manager::commit()
+{
+	wipe_shadow_table();
+	bm_->flush();
+}
+
 transaction_manager::write_ref
 transaction_manager::begin(block_address superblock, validator v)
 {

persistent-data/transaction_manager.h

@@ -42,6 +42,8 @@ namespace persistent_data {
 				    space_map::ptr sm);
 		~transaction_manager();
 
+		void commit();
+
 		// Drop the superblock reference to commit
 		write_ref begin(block_address superblock, validator v);
 		write_ref new_block(validator v);

thin-provisioning/metadata_checker.cc

@@ -603,8 +603,9 @@ void check_options::set_ignore_non_fatal() {
 	ignore_non_fatal_ = true;
 }
 
-void check_options::set_fix_metadata_leaks() {
+void check_options::set_auto_repair() {
 	fix_metadata_leaks_ = true;
+	clear_needs_check_ = true;
 }
 
 void check_options::set_clear_needs_check() {
@@ -650,8 +651,7 @@ thin_provisioning::check_metadata(std::string const &path,
 	checker.check();
 	if (check_opts.fix_metadata_leaks_)
 		checker.fix_metadata_leaks(check_opts.open_transaction_);
-	if (check_opts.fix_metadata_leaks_ ||
+	if (check_opts.clear_needs_check_)
-	    check_opts.clear_needs_check_)
 		checker.clear_needs_check_flag();
 
 	return checker.get_status();

thin-provisioning/metadata_checker.h

@@ -45,7 +45,7 @@ namespace thin_provisioning {
 		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_auto_repair();
 		void set_clear_needs_check();
 
 		bool use_metadata_snap_;

thin-provisioning/thin_check.cc

@@ -166,7 +166,7 @@ thin_check_cmd::run(int argc, char **argv)
 
 		case 6:
 			// auto-repair
-			fs.check_opts.set_fix_metadata_leaks();
+			fs.check_opts.set_auto_repair();
 			break;
 
 		default:

unit-tests/space_map_t.cc

@@ -17,6 +17,8 @@
 // <http://www.gnu.org/licenses/>.
 
 #include "gmock/gmock.h"
+#include "persistent-data/data-structures/btree_counter.h"
+#include "persistent-data/data-structures/simple_traits.h"
 #include "persistent-data/space-maps/disk.h"
 #include "persistent-data/space-maps/core.h"
 #include "persistent-data/space-maps/careful_alloc.h"
@@ -34,6 +36,8 @@ namespace {
 	unsigned const ENTRIES_PER_BLOCK = (MD_BLOCK_SIZE - sizeof(sm_disk_detail::bitmap_header)) * 4;
 	unsigned const NR_BITMAPS = 2;
 	unsigned const NR_BLOCKS = ENTRIES_PER_BLOCK * NR_BITMAPS;
+	unsigned const NR_DATA_BITMAPS = 4;
+	unsigned const NR_DATA_BLOCKS = ENTRIES_PER_BLOCK * NR_DATA_BITMAPS;
 	block_address const SUPERBLOCK = 0;
 	unsigned const MAX_LOCKS = 8;
 
@@ -245,6 +249,8 @@ namespace {
 	public:
 		MetadataSpaceMapTests() {
 			memset(metadata_sm_root_, 0, sizeof(metadata_sm_root_));
+			memset(data_sm_root_, 0, sizeof(data_sm_root_));
+
 			create_metadata_sm(NR_BLOCKS);
 		}
 
@@ -253,6 +259,13 @@ namespace {
 
 			ASSERT_THAT(metadata_sm_->root_size(), Le(sizeof(metadata_sm_root_)));
 			metadata_sm_->copy_root(metadata_sm_root_, sizeof(metadata_sm_root_));
+
+			if (!!data_sm_)
+				data_sm_->copy_root(data_sm_root_, sizeof(data_sm_root_));
+			else
+				memset(data_sm_root_, 0, sizeof(data_sm_root_));
+
+			tm_->commit();
 		}
 
 		void open() {
@@ -260,6 +273,8 @@ namespace {
 			space_map::ptr core_sm{create_core_map(NR_BLOCKS)};
 			tm_ = transaction_manager::ptr(new transaction_manager(bm_, core_sm));
 			metadata_sm_ = persistent_data::open_metadata_sm(*tm_, metadata_sm_root_);
+			if (*(uint64_t*)data_sm_root_)
+				data_sm_ = persistent_data::open_disk_sm(*tm_, data_sm_root_);
 			tm_->set_sm(metadata_sm_);
 		}
 
@@ -291,6 +306,12 @@ namespace {
 			sm_disk_detail::sm_root_traits::unpack(d, v);
 		}
 
+		void get_data_sm_root(sm_disk_detail::sm_root &v) const {
+			sm_disk_detail::sm_root_disk d;
+			data_sm_->copy_root(&d, sizeof(d));
+			sm_disk_detail::sm_root_traits::unpack(d, v);
+		}
+
 		void mark_shadowed() {
 			block_counter bc(true);
 			metadata_sm_->count_metadata(bc);
@@ -305,7 +326,7 @@ namespace {
 		checked_space_map::ptr metadata_sm_;
 		checked_space_map::ptr data_sm_;
 
-	private:
+	protected:
 		void create_metadata_sm(block_address nr_blocks) {
 			bm_ = test::create_bm(NR_BLOCKS);
 			space_map::ptr core_sm{create_core_map(nr_blocks)};
@@ -327,6 +348,210 @@ namespace {
 		transaction_manager::ptr tm_;
 
 		unsigned char metadata_sm_root_[128];
+		unsigned char data_sm_root_[128];
+	};
+
+	//--------------------------------
+
+	class MetaSMCountingTests : public MetadataSpaceMapTests {
+	public:
+		MetaSMCountingTests() {
+			populate_ref_counts();
+			commit();
+
+			block_counter bc(true);
+			metadata_sm_->count_metadata(bc);
+			sm_total_blocks_ = bc.get_counts().size();
+		}
+
+		void trash_bitmap_root() {
+			sm_disk_detail::sm_root root;
+			get_root(root);
+			test::zero_block(bm_, root.bitmap_root_);
+		}
+
+		// TODO: trash the bitmap corresponding to a given key
+		void trash_bitmap_block(unsigned index) {
+			std::vector<sm_disk_detail::index_entry> entries;
+			load_ies(entries);
+			ASSERT_LT(index, entries.size());
+			test::zero_block(bm_, entries[index].blocknr_);
+		}
+
+		void trash_ref_count_root() {
+			sm_disk_detail::sm_root root;
+			get_root(root);
+			test::zero_block(bm_, root.ref_count_root_);
+		}
+
+		// TODO: trash the node corresponding to a given key
+		void trash_ref_count_node() {
+			count_ref_count_blocks();
+
+			sm_disk_detail::sm_root root;
+			get_root(root);
+			ref_count_blocks_.erase(root.ref_count_root_);
+
+			ASSERT_GT(ref_count_blocks_.size(), 0u);
+			test::zero_block(bm_, *ref_count_blocks_.begin());
+		}
+
+		std::set<block_address> ref_count_blocks_;
+
+		size_t sm_total_blocks_;
+
+	private:
+		void populate_ref_counts() {
+			while (true) {
+				space_map::maybe_block b = metadata_sm_->new_block();
+				if (!b)
+					break;
+
+				// FIXME: the statement might throw if there's
+				// no free block for ref-count tree
+				metadata_sm_->set_count(*b, 3);
+			}
+		}
+
+		void count_ref_count_blocks() {
+			sm_disk_detail::sm_root root;
+			get_root(root);
+
+			block_counter bc(true);
+			btree<1, uint32_traits> ref_counts(*tm_,
+					root.ref_count_root_,
+					uint32_traits::ref_counter());
+			persistent_data::noop_value_counter<uint32_t> vc;
+			count_btree_blocks(ref_counts, bc, vc);
+
+			block_counter::count_map const &m = bc.get_counts();
+			for (auto const &c : m)
+				ref_count_blocks_.insert(c.first);
+		}
+	};
+
+	class DataSMCountingTests : public MetadataSpaceMapTests {
+	public:
+		DataSMCountingTests() {
+			create_data_sm(NR_DATA_BLOCKS);
+			populate_ref_counts();
+			commit();
+
+			count_index_store_blocks();
+			count_ref_count_blocks();
+			collect_bitmaps();
+
+			sm_total_blocks_ = nr_index_store_blocks_
+					+ nr_ref_count_blocks_
+					+ NR_DATA_BITMAPS;
+		}
+
+		void trash_bitmap_root() {
+			sm_disk_detail::sm_root root;
+			get_data_sm_root(root);
+			test::zero_block(bm_, root.bitmap_root_);
+		}
+
+		// TODO: trash the bitmap corresponding to a given key
+		void trash_bitmap_block() {
+			test::zero_block(bm_, *bitmap_blocks_.begin());
+		}
+
+		void trash_ref_count_root() {
+			sm_disk_detail::sm_root root;
+			get_data_sm_root(root);
+			test::zero_block(bm_, root.ref_count_root_);
+		}
+
+		// TODO: trash the node corresponding to a given key
+		void trash_ref_count_node() {
+			sm_disk_detail::sm_root root;
+			get_data_sm_root(root);
+			ref_count_blocks_.erase(root.ref_count_root_);
+
+			ASSERT_GT(ref_count_blocks_.size(), 0u);
+			test::zero_block(bm_, *ref_count_blocks_.begin());
+		}
+
+		std::set<block_address> index_store_blocks_;
+		std::set<block_address> bitmap_blocks_;
+		std::set<block_address> ref_count_blocks_;
+
+		size_t sm_total_blocks_;
+		size_t nr_index_store_blocks_;
+		size_t nr_ref_count_blocks_;
+
+	private:
+		void populate_ref_counts() {
+			while (true) {
+				space_map::maybe_block b = data_sm_->new_block();
+				if (!b)
+					break;
+
+				// FIXME: the statement might throw if there's
+				// no free block for ref-count tree
+				data_sm_->set_count(*b, 3);
+			}
+		}
+
+		// nodes for index_store, except bitmaps
+		void count_index_store_blocks() {
+			sm_disk_detail::sm_root root;
+			get_data_sm_root(root);
+
+			block_counter bc(true);
+			btree<1, sm_disk_detail::index_entry_traits> index_store(*tm_,
+					root.bitmap_root_,
+					sm_disk_detail::index_entry_traits::ref_counter());
+			persistent_data::noop_value_counter<sm_disk_detail::index_entry> vc;
+			count_btree_blocks(index_store, bc, vc);
+
+			block_counter::count_map const &m = bc.get_counts();
+			for (auto const &c : m)
+				index_store_blocks_.insert(c.first);
+
+			nr_index_store_blocks_ = index_store_blocks_.size();
+		}
+
+		void count_ref_count_blocks() {
+			sm_disk_detail::sm_root root;
+			get_data_sm_root(root);
+
+			block_counter bc(true);
+			btree<1, uint32_traits> ref_counts(*tm_,
+					root.ref_count_root_,
+					uint32_traits::ref_counter());
+			persistent_data::noop_value_counter<uint32_t> vc;
+			count_btree_blocks(ref_counts, bc, vc);
+
+			block_counter::count_map const &m = bc.get_counts();
+			for (auto const &c : m)
+				ref_count_blocks_.insert(c.first);
+
+			nr_ref_count_blocks_ = ref_count_blocks_.size();
+		}
+
+		void collect_bitmaps() {
+			block_counter bc(true);
+			data_sm_->count_metadata(bc);
+
+			block_counter::count_map const &m = bc.get_counts();
+			for (auto const &c : m)
+				bitmap_blocks_.insert(c.first);
+
+			for (auto const &b : index_store_blocks_)
+				bitmap_blocks_.erase(b);
+
+			for (auto const &b : ref_count_blocks_)
+				bitmap_blocks_.erase(b);
+
+			size_t total = m.size();
+
+			ASSERT_EQ(bitmap_blocks_.size(), NR_DATA_BITMAPS);
+			ASSERT_EQ(total, nr_index_store_blocks_
+					+ nr_ref_count_blocks_
+					+ NR_DATA_BITMAPS);
+		}
 	};
 }
 
@@ -363,7 +588,7 @@ TEST_F(SpaceMapTests, test_sm_metadata)
 
 TEST_F(MetadataSpaceMapTests, test_metadata_and_disk)
 {
-	create_data_sm(NR_BLOCKS * 2);
+	create_data_sm(NR_DATA_BLOCKS);
 }
 
 // Test whether sm_recursive protected allocated blocks in a recursion
@@ -513,3 +738,103 @@ TEST_F(MetadataSpaceMapTests, test_intended_in_place_modification)
 }
 
 //----------------------------------------------------------------
+
+TEST_F(MetaSMCountingTests, test_trashed_bitmap_root)
+{
+	sm_disk_detail::sm_root_disk d;
+	metadata_sm_->copy_root(&d, sizeof(d));
+
+	trash_bitmap_root();
+
+	// explicitly test open_metadata_sm()
+	block_manager::ptr bm(new block_manager("./test.data", NR_BLOCKS, MAX_LOCKS, block_manager::READ_WRITE));
+	space_map::ptr core_sm{create_core_map(NR_BLOCKS)};
+	transaction_manager::ptr tm(new transaction_manager(bm_, core_sm));
+	ASSERT_THROW(persistent_data::open_metadata_sm(*tm, &d), runtime_error);
+}
+
+TEST_F(MetaSMCountingTests, test_trashed_bitmap_block)
+{
+	trash_bitmap_block(1);
+
+	block_counter bc_ignore_errors;
+	ASSERT_NO_THROW(metadata_sm_->count_metadata(bc_ignore_errors));
+	ASSERT_EQ(bc_ignore_errors.get_counts().size(), sm_total_blocks_ - 1u);
+
+	block_counter bc_stop_on_error(true);
+	ASSERT_THROW(metadata_sm_->count_metadata(bc_stop_on_error), runtime_error);
+}
+
+TEST_F(MetaSMCountingTests, test_trashed_ref_count_root)
+{
+	trash_ref_count_root();
+
+	block_counter bc_ignore_errors;
+	ASSERT_NO_THROW(metadata_sm_->count_metadata(bc_ignore_errors));
+	ASSERT_EQ(bc_ignore_errors.get_counts().size(), 1u + NR_BITMAPS);
+
+	block_counter bc_stop_on_error(true);
+	ASSERT_THROW(metadata_sm_->count_metadata(bc_stop_on_error), runtime_error);
+}
+
+TEST_F(MetaSMCountingTests, test_trashed_ref_count_node)
+{
+	trash_ref_count_node();
+
+	block_counter bc_ignore_errors;
+	ASSERT_NO_THROW(metadata_sm_->count_metadata(bc_ignore_errors));
+
+	block_counter bc_stop_on_error(true);
+	ASSERT_THROW(metadata_sm_->count_metadata(bc_stop_on_error), runtime_error);
+}
+
+//----------------------------------------------------------------
+
+TEST_F(DataSMCountingTests, test_trashed_bitmap_root)
+{
+	trash_bitmap_root();
+
+	block_counter bc_ignore_errors;
+	ASSERT_NO_THROW(data_sm_->count_metadata(bc_ignore_errors));
+	ASSERT_EQ(bc_ignore_errors.get_counts().size(), nr_ref_count_blocks_);
+
+	block_counter bc_stop_on_error(true);
+	ASSERT_THROW(data_sm_->count_metadata(bc_stop_on_error), runtime_error);
+}
+
+TEST_F(DataSMCountingTests, test_trashed_bitmap_block)
+{
+	trash_bitmap_block();
+
+	block_counter bc_ignore_errors;
+	ASSERT_NO_THROW(data_sm_->count_metadata(bc_ignore_errors));
+	ASSERT_EQ(bc_ignore_errors.get_counts().size(), sm_total_blocks_ - 1u);
+
+	block_counter bc_stop_on_error(true);
+	ASSERT_THROW(data_sm_->count_metadata(bc_stop_on_error), runtime_error);
+}
+
+TEST_F(DataSMCountingTests, test_trashed_ref_count_root)
+{
+	trash_ref_count_root();
+
+	block_counter bc_ignore_errors;
+	ASSERT_NO_THROW(data_sm_->count_metadata(bc_ignore_errors));
+	ASSERT_EQ(bc_ignore_errors.get_counts().size(), nr_index_store_blocks_ + NR_DATA_BITMAPS);
+
+	block_counter bc_stop_on_error(true);
+	ASSERT_THROW(data_sm_->count_metadata(bc_stop_on_error), runtime_error);
+}
+
+TEST_F(DataSMCountingTests, test_trashed_ref_count_node)
+{
+	trash_ref_count_node();
+
+	block_counter bc_ignore_errors;
+	ASSERT_NO_THROW(data_sm_->count_metadata(bc_ignore_errors));
+
+	block_counter bc_stop_on_error(true);
+	ASSERT_THROW(data_sm_->count_metadata(bc_stop_on_error), runtime_error);
+}
+
+//----------------------------------------------------------------