From 9f6546f621b4276b5eae77d73d7f2c1fc46fa4b3 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Wed, 19 Feb 2014 16:08:05 +0000 Subject: [PATCH 01/31] put space map checking back in --- persistent-data/data-structures/array.h | 17 +---- .../data-structures/btree_counter.h | 14 ++++ persistent-data/space-maps/careful_alloc.cc | 4 ++ persistent-data/space-maps/core.h | 3 + persistent-data/space-maps/disk.cc | 41 +++++++++++ persistent-data/space-maps/recursive.cc | 4 ++ persistent-data/space_map.h | 1 + thin-provisioning/thin_check.cc | 70 ++++++++++++++++++- 8 files changed, 135 insertions(+), 19 deletions(-) diff --git a/persistent-data/data-structures/array.h b/persistent-data/data-structures/array.h index 6c76bf2..faa800e 100644 --- a/persistent-data/data-structures/array.h +++ b/persistent-data/data-structures/array.h @@ -288,23 +288,8 @@ namespace persistent_data { } } - //-------------------------------- - - struct ablock_counter { - ablock_counter(block_counter &bc) - : bc_(bc) { - } - - void visit(btree_detail::node_location const &loc, block_address b) { - bc_.inc(b); - } - - private: - block_counter &bc_; - }; - void count_metadata_blocks(block_counter &bc) const { - ablock_counter vc(bc); + block_address_counter vc(bc); count_btree_blocks(block_tree_, bc, vc); } diff --git a/persistent-data/data-structures/btree_counter.h b/persistent-data/data-structures/btree_counter.h index 2f00173..ed7b845 100644 --- a/persistent-data/data-structures/btree_counter.h +++ b/persistent-data/data-structures/btree_counter.h @@ -1,6 +1,7 @@ #ifndef PERSISTENT_DATA_DATA_STRUCTURES_BTREE_COUNTER_H #define PERSISTENT_DATA_DATA_STRUCTURES_BTREE_COUNTER_H +#include "persistent-data/data-structures/btree.h" #include "persistent-data/block_counter.h" //---------------------------------------------------------------- @@ -65,6 +66,19 @@ namespace persistent_data { } }; + struct block_address_counter { + block_address_counter(block_counter &bc) + : bc_(bc) { + } + + void visit(btree_detail::node_location const &loc, block_address b) { + bc_.inc(b); + } + + private: + block_counter &bc_; + }; + // Counts how many times each metadata block is referenced in the // tree. Blocks already referenced in the block counter are not // walked. This walk should only be done once you're sure the tree diff --git a/persistent-data/space-maps/careful_alloc.cc b/persistent-data/space-maps/careful_alloc.cc index a25653c..5bca0ca 100644 --- a/persistent-data/space-maps/careful_alloc.cc +++ b/persistent-data/space-maps/careful_alloc.cc @@ -89,6 +89,10 @@ namespace { sm_->iterate(it); } + virtual void count_metadata(block_counter &bc) const { + sm_->count_metadata(bc); + } + virtual size_t root_size() const { return sm_->root_size(); } diff --git a/persistent-data/space-maps/core.h b/persistent-data/space-maps/core.h index d3d0e45..4547f12 100644 --- a/persistent-data/space-maps/core.h +++ b/persistent-data/space-maps/core.h @@ -105,6 +105,9 @@ namespace persistent_data { throw std::runtime_error("'extend' not implemented"); } + void count_metadata(block_counter &bc) const { + } + // FIXME: meaningless, but this class is only used for testing size_t root_size() const { return 0; diff --git a/persistent-data/space-maps/disk.cc b/persistent-data/space-maps/disk.cc index df3ed43..388450f 100644 --- a/persistent-data/space-maps/disk.cc +++ b/persistent-data/space-maps/disk.cc @@ -22,6 +22,7 @@ #include "persistent-data/space-maps/careful_alloc.h" #include "persistent-data/data-structures/btree_damage_visitor.h" +#include "persistent-data/data-structures/btree_counter.h" #include "persistent-data/checksum.h" #include "persistent-data/endian_utils.h" #include "persistent-data/math_utils.h" @@ -222,6 +223,7 @@ namespace { public: typedef boost::shared_ptr ptr; + virtual void count_metadata(block_counter &bc) const = 0; virtual void resize(block_address nr_indexes) = 0; virtual index_entry find_ie(block_address b) const = 0; virtual void save_ie(block_address b, struct index_entry ie) = 0; @@ -405,6 +407,13 @@ namespace { } } + virtual void count_metadata(block_counter &bc) const { + indexes_->count_metadata(bc); + + noop_value_counter vc; + count_btree_blocks(ref_counts_, bc, vc); + } + virtual size_t root_size() const { return sizeof(sm_root_disk); } @@ -554,6 +563,29 @@ namespace { bitmaps_(tm, root, index_entry_traits::ref_counter()) { } + //-------------------------------- + + struct index_entry_counter { + index_entry_counter(block_counter &bc) + : bc_(bc) { + } + + void visit(btree_detail::node_location const &loc, index_entry const &ie) { + if (ie.blocknr_ != 0) + bc_.inc(ie.blocknr_); + } + + private: + block_counter &bc_; + }; + + virtual void count_metadata(block_counter &bc) const { + index_entry_counter vc(bc); + count_btree_blocks(bitmaps_, bc, vc); + } + + //-------------------------------- + virtual void resize(block_address nr_entries) { // No op } @@ -612,6 +644,15 @@ namespace { load_ies(); } + virtual void count_metadata(block_counter &bc) const { + for (unsigned i = 0; i < entries_.size(); i++) { + block_address b = entries_[i].blocknr_; + + if (b != 0) + bc.inc(b); + } + } + virtual void resize(block_address nr_indexes) { entries_.resize(nr_indexes); } diff --git a/persistent-data/space-maps/recursive.cc b/persistent-data/space-maps/recursive.cc index 6533c20..76976a8 100644 --- a/persistent-data/space-maps/recursive.cc +++ b/persistent-data/space-maps/recursive.cc @@ -176,6 +176,10 @@ namespace { sm_->iterate(it); } + virtual void count_metadata(block_counter &bc) const { + sm_->count_metadata(bc); + } + virtual size_t root_size() const { cant_recurse("root_size"); recursing_const_lock lock(*this); diff --git a/persistent-data/space_map.h b/persistent-data/space_map.h index 5395c90..064ff76 100644 --- a/persistent-data/space_map.h +++ b/persistent-data/space_map.h @@ -141,6 +141,7 @@ namespace persistent_data { public: typedef boost::shared_ptr ptr; + virtual void count_metadata(block_counter &bc) const = 0; virtual size_t root_size() const = 0; virtual void copy_root(void *dest, size_t len) const = 0; }; diff --git a/thin-provisioning/thin_check.cc b/thin-provisioning/thin_check.cc index ea5e56e..1202876 100644 --- a/thin-provisioning/thin_check.cc +++ b/thin-provisioning/thin_check.cc @@ -24,7 +24,9 @@ #include "base/error_state.h" #include "base/nested_output.h" +#include "persistent-data/data-structures/btree_counter.h" #include "persistent-data/space-maps/core.h" +#include "persistent-data/space-maps/disk.h" #include "persistent-data/file_utils.h" #include "thin-provisioning/device_tree.h" #include "thin-provisioning/mapping_tree.h" @@ -209,9 +211,71 @@ namespace { } } - return combine_errors(sb_rep.get_error(), - combine_errors(mapping_rep.get_error(), - dev_rep.get_error())); + error_state mplus_err = combine_errors(sb_rep.get_error(), + combine_errors(mapping_rep.get_error(), + dev_rep.get_error())); + + // if we're checking everything, and there were no errors, + // then we should check the space maps too. + if (fs.check_device_tree && fs.check_mapping_tree_level2 && mplus_err == NO_ERROR) { + out << "checking space map counts" << end_message(); + { + nested_output::nest _ = out.push(); + block_counter bc; + + // Count the device tree + { + noop_value_counter vc; + device_tree dtree(tm, sb.device_details_root_, + device_tree_detail::device_details_traits::ref_counter()); + count_btree_blocks(dtree, bc, vc); + } + + // Count the mapping tree + { + noop_value_counter vc; + mapping_tree mtree(tm, sb.data_mapping_root_, + mapping_tree_detail::block_traits::ref_counter(tm->get_sm())); + count_btree_blocks(mtree, bc, vc); + } + + // Count the metadata space map + { + persistent_space_map::ptr metadata_sm = + open_metadata_sm(tm, static_cast(&sb.metadata_space_map_root_)); + metadata_sm->count_metadata(bc); + } + + // Count the data space map + { + persistent_space_map::ptr data_sm = + open_disk_sm(tm, static_cast(&sb.data_space_map_root_)); + data_sm->count_metadata(bc); + } + + // Finally we need to check the metadata + // space map agrees with the counts we've + // just calculated. + { + persistent_space_map::ptr metadata_sm = + open_metadata_sm(tm, static_cast(&sb.metadata_space_map_root_)); + for (unsigned b = 0; b < metadata_sm->get_nr_blocks(); b++) { + ref_t c_actual = metadata_sm->get_count(b); + ref_t c_expected = bc.get_count(b); + + if (c_actual != c_expected) { + out << "metadata reference counts differ for block " << b + << ", expected " << c_expected + << ", but got " << c_actual + << end_message(); + mplus_err = combine_errors(mplus_err, FATAL); + } + } + } + } + } + + return mplus_err; } int check(string const &path, flags fs) { From 50d1a3e7d2cab4a80fa9ab45cbf21cb8243326bc Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 20 Feb 2014 16:36:03 +0000 Subject: [PATCH 02/31] [thin_check] inc superblock and metadata snap in space map checking --- thin-provisioning/thin_check.cc | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/thin-provisioning/thin_check.cc b/thin-provisioning/thin_check.cc index 1202876..7baf2c8 100644 --- a/thin-provisioning/thin_check.cc +++ b/thin-provisioning/thin_check.cc @@ -223,6 +223,18 @@ namespace { nested_output::nest _ = out.push(); block_counter bc; + // Count the superblock + bc.inc(superblock_detail::SUPERBLOCK_LOCATION); + + // Count the metadata snap, if present + if (sb.metadata_snap_ != superblock_detail::SUPERBLOCK_LOCATION) { + bc.inc(sb.metadata_snap_); + + superblock_detail::superblock snap = read_superblock(bm, sb.metadata_snap_); + bc.inc(snap.data_mapping_root_); + bc.inc(snap.device_details_root_); + } + // Count the device tree { noop_value_counter vc; From 92345b4b64cfb6d2d38620dc1c8ac0210d9c105f Mon Sep 17 00:00:00 2001 From: Alexander Holler Date: Sat, 15 Nov 2014 13:45:14 +0100 Subject: [PATCH 03/31] [persistent-data/space_map.h] Make destructor for space_map_detail::damage public The compiler is unable to create a default desctructor for the derived class missing_counts if the virtual destructor for the class damage is private. This fixes compilation bugs with CXXFLAGS=-std=gnu++11 together with gcc 4.8.3 and boost 1.55. --- persistent-data/space_map.h | 1 + 1 file changed, 1 insertion(+) diff --git a/persistent-data/space_map.h b/persistent-data/space_map.h index 5395c90..823b4ba 100644 --- a/persistent-data/space_map.h +++ b/persistent-data/space_map.h @@ -119,6 +119,7 @@ namespace persistent_data { namespace space_map_detail { class damage { + public: virtual ~damage() {} }; From baa70ecfe4a465b3cbba204a025d859603bd2f35 Mon Sep 17 00:00:00 2001 From: Alexander Holler Date: Sat, 15 Nov 2014 14:17:22 +0100 Subject: [PATCH 04/31] [caching/hint_array.cc] Fix ambigious shared_ptr (C++11) Class shared_ptr exist in the namespace std for C++11 as well as in boost. Explicitly use the one from boost in order to be compatible. This fixes compilation bugs with CXXFLAGS=-std=gnu++11 together with gcc 4.8.3 and boost 1.55. --- caching/hint_array.cc | 46 +++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/caching/hint_array.cc b/caching/hint_array.cc index cb9f2f1..7fccb82 100644 --- a/caching/hint_array.cc +++ b/caching/hint_array.cc @@ -38,16 +38,16 @@ namespace { xx(4); template - shared_ptr mk_array(transaction_manager &tm) { + boost::shared_ptr mk_array(transaction_manager &tm) { typedef hint_traits traits; typedef array ha; - shared_ptr r = typename ha::ptr(new ha(tm, typename traits::ref_counter())); + boost::shared_ptr r = typename ha::ptr(new ha(tm, typename traits::ref_counter())); return r; } - shared_ptr mk_array(transaction_manager &tm, uint32_t width) { + boost::shared_ptr mk_array(transaction_manager &tm, uint32_t width) { switch (width) { #define xx(n) case n: return mk_array(tm) @@ -58,15 +58,15 @@ namespace { } // never get here - return shared_ptr(); + return boost::shared_ptr(); } //-------------------------------- template - shared_ptr - downcast_array(shared_ptr base) { - shared_ptr a = dynamic_pointer_cast(base); + boost::shared_ptr + downcast_array(boost::shared_ptr base) { + boost::shared_ptr a = dynamic_pointer_cast(base); if (!a) throw runtime_error("internal error: couldn't cast hint array"); @@ -76,16 +76,16 @@ namespace { //-------------------------------- template - shared_ptr mk_array(transaction_manager &tm, block_address root, unsigned nr_entries) { + boost::shared_ptr mk_array(transaction_manager &tm, block_address root, unsigned nr_entries) { typedef hint_traits traits; typedef array ha; - shared_ptr r = typename ha::ptr(new ha(tm, typename traits::ref_counter(), root, nr_entries)); + boost::shared_ptr r = typename ha::ptr(new ha(tm, typename traits::ref_counter(), root, nr_entries)); return r; } - shared_ptr mk_array(transaction_manager &tm, uint32_t width, block_address root, unsigned nr_entries) { + boost::shared_ptr mk_array(transaction_manager &tm, uint32_t width, block_address root, unsigned nr_entries) { switch (width) { #define xx(n) case n: return mk_array(tm, root, nr_entries) all_widths @@ -95,21 +95,21 @@ namespace { } // never get here - return shared_ptr(); + return boost::shared_ptr(); } //-------------------------------- template - void get_hint(shared_ptr base, unsigned index, vector &data) { + void get_hint(boost::shared_ptr base, unsigned index, vector &data) { typedef hint_traits traits; typedef array ha; - shared_ptr a = downcast_array(base); + boost::shared_ptr a = downcast_array(base); data = a->get(index); } - void get_hint_(uint32_t width, shared_ptr base, unsigned index, vector &data) { + void get_hint_(uint32_t width, boost::shared_ptr base, unsigned index, vector &data) { switch (width) { #define xx(n) case n: return get_hint(base, index, data) all_widths @@ -120,15 +120,15 @@ namespace { //-------------------------------- template - void set_hint(shared_ptr base, unsigned index, vector const &data) { + void set_hint(boost::shared_ptr base, unsigned index, vector const &data) { typedef hint_traits traits; typedef array ha; - shared_ptr a = downcast_array(base); + boost::shared_ptr a = downcast_array(base); a->set(index, data); } - void set_hint_(uint32_t width, shared_ptr base, + void set_hint_(uint32_t width, boost::shared_ptr base, unsigned index, vector const &data) { switch (width) { #define xx(n) case n: return set_hint(base, index, data) @@ -140,15 +140,15 @@ namespace { //-------------------------------- template - void grow(shared_ptr base, unsigned new_nr_entries, vector const &value) { + void grow(boost::shared_ptr base, unsigned new_nr_entries, vector const &value) { typedef hint_traits traits; typedef array ha; - shared_ptr a = downcast_array(base); + boost::shared_ptr a = downcast_array(base); a->grow(new_nr_entries, value); } - void grow_(uint32_t width, shared_ptr base, + void grow_(uint32_t width, boost::shared_ptr base, unsigned new_nr_entries, vector const &value) { switch (width) { @@ -194,17 +194,17 @@ namespace { }; template - void walk_hints(shared_ptr base, hint_visitor &hv, damage_visitor &dv) { + void walk_hints(boost::shared_ptr base, hint_visitor &hv, damage_visitor &dv) { typedef hint_traits traits; typedef array ha; - shared_ptr a = downcast_array(base); + boost::shared_ptr a = downcast_array(base); value_adapter vv(hv); ll_damage_visitor ll(dv); a->visit_values(vv, ll); } - void walk_hints_(uint32_t width, shared_ptr base, + void walk_hints_(uint32_t width, boost::shared_ptr base, hint_visitor &hv, damage_visitor &dv) { switch (width) { #define xx(n) case n: walk_hints(base, hv, dv); break From 691ad882613ad061172095d881bb36b221a64d93 Mon Sep 17 00:00:00 2001 From: Alexander Holler Date: Sat, 15 Nov 2014 14:45:11 +0100 Subject: [PATCH 05/31] [caching/hint_array.cc] Fix ambigious array (C++11) Template array exist in the namespace persistent_data as well as in std of C++11. Explicitly use the one from persistent_data. This fixes compilation bugs with CXXFLAGS=-std=gnu++11 together with gcc 4.8.3 and boost 1.55. --- caching/hint_array.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/caching/hint_array.cc b/caching/hint_array.cc index 7fccb82..66a3a27 100644 --- a/caching/hint_array.cc +++ b/caching/hint_array.cc @@ -40,7 +40,7 @@ namespace { template boost::shared_ptr mk_array(transaction_manager &tm) { typedef hint_traits traits; - typedef array ha; + typedef persistent_data::array ha; boost::shared_ptr r = typename ha::ptr(new ha(tm, typename traits::ref_counter())); @@ -78,7 +78,7 @@ namespace { template boost::shared_ptr mk_array(transaction_manager &tm, block_address root, unsigned nr_entries) { typedef hint_traits traits; - typedef array ha; + typedef persistent_data::array ha; boost::shared_ptr r = typename ha::ptr(new ha(tm, typename traits::ref_counter(), root, nr_entries)); @@ -103,7 +103,7 @@ namespace { template void get_hint(boost::shared_ptr base, unsigned index, vector &data) { typedef hint_traits traits; - typedef array ha; + typedef persistent_data::array ha; boost::shared_ptr a = downcast_array(base); data = a->get(index); @@ -122,7 +122,7 @@ namespace { template void set_hint(boost::shared_ptr base, unsigned index, vector const &data) { typedef hint_traits traits; - typedef array ha; + typedef persistent_data::array ha; boost::shared_ptr a = downcast_array(base); a->set(index, data); @@ -142,7 +142,7 @@ namespace { template void grow(boost::shared_ptr base, unsigned new_nr_entries, vector const &value) { typedef hint_traits traits; - typedef array ha; + typedef persistent_data::array ha; boost::shared_ptr a = downcast_array(base); a->grow(new_nr_entries, value); @@ -196,7 +196,7 @@ namespace { template void walk_hints(boost::shared_ptr base, hint_visitor &hv, damage_visitor &dv) { typedef hint_traits traits; - typedef array ha; + typedef persistent_data::array ha; boost::shared_ptr a = downcast_array(base); value_adapter vv(hv); From b56aec4d962f5f5ff17f2f1e42d0968c85aff098 Mon Sep 17 00:00:00 2001 From: Alexander Holler Date: Sat, 15 Nov 2014 15:42:58 +0100 Subject: [PATCH 06/31] [unit-tests/bloom_filter_t.cc] Fix ambigious uniform_int_distribution (C++11) uniform_int_distribution exist in the namespace boost as well as in std of C++11. Use the one provided by boost. This fixes compilation bugs with CXXFLAGS=-std=gnu++11 together with gcc 4.8.3 and boost 1.55. --- unit-tests/bloom_filter_t.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unit-tests/bloom_filter_t.cc b/unit-tests/bloom_filter_t.cc index 032eb48..13c2418 100644 --- a/unit-tests/bloom_filter_t.cc +++ b/unit-tests/bloom_filter_t.cc @@ -40,7 +40,7 @@ namespace { using namespace boost::random; - uniform_int_distribution uniform_dist(0, max); + boost::random::uniform_int_distribution uniform_dist(0, max); while (r.size() < count) { block_address b = uniform_dist(rng_); From 8e921580554ed91e84bb68ea32a8c2ea47ad6ff3 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 18 Nov 2014 16:03:03 +0000 Subject: [PATCH 07/31] [thin_trim] first code drop. No testing done as yet. --- Makefile.in | 3 + thin-provisioning/commands.h | 1 + thin-provisioning/thin_trim.cc | 198 +++++++++++++++++++++++++++++++++ 3 files changed, 202 insertions(+) create mode 100644 thin-provisioning/thin_trim.cc diff --git a/Makefile.in b/Makefile.in index ff2a2a3..fe666f5 100644 --- a/Makefile.in +++ b/Makefile.in @@ -86,6 +86,7 @@ SOURCE=\ thin-provisioning/thin_repair.cc \ thin-provisioning/thin_restore.cc \ thin-provisioning/thin_rmap.cc \ + thin-provisioning/thin_trim.cc \ thin-provisioning/xml_format.cc CC:=@CC@ @@ -171,6 +172,7 @@ install: bin/pdata_tools ln -s -f pdata_tools $(BINDIR)/thin_repair ln -s -f pdata_tools $(BINDIR)/thin_restore ln -s -f pdata_tools $(BINDIR)/thin_rmap + ln -s -f pdata_tools $(BINDIR)/thin_trim ln -s -f pdata_tools $(BINDIR)/thin_metadata_size ln -s -f pdata_tools $(BINDIR)/era_check ln -s -f pdata_tools $(BINDIR)/era_dump @@ -186,6 +188,7 @@ install: bin/pdata_tools $(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_trim.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 diff --git a/thin-provisioning/commands.h b/thin-provisioning/commands.h index 5635665..de63e53 100644 --- a/thin-provisioning/commands.h +++ b/thin-provisioning/commands.h @@ -13,6 +13,7 @@ namespace thin_provisioning { extern base::command thin_restore_cmd; extern base::command thin_repair_cmd; extern base::command thin_rmap_cmd; + extern base::command thin_trim_cmd; extern base::command thin_metadata_size_cmd; } diff --git a/thin-provisioning/thin_trim.cc b/thin-provisioning/thin_trim.cc new file mode 100644 index 0000000..ae57092 --- /dev/null +++ b/thin-provisioning/thin_trim.cc @@ -0,0 +1,198 @@ +#include +#include +#include +#include + +#undef BLOCK_SIZE + +#include "thin-provisioning/commands.h" +#include "metadata.h" +#include "version.h" + +using namespace persistent_data; +using namespace std; +using namespace thin_provisioning; + +//---------------------------------------------------------------- + +namespace { + void confirm_pool_is_not_active() { + cout << "The pool must *not* be active when running this tool.\n" + << "Do you wish to continue? [Y/N]\n" + << endl; + + string input; + cin >> input; + if (input != "Y") + exit(0); + } + + class discard_emitter { + public: + discard_emitter(string const &data_dev, unsigned block_size, uint64_t nr_blocks) + : fd_(open_dev(data_dev, block_size * nr_blocks)), + block_size_(block_size) { + } + + ~discard_emitter() { + ::close(fd_); + } + + void emit(block_address b, block_address e) { + uint64_t range[2]; + + range[0] = block_to_byte(b); + range[1] = block_to_byte(e) - range[0]; + + if (ioctl(fd_, BLKDISCARD, &range)) + throw runtime_error("discard ioctl failed"); + } + + private: + static int open_dev(string const &data_dev, uint64_t expected_size) { + int r, fd; + uint64_t blksize; + struct stat info; + + fd = ::open(data_dev.c_str(), O_WRONLY); + if (fd < 0) { + ostringstream out; + out << "Couldn't open data device '" << data_dev << "'"; + throw runtime_error(out.str()); + } + + try { + r = fstat(fd, &info); + if (r) + throw runtime_error("Couldn't stat data device"); + + if (!S_ISBLK(info.st_mode)) + throw runtime_error("Data device is not a block device"); + + r = ioctl(fd, BLKGETSIZE64, &blksize); + if (r) + throw runtime_error("Couldn't get data device size"); + + if (blksize != (expected_size << 9)) + throw runtime_error("Data device is not the expected size"); + + } catch (...) { + ::close(fd); + throw; + } + + return fd; + } + + uint64_t block_to_byte(block_address b) { + return (b * block_size_) << 9; + } + + int fd_; + unsigned block_size_; + }; + + class trim_visitor : public space_map_detail::visitor { + public: + trim_visitor(discard_emitter &e) + : emitter_(e) { + } + + virtual void visit(space_map_detail::missing_counts const &mc) { + throw std::runtime_error("corrupt metadata, please use thin_check for details"); + } + + virtual void visit(block_address b, uint32_t count) { + if (last_visited_ && (b > *last_visited_ + 1)) + emitter_.emit(*last_visited_ + 1, b); + + last_visited_ = b; + } + + private: + discard_emitter &emitter_; + boost::optional last_visited_; + }; + + int trim(string const &metadata_dev, string const &data_dev) { + // We can trim any block that has zero count in the data + // space map. + metadata md(metadata_dev, 0); + + if (!md.data_sm_->get_nr_free()) + return 0; + + discard_emitter de(data_dev, md.sb_.data_block_size_, + md.data_sm_->get_nr_blocks()); + trim_visitor tv(de); + + confirm_pool_is_not_active(); + md.data_sm_->visit(tv); + + return 0; + } + + void usage(ostream &out, string const &cmd) { + out << "Usage: " << cmd << " [options] {device|file}\n" + << "Options:\n" + << " {--pool-inactive}\n" + << " {-h|--help}\n" + << " {-V|--version}" << endl; + } + + struct flags { + boost::optional metadata_dev; + boost::optional data_dev; + }; +} + +//---------------------------------------------------------------- + +int thin_trim_main(int argc, char **argv) +{ + int c; + flags fs; + const char shortopts[] = "hV"; + + const struct option longopts[] = { + { "help", no_argument, NULL, 'h' }, + { "version", no_argument, NULL, 'V' }, + { "metadata-dev", required_argument, NULL, 0 }, + { "data-dev", required_argument, NULL, 1 }, + { "pool-inactive", no_argument, NULL, 2 }, + { NULL, no_argument, NULL, 0 } + }; + + while ((c = getopt_long(argc, argv, shortopts, longopts, NULL)) != -1) { + switch(c) { + case 0: + fs.metadata_dev = optarg; + break; + + case 1: + fs.data_dev = optarg; + break; + + case 'h': + usage(cout, basename(argv[0])); + return 0; + + case 'V': + cout << THIN_PROVISIONING_TOOLS_VERSION << endl; + return 0; + + default: + usage(cerr, basename(argv[0])); + return 1; + } + } + + if (!fs.metadata_dev || !fs.data_dev) { + usage(cerr, basename(argv[0])); + return 1; + } + + return trim(*fs.metadata_dev, *fs.data_dev); +} + +//---------------------------------------------------------------- From dd9bd206c65f877b86ad33c02a543272c5acf876 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 16 Jan 2015 10:12:30 +0000 Subject: [PATCH 08/31] Old glibc doesn't provide these macros, so we have to define them. Signed-off-by: Mikulas Patocka --- base/endian_utils.h | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/base/endian_utils.h b/base/endian_utils.h index 3ad75ae..82b5b59 100644 --- a/base/endian_utils.h +++ b/base/endian_utils.h @@ -25,6 +25,26 @@ //---------------------------------------------------------------- +/* An old glic doesn't provide these macros */ +#if !defined(htole16) || !defined(le16toh) || !defined(htole32) || !defined(le32toh) || !defined(htole64) || !defined(le64toh) +#include +#if __BYTE_ORDER == __LITTLE_ENDIAN +#define htole16(x) (x) +#define le16toh(x) (x) +#define htole32(x) (x) +#define le32toh(x) (x) +#define htole64(x) (x) +#define le64toh(x) (x) +#else +#define htole16(x) __bswap_16(x) +#define le16toh(x) __bswap_16(x) +#define htole32(x) __bswap_32(x) +#define le32toh(x) __bswap_32(x) +#define htole64(x) __bswap_64(x) +#define le64toh(x) __bswap_64(x) +#endif +#endif + namespace base { // These are just little wrapper types to make the compiler From 150a3c486d90ac18ed2d190b3a97f5163ef127fd Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 16 Jan 2015 10:13:36 +0000 Subject: [PATCH 09/31] Fix these errors: caching/superblock.cc:306: error: reference to 'validator' is ambiguous caching/superblock.cc:271: error: candidates are: namespace validator { } ./block-cache/block_cache.h:22: error: class bcache::validator caching/superblock.cc:316: error: reference to 'validator' is ambiguous caching/superblock.cc:271: error: candidates are: namespace validator { } ./block-cache/block_cache.h:22: error: class bcache::validator Signed-off-by: Mikulas Patocka --- caching/superblock.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/caching/superblock.cc b/caching/superblock.cc index 7ecc916..4089eee 100644 --- a/caching/superblock.cc +++ b/caching/superblock.cc @@ -302,8 +302,9 @@ namespace validator { superblock caching::read_superblock(block_manager<>::ptr bm, block_address location) { + using namespace validator; superblock sb; - block_manager<>::read_ref r = bm->read_lock(location, validator::mk_v()); + block_manager<>::read_ref r = bm->read_lock(location, mk_v()); superblock_disk const *sbd = reinterpret_cast(r.data()); superblock_traits::unpack(*sbd, sb); @@ -313,7 +314,8 @@ caching::read_superblock(block_manager<>::ptr bm, block_address location) void caching::write_superblock(block_manager<>::ptr bm, superblock const &sb, block_address location) { - block_manager<>::write_ref w = bm->superblock_zero(location, validator::mk_v()); + using namespace validator; + block_manager<>::write_ref w = bm->superblock_zero(location, mk_v()); superblock_traits::pack(sb, *reinterpret_cast(w.data())); } From fe64da2c7cffcbc1bdc229c686ba412c3461ac64 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 16 Jan 2015 10:14:28 +0000 Subject: [PATCH 10/31] Fix these errors: thin-provisioning/thin_pool.cc:206: error: reference to 'sector_t' is ambiguous ./thin-provisioning/metadata.h:40: error: candidates are: typedef uint64_t thin_provisioning::sector_t ./block-cache/block_cache.h:20: error: typedef uint64_t bcache::sector_t thin-provisioning/thin_pool.cc:206: error: reference to 'sector_t' is ambiguous ./thin-provisioning/metadata.h:40: error: candidates are: typedef uint64_t thin_provisioning::sector_t ./block-cache/block_cache.h:20: error: typedef uint64_t bcache::sector_t thin-provisioning/thin_pool.cc:206: error: 'sector_t' does not name a type Signed-off-by: Mikulas Patocka --- thin-provisioning/thin_pool.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/thin-provisioning/thin_pool.cc b/thin-provisioning/thin_pool.cc index 53940e3..1596c90 100644 --- a/thin-provisioning/thin_pool.cc +++ b/thin-provisioning/thin_pool.cc @@ -203,7 +203,7 @@ thin_pool::get_nr_free_data_blocks() const return md_->data_sm_->get_nr_free(); } -sector_t +thin_provisioning::sector_t thin_pool::get_data_block_size() const { return md_->sb_.data_block_size_; From bd2c0df22681294c01359897332dd69ba2bd6f6f Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 16 Jan 2015 10:15:01 +0000 Subject: [PATCH 11/31] Fix this error: persistent-data/data-structures/bloom_filter.cc:10: error: integer constant is too large for 'unsigned long' type Signed-off-by: Mikulas Patocka --- persistent-data/data-structures/bloom_filter.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/persistent-data/data-structures/bloom_filter.cc b/persistent-data/data-structures/bloom_filter.cc index a250835..08516e1 100644 --- a/persistent-data/data-structures/bloom_filter.cc +++ b/persistent-data/data-structures/bloom_filter.cc @@ -7,7 +7,7 @@ using namespace persistent_data; //---------------------------------------------------------------- namespace { - static const uint64_t m1 = 0x9e37fffffffc0001UL; + static const uint64_t m1 = 0x9e37fffffffc0001ULL; static const unsigned bits = 18; static uint32_t hash1(block_address const &b) { From f25e0ca6d342e5f0d0dfb15f11bbfec59460b06f Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 16 Jan 2015 10:15:55 +0000 Subject: [PATCH 12/31] g++-4.2 and older doesn't accept binary constants. Signed-off-by: Mikulas Patocka --- persistent-data/space-maps/disk.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/persistent-data/space-maps/disk.cc b/persistent-data/space-maps/disk.cc index c9ca9ea..bc017c3 100644 --- a/persistent-data/space-maps/disk.cc +++ b/persistent-data/space-maps/disk.cc @@ -112,7 +112,7 @@ namespace { ref_t b1 = test_bit_le(bits, b * 2); ref_t b2 = test_bit_le(bits, b * 2 + 1); ref_t result = b2 ? 1 : 0; - result |= b1 ? 0b10 : 0; + result |= b1 ? 2 : 0; return result; } @@ -165,7 +165,7 @@ namespace { ref_t b1 = test_bit_le(bits, b * 2); ref_t b2 = test_bit_le(bits, b * 2 + 1); ref_t result = b2 ? 1 : 0; - result |= b1 ? 0b10 : 0; + result |= b1 ? 2 : 0; it(offset + b, result); } } From 50341faa64b2b252385305efd930ce78aca590e4 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 16 Jan 2015 10:18:21 +0000 Subject: [PATCH 13/31] Fix these errors: unit-tests/array_block_t.cc:38: error: using 'typename' outside of template unit-tests/array_block_t.cc:39: error: using 'typename' outside of template unit-tests/array_block_t.cc:40: error: using 'typename' outside of template Signed-off-by: Mikulas Patocka --- unit-tests/array_block_t.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/unit-tests/array_block_t.cc b/unit-tests/array_block_t.cc index 2ba122d..9ce3823 100644 --- a/unit-tests/array_block_t.cc +++ b/unit-tests/array_block_t.cc @@ -35,9 +35,9 @@ using namespace testing; namespace { uint64_t MAX_VALUE = 1000ull; block_address const NR_BLOCKS = 1024; - typedef typename bcache::noop_validator noop_validator; - typedef typename block_manager<>::read_ref read_ref; - typedef typename block_manager<>::write_ref write_ref; + typedef bcache::noop_validator noop_validator; + typedef block_manager<>::read_ref read_ref; + typedef block_manager<>::write_ref write_ref; // FIXME: lift to utils? class simple_ref_counter { From ef517035f114d35b7402ff03783c3017d549348a Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 16 Jan 2015 10:19:25 +0000 Subject: [PATCH 14/31] The file boost/random/uniform_int_distribution.hpp was introduced in boost version 1.47. If we have older Boost, use random numbers from libc. Signed-off-by: Mikulas Patocka --- unit-tests/bloom_filter_t.cc | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/unit-tests/bloom_filter_t.cc b/unit-tests/bloom_filter_t.cc index 13c2418..bf44ffa 100644 --- a/unit-tests/bloom_filter_t.cc +++ b/unit-tests/bloom_filter_t.cc @@ -5,8 +5,16 @@ #include "persistent-data/data-structures/array_block.h" #include "test_utils.h" +#include + +#if BOOST_VERSION >= 104700 +#define HAVE_RANDOM_UNIFORM_INT_DISTRIBUTION +#endif + #include +#ifdef HAVE_RANDOM_UNIFORM_INT_DISTRIBUTION #include +#endif #include #include #include @@ -40,10 +48,16 @@ namespace { using namespace boost::random; +#ifdef HAVE_RANDOM_UNIFORM_INT_DISTRIBUTION boost::random::uniform_int_distribution uniform_dist(0, max); +#endif while (r.size() < count) { +#ifdef HAVE_RANDOM_UNIFORM_INT_DISTRIBUTION block_address b = uniform_dist(rng_); +#else + block_address b = random() % max; +#endif r.insert(b); } @@ -75,7 +89,9 @@ namespace { space_map::ptr sm_; transaction_manager tm_; +#ifdef HAVE_RANDOM_UNIFORM_INT_DISTRIBUTION boost::random::mt19937 rng_; +#endif }; } @@ -312,3 +328,4 @@ TEST_F(BloomFilterTests, false_positives_over_multiple_eras) } //---------------------------------------------------------------- + From f1130198e14d99f7f5805e661367e985b7f8a4f8 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 16 Jan 2015 10:48:19 +0000 Subject: [PATCH 15/31] include libgen.h in application.cc for the declaration of basename. Unfortunately it defines basename as a macro, so also change member function name from basename() to get_basename(). --- base/application.cc | 5 +++-- base/application.h | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/base/application.cc b/base/application.cc index 6012e06..9e1f0dd 100644 --- a/base/application.cc +++ b/base/application.cc @@ -1,5 +1,6 @@ #include "base/application.h" +#include #include #include @@ -11,7 +12,7 @@ using namespace std; int application::run(int argc, char **argv) { - string cmd = basename(argv[0]); + string cmd = get_basename(argv[0]); if (cmd == string("pdata_tools")) { argc--; @@ -49,7 +50,7 @@ application::usage() } std::string -application::basename(std::string const &path) const +application::get_basename(std::string const &path) const { char buffer[PATH_MAX + 1]; diff --git a/base/application.h b/base/application.h index 8585129..d01eb36 100644 --- a/base/application.h +++ b/base/application.h @@ -41,7 +41,7 @@ namespace base { private: void usage(); - std::string basename(std::string const &path) const; + std::string get_basename(std::string const &path) const; std::list cmds_; }; From 25b4b526f495551811778d2273bc32359e735925 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 16 Jan 2015 12:54:09 +0000 Subject: [PATCH 16/31] Introduce error_string() as a portable replacement for strerror_r() --- Makefile.in | 2 ++ caching/cache_check.cc | 6 ++---- configure.ac | 8 ++++++++ era/era_check.cc | 6 ++---- persistent-data/block.tcc | 8 +++----- 5 files changed, 17 insertions(+), 13 deletions(-) diff --git a/Makefile.in b/Makefile.in index fe666f5..8ce0f17 100644 --- a/Makefile.in +++ b/Makefile.in @@ -29,6 +29,7 @@ SOURCE=\ base/base64.cc \ base/endian_utils.cc \ base/error_state.cc \ + base/error_string.cc \ base/progress_monitor.cc \ base/xml_utils.cc \ block-cache/block_cache.cc \ @@ -98,6 +99,7 @@ CFLAGS+=-g -Wall -O3 CXXFLAGS+=-g -Wall -fno-strict-aliasing CXXFLAGS+=@CXXOPTIMISE_FLAG@ CXXFLAGS+=@CXXDEBUG_FLAG@ +CXXFLAGS+=@CXX_STRERROR_FLAG@ INCLUDES+=-I$(TOP_BUILDDIR) -I$(TOP_DIR) -I$(TOP_DIR)/thin-provisioning LIBS:=-lstdc++ -laio -lexpat INSTALL:=@INSTALL@ diff --git a/caching/cache_check.cc b/caching/cache_check.cc index 400b214..ba750d5 100644 --- a/caching/cache_check.cc +++ b/caching/cache_check.cc @@ -13,6 +13,7 @@ #include #include "base/error_state.h" +#include "base/error_string.h" #include "base/nested_output.h" #include "caching/commands.h" #include "caching/metadata.h" @@ -202,10 +203,7 @@ namespace { int r = ::stat(path.c_str(), &info); if (r) { ostringstream msg; - char buffer[128], *ptr; - - ptr = ::strerror_r(errno, buffer, sizeof(buffer)); - msg << path << ": " << ptr; + msg << path << ": " << error_string(errno); throw runtime_error(msg.str()); } diff --git a/configure.ac b/configure.ac index f15906b..3e6c6a9 100644 --- a/configure.ac +++ b/configure.ac @@ -42,6 +42,13 @@ AC_PROG_MAKE_SET AC_PROG_MKDIR_P AC_PROG_INSTALL +################################################################ +dnl -- Checks for functions. +AC_FUNC_STRERROR_R +if test x$ac_cv_func_strerror_r_char_p = xyes; then + CXX_STRERROR_FLAG="-DSTRERROR_R_CHAR_P" +fi + ################################################################################ dnl -- Prefix is /usr by default, the exec_prefix default is setup later AC_PREFIX_DEFAULT(/usr) @@ -136,6 +143,7 @@ VERSION_PATCHLEVEL=`echo "$VER" | $AWK -F '[[(.]]' '{print $3}'` ################################################################ AC_SUBST(CXXDEBUG_FLAG) AC_SUBST(CXXOPTIMISE_FLAG) +AC_SUBST(CXX_STRERROR_FLAG) AC_SUBST(INSTALL) AC_SUBST(prefix) AC_SUBST(RELEASE_DATE) diff --git a/era/era_check.cc b/era/era_check.cc index fed199e..d64999d 100644 --- a/era/era_check.cc +++ b/era/era_check.cc @@ -13,6 +13,7 @@ #include #include "base/error_state.h" +#include "base/error_string.h" #include "base/nested_output.h" #include "era/commands.h" #include "era/writeset_tree.h" @@ -186,10 +187,7 @@ namespace { int r = ::stat(path.c_str(), &info); if (r) { ostringstream msg; - char buffer[128], *ptr; - - ptr = ::strerror_r(errno, buffer, sizeof(buffer)); - msg << path << ": " << ptr; + msg << path << ": " << error_string(errno);; throw runtime_error(msg.str()); } diff --git a/persistent-data/block.tcc b/persistent-data/block.tcc index f2f3a7a..529f7af 100644 --- a/persistent-data/block.tcc +++ b/persistent-data/block.tcc @@ -18,13 +18,14 @@ #include "block.h" +#include "base/error_string.h" + #include #include #include #include #include #include -#include #include #include @@ -44,11 +45,8 @@ namespace { // FIXME: introduce a new exception for this, or at least lift this // to exception.h void syscall_failed(char const *call) { - char buffer[128]; - char *msg = strerror_r(errno, buffer, sizeof(buffer)); - ostringstream out; - out << "syscall '" << call << "' failed: " << msg; + out << "syscall '" << call << "' failed: " << base::error_string(errno);; throw runtime_error(out.str()); } From 408b38a0f8d1da9ec7cb8f0d62ac2bc3e63ba123 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Sat, 17 Jan 2015 10:22:57 +0000 Subject: [PATCH 17/31] Forgot error_string.h/cc from previous commit --- base/error_string.cc | 39 +++++++++++++++++++++++++++++++++++++++ base/error_string.h | 16 ++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 base/error_string.cc create mode 100644 base/error_string.h diff --git a/base/error_string.cc b/base/error_string.cc new file mode 100644 index 0000000..6cb02c4 --- /dev/null +++ b/base/error_string.cc @@ -0,0 +1,39 @@ +#include "base/error_string.h" + +#include +#include + +#include + +using namespace std; + +//---------------------------------------------------------------- + +#ifdef STRERROR_R_CHAR_P + +string base::error_string(int err) +{ + char *ptr; + char buffer[128]; + + ptr = strerror_r(errno, buffer, sizeof(buffer)); + return string(ptr); +} + +#else + +string base::error_string(int err) +{ + int r; + char buffer[128]; + + r = strerror_r(errno, buffer, sizeof(buffer)); + if (r) + throw runtime_error("strerror_r failed"); + + return string(buffer); +} + +#endif + +//---------------------------------------------------------------- diff --git a/base/error_string.h b/base/error_string.h new file mode 100644 index 0000000..dd7549a --- /dev/null +++ b/base/error_string.h @@ -0,0 +1,16 @@ +#ifndef BASE_ERROR_STRING_H +#define BASE_ERROR_STRING_H + +#include + +//---------------------------------------------------------------- + +namespace base { + // There are a couple of version of strerror_r kicking around, so + // we wrap it. + std::string error_string(int err); +} + +//---------------------------------------------------------------- + +#endif From c6ae25417bad8ddb4240f573dd599e4995886ee2 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Sat, 17 Jan 2015 11:45:09 +0000 Subject: [PATCH 18/31] Add missing include to thin_trim --- thin-provisioning/thin_trim.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/thin-provisioning/thin_trim.cc b/thin-provisioning/thin_trim.cc index ae57092..0118b62 100644 --- a/thin-provisioning/thin_trim.cc +++ b/thin-provisioning/thin_trim.cc @@ -2,6 +2,7 @@ #include #include #include +#include #undef BLOCK_SIZE From 4d7da25859333c1756971d48367cb9b831c310fa Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 10 Mar 2015 08:52:13 +0000 Subject: [PATCH 19/31] Add thin_trim man page --- man8/thin_trim.8 | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 man8/thin_trim.8 diff --git a/man8/thin_trim.8 b/man8/thin_trim.8 new file mode 100644 index 0000000..de702f0 --- /dev/null +++ b/man8/thin_trim.8 @@ -0,0 +1,34 @@ +.TH THIN_TRIM 8 "Thin Provisioning Tools" "Red Hat, Inc." \" -*- nroff -*- +.SH NAME +thin_trim \- Issue discard requests for free pool space (offline tool). + +.SH SYNOPSIS +.B thin_trim +.RB [ options ] +.I {device|file} + +.SH DESCRIPTION +.B thin_trim +sends discard requests to the pool device for unprovisioned areas. It is an offline tool, +.B do not run it while the pool is active +. + +.SH OPTIONS +.IP "\fB\-\-pool-inactive\fP" +Indicates you are aware the pool should be inactive. Suppresses a warning message and prompt. + +.IP "\fB\-h, \-\-help\fP" +Print help and exit. + +.IP "\fB\-V, \-\-version\fP" +Output version information and exit. + +.SH SEE ALSO +.B thin_dump(8) +.B thin_repair(8) +.B thin_restore(8) +.B thin_rmap(8) +.B thin_metadata_size(8) + +.SH AUTHOR +Joe Thornber From 45422dbf7ae2f41ba169e6a68657745fa1309a99 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 24 Mar 2015 13:36:45 +0000 Subject: [PATCH 20/31] [thin_delta] Mappings were being missed off from the tail of a device --- thin-provisioning/thin_delta.cc | 81 ++++++++++++++++++++++----------- 1 file changed, 55 insertions(+), 26 deletions(-) diff --git a/thin-provisioning/thin_delta.cc b/thin-provisioning/thin_delta.cc index da3a890..b0c460d 100644 --- a/thin-provisioning/thin_delta.cc +++ b/thin-provisioning/thin_delta.cc @@ -123,51 +123,63 @@ namespace local { class mapping_recorder { public: mapping_recorder() { - reset_range(); + no_range(); } void visit(btree_path const &path, mapping_tree_detail::block_time const &bt) { record(path[0], bt.block_); } + void complete() { + if (range_in_progress()) { + push_range(); + no_range(); + } + } + mapping_deque const &get_mappings() const { return mappings_; } private: - void reset_range() { + void no_range() { obegin_ = oend_ = 0; dbegin_ = dend_ = 0; } - void record(uint64_t oblock, uint64_t dblock) { - if (obegin_ == oend_) { - // We're starting a new range - obegin_ = oblock; - oend_ = obegin_; - - dbegin_ = dblock; - dend_ = dbegin_; - - } else { - if (oblock != oend_ || dblock != dend_) { - // Emit the current range ... - push_mapping(obegin_, dbegin_, oend_ - obegin_); - - obegin_ = oblock; - oend_ = obegin_; - - dbegin_ = dblock; - dend_ = dbegin_; - } - } - + void inc_range() { oend_++; dend_++; } - void push_mapping(uint64_t vbegin, uint64_t dbegin, uint64_t len) { - mappings_.push_back(mapping(vbegin, dbegin, len)); + void begin_range(uint64_t oblock, uint64_t dblock) { + obegin_ = oend_ = oblock; + dbegin_ = dend_ = dblock; + inc_range(); + } + + bool range_in_progress() { + return oend_ != obegin_; + } + + bool continues_range(uint64_t oblock, uint64_t dblock) { + return (oblock == oend_) && (dblock == dend_); + } + + void push_range() { + mapping m(obegin_, dbegin_, oend_ - obegin_); + mappings_.push_back(m); + } + + void record(uint64_t oblock, uint64_t dblock) { + if (!range_in_progress()) + begin_range(oblock, dblock); + + else if (!continues_range(oblock, dblock)) { + push_range(); + begin_range(oblock, dblock); + } else + inc_range(); } uint64_t obegin_, oend_; @@ -437,6 +449,20 @@ namespace local { } } + while (left_it != left.end()) { + left_mapping = *left_it++; + + if (left_mapping.len_) + e.left_only(left_mapping.vbegin_, left_mapping.dbegin_, left_mapping.len_); + } + + while (right_it != right.end()) { + right_mapping = *right_it++; + + if (right_mapping.len_) + e.right_only(right_mapping.vbegin_, right_mapping.dbegin_, right_mapping.len_); + } + e.complete(); } @@ -476,7 +502,10 @@ namespace local { single_mapping_tree snap2(*tm, *snap2_root, mapping_tree_detail::block_traits::ref_counter(tm->get_sm())); btree_visit_values(snap1, mr1, damage_v); + mr1.complete(); + btree_visit_values(snap2, mr2, damage_v); + mr2.complete(); } if (fs.verbose) { From 055b64a9c04dc78a62ec78f5d03b1a50a8c12f54 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 24 Mar 2015 14:08:21 +0000 Subject: [PATCH 21/31] Add a simple man page for thin_delta --- man8/thin_delta.8 | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 man8/thin_delta.8 diff --git a/man8/thin_delta.8 b/man8/thin_delta.8 new file mode 100644 index 0000000..1ebfcbe --- /dev/null +++ b/man8/thin_delta.8 @@ -0,0 +1,47 @@ +.TH THIN_DELTA 8 "Thin Provisioning Tools" "Red Hat, Inc." \" -*- nroff -*- +.SH NAME +thin_delta \- Print the differences in the mappings between two thin devices. + +.SH SYNOPSIS +.B thin_delta +.RB [ options ] +.I {device|file} + +.SH DESCRIPTION +.B thin_delta +allows you to compare the mappings in two thin volumes (snapshots allow common blocks between thin volumes). +. + +.SH OPTIONS +.IP "\fB\-\-thin1, \-\-snap1\fP" +The numeric identifier for the first thin volume to diff. + +.IP "\fB\-\-thin1, \-\-snap1\fP" +The numeric identifier for the second thin volume to diff. + +.IP "\fB\-m, \-\-metadata\-snap\fP [block#]" + +If you want to get information out of a live pool then you will need +to take a metadata snapshot and use this switch. In order for the +information to be meaningful you need to ensure the thin volumes +you're examining are not changing (eg, do not activate those thins). + +.IP "\fB\-\-verbose" +Provide extra information on the mappings. + +.IP "\fB\-h, \-\-help\fP" +Print help and exit. + +.IP "\fB\-V, \-\-version\fP" +Output version information and exit. + +.SH SEE ALSO +.B thin_dump(8) +.B thin_repair(8) +.B thin_restore(8) +.B thin_rmap(8) +.B thin_trim(8) +.B thin_metadata_size(8) + +.SH AUTHOR +Joe Thornber From 182be112fa8ba915127ce44fa7cb85f1d46e56ef Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 24 Mar 2015 14:09:36 +0000 Subject: [PATCH 22/31] Add thin_delta to the build --- CHANGES | 5 +++++ Makefile.in | 2 ++ 2 files changed, 7 insertions(+) diff --git a/CHANGES b/CHANGES index 7e22fdc..3de75c1 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +v0.5 +==== + +- thin_delta, thin_trim + v0.4 ==== diff --git a/Makefile.in b/Makefile.in index 8ce0f17..eaff125 100644 --- a/Makefile.in +++ b/Makefile.in @@ -170,6 +170,7 @@ install: bin/pdata_tools ln -s -f pdata_tools $(BINDIR)/cache_repair ln -s -f pdata_tools $(BINDIR)/cache_restore ln -s -f pdata_tools $(BINDIR)/thin_check + ln -s -f pdata_tools $(BINDIR)/thin_delta ln -s -f pdata_tools $(BINDIR)/thin_dump ln -s -f pdata_tools $(BINDIR)/thin_repair ln -s -f pdata_tools $(BINDIR)/thin_restore @@ -186,6 +187,7 @@ install: bin/pdata_tools $(INSTALL_DATA) man8/cache_repair.8 $(MANPATH)/man8 $(INSTALL_DATA) man8/cache_restore.8 $(MANPATH)/man8 $(INSTALL_DATA) man8/thin_check.8 $(MANPATH)/man8 + $(INSTALL_DATA) man8/thin_delta.8 $(MANPATH)/man8 $(INSTALL_DATA) man8/thin_dump.8 $(MANPATH)/man8 $(INSTALL_DATA) man8/thin_repair.8 $(MANPATH)/man8 $(INSTALL_DATA) man8/thin_restore.8 $(MANPATH)/man8 From 0e72f772d0bec63360fc0e69fa25147528b977ed Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Wed, 25 Mar 2015 10:09:39 +0000 Subject: [PATCH 23/31] [thin_delta] Add superblock and diff tags --- thin-provisioning/thin_delta.cc | 116 ++++++++++++++++++++++++++------ 1 file changed, 96 insertions(+), 20 deletions(-) diff --git a/thin-provisioning/thin_delta.cc b/thin-provisioning/thin_delta.cc index b0c460d..196887d 100644 --- a/thin-provisioning/thin_delta.cc +++ b/thin-provisioning/thin_delta.cc @@ -6,9 +6,11 @@ #include "version.h" +#include "base/indented_stream.h" #include "persistent-data/data-structures/btree_damage_visitor.h" #include "persistent-data/run.h" #include "persistent-data/space-maps/core.h" +#include "persistent-data/space-maps/disk.h" #include "persistent-data/file_utils.h" #include "thin-provisioning/superblock.h" #include "thin-provisioning/mapping_tree.h" @@ -27,8 +29,11 @@ namespace local { } void usage(ostream &out) { - out << "Usage: " << cmd_ << " [options] --snap1 --snap2 \n" + out << "Usage: " << cmd_ << " [options] \n" << "Options:\n" + << " {--thin1, --snap1}\n" + << " {--thin2, --snap2}\n" + << " {-m, --metadata-snap}\n" << " {--verbose}\n" << " {-h|--help}\n" << " {-V|--version}" << endl; @@ -201,7 +206,7 @@ namespace local { class diff_emitter { public: - diff_emitter(ostream &out) + diff_emitter(indented_stream &out) : out_(out) { } @@ -212,18 +217,22 @@ namespace local { virtual void complete() = 0; protected: - ostream &out() { + void indent() { + out_.indent(); + } + + indented_stream &out() { return out_; } private: - ostream &out_; + indented_stream &out_; }; class simple_emitter : public diff_emitter { public: - simple_emitter(ostream &out) + simple_emitter(indented_stream &out) : diff_emitter(out) { } @@ -272,6 +281,7 @@ namespace local { if (!current_type_) return; + indent(); switch (*current_type_) { case LEFT_ONLY: out() << "\n"; } void right_only(uint64_t vbegin, uint64_t dbegin, uint64_t len) { begin_block(RIGHT_ONLY); - out() << " \n"; } void blocks_differ(uint64_t vbegin, uint64_t left_dbegin, uint64_t right_dbegin, uint64_t len) { begin_block(DIFFER); - out() << " \n"; @@ -328,7 +341,8 @@ namespace local { void blocks_same(uint64_t vbegin, uint64_t dbegin, uint64_t len) { begin_block(SAME); - out() << " \n"; } @@ -359,6 +373,7 @@ namespace local { } void open(block_type t) { + indent(); switch (t) { case LEFT_ONLY: out() << "\n"; @@ -376,24 +391,27 @@ namespace local { out() << "\n"; break; } + out().inc(); } void close(block_type t) { + out().dec(); + indent(); switch (t) { case LEFT_ONLY: - out() << "\n\n"; + out() << "\n"; break; case RIGHT_ONLY: - out() << "\n\n"; + out() << "\n"; break; case DIFFER: - out() << "\n\n"; + out() << "\n"; break; case SAME: - out() << "\n\n"; + out() << "\n"; break; } @@ -466,16 +484,59 @@ namespace local { e.complete(); } + // FIXME: duplication with xml_format + void begin_superblock(indented_stream &out, + string const &uuid, + uint64_t time, + uint64_t trans_id, + uint32_t data_block_size, + uint64_t nr_data_blocks, + boost::optional metadata_snap) { + out.indent(); + out << "\n"; + out.inc(); + } + + void end_superblock(indented_stream &out) { + out.dec(); + out.indent(); + out << "\n"; + } + + void begin_diff(indented_stream &out, uint64_t snap1, uint64_t snap2) { + out.indent(); + out << "\n"; + out.inc(); + } + + void end_diff(indented_stream &out) { + out.dec(); + out.indent(); + out << "\n"; + } + void delta_(application &app, flags const &fs) { mapping_recorder mr1; mapping_recorder mr2; damage_visitor damage_v; + superblock_detail::superblock sb; + checked_space_map::ptr data_sm; { block_manager<>::ptr bm = open_bm(*fs.dev); transaction_manager::ptr tm = open_tm(bm); - superblock_detail::superblock sb = read_superblock(bm); + sb = read_superblock(bm); + data_sm = open_disk_sm(*tm, static_cast(&sb.data_space_map_root_)); dev_tree dtree(*tm, sb.data_mapping_root_, mapping_tree_detail::mtree_traits::ref_counter(tm)); @@ -508,13 +569,26 @@ namespace local { mr2.complete(); } + indented_stream is(cout); + begin_superblock(is, "", sb.time_, + sb.trans_id_, + sb.data_block_size_, + data_sm->get_nr_blocks(), + sb.metadata_snap_ ? + boost::optional(sb.metadata_snap_) : + boost::optional()); + begin_diff(is, *fs.snap1, *fs.snap2); + if (fs.verbose) { - verbose_emitter e(cout); + verbose_emitter e(is); dump_diff(mr1.get_mappings(), mr2.get_mappings(), e); } else { - simple_emitter e(cout); + simple_emitter e(is); dump_diff(mr1.get_mappings(), mr2.get_mappings(), e); } + + end_diff(is); + end_superblock(is); } int delta(application &app, flags const &fs) { @@ -541,13 +615,15 @@ int thin_delta_main(int argc, char **argv) flags fs; local::application app(basename(argv[0])); - char const shortopts[] = "hV"; + char const shortopts[] = "hVm"; option const longopts[] = { { "help", no_argument, NULL, 'h' }, { "version", no_argument, NULL, 'V' }, + { "thin1", required_argument, NULL, 1 }, { "snap1", required_argument, NULL, 1 }, + { "thin2", required_argument, NULL, 2 }, { "snap2", required_argument, NULL, 2 }, - { "metadata-snap", no_argument, NULL, 3 }, + { "metadata-snap", no_argument, NULL, 'm' }, { "verbose", no_argument, NULL, 4 } }; @@ -569,7 +645,7 @@ int thin_delta_main(int argc, char **argv) fs.snap2 = app.parse_snap(optarg); break; - case 3: + case 'm': abort(); break; From cc44652cc3c13a3cfc31631688c080bb43e5a5a2 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Wed, 25 Mar 2015 11:10:18 +0000 Subject: [PATCH 24/31] [thin_delta] support metadata snapshots --- thin-provisioning/thin_delta.cc | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/thin-provisioning/thin_delta.cc b/thin-provisioning/thin_delta.cc index 196887d..7cb7d75 100644 --- a/thin-provisioning/thin_delta.cc +++ b/thin-provisioning/thin_delta.cc @@ -33,7 +33,7 @@ namespace local { << "Options:\n" << " {--thin1, --snap1}\n" << " {--thin2, --snap2}\n" - << " {-m, --metadata-snap}\n" + << " {-m, --metadata-snap} [block#]\n" << " {--verbose}\n" << " {-h|--help}\n" << " {-V|--version}" << endl; @@ -45,13 +45,13 @@ namespace local { exit(1); } - uint64_t parse_snap(string const &str) { + uint64_t parse_int(string const &str, string const &desc) { try { return boost::lexical_cast(str); } catch (...) { ostringstream out; - out << "Couldn't parse snapshot designator: '" << str << "'"; + out << "Couldn't parse " << desc << ": '" << str << "'"; die(out.str()); } @@ -68,6 +68,7 @@ namespace local { } boost::optional dev; + boost::optional metadata_snap; boost::optional snap1; boost::optional snap2; bool verbose; @@ -535,7 +536,7 @@ namespace local { block_manager<>::ptr bm = open_bm(*fs.dev); transaction_manager::ptr tm = open_tm(bm); - sb = read_superblock(bm); + sb = fs.metadata_snap ? read_superblock(bm, *fs.metadata_snap) : read_superblock(bm); data_sm = open_disk_sm(*tm, static_cast(&sb.data_space_map_root_)); dev_tree dtree(*tm, sb.data_mapping_root_, @@ -638,15 +639,15 @@ int thin_delta_main(int argc, char **argv) return 0; case 1: - fs.snap1 = app.parse_snap(optarg); + fs.snap1 = app.parse_int(optarg, "thin id 1"); break; case 2: - fs.snap2 = app.parse_snap(optarg); + fs.snap2 = app.parse_int(optarg, "thin id 2"); break; case 'm': - abort(); + fs.metadata_snap = app.parse_int(optarg, "metadata snapshot block"); break; case 4: From f581f34be868201a1ea955178fd4e6ad932c9219 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 7 Apr 2015 12:10:38 +0100 Subject: [PATCH 25/31] add comment explaining mtree_traits --- thin-provisioning/mapping_tree.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/thin-provisioning/mapping_tree.h b/thin-provisioning/mapping_tree.h index be3bcf8..d417b47 100644 --- a/thin-provisioning/mapping_tree.h +++ b/thin-provisioning/mapping_tree.h @@ -54,6 +54,9 @@ namespace thin_provisioning { transaction_manager::ptr tm_; }; + // This value type is itself a tree containing mappings. + // Used when manipulating the top level of the mapping + // tree. struct mtree_traits { typedef base::le64 disk_type; typedef uint64_t value_type; From 7f643b70507e4c188253e951eccf613a901aa79d Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 7 Apr 2015 12:16:46 +0100 Subject: [PATCH 26/31] [thin] Use specific damage visitors to improve error messages. There's now a damage visitor for dev_trees, mapping_trees and single_mapping_trees. --- thin-provisioning/mapping_tree.cc | 56 +++++++++++++++++++++++++++---- 1 file changed, 49 insertions(+), 7 deletions(-) diff --git a/thin-provisioning/mapping_tree.cc b/thin-provisioning/mapping_tree.cc index ea3292f..421c8fe 100644 --- a/thin-provisioning/mapping_tree.cc +++ b/thin-provisioning/mapping_tree.cc @@ -141,9 +141,9 @@ namespace { } }; - class ll_damage_visitor { + class dev_tree_damage_visitor { public: - ll_damage_visitor(damage_visitor &v) + dev_tree_damage_visitor(damage_visitor &v) : v_(v) { } @@ -158,14 +158,56 @@ namespace { break; default: - // shouldn't get here. - throw std::runtime_error("ll_damage_visitor: path too long"); + throw std::runtime_error("dev_tree_damage_visitor: path too long"); } } private: damage_visitor &v_; }; + + class mapping_tree_damage_visitor { + public: + mapping_tree_damage_visitor(damage_visitor &v) + : v_(v) { + } + + virtual void visit(btree_path const &path, btree_detail::damage const &d) { + switch (path.size()) { + case 0: + v_.visit(missing_devices(d.desc_, d.lost_keys_)); + break; + + default: + throw std::runtime_error("mapping_tree_damage_visitor: path too long"); + } + } + + private: + damage_visitor &v_; + }; + + class single_mapping_tree_damage_visitor { + public: + single_mapping_tree_damage_visitor(damage_visitor &v) + : v_(v) { + } + + virtual void visit(btree_path const &path, btree_detail::damage const &d) { + switch (path.size()) { + case 0: + v_.visit(missing_mappings(d.desc_, path[0], d.lost_keys_)); + break; + + default: + throw std::runtime_error("single_mapping_tree_damage_visitor: path too long"); + } + } + + private: + damage_visitor &v_; + }; + } void @@ -173,7 +215,7 @@ thin_provisioning::walk_mapping_tree(dev_tree const &tree, mapping_tree_detail::device_visitor &dev_v, mapping_tree_detail::damage_visitor &dv) { - ll_damage_visitor ll_dv(dv); + dev_tree_damage_visitor ll_dv(dv); btree_visit_values(tree, dev_v, ll_dv); } @@ -190,7 +232,7 @@ thin_provisioning::walk_mapping_tree(mapping_tree const &tree, mapping_tree_detail::mapping_visitor &mv, mapping_tree_detail::damage_visitor &dv) { - ll_damage_visitor ll_dv(dv); + mapping_tree_damage_visitor ll_dv(dv); btree_visit_values(tree, mv, ll_dv); } @@ -207,7 +249,7 @@ thin_provisioning::walk_mapping_tree(single_mapping_tree const &tree, mapping_tree_detail::mapping_visitor &mv, mapping_tree_detail::damage_visitor &dv) { - ll_damage_visitor ll_dv(dv); + single_mapping_tree_damage_visitor ll_dv(dv); btree_visit_values(tree, mv, ll_dv); } From 20079f3d28b525e76d7b0fbf2222dd0ee974c1cb Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 7 Apr 2015 13:31:45 +0100 Subject: [PATCH 27/31] Pass tm's by reference --- thin-provisioning/thin_check.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/thin-provisioning/thin_check.cc b/thin-provisioning/thin_check.cc index 339f005..453ac4d 100644 --- a/thin-provisioning/thin_check.cc +++ b/thin-provisioning/thin_check.cc @@ -250,7 +250,7 @@ namespace { // Count the device tree { noop_value_counter vc; - device_tree dtree(tm, sb.device_details_root_, + device_tree dtree(*tm, sb.device_details_root_, device_tree_detail::device_details_traits::ref_counter()); count_btree_blocks(dtree, bc, vc); } @@ -258,7 +258,7 @@ namespace { // Count the mapping tree { noop_value_counter vc; - mapping_tree mtree(tm, sb.data_mapping_root_, + mapping_tree mtree(*tm, sb.data_mapping_root_, mapping_tree_detail::block_traits::ref_counter(tm->get_sm())); count_btree_blocks(mtree, bc, vc); } @@ -266,14 +266,14 @@ namespace { // Count the metadata space map { persistent_space_map::ptr metadata_sm = - open_metadata_sm(tm, static_cast(&sb.metadata_space_map_root_)); + open_metadata_sm(*tm, static_cast(&sb.metadata_space_map_root_)); metadata_sm->count_metadata(bc); } // Count the data space map { persistent_space_map::ptr data_sm = - open_disk_sm(tm, static_cast(&sb.data_space_map_root_)); + open_disk_sm(*tm, static_cast(&sb.data_space_map_root_)); data_sm->count_metadata(bc); } @@ -282,7 +282,7 @@ namespace { // just calculated. { persistent_space_map::ptr metadata_sm = - open_metadata_sm(tm, static_cast(&sb.metadata_space_map_root_)); + open_metadata_sm(*tm, static_cast(&sb.metadata_space_map_root_)); for (unsigned b = 0; b < metadata_sm->get_nr_blocks(); b++) { ref_t c_actual = metadata_sm->get_count(b); ref_t c_expected = bc.get_count(b); From 34df640d8d8ef3851310c076f2024479533bd41d Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Wed, 8 Apr 2015 12:32:00 +0100 Subject: [PATCH 28/31] [metadata space map] index bitmap root wasn't being counted in count_metadata() --- persistent-data/space-maps/disk.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/persistent-data/space-maps/disk.cc b/persistent-data/space-maps/disk.cc index 6cfd148..e641c89 100644 --- a/persistent-data/space-maps/disk.cc +++ b/persistent-data/space-maps/disk.cc @@ -646,6 +646,8 @@ namespace { } virtual void count_metadata(block_counter &bc) const { + bc.inc(bitmap_root_); + for (unsigned i = 0; i < entries_.size(); i++) { block_address b = entries_[i].blocknr_; From 0fee897fdaf7f590739adad601aad80497272f4c Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Wed, 8 Apr 2015 12:32:31 +0100 Subject: [PATCH 29/31] [thin_check] A space map count being too high should be a NON_FATAL error. --- thin-provisioning/thin_check.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/thin-provisioning/thin_check.cc b/thin-provisioning/thin_check.cc index 453ac4d..05c9886 100644 --- a/thin-provisioning/thin_check.cc +++ b/thin-provisioning/thin_check.cc @@ -292,7 +292,9 @@ namespace { << ", expected " << c_expected << ", but got " << c_actual << end_message(); - mplus_err = combine_errors(mplus_err, FATAL); + + mplus_err = combine_errors(mplus_err, + c_actual > c_expected ? NON_FATAL : FATAL); } } } From a934ee69c4509cdba9add48ffe9fdd929330b0b2 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Wed, 8 Apr 2015 13:58:41 +0100 Subject: [PATCH 30/31] [error_state] add a sneaky little stream operator to simplify combining error_states --- base/error_state.h | 5 +++++ unit-tests/Makefile.in | 1 + unit-tests/error_state_t.cc | 31 +++++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+) create mode 100644 unit-tests/error_state_t.cc diff --git a/base/error_state.h b/base/error_state.h index 9864f6a..079fed9 100644 --- a/base/error_state.h +++ b/base/error_state.h @@ -11,6 +11,11 @@ namespace base { }; error_state combine_errors(error_state lhs, error_state rhs); + + inline error_state &operator <<(error_state &err, error_state rhs) { + err = combine_errors(err, rhs); + return err; + } } //---------------------------------------------------------------- diff --git a/unit-tests/Makefile.in b/unit-tests/Makefile.in index d57f4ce..9307e5f 100644 --- a/unit-tests/Makefile.in +++ b/unit-tests/Makefile.in @@ -57,6 +57,7 @@ TEST_SOURCE=\ unit-tests/cache_superblock_t.cc \ unit-tests/damage_tracker_t.cc \ unit-tests/endian_t.cc \ + unit-tests/error_state_t.cc \ unit-tests/rmap_visitor_t.cc \ unit-tests/run_set_t.cc \ unit-tests/space_map_t.cc \ diff --git a/unit-tests/error_state_t.cc b/unit-tests/error_state_t.cc new file mode 100644 index 0000000..55e096c --- /dev/null +++ b/unit-tests/error_state_t.cc @@ -0,0 +1,31 @@ +#include "gmock/gmock.h" +#include "base/error_state.h" + +using namespace base; +using namespace std; +using namespace testing; + +//---------------------------------------------------------------- + +TEST(ErrorStateTests, stream_operator) +{ + { + error_state err = NO_ERROR; + err << NO_ERROR << FATAL; + ASSERT_THAT(err, Eq(FATAL)); + } + + { + error_state err = NO_ERROR; + err << NO_ERROR << NON_FATAL; + ASSERT_THAT(err, Eq(NON_FATAL)); + } + + { + error_state err = NO_ERROR; + err << NO_ERROR << FATAL << NO_ERROR << NON_FATAL; + ASSERT_THAT(err, Eq(FATAL)); + } +} + +//---------------------------------------------------------------- From 270c0f7041827ff162fd61732601cdf33faf52e2 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Wed, 8 Apr 2015 14:07:38 +0100 Subject: [PATCH 31/31] [thin_check] factor out check_space_map_counts() --- thin-provisioning/thin_check.cc | 159 ++++++++++++++++---------------- 1 file changed, 80 insertions(+), 79 deletions(-) diff --git a/thin-provisioning/thin_check.cc b/thin-provisioning/thin_check.cc index 05c9886..7eb5bd0 100644 --- a/thin-provisioning/thin_check.cc +++ b/thin-provisioning/thin_check.cc @@ -41,8 +41,6 @@ using namespace thin_provisioning; //---------------------------------------------------------------- namespace { - //-------------------------------- - block_manager<>::ptr open_bm(string const &path) { block_address nr_blocks = get_nr_blocks(path); @@ -73,7 +71,7 @@ namespace { nested_output::nest _ = out_.push(); out_ << d.desc_ << end_message(); } - err_ = combine_errors(err_, FATAL); + err_ << FATAL; } base::error_state get_error() const { @@ -101,7 +99,7 @@ namespace { out_ << d.desc_ << end_message(); } - err_ = combine_errors(err_, FATAL); + err_ << FATAL; } error_state get_error() const { @@ -128,7 +126,7 @@ namespace { nested_output::nest _ = out_.push(); out_ << d.desc_ << end_message(); } - err_ = combine_errors(err_, FATAL); + err_ << FATAL; } virtual void visit(mapping_tree_detail::missing_mappings const &d) { @@ -137,7 +135,7 @@ namespace { nested_output::nest _ = out_.push(); out_ << d.desc_ << end_message(); } - err_ = combine_errors(err_, FATAL); + err_ << FATAL; } error_state get_error() const { @@ -171,6 +169,77 @@ namespace { bool clear_needs_check_flag_on_success; }; + error_state check_space_map_counts(flags const &fs, nested_output &out, + superblock_detail::superblock &sb, + block_manager<>::ptr bm, + transaction_manager::ptr tm) { + block_counter bc; + + // Count the superblock + bc.inc(superblock_detail::SUPERBLOCK_LOCATION); + + // Count the metadata snap, if present + if (sb.metadata_snap_ != superblock_detail::SUPERBLOCK_LOCATION) { + bc.inc(sb.metadata_snap_); + + superblock_detail::superblock snap = read_superblock(bm, sb.metadata_snap_); + bc.inc(snap.data_mapping_root_); + bc.inc(snap.device_details_root_); + } + + // Count the device tree + { + noop_value_counter vc; + device_tree dtree(*tm, sb.device_details_root_, + device_tree_detail::device_details_traits::ref_counter()); + count_btree_blocks(dtree, bc, vc); + } + + // Count the mapping tree + { + noop_value_counter vc; + mapping_tree mtree(*tm, sb.data_mapping_root_, + mapping_tree_detail::block_traits::ref_counter(tm->get_sm())); + count_btree_blocks(mtree, bc, vc); + } + + // Count the metadata space map + { + persistent_space_map::ptr metadata_sm = + open_metadata_sm(*tm, static_cast(&sb.metadata_space_map_root_)); + metadata_sm->count_metadata(bc); + } + + // Count the data space map + { + persistent_space_map::ptr data_sm = + open_disk_sm(*tm, static_cast(&sb.data_space_map_root_)); + data_sm->count_metadata(bc); + } + + // Finally we need to check the metadata space map agrees + // with the counts we've just calculated. + error_state err = NO_ERROR; + nested_output::nest _ = out.push(); + persistent_space_map::ptr metadata_sm = + open_metadata_sm(*tm, static_cast(&sb.metadata_space_map_root_)); + for (unsigned b = 0; b < metadata_sm->get_nr_blocks(); b++) { + ref_t c_actual = metadata_sm->get_count(b); + ref_t c_expected = bc.get_count(b); + + if (c_actual != c_expected) { + out << "metadata reference counts differ for block " << b + << ", expected " << c_expected + << ", but got " << c_actual + << end_message(); + + err << (c_actual > c_expected ? NON_FATAL : FATAL); + } + } + + return err; + } + error_state metadata_check(string const &path, flags fs) { block_manager<>::ptr bm = open_bm(path); @@ -223,85 +292,17 @@ namespace { } } - error_state mplus_err = combine_errors(sb_rep.get_error(), - combine_errors(mapping_rep.get_error(), - dev_rep.get_error())); + error_state err = NO_ERROR; + err << sb_rep.get_error() << mapping_rep.get_error() << dev_rep.get_error(); // if we're checking everything, and there were no errors, // then we should check the space maps too. - if (fs.check_device_tree && fs.check_mapping_tree_level2 && mplus_err == NO_ERROR) { + if (fs.check_device_tree && fs.check_mapping_tree_level2 && err != FATAL) { out << "checking space map counts" << end_message(); - { - nested_output::nest _ = out.push(); - block_counter bc; - - // Count the superblock - bc.inc(superblock_detail::SUPERBLOCK_LOCATION); - - // Count the metadata snap, if present - if (sb.metadata_snap_ != superblock_detail::SUPERBLOCK_LOCATION) { - bc.inc(sb.metadata_snap_); - - superblock_detail::superblock snap = read_superblock(bm, sb.metadata_snap_); - bc.inc(snap.data_mapping_root_); - bc.inc(snap.device_details_root_); - } - - // Count the device tree - { - noop_value_counter vc; - device_tree dtree(*tm, sb.device_details_root_, - device_tree_detail::device_details_traits::ref_counter()); - count_btree_blocks(dtree, bc, vc); - } - - // Count the mapping tree - { - noop_value_counter vc; - mapping_tree mtree(*tm, sb.data_mapping_root_, - mapping_tree_detail::block_traits::ref_counter(tm->get_sm())); - count_btree_blocks(mtree, bc, vc); - } - - // Count the metadata space map - { - persistent_space_map::ptr metadata_sm = - open_metadata_sm(*tm, static_cast(&sb.metadata_space_map_root_)); - metadata_sm->count_metadata(bc); - } - - // Count the data space map - { - persistent_space_map::ptr data_sm = - open_disk_sm(*tm, static_cast(&sb.data_space_map_root_)); - data_sm->count_metadata(bc); - } - - // Finally we need to check the metadata - // space map agrees with the counts we've - // just calculated. - { - persistent_space_map::ptr metadata_sm = - open_metadata_sm(*tm, static_cast(&sb.metadata_space_map_root_)); - for (unsigned b = 0; b < metadata_sm->get_nr_blocks(); b++) { - ref_t c_actual = metadata_sm->get_count(b); - ref_t c_expected = bc.get_count(b); - - if (c_actual != c_expected) { - out << "metadata reference counts differ for block " << b - << ", expected " << c_expected - << ", but got " << c_actual - << end_message(); - - mplus_err = combine_errors(mplus_err, - c_actual > c_expected ? NON_FATAL : FATAL); - } - } - } - } + err << check_space_map_counts(fs, out, sb, bm, tm); } - return mplus_err; + return err; } void clear_needs_check(string const &path) {