From e59293d0cca21838b0c8c283ad9aabd4ea73dabf Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Tue, 12 Jan 2016 13:00:34 -0500 Subject: [PATCH] librbd: add additional granularity to lock states Allow all pre-release actions to see that the exclusive lock is still owned and all post-acquire actions to see that the exclusive lock is now owned. Signed-off-by: Jason Dillaman --- src/librbd/ExclusiveLock.cc | 74 ++++++++++++------- src/librbd/ExclusiveLock.h | 25 +------ src/librbd/exclusive_lock/AcquireRequest.cc | 16 +++- src/librbd/exclusive_lock/AcquireRequest.h | 6 +- src/librbd/exclusive_lock/ReleaseRequest.cc | 53 +++++++++++-- src/librbd/exclusive_lock/ReleaseRequest.h | 12 ++- .../test_mock_AcquireRequest.cc | 39 ++++++---- .../test_mock_ReleaseRequest.cc | 48 +++++++++++- src/test/librbd/test_mock_ExclusiveLock.cc | 12 +-- 9 files changed, 193 insertions(+), 92 deletions(-) diff --git a/src/librbd/ExclusiveLock.cc b/src/librbd/ExclusiveLock.cc index 92e7ef7136ea..fd362ef4a98c 100644 --- a/src/librbd/ExclusiveLock.cc +++ b/src/librbd/ExclusiveLock.cc @@ -25,6 +25,16 @@ namespace { const std::string WATCHER_LOCK_COOKIE_PREFIX = "auto"; +template +struct C_SendReleaseRequest : public Context { + ReleaseRequest* request; + C_SendReleaseRequest(ReleaseRequest* request) : request(request) { + } + virtual void finish(int r) override { + request->send(); + } +}; + } // anonymous namespace template @@ -319,6 +329,7 @@ void ExclusiveLock::send_acquire_lock() { using el = ExclusiveLock; AcquireRequest* req = AcquireRequest::create( m_image_ctx, encode_lock_cookie(), + util::create_context_callback(this), util::create_context_callback(this)); m_lock.Unlock(); @@ -326,6 +337,18 @@ void ExclusiveLock::send_acquire_lock() { m_lock.Lock(); } +template +void ExclusiveLock::handle_acquiring_lock(int r) { + CephContext *cct = m_image_ctx.cct; + ldout(cct, 10) << this << " " << __func__ << dendl; + + assert(r == 0); + assert(m_state == STATE_ACQUIRING); + + // lock is owned at this point + m_state = STATE_POST_ACQUIRING; +} + template void ExclusiveLock::handle_acquire_lock(int r) { CephContext *cct = m_image_ctx.cct; @@ -342,6 +365,9 @@ void ExclusiveLock::handle_acquire_lock(int r) { { m_lock.Lock(); + assert(m_state == STATE_ACQUIRING || + m_state == STATE_POST_ACQUIRING); + Action action = get_active_action(); assert(action == ACTION_TRY_LOCK || action == ACTION_REQUEST_LOCK); if (action == ACTION_REQUEST_LOCK && r < 0 && r != -EBLACKLISTED) { @@ -361,10 +387,6 @@ void ExclusiveLock::handle_acquire_lock(int r) { } if (next_state == STATE_LOCKED) { - { - Mutex::Locker locker(m_lock); - m_state = STATE_POST_ACQUIRING; - } m_image_ctx.image_watcher->notify_acquired_lock(); m_image_ctx.aio_work_queue->unblock_writes(); } @@ -384,31 +406,26 @@ void ExclusiveLock::send_release_lock() { ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl; m_state = STATE_PRE_RELEASING; - m_image_ctx.op_work_queue->queue( - new C_BlockWrites(m_image_ctx, new C_ReleaseBlockWrites(this)), 0); + using el = ExclusiveLock; + ReleaseRequest* req = ReleaseRequest::create( + m_image_ctx, encode_lock_cookie(), + 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); } template -void ExclusiveLock::handle_release_blocked_writes(int r) { - if (r < 0) { - handle_release_lock(r); - return; - } - - ldout(m_image_ctx.cct, 10) << this << " " << __func__ << ": r=" << r << dendl; - +void ExclusiveLock::handle_releasing_lock(int r) { Mutex::Locker locker(m_lock); - assert(m_state == STATE_PRE_RELEASING); - m_state = STATE_RELEASING; + ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl; - using el = ExclusiveLock; - ReleaseRequest* req = ReleaseRequest::create( - m_image_ctx, encode_lock_cookie(), - util::create_context_callback(this)); + assert(r == 0); + assert(m_state == STATE_PRE_RELEASING); - m_lock.Unlock(); - req->send(); - m_lock.Lock(); + // all IO and ops should be blocked/canceled by this point + m_state = STATE_RELEASING; } template @@ -419,10 +436,9 @@ void ExclusiveLock::handle_release_lock(int r) { ldout(m_image_ctx.cct, 10) << this << " " << __func__ << ": r=" << r << dendl; - assert(m_state == STATE_RELEASING); - if (r < 0) { - m_image_ctx.aio_work_queue->unblock_writes(); - } else { + assert(m_state == STATE_PRE_RELEASING || + m_state == STATE_RELEASING); + if (r >= 0) { m_lock.Unlock(); m_image_ctx.image_watcher->notify_released_lock(); pending_writes = !m_image_ctx.aio_work_queue->writes_empty(); @@ -470,7 +486,7 @@ void ExclusiveLock::send_shutdown_release() { using el = ExclusiveLock; ReleaseRequest* req = ReleaseRequest::create( - m_image_ctx, cookie, + m_image_ctx, cookie, nullptr, util::create_context_callback(this)); req->send(); } @@ -483,6 +499,8 @@ void ExclusiveLock::handle_shutdown(int r) { if (r < 0) { lderr(cct) << "failed to shut down exclusive lock: " << cpp_strerror(r) << dendl; + } else { + m_image_ctx.aio_work_queue->unblock_writes(); } m_image_ctx.image_watcher->notify_released_lock(); diff --git a/src/librbd/ExclusiveLock.h b/src/librbd/ExclusiveLock.h index af107c25f7dc..f29ae5971739 100644 --- a/src/librbd/ExclusiveLock.h +++ b/src/librbd/ExclusiveLock.h @@ -108,28 +108,6 @@ private: } }; - struct C_BlockWrites : public Context { - ImageCtxT &image_ctx; - Context *on_finish; - C_BlockWrites(ImageCtxT &image_ctx, Context *on_finish) - : image_ctx(image_ctx), on_finish(on_finish) { - } - virtual void finish(int r) override { - RWLock::RLocker owner_locker(image_ctx.owner_lock); - image_ctx.aio_work_queue->block_writes(on_finish); - } - }; - - struct C_ReleaseBlockWrites : public Context { - ExclusiveLock *exclusive_lock; - C_ReleaseBlockWrites(ExclusiveLock *exclusive_lock) - : exclusive_lock(exclusive_lock) { - } - virtual void finish(int r) override { - exclusive_lock->handle_release_blocked_writes(r); - } - }; - struct C_ShutDownRelease : public Context { ExclusiveLock *exclusive_lock; C_ShutDownRelease(ExclusiveLock *exclusive_lock) @@ -164,10 +142,11 @@ private: void handle_init_complete(); void send_acquire_lock(); + void handle_acquiring_lock(int r); void handle_acquire_lock(int r); void send_release_lock(); - void handle_release_blocked_writes(int r); + void handle_releasing_lock(int r); void handle_release_lock(int r); void send_shutdown(); diff --git a/src/librbd/exclusive_lock/AcquireRequest.cc b/src/librbd/exclusive_lock/AcquireRequest.cc index f0826b1252c2..ad59c046e26f 100644 --- a/src/librbd/exclusive_lock/AcquireRequest.cc +++ b/src/librbd/exclusive_lock/AcquireRequest.cc @@ -53,18 +53,24 @@ struct C_BlacklistClient : public Context { template AcquireRequest* AcquireRequest::create(I &image_ctx, const std::string &cookie, + Context *on_acquire, Context *on_finish) { - return new AcquireRequest(image_ctx, cookie, on_finish); + return new AcquireRequest(image_ctx, cookie, on_acquire, on_finish); } template AcquireRequest::AcquireRequest(I &image_ctx, const std::string &cookie, - Context *on_finish) - : m_image_ctx(image_ctx), m_cookie(cookie), + Context *on_acquire, Context *on_finish) + : m_image_ctx(image_ctx), m_cookie(cookie), m_on_acquire(on_acquire), m_on_finish(create_async_context_callback(image_ctx, on_finish)), m_object_map(nullptr), m_journal(nullptr), m_error_result(0) { } +template +AcquireRequest::~AcquireRequest() { + delete m_on_acquire; +} + template void AcquireRequest::send() { send_lock(); @@ -106,6 +112,10 @@ Context *AcquireRequest::handle_lock(int *ret_val) { template Context *AcquireRequest::send_open_journal() { + // alert caller that we now own the exclusive lock + m_on_acquire->complete(0); + m_on_acquire = nullptr; + if (!m_image_ctx.test_features(RBD_FEATURE_JOURNALING)) { apply(); return m_on_finish; diff --git a/src/librbd/exclusive_lock/AcquireRequest.h b/src/librbd/exclusive_lock/AcquireRequest.h index e3062156e59f..865b3c66bd49 100644 --- a/src/librbd/exclusive_lock/AcquireRequest.h +++ b/src/librbd/exclusive_lock/AcquireRequest.h @@ -24,8 +24,9 @@ template class AcquireRequest { public: static AcquireRequest* create(ImageCtxT &image_ctx, const std::string &cookie, - Context *on_finish); + Context *on_acquire, Context *on_finish); + ~AcquireRequest(); void send(); private: @@ -61,10 +62,11 @@ private: */ AcquireRequest(ImageCtxT &image_ctx, const std::string &cookie, - Context *on_finish); + Context *on_acquire, Context *on_finish); ImageCtxT &m_image_ctx; std::string m_cookie; + Context *m_on_acquire; Context *m_on_finish; bufferlist m_out_bl; diff --git a/src/librbd/exclusive_lock/ReleaseRequest.cc b/src/librbd/exclusive_lock/ReleaseRequest.cc index fbf325490fed..3b65199a3143 100644 --- a/src/librbd/exclusive_lock/ReleaseRequest.cc +++ b/src/librbd/exclusive_lock/ReleaseRequest.cc @@ -8,6 +8,7 @@ #include "common/errno.h" #include "common/WorkQueue.h" #include "include/stringify.h" +#include "librbd/AioImageRequestWQ.h" #include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/Journal.h" @@ -28,21 +29,56 @@ using util::create_rados_safe_callback; template ReleaseRequest* ReleaseRequest::create(I &image_ctx, const std::string &cookie, + Context *on_releasing, Context *on_finish) { - return new ReleaseRequest(image_ctx, cookie, on_finish); + return new ReleaseRequest(image_ctx, cookie, on_releasing, on_finish); } template ReleaseRequest::ReleaseRequest(I &image_ctx, const std::string &cookie, - Context *on_finish) - : m_image_ctx(image_ctx), m_cookie(cookie), + Context *on_releasing, Context *on_finish) + : m_image_ctx(image_ctx), m_cookie(cookie), m_on_releasing(on_releasing), m_on_finish(create_async_context_callback(image_ctx, on_finish)), m_object_map(nullptr), m_journal(nullptr) { } +template +ReleaseRequest::~ReleaseRequest() { + delete m_on_releasing; +} + template void ReleaseRequest::send() { + send_block_writes(); +} + +template +void ReleaseRequest::send_block_writes() { + CephContext *cct = m_image_ctx.cct; + ldout(cct, 10) << __func__ << dendl; + + using klass = ReleaseRequest; + Context *ctx = create_context_callback< + klass, &klass::handle_block_writes>(this); + + { + RWLock::RLocker owner_locker(m_image_ctx.owner_lock); + m_image_ctx.aio_work_queue->block_writes(ctx); + } +} + +template +Context *ReleaseRequest::handle_block_writes(int *ret_val) { + CephContext *cct = m_image_ctx.cct; + ldout(cct, 10) << __func__ << ": r=" << *ret_val << dendl; + + if (*ret_val < 0) { + m_image_ctx.aio_work_queue->unblock_writes(); + return m_on_finish; + } + send_cancel_op_requests(); + return nullptr; } template @@ -51,8 +87,8 @@ void ReleaseRequest::send_cancel_op_requests() { ldout(cct, 10) << __func__ << dendl; using klass = ReleaseRequest; - Context *ctx = create_context_callback(this); + Context *ctx = create_context_callback< + klass, &klass::handle_cancel_op_requests>(this); m_image_ctx.cancel_async_requests(ctx); } @@ -62,6 +98,13 @@ Context *ReleaseRequest::handle_cancel_op_requests(int *ret_val) { ldout(cct, 10) << __func__ << ": r=" << *ret_val << dendl; assert(*ret_val == 0); + + if (m_on_releasing != nullptr) { + // alert caller that we no longer own the exclusive lock + m_on_releasing->complete(0); + m_on_releasing = nullptr; + } + send_close_journal(); return nullptr; } diff --git a/src/librbd/exclusive_lock/ReleaseRequest.h b/src/librbd/exclusive_lock/ReleaseRequest.h index a7946f52a7a5..b23e6c7d8797 100644 --- a/src/librbd/exclusive_lock/ReleaseRequest.h +++ b/src/librbd/exclusive_lock/ReleaseRequest.h @@ -21,8 +21,9 @@ template class ReleaseRequest { public: static ReleaseRequest* create(ImageCtxT &image_ctx, const std::string &cookie, - Context *on_finish); + Context *on_releasing, Context *on_finish); + ~ReleaseRequest(); void send(); private: @@ -32,6 +33,9 @@ private: * * | * v + * BLOCK_WRITES + * | + * v * CANCEL_OP_REQUESTS . . . . . . . . . . . . * | . * v . @@ -50,15 +54,19 @@ private: */ ReleaseRequest(ImageCtxT &image_ctx, const std::string &cookie, - Context *on_finish); + Context *on_releasing, Context *on_finish); ImageCtxT &m_image_ctx; std::string m_cookie; + Context *m_on_releasing; Context *m_on_finish; decltype(m_image_ctx.object_map) m_object_map; decltype(m_image_ctx.journal) m_journal; + void send_block_writes(); + Context *handle_block_writes(int *ret_val); + void send_cancel_op_requests(); Context *handle_cancel_op_requests(int *ret_val); diff --git a/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc b/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc index 7cf7c9dd9ef5..2732bd81633b 100644 --- a/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc +++ b/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc @@ -172,11 +172,13 @@ TEST_F(TestMockExclusiveLockAcquireRequest, Success) { expect_create_journal(mock_image_ctx, &mock_journal); expect_open_journal(mock_image_ctx, mock_journal, 0); + C_SaferCond acquire_ctx; C_SaferCond ctx; MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx, TEST_COOKIE, - &ctx); + &acquire_ctx, &ctx); req->send(); + ASSERT_EQ(0, acquire_ctx.wait()); ASSERT_EQ(0, ctx.wait()); } @@ -200,11 +202,13 @@ TEST_F(TestMockExclusiveLockAcquireRequest, SuccessJournalDisabled) { expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, false); + C_SaferCond acquire_ctx; C_SaferCond ctx; MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx, TEST_COOKIE, - &ctx); + &acquire_ctx, &ctx); req->send(); + ASSERT_EQ(0, acquire_ctx.wait()); ASSERT_EQ(0, ctx.wait()); } @@ -227,11 +231,13 @@ TEST_F(TestMockExclusiveLockAcquireRequest, SuccessObjectMapDisabled) { expect_create_journal(mock_image_ctx, &mock_journal); expect_open_journal(mock_image_ctx, mock_journal, 0); + C_SaferCond acquire_ctx; C_SaferCond ctx; MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx, TEST_COOKIE, - &ctx); + &acquire_ctx, &ctx); req->send(); + ASSERT_EQ(0, acquire_ctx.wait()); ASSERT_EQ(0, ctx.wait()); } @@ -259,10 +265,11 @@ TEST_F(TestMockExclusiveLockAcquireRequest, JournalError) { expect_open_journal(mock_image_ctx, *mock_journal, -EINVAL); expect_unlock_object_map(mock_image_ctx, *mock_object_map); + C_SaferCond acquire_ctx; C_SaferCond ctx; MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx, TEST_COOKIE, - &ctx); + &acquire_ctx, &ctx); req->send(); ASSERT_EQ(-EINVAL, ctx.wait()); } @@ -289,7 +296,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, LockBusy) { C_SaferCond ctx; MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx, TEST_COOKIE, - &ctx); + nullptr, &ctx); req->send(); ASSERT_EQ(-ENOENT, ctx.wait()); } @@ -311,7 +318,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoError) { C_SaferCond ctx; MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx, TEST_COOKIE, - &ctx); + nullptr, &ctx); req->send(); ASSERT_EQ(-EINVAL, ctx.wait()); } @@ -334,7 +341,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoEmpty) { C_SaferCond ctx; MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx, TEST_COOKIE, - &ctx); + nullptr, &ctx); req->send(); ASSERT_EQ(-EINVAL, ctx.wait()); } @@ -356,7 +363,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoExternalTag) { C_SaferCond ctx; MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx, TEST_COOKIE, - &ctx); + nullptr, &ctx); req->send(); ASSERT_EQ(-EBUSY, ctx.wait()); } @@ -379,7 +386,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoShared) { C_SaferCond ctx; MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx, TEST_COOKIE, - &ctx); + nullptr, &ctx); req->send(); ASSERT_EQ(-EBUSY, ctx.wait()); } @@ -402,7 +409,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoExternalCookie) { C_SaferCond ctx; MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx, TEST_COOKIE, - &ctx); + nullptr, &ctx); req->send(); ASSERT_EQ(-EBUSY, ctx.wait()); } @@ -426,7 +433,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetWatchersError) { C_SaferCond ctx; MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx, TEST_COOKIE, - &ctx); + nullptr, &ctx); req->send(); ASSERT_EQ(-EINVAL, ctx.wait()); } @@ -450,7 +457,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetWatchersAlive) { C_SaferCond ctx; MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx, TEST_COOKIE, - &ctx); + nullptr, &ctx); req->send(); ASSERT_EQ(-EAGAIN, ctx.wait()); } @@ -477,7 +484,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BlacklistDisabled) { C_SaferCond ctx; MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx, TEST_COOKIE, - &ctx); + nullptr, &ctx); req->send(); ASSERT_EQ(-ENOENT, ctx.wait()); } @@ -502,7 +509,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BlacklistError) { C_SaferCond ctx; MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx, TEST_COOKIE, - &ctx); + nullptr, &ctx); req->send(); ASSERT_EQ(-EINVAL, ctx.wait()); } @@ -529,7 +536,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BreakLockMissing) { C_SaferCond ctx; MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx, TEST_COOKIE, - &ctx); + nullptr, &ctx); req->send(); ASSERT_EQ(-EINVAL, ctx.wait()); } @@ -555,7 +562,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BreakLockError) { C_SaferCond ctx; MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx, TEST_COOKIE, - &ctx); + nullptr, &ctx); req->send(); ASSERT_EQ(-EINVAL, ctx.wait()); } diff --git a/src/test/librbd/exclusive_lock/test_mock_ReleaseRequest.cc b/src/test/librbd/exclusive_lock/test_mock_ReleaseRequest.cc index 09595afdca51..c99b361e4d1b 100644 --- a/src/test/librbd/exclusive_lock/test_mock_ReleaseRequest.cc +++ b/src/test/librbd/exclusive_lock/test_mock_ReleaseRequest.cc @@ -29,6 +29,15 @@ class TestMockExclusiveLockReleaseRequest : public TestMockFixture { public: typedef ReleaseRequest MockReleaseRequest; + void expect_block_writes(MockImageCtx &mock_image_ctx, int r) { + EXPECT_CALL(*mock_image_ctx.aio_work_queue, block_writes(_)) + .WillOnce(CompleteContext(r, mock_image_ctx.image_ctx->op_work_queue)); + } + + void expect_unblock_writes(MockImageCtx &mock_image_ctx) { + EXPECT_CALL(*mock_image_ctx.aio_work_queue, unblock_writes()); + } + void expect_cancel_op_requests(MockImageCtx &mock_image_ctx, int r) { EXPECT_CALL(mock_image_ctx, cancel_async_requests(_)) .WillOnce(CompleteContext(r, mock_image_ctx.image_ctx->op_work_queue)); @@ -63,6 +72,7 @@ TEST_F(TestMockExclusiveLockReleaseRequest, Success) { expect_op_work_queue(mock_image_ctx); InSequence seq; + expect_block_writes(mock_image_ctx, 0); expect_cancel_op_requests(mock_image_ctx, 0); MockJournal *mock_journal = new MockJournal(); @@ -75,11 +85,13 @@ TEST_F(TestMockExclusiveLockReleaseRequest, Success) { expect_unlock(mock_image_ctx, 0); + C_SaferCond release_ctx; C_SaferCond ctx; MockReleaseRequest *req = MockReleaseRequest::create(mock_image_ctx, TEST_COOKIE, - &ctx); + &release_ctx, &ctx); req->send(); + ASSERT_EQ(0, release_ctx.wait()); ASSERT_EQ(0, ctx.wait()); } @@ -90,6 +102,7 @@ TEST_F(TestMockExclusiveLockReleaseRequest, SuccessJournalDisabled) { ASSERT_EQ(0, open_image(m_image_name, &ictx)); MockImageCtx mock_image_ctx(*ictx); + expect_block_writes(mock_image_ctx, 0); expect_op_work_queue(mock_image_ctx); InSequence seq; @@ -101,11 +114,13 @@ TEST_F(TestMockExclusiveLockReleaseRequest, SuccessJournalDisabled) { expect_unlock(mock_image_ctx, 0); + C_SaferCond release_ctx; C_SaferCond ctx; MockReleaseRequest *req = MockReleaseRequest::create(mock_image_ctx, TEST_COOKIE, - &ctx); + &release_ctx, &ctx); req->send(); + ASSERT_EQ(0, release_ctx.wait()); ASSERT_EQ(0, ctx.wait()); } @@ -116,6 +131,7 @@ TEST_F(TestMockExclusiveLockReleaseRequest, SuccessObjectMapDisabled) { ASSERT_EQ(0, open_image(m_image_name, &ictx)); MockImageCtx mock_image_ctx(*ictx); + expect_block_writes(mock_image_ctx, 0); expect_op_work_queue(mock_image_ctx); InSequence seq; @@ -123,14 +139,37 @@ TEST_F(TestMockExclusiveLockReleaseRequest, SuccessObjectMapDisabled) { expect_unlock(mock_image_ctx, 0); + C_SaferCond release_ctx; C_SaferCond ctx; MockReleaseRequest *req = MockReleaseRequest::create(mock_image_ctx, TEST_COOKIE, - &ctx); + &release_ctx, &ctx); req->send(); + ASSERT_EQ(0, release_ctx.wait()); ASSERT_EQ(0, ctx.wait()); } +TEST_F(TestMockExclusiveLockReleaseRequest, BlockWritesError) { + REQUIRE_FEATURE(RBD_FEATURE_EXCLUSIVE_LOCK); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockImageCtx mock_image_ctx(*ictx); + expect_op_work_queue(mock_image_ctx); + + InSequence seq; + expect_block_writes(mock_image_ctx, -EINVAL); + expect_unblock_writes(mock_image_ctx); + + C_SaferCond ctx; + MockReleaseRequest *req = MockReleaseRequest::create(mock_image_ctx, + TEST_COOKIE, + nullptr, &ctx); + req->send(); + ASSERT_EQ(-EINVAL, ctx.wait()); +} + TEST_F(TestMockExclusiveLockReleaseRequest, UnlockError) { REQUIRE_FEATURE(RBD_FEATURE_EXCLUSIVE_LOCK); @@ -141,6 +180,7 @@ TEST_F(TestMockExclusiveLockReleaseRequest, UnlockError) { expect_op_work_queue(mock_image_ctx); InSequence seq; + expect_block_writes(mock_image_ctx, 0); expect_cancel_op_requests(mock_image_ctx, 0); expect_unlock(mock_image_ctx, -EINVAL); @@ -148,7 +188,7 @@ TEST_F(TestMockExclusiveLockReleaseRequest, UnlockError) { C_SaferCond ctx; MockReleaseRequest *req = MockReleaseRequest::create(mock_image_ctx, TEST_COOKIE, - &ctx); + nullptr, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } diff --git a/src/test/librbd/test_mock_ExclusiveLock.cc b/src/test/librbd/test_mock_ExclusiveLock.cc index 56b2701295c0..cac28a6c64ae 100644 --- a/src/test/librbd/test_mock_ExclusiveLock.cc +++ b/src/test/librbd/test_mock_ExclusiveLock.cc @@ -17,12 +17,14 @@ namespace exclusive_lock { template struct BaseRequest { static std::list s_requests; + Context *on_lock_unlock; Context *on_finish; static T* create(MockImageCtx &image_ctx, const std::string &cookie, - Context *on_finish) { + Context *on_lock_unlock, Context *on_finish) { assert(!s_requests.empty()); T* req = s_requests.front(); + req->on_lock_unlock = on_lock_unlock; req->on_finish = on_finish; s_requests.pop_front(); return req; @@ -87,21 +89,14 @@ public: .WillOnce(FinishRequest(&acquire_request, r, &mock_image_ctx)); if (r == 0) { expect_notify_acquired_lock(mock_image_ctx); - expect_unblock_writes(mock_image_ctx); } } void expect_release_lock(MockImageCtx &mock_image_ctx, MockReleaseRequest &release_request, int r, bool shutting_down = false) { - if (!shutting_down) { - expect_block_writes(mock_image_ctx); - } EXPECT_CALL(release_request, send()) .WillOnce(FinishRequest(&release_request, r, &mock_image_ctx)); - if (!shutting_down && r < 0) { - expect_unblock_writes(mock_image_ctx); - } if (r == 0) { expect_notify_released_lock(mock_image_ctx); expect_writes_empty(mock_image_ctx); @@ -506,7 +501,6 @@ TEST_F(TestMockExclusiveLock, ConcurrentRequests) { MockReleaseRequest release; C_SaferCond wait_for_send_ctx2; - expect_block_writes(mock_image_ctx); EXPECT_CALL(release, send()) .WillOnce(Notify(&wait_for_send_ctx2)); expect_notify_released_lock(mock_image_ctx); -- 2.47.3