librbd: initialize object map before replaying journal

The journal might replay write events which will need to update the
object map.

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

diff --git a/src/librbd/exclusive_lock/AcquireRequest.cc b/src/librbd/exclusive_lock/AcquireRequest.cc
index 8c19282165c118d37fb2300e59e623963cd80eb1..f0826b1252c2eabcbc34e652d6e1ef978e8abbc4 100644 (file)
--- a/src/librbd/exclusive_lock/AcquireRequest.cc
+++ b/src/librbd/exclusive_lock/AcquireRequest.cc
@@ -62,7 +62,7 @@ AcquireRequest<I>::AcquireRequest(I &image_ctx, const std::string &cookie,
                                   Context *on_finish)
   : m_image_ctx(image_ctx), m_cookie(cookie),
     m_on_finish(create_async_context_callback(image_ctx, on_finish)),
-    m_object_map(nullptr), m_journal(nullptr) {
+    m_object_map(nullptr), m_journal(nullptr), m_error_result(0) {
 }
 
 template <typename I>
@@ -94,7 +94,7 @@ Context *AcquireRequest<I>::handle_lock(int *ret_val) {
   ldout(cct, 10) << __func__ << ": r=" << *ret_val << dendl;
 
   if (*ret_val == 0) {
-    return send_open_journal();
+    return send_open_object_map();
   } else if (*ret_val != -EBUSY) {
     lderr(cct) << "failed to lock: " << cpp_strerror(*ret_val) << dendl;
     return m_on_finish;
@@ -107,7 +107,8 @@ Context *AcquireRequest<I>::handle_lock(int *ret_val) {
 template <typename I>
 Context *AcquireRequest<I>::send_open_journal() {
   if (!m_image_ctx.test_features(RBD_FEATURE_JOURNALING)) {
-    return send_open_object_map();
+    apply();
+    return m_on_finish;
   }
 
   CephContext *cct = m_image_ctx.cct;
@@ -117,6 +118,10 @@ Context *AcquireRequest<I>::send_open_journal() {
   Context *ctx = create_context_callback<klass, &klass::handle_open_journal>(
     this);
   m_journal = m_image_ctx.create_journal();
+
+  // journal playback required object map (if enabled) and itself
+  apply();
+
   m_journal->open(ctx);
   return nullptr;
 }
@@ -128,20 +133,17 @@ Context *AcquireRequest<I>::handle_open_journal(int *ret_val) {
 
   if (*ret_val < 0) {
     lderr(cct) << "failed to open journal: " << cpp_strerror(*ret_val) << dendl;
-
-    delete m_journal;
-    return m_on_finish;
+    m_error_result = *ret_val;
+    return send_unlock_object_map();
   }
 
-  assert(m_image_ctx.journal == nullptr);
-  return send_open_object_map();
+  return m_on_finish;
 }
 
 template <typename I>
 Context *AcquireRequest<I>::send_open_object_map() {
   if (!m_image_ctx.test_features(RBD_FEATURE_OBJECT_MAP)) {
-    apply();
-    return m_on_finish;
+    return send_open_journal();
   }
 
   CephContext *cct = m_image_ctx.cct;
@@ -150,6 +152,7 @@ Context *AcquireRequest<I>::send_open_object_map() {
   using klass = AcquireRequest<I>;
   Context *ctx = create_context_callback<klass, &klass::handle_open_object_map>(
     this);
+
   m_object_map = m_image_ctx.create_object_map(CEPH_NOSNAP);
   m_object_map->open(ctx);
   return nullptr;
@@ -186,8 +189,38 @@ Context *AcquireRequest<I>::handle_lock_object_map(int *ret_val) {
 
   // object map should never result in an error
   assert(*ret_val == 0);
+  return send_open_journal();
+}
 
-  apply();
+template <typename I>
+Context *AcquireRequest<I>::send_unlock_object_map() {
+  if (m_object_map == nullptr) {
+    revert();
+    return m_on_finish;
+  }
+
+  CephContext *cct = m_image_ctx.cct;
+  ldout(cct, 10) << __func__ << dendl;
+
+  using klass = AcquireRequest<I>;
+  Context *ctx = create_context_callback<
+    klass, &klass::handle_unlock_object_map>(this);
+  m_object_map->unlock(ctx);
+  return nullptr;
+}
+
+template <typename I>
+Context *AcquireRequest<I>::handle_unlock_object_map(int *ret_val) {
+  CephContext *cct = m_image_ctx.cct;
+  ldout(cct, 10) << __func__ << ": r=" << *ret_val << dendl;
+
+  // object map should never result in an error
+  assert(*ret_val == 0);
+
+  assert(m_error_result < 0);
+  *ret_val = m_error_result;
+
+  revert();
   return m_on_finish;
 }
 
@@ -397,6 +430,16 @@ void AcquireRequest<I>::apply() {
   m_image_ctx.journal = m_journal;
 }
 
+template <typename I>
+void AcquireRequest<I>::revert() {
+  RWLock::WLocker snap_locker(m_image_ctx.snap_lock);
+  m_image_ctx.object_map = nullptr;
+  m_image_ctx.journal = nullptr;
+
+  delete m_object_map;
+  delete m_journal;
+}
+
 } // namespace exclusive_lock
 } // namespace librbd
 

diff --git a/src/librbd/exclusive_lock/AcquireRequest.h b/src/librbd/exclusive_lock/AcquireRequest.h
index 71d4a9dea25c83544eda2a01db685eba0831ecb8..e3062156e59f0fbc617ef5db11d0c98c415430ba 100644 (file)
--- a/src/librbd/exclusive_lock/AcquireRequest.h
+++ b/src/librbd/exclusive_lock/AcquireRequest.h
@@ -44,18 +44,19 @@ private:
    *          .   |                       |                             |
    *    . . . .   |                       |                             |
    *    .         v                       v                             |
-   *    .     OPEN_JOURNAL  . . .       GET_WATCHERS . . .              |
-   *    .         |             .         |              .              |
-   *    .         v             .         v              .              |
-   *    . . > OPEN_OBJECT_MAP   .       BLACKLIST        . (blacklist   |
-   *    .         |             .         |              .  disabled)   |
-   *    .         v             .         v              .              |
-   *    .     LOCK_OBJECT_MAP   .       BREAK_LOCK < . . .              |
-   *    .         |             .         |                             |
-   *    .         v             .         |                             |
-   *    . . > <finish>  < . . . .         |                             |
-   *                                      \-----------------------------/
-   *
+   *    .     OPEN_OBJECT_MAP           GET_WATCHERS . . .              |
+   *    .         |                       |              .              |
+   *    .         v                       v              .              |
+   *    .     LOCK_OBJECT_MAP           BLACKLIST        . (blacklist   |
+   *    .         |                       |              .  disabled)   |
+   *    .         v                       v              .              |
+   *    . . > OPEN_JOURNAL * *          BREAK_LOCK < . . .              |
+   *    .         |          *            |                             |
+   *    .         |          v            |                             |
+   *    .         |    UNLOCK_OBJECT_MAP  |                             |
+   *    .         |          |            \-----------------------------/
+   *    .         v          |
+   *    . . > <finish> <-----/
    * @endverbatim
    */
 
@@ -79,6 +80,8 @@ private:
   std::string m_locker_address;
   uint64_t m_locker_handle;
 
+  int m_error_result;
+
   void send_lock();
   Context *handle_lock(int *ret_val);
 
@@ -91,6 +94,9 @@ private:
   Context *send_lock_object_map();
   Context *handle_lock_object_map(int *ret_val);
 
+  Context *send_unlock_object_map();
+  Context *handle_unlock_object_map(int *ret_val);
+
   void send_get_lockers();
   Context *handle_get_lockers(int *ret_val);
 
@@ -104,6 +110,7 @@ private:
   Context *handle_break_lock(int *ret_val);
 
   void apply();
+  void revert();
 };
 
 } // namespace exclusive_lock

diff --git a/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc b/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc
index 31e17a0385c95dd76abd74781bb064827b443f0c..7cf7c9dd9ef5e9fc2bf7e4f2c058eb8dea6fab44 100644 (file)
--- a/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc
+++ b/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc
@@ -67,6 +67,12 @@ public:
                   .WillOnce(CompleteContext(0, mock_image_ctx.image_ctx->op_work_queue));
   }
 
+  void expect_unlock_object_map(MockImageCtx &mock_image_ctx,
+                              MockObjectMap &mock_object_map) {
+    EXPECT_CALL(mock_object_map, unlock(_))
+                  .WillOnce(CompleteContext(0, mock_image_ctx.image_ctx->op_work_queue));
+  }
+
   void expect_create_journal(MockImageCtx &mock_image_ctx,
                              MockJournal *mock_journal) {
     EXPECT_CALL(mock_image_ctx, create_journal())
@@ -155,17 +161,17 @@ TEST_F(TestMockExclusiveLockAcquireRequest, Success) {
   InSequence seq;
   expect_lock(mock_image_ctx, 0);
 
-  MockJournal mock_journal;
-  expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, true);
-  expect_create_journal(mock_image_ctx, &mock_journal);
-  expect_open_journal(mock_image_ctx, mock_journal, 0);
-
   MockObjectMap mock_object_map;
   expect_test_features(mock_image_ctx, RBD_FEATURE_OBJECT_MAP, true);
   expect_create_object_map(mock_image_ctx, &mock_object_map);
   expect_open_object_map(mock_image_ctx, mock_object_map);
   expect_lock_object_map(mock_image_ctx, mock_object_map);
 
+  MockJournal mock_journal;
+  expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, true);
+  expect_create_journal(mock_image_ctx, &mock_journal);
+  expect_open_journal(mock_image_ctx, mock_journal, 0);
+
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
@@ -186,14 +192,14 @@ TEST_F(TestMockExclusiveLockAcquireRequest, SuccessJournalDisabled) {
   InSequence seq;
   expect_lock(mock_image_ctx, 0);
 
-  expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, false);
-
   MockObjectMap mock_object_map;
   expect_test_features(mock_image_ctx, RBD_FEATURE_OBJECT_MAP, true);
   expect_create_object_map(mock_image_ctx, &mock_object_map);
   expect_open_object_map(mock_image_ctx, mock_object_map);
   expect_lock_object_map(mock_image_ctx, mock_object_map);
 
+  expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, false);
+
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
@@ -214,13 +220,13 @@ TEST_F(TestMockExclusiveLockAcquireRequest, SuccessObjectMapDisabled) {
   InSequence seq;
   expect_lock(mock_image_ctx, 0);
 
+  expect_test_features(mock_image_ctx, RBD_FEATURE_OBJECT_MAP, false);
+
   MockJournal mock_journal;
   expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, true);
   expect_create_journal(mock_image_ctx, &mock_journal);
   expect_open_journal(mock_image_ctx, mock_journal, 0);
 
-  expect_test_features(mock_image_ctx, RBD_FEATURE_OBJECT_MAP, false);
-
   C_SaferCond ctx;
   MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
                                                        TEST_COOKIE,
@@ -229,6 +235,38 @@ TEST_F(TestMockExclusiveLockAcquireRequest, SuccessObjectMapDisabled) {
   ASSERT_EQ(0, ctx.wait());
 }
 
+TEST_F(TestMockExclusiveLockAcquireRequest, JournalError) {
+  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_lock(mock_image_ctx, 0);
+
+  MockObjectMap *mock_object_map = new MockObjectMap();
+  expect_test_features(mock_image_ctx, RBD_FEATURE_OBJECT_MAP, true);
+  expect_create_object_map(mock_image_ctx, mock_object_map);
+  expect_open_object_map(mock_image_ctx, *mock_object_map);
+  expect_lock_object_map(mock_image_ctx, *mock_object_map);
+
+  MockJournal *mock_journal = new MockJournal();
+  expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, true);
+  expect_create_journal(mock_image_ctx, mock_journal);
+  expect_open_journal(mock_image_ctx, *mock_journal, -EINVAL);
+  expect_unlock_object_map(mock_image_ctx, *mock_object_map);
+
+  C_SaferCond ctx;
+  MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx,
+                                                       TEST_COOKIE,
+                                                       &ctx);
+  req->send();
+  ASSERT_EQ(-EINVAL, ctx.wait());
+}
+
 TEST_F(TestMockExclusiveLockAcquireRequest, LockBusy) {
   REQUIRE_FEATURE(RBD_FEATURE_EXCLUSIVE_LOCK);