librbd: deadlock when replaying journal during image open

Fixes: http://tracker.ceph.com/issues/17188
Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/librbd/ExclusiveLock.cc b/src/librbd/ExclusiveLock.cc
index 5723fe91d10cbbb6338ba19b02be972dc50676fa..6b3013c582243188d40065f6b78196e665c5702c 100644 (file)
--- a/src/librbd/ExclusiveLock.cc
+++ b/src/librbd/ExclusiveLock.cc
@@ -416,12 +416,8 @@ void ExclusiveLock<I>::send_acquire_lock() {
     m_image_ctx, m_cookie,
     util::create_context_callback<el, &el::handle_acquiring_lock>(this),
     util::create_context_callback<el, &el::handle_acquire_lock>(this));
-
-  // acquire the lock if the image is not busy performing other actions
-  m_image_ctx.state->prepare_lock(new FunctionContext([this, req](int r) {
-      m_image_ctx.op_work_queue->queue(
-        new C_SendRequest<AcquireRequest<I> >(req), 0);
-    }));
+  m_image_ctx.op_work_queue->queue(new C_SendRequest<AcquireRequest<I> >(req),
+                                   0);
 }
 
 template <typename I>
@@ -450,7 +446,6 @@ void ExclusiveLock<I>::handle_acquire_lock(int r) {
     ldout(cct, 5) << "successfully acquired exclusive lock" << dendl;
   }
 
-  m_image_ctx.state->handle_prepare_lock_complete();
   {
     m_lock.Lock();
     assert(m_state == STATE_ACQUIRING ||
@@ -587,13 +582,10 @@ void ExclusiveLock<I>::send_release_lock() {
   ReleaseRequest<I>* req = ReleaseRequest<I>::create(
     m_image_ctx, m_cookie,
     util::create_context_callback<el, &el::handle_releasing_lock>(this),
-    util::create_context_callback<el, &el::handle_release_lock>(this));
-
-  // release the lock if the image is not busy performing other actions
-  m_image_ctx.state->prepare_lock(new FunctionContext([this, req](int r) {
-      m_image_ctx.op_work_queue->queue(
-        new C_SendRequest<ReleaseRequest<I> >(req), 0);
-    }));
+    util::create_context_callback<el, &el::handle_release_lock>(this),
+    false);
+  m_image_ctx.op_work_queue->queue(new C_SendRequest<ReleaseRequest<I> >(req),
+                                   0);
 }
 
 template <typename I>
@@ -610,8 +602,6 @@ void ExclusiveLock<I>::handle_releasing_lock(int r) {
 
 template <typename I>
 void ExclusiveLock<I>::handle_release_lock(int r) {
-  m_image_ctx.state->handle_prepare_lock_complete();
-
   bool lock_request_needed = false;
   {
     Mutex::Locker locker(m_lock);
@@ -670,7 +660,8 @@ void ExclusiveLock<I>::send_shutdown_release() {
   ReleaseRequest<I>* req = ReleaseRequest<I>::create(
     m_image_ctx, cookie,
     util::create_context_callback<el, &el::handle_shutdown_releasing>(this),
-    util::create_context_callback<el, &el::handle_shutdown_released>(this));
+    util::create_context_callback<el, &el::handle_shutdown_released>(this),
+    true);
   req->send();
 }
 

diff --git a/src/librbd/exclusive_lock/AcquireRequest.cc b/src/librbd/exclusive_lock/AcquireRequest.cc
index df354178b04261e269075ae71e4d999363ad12d6..3f8923ccba86bbf947dd086a18185cc25d3d6985 100644 (file)
--- a/src/librbd/exclusive_lock/AcquireRequest.cc
+++ b/src/librbd/exclusive_lock/AcquireRequest.cc
@@ -72,12 +72,35 @@ AcquireRequest<I>::AcquireRequest(I &image_ctx, const std::string &cookie,
 
 template <typename I>
 AcquireRequest<I>::~AcquireRequest() {
+  if (!m_prepare_lock_completed) {
+    m_image_ctx.state->handle_prepare_lock_complete();
+  }
   delete m_on_acquire;
 }
 
 template <typename I>
 void AcquireRequest<I>::send() {
+  send_prepare_lock();
+}
+
+template <typename I>
+void AcquireRequest<I>::send_prepare_lock() {
+  CephContext *cct = m_image_ctx.cct;
+  ldout(cct, 10) << __func__ << dendl;
+
+  // acquire the lock if the image is not busy performing other actions
+  Context *ctx = create_context_callback<
+    AcquireRequest<I>, &AcquireRequest<I>::handle_prepare_lock>(this);
+  m_image_ctx.state->prepare_lock(ctx);
+}
+
+template <typename I>
+Context *AcquireRequest<I>::handle_prepare_lock(int *ret_val) {
+  CephContext *cct = m_image_ctx.cct;
+  ldout(cct, 10) << __func__ << ": r=" << *ret_val << dendl;
+
   send_flush_notifies();
+  return nullptr;
 }
 
 template <typename I>
@@ -563,12 +586,17 @@ Context *AcquireRequest<I>::handle_break_lock(int *ret_val) {
 
 template <typename I>
 void AcquireRequest<I>::apply() {
-  RWLock::WLocker snap_locker(m_image_ctx.snap_lock);
-  assert(m_image_ctx.object_map == nullptr);
-  m_image_ctx.object_map = m_object_map;
+  {
+    RWLock::WLocker snap_locker(m_image_ctx.snap_lock);
+    assert(m_image_ctx.object_map == nullptr);
+    m_image_ctx.object_map = m_object_map;
+
+    assert(m_image_ctx.journal == nullptr);
+    m_image_ctx.journal = m_journal;
+  }
 
-  assert(m_image_ctx.journal == nullptr);
-  m_image_ctx.journal = m_journal;
+  m_prepare_lock_completed = true;
+  m_image_ctx.state->handle_prepare_lock_complete();
 }
 
 template <typename I>

diff --git a/src/librbd/exclusive_lock/AcquireRequest.h b/src/librbd/exclusive_lock/AcquireRequest.h
index 7b31d92f824fdcc48f9d2ba6374f1fb452e7c781..64f078b08c25cff72e5a3641f1dcb6ac11d7dbb0 100644 (file)
--- a/src/librbd/exclusive_lock/AcquireRequest.h
+++ b/src/librbd/exclusive_lock/AcquireRequest.h
@@ -35,6 +35,9 @@ private:
    * <start>
    *    |
    *    v
+   * PREPARE_LOCK
+   *    |
+   *    v
    * FLUSH_NOTIFIES
    *    |
    *    |     /-----------------------------------------------------------\
@@ -97,6 +100,10 @@ private:
   uint64_t m_locker_handle;
 
   int m_error_result;
+  bool m_prepare_lock_completed = false;
+
+  void send_prepare_lock();
+  Context *handle_prepare_lock(int *ret_val);
 
   void send_flush_notifies();
   Context *handle_flush_notifies(int *ret_val);

diff --git a/src/librbd/exclusive_lock/ReleaseRequest.cc b/src/librbd/exclusive_lock/ReleaseRequest.cc
index bed95171882be9f900dc29a06c37b4667bb2fb1e..52343f85dfed11d0c854be7075d3805d19947775 100644 (file)
--- a/src/librbd/exclusive_lock/ReleaseRequest.cc
+++ b/src/librbd/exclusive_lock/ReleaseRequest.cc
@@ -8,6 +8,7 @@
 #include "common/errno.h"
 #include "librbd/AioImageRequestWQ.h"
 #include "librbd/ExclusiveLock.h"
+#include "librbd/ImageState.h"
 #include "librbd/ImageWatcher.h"
 #include "librbd/Journal.h"
 #include "librbd/ObjectMap.h"
@@ -28,26 +29,57 @@ template <typename I>
 ReleaseRequest<I>* ReleaseRequest<I>::create(I &image_ctx,
                                              const std::string &cookie,
                                              Context *on_releasing,
-                                             Context *on_finish) {
-  return new ReleaseRequest(image_ctx, cookie, on_releasing, on_finish);
+                                             Context *on_finish,
+                                             bool shutting_down) {
+  return new ReleaseRequest(image_ctx, cookie, on_releasing, on_finish,
+                            shutting_down);
 }
 
 template <typename I>
 ReleaseRequest<I>::ReleaseRequest(I &image_ctx, const std::string &cookie,
-                                  Context *on_releasing, Context *on_finish)
+                                  Context *on_releasing, Context *on_finish,
+                                  bool shutting_down)
   : m_image_ctx(image_ctx), m_cookie(cookie), m_on_releasing(on_releasing),
     m_on_finish(create_async_context_callback(image_ctx, on_finish)),
-    m_object_map(nullptr), m_journal(nullptr) {
+    m_shutting_down(shutting_down), m_object_map(nullptr), m_journal(nullptr) {
 }
 
 template <typename I>
 ReleaseRequest<I>::~ReleaseRequest() {
+  if (!m_shutting_down) {
+    m_image_ctx.state->handle_prepare_lock_complete();
+  }
   delete m_on_releasing;
 }
 
 template <typename I>
 void ReleaseRequest<I>::send() {
+  send_prepare_lock();
+}
+
+template <typename I>
+void ReleaseRequest<I>::send_prepare_lock() {
+  if (m_shutting_down) {
+    send_cancel_op_requests();
+    return;
+  }
+
+  CephContext *cct = m_image_ctx.cct;
+  ldout(cct, 10) << __func__ << dendl;
+
+  // release the lock if the image is not busy performing other actions
+  Context *ctx = create_context_callback<
+    ReleaseRequest<I>, &ReleaseRequest<I>::handle_prepare_lock>(this);
+  m_image_ctx.state->prepare_lock(ctx);
+}
+
+template <typename I>
+Context *ReleaseRequest<I>::handle_prepare_lock(int *ret_val) {
+  CephContext *cct = m_image_ctx.cct;
+  ldout(cct, 10) << __func__ << ": r=" << *ret_val << dendl;
+
   send_cancel_op_requests();
+  return nullptr;
 }
 
 template <typename I>

diff --git a/src/librbd/exclusive_lock/ReleaseRequest.h b/src/librbd/exclusive_lock/ReleaseRequest.h
index a68530bbe231d0477e38fc01535cfe07cdd0f2c1..7c070ef69035202f90fc83cc4b8f7e6b3068139a 100644 (file)
--- a/src/librbd/exclusive_lock/ReleaseRequest.h
+++ b/src/librbd/exclusive_lock/ReleaseRequest.h
@@ -20,7 +20,8 @@ template <typename ImageCtxT = ImageCtx>
 class ReleaseRequest {
 public:
   static ReleaseRequest* create(ImageCtxT &image_ctx, const std::string &cookie,
-                                Context *on_releasing, Context *on_finish);
+                                Context *on_releasing, Context *on_finish,
+                                bool shutting_down);
 
   ~ReleaseRequest();
   void send();
@@ -32,6 +33,9 @@ private:
    * <start>
    *    |
    *    v
+   * PREPARE_LOCK
+   *    |
+   *    v
    * CANCEL_OP_REQUESTS
    *    |
    *    v
@@ -56,16 +60,21 @@ private:
    */
 
   ReleaseRequest(ImageCtxT &image_ctx, const std::string &cookie,
-                 Context *on_releasing, Context *on_finish);
+                 Context *on_releasing, Context *on_finish,
+                 bool shutting_down);
 
   ImageCtxT &m_image_ctx;
   std::string m_cookie;
   Context *m_on_releasing;
   Context *m_on_finish;
+  bool m_shutting_down;
 
   decltype(m_image_ctx.object_map) m_object_map;
   decltype(m_image_ctx.journal) m_journal;
 
+  void send_prepare_lock();
+  Context *handle_prepare_lock(int *ret_val);
+
   void send_cancel_op_requests();
   Context *handle_cancel_op_requests(int *ret_val);
 

diff --git a/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc b/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc
index 7037dbf8e8da3dac2fec400f81da447c58e84b5d..6840b2afad8db93e4d94ed1ed3fd1c9633fa39c5 100644 (file)
--- a/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc
+++ b/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc
@@ -68,6 +68,7 @@ namespace exclusive_lock {
 using ::testing::_;
 using ::testing::DoAll;
 using ::testing::InSequence;
+using ::testing::Invoke;
 using ::testing::Return;
 using ::testing::SetArgPointee;
 using ::testing::StrEq;
@@ -240,6 +241,18 @@ public:
     EXPECT_CALL(*mock_image_ctx.image_watcher, flush(_))
                   .WillOnce(CompleteContext(0, mock_image_ctx.image_ctx->op_work_queue));
   }
+
+  void expect_prepare_lock(MockTestImageCtx &mock_image_ctx) {
+    EXPECT_CALL(*mock_image_ctx.state, prepare_lock(_))
+      .WillOnce(Invoke([](Context *on_ready) {
+                  on_ready->complete(0);
+                }));
+  }
+
+  void expect_handle_prepare_lock_complete(MockTestImageCtx &mock_image_ctx) {
+    EXPECT_CALL(*mock_image_ctx.state, handle_prepare_lock_complete());
+  }
+
 };
 
 TEST_F(TestMockExclusiveLockAcquireRequest, Success) {
@@ -252,6 +265,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, Success) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_prepare_lock(mock_image_ctx);
   expect_flush_notifies(mock_image_ctx);
   expect_lock(mock_image_ctx, 0);
   expect_is_refresh_required(mock_image_ctx, false);
@@ -268,6 +282,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, Success) {
   expect_get_journal_policy(mock_image_ctx, mock_journal_policy);
   expect_journal_disabled(mock_journal_policy, false);
   expect_create_journal(mock_image_ctx, &mock_journal);
+  expect_handle_prepare_lock_complete(mock_image_ctx);
   expect_open_journal(mock_image_ctx, mock_journal, 0);
   expect_get_journal_policy(mock_image_ctx, mock_journal_policy);
   expect_allocate_journal_tag(mock_image_ctx, mock_journal_policy, 0);
@@ -293,6 +308,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, SuccessRefresh) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_prepare_lock(mock_image_ctx);
   expect_flush_notifies(mock_image_ctx);
   expect_lock(mock_image_ctx, 0);
   expect_is_refresh_required(mock_image_ctx, true);
@@ -302,6 +318,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, SuccessRefresh) {
   expect_test_features(mock_image_ctx, RBD_FEATURE_OBJECT_MAP, false);
   expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING,
                        mock_image_ctx.snap_lock, false);
+  expect_handle_prepare_lock_complete(mock_image_ctx);
 
   C_SaferCond acquire_ctx;
   C_SaferCond ctx;
@@ -323,6 +340,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, SuccessJournalDisabled) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_prepare_lock(mock_image_ctx);
   expect_flush_notifies(mock_image_ctx);
   expect_lock(mock_image_ctx, 0);
   expect_is_refresh_required(mock_image_ctx, false);
@@ -334,6 +352,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, SuccessJournalDisabled) {
 
   expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING,
                        mock_image_ctx.snap_lock, false);
+  expect_handle_prepare_lock_complete(mock_image_ctx);
 
   C_SaferCond acquire_ctx;
   C_SaferCond ctx;
@@ -355,6 +374,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, SuccessObjectMapDisabled) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_prepare_lock(mock_image_ctx);
   expect_flush_notifies(mock_image_ctx);
   expect_lock(mock_image_ctx, 0);
   expect_is_refresh_required(mock_image_ctx, false);
@@ -368,6 +388,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, SuccessObjectMapDisabled) {
   expect_get_journal_policy(mock_image_ctx, mock_journal_policy);
   expect_journal_disabled(mock_journal_policy, false);
   expect_create_journal(mock_image_ctx, &mock_journal);
+  expect_handle_prepare_lock_complete(mock_image_ctx);
   expect_open_journal(mock_image_ctx, mock_journal, 0);
   expect_get_journal_policy(mock_image_ctx, mock_journal_policy);
   expect_allocate_journal_tag(mock_image_ctx, mock_journal_policy, 0);
@@ -393,11 +414,13 @@ TEST_F(TestMockExclusiveLockAcquireRequest, RefreshError) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_prepare_lock(mock_image_ctx);
   expect_flush_notifies(mock_image_ctx);
   expect_lock(mock_image_ctx, 0);
   expect_is_refresh_required(mock_image_ctx, true);
   expect_refresh(mock_image_ctx, mock_refresh_request, -EINVAL);
   expect_unlock(mock_image_ctx, 0);
+  expect_handle_prepare_lock_complete(mock_image_ctx);
 
   C_SaferCond *acquire_ctx = new C_SaferCond();
   C_SaferCond ctx;
@@ -418,6 +441,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, JournalError) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_prepare_lock(mock_image_ctx);
   expect_flush_notifies(mock_image_ctx);
   expect_lock(mock_image_ctx, 0);
   expect_is_refresh_required(mock_image_ctx, false);
@@ -434,6 +458,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, JournalError) {
   expect_get_journal_policy(mock_image_ctx, mock_journal_policy);
   expect_journal_disabled(mock_journal_policy, false);
   expect_create_journal(mock_image_ctx, mock_journal);
+  expect_handle_prepare_lock_complete(mock_image_ctx);
   expect_open_journal(mock_image_ctx, *mock_journal, -EINVAL);
   expect_close_journal(mock_image_ctx, *mock_journal);
   expect_close_object_map(mock_image_ctx, *mock_object_map);
@@ -458,6 +483,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, AllocateJournalTagError) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_prepare_lock(mock_image_ctx);
   expect_flush_notifies(mock_image_ctx);
   expect_lock(mock_image_ctx, 0);
   expect_is_refresh_required(mock_image_ctx, false);
@@ -474,6 +500,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, AllocateJournalTagError) {
   expect_get_journal_policy(mock_image_ctx, mock_journal_policy);
   expect_journal_disabled(mock_journal_policy, false);
   expect_create_journal(mock_image_ctx, mock_journal);
+  expect_handle_prepare_lock_complete(mock_image_ctx);
   expect_open_journal(mock_image_ctx, *mock_journal, 0);
   expect_get_journal_policy(mock_image_ctx, mock_journal_policy);
   expect_allocate_journal_tag(mock_image_ctx, mock_journal_policy, -EPERM);
@@ -500,6 +527,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, LockBusy) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_prepare_lock(mock_image_ctx);
   expect_flush_notifies(mock_image_ctx);
   expect_lock(mock_image_ctx, -EBUSY);
   expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4",
@@ -509,6 +537,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, LockBusy) {
   expect_blacklist_add(mock_image_ctx, 0);
   expect_break_lock(mock_image_ctx, 0);
   expect_lock(mock_image_ctx, -ENOENT);
+  expect_handle_prepare_lock_complete(mock_image_ctx);
 
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
@@ -528,10 +557,12 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoError) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_prepare_lock(mock_image_ctx);
   expect_flush_notifies(mock_image_ctx);
   expect_lock(mock_image_ctx, -EBUSY);
   expect_get_lock_info(mock_image_ctx, -EINVAL, entity_name_t::CLIENT(1), "",
                        "", "", LOCK_EXCLUSIVE);
+  expect_handle_prepare_lock_complete(mock_image_ctx);
 
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
@@ -551,11 +582,13 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoEmpty) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_prepare_lock(mock_image_ctx);
   expect_flush_notifies(mock_image_ctx);
   expect_lock(mock_image_ctx, -EBUSY);
   expect_get_lock_info(mock_image_ctx, -ENOENT, entity_name_t::CLIENT(1), "",
                        "", "", LOCK_EXCLUSIVE);
   expect_lock(mock_image_ctx, -EINVAL);
+  expect_handle_prepare_lock_complete(mock_image_ctx);
 
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
@@ -575,10 +608,12 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoExternalTag) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_prepare_lock(mock_image_ctx);
   expect_flush_notifies(mock_image_ctx);
   expect_lock(mock_image_ctx, -EBUSY);
   expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4",
                        "auto 123", "external tag", LOCK_EXCLUSIVE);
+  expect_handle_prepare_lock_complete(mock_image_ctx);
 
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
@@ -598,11 +633,13 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoShared) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_prepare_lock(mock_image_ctx);
   expect_flush_notifies(mock_image_ctx);
   expect_lock(mock_image_ctx, -EBUSY);
   expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4",
                        "auto 123", ExclusiveLock<>::WATCHER_LOCK_TAG,
                        LOCK_SHARED);
+  expect_handle_prepare_lock_complete(mock_image_ctx);
 
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
@@ -622,11 +659,13 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoExternalCookie) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_prepare_lock(mock_image_ctx);
   expect_flush_notifies(mock_image_ctx);
   expect_lock(mock_image_ctx, -EBUSY);
   expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4",
                        "external cookie", ExclusiveLock<>::WATCHER_LOCK_TAG,
                        LOCK_EXCLUSIVE);
+  expect_handle_prepare_lock_complete(mock_image_ctx);
 
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
@@ -646,12 +685,14 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetWatchersError) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_prepare_lock(mock_image_ctx);
   expect_flush_notifies(mock_image_ctx);
   expect_lock(mock_image_ctx, -EBUSY);
   expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4",
                        "auto 123", ExclusiveLock<>::WATCHER_LOCK_TAG,
                        LOCK_EXCLUSIVE);
   expect_list_watchers(mock_image_ctx, -EINVAL, "dead client", 123);
+  expect_handle_prepare_lock_complete(mock_image_ctx);
 
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
@@ -671,12 +712,14 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetWatchersAlive) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_prepare_lock(mock_image_ctx);
   expect_flush_notifies(mock_image_ctx);
   expect_lock(mock_image_ctx, -EBUSY);
   expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4",
                        "auto 123", ExclusiveLock<>::WATCHER_LOCK_TAG,
                        LOCK_EXCLUSIVE);
   expect_list_watchers(mock_image_ctx, 0, "1.2.3.4", 123);
+  expect_handle_prepare_lock_complete(mock_image_ctx);
 
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
@@ -697,6 +740,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BlacklistDisabled) {
   mock_image_ctx.blacklist_on_break_lock = false;
 
   InSequence seq;
+  expect_prepare_lock(mock_image_ctx);
   expect_flush_notifies(mock_image_ctx);
   expect_lock(mock_image_ctx, -EBUSY);
   expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4",
@@ -705,6 +749,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BlacklistDisabled) {
   expect_list_watchers(mock_image_ctx, 0, "dead client", 123);
   expect_break_lock(mock_image_ctx, 0);
   expect_lock(mock_image_ctx, -ENOENT);
+  expect_handle_prepare_lock_complete(mock_image_ctx);
 
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
@@ -724,6 +769,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BlacklistError) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_prepare_lock(mock_image_ctx);
   expect_flush_notifies(mock_image_ctx);
   expect_lock(mock_image_ctx, -EBUSY);
   expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4",
@@ -731,6 +777,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BlacklistError) {
                        LOCK_EXCLUSIVE);
   expect_list_watchers(mock_image_ctx, 0, "dead client", 123);
   expect_blacklist_add(mock_image_ctx, -EINVAL);
+  expect_handle_prepare_lock_complete(mock_image_ctx);
 
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
@@ -750,6 +797,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BreakLockMissing) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_prepare_lock(mock_image_ctx);
   expect_flush_notifies(mock_image_ctx);
   expect_lock(mock_image_ctx, -EBUSY);
   expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4",
@@ -759,6 +807,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BreakLockMissing) {
   expect_blacklist_add(mock_image_ctx, 0);
   expect_break_lock(mock_image_ctx, -ENOENT);
   expect_lock(mock_image_ctx, -EINVAL);
+  expect_handle_prepare_lock_complete(mock_image_ctx);
 
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
@@ -778,6 +827,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BreakLockError) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_prepare_lock(mock_image_ctx);
   expect_flush_notifies(mock_image_ctx);
   expect_lock(mock_image_ctx, -EBUSY);
   expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4",
@@ -786,6 +836,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BreakLockError) {
   expect_list_watchers(mock_image_ctx, 0, "dead client", 123);
   expect_blacklist_add(mock_image_ctx, 0);
   expect_break_lock(mock_image_ctx, -EINVAL);
+  expect_handle_prepare_lock_complete(mock_image_ctx);
 
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
@@ -805,6 +856,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, OpenObjectMapError) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_prepare_lock(mock_image_ctx);
   expect_flush_notifies(mock_image_ctx);
   expect_lock(mock_image_ctx, 0);
   expect_is_refresh_required(mock_image_ctx, false);
@@ -821,6 +873,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, OpenObjectMapError) {
   expect_get_journal_policy(mock_image_ctx, mock_journal_policy);
   expect_journal_disabled(mock_journal_policy, false);
   expect_create_journal(mock_image_ctx, &mock_journal);
+  expect_handle_prepare_lock_complete(mock_image_ctx);
   expect_open_journal(mock_image_ctx, mock_journal, 0);
   expect_get_journal_policy(mock_image_ctx, mock_journal_policy);
   expect_allocate_journal_tag(mock_image_ctx, mock_journal_policy, 0);

diff --git a/src/test/librbd/exclusive_lock/test_mock_ReleaseRequest.cc b/src/test/librbd/exclusive_lock/test_mock_ReleaseRequest.cc
index 99ae094045bd0fae85fc28735f23f4c15910c66d..e2fb8420e2401631a78200d8808d91f8022f58c4 100644 (file)
--- a/src/test/librbd/exclusive_lock/test_mock_ReleaseRequest.cc
+++ b/src/test/librbd/exclusive_lock/test_mock_ReleaseRequest.cc
@@ -30,6 +30,7 @@ struct MockContext : public Context {
 
 using ::testing::_;
 using ::testing::InSequence;
+using ::testing::Invoke;
 using ::testing::Return;
 using ::testing::StrEq;
 
@@ -94,6 +95,18 @@ public:
     EXPECT_CALL(*mock_image_ctx.image_watcher, flush(_))
                   .WillOnce(CompleteContext(0, mock_image_ctx.image_ctx->op_work_queue));
   }
+
+  void expect_prepare_lock(MockImageCtx &mock_image_ctx) {
+    EXPECT_CALL(*mock_image_ctx.state, prepare_lock(_))
+      .WillOnce(Invoke([](Context *on_ready) {
+                  on_ready->complete(0);
+                }));
+  }
+
+  void expect_handle_prepare_lock_complete(MockImageCtx &mock_image_ctx) {
+    EXPECT_CALL(*mock_image_ctx.state, handle_prepare_lock_complete());
+  }
+
 };
 
 TEST_F(TestMockExclusiveLockReleaseRequest, Success) {
@@ -106,6 +119,7 @@ TEST_F(TestMockExclusiveLockReleaseRequest, Success) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_prepare_lock(mock_image_ctx);
   expect_cancel_op_requests(mock_image_ctx, 0);
   expect_block_writes(mock_image_ctx, 0);
   expect_flush_notifies(mock_image_ctx);
@@ -121,12 +135,13 @@ TEST_F(TestMockExclusiveLockReleaseRequest, Success) {
   MockContext mock_releasing_ctx;
   expect_complete_context(mock_releasing_ctx, 0);
   expect_unlock(mock_image_ctx, 0);
+  expect_handle_prepare_lock_complete(mock_image_ctx);
 
   C_SaferCond ctx;
   MockReleaseRequest *req = MockReleaseRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
                                                        &mock_releasing_ctx,
-                                                       &ctx);
+                                                       &ctx, false);
   req->send();
   ASSERT_EQ(0, ctx.wait());
 }
@@ -142,6 +157,7 @@ TEST_F(TestMockExclusiveLockReleaseRequest, SuccessJournalDisabled) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_prepare_lock(mock_image_ctx);
   expect_cancel_op_requests(mock_image_ctx, 0);
   expect_flush_notifies(mock_image_ctx);
 
@@ -150,12 +166,14 @@ TEST_F(TestMockExclusiveLockReleaseRequest, SuccessJournalDisabled) {
   expect_close_object_map(mock_image_ctx, *mock_object_map);
 
   expect_unlock(mock_image_ctx, 0);
+  expect_handle_prepare_lock_complete(mock_image_ctx);
 
   C_SaferCond release_ctx;
   C_SaferCond ctx;
   MockReleaseRequest *req = MockReleaseRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       &release_ctx, &ctx);
+                                                       &release_ctx, &ctx,
+                                                       false);
   req->send();
   ASSERT_EQ(0, release_ctx.wait());
   ASSERT_EQ(0, ctx.wait());
@@ -181,7 +199,8 @@ TEST_F(TestMockExclusiveLockReleaseRequest, SuccessObjectMapDisabled) {
   C_SaferCond ctx;
   MockReleaseRequest *req = MockReleaseRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       &release_ctx, &ctx);
+                                                       &release_ctx, &ctx,
+                                                       true);
   req->send();
   ASSERT_EQ(0, release_ctx.wait());
   ASSERT_EQ(0, ctx.wait());
@@ -204,7 +223,8 @@ TEST_F(TestMockExclusiveLockReleaseRequest, BlockWritesError) {
   C_SaferCond ctx;
   MockReleaseRequest *req = MockReleaseRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       nullptr, &ctx);
+                                                       nullptr, &ctx,
+                                                       true);
   req->send();
   ASSERT_EQ(-EINVAL, ctx.wait());
 }
@@ -228,7 +248,8 @@ TEST_F(TestMockExclusiveLockReleaseRequest, UnlockError) {
   C_SaferCond ctx;
   MockReleaseRequest *req = MockReleaseRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       nullptr, &ctx);
+                                                       nullptr, &ctx,
+                                                       true);
   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 821f857b769b94c8886de49edb5e80535a05681a..5d6eb17458eda2bba1b156ea4704b7cea6f88045 100644 (file)
--- a/src/test/librbd/test_mock_ExclusiveLock.cc
+++ b/src/test/librbd/test_mock_ExclusiveLock.cc
@@ -4,7 +4,6 @@
 #include "test/librbd/test_mock_fixture.h"
 #include "test/librbd/test_support.h"
 #include "test/librbd/mock/MockImageCtx.h"
-#include "test/librbd/mock/MockImageState.h"
 #include "librbd/ExclusiveLock.h"
 #include "librbd/exclusive_lock/AcquireRequest.h"
 #include "librbd/exclusive_lock/ReacquireRequest.h"
@@ -33,7 +32,8 @@ struct BaseRequest {
   Context *on_finish = nullptr;
 
   static T* create(MockExclusiveLockImageCtx &image_ctx, const std::string &cookie,
-                   Context *on_lock_unlock, Context *on_finish) {
+                   Context *on_lock_unlock, Context *on_finish,
+                   bool shutting_down = false) {
     assert(!s_requests.empty());
     T* req = s_requests.front();
     req->on_lock_unlock = on_lock_unlock;
@@ -130,11 +130,9 @@ public:
   void expect_acquire_lock(MockExclusiveLockImageCtx &mock_image_ctx,
                            MockAcquireRequest &acquire_request, int r) {
     expect_get_watch_handle(mock_image_ctx);
-    expect_prepare_lock(mock_image_ctx);
     EXPECT_CALL(acquire_request, send())
                   .WillOnce(DoAll(FinishLockUnlock(&acquire_request),
                                   FinishRequest(&acquire_request, r, &mock_image_ctx)));
-    expect_handle_prepare_lock_complete(mock_image_ctx);
     if (r == 0) {
       expect_notify_acquired_lock(mock_image_ctx);
       expect_unblock_writes(mock_image_ctx);
@@ -144,15 +142,9 @@ public:
   void expect_release_lock(MockExclusiveLockImageCtx &mock_image_ctx,
                            MockReleaseRequest &release_request, int r,
                            bool shutting_down = false) {
-    if (!shutting_down) {
-      expect_prepare_lock(mock_image_ctx);
-    }
     EXPECT_CALL(release_request, send())
                   .WillOnce(DoAll(FinishLockUnlock(&release_request),
                                   FinishRequest(&release_request, r, &mock_image_ctx)));
-    if (!shutting_down) {
-      expect_handle_prepare_lock_complete(mock_image_ctx);
-    }
     if (r == 0) {
       if (shutting_down) {
         expect_unblock_writes(mock_image_ctx);
@@ -197,17 +189,6 @@ public:
                   .WillOnce(CompleteContext(0, mock_image_ctx.image_ctx->op_work_queue));
   }
 
-  void expect_prepare_lock(MockExclusiveLockImageCtx &mock_image_ctx) {
-    EXPECT_CALL(*mock_image_ctx.state, prepare_lock(_))
-      .WillOnce(Invoke([](Context *on_ready) {
-                  on_ready->complete(0);
-                }));
-  }
-
-  void expect_handle_prepare_lock_complete(MockExclusiveLockImageCtx &mock_image_ctx) {
-    EXPECT_CALL(*mock_image_ctx.state, handle_prepare_lock_complete());
-  }
-
   int when_init(MockExclusiveLockImageCtx &mock_image_ctx,
                 MockExclusiveLock &exclusive_lock) {
     C_SaferCond ctx;
@@ -571,20 +552,16 @@ TEST_F(TestMockExclusiveLock, ConcurrentRequests) {
   MockAcquireRequest try_lock_acquire;
   C_SaferCond wait_for_send_ctx1;
   expect_get_watch_handle(mock_image_ctx);
-  expect_prepare_lock(mock_image_ctx);
   EXPECT_CALL(try_lock_acquire, send())
                 .WillOnce(Notify(&wait_for_send_ctx1));
-  expect_handle_prepare_lock_complete(mock_image_ctx);
 
   MockAcquireRequest request_acquire;
   expect_acquire_lock(mock_image_ctx, request_acquire, 0);
 
   MockReleaseRequest release;
   C_SaferCond wait_for_send_ctx2;
-  expect_prepare_lock(mock_image_ctx);
   EXPECT_CALL(release, send())
                 .WillOnce(Notify(&wait_for_send_ctx2));
-  expect_handle_prepare_lock_complete(mock_image_ctx);
   expect_notify_released_lock(mock_image_ctx);
   expect_is_lock_request_needed(mock_image_ctx, false);