From 583ac91872859e81d68c9d346516522c6aa1614c Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Wed, 17 Aug 2016 14:58:22 -0400 Subject: [PATCH] librbd: interlock image refresh and lock operations Fixes: http://tracker.ceph.com/issues/16773 Signed-off-by: Jason Dillaman --- src/librbd/ExclusiveLock.cc | 27 ++- src/librbd/exclusive_lock/AcquireRequest.cc | 17 +- .../test_mock_AcquireRequest.cc | 154 +++++++++++------- src/test/librbd/test_mock_ExclusiveLock.cc | 24 +++ src/test/librbd/test_mock_fixture.cc | 2 - 5 files changed, 153 insertions(+), 71 deletions(-) diff --git a/src/librbd/ExclusiveLock.cc b/src/librbd/ExclusiveLock.cc index 654529026157c..5723fe91d10cb 100644 --- a/src/librbd/ExclusiveLock.cc +++ b/src/librbd/ExclusiveLock.cc @@ -7,6 +7,7 @@ #include "common/errno.h" #include "librbd/AioImageRequestWQ.h" #include "librbd/ImageCtx.h" +#include "librbd/ImageState.h" #include "librbd/ImageWatcher.h" #include "librbd/Utils.h" #include "librbd/exclusive_lock/AcquireRequest.h" @@ -26,10 +27,10 @@ namespace { const std::string WATCHER_LOCK_COOKIE_PREFIX = "auto"; -template -struct C_SendReleaseRequest : public Context { - ReleaseRequest* request; - explicit C_SendReleaseRequest(ReleaseRequest* request) : request(request) { +template +struct C_SendRequest : public Context { + R* request; + explicit C_SendRequest(R* request) : request(request) { } virtual void finish(int r) override { request->send(); @@ -416,9 +417,11 @@ void ExclusiveLock::send_acquire_lock() { util::create_context_callback(this), util::create_context_callback(this)); - m_lock.Unlock(); - req->send(); - m_lock.Lock(); + // acquire the lock if the image is not busy performing other actions + m_image_ctx.state->prepare_lock(new FunctionContext([this, req](int r) { + m_image_ctx.op_work_queue->queue( + new C_SendRequest >(req), 0); + })); } template @@ -447,6 +450,7 @@ void ExclusiveLock::handle_acquire_lock(int r) { ldout(cct, 5) << "successfully acquired exclusive lock" << dendl; } + m_image_ctx.state->handle_prepare_lock_complete(); { m_lock.Lock(); assert(m_state == STATE_ACQUIRING || @@ -585,8 +589,11 @@ void ExclusiveLock::send_release_lock() { util::create_context_callback(this), util::create_context_callback(this)); - // send in alternate thread context to avoid re-entrant locking - m_image_ctx.op_work_queue->queue(new C_SendReleaseRequest(req), 0); + // release the lock if the image is not busy performing other actions + m_image_ctx.state->prepare_lock(new FunctionContext([this, req](int r) { + m_image_ctx.op_work_queue->queue( + new C_SendRequest >(req), 0); + })); } template @@ -603,6 +610,8 @@ void ExclusiveLock::handle_releasing_lock(int r) { template void ExclusiveLock::handle_release_lock(int r) { + m_image_ctx.state->handle_prepare_lock_complete(); + bool lock_request_needed = false; { Mutex::Locker locker(m_lock); diff --git a/src/librbd/exclusive_lock/AcquireRequest.cc b/src/librbd/exclusive_lock/AcquireRequest.cc index 94fee20e058e4..df354178b0426 100644 --- a/src/librbd/exclusive_lock/AcquireRequest.cc +++ b/src/librbd/exclusive_lock/AcquireRequest.cc @@ -15,6 +15,7 @@ #include "librbd/Journal.h" #include "librbd/ObjectMap.h" #include "librbd/Utils.h" +#include "librbd/image/RefreshRequest.h" #include "librbd/journal/Policy.h" #define dout_subsys ceph_subsys_rbd @@ -107,7 +108,7 @@ void AcquireRequest::send_lock() { librados::ObjectWriteOperation op; rados::cls::lock::lock(&op, RBD_LOCK_NAME, LOCK_EXCLUSIVE, m_cookie, - ExclusiveLock::WATCHER_LOCK_TAG, "", utime_t(), 0); + ExclusiveLock<>::WATCHER_LOCK_TAG, "", utime_t(), 0); using klass = AcquireRequest; librados::AioCompletion *rados_completion = @@ -144,8 +145,14 @@ Context *AcquireRequest::send_refresh() { ldout(cct, 10) << __func__ << dendl; using klass = AcquireRequest; - Context *ctx = create_context_callback(this); - m_image_ctx.state->acquire_lock_refresh(ctx); + Context *ctx = create_async_context_callback( + m_image_ctx, create_context_callback(this)); + + // ImageState is blocked waiting for lock to complete -- safe to directly + // refresh + image::RefreshRequest *req = image::RefreshRequest::create( + m_image_ctx, true, ctx); + req->send(); return nullptr; } @@ -403,7 +410,7 @@ Context *AcquireRequest::handle_get_lockers(int *ret_val) { return nullptr; } - if (lock_tag != ExclusiveLock::WATCHER_LOCK_TAG) { + if (lock_tag != ExclusiveLock<>::WATCHER_LOCK_TAG) { ldout(cct, 5) <<"locked by external mechanism: tag=" << lock_tag << dendl; *ret_val = -EBUSY; return m_on_finish; @@ -417,7 +424,7 @@ Context *AcquireRequest::handle_get_lockers(int *ret_val) { std::map::iterator iter = lockers.begin(); - if (!ExclusiveLock::decode_lock_cookie(iter->first.cookie, + if (!ExclusiveLock<>::decode_lock_cookie(iter->first.cookie, &m_locker_handle)) { ldout(cct, 5) << "locked by external mechanism: " << "cookie=" << iter->first.cookie << dendl; diff --git a/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc b/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc index 4c029521849f6..7037dbf8e8da3 100644 --- a/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc +++ b/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc @@ -13,15 +13,54 @@ #include "cls/lock/cls_lock_ops.h" #include "librbd/ExclusiveLock.h" #include "librbd/exclusive_lock/AcquireRequest.h" +#include "librbd/image/RefreshRequest.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include #include +namespace librbd { +namespace { + +struct MockTestImageCtx : public librbd::MockImageCtx { + MockTestImageCtx(librbd::ImageCtx &image_ctx) + : librbd::MockImageCtx(image_ctx) { + } +}; + +} // anonymous namespace + +namespace image { + +template<> +struct RefreshRequest { + static RefreshRequest *s_instance; + Context *on_finish; + + static RefreshRequest *create(librbd::MockTestImageCtx &image_ctx, + bool acquire_lock_refresh, + Context *on_finish) { + EXPECT_TRUE(acquire_lock_refresh); + assert(s_instance != nullptr); + s_instance->on_finish = on_finish; + return s_instance; + } + + RefreshRequest() { + s_instance = this; + } + MOCK_METHOD0(send, void()); +}; + +RefreshRequest *RefreshRequest::s_instance = nullptr; + +} // namespace image +} // namespace librbd + // template definitions #include "librbd/Journal.cc" #include "librbd/exclusive_lock/AcquireRequest.cc" -template class librbd::exclusive_lock::AcquireRequest; +template class librbd::exclusive_lock::AcquireRequest; namespace librbd { namespace exclusive_lock { @@ -38,80 +77,83 @@ static const std::string TEST_COOKIE("auto 123"); class TestMockExclusiveLockAcquireRequest : public TestMockFixture { public: - typedef AcquireRequest MockAcquireRequest; - typedef ExclusiveLock MockExclusiveLock; + typedef AcquireRequest MockAcquireRequest; + typedef ExclusiveLock MockExclusiveLock; + typedef librbd::image::RefreshRequest MockRefreshRequest; - void expect_test_features(MockImageCtx &mock_image_ctx, uint64_t features, + void expect_test_features(MockTestImageCtx &mock_image_ctx, uint64_t features, bool enabled) { EXPECT_CALL(mock_image_ctx, test_features(features)) .WillOnce(Return(enabled)); } - void expect_test_features(MockImageCtx &mock_image_ctx, uint64_t features, + void expect_test_features(MockTestImageCtx &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) { + void expect_lock(MockTestImageCtx &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"), _, _, _)) .WillOnce(Return(r)); } - void expect_unlock(MockImageCtx &mock_image_ctx, int r) { + void expect_unlock(MockTestImageCtx &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("unlock"), _, _, _)) .WillOnce(Return(r)); } - void expect_is_refresh_required(MockImageCtx &mock_image_ctx, bool required) { + void expect_is_refresh_required(MockTestImageCtx &mock_image_ctx, bool required) { EXPECT_CALL(*mock_image_ctx.state, is_refresh_required()) .WillOnce(Return(required)); } - void expect_refresh(MockImageCtx &mock_image_ctx, int r) { - EXPECT_CALL(*mock_image_ctx.state, acquire_lock_refresh(_)) - .WillOnce(CompleteContext(r, mock_image_ctx.image_ctx->op_work_queue)); + void expect_refresh(MockTestImageCtx &mock_image_ctx, + MockRefreshRequest &mock_refresh_request, int r) { + EXPECT_CALL(mock_refresh_request, send()) + .WillOnce(FinishRequest(&mock_refresh_request, r, + &mock_image_ctx)); } - void expect_create_object_map(MockImageCtx &mock_image_ctx, + void expect_create_object_map(MockTestImageCtx &mock_image_ctx, MockObjectMap *mock_object_map) { EXPECT_CALL(mock_image_ctx, create_object_map(_)) .WillOnce(Return(mock_object_map)); } - void expect_open_object_map(MockImageCtx &mock_image_ctx, + void expect_open_object_map(MockTestImageCtx &mock_image_ctx, MockObjectMap &mock_object_map, int r) { EXPECT_CALL(mock_object_map, open(_)) .WillOnce(CompleteContext(r, mock_image_ctx.image_ctx->op_work_queue)); } - void expect_close_object_map(MockImageCtx &mock_image_ctx, + void expect_close_object_map(MockTestImageCtx &mock_image_ctx, MockObjectMap &mock_object_map) { EXPECT_CALL(mock_object_map, close(_)) .WillOnce(CompleteContext(0, mock_image_ctx.image_ctx->op_work_queue)); } - void expect_create_journal(MockImageCtx &mock_image_ctx, + void expect_create_journal(MockTestImageCtx &mock_image_ctx, MockJournal *mock_journal) { EXPECT_CALL(mock_image_ctx, create_journal()) .WillOnce(Return(mock_journal)); } - void expect_open_journal(MockImageCtx &mock_image_ctx, + void expect_open_journal(MockTestImageCtx &mock_image_ctx, MockJournal &mock_journal, int r) { EXPECT_CALL(mock_journal, open(_)) .WillOnce(CompleteContext(r, mock_image_ctx.image_ctx->op_work_queue)); } - void expect_close_journal(MockImageCtx &mock_image_ctx, + void expect_close_journal(MockTestImageCtx &mock_image_ctx, MockJournal &mock_journal) { EXPECT_CALL(mock_journal, close(_)) .WillOnce(CompleteContext(0, mock_image_ctx.image_ctx->op_work_queue)); } - void expect_get_journal_policy(MockImageCtx &mock_image_ctx, + void expect_get_journal_policy(MockTestImageCtx &mock_image_ctx, MockJournalPolicy &mock_journal_policy) { EXPECT_CALL(mock_image_ctx, get_journal_policy()) .WillOnce(Return(&mock_journal_policy)); @@ -123,14 +165,14 @@ public: .WillOnce(Return(disabled)); } - void expect_allocate_journal_tag(MockImageCtx &mock_image_ctx, + void expect_allocate_journal_tag(MockTestImageCtx &mock_image_ctx, MockJournalPolicy &mock_journal_policy, int r) { EXPECT_CALL(mock_journal_policy, allocate_tag_on_lock(_)) .WillOnce(CompleteContext(r, mock_image_ctx.image_ctx->op_work_queue)); } - void expect_get_lock_info(MockImageCtx &mock_image_ctx, int r, + void expect_get_lock_info(MockTestImageCtx &mock_image_ctx, int r, const entity_name_t &locker_entity, const std::string &locker_address, const std::string &locker_cookie, @@ -165,7 +207,7 @@ public: } } - void expect_list_watchers(MockImageCtx &mock_image_ctx, int r, + void expect_list_watchers(MockTestImageCtx &mock_image_ctx, int r, const std::string &address, uint64_t watch_handle) { auto &expect = EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), list_watchers(mock_image_ctx.header_oid, _)); @@ -183,18 +225,18 @@ public: } } - void expect_blacklist_add(MockImageCtx &mock_image_ctx, int r) { + void expect_blacklist_add(MockTestImageCtx &mock_image_ctx, int r) { EXPECT_CALL(get_mock_rados_client(), blacklist_add(_, _)) .WillOnce(Return(r)); } - void expect_break_lock(MockImageCtx &mock_image_ctx, int r) { + void expect_break_lock(MockTestImageCtx &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("break_lock"), _, _, _)) .WillOnce(Return(r)); } - void expect_flush_notifies(MockImageCtx &mock_image_ctx) { + void expect_flush_notifies(MockTestImageCtx &mock_image_ctx) { EXPECT_CALL(*mock_image_ctx.image_watcher, flush(_)) .WillOnce(CompleteContext(0, mock_image_ctx.image_ctx->op_work_queue)); } @@ -206,7 +248,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, Success) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); - MockImageCtx mock_image_ctx(*ictx); + MockTestImageCtx mock_image_ctx(*ictx); expect_op_work_queue(mock_image_ctx); InSequence seq; @@ -246,14 +288,15 @@ TEST_F(TestMockExclusiveLockAcquireRequest, SuccessRefresh) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); - MockImageCtx mock_image_ctx(*ictx); + MockTestImageCtx mock_image_ctx(*ictx); + MockRefreshRequest mock_refresh_request; expect_op_work_queue(mock_image_ctx); InSequence seq; expect_flush_notifies(mock_image_ctx); expect_lock(mock_image_ctx, 0); expect_is_refresh_required(mock_image_ctx, true); - expect_refresh(mock_image_ctx, 0); + expect_refresh(mock_image_ctx, mock_refresh_request, 0); MockObjectMap mock_object_map; expect_test_features(mock_image_ctx, RBD_FEATURE_OBJECT_MAP, false); @@ -276,7 +319,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, SuccessJournalDisabled) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); - MockImageCtx mock_image_ctx(*ictx); + MockTestImageCtx mock_image_ctx(*ictx); expect_op_work_queue(mock_image_ctx); InSequence seq; @@ -308,7 +351,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, SuccessObjectMapDisabled) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); - MockImageCtx mock_image_ctx(*ictx); + MockTestImageCtx mock_image_ctx(*ictx); expect_op_work_queue(mock_image_ctx); InSequence seq; @@ -345,14 +388,15 @@ TEST_F(TestMockExclusiveLockAcquireRequest, RefreshError) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); - MockImageCtx mock_image_ctx(*ictx); + MockTestImageCtx mock_image_ctx(*ictx); + MockRefreshRequest mock_refresh_request; expect_op_work_queue(mock_image_ctx); InSequence seq; expect_flush_notifies(mock_image_ctx); expect_lock(mock_image_ctx, 0); expect_is_refresh_required(mock_image_ctx, true); - expect_refresh(mock_image_ctx, -EINVAL); + expect_refresh(mock_image_ctx, mock_refresh_request, -EINVAL); expect_unlock(mock_image_ctx, 0); C_SaferCond *acquire_ctx = new C_SaferCond(); @@ -370,7 +414,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, JournalError) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); - MockImageCtx mock_image_ctx(*ictx); + MockTestImageCtx mock_image_ctx(*ictx); expect_op_work_queue(mock_image_ctx); InSequence seq; @@ -410,7 +454,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, AllocateJournalTagError) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); - MockImageCtx mock_image_ctx(*ictx); + MockTestImageCtx mock_image_ctx(*ictx); expect_op_work_queue(mock_image_ctx); InSequence seq; @@ -452,14 +496,14 @@ TEST_F(TestMockExclusiveLockAcquireRequest, LockBusy) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); - MockImageCtx mock_image_ctx(*ictx); + MockTestImageCtx mock_image_ctx(*ictx); expect_op_work_queue(mock_image_ctx); InSequence seq; expect_flush_notifies(mock_image_ctx); expect_lock(mock_image_ctx, -EBUSY); expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4", - "auto 123", MockExclusiveLock::WATCHER_LOCK_TAG, + "auto 123", ExclusiveLock<>::WATCHER_LOCK_TAG, LOCK_EXCLUSIVE); expect_list_watchers(mock_image_ctx, 0, "dead client", 123); expect_blacklist_add(mock_image_ctx, 0); @@ -480,7 +524,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoError) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); - MockImageCtx mock_image_ctx(*ictx); + MockTestImageCtx mock_image_ctx(*ictx); expect_op_work_queue(mock_image_ctx); InSequence seq; @@ -503,7 +547,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoEmpty) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); - MockImageCtx mock_image_ctx(*ictx); + MockTestImageCtx mock_image_ctx(*ictx); expect_op_work_queue(mock_image_ctx); InSequence seq; @@ -527,7 +571,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoExternalTag) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); - MockImageCtx mock_image_ctx(*ictx); + MockTestImageCtx mock_image_ctx(*ictx); expect_op_work_queue(mock_image_ctx); InSequence seq; @@ -550,14 +594,14 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoShared) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); - MockImageCtx mock_image_ctx(*ictx); + MockTestImageCtx mock_image_ctx(*ictx); expect_op_work_queue(mock_image_ctx); InSequence seq; expect_flush_notifies(mock_image_ctx); expect_lock(mock_image_ctx, -EBUSY); expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4", - "auto 123", MockExclusiveLock::WATCHER_LOCK_TAG, + "auto 123", ExclusiveLock<>::WATCHER_LOCK_TAG, LOCK_SHARED); C_SaferCond ctx; @@ -574,14 +618,14 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoExternalCookie) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); - MockImageCtx mock_image_ctx(*ictx); + MockTestImageCtx mock_image_ctx(*ictx); expect_op_work_queue(mock_image_ctx); InSequence seq; expect_flush_notifies(mock_image_ctx); expect_lock(mock_image_ctx, -EBUSY); expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4", - "external cookie", MockExclusiveLock::WATCHER_LOCK_TAG, + "external cookie", ExclusiveLock<>::WATCHER_LOCK_TAG, LOCK_EXCLUSIVE); C_SaferCond ctx; @@ -598,14 +642,14 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetWatchersError) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); - MockImageCtx mock_image_ctx(*ictx); + MockTestImageCtx mock_image_ctx(*ictx); expect_op_work_queue(mock_image_ctx); InSequence seq; expect_flush_notifies(mock_image_ctx); expect_lock(mock_image_ctx, -EBUSY); expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4", - "auto 123", MockExclusiveLock::WATCHER_LOCK_TAG, + "auto 123", ExclusiveLock<>::WATCHER_LOCK_TAG, LOCK_EXCLUSIVE); expect_list_watchers(mock_image_ctx, -EINVAL, "dead client", 123); @@ -623,14 +667,14 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetWatchersAlive) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); - MockImageCtx mock_image_ctx(*ictx); + MockTestImageCtx mock_image_ctx(*ictx); expect_op_work_queue(mock_image_ctx); InSequence seq; expect_flush_notifies(mock_image_ctx); expect_lock(mock_image_ctx, -EBUSY); expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4", - "auto 123", MockExclusiveLock::WATCHER_LOCK_TAG, + "auto 123", ExclusiveLock<>::WATCHER_LOCK_TAG, LOCK_EXCLUSIVE); expect_list_watchers(mock_image_ctx, 0, "1.2.3.4", 123); @@ -648,7 +692,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BlacklistDisabled) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); - MockImageCtx mock_image_ctx(*ictx); + MockTestImageCtx mock_image_ctx(*ictx); expect_op_work_queue(mock_image_ctx); mock_image_ctx.blacklist_on_break_lock = false; @@ -656,7 +700,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BlacklistDisabled) { expect_flush_notifies(mock_image_ctx); expect_lock(mock_image_ctx, -EBUSY); expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4", - "auto 123", MockExclusiveLock::WATCHER_LOCK_TAG, + "auto 123", ExclusiveLock<>::WATCHER_LOCK_TAG, LOCK_EXCLUSIVE); expect_list_watchers(mock_image_ctx, 0, "dead client", 123); expect_break_lock(mock_image_ctx, 0); @@ -676,14 +720,14 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BlacklistError) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); - MockImageCtx mock_image_ctx(*ictx); + MockTestImageCtx mock_image_ctx(*ictx); expect_op_work_queue(mock_image_ctx); InSequence seq; expect_flush_notifies(mock_image_ctx); expect_lock(mock_image_ctx, -EBUSY); expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4", - "auto 123", MockExclusiveLock::WATCHER_LOCK_TAG, + "auto 123", ExclusiveLock<>::WATCHER_LOCK_TAG, LOCK_EXCLUSIVE); expect_list_watchers(mock_image_ctx, 0, "dead client", 123); expect_blacklist_add(mock_image_ctx, -EINVAL); @@ -702,14 +746,14 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BreakLockMissing) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); - MockImageCtx mock_image_ctx(*ictx); + MockTestImageCtx mock_image_ctx(*ictx); expect_op_work_queue(mock_image_ctx); InSequence seq; expect_flush_notifies(mock_image_ctx); expect_lock(mock_image_ctx, -EBUSY); expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4", - "auto 123", MockExclusiveLock::WATCHER_LOCK_TAG, + "auto 123", ExclusiveLock<>::WATCHER_LOCK_TAG, LOCK_EXCLUSIVE); expect_list_watchers(mock_image_ctx, 0, "dead client", 123); expect_blacklist_add(mock_image_ctx, 0); @@ -730,14 +774,14 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BreakLockError) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); - MockImageCtx mock_image_ctx(*ictx); + MockTestImageCtx mock_image_ctx(*ictx); expect_op_work_queue(mock_image_ctx); InSequence seq; expect_flush_notifies(mock_image_ctx); expect_lock(mock_image_ctx, -EBUSY); expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4", - "auto 123", MockExclusiveLock::WATCHER_LOCK_TAG, + "auto 123", ExclusiveLock<>::WATCHER_LOCK_TAG, LOCK_EXCLUSIVE); expect_list_watchers(mock_image_ctx, 0, "dead client", 123); expect_blacklist_add(mock_image_ctx, 0); @@ -757,7 +801,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, OpenObjectMapError) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); - MockImageCtx mock_image_ctx(*ictx); + MockTestImageCtx mock_image_ctx(*ictx); expect_op_work_queue(mock_image_ctx); InSequence seq; diff --git a/src/test/librbd/test_mock_ExclusiveLock.cc b/src/test/librbd/test_mock_ExclusiveLock.cc index 26b066a88d37f..821f857b769b9 100644 --- a/src/test/librbd/test_mock_ExclusiveLock.cc +++ b/src/test/librbd/test_mock_ExclusiveLock.cc @@ -4,6 +4,7 @@ #include "test/librbd/test_mock_fixture.h" #include "test/librbd/test_support.h" #include "test/librbd/mock/MockImageCtx.h" +#include "test/librbd/mock/MockImageState.h" #include "librbd/ExclusiveLock.h" #include "librbd/exclusive_lock/AcquireRequest.h" #include "librbd/exclusive_lock/ReacquireRequest.h" @@ -129,9 +130,11 @@ public: void expect_acquire_lock(MockExclusiveLockImageCtx &mock_image_ctx, MockAcquireRequest &acquire_request, int r) { expect_get_watch_handle(mock_image_ctx); + expect_prepare_lock(mock_image_ctx); EXPECT_CALL(acquire_request, send()) .WillOnce(DoAll(FinishLockUnlock(&acquire_request), FinishRequest(&acquire_request, r, &mock_image_ctx))); + expect_handle_prepare_lock_complete(mock_image_ctx); if (r == 0) { expect_notify_acquired_lock(mock_image_ctx); expect_unblock_writes(mock_image_ctx); @@ -141,9 +144,15 @@ public: void expect_release_lock(MockExclusiveLockImageCtx &mock_image_ctx, MockReleaseRequest &release_request, int r, bool shutting_down = false) { + if (!shutting_down) { + expect_prepare_lock(mock_image_ctx); + } EXPECT_CALL(release_request, send()) .WillOnce(DoAll(FinishLockUnlock(&release_request), FinishRequest(&release_request, r, &mock_image_ctx))); + if (!shutting_down) { + expect_handle_prepare_lock_complete(mock_image_ctx); + } if (r == 0) { if (shutting_down) { expect_unblock_writes(mock_image_ctx); @@ -188,6 +197,17 @@ public: .WillOnce(CompleteContext(0, mock_image_ctx.image_ctx->op_work_queue)); } + void expect_prepare_lock(MockExclusiveLockImageCtx &mock_image_ctx) { + EXPECT_CALL(*mock_image_ctx.state, prepare_lock(_)) + .WillOnce(Invoke([](Context *on_ready) { + on_ready->complete(0); + })); + } + + void expect_handle_prepare_lock_complete(MockExclusiveLockImageCtx &mock_image_ctx) { + EXPECT_CALL(*mock_image_ctx.state, handle_prepare_lock_complete()); + } + int when_init(MockExclusiveLockImageCtx &mock_image_ctx, MockExclusiveLock &exclusive_lock) { C_SaferCond ctx; @@ -551,16 +571,20 @@ TEST_F(TestMockExclusiveLock, ConcurrentRequests) { MockAcquireRequest try_lock_acquire; C_SaferCond wait_for_send_ctx1; expect_get_watch_handle(mock_image_ctx); + expect_prepare_lock(mock_image_ctx); EXPECT_CALL(try_lock_acquire, send()) .WillOnce(Notify(&wait_for_send_ctx1)); + expect_handle_prepare_lock_complete(mock_image_ctx); MockAcquireRequest request_acquire; expect_acquire_lock(mock_image_ctx, request_acquire, 0); MockReleaseRequest release; C_SaferCond wait_for_send_ctx2; + expect_prepare_lock(mock_image_ctx); EXPECT_CALL(release, send()) .WillOnce(Notify(&wait_for_send_ctx2)); + expect_handle_prepare_lock_complete(mock_image_ctx); expect_notify_released_lock(mock_image_ctx); expect_is_lock_request_needed(mock_image_ctx, false); diff --git a/src/test/librbd/test_mock_fixture.cc b/src/test/librbd/test_mock_fixture.cc index 3fb246d28d37c..efa85562b1cc6 100644 --- a/src/test/librbd/test_mock_fixture.cc +++ b/src/test/librbd/test_mock_fixture.cc @@ -9,12 +9,10 @@ // template definitions #include "librbd/AsyncRequest.cc" #include "librbd/AsyncObjectThrottle.cc" -#include "librbd/ExclusiveLock.cc" #include "librbd/operation/Request.cc" template class librbd::AsyncRequest; template class librbd::AsyncObjectThrottle; -template class librbd::ExclusiveLock; template class librbd::operation::Request; using ::testing::_; -- 2.39.5