From 09afab6b173a6a9a0054866224262dbec0990f6f Mon Sep 17 00:00:00 2001 From: shangdehao1 Date: Fri, 24 May 2019 05:46:24 +0800 Subject: [PATCH] librbd: add TypeTrait class to librbd/cache - add TypeTraits to librbd/cache - remove the second template parameter of parent image dispatch - remove m_cache_client and m_image_ctx to private selection, and add two public method to get these two data member. - modify parent image cache uite testing Signed-off-by: Dehao Shang --- src/librbd/cache/ParentCacheObjectDispatch.cc | 35 +++++----- src/librbd/cache/ParentCacheObjectDispatch.h | 28 +++++--- src/librbd/cache/TypeTraits.h | 26 +++++++ .../cache/test_mock_ParentImageCache.cc | 67 +++++++++---------- 4 files changed, 95 insertions(+), 61 deletions(-) create mode 100644 src/librbd/cache/TypeTraits.h diff --git a/src/librbd/cache/ParentCacheObjectDispatch.cc b/src/librbd/cache/ParentCacheObjectDispatch.cc index 251e8b5505e..2581fd88523 100644 --- a/src/librbd/cache/ParentCacheObjectDispatch.cc +++ b/src/librbd/cache/ParentCacheObjectDispatch.cc @@ -19,24 +19,29 @@ #define dout_prefix *_dout << "librbd::cache::ParentCacheObjectDispatch: " \ << this << " " << __func__ << ": " +using namespace ceph::immutable_obj_cache; + namespace librbd { namespace cache { -template -ParentCacheObjectDispatch::ParentCacheObjectDispatch( +template +ParentCacheObjectDispatch::ParentCacheObjectDispatch( I* image_ctx) : m_image_ctx(image_ctx), m_cache_client(nullptr), m_initialized(false), m_object_store(nullptr) { + std::string controller_path = + ((CephContext*)(m_image_ctx->cct))->_conf.get_val("immutable_object_cache_sock"); + m_cache_client = new CacheClient(controller_path.c_str(), m_image_ctx->cct); } -template -ParentCacheObjectDispatch::~ParentCacheObjectDispatch() { +template +ParentCacheObjectDispatch::~ParentCacheObjectDispatch() { delete m_object_store; delete m_cache_client; } // TODO if connect fails, init will return error to high layer. -template -void ParentCacheObjectDispatch::init() { +template +void ParentCacheObjectDispatch::init() { auto cct = m_image_ctx->cct; ldout(cct, 5) << dendl; @@ -47,10 +52,6 @@ void ParentCacheObjectDispatch::init() { ldout(cct, 5) << "parent image: setup SRO cache client" << dendl; - std::string controller_path = ((CephContext*)cct)->_conf.get_val("immutable_object_cache_sock"); - if(m_cache_client == nullptr) { - m_cache_client = new C(controller_path.c_str(), m_image_ctx->cct); - } m_cache_client->run(); int ret = m_cache_client->connect(); @@ -78,8 +79,8 @@ void ParentCacheObjectDispatch::init() { } } -template -bool ParentCacheObjectDispatch::read( +template +bool ParentCacheObjectDispatch::read( const std::string &oid, uint64_t object_no, uint64_t object_off, uint64_t object_len, librados::snap_t snap_id, int op_flags, const ZTracer::Trace &parent_trace, ceph::bufferlist* read_data, @@ -111,8 +112,8 @@ bool ParentCacheObjectDispatch::read( return true; } -template -void ParentCacheObjectDispatch::handle_read_cache( +template +void ParentCacheObjectDispatch::handle_read_cache( ObjectCacheRequest* ack, uint64_t read_off, uint64_t read_len, ceph::bufferlist* read_data, io::DispatchResult* dispatch_result, Context* on_dispatched) { @@ -142,8 +143,8 @@ void ParentCacheObjectDispatch::handle_read_cache( on_dispatched->complete(r); } -template -int ParentCacheObjectDispatch::handle_register_client(bool reg) { +template +int ParentCacheObjectDispatch::handle_register_client(bool reg) { auto cct = m_image_ctx->cct; ldout(cct, 20) << dendl; @@ -157,4 +158,4 @@ int ParentCacheObjectDispatch::handle_register_client(bool reg) { } // namespace cache } // namespace librbd -template class librbd::cache::ParentCacheObjectDispatch; +template class librbd::cache::ParentCacheObjectDispatch; diff --git a/src/librbd/cache/ParentCacheObjectDispatch.h b/src/librbd/cache/ParentCacheObjectDispatch.h index 5a10d5754e9..9fcc1da5c56 100644 --- a/src/librbd/cache/ParentCacheObjectDispatch.h +++ b/src/librbd/cache/ParentCacheObjectDispatch.h @@ -8,18 +8,21 @@ #include "SharedPersistentObjectCacher.h" #include "librbd/io/ObjectDispatchInterface.h" #include "tools/immutable_object_cache/CacheClient.h" +#include "librbd/cache/TypeTraits.h" #include "tools/immutable_object_cache/Types.h" -using namespace ceph::immutable_obj_cache; - namespace librbd { class ImageCtx; namespace cache { -template +template class ParentCacheObjectDispatch : public io::ObjectDispatchInterface { + // mock unit testing support + typedef cache::TypeTraits TypeTraits; + typedef typename TypeTraits::CacheClient CacheClient; + public: static ParentCacheObjectDispatch* create(ImageCtxT* image_ctx) { return new ParentCacheObjectDispatch(image_ctx); @@ -108,18 +111,25 @@ public: return m_initialized; } - CacheClientT *m_cache_client = nullptr; - ImageCtxT* m_image_ctx; + ImageCtxT* get_image_ctx() { + return m_image_ctx; + } -private: + CacheClient* get_cache_client() { + return m_cache_client; + } +private: void handle_read_cache( - ObjectCacheRequest* ack, uint64_t read_off, - uint64_t read_len, ceph::bufferlist* read_data, + ceph::immutable_obj_cache::ObjectCacheRequest* ack, + uint64_t read_off, uint64_t read_len, + ceph::bufferlist* read_data, io::DispatchResult* dispatch_result, Context* on_dispatched); int handle_register_client(bool reg); + CacheClient *m_cache_client = nullptr; + ImageCtxT* m_image_ctx; SharedPersistentObjectCacher *m_object_store = nullptr; bool m_initialized; }; @@ -127,6 +137,6 @@ private: } // namespace cache } // namespace librbd -extern template class librbd::cache::ParentCacheObjectDispatch; +extern template class librbd::cache::ParentCacheObjectDispatch; #endif // CEPH_LIBRBD_CACHE_PARENT_CACHER_OBJECT_DISPATCH_H diff --git a/src/librbd/cache/TypeTraits.h b/src/librbd/cache/TypeTraits.h new file mode 100644 index 00000000000..dd7075e8d83 --- /dev/null +++ b/src/librbd/cache/TypeTraits.h @@ -0,0 +1,26 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#ifndef CEPH_LIBRBD_CACHE_TYPE_TRAITS_H +#define CEPH_LIBRBD_CACHE_TYPE_TRAITS_H + +namespace ceph { +namespace immutable_obj_cache { + +class CacheClient; + +} // namespace immutable_obj_cache +} // namespace ceph + +namespace librbd { +namespace cache { + +template +struct TypeTraits { + typedef ceph::immutable_obj_cache::CacheClient CacheClient; +}; + +} // namespace librbd +} // namespace cache + +#endif diff --git a/src/test/librbd/cache/test_mock_ParentImageCache.cc b/src/test/librbd/cache/test_mock_ParentImageCache.cc index d37e48e2ed4..b8916107e85 100644 --- a/src/test/librbd/cache/test_mock_ParentImageCache.cc +++ b/src/test/librbd/cache/test_mock_ParentImageCache.cc @@ -31,11 +31,20 @@ struct MockParentImageCacheImageCtx : public MockImageCtx { }; // anonymous namespace +namespace cache { + +template<> +struct TypeTraits { + typedef ceph::immutable_obj_cache::MockCacheClient CacheClient; }; +}; // namespace cache + +}; // namespace librbd + #include "librbd/cache/ParentCacheObjectDispatch.cc" #include "librbd/cache/SharedPersistentObjectCacher.cc" -template class librbd::cache::ParentCacheObjectDispatch; +template class librbd::cache::ParentCacheObjectDispatch; template class librbd::cache::SharedPersistentObjectCacher; namespace librbd { @@ -50,18 +59,18 @@ using ::testing::WithArgs; class TestMockParentImageCache : public TestMockFixture { public : - typedef cache::ParentCacheObjectDispatch MockParentImageCache; + typedef cache::ParentCacheObjectDispatch MockParentImageCache; // ====== mock cache client ==== void expect_cache_run(MockParentImageCache& mparent_image_cache, bool ret_val) { - auto& expect = EXPECT_CALL(*(mparent_image_cache.m_cache_client), run()); + auto& expect = EXPECT_CALL(*(mparent_image_cache.get_cache_client()), run()); expect.WillOnce((Invoke([ret_val]() { }))); } void expect_cache_session_state(MockParentImageCache& mparent_image_cache, bool ret_val) { - auto & expect = EXPECT_CALL(*(mparent_image_cache.m_cache_client), is_session_work()); + auto & expect = EXPECT_CALL(*(mparent_image_cache.get_cache_client()), is_session_work()); expect.WillOnce((Invoke([ret_val]() { return ret_val; @@ -69,7 +78,7 @@ class TestMockParentImageCache : public TestMockFixture { } void expect_cache_connect(MockParentImageCache& mparent_image_cache, int ret_val) { - auto& expect = EXPECT_CALL(*(mparent_image_cache.m_cache_client), connect()); + auto& expect = EXPECT_CALL(*(mparent_image_cache.get_cache_client()), connect()); expect.WillOnce((Invoke([ret_val]() { return ret_val; @@ -78,7 +87,7 @@ class TestMockParentImageCache : public TestMockFixture { void expect_cache_lookup_object(MockParentImageCache& mparent_image_cache, Context* on_finish) { - auto& expect = EXPECT_CALL(*(mparent_image_cache.m_cache_client), + auto& expect = EXPECT_CALL(*(mparent_image_cache.get_cache_client()), internal_lookup(_, _, _, _)); expect.WillOnce(WithArg<3>(Invoke([on_finish](std::string oid) { @@ -87,21 +96,21 @@ class TestMockParentImageCache : public TestMockFixture { } void expect_cache_close(MockParentImageCache& mparent_image_cache, int ret_val) { - auto& expect = EXPECT_CALL(*(mparent_image_cache.m_cache_client), close()); + auto& expect = EXPECT_CALL(*(mparent_image_cache.get_cache_client()), close()); expect.WillOnce((Invoke([ret_val]() { }))); } void expect_cache_stop(MockParentImageCache& mparent_image_cache, int ret_val) { - auto& expect = EXPECT_CALL(*(mparent_image_cache.m_cache_client), stop()); + auto& expect = EXPECT_CALL(*(mparent_image_cache.get_cache_client()), stop()); expect.WillOnce((Invoke([ret_val]() { }))); } void expect_cache_register(MockParentImageCache& mparent_image_cache, Context* mock_handle_register, int ret_val) { - auto& expect = EXPECT_CALL(*(mparent_image_cache.m_cache_client), register_client(_)); + auto& expect = EXPECT_CALL(*(mparent_image_cache.get_cache_client()), register_client(_)); expect.WillOnce(WithArg<0>(Invoke([mock_handle_register, ret_val](Context* ctx) { if(ret_val == 0) { @@ -116,7 +125,7 @@ class TestMockParentImageCache : public TestMockFixture { void expect_io_object_dispatcher_register_state(MockParentImageCache& mparent_image_cache, int ret_val) { - auto& expect = EXPECT_CALL((*(mparent_image_cache.m_image_ctx->io_object_dispatcher)), + auto& expect = EXPECT_CALL((*(mparent_image_cache.get_image_ctx()->io_object_dispatcher)), register_object_dispatch(_)); expect.WillOnce(WithArg<0>(Invoke([ret_val, &mparent_image_cache] @@ -133,9 +142,6 @@ TEST_F(TestMockParentImageCache, test_initialization_success) { auto mock_parent_image_cache = MockParentImageCache::create(&mock_image_ctx); - // will be released by MockParentImageCache's destruction. - mock_parent_image_cache->m_cache_client = new MockCacheClient("/cache/path", ictx->cct); - expect_cache_run(*mock_parent_image_cache, 0); expect_cache_connect(*mock_parent_image_cache, 0); Context* ctx = new FunctionContext([](bool reg) { @@ -152,10 +158,10 @@ TEST_F(TestMockParentImageCache, test_initialization_success) { ASSERT_EQ(mock_parent_image_cache->get_object_dispatch_layer(), io::OBJECT_DISPATCH_LAYER_PARENT_CACHE); ASSERT_EQ(mock_parent_image_cache->get_state(), true); - ASSERT_EQ(mock_parent_image_cache->m_cache_client->is_session_work(), true); + ASSERT_EQ(mock_parent_image_cache->get_cache_client()->is_session_work(), true); - mock_parent_image_cache->m_cache_client->close(); - mock_parent_image_cache->m_cache_client->stop(); + mock_parent_image_cache->get_cache_client()->close(); + mock_parent_image_cache->get_cache_client()->stop(); delete mock_parent_image_cache; } @@ -167,8 +173,6 @@ TEST_F(TestMockParentImageCache, test_initialization_fail_at_connect) { auto mock_parent_image_cache = MockParentImageCache::create(&mock_image_ctx); - mock_parent_image_cache->m_cache_client = new MockCacheClient("/cache/path", ictx->cct); - expect_cache_run(*mock_parent_image_cache, 0); expect_cache_connect(*mock_parent_image_cache, -1); expect_cache_session_state(*mock_parent_image_cache, false); @@ -181,10 +185,10 @@ TEST_F(TestMockParentImageCache, test_initialization_fail_at_connect) { ASSERT_EQ(mock_parent_image_cache->get_object_dispatch_layer(), io::OBJECT_DISPATCH_LAYER_PARENT_CACHE); ASSERT_EQ(mock_parent_image_cache->get_state(), false); - ASSERT_EQ(mock_parent_image_cache->m_cache_client->is_session_work(), false); + ASSERT_EQ(mock_parent_image_cache->get_cache_client()->is_session_work(), false); - mock_parent_image_cache->m_cache_client->close(); - mock_parent_image_cache->m_cache_client->stop(); + mock_parent_image_cache->get_cache_client()->close(); + mock_parent_image_cache->get_cache_client()->stop(); delete mock_parent_image_cache; @@ -197,8 +201,6 @@ TEST_F(TestMockParentImageCache, test_initialization_fail_at_register) { auto mock_parent_image_cache = MockParentImageCache::create(&mock_image_ctx); - mock_parent_image_cache->m_cache_client = new MockCacheClient("/cache/path", ictx->cct); - expect_cache_run(*mock_parent_image_cache, 0); expect_cache_connect(*mock_parent_image_cache, 0); Context* ctx = new FunctionContext([](bool reg) { @@ -214,10 +216,10 @@ TEST_F(TestMockParentImageCache, test_initialization_fail_at_register) { ASSERT_EQ(mock_parent_image_cache->get_object_dispatch_layer(), io::OBJECT_DISPATCH_LAYER_PARENT_CACHE); ASSERT_EQ(mock_parent_image_cache->get_state(), false); - ASSERT_EQ(mock_parent_image_cache->m_cache_client->is_session_work(), false); + ASSERT_EQ(mock_parent_image_cache->get_cache_client()->is_session_work(), false); - mock_parent_image_cache->m_cache_client->close(); - mock_parent_image_cache->m_cache_client->stop(); + mock_parent_image_cache->get_cache_client()->close(); + mock_parent_image_cache->get_cache_client()->stop(); delete mock_parent_image_cache; } @@ -269,11 +271,6 @@ TEST_F(TestMockParentImageCache, test_read) { auto mock_parent_image_cache = MockParentImageCache::create(&mock_image_ctx); - mock_parent_image_cache->m_cache_client = new MockCacheClient("/cache/path", ictx->cct); - - // will be released by MockParentImageCache's destruction. - mock_parent_image_cache->m_cache_client = new MockCacheClient("/cache/path", ictx->cct); - expect_cache_run(*mock_parent_image_cache, 0); expect_cache_connect(*mock_parent_image_cache, 0); Context* ctx = new FunctionContext([](bool reg) { @@ -290,12 +287,12 @@ TEST_F(TestMockParentImageCache, test_read) { ASSERT_EQ(mock_parent_image_cache->get_object_dispatch_layer(), io::OBJECT_DISPATCH_LAYER_PARENT_CACHE); ASSERT_EQ(mock_parent_image_cache->get_state(), true); - ASSERT_EQ(mock_parent_image_cache->m_cache_client->is_session_work(), true); + ASSERT_EQ(mock_parent_image_cache->get_cache_client()->is_session_work(), true); C_SaferCond cond; Context* on_finish = &cond; - auto& expect = EXPECT_CALL(*(mock_parent_image_cache->m_cache_client), is_session_work()) + auto& expect = EXPECT_CALL(*(mock_parent_image_cache->get_cache_client()), is_session_work()) .WillOnce(Return(true)); expect_cache_lookup_object(*mock_parent_image_cache, on_finish); @@ -303,8 +300,8 @@ TEST_F(TestMockParentImageCache, test_read) { nullptr, nullptr, nullptr, nullptr, &on_finish, nullptr); ASSERT_EQ(0, cond.wait()); - mock_parent_image_cache->m_cache_client->close(); - mock_parent_image_cache->m_cache_client->stop(); + mock_parent_image_cache->get_cache_client()->close(); + mock_parent_image_cache->get_cache_client()->stop(); delete mock_parent_image_cache; } -- 2.39.5