Merge branch '2020-05-27-check-data-space-map'

Makefile.in

@@ -318,7 +318,6 @@ install: bin/pdata_tools $(MANPAGES)
 	$(INSTALL_DATA) man8/thin_repair.8 $(MANPATH)/man8
 	$(INSTALL_DATA) man8/thin_restore.8 $(MANPATH)/man8
 	$(INSTALL_DATA) man8/thin_rmap.8 $(MANPATH)/man8
-
 	$(INSTALL_DATA) man8/thin_metadata_size.8 $(MANPATH)/man8
 	$(INSTALL_DATA) man8/era_check.8 $(MANPATH)/man8
 	$(INSTALL_DATA) man8/era_dump.8 $(MANPATH)/man8
@@ -333,10 +332,16 @@ ifeq ("@DEVTOOLS@", "yes")
 	ln -s -f pdata_tools $(BINDIR)/thin_scan
 endif
 
-.PHONY: install install-rust-tools
+.PHONY: install install-rust-tools rust-tools
 
-install-rust-tools:
+rust-tools:
-	cargo install --path . --root $(BINDIR)
+	cargo build --release
+
+install-rust-tools: man8/thin_metadata_pack.8 man8/thin_metadata_unpack.8 rust-tools
+	$(INSTALL_PROGRAM) target/release/thin_metadata_pack  $(BINDIR)
+	$(INSTALL_PROGRAM) target/release/thin_metadata_unpack  $(BINDIR)
+	$(STRIP) $(BINDIR)/thin_metadata_pack
+	$(STRIP) $(BINDIR)/thin_metadata_unpack
 	$(INSTALL_DATA) man8/thin_metadata_pack.8 $(MANPATH)/man8
 	$(INSTALL_DATA) man8/thin_metadata_unpack.8 $(MANPATH)/man8
 

functional-tests/scenario-string-constants.scm

@@ -29,6 +29,7 @@ Options:
   {-q|--quiet}
   {-h|--help}
   {-V|--version}
+  {-m|--metadata-snap}
   {--override-mapping-root}
   {--clear-needs-check-flag}
   {--ignore-non-fatal-errors}

thin-provisioning/metadata_checker.cc

@@ -18,11 +18,13 @@
 
 #include "base/nested_output.h"
 #include "persistent-data/file_utils.h"
+#include "persistent-data/space-maps/core.h"
 #include "thin-provisioning/metadata.h"
 #include "thin-provisioning/metadata_checker.h"
 #include "thin-provisioning/metadata_counter.h"
 #include "thin-provisioning/superblock.h"
 
+using namespace boost;
 using namespace persistent_data;
 using namespace thin_provisioning;
 
@@ -84,6 +86,20 @@ namespace {
 
 	//--------------------------------
 
+	class data_ref_counter : public mapping_tree_detail::mapping_visitor {
+	public:
+		data_ref_counter(space_map::ptr sm)
+			: sm_(sm) {
+		}
+
+		virtual void visit(btree_path const &path, mapping_tree_detail::block_time const &bt) {
+			sm_->inc(bt.block_);
+		}
+
+        private:
+	        space_map::ptr sm_;
+	};
+
 	class mapping_reporter : public mapping_tree_detail::damage_visitor {
 	public:
 		mapping_reporter(nested_output &out)
@@ -121,12 +137,13 @@ namespace {
 	//--------------------------------
 
 	error_state examine_superblock(block_manager::ptr bm,
+                                       block_address sb_location,
 				       nested_output &out) {
 		out << "examining superblock" << end_message();
 		nested_output::nest _ = out.push();
 
 		superblock_reporter sb_rep(out);
-		check_superblock(bm, sb_rep);
+		check_superblock(bm, sb_rep, sb_location);
 
 		return sb_rep.get_error();
 	}
@@ -161,14 +178,20 @@ namespace {
 
 	error_state examine_mapping_tree_(transaction_manager::ptr tm,
 					  superblock_detail::superblock const &sb,
-					  nested_output &out) {
+					  nested_output &out,
+                                          optional<space_map::ptr> data_sm) {
 		out << "examining mapping tree" << end_message();
 		nested_output::nest _ = out.push();
 
 		mapping_reporter mapping_rep(out);
 		mapping_tree mtree(*tm, sb.data_mapping_root_,
 				   mapping_tree_detail::block_traits::ref_counter(tm->get_sm()));
-		check_mapping_tree(mtree, mapping_rep);
+
+		if (data_sm) {
+			data_ref_counter dcounter(*data_sm);
+			walk_mapping_tree(mtree, dcounter, mapping_rep);
+		} else
+			check_mapping_tree(mtree, mapping_rep);
 
 		return mapping_rep.get_error();
 	}
@@ -184,9 +207,10 @@ namespace {
 
 	error_state examine_mapping_tree(transaction_manager::ptr tm,
 					 superblock_detail::superblock const &sb,
-					 nested_output &out) {
+					 nested_output &out,
+                                         optional<space_map::ptr> data_sm) {
 		error_state err = examine_devices_tree_(tm, sb, out);
-		err << examine_mapping_tree_(tm, sb, out);
+		err << examine_mapping_tree_(tm, sb, out, data_sm);
 
 		return err;
 	}
@@ -222,6 +246,34 @@ namespace {
 		return err;
 	}
 
+	error_state compare_space_maps(space_map::ptr actual, space_map::ptr expected,
+                                       nested_output &out)
+	{
+		error_state err = NO_ERROR;
+		auto nr_blocks = actual->get_nr_blocks();
+
+		if (expected->get_nr_blocks() != nr_blocks) {
+			out << "internal error: nr blocks in space maps differ"
+			    << end_message();
+			err << FATAL;
+		} else {
+			for (block_address b = 0; b < nr_blocks; b++) {
+				auto a_count = actual->get_count(b);
+				auto e_count = actual->get_count(b);
+				
+				if (a_count != e_count) {
+					out << "data reference counts differ for block " << b
+					    << ", expected " << e_count
+					    << ", but got " << a_count
+					    << end_message();
+					err << (a_count > e_count ? NON_FATAL : FATAL);
+				}
+			}
+		}
+
+		return err;
+	}
+
 	void print_info(transaction_manager::ptr tm,
 			superblock_detail::superblock const &sb,
                         nested_output &out)
@@ -238,11 +290,11 @@ namespace {
 
 	//--------------------------------
 
-	class base_metadata_checker : public metadata_checker {
+	class metadata_checker {
 	public:
-		base_metadata_checker(block_manager::ptr bm,
+		metadata_checker(block_manager::ptr bm,
-				      check_options check_opts,
+		      		 check_options check_opts,
-				      output_options output_opts)
+				 output_options output_opts)
 			: bm_(bm),
 			  options_(check_opts),
 			  out_(cerr, 2),
@@ -256,28 +308,44 @@ namespace {
 
 		error_state check() {
 			error_state err = NO_ERROR;
+			auto sb_location = superblock_detail::SUPERBLOCK_LOCATION;
 
-			err << examine_superblock(bm_, out_);
+			if (options_.use_metadata_snap_) {
-
+				superblock_detail::superblock sb = read_superblock(bm_, sb_location);
+				sb_location = sb.metadata_snap_;
+				if (sb_location == superblock_detail::SUPERBLOCK_LOCATION)
+					throw runtime_error("No metadata snapshot found.");
+			}
+			
+			err << examine_superblock(bm_, sb_location, out_);
 			if (err == FATAL) {
 				if (check_for_xml(bm_))
 					out_ << "This looks like XML.  thin_check only checks the binary metadata format." << end_message();
 				return err;
 			}
 
-			superblock_detail::superblock sb = read_superblock(bm_);
+			superblock_detail::superblock sb = read_superblock(bm_, sb_location);
-			transaction_manager::ptr tm =
+			transaction_manager::ptr tm = open_tm(bm_, sb_location);
-				open_tm(bm_, superblock_detail::SUPERBLOCK_LOCATION);
 			sb.data_mapping_root_ = mapping_root(sb, options_);
 
 			print_info(tm, sb, info_out_);
 
-			err << examine_data_mappings(tm, sb, options_.check_data_mappings_, out_);
+			if (options_.sm_opts_ == check_options::SPACE_MAP_FULL) {
+				space_map::ptr data_sm{open_disk_sm(*tm, &sb.data_space_map_root_)};
+				optional<space_map::ptr> core_sm{create_core_map(data_sm->get_nr_blocks())};
+				err << examine_data_mappings(tm, sb, options_.check_data_mappings_, out_, core_sm);
 
-			// if we're checking everything, and there were no errors,
+				// if we're checking everything, and there were no errors,
-			// then we should check the space maps too.
+				// then we should check the space maps too.
-			if (err != FATAL)
+				if (err != FATAL) {
-				err << examine_metadata_space_map(tm, sb, options_.check_metadata_space_map_, out_);
+					err << examine_metadata_space_map(tm, sb, options_.sm_opts_, out_);
+
+					if (core_sm)
+						err << compare_space_maps(data_sm, *core_sm, out_);
+				}
+			} else
+				err << examine_data_mappings(tm, sb, options_.check_data_mappings_, out_,
+                                                             optional<space_map::ptr>());
 
 			return err;
 		}
@@ -287,7 +355,8 @@ namespace {
 		examine_data_mappings(transaction_manager::ptr tm,
 				      superblock_detail::superblock const &sb,
 				      check_options::data_mapping_options option,
-				      nested_output &out) {
+				      nested_output &out,
+                                      optional<space_map::ptr> data_sm) {
 			error_state err = NO_ERROR;
 
 			switch (option) {
@@ -295,7 +364,7 @@ namespace {
 				err << examine_top_level_mapping_tree(tm, sb, out);
 				break;
 			case check_options::DATA_MAPPING_LEVEL2:
-				err << examine_mapping_tree(tm, sb, out);
+				err << examine_mapping_tree(tm, sb, out, data_sm);
 				break;
 			default:
 				break; // do nothing
@@ -307,12 +376,12 @@ namespace {
 		static error_state
 		examine_metadata_space_map(transaction_manager::ptr tm,
 					   superblock_detail::superblock const &sb,
-					   check_options::metadata_space_map_options option,
+					   check_options::space_map_options option,
 					   nested_output &out) {
 			error_state err = NO_ERROR;
 
 			switch (option) {
-			case check_options::METADATA_SPACE_MAP_FULL:
+			case check_options::SPACE_MAP_FULL:
 				err << check_space_map_counts(tm, sb, out);
 				break;
 			default:
@@ -332,32 +401,37 @@ namespace {
 //----------------------------------------------------------------
 
 check_options::check_options()
-	: check_data_mappings_(DATA_MAPPING_LEVEL2),
+	: use_metadata_snap_(false),
-	  check_metadata_space_map_(METADATA_SPACE_MAP_FULL) {
+	  check_data_mappings_(DATA_MAPPING_LEVEL2),
+	  sm_opts_(SPACE_MAP_FULL) {
 }
 
 void check_options::set_superblock_only() {
 	check_data_mappings_ = DATA_MAPPING_NONE;
-	check_metadata_space_map_ = METADATA_SPACE_MAP_NONE;
+	sm_opts_ = SPACE_MAP_NONE;
 }
 
 void check_options::set_skip_mappings() {
 	check_data_mappings_ = DATA_MAPPING_LEVEL1;
-	check_metadata_space_map_ = METADATA_SPACE_MAP_NONE;
+	sm_opts_ = SPACE_MAP_NONE;
 }
 
 void check_options::set_override_mapping_root(block_address b) {
 	override_mapping_root_ = b;
 }
 
-metadata_checker::ptr
+void check_options::set_metadata_snap() {
-thin_provisioning::create_base_checker(block_manager::ptr bm,
+	use_metadata_snap_ = true;
-				       check_options const &check_opts,
+	sm_opts_ = SPACE_MAP_NONE;
-				       output_options output_opts)
+}
+		
+base::error_state
+thin_provisioning::check_metadata(block_manager::ptr bm,
+				  check_options const &check_opts,
+				  output_options output_opts)
 {
-	metadata_checker::ptr checker;
+	metadata_checker checker(bm, check_opts, output_opts);
-	checker = metadata_checker::ptr(new base_metadata_checker(bm, check_opts, output_opts));
+	return checker.check();
-	return checker;
 }
 
 //----------------------------------------------------------------

thin-provisioning/metadata_checker.h

@@ -33,9 +33,9 @@ namespace thin_provisioning {
 			DATA_MAPPING_LEVEL2,
 		};
 
-		enum metadata_space_map_options {
+		enum space_map_options {
-			METADATA_SPACE_MAP_NONE,
+			SPACE_MAP_NONE,
-			METADATA_SPACE_MAP_FULL,
+			SPACE_MAP_FULL,
 		};
 
 		check_options();
@@ -43,9 +43,11 @@ namespace thin_provisioning {
 		void set_superblock_only();
 		void set_skip_mappings();
 		void set_override_mapping_root(bcache::block_address b);
+		void set_metadata_snap();
 
+		bool use_metadata_snap_;
 		data_mapping_options check_data_mappings_;
-		metadata_space_map_options check_metadata_space_map_;
+		space_map_options sm_opts_;
 		boost::optional<bcache::block_address> override_mapping_root_;
 	};
 
@@ -54,19 +56,10 @@ namespace thin_provisioning {
 		OUTPUT_QUIET,
 	};
 
-	class metadata_checker {
+	base::error_state
-	public:
+	check_metadata(persistent_data::block_manager::ptr bm,
-		typedef std::shared_ptr<metadata_checker> ptr;
+		       check_options const &check_opts,
-
+		       output_options output_opts);
-		virtual ~metadata_checker() {}
-
-		virtual base::error_state check() = 0;
-	};
-
-	metadata_checker::ptr
-	create_base_checker(persistent_data::block_manager::ptr bm,
-			    check_options const &check_opts,
-			    output_options output_opts);
 }
 
 //----------------------------------------------------------------

thin-provisioning/superblock.cc

@@ -194,11 +194,12 @@ namespace thin_provisioning {
 
 	void
 	check_superblock(block_manager::ptr bm,
-			 superblock_detail::damage_visitor &visitor) {
+			 superblock_detail::damage_visitor &visitor,
+                         block_address sb_location) {
 		using namespace superblock_detail;
 
 		try {
-			bm->read_lock(SUPERBLOCK_LOCATION, superblock_validator());
+			bm->read_lock(sb_location, superblock_validator());
 
 		} catch (std::exception const &e) {
 			visitor.visit(superblock_corruption(e.what()));

thin-provisioning/superblock.h

@@ -139,7 +139,8 @@ namespace thin_provisioning {
 			      superblock_detail::superblock const &sb);
 
 	void check_superblock(persistent_data::block_manager::ptr bm,
-			      superblock_detail::damage_visitor &visitor);
+			      superblock_detail::damage_visitor &visitor,
+                              persistent_data::block_address sb_location = superblock_detail::SUPERBLOCK_LOCATION);
 }
 
 //----------------------------------------------------------------

thin-provisioning/thin_check.cc

@@ -76,10 +76,10 @@ namespace {
 				return 1;
 			}
 
-			block_manager::ptr bm = open_bm(path);
+			block_manager::ptr bm = open_bm(path, block_manager::READ_ONLY,
+                                                        !fs.check_opts.use_metadata_snap_);
 			output_options output_opts = !fs.quiet ? OUTPUT_NORMAL : OUTPUT_QUIET;
-			metadata_checker::ptr checker = create_base_checker(bm, fs.check_opts, output_opts);
+			error_state err = check_metadata(bm, fs.check_opts, output_opts);
-			error_state err = checker->check();
 
 			if (fs.ignore_non_fatal_errors)
 				success = (err == FATAL) ? false : true;
@@ -110,15 +110,16 @@ thin_check_cmd::thin_check_cmd()
 void
 thin_check_cmd::usage(std::ostream &out) const
 {
-	out << "Usage: " << get_name() << " [options] {device|file}" << endl
+	out << "Usage: " << get_name() << " [options] {device|file}\n"
-	    << "Options:" << endl
+	    << "Options:\n"
-	    << "  {-q|--quiet}" << endl
+	    << "  {-q|--quiet}\n"
-	    << "  {-h|--help}" << endl
+	    << "  {-h|--help}\n"
-	    << "  {-V|--version}" << endl
+	    << "  {-V|--version}\n"
-	    << "  {--override-mapping-root}" << endl
+	    << "  {-m|--metadata-snap}\n"
-	    << "  {--clear-needs-check-flag}" << endl
+	    << "  {--override-mapping-root}\n"
-	    << "  {--ignore-non-fatal-errors}" << endl
+	    << "  {--clear-needs-check-flag}\n"
-	    << "  {--skip-mappings}" << endl
+	    << "  {--ignore-non-fatal-errors}\n"
+	    << "  {--skip-mappings}\n"
 	    << "  {--super-block-only}" << endl;
 }
 
@@ -128,11 +129,12 @@ thin_check_cmd::run(int argc, char **argv)
 	int c;
 	flags fs;
 
-	char const shortopts[] = "qhV";
+	char const shortopts[] = "qhVm";
 	option const longopts[] = {
 		{ "quiet", no_argument, NULL, 'q'},
 		{ "help", no_argument, NULL, 'h'},
 		{ "version", no_argument, NULL, 'V'},
+		{ "metadata-snap", no_argument, NULL, 'm'},
 		{ "super-block-only", no_argument, NULL, 1},
 		{ "skip-mappings", no_argument, NULL, 2},
 		{ "ignore-non-fatal-errors", no_argument, NULL, 3},
@@ -155,6 +157,10 @@ thin_check_cmd::run(int argc, char **argv)
 			cout << THIN_PROVISIONING_TOOLS_VERSION << endl;
 			return 0;
 
+		case 'm':
+			fs.check_opts.set_metadata_snap();
+			break;
+
 		case 1:
 			// super-block-only
 			fs.check_opts.set_superblock_only();
@@ -186,6 +192,12 @@ thin_check_cmd::run(int argc, char **argv)
 		}
 	}
 
+	if (fs.clear_needs_check_flag_on_success && fs.check_opts.use_metadata_snap_) {
+		cerr << "--metadata-snap cannot be combined with --clear-needs-check-flag.";
+		usage(cerr);
+		exit(1);
+	}
+
 	if (argc == optind) {
 		if (!fs.quiet) {
 			cerr << "No input file provided." << endl;