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 <dehao.shang@intel.com>

diff --git a/src/librbd/cache/ParentCacheObjectDispatch.cc b/src/librbd/cache/ParentCacheObjectDispatch.cc
index 251e8b5505e09614aef430ad7c9fe226c8cd721c..2581fd8852367595d477d724654f9b055cc095a2 100644 (file)
--- 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 <typename I, typename C>
-ParentCacheObjectDispatch<I, C>::ParentCacheObjectDispatch(
+template <typename I>
+ParentCacheObjectDispatch<I>::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<std::string>("immutable_object_cache_sock");
+  m_cache_client = new CacheClient(controller_path.c_str(), m_image_ctx->cct);
 }
 
-template <typename I, typename C>
-ParentCacheObjectDispatch<I, C>::~ParentCacheObjectDispatch() {
+template <typename I>
+ParentCacheObjectDispatch<I>::~ParentCacheObjectDispatch() {
     delete m_object_store;
     delete m_cache_client;
 }
 
 // TODO if connect fails, init will return error to high layer.
-template <typename I, typename C>
-void ParentCacheObjectDispatch<I, C>::init() {
+template <typename I>
+void ParentCacheObjectDispatch<I>::init() {
   auto cct = m_image_ctx->cct;
   ldout(cct, 5) << dendl;
 
@@ -47,10 +52,6 @@ void ParentCacheObjectDispatch<I, C>::init() {
 
   ldout(cct, 5) << "parent image: setup SRO cache client" << dendl;
 
-  std::string controller_path = ((CephContext*)cct)->_conf.get_val<std::string>("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<I, C>::init() {
   }
 }
 
-template <typename I, typename C>
-bool ParentCacheObjectDispatch<I, C>::read(
+template <typename I>
+bool ParentCacheObjectDispatch<I>::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<I, C>::read(
   return true;
 }
 
-template <typename I, typename C>
-void ParentCacheObjectDispatch<I, C>::handle_read_cache(
+template <typename I>
+void ParentCacheObjectDispatch<I>::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<I, C>::handle_read_cache(
   on_dispatched->complete(r);
 }
 
-template <typename I, typename C>
-int ParentCacheObjectDispatch<I, C>::handle_register_client(bool reg) {
+template <typename I>
+int ParentCacheObjectDispatch<I>::handle_register_client(bool reg) {
   auto cct = m_image_ctx->cct;
   ldout(cct, 20) << dendl;
 
@@ -157,4 +158,4 @@ int ParentCacheObjectDispatch<I, C>::handle_register_client(bool reg) {
 } // namespace cache
 } // namespace librbd
 
-template class librbd::cache::ParentCacheObjectDispatch<librbd::ImageCtx, ceph::immutable_obj_cache::CacheClient>;
+template class librbd::cache::ParentCacheObjectDispatch<librbd::ImageCtx>;

diff --git a/src/librbd/cache/ParentCacheObjectDispatch.h b/src/librbd/cache/ParentCacheObjectDispatch.h
index 5a10d5754e9c610f531236639fafc4c8e8c51fc2..9fcc1da5c56c314fc01d3aa3a2d9b5cf7c8fe0c9 100644 (file)
--- 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 <typename ImageCtxT = ImageCtx, typename CacheClientT = CacheClient>
+template <typename ImageCtxT = ImageCtx>
 class ParentCacheObjectDispatch : public io::ObjectDispatchInterface {
+  // mock unit testing support
+  typedef cache::TypeTraits<ImageCtxT> 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<ImageCtxT> *m_object_store = nullptr;
   bool m_initialized;
 };
@@ -127,6 +137,6 @@ private:
 } // namespace cache
 } // namespace librbd
 
-extern template class librbd::cache::ParentCacheObjectDispatch<librbd::ImageCtx, ceph::immutable_obj_cache::CacheClient>;
+extern template class librbd::cache::ParentCacheObjectDispatch<librbd::ImageCtx>;
 
 #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 (file)
index 0000000..dd7075e
--- /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 <typename ImageCtxT>
+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 d37e48e2ed43d68483790fe3bac40df07f194cd5..b8916107e85c539abbdbfc77393e97a9d671d8bf 100644 (file)
--- 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<MockParentImageCacheImageCtx> {
+  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<librbd::MockParentImageCacheImageCtx, MockCacheClient>;
+template class librbd::cache::ParentCacheObjectDispatch<librbd::MockParentImageCacheImageCtx>;
 template class librbd::cache::SharedPersistentObjectCacher<librbd::MockParentImageCacheImageCtx>;
 
 namespace librbd {
@@ -50,18 +59,18 @@ using ::testing::WithArgs;
 
 class TestMockParentImageCache : public TestMockFixture {
   public : 
-  typedef cache::ParentCacheObjectDispatch<librbd::MockParentImageCacheImageCtx, MockCacheClient> MockParentImageCache;
+  typedef cache::ParentCacheObjectDispatch<librbd::MockParentImageCacheImageCtx> 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;
 }