diff --git a/caching/cache_check.cc b/caching/cache_check.cc index f1667c3..2cbd0e4 100644 --- a/caching/cache_check.cc +++ b/caching/cache_check.cc @@ -58,17 +58,27 @@ namespace { error_state err_; }; - class superblock_reporter : public superblock_detail::damage_visitor, reporter_base { + class superblock_reporter : public superblock_damage::damage_visitor, reporter_base { public: superblock_reporter(nested_output &o) : reporter_base(o) { } - virtual void visit(superblock_detail::superblock_corruption const &d) { + virtual void visit(superblock_damage::superblock_corrupt const &d) { out() << "superblock is corrupt" << end_message(); { nested_output::nest _ = push(); - out() << d.desc_ << end_message(); + out() << d.get_desc() << end_message(); + } + + mplus_error(FATAL); + } + + virtual void visit(superblock_damage::superblock_invalid const &d) { + out() << "superblock is invalid" << end_message(); + { + nested_output::nest _ = push(); + out() << d.get_desc() << end_message(); } mplus_error(FATAL); @@ -95,7 +105,7 @@ namespace { transaction_manager::ptr open_tm(block_manager<>::ptr bm) { space_map::ptr sm(new core_map(bm->get_nr_blocks())); - sm->inc(superblock_detail::SUPERBLOCK_LOCATION); + sm->inc(SUPERBLOCK_LOCATION); transaction_manager::ptr tm(new transaction_manager(bm, sm)); return tm; } @@ -146,13 +156,13 @@ namespace { out << "examining superblock" << end_message(); { nested_output::nest _ = out.push(); - check_superblock(bm, sb_rep); + check_superblock(bm, bm->get_nr_blocks(), sb_rep); } if (sb_rep.get_error() == FATAL) return FATAL; - superblock_detail::superblock sb = read_superblock(bm); + superblock sb = read_superblock(bm); transaction_manager::ptr tm = open_tm(bm); if (fs.check_mappings_) { diff --git a/caching/metadata.cc b/caching/metadata.cc index 90a1dd3..8ffe533 100644 --- a/caching/metadata.cc +++ b/caching/metadata.cc @@ -7,8 +7,6 @@ using namespace caching; //---------------------------------------------------------------- namespace { - using namespace superblock_detail; - unsigned const METADATA_CACHE_SIZE = 1024; // FIXME: duplication @@ -133,9 +131,7 @@ metadata::commit_hints() void metadata::commit_superblock() { - write_ref superblock = tm_->get_bm()->superblock_zero(SUPERBLOCK_LOCATION, superblock_validator()); - superblock_disk *disk = reinterpret_cast(superblock.data().raw()); - superblock_traits::pack(sb_, *disk); + write_superblock(tm_->get_bm(), sb_); } //---------------------------------------------------------------- diff --git a/caching/metadata.h b/caching/metadata.h index e4fe965..418d0e6 100644 --- a/caching/metadata.h +++ b/caching/metadata.h @@ -33,7 +33,7 @@ namespace caching { typedef persistent_data::transaction_manager tm; tm::ptr tm_; - superblock_detail::superblock sb_; + superblock sb_; checked_space_map::ptr metadata_sm_; mapping_array::ptr mappings_; hint_array::ptr hints_; diff --git a/caching/restore_emitter.cc b/caching/restore_emitter.cc index 35654da..cdf1f4c 100644 --- a/caching/restore_emitter.cc +++ b/caching/restore_emitter.cc @@ -5,7 +5,7 @@ using namespace caching; using namespace mapping_array_detail; using namespace std; -using namespace superblock_detail; +using namespace superblock_damage; //---------------------------------------------------------------- @@ -27,38 +27,14 @@ namespace { size_t hint_width) { superblock &sb = md_->sb_; - - sb.flags = 0; - memset(sb.uuid, 0, sizeof(sb.uuid)); - sb.magic = caching::superblock_detail::SUPERBLOCK_MAGIC; - sb.version = 0; // FIXME: fix strncpy((char *) sb.policy_name, policy.c_str(), sizeof(sb.policy_name)); - memset(sb.policy_version, 0, sizeof(sb.policy_version)); + memset(sb.policy_version, 0, sizeof(sb.policy_version)); // FIXME: should come from xml sb.policy_hint_size = hint_width; md_->setup_hint_array(hint_width); - memset(sb.metadata_space_map_root, 0, - sizeof(sb.metadata_space_map_root)); - sb.mapping_root = 0; - sb.hint_root = 0; - - sb.discard_root = 0; - sb.discard_block_size = 0; - sb.discard_nr_blocks = 0; - sb.data_block_size = block_size; - sb.metadata_block_size = 0; sb.cache_blocks = nr_cache_blocks; - sb.compat_flags = 0; - sb.compat_ro_flags = 0; - sb.incompat_flags = 0; - - sb.read_hits = 0; - sb.read_misses = 0; - sb.write_hits = 0; - sb.write_misses = 0; - struct mapping unmapped_value; unmapped_value.oblock_ = 0; unmapped_value.flags_ = 0; diff --git a/caching/superblock.cc b/caching/superblock.cc index 2766c24..b3cfaee 100644 --- a/caching/superblock.cc +++ b/caching/superblock.cc @@ -1,7 +1,92 @@ #include "caching/superblock.h" using namespace caching; -using namespace superblock_detail; +using namespace superblock_damage; + +//---------------------------------------------------------------- + +namespace { + using namespace base; + + struct superblock_disk { + le32 csum; + le32 flags; + le64 blocknr; + + __u8 uuid[16]; + le64 magic; + le32 version; + + __u8 policy_name[CACHE_POLICY_NAME_SIZE]; + le32 policy_version[CACHE_POLICY_VERSION_SIZE]; + le32 policy_hint_size; + + __u8 metadata_space_map_root[SPACE_MAP_ROOT_SIZE]; + + le64 mapping_root; + le64 hint_root; + + le64 discard_root; + le64 discard_block_size; + le64 discard_nr_blocks; + + le32 data_block_size; /* in 512-byte sectors */ + le32 metadata_block_size; /* in 512-byte sectors */ + le32 cache_blocks; + + le32 compat_flags; + le32 compat_ro_flags; + le32 incompat_flags; + + le32 read_hits; + le32 read_misses; + le32 write_hits; + le32 write_misses; + } __attribute__ ((packed)); + + struct superblock_traits { + typedef superblock_disk disk_type; + typedef superblock value_type; + + static void unpack(superblock_disk const &disk, superblock &value); + static void pack(superblock const &value, superblock_disk &disk); + }; + + uint32_t const SUPERBLOCK_MAGIC = 06142003; + uint32_t const VERSION_BEGIN = 1; + uint32_t const VERSION_END = 2; +} + +//---------------------------------------------------------------- + +superblock::superblock() + : csum(0), + flags(0), + blocknr(SUPERBLOCK_LOCATION), + magic(SUPERBLOCK_MAGIC), + version(VERSION_BEGIN), + policy_hint_size(0), + mapping_root(0), + hint_root(0), + discard_root(0), + discard_block_size(0), + discard_nr_blocks(0), + data_block_size(0), + metadata_block_size(8), + cache_blocks(0), + compat_flags(0), + compat_ro_flags(0), + incompat_flags(0), + read_hits(0), + read_misses(0), + write_hits(0), + write_misses(0) +{ + ::memset(uuid, 0, sizeof(uuid)); + ::memset(policy_name, 0, sizeof(policy_name)); + ::memset(policy_version, 0, sizeof(policy_version)); + ::memset(metadata_space_map_root, 0, sizeof(metadata_space_map_root)); +} //---------------------------------------------------------------- @@ -94,22 +179,33 @@ superblock_traits::pack(superblock const &core, superblock_disk &disk) //-------------------------------- -superblock_corruption::superblock_corruption(std::string const &desc) - : desc_(desc) +superblock_corrupt::superblock_corrupt(std::string const &desc) + : damage(desc) { } void -superblock_corruption::visit(damage_visitor &v) const +superblock_corrupt::visit(damage_visitor &v) const +{ + v.visit(*this); +} + +superblock_invalid::superblock_invalid(std::string const &desc) + : damage(desc) +{ +} + +void +superblock_invalid::visit(damage_visitor &v) const { v.visit(*this); } //-------------------------------- -namespace { +// anonymous namespace doesn't work for some reason +namespace validator { using namespace persistent_data; - using namespace superblock_detail; uint32_t const VERSION = 1; unsigned const SECTOR_TO_BLOCK_SHIFT = 3; @@ -131,65 +227,122 @@ namespace { sbd->csum = to_disk(sum.get_sum()); } }; -} -persistent_data::block_manager<>::validator::ptr -caching::superblock_validator() -{ - return block_manager<>::validator::ptr(new sb_validator); + block_manager<>::validator::ptr mk_v() { + return block_manager<>::validator::ptr(new sb_validator); + } } //-------------------------------- -superblock_detail::superblock +superblock caching::read_superblock(block_manager<>::ptr bm, block_address location) { - using namespace superblock_detail; - superblock sb; - block_manager<>::read_ref r = bm->read_lock(location, superblock_validator()); + block_manager<>::read_ref r = bm->read_lock(location, validator::mk_v()); superblock_disk const *sbd = reinterpret_cast(&r.data()); superblock_traits::unpack(*sbd, sb); return sb; } -superblock_detail::superblock -caching::read_superblock(persistent_data::block_manager<>::ptr bm) +void +caching::write_superblock(block_manager<>::ptr bm, superblock const &sb, block_address location) { - return read_superblock(bm, SUPERBLOCK_LOCATION); + block_manager<>::write_ref w = bm->superblock_zero(location, validator::mk_v()); + superblock_traits::pack(sb, *reinterpret_cast(w.data().raw())); } void -caching::write_superblock(block_manager<>::ptr bm, - block_address location, - superblock_detail::superblock const &sb) +caching::check_superblock(superblock const &sb, + block_address nr_metadata_blocks, + damage_visitor &visitor) { - using namespace superblock_detail; + if (sb.flags != 0) { + ostringstream msg; + msg << "invalid flags: " << hex << sb.flags; + visitor.visit(superblock_invalid(msg.str())); + } - block_manager<>::write_ref w = bm->write_lock(location, superblock_validator()); - superblock_traits::pack(sb, *reinterpret_cast(&w.data())); -} + if (sb.blocknr >= nr_metadata_blocks) { + ostringstream msg; + msg << "blocknr out of bounds: " << sb.blocknr << " >= " << nr_metadata_blocks; + visitor.visit(superblock_invalid(msg.str())); + } -void -caching::write_superblock(block_manager<>::ptr bm, - superblock_detail::superblock const &sb) -{ - write_superblock(bm, SUPERBLOCK_LOCATION, sb); + if (sb.magic != SUPERBLOCK_MAGIC) { + ostringstream msg; + msg << "magic in incorrect: " << sb.magic; + visitor.visit(superblock_invalid(msg.str())); + } + + if (sb.version >= VERSION_END) { + ostringstream msg; + msg << "version incorrect: " << sb.version; + visitor.visit(superblock_invalid(msg.str())); + } + + if (sb.version < VERSION_BEGIN) { + ostringstream msg; + msg << "version incorrect: " << sb.version; + visitor.visit(superblock_invalid(msg.str())); + } + + if (::strnlen((char const *) sb.policy_name, CACHE_POLICY_NAME_SIZE) == CACHE_POLICY_NAME_SIZE) { + visitor.visit(superblock_invalid("policy name is not null terminated")); + } + + if (sb.policy_hint_size % 4 || sb.policy_hint_size > 128) { + ostringstream msg; + msg << "policy hint size invalid: " << sb.policy_hint_size; + visitor.visit(superblock_invalid(msg.str())); + } + + if (sb.metadata_block_size != 8) { + ostringstream msg; + msg << "metadata block size incorrect: " << sb.metadata_block_size; + visitor.visit(superblock_invalid(msg.str())); + } + + if (sb.compat_flags != 0) { + ostringstream msg; + msg << "compat_flags invalid (can only be 0): " << sb.compat_flags; + visitor.visit(superblock_invalid(msg.str())); + } + + if (sb.compat_ro_flags != 0) { + ostringstream msg; + msg << "compat_ro_flags invalid (can only be 0): " << sb.compat_ro_flags; + visitor.visit(superblock_invalid(msg.str())); + } + + if (sb.incompat_flags != 0) { + ostringstream msg; + msg << "incompat_flags invalid (can only be 0): " << sb.incompat_flags; + visitor.visit(superblock_invalid(msg.str())); + } } void caching::check_superblock(persistent_data::block_manager<>::ptr bm, - superblock_detail::damage_visitor &visitor) + block_address nr_metadata_blocks, + damage_visitor &visitor) { - using namespace superblock_detail; + superblock sb; try { - bm->read_lock(SUPERBLOCK_LOCATION, superblock_validator()); + sb = read_superblock(bm, SUPERBLOCK_LOCATION); } catch (std::exception const &e) { - visitor.visit(superblock_corruption(e.what())); + + // FIXME: what if it fails due to a zero length file? Not + // really a corruption, so much as an io error. Should we + // separate these? + + visitor.visit(superblock_corrupt(e.what())); } + + check_superblock(sb, nr_metadata_blocks, visitor); } //---------------------------------------------------------------- diff --git a/caching/superblock.h b/caching/superblock.h index 92db272..390970d 100644 --- a/caching/superblock.h +++ b/caching/superblock.h @@ -7,112 +7,82 @@ //---------------------------------------------------------------- namespace caching { - namespace superblock_detail { - using namespace base; + typedef unsigned char __u8; - unsigned const SPACE_MAP_ROOT_SIZE = 128; - unsigned const CACHE_POLICY_NAME_SIZE = 16; - unsigned const CACHE_POLICY_VERSION_SIZE = 3; + unsigned const SPACE_MAP_ROOT_SIZE = 128; + unsigned const CACHE_POLICY_NAME_SIZE = 16; + unsigned const CACHE_POLICY_VERSION_SIZE = 3; + block_address const SUPERBLOCK_LOCATION = 0; - typedef unsigned char __u8; + struct superblock { + superblock(); - struct superblock_disk { - le32 csum; - le32 flags; - le64 blocknr; + uint32_t csum; + uint32_t flags; + uint64_t blocknr; - __u8 uuid[16]; - le64 magic; - le32 version; + __u8 uuid[16]; + uint64_t magic; + uint32_t version; - __u8 policy_name[CACHE_POLICY_NAME_SIZE]; - le32 policy_version[CACHE_POLICY_VERSION_SIZE]; - le32 policy_hint_size; + __u8 policy_name[CACHE_POLICY_NAME_SIZE]; + uint32_t policy_version[CACHE_POLICY_VERSION_SIZE]; + uint32_t policy_hint_size; - __u8 metadata_space_map_root[SPACE_MAP_ROOT_SIZE]; + __u8 metadata_space_map_root[SPACE_MAP_ROOT_SIZE]; - le64 mapping_root; - le64 hint_root; + uint64_t mapping_root; + uint64_t hint_root; - le64 discard_root; - le64 discard_block_size; - le64 discard_nr_blocks; + uint64_t discard_root; + uint64_t discard_block_size; + uint64_t discard_nr_blocks; - le32 data_block_size; /* in 512-byte sectors */ - le32 metadata_block_size; /* in 512-byte sectors */ - le32 cache_blocks; + uint32_t data_block_size; /* in 512-byte sectors */ + uint32_t metadata_block_size; /* in 512-byte sectors */ + uint32_t cache_blocks; - le32 compat_flags; - le32 compat_ro_flags; - le32 incompat_flags; + uint32_t compat_flags; + uint32_t compat_ro_flags; + uint32_t incompat_flags; - le32 read_hits; - le32 read_misses; - le32 write_hits; - le32 write_misses; - } __attribute__ ((packed)); + uint32_t read_hits; + uint32_t read_misses; + uint32_t write_hits; + uint32_t write_misses; + }; - struct superblock { - uint32_t csum; - uint32_t flags; - uint64_t blocknr; + //-------------------------------- - __u8 uuid[16]; - uint64_t magic; - uint32_t version; - - __u8 policy_name[CACHE_POLICY_NAME_SIZE]; - uint32_t policy_version[CACHE_POLICY_VERSION_SIZE]; - uint32_t policy_hint_size; - - __u8 metadata_space_map_root[SPACE_MAP_ROOT_SIZE]; - - uint64_t mapping_root; - uint64_t hint_root; - - uint64_t discard_root; - uint64_t discard_block_size; - uint64_t discard_nr_blocks; - - uint32_t data_block_size; /* in 512-byte sectors */ - uint32_t metadata_block_size; /* in 512-byte sectors */ - uint32_t cache_blocks; - - uint32_t compat_flags; - uint32_t compat_ro_flags; - uint32_t incompat_flags; - - uint32_t read_hits; - uint32_t read_misses; - uint32_t write_hits; - uint32_t write_misses; - }; - - struct superblock_traits { - typedef superblock_disk disk_type; - typedef superblock value_type; - - static void unpack(superblock_disk const &disk, superblock &value); - static void pack(superblock const &value, superblock_disk &disk); - }; - - block_address const SUPERBLOCK_LOCATION = 0; - uint32_t const SUPERBLOCK_MAGIC = 06142003; - - //-------------------------------- + namespace superblock_damage { class damage_visitor; - struct damage { + class damage { + public: + damage(std::string const &desc) + : desc_(desc) { + } + virtual ~damage() {} virtual void visit(damage_visitor &v) const = 0; + + std::string const &get_desc() const { + return desc_; + } + + private: + std::string desc_; }; - struct superblock_corruption : public damage { - superblock_corruption(std::string const &desc); + struct superblock_corrupt : public damage { + superblock_corrupt(std::string const &desc); void visit(damage_visitor &v) const; + }; - std::string desc_; + struct superblock_invalid : public damage { + superblock_invalid(std::string const &desc); + void visit(damage_visitor &v) const; }; class damage_visitor { @@ -121,24 +91,29 @@ namespace caching { void visit(damage const &d); - virtual void visit(superblock_corruption const &d) = 0; + virtual void visit(superblock_corrupt const &d) = 0; + virtual void visit(superblock_invalid const &d) = 0; }; } + //-------------------------------- + persistent_data::block_manager<>::validator::ptr superblock_validator(); - superblock_detail::superblock read_superblock(persistent_data::block_manager<>::ptr bm); - superblock_detail::superblock read_superblock(persistent_data::block_manager<>::ptr bm, - persistent_data::block_address location); + superblock read_superblock(persistent_data::block_manager<>::ptr bm, + persistent_data::block_address location = SUPERBLOCK_LOCATION); void write_superblock(persistent_data::block_manager<>::ptr bm, - superblock_detail::superblock const &sb); - void write_superblock(persistent_data::block_manager<>::ptr bm, - persistent_data::block_address location, - superblock_detail::superblock const &sb); + superblock const &sb, + persistent_data::block_address location = SUPERBLOCK_LOCATION); + + void check_superblock(superblock const &sb, + persistent_data::block_address nr_metadata_blocks, + superblock_damage::damage_visitor &visitor); void check_superblock(persistent_data::block_manager<>::ptr bm, - superblock_detail::damage_visitor &visitor); + persistent_data::block_address nr_metadata_blocks, + superblock_damage::damage_visitor &visitor); } //---------------------------------------------------------------- diff --git a/unit-tests/Makefile.in b/unit-tests/Makefile.in index 808e20b..ffc1afc 100644 --- a/unit-tests/Makefile.in +++ b/unit-tests/Makefile.in @@ -53,6 +53,7 @@ TEST_SOURCE=\ unit-tests/btree_damage_visitor_t.cc \ unit-tests/buffer_t.cc \ unit-tests/cache_t.cc \ + unit-tests/cache_superblock_t.cc \ unit-tests/damage_tracker_t.cc \ unit-tests/endian_t.cc \ unit-tests/rmap_visitor_t.cc \ diff --git a/unit-tests/cache_superblock_t.cc b/unit-tests/cache_superblock_t.cc new file mode 100644 index 0000000..c5720fa --- /dev/null +++ b/unit-tests/cache_superblock_t.cc @@ -0,0 +1,140 @@ +#include "gmock/gmock.h" +#include "caching/superblock.h" + +using namespace caching; +using namespace superblock_damage; +using namespace testing; + +//---------------------------------------------------------------- + +namespace { + class damage_visitor_mock : public damage_visitor { + public: + MOCK_METHOD1(visit, void(superblock_corrupt const &)); + MOCK_METHOD1(visit, void(superblock_invalid const &)); + }; + + class CacheSuperblockTests : public Test { + public: + CacheSuperblockTests() { + } + + void check() { + check_superblock(sb_, 100, visitor_); + } + + void expect_invalid() { + EXPECT_CALL(visitor_, visit(Matcher(_))).Times(1); + } + + void check_invalid() { + expect_invalid(); + check(); + } + + damage_visitor_mock visitor_; + superblock sb_; + }; + + bool operator ==(superblock_corrupt const &lhs, superblock_corrupt const &rhs) { + return lhs.get_desc() == rhs.get_desc(); + } + + bool operator ==(superblock_invalid const &lhs, superblock_invalid const &rhs) { + return lhs.get_desc() == rhs.get_desc(); + } + + ostream &operator <<(ostream &out, damage const &d) { + out << d.get_desc(); + return out; + } + + ostream &operator <<(ostream &out, superblock_invalid const &d) { + out << "superblock_invalid: " << d.get_desc(); + return out; + } +} + +//---------------------------------------------------------------- + +TEST_F(CacheSuperblockTests, default_constructed_superblock_is_valid) +{ + sb_.flags = 0; + check(); +} + +TEST_F(CacheSuperblockTests, non_zero_flags_are_invalid) +{ + sb_.flags = 1; + check_invalid(); +} + +TEST_F(CacheSuperblockTests, blocknr_is_in_range) +{ + sb_.blocknr = 101; + check_invalid(); +} + +TEST_F(CacheSuperblockTests, magic_is_checked) +{ + sb_.magic = 12345; + check_invalid(); +} + +TEST_F(CacheSuperblockTests, version_gt_1_is_checked) +{ + sb_.version = 2; + check_invalid(); +} + +TEST_F(CacheSuperblockTests, version_lt_1_is_checked) +{ + sb_.version = 0; + check_invalid(); +} + +TEST_F(CacheSuperblockTests, policy_name_must_be_null_terminated) +{ + for (unsigned i = 0; i < CACHE_POLICY_NAME_SIZE; i++) + sb_.policy_name[i] = 'a'; + + check_invalid(); +} + +TEST_F(CacheSuperblockTests, policy_hint_size_checked) +{ + sb_.policy_hint_size = 3; + check_invalid(); + + sb_.policy_hint_size = 129; + check_invalid(); + + sb_.policy_hint_size = 132; + check_invalid(); +} + +TEST_F(CacheSuperblockTests, metadata_block_size_checked) +{ + sb_.metadata_block_size = 16; + check_invalid(); +} + +TEST_F(CacheSuperblockTests, compat_flags_checked) +{ + sb_.compat_flags = 1; + check_invalid(); +} + +TEST_F(CacheSuperblockTests, compat_ro_flags_checked) +{ + sb_.compat_ro_flags = 1; + check_invalid(); +} + +TEST_F(CacheSuperblockTests, incompat_flags_checked) +{ + sb_.incompat_flags = 1; + check_invalid(); +} + +//----------------------------------------------------------------