From 73e4c65c809a1e4161229f49285b21b2cfc623ca Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Fri, 12 Aug 2016 12:21:02 -0400 Subject: [PATCH] librbd: new journal policy to disable initializing the journal This will be used in the case where the journal is being disabled. Signed-off-by: Jason Dillaman --- src/librbd/Makefile.am | 3 +- src/librbd/exclusive_lock/AcquireRequest.cc | 9 ++- src/librbd/image/RefreshRequest.cc | 23 +++++-- src/librbd/journal/DisabledPolicy.h | 32 ++++++++++ src/librbd/journal/Policy.h | 1 + src/librbd/journal/StandardPolicy.h | 3 + .../test_mock_AcquireRequest.cc | 44 +++++++++++--- .../librbd/image/test_mock_RefreshRequest.cc | 60 +++++++++++++++++++ src/test/librbd/mock/MockJournalPolicy.h | 1 + src/tools/rbd_mirror/ImageDeleter.cc | 3 + .../image_replayer/OpenLocalImageRequest.cc | 3 + 11 files changed, 167 insertions(+), 15 deletions(-) create mode 100644 src/librbd/journal/DisabledPolicy.h diff --git a/src/librbd/Makefile.am b/src/librbd/Makefile.am index 7c098196276..57585b7c24c 100644 --- a/src/librbd/Makefile.am +++ b/src/librbd/Makefile.am @@ -133,9 +133,10 @@ noinst_HEADERS += \ librbd/image/SetSnapRequest.h \ librbd/image_watcher/Notifier.h \ librbd/image_watcher/NotifyLockOwner.h \ - librbd/journal/RemoveRequest.h \ librbd/journal/CreateRequest.h \ + librbd/journal/DisabledPolicy.h \ librbd/journal/Policy.h \ + librbd/journal/RemoveRequest.h \ librbd/journal/Replay.h \ librbd/journal/StandardPolicy.h \ librbd/journal/Types.h \ diff --git a/src/librbd/exclusive_lock/AcquireRequest.cc b/src/librbd/exclusive_lock/AcquireRequest.cc index f030b0ea3bc..94fee20e058 100644 --- a/src/librbd/exclusive_lock/AcquireRequest.cc +++ b/src/librbd/exclusive_lock/AcquireRequest.cc @@ -171,7 +171,14 @@ Context *AcquireRequest::send_open_journal() { m_on_acquire->complete(0); m_on_acquire = nullptr; - if (!m_image_ctx.test_features(RBD_FEATURE_JOURNALING)) { + bool journal_enabled; + { + RWLock::RLocker snap_locker(m_image_ctx.snap_lock); + journal_enabled = (m_image_ctx.test_features(RBD_FEATURE_JOURNALING, + m_image_ctx.snap_lock) && + !m_image_ctx.get_journal_policy()->journal_disabled()); + } + if (!journal_enabled) { apply(); return m_on_finish; } diff --git a/src/librbd/image/RefreshRequest.cc b/src/librbd/image/RefreshRequest.cc index dc23ca63b0b..f1a9d110459 100644 --- a/src/librbd/image/RefreshRequest.cc +++ b/src/librbd/image/RefreshRequest.cc @@ -13,6 +13,7 @@ #include "librbd/ObjectMap.h" #include "librbd/Utils.h" #include "librbd/image/RefreshParentRequest.h" +#include "librbd/journal/Policy.h" #define dout_subsys ceph_subsys_rbd #undef dout_prefix @@ -477,15 +478,25 @@ Context *RefreshRequest::handle_v2_init_exclusive_lock(int *result) { template void RefreshRequest::send_v2_open_journal() { - if ((m_features & RBD_FEATURE_JOURNALING) == 0 || - m_image_ctx.read_only || - !m_image_ctx.snap_name.empty() || - m_image_ctx.journal != nullptr || - m_image_ctx.exclusive_lock == nullptr || - !m_image_ctx.exclusive_lock->is_lock_owner()) { + bool journal_disabled = ( + (m_features & RBD_FEATURE_JOURNALING) == 0 || + m_image_ctx.read_only || + !m_image_ctx.snap_name.empty() || + m_image_ctx.journal != nullptr || + m_image_ctx.exclusive_lock == nullptr || + !m_image_ctx.exclusive_lock->is_lock_owner()); + bool journal_disabled_by_policy; + { + RWLock::RLocker snap_locker(m_image_ctx.snap_lock); + journal_disabled_by_policy = ( + !journal_disabled && + m_image_ctx.get_journal_policy()->journal_disabled()); + } + if (journal_disabled || journal_disabled_by_policy) { // journal dynamically enabled -- doesn't own exclusive lock if ((m_features & RBD_FEATURE_JOURNALING) != 0 && + !journal_disabled_by_policy && m_image_ctx.exclusive_lock != nullptr && m_image_ctx.journal == nullptr) { m_image_ctx.aio_work_queue->set_require_lock_on_read(); diff --git a/src/librbd/journal/DisabledPolicy.h b/src/librbd/journal/DisabledPolicy.h new file mode 100644 index 00000000000..119738f639d --- /dev/null +++ b/src/librbd/journal/DisabledPolicy.h @@ -0,0 +1,32 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#ifndef CEPH_LIBRBD_JOURNAL_DISABLED_POLICY_H +#define CEPH_LIBRBD_JOURNAL_DISABLED_POLICY_H + +#include "librbd/journal/Policy.h" + +namespace librbd { + +struct ImageCtx; + +namespace journal { + +class DisabledPolicy : public Policy { +public: + virtual bool append_disabled() const { + assert(false); + return false; + } + virtual bool journal_disabled() const { + return true; + } + virtual void allocate_tag_on_lock(Context *on_finish) { + assert(false); + } +}; + +} // namespace journal +} // namespace librbd + +#endif // CEPH_LIBRBD_JOURNAL_DISABLED_POLICY_H diff --git a/src/librbd/journal/Policy.h b/src/librbd/journal/Policy.h index 2ef21e6fe0e..1ced3c53e87 100644 --- a/src/librbd/journal/Policy.h +++ b/src/librbd/journal/Policy.h @@ -15,6 +15,7 @@ struct Policy { } virtual bool append_disabled() const = 0; + virtual bool journal_disabled() const = 0; virtual void allocate_tag_on_lock(Context *on_finish) = 0; }; diff --git a/src/librbd/journal/StandardPolicy.h b/src/librbd/journal/StandardPolicy.h index c2c997c8aec..b1c99cb34ff 100644 --- a/src/librbd/journal/StandardPolicy.h +++ b/src/librbd/journal/StandardPolicy.h @@ -20,6 +20,9 @@ public: virtual bool append_disabled() const { return false; } + virtual bool journal_disabled() const { + return false; + } virtual void allocate_tag_on_lock(Context *on_finish); private: diff --git a/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc b/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc index a2beba5c85c..4c029521849 100644 --- a/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc +++ b/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc @@ -47,6 +47,12 @@ public: .WillOnce(Return(enabled)); } + void expect_test_features(MockImageCtx &mock_image_ctx, uint64_t features, + RWLock &lock, bool enabled) { + EXPECT_CALL(mock_image_ctx, test_features(features, _)) + .WillOnce(Return(enabled)); + } + void expect_lock(MockImageCtx &mock_image_ctx, int r) { EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), exec(mock_image_ctx.header_oid, _, StrEq("lock"), StrEq("lock"), _, _, _)) @@ -111,6 +117,12 @@ public: .WillOnce(Return(&mock_journal_policy)); } + void expect_journal_disabled(MockJournalPolicy &mock_journal_policy, + bool disabled) { + EXPECT_CALL(mock_journal_policy, journal_disabled()) + .WillOnce(Return(disabled)); + } + void expect_allocate_journal_tag(MockImageCtx &mock_image_ctx, MockJournalPolicy &mock_journal_policy, int r) { @@ -209,7 +221,10 @@ TEST_F(TestMockExclusiveLockAcquireRequest, Success) { MockJournal mock_journal; MockJournalPolicy mock_journal_policy; - expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, true); + expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, + mock_image_ctx.snap_lock, true); + expect_get_journal_policy(mock_image_ctx, mock_journal_policy); + expect_journal_disabled(mock_journal_policy, false); expect_create_journal(mock_image_ctx, &mock_journal); expect_open_journal(mock_image_ctx, mock_journal, 0); expect_get_journal_policy(mock_image_ctx, mock_journal_policy); @@ -242,7 +257,8 @@ TEST_F(TestMockExclusiveLockAcquireRequest, SuccessRefresh) { MockObjectMap mock_object_map; expect_test_features(mock_image_ctx, RBD_FEATURE_OBJECT_MAP, false); - expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, false); + expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, + mock_image_ctx.snap_lock, false); C_SaferCond acquire_ctx; C_SaferCond ctx; @@ -273,7 +289,8 @@ TEST_F(TestMockExclusiveLockAcquireRequest, SuccessJournalDisabled) { expect_create_object_map(mock_image_ctx, &mock_object_map); expect_open_object_map(mock_image_ctx, mock_object_map, 0); - expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, false); + expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, + mock_image_ctx.snap_lock, false); C_SaferCond acquire_ctx; C_SaferCond ctx; @@ -303,7 +320,10 @@ TEST_F(TestMockExclusiveLockAcquireRequest, SuccessObjectMapDisabled) { MockJournal mock_journal; MockJournalPolicy mock_journal_policy; - expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, true); + expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, + mock_image_ctx.snap_lock, true); + expect_get_journal_policy(mock_image_ctx, mock_journal_policy); + expect_journal_disabled(mock_journal_policy, false); expect_create_journal(mock_image_ctx, &mock_journal); expect_open_journal(mock_image_ctx, mock_journal, 0); expect_get_journal_policy(mock_image_ctx, mock_journal_policy); @@ -364,7 +384,11 @@ TEST_F(TestMockExclusiveLockAcquireRequest, JournalError) { expect_open_object_map(mock_image_ctx, *mock_object_map, 0); MockJournal *mock_journal = new MockJournal(); - expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, true); + MockJournalPolicy mock_journal_policy; + expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, + mock_image_ctx.snap_lock, true); + expect_get_journal_policy(mock_image_ctx, mock_journal_policy); + expect_journal_disabled(mock_journal_policy, false); expect_create_journal(mock_image_ctx, mock_journal); expect_open_journal(mock_image_ctx, *mock_journal, -EINVAL); expect_close_journal(mock_image_ctx, *mock_journal); @@ -401,7 +425,10 @@ TEST_F(TestMockExclusiveLockAcquireRequest, AllocateJournalTagError) { MockJournal *mock_journal = new MockJournal(); MockJournalPolicy mock_journal_policy; - expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, true); + expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, + mock_image_ctx.snap_lock, true); + expect_get_journal_policy(mock_image_ctx, mock_journal_policy); + expect_journal_disabled(mock_journal_policy, false); expect_create_journal(mock_image_ctx, mock_journal); expect_open_journal(mock_image_ctx, *mock_journal, 0); expect_get_journal_policy(mock_image_ctx, mock_journal_policy); @@ -745,7 +772,10 @@ TEST_F(TestMockExclusiveLockAcquireRequest, OpenObjectMapError) { MockJournal mock_journal; MockJournalPolicy mock_journal_policy; - expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, true); + expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, + mock_image_ctx.snap_lock, true); + expect_get_journal_policy(mock_image_ctx, mock_journal_policy); + expect_journal_disabled(mock_journal_policy, false); expect_create_journal(mock_image_ctx, &mock_journal); expect_open_journal(mock_image_ctx, mock_journal, 0); expect_get_journal_policy(mock_image_ctx, mock_journal_policy); diff --git a/src/test/librbd/image/test_mock_RefreshRequest.cc b/src/test/librbd/image/test_mock_RefreshRequest.cc index 091a7fc4191..70eb74b7ffe 100644 --- a/src/test/librbd/image/test_mock_RefreshRequest.cc +++ b/src/test/librbd/image/test_mock_RefreshRequest.cc @@ -5,6 +5,7 @@ #include "test/librbd/test_support.h" #include "test/librbd/mock/MockImageCtx.h" #include "test/librbd/mock/MockJournal.h" +#include "test/librbd/mock/MockJournalPolicy.h" #include "test/librbd/mock/MockObjectMap.h" #include "test/librados_test_stub/MockTestMemIoCtxImpl.h" #include "test/librados_test_stub/MockTestMemRadosClient.h" @@ -249,6 +250,18 @@ public: EXPECT_CALL(mock_exclusive_lock, is_lock_owner()).WillOnce(Return(is_owner)); } + void expect_get_journal_policy(MockImageCtx &mock_image_ctx, + MockJournalPolicy &mock_journal_policy) { + EXPECT_CALL(mock_image_ctx, get_journal_policy()) + .WillOnce(Return(&mock_journal_policy)); + } + + void expect_journal_disabled(MockJournalPolicy &mock_journal_policy, + bool disabled) { + EXPECT_CALL(mock_journal_policy, journal_disabled()) + .WillOnce(Return(disabled)); + } + void expect_open_journal(MockRefreshImageCtx &mock_image_ctx, MockJournal &mock_journal, int r) { EXPECT_CALL(mock_image_ctx, create_journal()) @@ -584,6 +597,49 @@ TEST_F(TestMockImageRefreshRequest, DisableExclusiveLockWhileAcquiringLock) { ASSERT_EQ(-ERESTART, ctx.wait()); } + +TEST_F(TestMockImageRefreshRequest, JournalDisabledByPolicy) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + if (ictx->test_features(RBD_FEATURE_FAST_DIFF)) { + ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_FAST_DIFF , false)); + } + + if (ictx->test_features(RBD_FEATURE_OBJECT_MAP)) { + ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_OBJECT_MAP , false)); + } + + MockRefreshImageCtx mock_image_ctx(*ictx); + MockRefreshParentRequest mock_refresh_parent_request; + + MockExclusiveLock mock_exclusive_lock; + mock_image_ctx.exclusive_lock = &mock_exclusive_lock; + + MockJournal mock_journal; + + expect_op_work_queue(mock_image_ctx); + expect_test_features(mock_image_ctx); + expect_is_exclusive_lock_owner(mock_exclusive_lock, true); + + InSequence seq; + expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_flags(mock_image_ctx, 0); + expect_refresh_parent_is_required(mock_refresh_parent_request, false); + + MockJournalPolicy mock_journal_policy; + expect_get_journal_policy(mock_image_ctx, mock_journal_policy); + expect_journal_disabled(mock_journal_policy, true); + + C_SaferCond ctx; + MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, false, &ctx); + req->send(); + + ASSERT_EQ(0, ctx.wait()); +} + TEST_F(TestMockImageRefreshRequest, EnableJournalWithExclusiveLock) { REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); @@ -615,6 +671,10 @@ TEST_F(TestMockImageRefreshRequest, EnableJournalWithExclusiveLock) { expect_get_mutable_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); expect_refresh_parent_is_required(mock_refresh_parent_request, false); + + MockJournalPolicy mock_journal_policy; + expect_get_journal_policy(mock_image_ctx, mock_journal_policy); + expect_journal_disabled(mock_journal_policy, false); expect_open_journal(mock_image_ctx, mock_journal, 0); C_SaferCond ctx; diff --git a/src/test/librbd/mock/MockJournalPolicy.h b/src/test/librbd/mock/MockJournalPolicy.h index 8ad6ff60952..3c95a8c57b7 100644 --- a/src/test/librbd/mock/MockJournalPolicy.h +++ b/src/test/librbd/mock/MockJournalPolicy.h @@ -12,6 +12,7 @@ namespace librbd { struct MockJournalPolicy : public journal::Policy { MOCK_CONST_METHOD0(append_disabled, bool()); + MOCK_CONST_METHOD0(journal_disabled, bool()); MOCK_METHOD1(allocate_tag_on_lock, void(Context*)); }; diff --git a/src/tools/rbd_mirror/ImageDeleter.cc b/src/tools/rbd_mirror/ImageDeleter.cc index 6af2537ba4c..0b0c775dba0 100644 --- a/src/tools/rbd_mirror/ImageDeleter.cc +++ b/src/tools/rbd_mirror/ImageDeleter.cc @@ -77,6 +77,9 @@ struct DeleteJournalPolicy : public librbd::journal::Policy { virtual bool append_disabled() const { return true; } + virtual bool journal_disabled() const { + return false; + } virtual void allocate_tag_on_lock(Context *on_finish) { on_finish->complete(0); diff --git a/src/tools/rbd_mirror/image_replayer/OpenLocalImageRequest.cc b/src/tools/rbd_mirror/image_replayer/OpenLocalImageRequest.cc index cbb42b929bd..7e4f9b6c19c 100644 --- a/src/tools/rbd_mirror/image_replayer/OpenLocalImageRequest.cc +++ b/src/tools/rbd_mirror/image_replayer/OpenLocalImageRequest.cc @@ -48,6 +48,9 @@ struct MirrorJournalPolicy : public librbd::journal::Policy { // avoid recording any events to the local journal return true; } + virtual bool journal_disabled() const { + return false; + } virtual void allocate_tag_on_lock(Context *on_finish) { // rbd-mirror will manually create tags by copying them from the peer -- 2.47.3