thin_dump now takes a --repair option

Makefile.in

@@ -19,7 +19,7 @@
 .PHONY: all
 
 PROGRAMS=\
-	thin_repair \
+	thin_check \
 	thin_dump \
 	thin_restore
 
@@ -45,8 +45,8 @@ SOURCE=\
 	xml_format.cc
 
 PROGRAM_SOURCE=\
+	thin_check.cc \
 	thin_dump.cc \
-	thin_repair.cc \
 	thin_restore.cc
 
 CXX:=@CXX@
@@ -87,7 +87,7 @@ test-programs: $(TEST_PROGRAMS)
 
 THIN_DUMP_SOURCE=$(SOURCE)
 THIN_RESTORE_SOURCE=$(SOURCE)
-THIN_REPAIR_SOURCE=\
+THIN_CHECK_SOURCE=\
 	checksum.cc \
 	endian_utils.cc \
 	error_set.cc \
@@ -103,7 +103,7 @@ THIN_REPAIR_SOURCE=\
 
 THIN_DUMP_OBJECTS=$(subst .cc,.o,$(THIN_DUMP_SOURCE))
 THIN_RESTORE_OBJECTS=$(subst .cc,.o,$(THIN_RESTORE_SOURCE))
-THIN_REPAIR_OBJECTS=$(subst .cc,.o,$(THIN_REPAIR_SOURCE))
+THIN_CHECK_OBJECTS=$(subst .cc,.o,$(THIN_CHECK_SOURCE))
 
 thin_dump: $(THIN_DUMP_OBJECTS) thin_dump.o
 	$(CXX) $(CXXFLAGS) -o $@ $+ $(LIBS) $(LIBEXPAT)
@@ -111,7 +111,7 @@ thin_dump: $(THIN_DUMP_OBJECTS) thin_dump.o
 thin_restore: $(THIN_RESTORE_OBJECTS) thin_restore.o
 	$(CXX) $(CXXFLAGS) -o $@ $+ $(LIBS) $(LIBEXPAT)
 
-thin_repair: $(THIN_REPAIR_OBJECTS) thin_repair.o
+thin_check: $(THIN_CHECK_OBJECTS) thin_check.o
 	$(CXX) $(CXXFLAGS) -o $@ $+ $(LIBS)
 
 clean:
@@ -122,11 +122,11 @@ distclean: clean
 
 install: $(PROGRAMS)
 	$(INSTALL_DIR) $(BINDIR)
-	$(INSTALL_PROGRAM) thin_repair $(BINDIR)/thin_repair
+	$(INSTALL_PROGRAM) thin_check $(BINDIR)/thin_check
 	$(INSTALL_PROGRAM) thin_dump $(BINDIR)/thin_dump
 	$(INSTALL_PROGRAM) thin_restore $(BINDIR)/thin_restore
 	$(INSTALL_DIR) $(MANPATH)/man8
-	$(INSTALL_DATA) man8/thin_repair.8 $(MANPATH)/man8/thin_repair.8
+	$(INSTALL_DATA) man8/thin_check.8 $(MANPATH)/man8/thin_check.8
 	$(INSTALL_DATA) man8/thin_dump.8 $(MANPATH)/man8/thin_dump.8
 	$(INSTALL_DATA) man8/thin_restore.8 $(MANPATH)/man8/thin_restore.8
 .PHONY: install

btree_checker.h

@@ -56,9 +56,10 @@ namespace persistent_data {
 	template <uint32_t Levels, typename ValueTraits>
 	class btree_checker : public btree<Levels, ValueTraits>::visitor {
 	public:
-		btree_checker(block_counter &counter)
+		btree_checker(block_counter &counter, bool avoid_repeated_visits = true)
 			: counter_(counter),
-			  errs_(new error_set("btree errors")) {
+			  errs_(new error_set("btree errors")),
+			  avoid_repeated_visits_(avoid_repeated_visits) {
 		}
 
 		bool visit_internal(unsigned level,
@@ -82,7 +83,7 @@ namespace persistent_data {
 			return check_leaf(level, sub_root, key, n);
 		}
 
-		boost::optional<error_set::ptr> get_errors() const {
+		error_set::ptr get_errors() const {
 			return errs_;
 		}
 
@@ -140,10 +141,13 @@ namespace persistent_data {
 
 			counter_.inc(b);
 
-			if (seen_.count(b) > 0)
+			if (avoid_repeated_visits_) {
-				return true;
+				if (seen_.count(b) > 0)
+					return true;
+
+				seen_.insert(b);
+			}
 
-			seen_.insert(b);
 			return false;
 		}
 
@@ -291,6 +295,7 @@ namespace persistent_data {
 		std::set<block_address> seen_;
 		error_set::ptr errs_;
 		boost::optional<uint64_t> last_leaf_key_[Levels];
+		bool avoid_repeated_visits_;
 	};
 }
 

error_set.cc

@@ -61,6 +61,12 @@ error_set::add_child(string const &err)
 	add_child(e);
 }
 
+bool
+error_set::empty() const
+{
+	return !children_.size();
+}
+
 //--------------------------------
 
 namespace {

error_set.h

@@ -43,6 +43,7 @@ namespace persistent_data {
 		void add_child(error_set::ptr err);
 		void add_child(boost::optional<error_set::ptr> maybe_errs);
 		void add_child(std::string const &err);
+		bool empty() const;
 
 	private:
 		std::string err_;

man8/thin_dump.8

@@ -18,6 +18,9 @@ the device-mapper target) or file.
   {-i|--input} {device|file}
   [{-f|--format} {xml|human_readable}]
 
+.B thin_dump
+  {-r|--repair}
+
 .B thin_dump
   {-h|--help}
 

metadata_checker.cc

@@ -27,27 +27,30 @@ namespace {
 	// devices having mappings defined, which can later be cross
 	// referenced with the details tree.  A separate block_counter is
 	// used to later verify the data space map.
-	class mapping_validator : public btree_checker<2, block_traits> {
+	class mapping_validator : public btree<2, block_traits>::visitor {
 	public:
 		typedef boost::shared_ptr<mapping_validator> ptr;
-		typedef btree_checker<2, block_traits> super;
+		typedef btree_checker<2, block_traits> checker;
 
 		mapping_validator(block_counter &metadata_counter, block_counter &data_counter)
-			: super(metadata_counter),
+			: checker_(metadata_counter),
-			  data_counter_(data_counter) {
+			  data_counter_(data_counter)
+		{
+		}
+
+		bool visit_internal(unsigned level,
+				    bool sub_root,
+				    optional<uint64_t> key,
+				    btree_detail::node_ref<uint64_traits> const &n) {
+			return checker_.visit_internal(level, sub_root, key, n);
 		}
 
-		// Sharing can only occur in level 1 nodes.
-		// FIXME: not true once we start having held roots.
 		bool visit_internal_leaf(unsigned level,
 					 bool sub_root,
 					 optional<uint64_t> key,
 					 btree_detail::node_ref<uint64_traits> const &n) {
 
-			bool r = super::visit_internal_leaf(level, sub_root, key, n);
+			bool r = checker_.visit_internal_leaf(level, sub_root, key, 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));
@@ -59,7 +62,7 @@ namespace {
 				bool sub_root,
 				optional<uint64_t> key,
 				btree_detail::node_ref<block_traits> const &n) {
-			bool r = super::visit_leaf(level, sub_root, key, n);
+			bool r = checker_.visit_leaf(level, sub_root, key, n);
 
 			if (r)
 				for (unsigned i = 0; i < n.get_nr_entries(); i++)
@@ -73,30 +76,46 @@ namespace {
 		}
 
 	private:
+	        checker checker_;
 		block_counter &data_counter_;
 		set<uint64_t> devices_;
 	};
 
-	class details_validator : public btree_checker<1, device_details_traits> {
+	class details_validator : public btree<1, device_details_traits>::visitor {
 	public:
 		typedef boost::shared_ptr<details_validator> ptr;
-		typedef btree_checker<1, device_details_traits> super;
+		typedef btree_checker<1, device_details_traits> checker;
 
 		details_validator(block_counter &counter)
-			: super(counter) {
+			: checker_(counter) {
+		}
+
+		bool visit_internal(unsigned level,
+				    bool sub_root,
+				    optional<uint64_t> key,
+				    btree_detail::node_ref<uint64_traits> const &n) {
+			return checker_.visit_internal(level, sub_root, key, n);
+		}
+
+		bool visit_internal_leaf(unsigned level,
+					 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);
 		}
 
 		bool visit_leaf(unsigned level,
 				bool sub_root,
 				optional<uint64_t> key,
 				btree_detail::node_ref<device_details_traits> const &n) {
-			bool r = super::visit_leaf(level, sub_root, key, n);
 
-			if (r)
+			if (!checker_.visit_leaf(level, sub_root, key, n))
-				for (unsigned i = 0; i < n.get_nr_entries(); i++)
+				return false;
-					devices_.insert(n.key_at(i));
 
-			return r;
+			for (unsigned i = 0; i < n.get_nr_entries(); i++)
+				devices_.insert(n.key_at(i));
+
+			return true;
 		}
 
 		set<uint64_t> const &get_devices() const {
@@ -104,6 +123,7 @@ namespace {
 		}
 
 	private:
+		checker checker_;
 		set<uint64_t> devices_;
 	};
 

metadata_dumper.cc

@@ -27,28 +27,48 @@ namespace {
 	class mappings_extractor : public btree<2, block_traits>::visitor {
 	public:
 		typedef boost::shared_ptr<mappings_extractor> ptr;
+		typedef btree_checker<2, block_traits> checker;
 
 		mappings_extractor(uint64_t dev_id, emitter::ptr e,
 				   space_map::ptr md_sm, space_map::ptr data_sm)
-			: dev_id_(dev_id),
+			: counter_(),
+			  checker_(counter_),
+			  dev_id_(dev_id),
 			  e_(e),
 			  md_sm_(md_sm),
 			  data_sm_(data_sm),
-			  in_range_(false) {
+			  in_range_(false),
+			  found_errors_(false) {
 		}
 
 		bool visit_internal(unsigned level, bool sub_root, boost::optional<uint64_t> key,
 				    btree_detail::node_ref<uint64_traits> const &n) {
+
+			if (!checker_.visit_internal(level, sub_root, key, n)) {
+				found_errors_ = true;
+				return false;
+			}
+
 			return (sub_root && key) ? (*key == dev_id_) : true;
 		}
 
 		bool visit_internal_leaf(unsigned level, bool sub_root, boost::optional<uint64_t> key,
 					 btree_detail::node_ref<uint64_traits> const &n) {
+			if (!checker_.visit_internal_leaf(level, sub_root, key, n)) {
+				found_errors_ = true;
+				return false;
+			}
+
 			return true;
 		}
 
 		bool visit_leaf(unsigned level, bool sub_root, boost::optional<uint64_t> maybe_key,
 				btree_detail::node_ref<block_traits> const &n) {
+			if (!checker_.visit_leaf(level, sub_root, maybe_key, n)) {
+				found_errors_ = true;
+				return false;
+			}
+
 			for (unsigned i = 0; i < n.get_nr_entries(); i++) {
 				block_time bt = n.value_at(i);
 				add_mapping(n.key_at(i), bt.block_, bt.time_);
@@ -57,11 +77,14 @@ namespace {
 			return true;
 		}
 
-
 		void visit_complete() {
 			end_mapping();
 		}
 
+		bool corruption() const {
+			return !checker_.get_errors()->empty();
+		}
+
 	private:
 		void start_mapping(uint64_t origin_block, uint64_t dest_block, uint32_t time) {
 			origin_start_ = origin_block;
@@ -97,6 +120,9 @@ namespace {
 			}
 		}
 
+		// Declaration order of counter_ and checker_ is important.
+		block_counter counter_;
+		checker checker_;
 		uint64_t dev_id_;
 		emitter::ptr e_;
 		space_map::ptr md_sm_;
@@ -105,28 +131,34 @@ namespace {
 		bool in_range_;
 		uint64_t origin_start_, dest_start_, len_;
 		uint32_t time_;
-
+		bool found_errors_;
 	};
 
 	class details_extractor : public btree<1, device_details_traits>::visitor {
 	public:
 		typedef boost::shared_ptr<details_extractor> ptr;
+		typedef btree_checker<1, device_details_traits> checker;
 
-		details_extractor() {
+		details_extractor()
+			: counter_(),
+			  checker_(counter_, false) {
 		}
 
 		bool visit_internal(unsigned level, bool sub_root, boost::optional<uint64_t> key,
 				    btree_detail::node_ref<uint64_traits> const &n) {
-			return true;
+			return checker_.visit_internal(level, sub_root, key, n);
 		}
 
 		bool visit_internal_leaf(unsigned level, bool sub_root, boost::optional<uint64_t> key,
 					 btree_detail::node_ref<uint64_traits> const &n) {
-			return true;
+			return checker_.visit_internal_leaf(level, sub_root, key, n);
 		}
 
 		bool visit_leaf(unsigned level, bool sub_root, boost::optional<uint64_t> maybe_key,
 				btree_detail::node_ref<device_details_traits> const &n) {
+			if (!checker_.visit_leaf(level, sub_root, maybe_key, n))
+				return false;
+
 			for (unsigned i = 0; i < n.get_nr_entries(); i++)
 				devices_.insert(make_pair(n.key_at(i), n.value_at(i)));
 
@@ -137,7 +169,14 @@ namespace {
 			return devices_;
 		}
 
+		bool corruption() const {
+			return !checker_.get_errors()->empty();
+		}
+
 	private:
+		// Declaration order of counter_ and checker_ is important.
+		block_counter counter_;
+		checker checker_;
 		map<uint64_t, device_details> devices_;
 	};
 }
@@ -145,13 +184,16 @@ namespace {
 //----------------------------------------------------------------
 
 void
-thin_provisioning::metadata_dump(metadata::ptr md, emitter::ptr e)
+thin_provisioning::metadata_dump(metadata::ptr md, emitter::ptr e, bool repair)
 {
 	e->begin_superblock("", md->sb_.time_, md->sb_.trans_id_, md->sb_.data_block_size_);
 
 	details_extractor::ptr de(new details_extractor);
 
 	md->details_->visit(de);
+	if (de->corruption() && !repair)
+		throw runtime_error("corruption in device details tree");
+
 	map<uint64_t, device_details> const &devs = de->get_devices();
 
 	map<uint64_t, device_details>::const_iterator it, end = devs.end();
@@ -168,6 +210,12 @@ thin_provisioning::metadata_dump(metadata::ptr md, emitter::ptr e)
 		mappings_extractor::ptr me(new mappings_extractor(dev_id, e, md->metadata_sm_, md->data_sm_));
 		md->mappings_->visit(me);
 
+		if (me->corruption() && !repair) {
+			ostringstream out;
+			out << "corruption in mappings for device " << dev_id;
+			throw runtime_error(out.str());
+		}
+
 		e->end_device();
 	}
 

metadata_dumper.h

@@ -25,7 +25,10 @@
 //----------------------------------------------------------------
 
 namespace thin_provisioning {
-	void metadata_dump(metadata::ptr md, emitter::ptr e);
+	// Set the @repair flag if your metadata is corrupt, and you'd like
+	// the dumper to do it's best to recover info.  If not set, any
+	// corruption encountered will cause an exception to be thrown.
+	void metadata_dump(metadata::ptr md, emitter::ptr e, bool repair);
 }
 
 //----------------------------------------------------------------

thin_check.cc

@@ -30,11 +30,16 @@ using namespace thin_provisioning;
 
 namespace {
 	int check(string const &path) {
-		metadata::ptr md(new metadata(path, metadata::OPEN));
+		try {
+			metadata::ptr md(new metadata(path, metadata::OPEN));
 
-		optional<error_set::ptr> maybe_errors = metadata_check(md);
+			optional<error_set::ptr> maybe_errors = metadata_check(md);
-		if (maybe_errors) {
+			if (maybe_errors) {
-			cerr << error_selector(*maybe_errors, 3);
+				cerr << error_selector(*maybe_errors, 3);
+				return 1;
+			}
+		} catch (std::exception &e) {
+			cerr << e.what();
 			return 1;
 		}
 

thin_dump.cc

@@ -33,59 +33,76 @@ using namespace thin_provisioning;
 //----------------------------------------------------------------
 
 namespace {
-	void dump(string const &path, string const &format) {
+	int dump(string const &path, string const &format, bool repair) {
-		metadata::ptr md(new metadata(path, metadata::OPEN));
+		try {
-		emitter::ptr e;
+			metadata::ptr md(new metadata(path, metadata::OPEN));
+			emitter::ptr e;
 
-		if (format == "xml")
+			if (format == "xml")
-			e = create_xml_emitter(cout);
+				e = create_xml_emitter(cout);
-		else if (format == "human_readable")
+			else if (format == "human_readable")
-			e = create_human_readable_emitter(cout);
+				e = create_human_readable_emitter(cout);
-		else {
+			else {
-			cerr << "unknown format '" << format << "'" << endl;
+				cerr << "unknown format '" << format << "'" << endl;
-			exit(1);
+				exit(1);
+			}
+
+			metadata_dump(md, e, repair);
+		} catch (std::exception &e) {
+			cerr << e.what();
+			return 1;
 		}
 
-		metadata_dump(md, e);
+		return 0;
 	}
 
 	void usage(string const &cmd) {
-		cerr << "Usage: " << cmd << " [options] {device|file}" << endl << endl;
+		cerr << "Usage: " << cmd << " [options] {device|file}" << endl << endl
-		cerr << "Options:" << endl;
+		     << "Options:" << endl
-                cerr << "  {-h|--help}" << endl;
+		     << "  {-h|--help}" << endl
-  		cerr << "  {-f|--format} {xml|human_readable}" << endl;
+		     << "  {-f|--format} {xml|human_readable}" << endl
-		cerr << "  {-i|--input} {xml|human_readable} input_file" << endl;
+		     << "  {-i|--input} {xml|human_readable} input_file" << endl
-		cerr << "  {-V|--version}" << endl;
+		     << "  {-r|--repair}" << endl
+		     << "  {-V|--version}" << endl;
 	}
 }
 
 int main(int argc, char **argv)
 {
 	int c;
+	bool repair = false;
 	const char shortopts[] = "hf:i:V";
 	string filename, format = "xml";
 	const struct option longopts[] = {
 		{ "help", no_argument, NULL, 'h'},
 		{ "format", required_argument, NULL, 'f' },
 		{ "input", required_argument, NULL, 'i'},
+		{ "repair", no_argument, NULL, 'r'},
 		{ "version", no_argument, NULL, 'V'},
 		{ NULL, no_argument, NULL, 0 }
 	};
 
 	while ((c = getopt_long(argc, argv, shortopts, longopts, NULL)) != -1) {
 		switch(c) {
-			case 'h':
+		case 'h':
-				usage(basename(argv[0]));
+			usage(basename(argv[0]));
-				return 0;
+			return 0;
-			case 'f':
+
-				format = optarg;
+		case 'f':
-				break;
+			format = optarg;
-			case 'i':
+			break;
-				filename = optarg;
+
-				break;
+		case 'i':
-			case 'V':
+			filename = optarg;
-				cerr << THIN_PROVISIONING_TOOLS_VERSION << endl;
+			break;
-				return 0;
+
+		case 'r':
+			repair = true;
+			break;
+
+		case 'V':
+			cerr << THIN_PROVISIONING_TOOLS_VERSION << endl;
+			return 0;
 		}
 	}
 
@@ -99,8 +116,7 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
-	dump(filename, format);
+	return dump(filename, format, repair);
-	return 0;
 }
 
 //----------------------------------------------------------------

thin_restore.cc

@@ -35,22 +35,21 @@ using namespace thin_provisioning;
 //----------------------------------------------------------------
 
 namespace {
-	void restore(string const &backup_file, string const &dev) {
+	int restore(string const &backup_file, string const &dev) {
-		// FIXME: hard coded
+		try {
-		block_address const NR_BLOCKS = 100000;
+			// FIXME: hard coded
+			block_address const NR_BLOCKS = 100000;
 
-		metadata::ptr md(new metadata(dev, metadata::CREATE, 128, NR_BLOCKS));
+			metadata::ptr md(new metadata(dev, metadata::CREATE, 128, NR_BLOCKS));
-		emitter::ptr restorer = create_restore_emitter(md);
+			emitter::ptr restorer = create_restore_emitter(md);
-		ifstream in(backup_file.c_str(), ifstream::in);
+			ifstream in(backup_file.c_str(), ifstream::in);
-		// FIXME: 
-		//try {
 			parse_xml(in, restorer);
-#if 0
+		} catch (std::exception &e) {
-		} catch (...) {
+			cerr << e.what();
-			in.close();
+			return 1;
-			throw;
 		}
-#endif
+
+		return 0;
 	}
 
 	void usage(string const &cmd) {
@@ -108,8 +107,7 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
-	restore(input, output);
+	return restore(input, output);
-	return 0;
 
 }