From dd253af868c31520ac6885af475cd4eb34f48dbb Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Thu, 3 Dec 2015 21:52:41 -0500 Subject: [PATCH] librbd: convert object_map::InvalidateRequest to template Signed-off-by: Jason Dillaman --- src/librbd/ObjectMap.cc | 2 +- src/librbd/object_map/InvalidateRequest.cc | 50 ++++++++++++------- src/librbd/object_map/InvalidateRequest.h | 18 ++++--- src/librbd/object_map/Request.cc | 5 +- .../object_map/SnapshotRemoveRequest.cc | 6 +-- .../object_map/SnapshotRollbackRequest.cc | 5 +- src/test/Makefile-client.am | 3 +- .../object_map/mock/MockInvalidateRequest.h | 42 ++++++++++++++++ .../object_map/test_mock_InvalidateRequest.cc | 10 ++-- 9 files changed, 103 insertions(+), 38 deletions(-) create mode 100644 src/test/librbd/object_map/mock/MockInvalidateRequest.h diff --git a/src/librbd/ObjectMap.cc b/src/librbd/ObjectMap.cc index 62a9c35d8eba0..aaacd9b08052c 100644 --- a/src/librbd/ObjectMap.cc +++ b/src/librbd/ObjectMap.cc @@ -387,7 +387,7 @@ void ObjectMap::invalidate(uint64_t snap_id, bool force) { // TODO remove once all methods are async C_SaferCond cond_ctx; - object_map::InvalidateRequest *req = new object_map::InvalidateRequest( + object_map::InvalidateRequest<> *req = new object_map::InvalidateRequest<>( m_image_ctx, m_snap_id, force, &cond_ctx); req->send(); diff --git a/src/librbd/object_map/InvalidateRequest.cc b/src/librbd/object_map/InvalidateRequest.cc index 34506e41027a9..7be26c63a7e58 100644 --- a/src/librbd/object_map/InvalidateRequest.cc +++ b/src/librbd/object_map/InvalidateRequest.cc @@ -14,58 +14,72 @@ namespace librbd { namespace object_map { -void InvalidateRequest::send() { - assert(m_image_ctx.owner_lock.is_locked()); - assert(m_image_ctx.snap_lock.is_wlocked()); +template +InvalidateRequest* InvalidateRequest::create(I &image_ctx, + uint64_t snap_id, bool force, + Context *on_finish) { + return new InvalidateRequest(image_ctx, snap_id, force, on_finish); +} + +template +void InvalidateRequest::send() { + I &image_ctx = this->m_image_ctx; + assert(image_ctx.owner_lock.is_locked()); + assert(image_ctx.snap_lock.is_wlocked()); uint64_t snap_flags; - int r = m_image_ctx.get_flags(m_snap_id, &snap_flags); + int r = image_ctx.get_flags(m_snap_id, &snap_flags); if (r < 0 || ((snap_flags & RBD_FLAG_OBJECT_MAP_INVALID) != 0)) { - async_complete(r); + this->async_complete(r); return; } - CephContext *cct = m_image_ctx.cct; + CephContext *cct = image_ctx.cct; lderr(cct) << this << " invalidating object map in-memory" << dendl; // update in-memory flags uint64_t flags = RBD_FLAG_OBJECT_MAP_INVALID; - if ((m_image_ctx.features & RBD_FEATURE_FAST_DIFF) != 0) { + if ((image_ctx.features & RBD_FEATURE_FAST_DIFF) != 0) { flags |= RBD_FLAG_FAST_DIFF_INVALID; } - r = m_image_ctx.update_flags(m_snap_id, flags, true); + r = image_ctx.update_flags(m_snap_id, flags, true); if (r < 0) { - async_complete(r); + this->async_complete(r); } // do not update on-disk flags if not image owner - if (m_image_ctx.image_watcher == NULL || - (m_image_ctx.image_watcher->is_lock_supported(m_image_ctx.snap_lock) && - !m_image_ctx.image_watcher->is_lock_owner() && !m_force)) { - async_complete(0); + if (image_ctx.image_watcher == NULL || + (image_ctx.image_watcher->is_lock_supported(image_ctx.snap_lock) && + !image_ctx.image_watcher->is_lock_owner() && !m_force)) { + this->async_complete(0); return; } lderr(cct) << this << " invalidating object map on-disk" << dendl; librados::ObjectWriteOperation op; if (m_snap_id == CEPH_NOSNAP && !m_force) { - m_image_ctx.image_watcher->assert_header_locked(&op); + image_ctx.image_watcher->assert_header_locked(&op); } cls_client::set_flags(&op, m_snap_id, flags, flags); - librados::AioCompletion *rados_completion = create_callback_completion(); - r = m_image_ctx.md_ctx.aio_operate(m_image_ctx.header_oid, rados_completion, + librados::AioCompletion *rados_completion = + this->create_callback_completion(); + r = image_ctx.md_ctx.aio_operate(image_ctx.header_oid, rados_completion, &op); assert(r == 0); rados_completion->release(); } -bool InvalidateRequest::should_complete(int r) { - CephContext *cct = m_image_ctx.cct; +template +bool InvalidateRequest::should_complete(int r) { + I &image_ctx = this->m_image_ctx; + CephContext *cct = image_ctx.cct; lderr(cct) << this << " " << __func__ << ": r=" << r << dendl; return true; } } // namespace object_map } // namespace librbd + +template class librbd::object_map::InvalidateRequest; diff --git a/src/librbd/object_map/InvalidateRequest.h b/src/librbd/object_map/InvalidateRequest.h index efc0d96c86d7c..b051379d0a315 100644 --- a/src/librbd/object_map/InvalidateRequest.h +++ b/src/librbd/object_map/InvalidateRequest.h @@ -15,18 +15,23 @@ class ImageCtx; namespace object_map { -class InvalidateRequest : public AsyncRequest<> { +template +class InvalidateRequest : public AsyncRequest { public: - InvalidateRequest(ImageCtx &image_ctx, uint64_t snap_id, bool force, + static InvalidateRequest* create(ImageCtxT &image_ctx, uint64_t snap_id, + bool force, Context *on_finish); + + InvalidateRequest(ImageCtxT &image_ctx, uint64_t snap_id, bool force, Context *on_finish) - : AsyncRequest(image_ctx, on_finish), m_snap_id(snap_id), m_force(force) { + : AsyncRequest(image_ctx, on_finish), + m_snap_id(snap_id), m_force(force) { } virtual void send(); protected: - virtual bool should_complete(int r); - virtual int filter_return_code(int r) const { + virtual bool should_complete(int r) override; + virtual int filter_return_code(int r) const override{ // never propagate an error back to the caller return 0; } @@ -34,10 +39,11 @@ protected: private: uint64_t m_snap_id; bool m_force; - }; } // namespace object_map } // namespace librbd +extern template class librbd::object_map::InvalidateRequest; + #endif // CEPH_LIBRBD_OBJECT_MAP_INVALIDATE_REQUEST_H diff --git a/src/librbd/object_map/Request.cc b/src/librbd/object_map/Request.cc index 5b129b97f3011..8a731e161be44 100644 --- a/src/librbd/object_map/Request.cc +++ b/src/librbd/object_map/Request.cc @@ -62,8 +62,9 @@ bool Request::invalidate() { RWLock::RLocker owner_locker(m_image_ctx.owner_lock); RWLock::WLocker snap_locker(m_image_ctx.snap_lock); - InvalidateRequest *req = new InvalidateRequest(m_image_ctx, m_snap_id, true, - create_callback_context()); + InvalidateRequest<> *req = new InvalidateRequest<>(m_image_ctx, m_snap_id, + true, + create_callback_context()); req->send(); return false; } diff --git a/src/librbd/object_map/SnapshotRemoveRequest.cc b/src/librbd/object_map/SnapshotRemoveRequest.cc index 6cc8987ed1b0b..c718af1da19ae 100644 --- a/src/librbd/object_map/SnapshotRemoveRequest.cc +++ b/src/librbd/object_map/SnapshotRemoveRequest.cc @@ -148,9 +148,9 @@ void SnapshotRemoveRequest::send_invalidate_next_map() { ldout(cct, 5) << this << " " << __func__ << dendl; m_state = STATE_INVALIDATE_NEXT_MAP; - InvalidateRequest *req = new InvalidateRequest(m_image_ctx, m_next_snap_id, - true, - create_callback_context()); + InvalidateRequest<> *req = new InvalidateRequest<>(m_image_ctx, + m_next_snap_id, true, + create_callback_context()); req->send(); } diff --git a/src/librbd/object_map/SnapshotRollbackRequest.cc b/src/librbd/object_map/SnapshotRollbackRequest.cc index 20cf5e376a455..9d4fc4a1c3ceb 100644 --- a/src/librbd/object_map/SnapshotRollbackRequest.cc +++ b/src/librbd/object_map/SnapshotRollbackRequest.cc @@ -121,8 +121,9 @@ void SnapshotRollbackRequest::send_invalidate_map() { ldout(cct, 5) << this << " " << __func__ << dendl; m_state = STATE_INVALIDATE_MAP; - InvalidateRequest *req = new InvalidateRequest(m_image_ctx, m_snap_id, false, - create_callback_context()); + InvalidateRequest<> *req = new InvalidateRequest<>(m_image_ctx, m_snap_id, + false, + create_callback_context()); req->send(); } diff --git a/src/test/Makefile-client.am b/src/test/Makefile-client.am index 00764b4ac41fd..533e81beebbe3 100644 --- a/src/test/Makefile-client.am +++ b/src/test/Makefile-client.am @@ -403,7 +403,8 @@ noinst_HEADERS += \ test/librbd/mock/MockImageCtx.h \ test/librbd/mock/MockImageWatcher.h \ test/librbd/mock/MockJournal.h \ - test/librbd/mock/MockObjectMap.h + test/librbd/mock/MockObjectMap.h \ + test/librbd/object_map/mock/MockInvalidateRequest.h if LINUX ceph_test_librbd_fsx_SOURCES = test/librbd/fsx.cc diff --git a/src/test/librbd/object_map/mock/MockInvalidateRequest.h b/src/test/librbd/object_map/mock/MockInvalidateRequest.h new file mode 100644 index 0000000000000..b7d02c6ea55d9 --- /dev/null +++ b/src/test/librbd/object_map/mock/MockInvalidateRequest.h @@ -0,0 +1,42 @@ +// -*- mode:C; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#include "librbd/object_map/InvalidateRequest.h" + +// template definitions +#include "librbd/object_map/InvalidateRequest.cc" + +namespace librbd { +namespace object_map { + +template <> +struct InvalidateRequest { + static std::list s_requests; + uint64_t snap_id; + bool force; + Context *on_finish; + + static InvalidateRequest* create(MockImageCtx &image_ctx, uint64_t snap_id, + bool force, Context *on_finish) { + assert(!s_requests.empty()); + InvalidateRequest* req = s_requests.front(); + req->snap_id = snap_id; + req->force = force; + req->on_finish = on_finish; + s_requests.pop_front(); + return req; + } + + InvalidateRequest() { + s_requests.push_back(this); + } + + MOCK_METHOD0(send, void()); +}; + +typedef InvalidateRequest MockInvalidateRequest; + +std::list*> InvalidateRequest::s_requests; + +} // namespace object_map +} // namespace librbd diff --git a/src/test/librbd/object_map/test_mock_InvalidateRequest.cc b/src/test/librbd/object_map/test_mock_InvalidateRequest.cc index 97a6eb53ace27..9c0f4ea6f4c4d 100644 --- a/src/test/librbd/object_map/test_mock_InvalidateRequest.cc +++ b/src/test/librbd/object_map/test_mock_InvalidateRequest.cc @@ -28,7 +28,7 @@ TEST_F(TestMockObjectMapInvalidateRequest, UpdatesInMemoryFlag) { ASSERT_FALSE(ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID)); C_SaferCond cond_ctx; - AsyncRequest<> *request = new InvalidateRequest(*ictx, CEPH_NOSNAP, false, &cond_ctx); + AsyncRequest<> *request = new InvalidateRequest<>(*ictx, CEPH_NOSNAP, false, &cond_ctx); EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx), exec(ictx->header_oid, _, "rbd", "set_flags", _, _, _)) @@ -52,7 +52,7 @@ TEST_F(TestMockObjectMapInvalidateRequest, UpdatesHeadOnDiskFlag) { ASSERT_EQ(0, acquire_exclusive_lock(*ictx)); C_SaferCond cond_ctx; - AsyncRequest<> *request = new InvalidateRequest(*ictx, CEPH_NOSNAP, false, &cond_ctx); + AsyncRequest<> *request = new InvalidateRequest<>(*ictx, CEPH_NOSNAP, false, &cond_ctx); EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx), exec(ictx->header_oid, _, "lock", "assert_locked", _, _, _)) @@ -81,7 +81,7 @@ TEST_F(TestMockObjectMapInvalidateRequest, UpdatesSnapOnDiskFlag) { ASSERT_EQ(0, librbd::snap_set(ictx, "snap1")); C_SaferCond cond_ctx; - AsyncRequest<> *request = new InvalidateRequest(*ictx, ictx->snap_id, false, + AsyncRequest<> *request = new InvalidateRequest<>(*ictx, ictx->snap_id, false, &cond_ctx); EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx), @@ -106,7 +106,7 @@ TEST_F(TestMockObjectMapInvalidateRequest, SkipOnDiskUpdateWithoutLock) { ASSERT_EQ(0, open_image(m_image_name, &ictx)); C_SaferCond cond_ctx; - AsyncRequest<> *request = new InvalidateRequest(*ictx, CEPH_NOSNAP, false, &cond_ctx); + AsyncRequest<> *request = new InvalidateRequest<>(*ictx, CEPH_NOSNAP, false, &cond_ctx); EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx), exec(ictx->header_oid, _, "rbd", "set_flags", _, _, _)) @@ -130,7 +130,7 @@ TEST_F(TestMockObjectMapInvalidateRequest, IgnoresOnDiskUpdateFailure) { ASSERT_EQ(0, acquire_exclusive_lock(*ictx)); C_SaferCond cond_ctx; - AsyncRequest<> *request = new InvalidateRequest(*ictx, CEPH_NOSNAP, false, &cond_ctx); + AsyncRequest<> *request = new InvalidateRequest<>(*ictx, CEPH_NOSNAP, false, &cond_ctx); EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx), exec(ictx->header_oid, _, "lock", "assert_locked", _, _, _)) -- 2.39.5