From 969a5f62c6bb46191f10e2a73af0524b812144ef Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Mon, 29 Apr 2013 15:37:53 +0100 Subject: [PATCH] Working on device_checker --- thin-provisioning/device_checker.cc | 92 +++++++++++++++++++++++++-- thin-provisioning/device_checker.h | 5 +- thin-provisioning/metadata_checker.cc | 77 +++------------------- thin-provisioning/metadata_checker.h | 40 ++++-------- unit-tests/metadata_checker_t.cc | 59 +++++++++++++---- 5 files changed, 164 insertions(+), 109 deletions(-) diff --git a/thin-provisioning/device_checker.cc b/thin-provisioning/device_checker.cc index dcca01b..071a2ca 100644 --- a/thin-provisioning/device_checker.cc +++ b/thin-provisioning/device_checker.cc @@ -1,19 +1,103 @@ #include "thin-provisioning/device_checker.h" +#include "persistent-data/data-structures/btree_checker.h" +#include "persistent-data/transaction_manager.h" +#include "persistent-data/space-maps/core.h" +#include "thin-provisioning/metadata.h" +#include "thin-provisioning/metadata_disk_structures.h" + +using namespace persistent_data; using namespace thin_provisioning; //---------------------------------------------------------------- -device_checker::device_checker(block_manager::ptr bm) - : checker(bm) +namespace { + // FIXME: duplication with metadata.cc + transaction_manager::ptr + open_tm(block_manager<>::ptr bm) { + space_map::ptr sm(new core_map(bm->get_nr_blocks())); + sm->inc(SUPERBLOCK_LOCATION); + transaction_manager::ptr tm(new transaction_manager(bm, sm)); + return tm; + } + + + class device_visitor : public btree<1, device_details_traits>::visitor { + public: + typedef boost::shared_ptr ptr; + typedef btree_checker<1, device_details_traits> checker; + + device_visitor(block_counter &counter) + : checker_(counter) { + } + + bool visit_internal(unsigned level, + bool sub_root, + optional key, + btree_detail::node_ref const &n) { + return checker_.visit_internal(level, sub_root, key, n); + } + + bool visit_internal_leaf(unsigned level, + bool sub_root, + optional key, + btree_detail::node_ref const &n) { + return checker_.visit_internal_leaf(level, sub_root, key, n); + } + + bool visit_leaf(unsigned level, + bool sub_root, + optional key, + btree_detail::node_ref const &n) { + + if (!checker_.visit_leaf(level, sub_root, key, n)) + return false; + + for (unsigned i = 0; i < n.get_nr_entries(); i++) + devices_.insert(n.key_at(i)); + + return true; + } + + set const &get_devices() const { + return devices_; + } + + private: + checker checker_; + set devices_; + }; +} + +//---------------------------------------------------------------- + +device_checker::device_checker(block_manager::ptr bm, + block_address root) + : checker(bm), + root_(root) { } damage_list_ptr device_checker::check() { - // FIXME: finish. - return damage_list_ptr(new damage_list); + block_counter counter; + device_visitor::ptr v(new device_visitor(counter)); + transaction_manager::ptr tm(open_tm(bm_)); + detail_tree::ptr details(new detail_tree(tm, root_, + device_details_traits::ref_counter())); + damage_list_ptr damage(new damage_list); + + try { + details->visit(v); + + } catch (std::exception const &e) { + + metadata_damage::ptr d(new missing_device_details(range64())); + damage->push_back(d); + } + + return damage; } //---------------------------------------------------------------- diff --git a/thin-provisioning/device_checker.h b/thin-provisioning/device_checker.h index ff4d561..b2b67d0 100644 --- a/thin-provisioning/device_checker.h +++ b/thin-provisioning/device_checker.h @@ -8,8 +8,11 @@ namespace thin_provisioning { class device_checker : public checker { public: - device_checker(block_manager::ptr bm); + device_checker(block_manager::ptr bm, block_address btree_root); damage_list_ptr check(); + + private: + block_address root_; }; } diff --git a/thin-provisioning/metadata_checker.cc b/thin-provisioning/metadata_checker.cc index 5276a0b..c4572d4 100644 --- a/thin-provisioning/metadata_checker.cc +++ b/thin-provisioning/metadata_checker.cc @@ -48,10 +48,8 @@ super_block_corruption::visit(metadata_damage_visitor &visitor) const //-------------------------------- -missing_device_details::missing_device_details(uint64_t missing_begin, - uint64_t missing_end) - : missing_begin_(missing_begin), - missing_end_(missing_end) +missing_device_details::missing_device_details(range64 missing) + : missing_(missing) { } @@ -63,10 +61,8 @@ missing_device_details::visit(metadata_damage_visitor &visitor) const //-------------------------------- -missing_devices::missing_devices(uint64_t missing_begin, - uint64_t missing_end) - : missing_begin_(missing_begin), - missing_end_(missing_end) +missing_devices::missing_devices(range64 missing) + : missing_(missing) { } @@ -78,12 +74,9 @@ missing_devices::visit(metadata_damage_visitor &visitor) const //-------------------------------- -missing_mappings::missing_mappings(uint64_t dev, - uint64_t missing_begin, - uint64_t missing_end) +missing_mappings::missing_mappings(uint64_t dev, range64 missing) : dev_(dev), - missing_begin_(missing_begin), - missing_end_(missing_end) + missing_(missing) { } @@ -129,10 +122,8 @@ bad_data_ref_count::visit(metadata_damage_visitor &visitor) const //-------------------------------- -missing_metadata_ref_counts::missing_metadata_ref_counts(block_address missing_begin, - block_address missing_end) - : missing_begin_(missing_begin), - missing_end_(missing_end) +missing_metadata_ref_counts::missing_metadata_ref_counts(range64 missing) + : missing_(missing) { } @@ -144,10 +135,8 @@ missing_metadata_ref_counts::visit(metadata_damage_visitor &visitor) const //-------------------------------- -missing_data_ref_counts::missing_data_ref_counts(block_address missing_begin, - block_address missing_end) - : missing_begin_(missing_begin), - missing_end_(missing_end) +missing_data_ref_counts::missing_data_ref_counts(range64 missing) + : missing_(missing) { } @@ -236,52 +225,6 @@ namespace { set devices_; }; - class details_validator : public btree<1, device_details_traits>::visitor { - public: - typedef boost::shared_ptr ptr; - typedef btree_checker<1, device_details_traits> checker; - - details_validator(block_counter &counter) - : checker_(counter) { - } - - bool visit_internal(unsigned level, - bool sub_root, - optional key, - btree_detail::node_ref const &n) { - return checker_.visit_internal(level, sub_root, key, n); - } - - bool visit_internal_leaf(unsigned level, - bool sub_root, - optional key, - btree_detail::node_ref const &n) { - return checker_.visit_internal_leaf(level, sub_root, key, n); - } - - bool visit_leaf(unsigned level, - bool sub_root, - optional key, - btree_detail::node_ref const &n) { - - if (!checker_.visit_leaf(level, sub_root, key, n)) - return false; - - for (unsigned i = 0; i < n.get_nr_entries(); i++) - devices_.insert(n.key_at(i)); - - return true; - } - - set const &get_devices() const { - return devices_; - } - - private: - checker checker_; - set devices_; - }; - struct check_count : public space_map::iterator { check_count(string const &desc, block_counter const &expected) : bad_(false), diff --git a/thin-provisioning/metadata_checker.h b/thin-provisioning/metadata_checker.h index 16f5ebb..d024138 100644 --- a/thin-provisioning/metadata_checker.h +++ b/thin-provisioning/metadata_checker.h @@ -23,6 +23,8 @@ #include "persistent-data/error_set.h" #include "persistent-data/space_map.h" +#include "thin-provisioning/range.h" + #include //---------------------------------------------------------------- @@ -53,36 +55,28 @@ namespace thin_provisioning { void visit(metadata_damage_visitor &visitor) const; }; - struct missing_device_details : public metadata_damage { - missing_device_details(uint64_t missing_begin, - uint64_t missing_end); + typedef range range64; + struct missing_device_details : public metadata_damage { + missing_device_details(range64 missing); virtual void visit(metadata_damage_visitor &visitor) const; - uint64_t missing_begin_; - uint64_t missing_end_; + range64 missing_; }; struct missing_devices : public metadata_damage { - missing_devices(uint64_t missing_begin, - uint64_t missing_end); - + missing_devices(range64 missing); virtual void visit(metadata_damage_visitor &visitor) const; - uint64_t missing_begin_; - uint64_t missing_end_; + range64 missing_; }; struct missing_mappings : public metadata_damage { - missing_mappings(uint64_t dev, - uint64_t missing_begin, - uint64_t missing_end); - + missing_mappings(uint64_t dev, range64 missing); virtual void visit(metadata_damage_visitor &visitor) const; uint64_t dev_; - uint64_t missing_begin_; - uint64_t missing_end_; + range64 missing_; }; struct bad_metadata_ref_count : public metadata_damage { @@ -110,23 +104,17 @@ namespace thin_provisioning { }; struct missing_metadata_ref_counts : public metadata_damage { - missing_metadata_ref_counts(block_address missing_begin, - block_address missing_end); - + missing_metadata_ref_counts(range64 missing); virtual void visit(metadata_damage_visitor &visitor) const; - block_address missing_begin_; - block_address missing_end_; + range64 missing_; }; struct missing_data_ref_counts : public metadata_damage { - missing_data_ref_counts(block_address missing_begin, - block_address missing_end); - + missing_data_ref_counts(range64 missing); virtual void visit(metadata_damage_visitor &visitor) const; - block_address missing_begin_; - block_address missing_end_; + range64 missing_; }; class metadata_damage_visitor { diff --git a/unit-tests/metadata_checker_t.cc b/unit-tests/metadata_checker_t.cc index 324fee0..348f2bd 100644 --- a/unit-tests/metadata_checker_t.cc +++ b/unit-tests/metadata_checker_t.cc @@ -6,6 +6,7 @@ #include "thin-provisioning/device_checker.h" #include "thin-provisioning/restore_emitter.h" #include "thin-provisioning/superblock_checker.h" +#include "thin-provisioning/superblock_validator.h" #include @@ -122,24 +123,29 @@ namespace { builder.build(); } + superblock read_superblock() { + superblock sb; + + block_manager<>::read_ref r = bm_->read_lock(SUPERBLOCK_LOCATION, superblock_validator()); + superblock_disk const *sbd = reinterpret_cast(&r.data()); + superblock_traits::unpack(*sbd, sb); + return sb; + } + + void zero_block(block_address b) { + ::test::zero_block(bm_, b); + } + with_temp_directory dir_; block_manager<>::ptr bm_; - }; class SuperBlockCheckerTests : public MetadataCheckerTests { public: void corrupt_superblock() { - zero_block(bm_, SUPERBLOCK_LOCATION); + zero_block(SUPERBLOCK_LOCATION); } }; - - //-------------------------------- - - class DevicesCheckerTests : public MetadataCheckerTests { - public: - - }; } //---------------------------------------------------------------- @@ -170,9 +176,40 @@ TEST_F(SuperBlockCheckerTests, fails_with_bad_checksum) //---------------------------------------------------------------- -TEST_F(DevicesCheckerTests, create_require_a_block_manager) +namespace { + class DeviceCheckerTests : public MetadataCheckerTests { + public: + DeviceCheckerTests() { + } + + block_address devices_root() { + superblock sb = read_superblock(); + return sb.device_details_root_; + } + + device_checker::ptr mk_checker() { + return device_checker::ptr(new device_checker(bm_, devices_root())); + } + }; +} + +TEST_F(DeviceCheckerTests, create_require_a_block_manager_and_a_root_block) { - device_checker dc(bm_); + mk_checker(); +} + +TEST_F(DeviceCheckerTests, passes_with_valid_metadata_and_zero_devices) +{ + damage_list_ptr damage = mk_checker()->check(); + ASSERT_THAT(damage->size(), Eq(0u)); +} + +TEST_F(DeviceCheckerTests, fails_with_corrupt_root) +{ + zero_block(devices_root()); + + damage_list_ptr damage = mk_checker()->check(); + ASSERT_THAT(damage->size(), Gt(0u)); } //----------------------------------------------------------------