librbd: utilize the journal disabled policy when removing images

Fixes: http://tracker.ceph.com/issues/23512
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(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 5782702d78f926640edc68b57e7088764e181e16..10efaa677df7c235c9a1fc49e98fc6aa2c949f69 100644 (file)
--- 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<typename I>
 void RemoveRequest<I>::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<I>;
   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 77c536d03a8712e46cc04c6797f9ad8a08f20042..e253c43deaa55cbbc2057ba9a324aa3c496aeb0a 100644 (file)
--- a/src/test/librbd/image/test_mock_RemoveRequest.cc
+++ b/src/test/librbd/image/test_mock_RemoveRequest.cc
@@ -113,6 +113,17 @@ DisableRequest<MockImageCtx> *DisableRequest<MockImageCtx>::s_instance;
 #include "librbd/image/RemoveRequest.cc"
 template class librbd::image::RemoveRequest<librbd::MockImageCtx>;
 
+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());