From 51ac19387c58dd81f66dce77fef4e0e36ce524ea Mon Sep 17 00:00:00 2001 From: Song Shun Date: Tue, 20 Mar 2018 11:07:28 +0800 Subject: [PATCH] librbd: fix refuse to release lock when cookie is the same at rewatch fix exclusive auto-mode lock refuse to release. when rewatch, owner_id is reset. at the same time, there is a chance to produce the same cookie, which should be different. code now skips reacquire lock when the cookie is the same, resulting in unsetting owner_id. when other clients request lock, client whose owner_id is null is considered invalid and refuse to release lock. but unluckily, watcher is always alive, so the client requested lock can't get lock. Signed-off-by: Song Shun --- src/librbd/ManagedLock.cc | 11 ++++- src/librbd/ManagedLock.h | 2 + src/test/librbd/test_mock_ManagedLock.cc | 52 ++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 1 deletion(-) diff --git a/src/librbd/ManagedLock.cc b/src/librbd/ManagedLock.cc index a9c6f6234fa6d..53bfd26226655 100644 --- a/src/librbd/ManagedLock.cc +++ b/src/librbd/ManagedLock.cc @@ -512,6 +512,13 @@ void ManagedLock::handle_acquire_lock(int r) { })); } +template +void ManagedLock::handle_no_op_reacquire_lock(int r) { + ldout(m_cct, 10) << "r=" << r << dendl; + assert(r >= 0); + complete_active_action(STATE_LOCKED, 0); +} + template void ManagedLock::handle_post_acquire_lock(int r) { ldout(m_cct, 10) << "r=" << r << dendl; @@ -564,7 +571,9 @@ void ManagedLock::send_reacquire_lock() { if (m_cookie == m_new_cookie) { ldout(m_cct, 10) << "skipping reacquire since cookie still valid" << dendl; - complete_active_action(STATE_LOCKED, 0); + auto ctx = create_context_callback< + ManagedLock, &ManagedLock::handle_no_op_reacquire_lock>(this); + post_reacquire_lock_handler(0, ctx); return; } diff --git a/src/librbd/ManagedLock.h b/src/librbd/ManagedLock.h index c619f4823dd22..fdb6d43d78c41 100644 --- a/src/librbd/ManagedLock.h +++ b/src/librbd/ManagedLock.h @@ -240,6 +240,8 @@ private: void send_acquire_lock(); void handle_pre_acquire_lock(int r); void handle_acquire_lock(int r); + void handle_no_op_reacquire_lock(int r); + void handle_post_acquire_lock(int r); void revert_to_unlock_state(int r); diff --git a/src/test/librbd/test_mock_ManagedLock.cc b/src/test/librbd/test_mock_ManagedLock.cc index 4cd72a13d202f..e4b8c4c0b0c10 100644 --- a/src/test/librbd/test_mock_ManagedLock.cc +++ b/src/test/librbd/test_mock_ManagedLock.cc @@ -26,6 +26,18 @@ struct Traits { }; } +struct MockMockManagedLock : public ManagedLock { + MockMockManagedLock(librados::IoCtx& ioctx, ContextWQ *work_queue, + const std::string& oid, librbd::MockImageWatcher *watcher, + managed_lock::Mode mode, bool blacklist_on_break_lock, + uint32_t blacklist_expire_seconds) + : ManagedLock(ioctx, work_queue, oid, watcher, + librbd::managed_lock::EXCLUSIVE, true, 0) { + }; + virtual ~MockMockManagedLock() = default; + MOCK_METHOD2(post_reacquire_lock_handler, void(int, Context*)); +}; + namespace managed_lock { template @@ -192,6 +204,17 @@ public: .WillOnce(CompleteContext(0, (ContextWQ *)nullptr)); } + void expect_post_reacquired_lock_handler(MockImageWatcher& watcher, + MockMockManagedLock &managed_lock, uint64_t &client_id) { + expect_get_watch_handle(watcher); + EXPECT_CALL(managed_lock, post_reacquire_lock_handler(_, _)) + .WillOnce(Invoke([&watcher, &client_id](int r, Context *on_finish){ + if (r >= 0) { + client_id = 98765; + } + on_finish->complete(r);})); + } + int when_acquire_lock(MockManagedLock &managed_lock) { C_SaferCond ctx; { @@ -503,4 +526,33 @@ TEST_F(TestMockManagedLock, ReacquireLockError) { ASSERT_FALSE(is_lock_owner(managed_lock)); } +TEST_F(TestMockManagedLock, ReacquireWithSameCookie) { + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockManagedLockImageCtx mock_image_ctx(*ictx); + MockMockManagedLock managed_lock(ictx->md_ctx, ictx->op_work_queue, + ictx->header_oid, mock_image_ctx.image_watcher, + librbd::managed_lock::EXCLUSIVE, true, 0); + InSequence seq; + + MockAcquireRequest request_lock_acquire; + expect_acquire_lock(*mock_image_ctx.image_watcher, ictx->op_work_queue, request_lock_acquire, 0); + ASSERT_EQ(0, when_acquire_lock(managed_lock)); + ASSERT_TRUE(is_lock_owner(managed_lock)); + + // watcher with same cookie after rewatch + uint64_t client_id = 0; + C_SaferCond reacquire_ctx; + expect_post_reacquired_lock_handler(*mock_image_ctx.image_watcher, managed_lock, client_id); + managed_lock.reacquire_lock(&reacquire_ctx); + ASSERT_LT(0, client_id); + ASSERT_TRUE(is_lock_owner(managed_lock)); + + MockReleaseRequest shutdown_release; + expect_release_lock(ictx->op_work_queue, shutdown_release, 0); + //ASSERT_EQ(0, when_release_lock(managed_lock)); + ASSERT_EQ(0, when_shut_down(managed_lock)); +} + } // namespace librbd -- 2.39.5