From cb35c6c3bc60b1ed6e2133acf1fbd3a803502f5a Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Wed, 7 Oct 2020 10:58:57 -0400 Subject: [PATCH] librbd: avoid failing IO with -ESHUTDOWN when disabling exclusive-lock When dynamically disabling the exclusive-lock feature with in-flight IO, it was previously possible for IO to fail with -ESHUTDOWN when it attempts to acquire the lock due to the PreReleaseRequest setting the lock required flag. There is also the possibility for a race with the first IO racing with exclusive-lock shutdown when the lock was not already acquired. Signed-off-by: Jason Dillaman --- src/librbd/exclusive_lock/ImageDispatch.cc | 4 +- .../exclusive_lock/PreReleaseRequest.cc | 6 +++ .../test_mock_PreReleaseRequest.cc | 44 +++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/librbd/exclusive_lock/ImageDispatch.cc b/src/librbd/exclusive_lock/ImageDispatch.cc index c4e04970a7c..cd7f450f275 100644 --- a/src/librbd/exclusive_lock/ImageDispatch.cc +++ b/src/librbd/exclusive_lock/ImageDispatch.cc @@ -290,7 +290,9 @@ void ImageDispatch::handle_acquire_lock(int r) { Context* failed_dispatch = nullptr; Contexts on_dispatches; - if (r < 0) { + if (r == -ESHUTDOWN) { + ldout(cct, 5) << "IO raced with exclusive lock shutdown" << dendl; + } else if (r < 0) { lderr(cct) << "failed to acquire exclusive lock: " << cpp_strerror(r) << dendl; failed_dispatch = m_on_dispatches.front(); diff --git a/src/librbd/exclusive_lock/PreReleaseRequest.cc b/src/librbd/exclusive_lock/PreReleaseRequest.cc index 83799eb5a24..94fe640ba2f 100644 --- a/src/librbd/exclusive_lock/PreReleaseRequest.cc +++ b/src/librbd/exclusive_lock/PreReleaseRequest.cc @@ -84,6 +84,12 @@ void PreReleaseRequest::handle_cancel_op_requests(int r) { template void PreReleaseRequest::send_set_require_lock() { + if (!m_image_ctx.test_features(RBD_FEATURE_EXCLUSIVE_LOCK)) { + // exclusive-lock was disabled, no need to block IOs + send_wait_for_ops(); + return; + } + CephContext *cct = m_image_ctx.cct; ldout(cct, 10) << dendl; diff --git a/src/test/librbd/exclusive_lock/test_mock_PreReleaseRequest.cc b/src/test/librbd/exclusive_lock/test_mock_PreReleaseRequest.cc index 1abf291fa5d..c1c2aa4435a 100644 --- a/src/test/librbd/exclusive_lock/test_mock_PreReleaseRequest.cc +++ b/src/test/librbd/exclusive_lock/test_mock_PreReleaseRequest.cc @@ -98,6 +98,7 @@ public: void expect_set_require_lock(MockTestImageCtx &mock_image_ctx, MockImageDispatch &mock_image_dispatch, bool init_shutdown, int r) { + expect_test_features(mock_image_ctx, RBD_FEATURE_EXCLUSIVE_LOCK, true); expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, ((mock_image_ctx.features & RBD_FEATURE_JOURNALING) != 0)); if (mock_image_ctx.clone_copy_on_read || @@ -335,5 +336,48 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, Blocklisted) { ASSERT_EQ(0, ctx.wait()); } +TEST_F(TestMockExclusiveLockPreReleaseRequest, Disabled) { + REQUIRE_FEATURE(RBD_FEATURE_EXCLUSIVE_LOCK); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockTestImageCtx mock_image_ctx(*ictx); + expect_op_work_queue(mock_image_ctx); + + InSequence seq; + + expect_cancel_op_requests(mock_image_ctx, 0); + MockImageDispatch mock_image_dispatch; + + expect_test_features(mock_image_ctx, RBD_FEATURE_EXCLUSIVE_LOCK, false); + + expect_prepare_lock(mock_image_ctx); + + expect_close_image_cache(mock_image_ctx, 0); + + expect_invalidate_cache(mock_image_ctx, 0); + + expect_flush_io(mock_image_ctx, 0); + + expect_flush_notifies(mock_image_ctx); + + MockJournal mock_journal; + mock_image_ctx.journal = &mock_journal; + expect_close_journal(mock_image_ctx, mock_journal, -EINVAL); + + MockObjectMap mock_object_map; + mock_image_ctx.object_map = &mock_object_map; + expect_close_object_map(mock_image_ctx, mock_object_map); + + expect_handle_prepare_lock_complete(mock_image_ctx); + + C_SaferCond ctx; + MockPreReleaseRequest *req = MockPreReleaseRequest::create( + mock_image_ctx, &mock_image_dispatch, false, m_async_op_tracker, &ctx); + req->send(); + ASSERT_EQ(0, ctx.wait()); +} + } // namespace exclusive_lock } // namespace librbd