librbd: unlock image if journal error encountered during lock

Explicitly unlock to prevent a client from accidentally blacklisting
itself when retrying the lock.

Fixes: http://tracker.ceph.com/issues/15709
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(cherry picked from commit a11f5e8e55fc448ed60616cbf66a3ea7db2247b9)

diff --git a/src/librbd/exclusive_lock/AcquireRequest.cc b/src/librbd/exclusive_lock/AcquireRequest.cc
index 6ec148e40ef9c5be8a7f88192452a187b69a6d5b..4008c827a1d2623c8fc3a9de8b64c778a0615192 100644 (file)
--- a/src/librbd/exclusive_lock/AcquireRequest.cc
+++ b/src/librbd/exclusive_lock/AcquireRequest.cc
@@ -218,7 +218,13 @@ Context *AcquireRequest<I>::handle_close_journal(int *ret_val) {
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 10) << __func__ << ": r=" << *ret_val << dendl;
 
-  return send_close_object_map(ret_val);
+  if (*ret_val < 0) {
+    lderr(cct) << "failed to close journal: " << cpp_strerror(*ret_val)
+               << dendl;
+  }
+
+  send_close_object_map();
+  return nullptr;
 }
 
 template <typename I>
@@ -250,10 +256,10 @@ Context *AcquireRequest<I>::handle_open_object_map(int *ret_val) {
 }
 
 template <typename I>
-Context *AcquireRequest<I>::send_close_object_map(int *ret_val) {
+void AcquireRequest<I>::send_close_object_map() {
   if (m_object_map == nullptr) {
-    revert(ret_val);
-    return m_on_finish;
+    send_unlock();
+    return;
   }
 
   CephContext *cct = m_image_ctx.cct;
@@ -263,7 +269,6 @@ Context *AcquireRequest<I>::send_close_object_map(int *ret_val) {
   Context *ctx = create_context_callback<
     klass, &klass::handle_close_object_map>(this);
   m_object_map->close(ctx);
-  return nullptr;
 }
 
 template <typename I>
@@ -273,6 +278,36 @@ Context *AcquireRequest<I>::handle_close_object_map(int *ret_val) {
 
   // object map should never result in an error
   assert(*ret_val == 0);
+  send_unlock();
+  return nullptr;
+}
+
+template <typename I>
+void AcquireRequest<I>::send_unlock() {
+  CephContext *cct = m_image_ctx.cct;
+  ldout(cct, 10) << __func__ << dendl;
+
+  librados::ObjectWriteOperation op;
+  rados::cls::lock::unlock(&op, RBD_LOCK_NAME, m_cookie);
+
+  using klass = AcquireRequest<I>;
+  librados::AioCompletion *rados_completion =
+    create_rados_safe_callback<klass, &klass::handle_unlock>(this);
+  int r = m_image_ctx.md_ctx.aio_operate(m_image_ctx.header_oid,
+                                         rados_completion, &op);
+  assert(r == 0);
+  rados_completion->release();
+}
+
+template <typename I>
+Context *AcquireRequest<I>::handle_unlock(int *ret_val) {
+  CephContext *cct = m_image_ctx.cct;
+  ldout(cct, 10) << __func__ << ": r=" << *ret_val << dendl;
+
+  if (*ret_val < 0) {
+    lderr(cct) << "failed to unlock image: " << cpp_strerror(*ret_val) << dendl;
+  }
+
   revert(ret_val);
   return m_on_finish;
 }

diff --git a/src/librbd/exclusive_lock/AcquireRequest.h b/src/librbd/exclusive_lock/AcquireRequest.h
index 2c4e05b86ceef569f086796fe755cb60302bfa06..9a16fc56e2ab5904911c4bb8382f99bb1e07300d 100644 (file)
--- a/src/librbd/exclusive_lock/AcquireRequest.h
+++ b/src/librbd/exclusive_lock/AcquireRequest.h
@@ -49,21 +49,26 @@ private:
    *          .   |                         |                             |
    *    . . . .   |                         |                             |
    *    .         v                         v                             |
-   *    .     OPEN_OBJECT_MAP             GET_WATCHERS . . .              |
-   *    .         |                         |              .              |
+   *    .     OPEN_OBJECT_MAP (skip if    GET_WATCHERS . . .              |
+   *    .         |            disabled)    |              .              |
    *    .         v                         v              .              |
-   *    . . > OPEN_JOURNAL * * * * * *    BLACKLIST        . (blacklist   |
-   *    .         |                  *      |              .  disabled)   |
-   *    .         v                  *      v              .              |
-   *    .     ALLOCATE_JOURNAL_TAG   *    BREAK_LOCK < . . .              |
-   *    .         |            *     *      |                             |
-   *    .         |            *     *      \-----------------------------/
-   *    .         |            v     v
+   *    . . > OPEN_JOURNAL (skip if       BLACKLIST        . (blacklist   |
+   *    .         |   *     disabled)       |              .  disabled)   |
+   *    .         |   *                     v              .              |
+   *    .         |   * * * * * * * *     BREAK_LOCK < . . .              |
+   *    .         v                 *       |                             |
+   *    .     ALLOCATE_JOURNAL_TAG  *       \-----------------------------/
+   *    .         |            *    *
+   *    .         |            *    *
+   *    .         |            v    v
    *    .         |         CLOSE_JOURNAL
    *    .         |               |
    *    .         |               v
    *    .         |         CLOSE_OBJECT_MAP
    *    .         |               |
+   *    .         |               v
+   *    .         |         UNLOCK_IMAGE
+   *    .         |               |
    *    .         v               |
    *    . . > <finish> <----------/
    *
@@ -105,15 +110,18 @@ private:
   void send_allocate_journal_tag();
   Context *handle_allocate_journal_tag(int *ret_val);
 
-  void send_close_journal();
-  Context *handle_close_journal(int *ret_val);
-
   Context *send_open_object_map();
   Context *handle_open_object_map(int *ret_val);
 
-  Context *send_close_object_map(int *ret_val);
+  void send_close_journal();
+  Context *handle_close_journal(int *ret_val);
+
+  void send_close_object_map();
   Context *handle_close_object_map(int *ret_val);
 
+  void send_unlock();
+  Context *handle_unlock(int *ret_val);
+
   void send_get_lockers();
   Context *handle_get_lockers(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 c7c9428fed11f7d9f8dc723ae6d8d4183de9a647..b5eef00b650f13c72e817fbebbe385474899c6f4 100644 (file)
--- a/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc
+++ b/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc
@@ -52,6 +52,12 @@ public:
                   .WillOnce(Return(r));
   }
 
+  void expect_unlock(MockImageCtx &mock_image_ctx, int r) {
+    EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx),
+                exec(mock_image_ctx.header_oid, _, StrEq("lock"), StrEq("unlock"), _, _, _))
+                  .WillOnce(Return(r));
+  }
+
   void expect_create_object_map(MockImageCtx &mock_image_ctx,
                                 MockObjectMap *mock_object_map) {
     EXPECT_CALL(mock_image_ctx, create_object_map(_))
@@ -293,6 +299,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, JournalError) {
   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);
+  expect_unlock(mock_image_ctx, 0);
 
   C_SaferCond acquire_ctx;
   C_SaferCond ctx;
@@ -330,6 +337,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, AllocateJournalTagError) {
   expect_allocate_journal_tag(mock_image_ctx, mock_journal_policy, -EPERM);
   expect_close_journal(mock_image_ctx, *mock_journal);
   expect_close_object_map(mock_image_ctx, *mock_object_map);
+  expect_unlock(mock_image_ctx, 0);
 
   C_SaferCond acquire_ctx;
   C_SaferCond ctx;