librbd: clean up pre-release lock handling

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

diff --git a/src/librbd/ExclusiveLock.cc b/src/librbd/ExclusiveLock.cc
index b500a980e1407f13f31d34d5baad9f1bb95a3772..593537ce0150bef4f1e482fb373fdb07e8816b19 100644 (file)
--- a/src/librbd/ExclusiveLock.cc
+++ b/src/librbd/ExclusiveLock.cc
@@ -30,8 +30,7 @@ ExclusiveLock<I>::ExclusiveLock(I &image_ctx)
           image_ctx.image_watcher, managed_lock::EXCLUSIVE,
           image_ctx.blacklist_on_break_lock,
           image_ctx.blacklist_expire_seconds),
-    m_image_ctx(image_ctx), m_pre_post_callback(nullptr),
-    m_shutting_down(false) {
+    m_image_ctx(image_ctx) {
   Mutex::Locker locker(ML<I>::m_lock);
   ML<I>::set_state_uninitialized();
 }
@@ -249,34 +248,13 @@ void ExclusiveLock<I>::pre_release_lock_handler(bool shutting_down,
   ldout(m_image_ctx.cct, 10) << dendl;
   Mutex::Locker locker(ML<I>::m_lock);
 
-  m_shutting_down = shutting_down;
-
-  using EL = ExclusiveLock<I>;
-  PreReleaseRequest<I> *req = PreReleaseRequest<I>::create(m_image_ctx,
-      util::create_context_callback<EL, &EL::handle_pre_releasing_lock>(this),
-      on_finish, shutting_down);
-
+  PreReleaseRequest<I> *req = PreReleaseRequest<I>::create(
+    m_image_ctx, shutting_down, on_finish);
   m_image_ctx.op_work_queue->queue(new FunctionContext([req](int r) {
     req->send();
   }));
 }
 
-template <typename I>
-void ExclusiveLock<I>::handle_pre_releasing_lock(int r) {
-  ldout(m_image_ctx.cct, 10) << dendl;
-
-  Mutex::Locker locker(ML<I>::m_lock);
-
-  assert(r == 0);
-
-  // all IO and ops should be blocked/canceled by this point
-  if (!m_shutting_down) {
-    ML<I>::set_state_releasing();
-  } else {
-    ML<I>::set_state_shutting_down();
-  }
-}
-
 template <typename I>
 void ExclusiveLock<I>::post_release_lock_handler(bool shutting_down, int r,
                                                  Context *on_finish) {

diff --git a/src/librbd/ExclusiveLock.h b/src/librbd/ExclusiveLock.h
index c12949c0fb9a11b018de82511f7a062b55673a2f..c03cf6414d2db4c7972e365792ed07f68bd7bd40 100644 (file)
--- a/src/librbd/ExclusiveLock.h
+++ b/src/librbd/ExclusiveLock.h
@@ -92,8 +92,7 @@ private:
   };
 
   ImageCtxT& m_image_ctx;
-  Context *m_pre_post_callback;
-  bool m_shutting_down;
+  Context *m_pre_post_callback = nullptr;
 
   uint32_t m_request_blocked_count = 0;
   int m_request_blocked_ret_val = 0;
@@ -103,7 +102,6 @@ private:
   void handle_init_complete();
   void handle_post_acquiring_lock(int r);
   void handle_post_acquired_lock(int r);
-  void handle_pre_releasing_lock(int r);
 };
 
 } // namespace librbd

diff --git a/src/librbd/ManagedLock.cc b/src/librbd/ManagedLock.cc
index 5673694b01629d2650c6f957774db8a516700a67..21062bce454a660dc7624949909b1159b007f471 100644 (file)
--- a/src/librbd/ManagedLock.cc
+++ b/src/librbd/ManagedLock.cc
@@ -240,10 +240,6 @@ void  ManagedLock<I>::post_acquire_lock_handler(int r, Context *on_finish) {
 template <typename I>
 void  ManagedLock<I>::pre_release_lock_handler(bool shutting_down,
                                                Context *on_finish) {
-  {
-    Mutex::Locker locker(m_lock);
-    m_state = shutting_down ? STATE_SHUTTING_DOWN : STATE_RELEASING;
-  }
   on_finish->complete(0);
 }
 
@@ -565,6 +561,12 @@ template <typename I>
 void ManagedLock<I>::handle_pre_release_lock(int r) {
   ldout(m_cct, 10) << ": r=" << r << dendl;
 
+  {
+    Mutex::Locker locker(m_lock);
+    assert(m_state == STATE_PRE_RELEASING);
+    m_state = STATE_RELEASING;
+  }
+
   if (r < 0) {
     handle_release_lock(r);
     return;
@@ -654,6 +656,9 @@ void ManagedLock<I>::handle_shutdown_pre_release(int r) {
   {
     Mutex::Locker locker(m_lock);
     cookie = m_cookie;
+
+    assert(m_state == STATE_PRE_SHUTTING_DOWN);
+    m_state = STATE_SHUTTING_DOWN;
   }
 
   using managed_lock::ReleaseRequest;

diff --git a/src/librbd/ManagedLock.h b/src/librbd/ManagedLock.h
index 7cbc295a93665aa521e675ee3cf0ebf6ad933d5c..b8bec661b918e83cf176e47c8716865c1cc221cd 100644 (file)
--- a/src/librbd/ManagedLock.h
+++ b/src/librbd/ManagedLock.h
@@ -89,16 +89,6 @@ protected:
     assert(m_state == STATE_ACQUIRING);
     m_state = STATE_POST_ACQUIRING;
   }
-  inline void set_state_releasing() {
-    assert(m_lock.is_locked());
-    assert(m_state == STATE_PRE_RELEASING);
-    m_state = STATE_RELEASING;
-  }
-  inline void set_state_shutting_down() {
-    assert(m_lock.is_locked());
-    assert(m_state == STATE_PRE_SHUTTING_DOWN);
-    m_state = STATE_SHUTTING_DOWN;
-  }
 
   bool is_state_shutdown() const;
   inline bool is_state_acquiring() const {

diff --git a/src/librbd/exclusive_lock/PreReleaseRequest.cc b/src/librbd/exclusive_lock/PreReleaseRequest.cc
index 5096ed8189c14b0f7126ba562995ce327a214899..44f9821720114b71aae47a75bd14131fbe6c6217 100644 (file)
--- a/src/librbd/exclusive_lock/PreReleaseRequest.cc
+++ b/src/librbd/exclusive_lock/PreReleaseRequest.cc
@@ -24,17 +24,15 @@ using util::create_context_callback;
 
 template <typename I>
 PreReleaseRequest<I>* PreReleaseRequest<I>::create(I &image_ctx,
-                                                   Context *on_releasing,
-                                                   Context *on_finish,
-                                                   bool shutting_down) {
-  return new PreReleaseRequest(image_ctx, on_releasing, on_finish,
-                               shutting_down);
+                                                   bool shutting_down,
+                                                   Context *on_finish) {
+  return new PreReleaseRequest(image_ctx, shutting_down, on_finish);
 }
 
 template <typename I>
-PreReleaseRequest<I>::PreReleaseRequest(I &image_ctx, Context *on_releasing,
-                                        Context *on_finish, bool shutting_down)
-  : m_image_ctx(image_ctx), m_on_releasing(on_releasing),
+PreReleaseRequest<I>::PreReleaseRequest(I &image_ctx, bool shutting_down,
+                                        Context *on_finish)
+  : m_image_ctx(image_ctx),
     m_on_finish(create_async_context_callback(image_ctx, on_finish)),
     m_shutting_down(shutting_down), m_error_result(0), m_object_map(nullptr),
     m_journal(nullptr) {
@@ -45,7 +43,6 @@ PreReleaseRequest<I>::~PreReleaseRequest() {
   if (!m_shutting_down) {
     m_image_ctx.state->handle_prepare_lock_complete();
   }
-  delete m_on_releasing;
 }
 
 template <typename I>
@@ -273,12 +270,6 @@ void PreReleaseRequest<I>::send_unlock() {
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 10) << __func__ << dendl;
 
-  if (m_on_releasing != nullptr) {
-    // alert caller that we no longer own the exclusive lock
-    m_on_releasing->complete(0);
-    m_on_releasing = nullptr;
-  }
-
   finish();
 }
 

diff --git a/src/librbd/exclusive_lock/PreReleaseRequest.h b/src/librbd/exclusive_lock/PreReleaseRequest.h
index 0fda36a091ad868057bffd95a2096f96f82d7652..98e3a1e9dac73cc467910603ff0baa2aa8c888de 100644 (file)
--- a/src/librbd/exclusive_lock/PreReleaseRequest.h
+++ b/src/librbd/exclusive_lock/PreReleaseRequest.h
@@ -18,8 +18,8 @@ namespace exclusive_lock {
 template <typename ImageCtxT = ImageCtx>
 class PreReleaseRequest {
 public:
-  static PreReleaseRequest* create(ImageCtxT &image_ctx, Context *on_releasing,
-                                   Context *on_finish, bool shutting_down);
+  static PreReleaseRequest* create(ImageCtxT &image_ctx, bool shutting_down,
+                                   Context *on_finish);
 
   ~PreReleaseRequest();
   void send();
@@ -57,11 +57,10 @@ private:
    * @endverbatim
    */
 
-  PreReleaseRequest(ImageCtxT &image_ctx, Context *on_releasing,
-                    Context *on_finish, bool shutting_down);
+  PreReleaseRequest(ImageCtxT &image_ctx, bool shutting_down,
+                    Context *on_finish);
 
   ImageCtxT &m_image_ctx;
-  Context *m_on_releasing;
   Context *m_on_finish;
   bool m_shutting_down;
 

diff --git a/src/test/librbd/exclusive_lock/test_mock_PreReleaseRequest.cc b/src/test/librbd/exclusive_lock/test_mock_PreReleaseRequest.cc
index b94ab914042eb10fc2444014d489db79d9e6873c..a2fda18bf20fb141ac97fb9f2f1bd694e4c9754b 100644 (file)
--- a/src/test/librbd/exclusive_lock/test_mock_PreReleaseRequest.cc
+++ b/src/test/librbd/exclusive_lock/test_mock_PreReleaseRequest.cc
@@ -145,13 +145,11 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, Success) {
   mock_image_ctx.object_map = mock_object_map;
   expect_close_object_map(mock_image_ctx, *mock_object_map);
 
-  MockContext mock_releasing_ctx;
-  expect_complete_context(mock_releasing_ctx, 0);
   expect_handle_prepare_lock_complete(mock_image_ctx);
 
   C_SaferCond ctx;
   MockPreReleaseRequest *req = MockPreReleaseRequest::create(
-    mock_image_ctx, &mock_releasing_ctx, &ctx, false);
+    mock_image_ctx, false, &ctx);
   req->send();
   ASSERT_EQ(0, ctx.wait());
 }
@@ -179,12 +177,10 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, SuccessJournalDisabled) {
 
   expect_handle_prepare_lock_complete(mock_image_ctx);
 
-  C_SaferCond release_ctx;
   C_SaferCond ctx;
   MockPreReleaseRequest *req = MockPreReleaseRequest::create(
-    mock_image_ctx, &release_ctx, &ctx, false);
+    mock_image_ctx, false, &ctx);
   req->send();
-  ASSERT_EQ(0, release_ctx.wait());
   ASSERT_EQ(0, ctx.wait());
 }
 
@@ -207,9 +203,8 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, SuccessObjectMapDisabled) {
   C_SaferCond release_ctx;
   C_SaferCond ctx;
   MockPreReleaseRequest *req = MockPreReleaseRequest::create(
-    mock_image_ctx, &release_ctx, &ctx, true);
+    mock_image_ctx, true, &ctx);
   req->send();
-  ASSERT_EQ(0, release_ctx.wait());
   ASSERT_EQ(0, ctx.wait());
 }
 
@@ -240,13 +235,11 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, Blacklisted) {
   mock_image_ctx.object_map = mock_object_map;
   expect_close_object_map(mock_image_ctx, *mock_object_map);
 
-  MockContext mock_releasing_ctx;
-  expect_complete_context(mock_releasing_ctx, 0);
   expect_handle_prepare_lock_complete(mock_image_ctx);
 
   C_SaferCond ctx;
   MockPreReleaseRequest *req = MockPreReleaseRequest::create(
-    mock_image_ctx, &mock_releasing_ctx, &ctx, false);
+    mock_image_ctx, false, &ctx);
   req->send();
   ASSERT_EQ(0, ctx.wait());
 }
@@ -268,7 +261,7 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, BlockWritesError) {
 
   C_SaferCond ctx;
   MockPreReleaseRequest *req = MockPreReleaseRequest::create(
-    mock_image_ctx, nullptr, &ctx, true);
+    mock_image_ctx, true, &ctx);
   req->send();
   ASSERT_EQ(-EINVAL, ctx.wait());
 }
@@ -291,7 +284,7 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, UnlockError) {
 
   C_SaferCond ctx;
   MockPreReleaseRequest *req = MockPreReleaseRequest::create(
-    mock_image_ctx, nullptr, &ctx, true);
+    mock_image_ctx, true, &ctx);
   req->send();
   ASSERT_EQ(0, ctx.wait());
 }

diff --git a/src/test/librbd/test_mock_ExclusiveLock.cc b/src/test/librbd/test_mock_ExclusiveLock.cc
index cf046f5442990d8e944ec44864f1412957101e98..fc36534da77b5d3d29f0195c233778c31d96ea5b 100644 (file)
--- a/src/test/librbd/test_mock_ExclusiveLock.cc
+++ b/src/test/librbd/test_mock_ExclusiveLock.cc
@@ -63,8 +63,6 @@ struct ManagedLock<MockExclusiveLockImageCtx> {
   MOCK_METHOD0(set_state_unlocked, void());
   MOCK_METHOD0(set_state_waiting_for_lock, void());
   MOCK_METHOD0(set_state_post_acquiring, void());
-  MOCK_METHOD0(set_state_releasing, void());
-  MOCK_METHOD0(set_state_shutting_down, void());
 
   MOCK_CONST_METHOD0(is_state_shutdown, bool());
   MOCK_CONST_METHOD0(is_state_acquiring, bool());
@@ -90,8 +88,7 @@ struct BaseRequest {
   Context *on_finish = nullptr;
 
   static T* create(MockExclusiveLockImageCtx &image_ctx,
-                   Context *on_lock_unlock, Context *on_finish,
-                   bool shutting_down = false) {
+                   Context *on_lock_unlock, Context *on_finish) {
     assert(!s_requests.empty());
     T* req = s_requests.front();
     req->on_lock_unlock = on_lock_unlock;
@@ -124,6 +121,11 @@ struct PostAcquireRequest<MockExclusiveLockImageCtx> : public BaseRequest<PostAc
 
 template <>
 struct PreReleaseRequest<MockExclusiveLockImageCtx> : public BaseRequest<PreReleaseRequest<MockExclusiveLockImageCtx> > {
+  static PreReleaseRequest<MockExclusiveLockImageCtx> *create(
+      MockExclusiveLockImageCtx &image_ctx, bool shutting_down,
+      Context *on_finish) {
+    return BaseRequest::create(image_ctx, nullptr, on_finish);
+  }
   MOCK_METHOD0(send, void());
 };