librbd: add additional granularity to lock states

Allow all pre-release actions to see that the exclusive lock is
still owned and all post-acquire actions to see that the exclusive
lock is now owned.

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

diff --git a/src/librbd/ExclusiveLock.cc b/src/librbd/ExclusiveLock.cc
index 92e7ef7136ea05dfd433bd3646b6602499c6a310..fd362ef4a98c7837635fec7af8a82eed07819fba 100644 (file)
--- a/src/librbd/ExclusiveLock.cc
+++ b/src/librbd/ExclusiveLock.cc
@@ -25,6 +25,16 @@ namespace {
 
 const std::string WATCHER_LOCK_COOKIE_PREFIX = "auto";
 
+template <typename I>
+struct C_SendReleaseRequest : public Context {
+  ReleaseRequest<I>* request;
+  C_SendReleaseRequest(ReleaseRequest<I>* request) : request(request) {
+  }
+  virtual void finish(int r) override {
+    request->send();
+  }
+};
+
 } // anonymous namespace
 
 template <typename I>
@@ -319,6 +329,7 @@ void ExclusiveLock<I>::send_acquire_lock() {
   using el = ExclusiveLock<I>;
   AcquireRequest<I>* req = AcquireRequest<I>::create(
     m_image_ctx, encode_lock_cookie(),
+    util::create_context_callback<el, &el::handle_acquiring_lock>(this),
     util::create_context_callback<el, &el::handle_acquire_lock>(this));
 
   m_lock.Unlock();
@@ -326,6 +337,18 @@ void ExclusiveLock<I>::send_acquire_lock() {
   m_lock.Lock();
 }
 
+template <typename I>
+void ExclusiveLock<I>::handle_acquiring_lock(int r) {
+  CephContext *cct = m_image_ctx.cct;
+  ldout(cct, 10) << this << " " << __func__ << dendl;
+
+  assert(r == 0);
+  assert(m_state == STATE_ACQUIRING);
+
+  // lock is owned at this point
+  m_state = STATE_POST_ACQUIRING;
+}
+
 template <typename I>
 void ExclusiveLock<I>::handle_acquire_lock(int r) {
   CephContext *cct = m_image_ctx.cct;
@@ -342,6 +365,9 @@ void ExclusiveLock<I>::handle_acquire_lock(int r) {
 
   {
     m_lock.Lock();
+    assert(m_state == STATE_ACQUIRING ||
+           m_state == STATE_POST_ACQUIRING);
+
     Action action = get_active_action();
     assert(action == ACTION_TRY_LOCK || action == ACTION_REQUEST_LOCK);
     if (action == ACTION_REQUEST_LOCK && r < 0 && r != -EBLACKLISTED) {
@@ -361,10 +387,6 @@ void ExclusiveLock<I>::handle_acquire_lock(int r) {
   }
 
   if (next_state == STATE_LOCKED) {
-    {
-      Mutex::Locker locker(m_lock);
-      m_state = STATE_POST_ACQUIRING;
-    }
     m_image_ctx.image_watcher->notify_acquired_lock();
     m_image_ctx.aio_work_queue->unblock_writes();
   }
@@ -384,31 +406,26 @@ void ExclusiveLock<I>::send_release_lock() {
   ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl;
   m_state = STATE_PRE_RELEASING;
 
-  m_image_ctx.op_work_queue->queue(
-    new C_BlockWrites(m_image_ctx, new C_ReleaseBlockWrites(this)), 0);
+  using el = ExclusiveLock<I>;
+  ReleaseRequest<I>* req = ReleaseRequest<I>::create(
+    m_image_ctx, encode_lock_cookie(),
+    util::create_context_callback<el, &el::handle_releasing_lock>(this),
+    util::create_context_callback<el, &el::handle_release_lock>(this));
+
+  // send in alternate thread context to avoid re-entrant locking
+  m_image_ctx.op_work_queue->queue(new C_SendReleaseRequest<I>(req), 0);
 }
 
 template <typename I>
-void ExclusiveLock<I>::handle_release_blocked_writes(int r) {
-  if (r < 0) {
-    handle_release_lock(r);
-    return;
-  }
-
-  ldout(m_image_ctx.cct, 10) << this << " " << __func__ << ": r=" << r << dendl;
-
+void ExclusiveLock<I>::handle_releasing_lock(int r) {
   Mutex::Locker locker(m_lock);
-  assert(m_state == STATE_PRE_RELEASING);
-  m_state = STATE_RELEASING;
+  ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl;
 
-  using el = ExclusiveLock<I>;
-  ReleaseRequest<I>* req = ReleaseRequest<I>::create(
-    m_image_ctx, encode_lock_cookie(),
-    util::create_context_callback<el, &el::handle_release_lock>(this));
+  assert(r == 0);
+  assert(m_state == STATE_PRE_RELEASING);
 
-  m_lock.Unlock();
-  req->send();
-  m_lock.Lock();
+  // all IO and ops should be blocked/canceled by this point
+  m_state = STATE_RELEASING;
 }
 
 template <typename I>
@@ -419,10 +436,9 @@ void ExclusiveLock<I>::handle_release_lock(int r) {
     ldout(m_image_ctx.cct, 10) << this << " " << __func__ << ": r=" << r
                                << dendl;
 
-    assert(m_state == STATE_RELEASING);
-    if (r < 0) {
-      m_image_ctx.aio_work_queue->unblock_writes();
-    } else {
+    assert(m_state == STATE_PRE_RELEASING ||
+           m_state == STATE_RELEASING);
+    if (r >= 0) {
       m_lock.Unlock();
       m_image_ctx.image_watcher->notify_released_lock();
       pending_writes = !m_image_ctx.aio_work_queue->writes_empty();
@@ -470,7 +486,7 @@ void ExclusiveLock<I>::send_shutdown_release() {
 
   using el = ExclusiveLock<I>;
   ReleaseRequest<I>* req = ReleaseRequest<I>::create(
-    m_image_ctx, cookie,
+    m_image_ctx, cookie, nullptr,
     util::create_context_callback<el, &el::handle_shutdown>(this));
   req->send();
 }
@@ -483,6 +499,8 @@ void ExclusiveLock<I>::handle_shutdown(int r) {
   if (r < 0) {
     lderr(cct) << "failed to shut down exclusive lock: " << cpp_strerror(r)
                << dendl;
+  } else {
+    m_image_ctx.aio_work_queue->unblock_writes();
   }
 
   m_image_ctx.image_watcher->notify_released_lock();

diff --git a/src/librbd/ExclusiveLock.h b/src/librbd/ExclusiveLock.h
index af107c25f7dc8befb38db90cf07e17268cbc34ff..f29ae59717393969c8d507c0fed1ea32883566b7 100644 (file)
--- a/src/librbd/ExclusiveLock.h
+++ b/src/librbd/ExclusiveLock.h
@@ -108,28 +108,6 @@ private:
     }
   };
 
-  struct C_BlockWrites : public Context {
-    ImageCtxT &image_ctx;
-    Context *on_finish;
-    C_BlockWrites(ImageCtxT &image_ctx, Context *on_finish)
-      : image_ctx(image_ctx), on_finish(on_finish) {
-    }
-    virtual void finish(int r) override {
-      RWLock::RLocker owner_locker(image_ctx.owner_lock);
-      image_ctx.aio_work_queue->block_writes(on_finish);
-    }
-  };
-
-  struct C_ReleaseBlockWrites : public Context {
-    ExclusiveLock *exclusive_lock;
-    C_ReleaseBlockWrites(ExclusiveLock *exclusive_lock)
-      : exclusive_lock(exclusive_lock) {
-    }
-    virtual void finish(int r) override {
-      exclusive_lock->handle_release_blocked_writes(r);
-    }
-  };
-
   struct C_ShutDownRelease : public Context {
     ExclusiveLock *exclusive_lock;
     C_ShutDownRelease(ExclusiveLock *exclusive_lock)
@@ -164,10 +142,11 @@ private:
   void handle_init_complete();
 
   void send_acquire_lock();
+  void handle_acquiring_lock(int r);
   void handle_acquire_lock(int r);
 
   void send_release_lock();
-  void handle_release_blocked_writes(int r);
+  void handle_releasing_lock(int r);
   void handle_release_lock(int r);
 
   void send_shutdown();

diff --git a/src/librbd/exclusive_lock/AcquireRequest.cc b/src/librbd/exclusive_lock/AcquireRequest.cc
index f0826b1252c2eabcbc34e652d6e1ef978e8abbc4..ad59c046e26f0e8ecd935d3646d2c505fceba2b7 100644 (file)
--- a/src/librbd/exclusive_lock/AcquireRequest.cc
+++ b/src/librbd/exclusive_lock/AcquireRequest.cc
@@ -53,18 +53,24 @@ struct C_BlacklistClient : public Context {
 template <typename I>
 AcquireRequest<I>* AcquireRequest<I>::create(I &image_ctx,
                                              const std::string &cookie,
+                                             Context *on_acquire,
                                              Context *on_finish) {
-  return new AcquireRequest(image_ctx, cookie, on_finish);
+  return new AcquireRequest(image_ctx, cookie, on_acquire, on_finish);
 }
 
 template <typename I>
 AcquireRequest<I>::AcquireRequest(I &image_ctx, const std::string &cookie,
-                                  Context *on_finish)
-  : m_image_ctx(image_ctx), m_cookie(cookie),
+                                  Context *on_acquire, Context *on_finish)
+  : m_image_ctx(image_ctx), m_cookie(cookie), m_on_acquire(on_acquire),
     m_on_finish(create_async_context_callback(image_ctx, on_finish)),
     m_object_map(nullptr), m_journal(nullptr), m_error_result(0) {
 }
 
+template <typename I>
+AcquireRequest<I>::~AcquireRequest() {
+  delete m_on_acquire;
+}
+
 template <typename I>
 void AcquireRequest<I>::send() {
   send_lock();
@@ -106,6 +112,10 @@ Context *AcquireRequest<I>::handle_lock(int *ret_val) {
 
 template <typename I>
 Context *AcquireRequest<I>::send_open_journal() {
+  // alert caller that we now own the exclusive lock
+  m_on_acquire->complete(0);
+  m_on_acquire = nullptr;
+
   if (!m_image_ctx.test_features(RBD_FEATURE_JOURNALING)) {
     apply();
     return m_on_finish;

diff --git a/src/librbd/exclusive_lock/AcquireRequest.h b/src/librbd/exclusive_lock/AcquireRequest.h
index e3062156e59f0fbc617ef5db11d0c98c415430ba..865b3c66bd49fed6bdbbcfb0427d41e1505715f2 100644 (file)
--- a/src/librbd/exclusive_lock/AcquireRequest.h
+++ b/src/librbd/exclusive_lock/AcquireRequest.h
@@ -24,8 +24,9 @@ template <typename ImageCtxT = ImageCtx>
 class AcquireRequest {
 public:
   static AcquireRequest* create(ImageCtxT &image_ctx, const std::string &cookie,
-                                Context *on_finish);
+                                Context *on_acquire, Context *on_finish);
 
+  ~AcquireRequest();
   void send();
 
 private:
@@ -61,10 +62,11 @@ private:
    */
 
   AcquireRequest(ImageCtxT &image_ctx, const std::string &cookie,
-                 Context *on_finish);
+                 Context *on_acquire, Context *on_finish);
 
   ImageCtxT &m_image_ctx;
   std::string m_cookie;
+  Context *m_on_acquire;
   Context *m_on_finish;
 
   bufferlist m_out_bl;

diff --git a/src/librbd/exclusive_lock/ReleaseRequest.cc b/src/librbd/exclusive_lock/ReleaseRequest.cc
index fbf325490fed221c0db485fce05b5021e84d9e01..3b65199a314348db33fb28dfa98e958bcff5a996 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 "common/WorkQueue.h"
 #include "include/stringify.h"
+#include "librbd/AioImageRequestWQ.h"
 #include "librbd/ExclusiveLock.h"
 #include "librbd/ImageCtx.h"
 #include "librbd/Journal.h"
@@ -28,21 +29,56 @@ using util::create_rados_safe_callback;
 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_finish);
+  return new ReleaseRequest(image_ctx, cookie, on_releasing, on_finish);
 }
 
 template <typename I>
 ReleaseRequest<I>::ReleaseRequest(I &image_ctx, const std::string &cookie,
-                                  Context *on_finish)
-  : m_image_ctx(image_ctx), m_cookie(cookie),
+                                  Context *on_releasing, Context *on_finish)
+  : 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) {
 }
 
+template <typename I>
+ReleaseRequest<I>::~ReleaseRequest() {
+  delete m_on_releasing;
+}
+
 template <typename I>
 void ReleaseRequest<I>::send() {
+  send_block_writes();
+}
+
+template <typename I>
+void ReleaseRequest<I>::send_block_writes() {
+  CephContext *cct = m_image_ctx.cct;
+  ldout(cct, 10) << __func__ << dendl;
+
+  using klass = ReleaseRequest<I>;
+  Context *ctx = create_context_callback<
+    klass, &klass::handle_block_writes>(this);
+
+  {
+    RWLock::RLocker owner_locker(m_image_ctx.owner_lock);
+    m_image_ctx.aio_work_queue->block_writes(ctx);
+  }
+}
+
+template <typename I>
+Context *ReleaseRequest<I>::handle_block_writes(int *ret_val) {
+  CephContext *cct = m_image_ctx.cct;
+  ldout(cct, 10) << __func__ << ": r=" << *ret_val << dendl;
+
+  if (*ret_val < 0) {
+    m_image_ctx.aio_work_queue->unblock_writes();
+    return m_on_finish;
+  }
+
   send_cancel_op_requests();
+  return nullptr;
 }
 
 template <typename I>
@@ -51,8 +87,8 @@ void ReleaseRequest<I>::send_cancel_op_requests() {
   ldout(cct, 10) << __func__ << dendl;
 
   using klass = ReleaseRequest<I>;
-  Context *ctx = create_context_callback<klass,
-                                         &klass::handle_cancel_op_requests>(this);
+  Context *ctx = create_context_callback<
+    klass, &klass::handle_cancel_op_requests>(this);
   m_image_ctx.cancel_async_requests(ctx);
 }
 
@@ -62,6 +98,13 @@ Context *ReleaseRequest<I>::handle_cancel_op_requests(int *ret_val) {
   ldout(cct, 10) << __func__ << ": r=" << *ret_val << dendl;
 
   assert(*ret_val == 0);
+
+  if (m_on_releasing != nullptr) {
+    // alert caller that we no longer own the exclusive lock
+    m_on_releasing->complete(0);
+    m_on_releasing = nullptr;
+  }
+
   send_close_journal();
   return nullptr;
 }

diff --git a/src/librbd/exclusive_lock/ReleaseRequest.h b/src/librbd/exclusive_lock/ReleaseRequest.h
index a7946f52a7a5d3bd9fda00eca8780836f5e48fff..b23e6c7d879700e9bf7bf37c8343e2a742ac9d90 100644 (file)
--- a/src/librbd/exclusive_lock/ReleaseRequest.h
+++ b/src/librbd/exclusive_lock/ReleaseRequest.h
@@ -21,8 +21,9 @@ template <typename ImageCtxT = ImageCtx>
 class ReleaseRequest {
 public:
   static ReleaseRequest* create(ImageCtxT &image_ctx, const std::string &cookie,
-                                Context *on_finish);
+                                Context *on_releasing, Context *on_finish);
 
+  ~ReleaseRequest();
   void send();
 
 private:
@@ -32,6 +33,9 @@ private:
    * <start>
    *    |
    *    v
+   * BLOCK_WRITES
+   *    |
+   *    v
    * CANCEL_OP_REQUESTS . . . . . . . . . . . .
    *    |                                     .
    *    v                                     .
@@ -50,15 +54,19 @@ private:
    */
 
   ReleaseRequest(ImageCtxT &image_ctx, const std::string &cookie,
-                 Context *on_finish);
+                 Context *on_releasing, Context *on_finish);
 
   ImageCtxT &m_image_ctx;
   std::string m_cookie;
+  Context *m_on_releasing;
   Context *m_on_finish;
 
   decltype(m_image_ctx.object_map) m_object_map;
   decltype(m_image_ctx.journal) m_journal;
 
+  void send_block_writes();
+  Context *handle_block_writes(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 7cf7c9dd9ef5e9fc2bf7e4f2c058eb8dea6fab44..2732bd81633bdd81b45b05eeb23ccb14b43b8e53 100644 (file)
--- a/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc
+++ b/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc
@@ -172,11 +172,13 @@ TEST_F(TestMockExclusiveLockAcquireRequest, Success) {
   expect_create_journal(mock_image_ctx, &mock_journal);
   expect_open_journal(mock_image_ctx, mock_journal, 0);
 
+  C_SaferCond acquire_ctx;
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       &ctx);
+                                                       &acquire_ctx, &ctx);
   req->send();
+  ASSERT_EQ(0, acquire_ctx.wait());
   ASSERT_EQ(0, ctx.wait());
 }
 
@@ -200,11 +202,13 @@ TEST_F(TestMockExclusiveLockAcquireRequest, SuccessJournalDisabled) {
 
   expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, false);
 
+  C_SaferCond acquire_ctx;
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       &ctx);
+                                                       &acquire_ctx, &ctx);
   req->send();
+  ASSERT_EQ(0, acquire_ctx.wait());
   ASSERT_EQ(0, ctx.wait());
 }
 
@@ -227,11 +231,13 @@ TEST_F(TestMockExclusiveLockAcquireRequest, SuccessObjectMapDisabled) {
   expect_create_journal(mock_image_ctx, &mock_journal);
   expect_open_journal(mock_image_ctx, mock_journal, 0);
 
+  C_SaferCond acquire_ctx;
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       &ctx);
+                                                       &acquire_ctx, &ctx);
   req->send();
+  ASSERT_EQ(0, acquire_ctx.wait());
   ASSERT_EQ(0, ctx.wait());
 }
 
@@ -259,10 +265,11 @@ TEST_F(TestMockExclusiveLockAcquireRequest, JournalError) {
   expect_open_journal(mock_image_ctx, *mock_journal, -EINVAL);
   expect_unlock_object_map(mock_image_ctx, *mock_object_map);
 
+  C_SaferCond acquire_ctx;
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       &ctx);
+                                                       &acquire_ctx, &ctx);
   req->send();
   ASSERT_EQ(-EINVAL, ctx.wait());
 }
@@ -289,7 +296,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, LockBusy) {
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       &ctx);
+                                                       nullptr, &ctx);
   req->send();
   ASSERT_EQ(-ENOENT, ctx.wait());
 }
@@ -311,7 +318,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoError) {
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       &ctx);
+                                                       nullptr, &ctx);
   req->send();
   ASSERT_EQ(-EINVAL, ctx.wait());
 }
@@ -334,7 +341,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoEmpty) {
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       &ctx);
+                                                       nullptr, &ctx);
   req->send();
   ASSERT_EQ(-EINVAL, ctx.wait());
 }
@@ -356,7 +363,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoExternalTag) {
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       &ctx);
+                                                       nullptr, &ctx);
   req->send();
   ASSERT_EQ(-EBUSY, ctx.wait());
 }
@@ -379,7 +386,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoShared) {
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       &ctx);
+                                                       nullptr, &ctx);
   req->send();
   ASSERT_EQ(-EBUSY, ctx.wait());
 }
@@ -402,7 +409,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoExternalCookie) {
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       &ctx);
+                                                       nullptr, &ctx);
   req->send();
   ASSERT_EQ(-EBUSY, ctx.wait());
 }
@@ -426,7 +433,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetWatchersError) {
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       &ctx);
+                                                       nullptr, &ctx);
   req->send();
   ASSERT_EQ(-EINVAL, ctx.wait());
 }
@@ -450,7 +457,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetWatchersAlive) {
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       &ctx);
+                                                       nullptr, &ctx);
   req->send();
   ASSERT_EQ(-EAGAIN, ctx.wait());
 }
@@ -477,7 +484,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BlacklistDisabled) {
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       &ctx);
+                                                       nullptr, &ctx);
   req->send();
   ASSERT_EQ(-ENOENT, ctx.wait());
 }
@@ -502,7 +509,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BlacklistError) {
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       &ctx);
+                                                       nullptr, &ctx);
   req->send();
   ASSERT_EQ(-EINVAL, ctx.wait());
 }
@@ -529,7 +536,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BreakLockMissing) {
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       &ctx);
+                                                       nullptr, &ctx);
   req->send();
   ASSERT_EQ(-EINVAL, ctx.wait());
 }
@@ -555,7 +562,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BreakLockError) {
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       &ctx);
+                                                       nullptr, &ctx);
   req->send();
   ASSERT_EQ(-EINVAL, ctx.wait());
 }

diff --git a/src/test/librbd/exclusive_lock/test_mock_ReleaseRequest.cc b/src/test/librbd/exclusive_lock/test_mock_ReleaseRequest.cc
index 09595afdca516947f01522f12844790471bbdccb..c99b361e4d1bc877fac0e7b3daa89657cb060cba 100644 (file)
--- a/src/test/librbd/exclusive_lock/test_mock_ReleaseRequest.cc
+++ b/src/test/librbd/exclusive_lock/test_mock_ReleaseRequest.cc
@@ -29,6 +29,15 @@ class TestMockExclusiveLockReleaseRequest : public TestMockFixture {
 public:
   typedef ReleaseRequest<MockImageCtx> MockReleaseRequest;
 
+  void expect_block_writes(MockImageCtx &mock_image_ctx, int r) {
+    EXPECT_CALL(*mock_image_ctx.aio_work_queue, block_writes(_))
+                  .WillOnce(CompleteContext(r, mock_image_ctx.image_ctx->op_work_queue));
+  }
+
+  void expect_unblock_writes(MockImageCtx &mock_image_ctx) {
+    EXPECT_CALL(*mock_image_ctx.aio_work_queue, unblock_writes());
+  }
+
   void expect_cancel_op_requests(MockImageCtx &mock_image_ctx, int r) {
     EXPECT_CALL(mock_image_ctx, cancel_async_requests(_))
                   .WillOnce(CompleteContext(r, mock_image_ctx.image_ctx->op_work_queue));
@@ -63,6 +72,7 @@ TEST_F(TestMockExclusiveLockReleaseRequest, Success) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_block_writes(mock_image_ctx, 0);
   expect_cancel_op_requests(mock_image_ctx, 0);
 
   MockJournal *mock_journal = new MockJournal();
@@ -75,11 +85,13 @@ TEST_F(TestMockExclusiveLockReleaseRequest, Success) {
 
   expect_unlock(mock_image_ctx, 0);
 
+  C_SaferCond release_ctx;
   C_SaferCond ctx;
   MockReleaseRequest *req = MockReleaseRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       &ctx);
+                                                       &release_ctx, &ctx);
   req->send();
+  ASSERT_EQ(0, release_ctx.wait());
   ASSERT_EQ(0, ctx.wait());
 }
 
@@ -90,6 +102,7 @@ TEST_F(TestMockExclusiveLockReleaseRequest, SuccessJournalDisabled) {
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
 
   MockImageCtx mock_image_ctx(*ictx);
+  expect_block_writes(mock_image_ctx, 0);
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
@@ -101,11 +114,13 @@ TEST_F(TestMockExclusiveLockReleaseRequest, SuccessJournalDisabled) {
 
   expect_unlock(mock_image_ctx, 0);
 
+  C_SaferCond release_ctx;
   C_SaferCond ctx;
   MockReleaseRequest *req = MockReleaseRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       &ctx);
+                                                       &release_ctx, &ctx);
   req->send();
+  ASSERT_EQ(0, release_ctx.wait());
   ASSERT_EQ(0, ctx.wait());
 }
 
@@ -116,6 +131,7 @@ TEST_F(TestMockExclusiveLockReleaseRequest, SuccessObjectMapDisabled) {
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
 
   MockImageCtx mock_image_ctx(*ictx);
+  expect_block_writes(mock_image_ctx, 0);
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
@@ -123,14 +139,37 @@ TEST_F(TestMockExclusiveLockReleaseRequest, SuccessObjectMapDisabled) {
 
   expect_unlock(mock_image_ctx, 0);
 
+  C_SaferCond release_ctx;
   C_SaferCond ctx;
   MockReleaseRequest *req = MockReleaseRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       &ctx);
+                                                       &release_ctx, &ctx);
   req->send();
+  ASSERT_EQ(0, release_ctx.wait());
   ASSERT_EQ(0, ctx.wait());
 }
 
+TEST_F(TestMockExclusiveLockReleaseRequest, BlockWritesError) {
+  REQUIRE_FEATURE(RBD_FEATURE_EXCLUSIVE_LOCK);
+
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  MockImageCtx mock_image_ctx(*ictx);
+  expect_op_work_queue(mock_image_ctx);
+
+  InSequence seq;
+  expect_block_writes(mock_image_ctx, -EINVAL);
+  expect_unblock_writes(mock_image_ctx);
+
+  C_SaferCond ctx;
+  MockReleaseRequest *req = MockReleaseRequest::create(mock_image_ctx,
+                                                       TEST_COOKIE,
+                                                       nullptr, &ctx);
+  req->send();
+  ASSERT_EQ(-EINVAL, ctx.wait());
+}
+
 TEST_F(TestMockExclusiveLockReleaseRequest, UnlockError) {
   REQUIRE_FEATURE(RBD_FEATURE_EXCLUSIVE_LOCK);
 
@@ -141,6 +180,7 @@ TEST_F(TestMockExclusiveLockReleaseRequest, UnlockError) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_block_writes(mock_image_ctx, 0);
   expect_cancel_op_requests(mock_image_ctx, 0);
 
   expect_unlock(mock_image_ctx, -EINVAL);
@@ -148,7 +188,7 @@ TEST_F(TestMockExclusiveLockReleaseRequest, UnlockError) {
   C_SaferCond ctx;
   MockReleaseRequest *req = MockReleaseRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
-                                                       &ctx);
+                                                       nullptr, &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 56b2701295c006d73b18d09240b576227e6bc9b4..cac28a6c64aef0c16126e882905518e939a6b13e 100644 (file)
--- a/src/test/librbd/test_mock_ExclusiveLock.cc
+++ b/src/test/librbd/test_mock_ExclusiveLock.cc
@@ -17,12 +17,14 @@ namespace exclusive_lock {
 template<typename T>
 struct BaseRequest {
   static std::list<T *> s_requests;
+  Context *on_lock_unlock;
   Context *on_finish;
 
   static T* create(MockImageCtx &image_ctx, const std::string &cookie,
-                   Context *on_finish) {
+                   Context *on_lock_unlock, Context *on_finish) {
     assert(!s_requests.empty());
     T* req = s_requests.front();
+    req->on_lock_unlock = on_lock_unlock;
     req->on_finish = on_finish;
     s_requests.pop_front();
     return req;
@@ -87,21 +89,14 @@ public:
                   .WillOnce(FinishRequest(&acquire_request, r, &mock_image_ctx));
     if (r == 0) {
       expect_notify_acquired_lock(mock_image_ctx);
-      expect_unblock_writes(mock_image_ctx);
     }
   }
 
   void expect_release_lock(MockImageCtx &mock_image_ctx,
                            MockReleaseRequest &release_request, int r,
                            bool shutting_down = false) {
-    if (!shutting_down) {
-      expect_block_writes(mock_image_ctx);
-    }
     EXPECT_CALL(release_request, send())
                   .WillOnce(FinishRequest(&release_request, r, &mock_image_ctx));
-    if (!shutting_down && r < 0) {
-      expect_unblock_writes(mock_image_ctx);
-    }
     if (r == 0) {
       expect_notify_released_lock(mock_image_ctx);
       expect_writes_empty(mock_image_ctx);
@@ -506,7 +501,6 @@ TEST_F(TestMockExclusiveLock, ConcurrentRequests) {
 
   MockReleaseRequest release;
   C_SaferCond wait_for_send_ctx2;
-  expect_block_writes(mock_image_ctx);
   EXPECT_CALL(release, send())
                 .WillOnce(Notify(&wait_for_send_ctx2));
   expect_notify_released_lock(mock_image_ctx);