From ecea6dcf1c36bc5d478cf030f7ba1e01ca35a2d0 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Thu, 4 Aug 2016 13:24:54 -0400 Subject: [PATCH] librbd: delay acquiring exclusive lock if watch has failed Fixes: http://tracker.ceph.com/issues/16923 Signed-off-by: Jason Dillaman (cherry picked from commit dfe9f3eac9cca3b83962e0e1b7eac38e6e76d7a5) --- src/librbd/ExclusiveLock.cc | 23 ++++++++++++++- src/librbd/ExclusiveLock.h | 13 ++++++--- src/librbd/ImageWatcher.cc | 9 ++++++ src/test/librbd/test_mock_ExclusiveLock.cc | 33 ++++++++++++++++++++++ 4 files changed, 73 insertions(+), 5 deletions(-) diff --git a/src/librbd/ExclusiveLock.cc b/src/librbd/ExclusiveLock.cc index c3ba7c72830a1..4c0501c917cf4 100644 --- a/src/librbd/ExclusiveLock.cc +++ b/src/librbd/ExclusiveLock.cc @@ -194,6 +194,20 @@ void ExclusiveLock::release_lock(Context *on_released) { on_released->complete(r); } +template +void ExclusiveLock::handle_watch_registered() { + Mutex::Locker locker(m_lock); + if (m_state != STATE_WAITING_FOR_REGISTER) { + return; + } + + ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl; + Action active_action = get_active_action(); + assert(active_action == ACTION_TRY_LOCK || + active_action == ACTION_REQUEST_LOCK); + execute_next_action(); +} + template void ExclusiveLock::handle_lock_released() { Mutex::Locker locker(m_lock); @@ -240,6 +254,7 @@ bool ExclusiveLock::is_transition_state() const { case STATE_INITIALIZING: case STATE_ACQUIRING: case STATE_WAITING_FOR_PEER: + case STATE_WAITING_FOR_REGISTER: case STATE_POST_ACQUIRING: case STATE_PRE_RELEASING: case STATE_RELEASING: @@ -358,10 +373,16 @@ void ExclusiveLock::send_acquire_lock() { return; } - ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl; + CephContext *cct = m_image_ctx.cct; + ldout(cct, 10) << this << " " << __func__ << dendl; m_state = STATE_ACQUIRING; m_watch_handle = m_image_ctx.image_watcher->get_watch_handle(); + if (m_watch_handle == 0) { + lderr(cct) << "image watcher not registered - delaying request" << dendl; + m_state = STATE_WAITING_FOR_REGISTER; + return; + } using el = ExclusiveLock; AcquireRequest* req = AcquireRequest::create( diff --git a/src/librbd/ExclusiveLock.h b/src/librbd/ExclusiveLock.h index f901ea22bd5ce..e82da208afc22 100644 --- a/src/librbd/ExclusiveLock.h +++ b/src/librbd/ExclusiveLock.h @@ -41,6 +41,7 @@ public: void request_lock(Context *on_locked); void release_lock(Context *on_released); + void handle_watch_registered(); void handle_lock_released(); void assert_header_locked(librados::ObjectWriteOperation *op); @@ -50,10 +51,13 @@ public: private: /** - * WAITING_FOR_PEER -----------------\ - * | ^ | - * | * (request_lock busy) | - * | * * * * * * * * * * * * | + * * * > WAITING_FOR_REGISTER --------\ + * | * (watch not registered) | + * | * | + * | * * > WAITING_FOR_PEER ------------\ + * | * (request_lock busy) | + * | * | + * | * * * * * * * * * * * * * * | * | * | * v (init) (try_lock/request_lock) * | * UNINITIALIZED -------> UNLOCKED ------------------------> ACQUIRING <--/ @@ -79,6 +83,7 @@ private: STATE_ACQUIRING, STATE_POST_ACQUIRING, STATE_WAITING_FOR_PEER, + STATE_WAITING_FOR_REGISTER, STATE_PRE_RELEASING, STATE_RELEASING, STATE_PRE_SHUTTING_DOWN, diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc index ef5cdf1102eeb..7aeb1c93489c8 100644 --- a/src/librbd/ImageWatcher.cc +++ b/src/librbd/ImageWatcher.cc @@ -1050,6 +1050,15 @@ void ImageWatcher::reregister_watch() { m_watch_state = WATCH_STATE_REGISTERED; } + + // if the exclusive lock state machine was paused waiting for the + // watch to be re-registered, wake it up + RWLock::RLocker owner_locker(m_image_ctx.owner_lock); + RWLock::RLocker snap_locker(m_image_ctx.snap_lock); + if (m_image_ctx.exclusive_lock != nullptr) { + m_image_ctx.exclusive_lock->handle_watch_registered(); + } + handle_payload(HeaderUpdatePayload(), NULL); } diff --git a/src/test/librbd/test_mock_ExclusiveLock.cc b/src/test/librbd/test_mock_ExclusiveLock.cc index e508c8d202710..f87a319b37796 100644 --- a/src/test/librbd/test_mock_ExclusiveLock.cc +++ b/src/test/librbd/test_mock_ExclusiveLock.cc @@ -627,5 +627,38 @@ TEST_F(TestMockExclusiveLock, BlockRequests) { ASSERT_FALSE(is_lock_owner(mock_image_ctx, exclusive_lock)); } +TEST_F(TestMockExclusiveLock, RequestLockWatchNotRegistered) { + REQUIRE_FEATURE(RBD_FEATURE_EXCLUSIVE_LOCK); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockExclusiveLockImageCtx mock_image_ctx(*ictx); + MockExclusiveLock exclusive_lock(mock_image_ctx); + expect_op_work_queue(mock_image_ctx); + + InSequence seq; + expect_block_writes(mock_image_ctx); + ASSERT_EQ(0, when_init(mock_image_ctx, exclusive_lock)); + + EXPECT_CALL(*mock_image_ctx.image_watcher, get_watch_handle()) + .WillOnce(DoAll(Invoke([&mock_image_ctx, &exclusive_lock]() { + mock_image_ctx.image_ctx->op_work_queue->queue( + new FunctionContext([&exclusive_lock](int r) { + exclusive_lock.handle_watch_registered(); + })); + }), + Return(0))); + MockAcquireRequest request_lock_acquire; + expect_acquire_lock(mock_image_ctx, request_lock_acquire, 0); + ASSERT_EQ(0, when_request_lock(mock_image_ctx, exclusive_lock)); + ASSERT_TRUE(is_lock_owner(mock_image_ctx, exclusive_lock)); + + MockReleaseRequest shutdown_release; + expect_release_lock(mock_image_ctx, shutdown_release, 0, true); + ASSERT_EQ(0, when_shut_down(mock_image_ctx, exclusive_lock)); + ASSERT_FALSE(is_lock_owner(mock_image_ctx, exclusive_lock)); +} + } // namespace librbd -- 2.39.5