[thin] Make the metadata class resposible for locating metadata snapshots

thin-provisioning/metadata.cc

@@ -112,33 +112,32 @@ metadata::metadata(block_manager<>::ptr bm, open_type ot,
 	}
 }
 
-metadata::metadata(block_manager<>::ptr bm, block_address metadata_snap)
+metadata::metadata(block_manager<>::ptr bm)
+{
+	tm_ = open_tm(bm);
+	sb_ = read_superblock(tm_->get_bm(), SUPERBLOCK_LOCATION);
+
+	open_space_maps();
+	open_btrees();
+}
+
+metadata::metadata(block_manager<>::ptr bm,
+		   boost::optional<block_address> metadata_snap)
 {
 	tm_ = open_tm(bm);
 
-	if (metadata_snap) {
+	superblock_detail::superblock actual_sb = read_superblock(bm, SUPERBLOCK_LOCATION);
-		if (metadata_snap != sb_.metadata_snap_)
+
+	if (!actual_sb.metadata_snap_)
+		throw runtime_error("no current metadata snap");
+
+	if (metadata_snap && *metadata_snap != actual_sb.metadata_snap_)
 			throw runtime_error("metadata snapshot does not match that in superblock");
 
-		sb_ = read_superblock(tm_->get_bm(), metadata_snap);
+	sb_ = read_superblock(bm, actual_sb.metadata_snap_);
 
-		// metadata snaps don't record the space maps
+	// metadata snaps don't record the space maps
-
+	open_btrees();
-	} else {
-		sb_ = read_superblock(tm_->get_bm(), SUPERBLOCK_LOCATION);
-
-		metadata_sm_ = open_metadata_sm(*tm_, &sb_.metadata_space_map_root_);
-		tm_->set_sm(metadata_sm_);
-
-		data_sm_ = open_disk_sm(*tm_, static_cast<void *>(&sb_.data_space_map_root_));
-	}
-
-	details_ = device_tree::ptr(new device_tree(*tm_, sb_.device_details_root_,
-						    device_tree_detail::device_details_traits::ref_counter()));
-	mappings_top_level_ = dev_tree::ptr(new dev_tree(*tm_, sb_.data_mapping_root_,
-							 mapping_tree_detail::mtree_ref_counter(tm_)));
-	mappings_ = mapping_tree::ptr(new mapping_tree(*tm_, sb_.data_mapping_root_,
-						       mapping_tree_detail::block_time_ref_counter(data_sm_)));
 }
 
 void
@@ -158,4 +157,22 @@ metadata::commit()
 	superblock_traits::pack(sb_, *disk);
 }
 
+void metadata::open_space_maps()
+{
+	metadata_sm_ = open_metadata_sm(*tm_, &sb_.metadata_space_map_root_);
+	tm_->set_sm(metadata_sm_);
+
+	data_sm_ = open_disk_sm(*tm_, static_cast<void *>(&sb_.data_space_map_root_));
+}
+
+void metadata::open_btrees()
+{
+	details_ = device_tree::ptr(new device_tree(*tm_, sb_.device_details_root_,
+						    device_tree_detail::device_details_traits::ref_counter()));
+	mappings_top_level_ = dev_tree::ptr(new dev_tree(*tm_, sb_.data_mapping_root_,
+							 mapping_tree_detail::mtree_ref_counter(tm_)));
+	mappings_ = mapping_tree::ptr(new mapping_tree(*tm_, sb_.data_mapping_root_,
+						       mapping_tree_detail::block_time_ref_counter(data_sm_)));
+}
+
 //----------------------------------------------------------------

thin-provisioning/metadata.h

@@ -62,7 +62,21 @@ namespace thin_provisioning {
 		metadata(block_manager<>::ptr bm, open_type ot,
 			 sector_t data_block_size = 128,
 			 block_address nr_data_blocks = 0); // Only used if CREATE
-		metadata(block_manager<>::ptr bm, block_address metadata_snap = 0);
+
+		// Ideally we'd like the metadata snap argument to be a
+		// boolean, and we'd read the snap location from the
+		// superblock.  But the command line interface for some of
+		// the tools allows the user to pass in a block, which
+		// they've retrieved from the pool status.  So we have to
+		// support 3 cases:
+		//
+		// i)   Read superblock
+		// ii)  Read the metadata snap as given in the superblock
+		// iii) Read the metadata snap given on command line, checking it matches superblock.
+		//
+		metadata(block_manager<>::ptr bm); // (i)
+		metadata(block_manager<>::ptr,
+			 boost::optional<block_address> metadata_snap); // (ii) and (iii)
 
 		void commit();
 
@@ -75,6 +89,10 @@ namespace thin_provisioning {
 		device_tree::ptr details_;
 		dev_tree::ptr mappings_top_level_;
 		mapping_tree::ptr mappings_;
+
+	private:
+		void open_space_maps();
+		void open_btrees();
 	};
 }
 

thin-provisioning/superblock.cc

@@ -181,18 +181,6 @@ namespace thin_provisioning {
 			visitor.visit(superblock_corruption(e.what()));
 		}
 	}
-
-	block_address find_metadata_snap(string const &path)
-	{
-		superblock_detail::superblock sb =
-			read_superblock(open_bm(path, block_manager<>::READ_ONLY, false), 0);
-		uint64_t ms = sb.metadata_snap_;
-
-		if (!ms)
-			throw runtime_error("no metadata snapshot found!\n");
-
-		return ms;
-	}
 }
 
 //----------------------------------------------------------------

thin-provisioning/superblock.h

@@ -137,9 +137,6 @@ namespace thin_provisioning {
 
 	void check_superblock(persistent_data::block_manager<>::ptr bm,
 			      superblock_detail::damage_visitor &visitor);
-
-	persistent_data::block_address find_metadata_snap(string const &path);
-
 }
 
 //----------------------------------------------------------------

thin-provisioning/thin_delta.cc

@@ -14,6 +14,7 @@
 #include "persistent-data/file_utils.h"
 #include "thin-provisioning/superblock.h"
 #include "thin-provisioning/mapping_tree.h"
+#include "thin-provisioning/metadata.h"
 #include "thin-provisioning/commands.h"
 
 using namespace std;
@@ -65,15 +66,16 @@ namespace local {
 	struct flags {
 		flags()
 			: verbose(false),
-			  find_metadata_snap(false) {
+			  use_metadata_snap(false) {
 		}
 
+		bool verbose;
+		bool use_metadata_snap;
+
 		boost::optional<string> dev;
 		boost::optional<uint64_t> metadata_snap;
 		boost::optional<uint64_t> snap1;
 		boost::optional<uint64_t> snap2;
-		bool verbose;
-		bool find_metadata_snap;
 	};
 
 	//--------------------------------
@@ -528,17 +530,12 @@ namespace local {
 		checked_space_map::ptr data_sm;
 
 		{
-			block_manager<>::ptr bm = open_bm(*fs.dev, block_manager<>::READ_ONLY, !fs.metadata_snap);
+			block_manager<>::ptr bm = open_bm(*fs.dev, block_manager<>::READ_ONLY, !fs.use_metadata_snap);
-			transaction_manager::ptr tm = open_tm(bm);
+			metadata::ptr md(fs.use_metadata_snap ? new metadata(bm, fs.metadata_snap) : new metadata(bm));
-
+			sb = md->sb_;
-			sb = fs.metadata_snap ? read_superblock(bm, *fs.metadata_snap) : read_superblock(bm);
-			data_sm = open_disk_sm(*tm, static_cast<void *>(&sb.data_space_map_root_));
-
-			dev_tree dtree(*tm, sb.data_mapping_root_,
-				       mapping_tree_detail::mtree_traits::ref_counter(tm));
 
 			dev_tree::key k = {*fs.snap1};
-			boost::optional<uint64_t> snap1_root = dtree.lookup(k);
+			boost::optional<uint64_t> snap1_root = md->mappings_top_level_->lookup(k);
 
 			if (!snap1_root) {
 				ostringstream out;
@@ -546,10 +543,11 @@ namespace local {
 				app.die(out.str());
 			}
 
-			single_mapping_tree snap1(*tm, *snap1_root, mapping_tree_detail::block_traits::ref_counter(tm->get_sm()));
+			single_mapping_tree snap1(*md->tm_, *snap1_root,
+						  mapping_tree_detail::block_traits::ref_counter(md->tm_->get_sm()));
 
 			k[0] = *fs.snap2;
-			boost::optional<uint64_t> snap2_root = dtree.lookup(k);
+			boost::optional<uint64_t> snap2_root = md->mappings_top_level_->lookup(k);
 
 			if (!snap2_root) {
 				ostringstream out;
@@ -557,7 +555,8 @@ namespace local {
 				app.die(out.str());
 			}
 
-			single_mapping_tree snap2(*tm, *snap2_root, mapping_tree_detail::block_traits::ref_counter(tm->get_sm()));
+			single_mapping_tree snap2(*md->tm_, *snap2_root,
+						  mapping_tree_detail::block_traits::ref_counter(md->tm_->get_sm()));
 			btree_visit_values(snap1, mr1, damage_v);
 			mr1.complete();
 
@@ -642,10 +641,9 @@ int thin_delta_main(int argc, char **argv)
 			break;
 
 		case 'm':
+			fs.use_metadata_snap = true;
 			if (optarg)
 				fs.metadata_snap = app.parse_int(optarg, "metadata snapshot block");
-			else
-				fs.find_metadata_snap = true;
 			break;
 
 		case 4:
@@ -669,9 +667,6 @@ int thin_delta_main(int argc, char **argv)
 	if (!fs.snap2)
 		app.die("--snap2 not specified.");
 
-	if (fs.find_metadata_snap)
-		fs.metadata_snap = find_metadata_snap(*fs.dev);
-
 	return delta(app, fs);
 }
 

thin-provisioning/thin_dump.cc

@@ -29,27 +29,42 @@
 #include "thin-provisioning/commands.h"
 #include "persistent-data/file_utils.h"
 
+using namespace boost;
 using namespace persistent_data;
 using namespace std;
 using namespace thin_provisioning;
 
-struct flags {
-	bool find_metadata_snap;
-	bool repair;
-};
-
 namespace {
-	int dump_(string const &path, ostream &out, string const &format, struct flags &flags,
+	// FIXME: put the path into the flags
-		  block_address metadata_snap) {
+	struct flags {
+		flags()
+			: repair(false),
+			  use_metadata_snap(false) {
+		}
+
+		bool repair;
+		bool use_metadata_snap;
+		optional<block_address> snap_location;
+	};
+
+	metadata::ptr open_metadata(string const &path, struct flags &flags) {
+		block_manager<>::ptr bm = open_bm(path, block_manager<>::READ_ONLY, !flags.use_metadata_snap);
+		metadata::ptr md(flags.use_metadata_snap ? new metadata(bm, flags.snap_location) : new metadata(bm));
+
+		return md;
+	}
+
+	int dump_(string const &path, ostream &out, string const &format, struct flags &flags) {
 		try {
-			block_manager<>::ptr bm = open_bm(path, block_manager<>::READ_ONLY, !metadata_snap);
+			metadata::ptr md = open_metadata(path, flags);
-			metadata::ptr md(new metadata(bm, metadata_snap));
 			emitter::ptr e;
 
 			if (format == "xml")
 				e = create_xml_emitter(out);
+
 			else if (format == "human_readable")
 				e = create_human_readable_emitter(out);
+
 			else {
 				cerr << "unknown format '" << format << "'" << endl;
 				exit(1);
@@ -65,13 +80,12 @@ namespace {
 		return 0;
 	}
 
-	int dump(string const &path, char const *output, string const &format, struct flags &flags,
+	int dump(string const &path, char const *output, string const &format, struct flags &flags) {
-		 block_address metadata_snap = 0) {
 		if (output) {
 			ofstream out(output);
-			return dump_(path, out, format, flags, metadata_snap);
+			return dump_(path, out, format, flags);
 		} else
-			return dump_(path, cout, format, flags, metadata_snap);
+			return dump_(path, cout, format, flags);
 	}
 
 	void usage(ostream &out, string const &cmd) {
@@ -95,7 +109,6 @@ int thin_dump_main(int argc, char **argv)
 	string format = "xml";
 	block_address metadata_snap = 0;
 	struct flags flags;
-	flags.find_metadata_snap = flags.repair = false;
 
 	const struct option longopts[] = {
 		{ "help", no_argument, NULL, 'h'},
@@ -122,16 +135,18 @@ int thin_dump_main(int argc, char **argv)
 			break;
 
 		case 'm':
+			flags.use_metadata_snap = true;
 			if (optarg) {
+				// FIXME: deprecate this option
 				metadata_snap = strtoull(optarg, &end_ptr, 10);
 				if (end_ptr == optarg) {
 					cerr << "couldn't parse <metadata_snap>" << endl;
 					usage(cerr, basename(argv[0]));
 					return 1;
 				}
-			} else
-				flags.find_metadata_snap = true;
 
+				flags.snap_location = metadata_snap;
+			}
 			break;
 
 		case 'o':
@@ -154,10 +169,7 @@ int thin_dump_main(int argc, char **argv)
 		return 1;
 	}
 
-	if (flags.find_metadata_snap)
+	return dump(argv[optind], output, format, flags);
-		metadata_snap = find_metadata_snap(argv[optind]);
-
-	return dump(argv[optind], output, format, flags, metadata_snap);
 }
 
 base::command thin_provisioning::thin_dump_cmd("thin_dump", thin_dump_main);