From: Jason Dillaman <dillaman@redhat.com>
Date: Tue, 27 Jun 2017 19:13:32 +0000 (-0400)
Subject: librbd: properly close and destroy journal when dynamically disabled
X-Git-Tag: v12.1.1~185^2~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=87f8187ab8d63c3b236c274dd2c31752a6fbc2ba;p=ceph.git

librbd: properly close and destroy journal when dynamically disabled

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
---

diff --git a/src/librbd/operation/DisableFeaturesRequest.cc b/src/librbd/operation/DisableFeaturesRequest.cc
index 41eddbd7c8c0..34cc9579fe84 100644
--- a/src/librbd/operation/DisableFeaturesRequest.cc
+++ b/src/librbd/operation/DisableFeaturesRequest.cc
@@ -354,14 +354,14 @@ void DisableFeaturesRequest<I>::send_close_journal() {
   {
     RWLock::WLocker locker(image_ctx.owner_lock);
     if (image_ctx.journal != nullptr) {
-
       ldout(cct, 20) << this << " " << __func__ << dendl;
 
+      std::swap(m_journal, image_ctx.journal);
       Context *ctx = create_context_callback<
 	DisableFeaturesRequest<I>,
 	&DisableFeaturesRequest<I>::handle_close_journal>(this);
 
-      image_ctx.journal->close(ctx);
+      m_journal->close(ctx);
       return;
     }
   }
@@ -378,9 +378,12 @@ Context *DisableFeaturesRequest<I>::handle_close_journal(int *result) {
   if (*result < 0) {
     lderr(cct) << "failed to close image journal: " << cpp_strerror(*result)
                << dendl;
-    return handle_finish(*result);
   }
 
+  assert(m_journal != nullptr);
+  delete m_journal;
+  m_journal = nullptr;
+
   send_remove_journal();
   return nullptr;
 }
diff --git a/src/librbd/operation/DisableFeaturesRequest.h b/src/librbd/operation/DisableFeaturesRequest.h
index 623eb1f87dca..b064bc45f793 100644
--- a/src/librbd/operation/DisableFeaturesRequest.h
+++ b/src/librbd/operation/DisableFeaturesRequest.h
@@ -4,6 +4,7 @@
 #ifndef CEPH_LIBRBD_OPERATION_DISABLE_FEATURES_REQUEST_H
 #define CEPH_LIBRBD_OPERATION_DISABLE_FEATURES_REQUEST_H
 
+#include "librbd/ImageCtx.h"
 #include "librbd/operation/Request.h"
 #include "cls/rbd/cls_rbd_client.h"
 
@@ -113,6 +114,7 @@ private:
   uint64_t m_disable_flags = 0;
   uint64_t m_features_mask = 0;
 
+  decltype(ImageCtxT::journal) m_journal = nullptr;
   cls::rbd::MirrorMode m_mirror_mode = cls::rbd::MIRROR_MODE_DISABLED;
   bufferlist m_out_bl;
 
diff --git a/src/test/librbd/operation/test_mock_DisableFeaturesRequest.cc b/src/test/librbd/operation/test_mock_DisableFeaturesRequest.cc
index 7ae6d96a872d..dcbfa2ec2aa8 100644
--- a/src/test/librbd/operation/test_mock_DisableFeaturesRequest.cc
+++ b/src/test/librbd/operation/test_mock_DisableFeaturesRequest.cc
@@ -292,9 +292,13 @@ TEST_F(TestMockOperationDisableFeaturesRequest, All) {
 
   MockOperationImageCtx mock_image_ctx(*ictx);
   MockExclusiveLock mock_exclusive_lock;
-  MockJournal mock_journal;
+  MockJournal mock_journal_stack;
+  MockJournal *mock_journal = &mock_journal_stack;
+  if (features_to_disable & RBD_FEATURE_JOURNALING) {
+    mock_journal = new MockJournal();
+  }
   MockObjectMap mock_object_map;
-  initialize_features(ictx, mock_image_ctx, mock_exclusive_lock, mock_journal,
+  initialize_features(ictx, mock_image_ctx, mock_exclusive_lock, *mock_journal,
 		      mock_object_map);
 
   expect_verify_lock_ownership(mock_image_ctx);
@@ -440,9 +444,9 @@ TEST_F(TestMockOperationDisableFeaturesRequest, Mirroring) {
 
   MockOperationImageCtx mock_image_ctx(*ictx);
   MockExclusiveLock mock_exclusive_lock;
-  MockJournal mock_journal;
+  MockJournal *mock_journal = new MockJournal();
   MockObjectMap mock_object_map;
-  initialize_features(ictx, mock_image_ctx, mock_exclusive_lock, mock_journal,
+  initialize_features(ictx, mock_image_ctx, mock_exclusive_lock, *mock_journal,
 		      mock_object_map);
 
   expect_verify_lock_ownership(mock_image_ctx);
@@ -483,9 +487,9 @@ TEST_F(TestMockOperationDisableFeaturesRequest, MirroringError) {
 
   MockOperationImageCtx mock_image_ctx(*ictx);
   MockExclusiveLock mock_exclusive_lock;
-  MockJournal mock_journal;
+  MockJournal *mock_journal = new MockJournal();
   MockObjectMap mock_object_map;
-  initialize_features(ictx, mock_image_ctx, mock_exclusive_lock, mock_journal,
+  initialize_features(ictx, mock_image_ctx, mock_exclusive_lock, *mock_journal,
 		      mock_object_map);
 
   expect_verify_lock_ownership(mock_image_ctx);