From b1e0ca220722c2aef7f6b585c5ddb47e144cb39a Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 26 Apr 2013 15:54:15 +0100 Subject: [PATCH] [block] make sure we can change validators --- persistent-data/block.h | 3 +- persistent-data/block.tcc | 43 +++++++--- unit-tests/block_t.cc | 175 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 206 insertions(+), 15 deletions(-) diff --git a/persistent-data/block.h b/persistent-data/block.h index f45aae8..1af46ba 100644 --- a/persistent-data/block.h +++ b/persistent-data/block.h @@ -119,7 +119,8 @@ namespace persistent_data { void flush(); - void change_validator(typename block_manager::validator::ptr v); + void change_validator(typename block_manager::validator::ptr v, + bool check = true); typename block_io::ptr io_; block_address location_; diff --git a/persistent-data/block.tcc b/persistent-data/block.tcc index 0291cb8..28c87dc 100644 --- a/persistent-data/block.tcc +++ b/persistent-data/block.tcc @@ -210,8 +210,9 @@ block_manager::block::block(typename block_io::ptr io, dirty_(false) { if (zero) { + // FIXME: duplicate memset memset(data_->raw(), 0, BlockSize); - dirty_ = true; + dirty_ = true; // redundant? } else { io_->read_buffer(location_, *data_); validator_->check(*data_, location_); @@ -237,14 +238,19 @@ block_manager::block::flush() template void -block_manager::block::change_validator(typename block_manager::validator::ptr v) +block_manager::block::change_validator(typename block_manager::validator::ptr v, + bool check) { if (v.get() != validator_.get()) { if (dirty_) + // It may have already happened, by calling + // this we ensure we're consistent. validator_->prepare(*data_, location_); validator_ = v; - validator_->check(*data_, location_); + + if (check) + validator_->check(*data_, location_); } } @@ -383,7 +389,10 @@ block_manager::write_lock(block_address location, boost::optional cached_block = cache_.get(location); if (cached_block) { - (*cached_block)->check_write_lockable(); + typename block::ptr cb = *cached_block; + cb->check_write_lockable(); + cb->change_validator(v); + return write_ref(*this, *cached_block); } @@ -409,8 +418,11 @@ block_manager::write_lock_zero(block_address location, boost::optional cached_block = cache_.get(location); if (cached_block) { - (*cached_block)->check_write_lockable(); + typename block::ptr cb = *cached_block; + cb->check_write_lockable(); + cb->change_validator(v, false); memset((*cached_block)->data_->raw(), 0, BlockSize); + return write_ref(*this, *cached_block); } @@ -436,9 +448,11 @@ block_manager::superblock(block_address location, boost::optional cached_block = cache_.get(location); if (cached_block) { - (*cached_block)->check_write_lockable(); - (*cached_block)->bt_ = BT_SUPERBLOCK; - (*cached_block)->validator_ = v; + typename block::ptr cb = *cached_block; + cb->check_write_lockable(); + cb->bt_ = BT_SUPERBLOCK; + cb->change_validator(v); + return write_ref(*this, *cached_block); } @@ -464,15 +478,16 @@ block_manager::superblock_zero(block_address location, boost::optional cached_block = cache_.get(location); if (cached_block) { - (*cached_block)->check_write_lockable(); - memset((*cached_block)->data_->raw(), 0, BlockSize); // FIXME: add a zero method to buffer - (*cached_block)->validator_ = v; + typename block::ptr cb = *cached_block; + cb->check_write_lockable(); + cb->bt_ = BT_SUPERBLOCK; + cb->change_validator(v, false); + memset(cb->data_->raw(), 0, BlockSize); // FIXME: add a zero method to buffer + return write_ref(*this, *cached_block); } - typename block::ptr b(new block(io_, location, BT_SUPERBLOCK, - mk_validator(new noop_validator), true)); - b->validator_ = v; + typename block::ptr b(new block(io_, location, BT_SUPERBLOCK, v, true)); cache_.insert(b); return write_ref(*this, b); diff --git a/unit-tests/block_t.cc b/unit-tests/block_t.cc index b4475d2..9b554d4 100644 --- a/unit-tests/block_t.cc +++ b/unit-tests/block_t.cc @@ -49,6 +49,14 @@ namespace { } }; + class validator_mock : public block_manager<4096>::validator { + public: + typedef boost::shared_ptr ptr; + + MOCK_CONST_METHOD2(check, void(buffer<4096> const &, block_address)); + MOCK_CONST_METHOD2(prepare, void(buffer<4096> &, block_address)); + }; + typedef block_manager<4096> bm4096; } @@ -208,3 +216,170 @@ TEST(BlockTests, write_then_read) } //---------------------------------------------------------------- + +namespace { + class ValidatorTests : public Test { + public: + ValidatorTests() + : bm(create_bm<4096>()), + vmock(new validator_mock), + vmock2(new validator_mock) { + } + + void expect_check(validator_mock::ptr v) { + EXPECT_CALL(*v, check(_, Eq(0ull))).Times(1); + } + + void expect_prepare(validator_mock::ptr v) { + EXPECT_CALL(*v, prepare(_, Eq(0ull))).Times(1); + } + + bm4096::ptr bm; + validator_mock::ptr vmock; + validator_mock::ptr vmock2; + }; +} + +//-------------------------------- + +TEST_F(ValidatorTests, check_on_read_lock) +{ + expect_check(vmock); + bm4096::read_ref rr = bm->read_lock(0, vmock); +} + +TEST_F(ValidatorTests, check_only_called_once_on_read_lock) +{ + { + expect_check(vmock); + bm4096::read_ref rr = bm->read_lock(0, vmock); + } + + bm4096::read_ref rr = bm->read_lock(0, vmock); +} + +TEST_F(ValidatorTests, validator_can_be_changed_by_read_lock) +{ + { + expect_check(vmock); + bm4096::read_ref rr = bm->read_lock(0, vmock); + } + + { + expect_check(vmock2); + bm4096::read_ref rr = bm->read_lock(0, vmock2); + } +} + +//-------------------------------- + +TEST_F(ValidatorTests, check_and_prepare_on_write_lock) +{ + expect_check(vmock); + expect_prepare(vmock); + bm4096::write_ref wr = bm->write_lock(0, vmock); +} + +TEST_F(ValidatorTests, check_only_called_once_on_write_lock) +{ + { + expect_check(vmock); + bm4096::write_ref wr = bm->write_lock(0, vmock); + } + + bm4096::write_ref wr = bm->write_lock(0, vmock); + expect_prepare(vmock); +} + +TEST_F(ValidatorTests, validator_can_be_changed_by_write_lock) +{ + { + expect_check(vmock); + expect_prepare(vmock); + bm4096::write_ref wr = bm->write_lock(0, vmock); + } + + { + expect_check(vmock2); + expect_prepare(vmock2); + bm4096::write_ref wr = bm->write_lock(0, vmock2); + } +} + +//-------------------------------- + +TEST_F(ValidatorTests, no_check_but_prepare_on_write_lock_zero) +{ + expect_prepare(vmock); + bm4096::write_ref wr = bm->write_lock_zero(0, vmock); +} + +TEST_F(ValidatorTests, validator_can_be_changed_by_write_lock_zero) +{ + expect_prepare(vmock); + expect_prepare(vmock2); + + { + bm4096::write_ref wr = bm->write_lock_zero(0, vmock); + } + + bm4096::write_ref wr = bm->write_lock_zero(0, vmock2); +} + +//-------------------------------- + +TEST_F(ValidatorTests, check_and_prepare_on_superblock_lock) +{ + expect_check(vmock); + expect_prepare(vmock); + bm4096::write_ref wr = bm->superblock(0, vmock); +} + +TEST_F(ValidatorTests, check_only_called_once_on_superblock_lock) +{ + { + expect_check(vmock); + expect_prepare(vmock); + bm4096::write_ref wr = bm->superblock(0, vmock); + } + + bm4096::write_ref wr = bm->superblock(0, vmock); + expect_prepare(vmock); +} + +TEST_F(ValidatorTests, validator_can_be_changed_by_superblock_lock) +{ + { + expect_check(vmock); + expect_prepare(vmock); + bm4096::write_ref wr = bm->write_lock(0, vmock); + } + + { + expect_check(vmock2); + expect_prepare(vmock2); + bm4096::write_ref wr = bm->write_lock(0, vmock2); + } +} + +//-------------------------------- + +TEST_F(ValidatorTests, no_check_but_prepare_on_superblock_lock_zero) +{ + expect_prepare(vmock); + bm4096::write_ref wr = bm->superblock_zero(0, vmock); +} + +TEST_F(ValidatorTests, validator_can_be_changed_by_superblock_zero) +{ + expect_prepare(vmock); + expect_prepare(vmock2); + + { + bm4096::write_ref wr = bm->write_lock(0, vmock); + } + + bm4096::write_ref wr = bm->write_lock(0, vmock2); +} + +//----------------------------------------------------------------