From 7b1a1109eac37620e5ae1b24412d1bb7dd0744e3 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Tue, 25 Aug 2020 15:06:06 -0400 Subject: [PATCH] librbd: add internal API for plugin libraries To avoid the need to expose internal librbd symbols as public, expose a select set of symbols via an internal plugin API for use by the plugins. Fixes: https://tracker.ceph.com/issues/46916 Signed-off-by: Jason Dillaman --- src/librbd/CMakeLists.txt | 35 ++++---- src/librbd/PluginRegistry.cc | 10 ++- src/librbd/PluginRegistry.h | 6 ++ src/librbd/cache/ParentCacheObjectDispatch.cc | 10 +-- src/librbd/cache/ParentCacheObjectDispatch.h | 11 ++- src/librbd/plugin/Api.cc | 23 ++++++ src/librbd/plugin/Api.h | 39 +++++++++ src/librbd/plugin/ParentCache.cc | 5 +- src/librbd/plugin/ParentCache.h | 2 +- src/librbd/plugin/Types.h | 6 +- .../test_mock_ParentCacheObjectDispatch.cc | 79 ++++++++----------- 11 files changed, 150 insertions(+), 76 deletions(-) create mode 100644 src/librbd/plugin/Api.cc create mode 100644 src/librbd/plugin/Api.h diff --git a/src/librbd/CMakeLists.txt b/src/librbd/CMakeLists.txt index 2f8bb82fa1b..638abc37215 100644 --- a/src/librbd/CMakeLists.txt +++ b/src/librbd/CMakeLists.txt @@ -167,28 +167,13 @@ set(librbd_internal_srcs operation/SnapshotLimitRequest.cc operation/SparsifyRequest.cc operation/TrimRequest.cc + plugin/Api.cc trash/MoveRequest.cc trash/RemoveRequest.cc watcher/Notifier.cc watcher/RewatchRequest.cc ${CMAKE_SOURCE_DIR}/src/common/ContextCompletion.cc) -add_custom_target(librbd_plugins) -set(librbd_plugins_dir ${CEPH_INSTALL_PKGLIBDIR}/librbd) - -set(rbd_plugin_parent_cache_srcs - cache/ParentCacheObjectDispatch.cc - plugin/ParentCache.cc) -add_library(librbd_plugin_parent_cache SHARED ${rbd_plugin_parent_cache_srcs}) -target_link_libraries(librbd_plugin_parent_cache PRIVATE - ceph_immutable_object_cache_lib) -set_target_properties(librbd_plugin_parent_cache PROPERTIES - OUTPUT_NAME ceph_librbd_parent_cache - VERSION 1.0.0 - SOVERSION 1) -install(TARGETS librbd_plugin_parent_cache DESTINATION ${librbd_plugins_dir}) -add_dependencies(librbd_plugins librbd_plugin_parent_cache) - if(WITH_EVENTTRACE) list(APPEND librbd_internal_srcs ../common/EventTrace.cc) endif() @@ -228,6 +213,24 @@ if(WITH_RBD_RWL) PUBLIC blk) endif() +add_custom_target(librbd_plugins) +set(librbd_plugins_dir ${CEPH_INSTALL_PKGLIBDIR}/librbd) + +set(rbd_plugin_parent_cache_srcs + cache/ParentCacheObjectDispatch.cc + plugin/ParentCache.cc) +add_library(librbd_plugin_parent_cache SHARED + ${rbd_plugin_parent_cache_srcs}) +target_link_libraries(librbd_plugin_parent_cache PRIVATE + ceph_immutable_object_cache_lib + librados) +set_target_properties(librbd_plugin_parent_cache PROPERTIES + OUTPUT_NAME ceph_librbd_parent_cache + VERSION 1.0.0 + SOVERSION 1) +install(TARGETS librbd_plugin_parent_cache DESTINATION ${librbd_plugins_dir}) +add_dependencies(librbd_plugins librbd_plugin_parent_cache) + add_library(librbd ${CEPH_SHARED} librbd.cc) if(WITH_LTTNG) diff --git a/src/librbd/PluginRegistry.cc b/src/librbd/PluginRegistry.cc index 90ebdbb203e..a4dabb7d33e 100644 --- a/src/librbd/PluginRegistry.cc +++ b/src/librbd/PluginRegistry.cc @@ -5,6 +5,7 @@ #include "include/Context.h" #include "common/dout.h" #include "librbd/ImageCtx.h" +#include "librbd/plugin/Api.h" #include #define dout_subsys ceph_subsys_rbd @@ -15,7 +16,12 @@ namespace librbd { template -PluginRegistry::PluginRegistry(I* image_ctx) : m_image_ctx(image_ctx) { +PluginRegistry::PluginRegistry(I* image_ctx) + : m_image_ctx(image_ctx), m_plugin_api(std::make_unique>()) { +} + +template +PluginRegistry::~PluginRegistry() { } template @@ -41,7 +47,7 @@ void PluginRegistry::init(const std::string& plugins, Context* on_finish) { m_plugin_hook_points.emplace_back(); auto hook_points = &m_plugin_hook_points.back(); - plugin->init(m_image_ctx, hook_points, ctx); + plugin->init(m_image_ctx, *m_plugin_api, hook_points, ctx); } gather_ctx->activate(); diff --git a/src/librbd/PluginRegistry.h b/src/librbd/PluginRegistry.h index 2e975f89d6b..ca343fdd814 100644 --- a/src/librbd/PluginRegistry.h +++ b/src/librbd/PluginRegistry.h @@ -5,6 +5,7 @@ #define CEPH_LIBRBD_PLUGIN_REGISTRY_H #include "librbd/plugin/Types.h" +#include #include #include @@ -14,10 +15,13 @@ namespace librbd { struct ImageCtx; +namespace plugin { template struct Api; } + template class PluginRegistry { public: PluginRegistry(ImageCtxT* image_ctx); + ~PluginRegistry(); void init(const std::string& plugins, Context* on_finish); @@ -25,6 +29,8 @@ private: typedef std::list PluginHookPoints; ImageCtxT* m_image_ctx; + std::unique_ptr> m_plugin_api; + std::string m_plugins; PluginHookPoints m_plugin_hook_points; diff --git a/src/librbd/cache/ParentCacheObjectDispatch.cc b/src/librbd/cache/ParentCacheObjectDispatch.cc index 4ca8212516e..55754ebe157 100644 --- a/src/librbd/cache/ParentCacheObjectDispatch.cc +++ b/src/librbd/cache/ParentCacheObjectDispatch.cc @@ -5,10 +5,10 @@ #include "librbd/ImageCtx.h" #include "librbd/Utils.h" #include "librbd/asio/ContextWQ.h" +#include "librbd/cache/ParentCacheObjectDispatch.h" #include "librbd/io/ObjectDispatchSpec.h" #include "librbd/io/ObjectDispatcherInterface.h" -#include "librbd/io/Utils.h" -#include "librbd/cache/ParentCacheObjectDispatch.h" +#include "librbd/plugin/Api.h" #include "osd/osd_types.h" #include "osdc/WritebackHandler.h" @@ -27,8 +27,8 @@ namespace cache { template ParentCacheObjectDispatch::ParentCacheObjectDispatch( - I* image_ctx) - : m_image_ctx(image_ctx), + I* image_ctx, plugin::Api& plugin_api) + : m_image_ctx(image_ctx), m_plugin_api(plugin_api), m_lock(ceph::make_mutex( "librbd::cache::ParentCacheObjectDispatch::lock", true, false)) { ceph_assert(m_image_ctx->data_ctx.is_valid()); @@ -136,7 +136,7 @@ void ParentCacheObjectDispatch::handle_read_cache( *dispatch_result = io::DISPATCH_RESULT_COMPLETE; on_dispatched->complete(r); }); - io::util::read_parent(m_image_ctx, object_no, {{read_off, read_len}}, + m_plugin_api.read_parent(m_image_ctx, object_no, {{read_off, read_len}}, snap_id, parent_trace, read_data, ctx); return; } diff --git a/src/librbd/cache/ParentCacheObjectDispatch.h b/src/librbd/cache/ParentCacheObjectDispatch.h index 314c9a585de..ec8966b1ae6 100644 --- a/src/librbd/cache/ParentCacheObjectDispatch.h +++ b/src/librbd/cache/ParentCacheObjectDispatch.h @@ -14,6 +14,8 @@ namespace librbd { class ImageCtx; +namespace plugin { template struct Api; } + namespace cache { template @@ -23,11 +25,13 @@ class ParentCacheObjectDispatch : public io::ObjectDispatchInterface { typedef typename TypeTraits::CacheClient CacheClient; public: - static ParentCacheObjectDispatch* create(ImageCtxT* image_ctx) { - return new ParentCacheObjectDispatch(image_ctx); + static ParentCacheObjectDispatch* create(ImageCtxT* image_ctx, + plugin::Api& plugin_api) { + return new ParentCacheObjectDispatch(image_ctx, plugin_api); } - ParentCacheObjectDispatch(ImageCtxT* image_ctx); + ParentCacheObjectDispatch(ImageCtxT* image_ctx, + plugin::Api& plugin_api); ~ParentCacheObjectDispatch() override; io::ObjectDispatchLayer get_dispatch_layer() const override { @@ -129,6 +133,7 @@ private: void create_cache_session(Context* on_finish, bool is_reconnect); ImageCtxT* m_image_ctx; + plugin::Api& m_plugin_api; ceph::mutex m_lock; CacheClient *m_cache_client = nullptr; diff --git a/src/librbd/plugin/Api.cc b/src/librbd/plugin/Api.cc new file mode 100644 index 00000000000..5e96c97c50d --- /dev/null +++ b/src/librbd/plugin/Api.cc @@ -0,0 +1,23 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#include "librbd/plugin/Api.h" +#include "librbd/ImageCtx.h" +#include "librbd/io/Utils.h" + +namespace librbd { +namespace plugin { + +template +void Api::read_parent( + I *image_ctx, uint64_t object_no, const Extents &extents, + librados::snap_t snap_id, const ZTracer::Trace &trace, + ceph::bufferlist* data, Context* on_finish) { + io::util::read_parent(image_ctx, object_no, extents, snap_id, trace, data, + on_finish); +} + +} // namespace plugin +} // namespace librbd + +template class librbd::plugin::Api; diff --git a/src/librbd/plugin/Api.h b/src/librbd/plugin/Api.h new file mode 100644 index 00000000000..1cba7b870d2 --- /dev/null +++ b/src/librbd/plugin/Api.h @@ -0,0 +1,39 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#ifndef CEPH_LIBRBD_PLUGIN_API_H +#define CEPH_LIBRBD_PLUGIN_API_H + +#include "include/common_fwd.h" +#include "include/int_types.h" +#include "include/rados/librados.hpp" +#include "librbd/io/Types.h" + +namespace ZTracer { struct Trace; } + +namespace librbd { + +struct ImageCtx; + +namespace plugin { + +template +struct Api { + using Extents = librbd::io::Extents; + + Api() {} + virtual ~Api() {} + + virtual void read_parent( + ImageCtxT *image_ctx, uint64_t object_no, const Extents &extents, + librados::snap_t snap_id, const ZTracer::Trace &trace, + ceph::bufferlist* data, Context* on_finish); + +}; + +} // namespace plugin +} // namespace librbd + +extern template class librbd::plugin::Api; + +#endif // CEPH_LIBRBD_PLUGIN_API_H diff --git a/src/librbd/plugin/ParentCache.cc b/src/librbd/plugin/ParentCache.cc index b462d0e63b3..9e1197933f5 100644 --- a/src/librbd/plugin/ParentCache.cc +++ b/src/librbd/plugin/ParentCache.cc @@ -33,7 +33,7 @@ namespace librbd { namespace plugin { template -void ParentCache::init(I* image_ctx, HookPoints* hook_points, +void ParentCache::init(I* image_ctx, Api& api, HookPoints* hook_points, Context* on_finish) { m_image_ctx = image_ctx; bool parent_cache_enabled = m_image_ctx->config.template get_val( @@ -46,7 +46,8 @@ void ParentCache::init(I* image_ctx, HookPoints* hook_points, auto cct = m_image_ctx->cct; ldout(cct, 5) << dendl; - auto parent_cache = cache::ParentCacheObjectDispatch::create(m_image_ctx); + auto parent_cache = cache::ParentCacheObjectDispatch::create( + m_image_ctx, api); on_finish = new LambdaContext([this, on_finish, parent_cache](int r) { if (r < 0) { // the object dispatcher will handle cleanup if successfully initialized diff --git a/src/librbd/plugin/ParentCache.h b/src/librbd/plugin/ParentCache.h index 0bb639e3b38..e456c5ac3d8 100644 --- a/src/librbd/plugin/ParentCache.h +++ b/src/librbd/plugin/ParentCache.h @@ -19,7 +19,7 @@ public: ParentCache(CephContext* cct) : Interface(cct) { } - void init(ImageCtxT* image_ctx, HookPoints* hook_points, + void init(ImageCtxT* image_ctx, Api& api, HookPoints* hook_points, Context* on_finish) override; private: diff --git a/src/librbd/plugin/Types.h b/src/librbd/plugin/Types.h index fb309b310de..af60f49ef63 100644 --- a/src/librbd/plugin/Types.h +++ b/src/librbd/plugin/Types.h @@ -12,6 +12,8 @@ struct Context; namespace librbd { namespace plugin { +template struct Api; + struct HookPoints { // TODO later commits will add support for exclusive-lock hook points }; @@ -21,8 +23,8 @@ struct Interface : public ceph::Plugin { Interface(CephContext* cct) : Plugin(cct) { } - virtual void init(ImageCtxT* image_ctx, HookPoints* hook_points, - Context* on_finish) = 0; + virtual void init(ImageCtxT* image_ctx, Api& api, + HookPoints* hook_points, Context* on_finish) = 0; }; } // namespace plugin diff --git a/src/test/librbd/cache/test_mock_ParentCacheObjectDispatch.cc b/src/test/librbd/cache/test_mock_ParentCacheObjectDispatch.cc index f66cf92e24e..d98a56fad59 100644 --- a/src/test/librbd/cache/test_mock_ParentCacheObjectDispatch.cc +++ b/src/test/librbd/cache/test_mock_ParentCacheObjectDispatch.cc @@ -9,8 +9,8 @@ #include "include/Context.h" #include "tools/immutable_object_cache/CacheClient.h" #include "test/immutable_object_cache/MockCacheDaemon.h" -#include "librbd/io/Utils.h" #include "librbd/cache/ParentCacheObjectDispatch.h" +#include "librbd/plugin/Api.h" #include "test/librbd/test_mock_fixture.h" #include "test/librbd/mock/MockImageCtx.h" @@ -27,7 +27,7 @@ struct MockParentImageCacheImageCtx : public MockImageCtx { ~MockParentImageCacheImageCtx() {} }; -}; // anonymous namespace +} // anonymous namespace namespace cache { @@ -36,42 +36,20 @@ struct TypeTraits { typedef ceph::immutable_obj_cache::MockCacheClient CacheClient; }; -}; // namespace cache +} // namespace cache -namespace io { -namespace util { +namespace plugin { -namespace { - -struct Mock { - static Mock* s_instance; - - Mock() { - s_instance = this; - } - - MOCK_METHOD7(read_parent, - void(librbd::MockParentImageCacheImageCtx *, uint64_t, - const io::Extents &, librados::snap_t, const ZTracer::Trace &, - ceph::bufferlist*, Context*)); +template <> +struct Api { + MOCK_METHOD7(read_parent, void(MockParentImageCacheImageCtx*, uint64_t, + const librbd::io::Extents &, librados::snap_t, + const ZTracer::Trace &, ceph::bufferlist*, + Context*)); }; -Mock *Mock::s_instance = nullptr; - -} // anonymous namespace - -template<> void read_parent( - librbd::MockParentImageCacheImageCtx *image_ctx, uint64_t object_no, - const io::Extents &extents, librados::snap_t snap_id, - const ZTracer::Trace &trace, ceph::bufferlist* data, Context* on_finish) { - Mock::s_instance->read_parent(image_ctx, object_no, extents, snap_id, trace, - data, on_finish); -} - -} // namespace util -} // namespace io - -}; // namespace librbd +} // namespace plugin +} // namespace librbd #include "librbd/cache/ParentCacheObjectDispatch.cc" template class librbd::cache::ParentCacheObjectDispatch; @@ -89,7 +67,7 @@ using ::testing::WithArgs; class TestMockParentCacheObjectDispatch : public TestMockFixture { public : typedef cache::ParentCacheObjectDispatch MockParentImageCache; - typedef io::util::Mock MockUtils; + typedef plugin::Api MockPluginApi; // ====== mock cache client ==== void expect_cache_run(MockParentImageCache& mparent_image_cache, bool ret_val) { @@ -135,10 +113,10 @@ public : }))); } - void expect_read_parent(MockUtils &mock_utils, uint64_t object_no, + void expect_read_parent(MockPluginApi &mock_plugin_api, uint64_t object_no, const io::Extents &extents, librados::snap_t snap_id, int r) { - EXPECT_CALL(mock_utils, + EXPECT_CALL(mock_plugin_api, read_parent(_, object_no, extents, snap_id, _, _, _)) .WillOnce(WithArg<6>(CompleteContext(r, static_cast(nullptr)))); } @@ -189,7 +167,9 @@ TEST_F(TestMockParentCacheObjectDispatch, test_initialization_success) { MockParentImageCacheImageCtx mock_image_ctx(*ictx); mock_image_ctx.child = &mock_image_ctx; - auto mock_parent_image_cache = MockParentImageCache::create(&mock_image_ctx); + MockPluginApi mock_plugin_api; + auto mock_parent_image_cache = MockParentImageCache::create(&mock_image_ctx, + mock_plugin_api); expect_cache_run(*mock_parent_image_cache, 0); C_SaferCond cond; @@ -226,7 +206,9 @@ TEST_F(TestMockParentCacheObjectDispatch, test_initialization_fail_at_connect) { MockParentImageCacheImageCtx mock_image_ctx(*ictx); mock_image_ctx.child = &mock_image_ctx; - auto mock_parent_image_cache = MockParentImageCache::create(&mock_image_ctx); + MockPluginApi mock_plugin_api; + auto mock_parent_image_cache = MockParentImageCache::create(&mock_image_ctx, + mock_plugin_api); expect_cache_run(*mock_parent_image_cache, 0); C_SaferCond cond; @@ -260,7 +242,9 @@ TEST_F(TestMockParentCacheObjectDispatch, test_initialization_fail_at_register) MockParentImageCacheImageCtx mock_image_ctx(*ictx); mock_image_ctx.child = &mock_image_ctx; - auto mock_parent_image_cache = MockParentImageCache::create(&mock_image_ctx); + MockPluginApi mock_plugin_api; + auto mock_parent_image_cache = MockParentImageCache::create(&mock_image_ctx, + mock_plugin_api); expect_cache_run(*mock_parent_image_cache, 0); C_SaferCond cond; @@ -297,7 +281,9 @@ TEST_F(TestMockParentCacheObjectDispatch, test_disble_interface) { MockParentImageCacheImageCtx mock_image_ctx(*ictx); mock_image_ctx.child = &mock_image_ctx; - auto mock_parent_image_cache = MockParentImageCache::create(&mock_image_ctx); + MockPluginApi mock_plugin_api; + auto mock_parent_image_cache = MockParentImageCache::create(&mock_image_ctx, + mock_plugin_api); std::string temp_oid("12345"); ceph::bufferlist temp_bl; @@ -337,7 +323,9 @@ TEST_F(TestMockParentCacheObjectDispatch, test_read) { MockParentImageCacheImageCtx mock_image_ctx(*ictx); mock_image_ctx.child = &mock_image_ctx; - auto mock_parent_image_cache = MockParentImageCache::create(&mock_image_ctx); + MockPluginApi mock_plugin_api; + auto mock_parent_image_cache = MockParentImageCache::create(&mock_image_ctx, + mock_plugin_api); expect_cache_run(*mock_parent_image_cache, 0); C_SaferCond conn_cond; @@ -386,7 +374,9 @@ TEST_F(TestMockParentCacheObjectDispatch, test_read_dne) { MockParentImageCacheImageCtx mock_image_ctx(*ictx); mock_image_ctx.child = &mock_image_ctx; - auto mock_parent_image_cache = MockParentImageCache::create(&mock_image_ctx); + MockPluginApi mock_plugin_api; + auto mock_parent_image_cache = MockParentImageCache::create(&mock_image_ctx, + mock_plugin_api); expect_cache_run(*mock_parent_image_cache, 0); C_SaferCond conn_cond; @@ -417,8 +407,7 @@ TEST_F(TestMockParentCacheObjectDispatch, test_read_dne) { expect_cache_lookup_object(*mock_parent_image_cache, ""); - MockUtils mock_utils; - expect_read_parent(mock_utils, 0, {{0, 4096}}, CEPH_NOSNAP, 0); + expect_read_parent(mock_plugin_api, 0, {{0, 4096}}, CEPH_NOSNAP, 0); C_SaferCond on_dispatched; io::DispatchResult dispatch_result;