From: Jason Dillaman Date: Wed, 30 May 2018 14:34:48 +0000 (-0400) Subject: librbd: utilize the journal disabled policy when removing images X-Git-Tag: v12.2.8~17^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F23595%2Fhead;p=ceph.git librbd: utilize the journal disabled policy when removing images Fixes: http://tracker.ceph.com/issues/23512 Signed-off-by: Jason Dillaman (cherry picked from commit cc6ef6d57a71e19dc748890299f41e4b6403058a) Conflicts: src/librbd/image/RemoveRequest.cc: trivial resolution src/test/librbd/image/test_mock_RemoveRequest.cc: trivial resolution --- diff --git a/src/librbd/image/RemoveRequest.cc b/src/librbd/image/RemoveRequest.cc index 5782702d78f9..10efaa677df7 100644 --- a/src/librbd/image/RemoveRequest.cc +++ b/src/librbd/image/RemoveRequest.cc @@ -9,6 +9,7 @@ #include "librbd/ObjectMap.h" #include "librbd/ExclusiveLock.h" #include "librbd/MirroringWatcher.h" +#include "librbd/journal/DisabledPolicy.h" #include "librbd/journal/RemoveRequest.h" #include "librbd/image/RemoveRequest.h" #include "librbd/operation/TrimRequest.h" @@ -103,6 +104,13 @@ template void RemoveRequest::acquire_exclusive_lock() { ldout(m_cct, 20) << dendl; + // do not attempt to open the journal when removing the image in case + // it's corrupt + if (m_image_ctx->test_features(RBD_FEATURE_JOURNALING)) { + RWLock::WLocker snap_locker(m_image_ctx->snap_lock); + m_image_ctx->set_journal_policy(new journal::DisabledPolicy()); + } + using klass = RemoveRequest; if (m_force) { Context *ctx = create_context_callback< diff --git a/src/test/librbd/image/test_mock_RemoveRequest.cc b/src/test/librbd/image/test_mock_RemoveRequest.cc index 77c536d03a87..e253c43deaa5 100644 --- a/src/test/librbd/image/test_mock_RemoveRequest.cc +++ b/src/test/librbd/image/test_mock_RemoveRequest.cc @@ -113,6 +113,17 @@ DisableRequest *DisableRequest::s_instance; #include "librbd/image/RemoveRequest.cc" template class librbd::image::RemoveRequest; +ACTION_P(TestFeatures, image_ctx) { + return ((image_ctx->features & arg0) != 0); +} + +ACTION_P(ShutDownExclusiveLock, image_ctx) { + // shutting down exclusive lock will close object map and journal + image_ctx->exclusive_lock = nullptr; + image_ctx->object_map = nullptr; + image_ctx->journal = nullptr; +} + namespace librbd { namespace image { @@ -230,6 +241,34 @@ public: _, _, _)) .WillOnce(Return(r)); } + + void expect_test_features(MockImageCtx &mock_image_ctx) { + if (m_mock_imctx->exclusive_lock != nullptr) { + EXPECT_CALL(mock_image_ctx, test_features(_)) + .WillRepeatedly(TestFeatures(&mock_image_ctx)); + } + } + + void expect_set_journal_policy(MockImageCtx &mock_image_ctx) { + if (m_test_imctx->test_features(RBD_FEATURE_JOURNALING)) { + EXPECT_CALL(mock_image_ctx, set_journal_policy(_)) + .WillOnce(Invoke([](journal::Policy* policy) { + ASSERT_TRUE(policy->journal_disabled()); + delete policy; + })); + } + } + + void expect_shut_down_exclusive_lock(MockImageCtx &mock_image_ctx, + MockExclusiveLock &mock_exclusive_lock, + int r) { + if (m_mock_imctx->exclusive_lock != nullptr) { + EXPECT_CALL(mock_exclusive_lock, shut_down(_)) + .WillOnce(DoAll(ShutDownExclusiveLock(&mock_image_ctx), + CompleteContext(r, mock_image_ctx.image_ctx->op_work_queue))); + } + } + }; TEST_F(TestMockImageRemoveRequest, SuccessV1) { @@ -285,6 +324,12 @@ TEST_F(TestMockImageRemoveRequest, SuccessV2) { REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); TestImageRemoveSetUp(); + MockExclusiveLock *mock_exclusive_lock = nullptr; + if (m_test_imctx->test_features(RBD_FEATURE_EXCLUSIVE_LOCK)) { + mock_exclusive_lock = new MockExclusiveLock(); + m_mock_imctx->exclusive_lock = mock_exclusive_lock; + } + C_SaferCond ctx; librbd::NoOpProgressContext no_op; ContextWQ op_work_queue; @@ -294,6 +339,11 @@ TEST_F(TestMockImageRemoveRequest, SuccessV2) { InSequence seq; expect_state_open(*m_mock_imctx, 0); + + expect_test_features(*m_mock_imctx); + expect_set_journal_policy(*m_mock_imctx); + expect_shut_down_exclusive_lock(*m_mock_imctx, *mock_exclusive_lock, 0); + expect_mirror_image_get(*m_mock_imctx, 0); expect_get_group(*m_mock_imctx, 0); expect_trim(*m_mock_imctx, mock_trim_request, 0); @@ -306,8 +356,8 @@ TEST_F(TestMockImageRemoveRequest, SuccessV2) { expect_remove_mirror_image(m_ioctx, 0); expect_dir_remove_image(m_ioctx, 0); - MockRemoveRequest *req = MockRemoveRequest::create(m_ioctx, m_image_name, "", - true, false, no_op, &op_work_queue, &ctx); + MockRemoveRequest *req = MockRemoveRequest::create( + m_ioctx, m_image_name, "", true, false, no_op, &op_work_queue, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); @@ -319,6 +369,12 @@ TEST_F(TestMockImageRemoveRequest, NotExistsV2) { REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); TestImageRemoveSetUp(); + MockExclusiveLock *mock_exclusive_lock = nullptr; + if (m_test_imctx->test_features(RBD_FEATURE_EXCLUSIVE_LOCK)) { + mock_exclusive_lock = new MockExclusiveLock(); + m_mock_imctx->exclusive_lock = mock_exclusive_lock; + } + C_SaferCond ctx; librbd::NoOpProgressContext no_op; ContextWQ op_work_queue; @@ -328,6 +384,11 @@ TEST_F(TestMockImageRemoveRequest, NotExistsV2) { InSequence seq; expect_state_open(*m_mock_imctx, 0); + + expect_test_features(*m_mock_imctx); + expect_set_journal_policy(*m_mock_imctx); + expect_shut_down_exclusive_lock(*m_mock_imctx, *mock_exclusive_lock, 0); + expect_mirror_image_get(*m_mock_imctx, 0); expect_get_group(*m_mock_imctx, 0); expect_trim(*m_mock_imctx, mock_trim_request, 0); @@ -340,8 +401,8 @@ TEST_F(TestMockImageRemoveRequest, NotExistsV2) { expect_remove_mirror_image(m_ioctx, 0); expect_dir_remove_image(m_ioctx, -ENOENT); - MockRemoveRequest *req = MockRemoveRequest::create(m_ioctx, m_image_name, "", - true, false, no_op, &op_work_queue, &ctx); + MockRemoveRequest *req = MockRemoveRequest::create( + m_ioctx, m_image_name, "", true, false, no_op, &op_work_queue, &ctx); req->send(); ASSERT_EQ(-ENOENT, ctx.wait());