]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
librbd: properly handle potential object map failures
authorJason Dillaman <dillaman@redhat.com>
Tue, 18 Sep 2018 18:37:12 +0000 (14:37 -0400)
committerJason Dillaman <dillaman@redhat.com>
Tue, 8 Oct 2019 20:16:18 +0000 (16:16 -0400)
Remove the "ceph_assert" statements and instead bubble any potential
error code up to the caller. The object map state machines should
attempt to return a 0 upon failure unless it was unable to flag the
object map as invalid.

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

Conflicts:
src/librbd/DeepCopyRequest.cc: trivial resolution
src/librbd/deep_copy/ObjectCopyRequest.cc: trivial resolution
src/librbd/deep_copy/SnapshotCopyRequest.cc: trivial resolution
src/librbd/exclusive_lock/PostAcquireRequest.cc: trivial resolution
src/librbd/exclusive_lock/PreReleaseRequest.cc: trivial resolution
src/librbd/image/RefreshRequest.cc: trivial resolution
src/librbd/io/CopyupRequest.cc: trivial resolution
src/librbd/io/ObjectRequest.cc: trivial resolution
src/librbd/object_map/InvalidateRequest.cc: trivial resolution
src/librbd/object_map/RefreshRequest.cc: trivial resolution
src/librbd/object_map/SnapshotRemoveRequest.cc: trivial resolution
src/librbd/operation/ResizeRequest.cc: trivial resolution
src/librbd/operation/SnapshotCreateRequest.cc: trivial resolution
src/librbd/operation/SnapshotRollbackRequest.cc: trivial resolution

20 files changed:
src/librbd/DeepCopyRequest.cc
src/librbd/deep_copy/ObjectCopyRequest.cc
src/librbd/deep_copy/SnapshotCopyRequest.cc
src/librbd/exclusive_lock/PostAcquireRequest.cc
src/librbd/exclusive_lock/PreReleaseRequest.cc
src/librbd/image/RefreshRequest.cc
src/librbd/io/CopyupRequest.cc
src/librbd/io/ObjectRequest.cc
src/librbd/object_map/InvalidateRequest.cc
src/librbd/object_map/RefreshRequest.cc
src/librbd/object_map/Request.h
src/librbd/object_map/SnapshotRemoveRequest.cc
src/librbd/operation/ResizeRequest.cc
src/librbd/operation/SnapshotRollbackRequest.cc
src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc
src/test/librbd/exclusive_lock/test_mock_PostAcquireRequest.cc
src/test/librbd/image/test_mock_RefreshRequest.cc
src/test/librbd/io/test_mock_ObjectRequest.cc
src/test/librbd/object_map/test_mock_RefreshRequest.cc
src/test/librbd/test_mock_DeepCopyRequest.cc

index d8bb9ff8a5f773d6f1926a96c2f41271f53fa896..020273e9d7ea115cf6ab052a2f56aa50152f1338 100644 (file)
@@ -226,7 +226,13 @@ template <typename I>
 void DeepCopyRequest<I>::handle_copy_object_map(int r) {
   ldout(m_cct, 20) << dendl;
 
-  assert(r == 0);
+  if (r < 0) {
+    lderr(m_cct) << "failed to roll back object map: " << cpp_strerror(r)
+                 << dendl;
+    finish(r);
+    return;
+  }
+
   send_refresh_object_map();
 }
 
@@ -260,9 +266,18 @@ template <typename I>
 void DeepCopyRequest<I>::handle_refresh_object_map(int r) {
   ldout(m_cct, 20) << "r=" << r << dendl;
 
-  assert(r == 0);
+  if (r < 0) {
+    lderr(m_cct) << "failed to open object map: " << cpp_strerror(r)
+                 << dendl;
+    delete m_object_map;
+
+    finish(r);
+    return;
+  }
+
   {
     RWLock::WLocker snap_locker(m_dst_image_ctx->snap_lock);
+    RWLock::WLocker object_map_locker(m_dst_image_ctx->object_map_lock);
     std::swap(m_dst_image_ctx->object_map, m_object_map);
   }
   delete m_object_map;
index 7f10de32ff87f33103a7c901e9d06d9959bfc6d2..600ecc55dd8201b94ad0920caa7ed5e88cdd45b5 100644 (file)
@@ -482,7 +482,12 @@ template <typename I>
 void ObjectCopyRequest<I>::handle_update_object_map(int r) {
   ldout(m_cct, 20) << "r=" << r << dendl;
 
-  assert(r == 0);
+  if (r < 0) {
+    lderr(m_cct) << "failed to update object map: " << cpp_strerror(r) << dendl;
+    finish(r);
+    return;
+  }
+
   if (!m_dst_object_state.empty()) {
     send_update_object_map();
     return;
index 597c99bbd390382f0682a6b8e1b4886994283d1b..f24fa1240c9ed35e54f198103355b0cea86b133f 100644 (file)
@@ -598,7 +598,13 @@ template <typename I>
 void SnapshotCopyRequest<I>::handle_resize_object_map(int r) {
   ldout(m_cct, 20) << "r=" << r << dendl;
 
-  assert(r == 0);
+  if (r < 0) {
+    lderr(m_cct) << "failed to resize object map: " << cpp_strerror(r)
+                 << dendl;
+    finish(r);
+    return;
+  }
+
   finish(0);
 }
 
index 870cf49b3d2597d5424c657176f26a0a442e1471..0e7c6eadc4f11271b956bc2bb82501cf3fd6a026 100644 (file)
@@ -225,10 +225,15 @@ void PostAcquireRequest<I>::handle_open_object_map(int r) {
 
   if (r < 0) {
     lderr(cct) << "failed to open object map: " << cpp_strerror(r) << dendl;
-
-    r = 0;
     delete m_object_map;
     m_object_map = nullptr;
+
+    if (r != -EFBIG) {
+      save_result(r);
+      revert();
+      finish();
+      return;
+    }
   }
 
   send_open_journal();
@@ -256,8 +261,10 @@ void PostAcquireRequest<I>::handle_close_object_map(int r) {
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 10) << "r=" << r << dendl;
 
-  // object map should never result in an error
-  assert(r == 0);
+  if (r < 0) {
+    lderr(cct) << "failed to close object map: " << cpp_strerror(r) << dendl;
+  }
+
   revert();
   finish();
 }
index e5ba42b057e43b09cb30d3dc94faa4fd83406bbd..b26727f02e7c7b10ec79036d86530a8413b44e7e 100644 (file)
@@ -271,10 +271,11 @@ void PreReleaseRequest<I>::handle_close_object_map(int r) {
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 10) << "r=" << r << dendl;
 
-  // object map shouldn't return errors
-  assert(r == 0);
-  delete m_object_map;
+  if (r < 0) {
+    lderr(cct) << "failed to close object map: " << cpp_strerror(r) << dendl;
+  }
 
+  delete m_object_map;
   send_unlock();
 }
 
index 610c5ef13a8d54a7854da5518ba17b6dfe79b493..e1e0f0a976557cddd4866611efa76239f5b70d7b 100644 (file)
@@ -901,6 +901,10 @@ Context *RefreshRequest<I>::handle_v2_open_object_map(int *result) {
                << dendl;
     delete m_object_map;
     m_object_map = nullptr;
+
+    if (*result != -EFBIG) {
+      save_result(result);
+    }
   }
 
   send_v2_open_journal();
@@ -1059,7 +1063,11 @@ Context *RefreshRequest<I>::handle_v2_close_object_map(int *result) {
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 10) << this << " " << __func__ << ": r=" << *result << dendl;
 
-  assert(*result == 0);
+  if (*result < 0) {
+    lderr(cct) << "failed to close object map: " << cpp_strerror(*result)
+               << dendl;
+  }
+
   assert(m_object_map != nullptr);
   delete m_object_map;
   m_object_map = nullptr;
index 44f4ce93418e73266f0278ace67626dd0e1f4025..8e981f38482cecd48e721b308aaf9b3f496ce14c 100644 (file)
@@ -254,12 +254,26 @@ bool CopyupRequest<I>::should_complete(int r)
 
   case STATE_OBJECT_MAP_HEAD:
     ldout(cct, 20) << "OBJECT_MAP_HEAD" << dendl;
-    assert(r == 0);
+    if (r < 0) {
+      lderr(cct) << "failed to update head object map: " << cpp_strerror(r)
+                 << dendl;
+      break;
+    }
+
     return send_object_map();
 
   case STATE_OBJECT_MAP:
     ldout(cct, 20) << "OBJECT_MAP" << dendl;
-    assert(r == 0);
+    if (r < 0) {
+      lderr(cct) << "failed to update object map: " << cpp_strerror(r)
+                 << dendl;
+      break;
+    }
+
+    if (!is_copyup_required()) {
+      ldout(cct, 20) << "skipping copyup" << dendl;
+      return true;
+    }
     return send_copyup();
 
   case STATE_COPYUP:
index 3d09b50448283b3cee2c6f568a2021dd9eb0756c..7d8999ac6ebdcdc8531470800c9a6830ad4cda47 100644 (file)
@@ -466,8 +466,13 @@ template <typename I>
 void AbstractObjectWriteRequest<I>::handle_pre_write_object_map_update(int r) {
   I *image_ctx = this->m_ictx;
   ldout(image_ctx->cct, 20) << "r=" << r << dendl;
+  if (r < 0) {
+    lderr(image_ctx->cct) << "failed to update object map: "
+                          << cpp_strerror(r) << dendl;
+    this->finish(r);
+    return;
+  }
 
-  assert(r == 0);
   write_object();
 }
 
@@ -608,8 +613,13 @@ template <typename I>
 void AbstractObjectWriteRequest<I>::handle_post_write_object_map_update(int r) {
   I *image_ctx = this->m_ictx;
   ldout(image_ctx->cct, 20) << "r=" << r << dendl;
+  if (r < 0) {
+    lderr(image_ctx->cct) << "failed to update object map: "
+                          << cpp_strerror(r) << dendl;
+    this->finish(r);
+    return;
+  }
 
-  assert(r == 0);
   this->finish(0);
 }
 
index e744add68db857b3b98f7b62f36603675c4f1700..4b7be69e546a999bcdce992f45d4bf3b1db20fa4 100644 (file)
@@ -53,7 +53,7 @@ void InvalidateRequest<I>::send() {
       (!m_force && m_snap_id == CEPH_NOSNAP &&
        image_ctx.exclusive_lock != nullptr &&
        !image_ctx.exclusive_lock->is_lock_owner())) {
-    this->async_complete(0);
+    this->async_complete(-EROFS);
     return;
   }
 
@@ -64,7 +64,7 @@ void InvalidateRequest<I>::send() {
   librados::AioCompletion *rados_completion =
     this->create_callback_completion();
   r = image_ctx.md_ctx.aio_operate(image_ctx.header_oid, rados_completion,
-                                     &op);
+                                   &op);
   assert(r == 0);
   rados_completion->release();
 }
index 840b8c1e7824a00a45fddeb08d4badfaf00c0fbd..8f59ad9ddfe535dda77c3ad34f3e2fce77150a6b 100644 (file)
@@ -181,7 +181,11 @@ Context *RefreshRequest<I>::handle_invalidate(int *ret_val) {
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 10) << this << " " << __func__ << ": r=" << *ret_val << dendl;
 
-  assert(*ret_val == 0);
+  if (*ret_val < 0) {
+    lderr(cct) << "failed to invalidate object map: " << cpp_strerror(*ret_val)
+               << dendl;
+  }
+
   apply();
   return m_on_finish;
 }
@@ -211,7 +215,13 @@ Context *RefreshRequest<I>::handle_resize_invalidate(int *ret_val) {
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 10) << this << " " << __func__ << ": r=" << *ret_val << dendl;
 
-  assert(*ret_val == 0);
+  if (*ret_val < 0) {
+    lderr(cct) << "failed to invalidate object map: " << cpp_strerror(*ret_val)
+               << dendl;
+    apply();
+    return m_on_finish;
+  }
+
   send_resize();
   return nullptr;
 }
@@ -249,6 +259,7 @@ Context *RefreshRequest<I>::handle_resize(int *ret_val) {
                << dendl;
     *ret_val = 0;
   }
+
   apply();
   return m_on_finish;
 }
@@ -275,9 +286,13 @@ Context *RefreshRequest<I>::handle_invalidate_and_close(int *ret_val) {
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 10) << this << " " << __func__ << ": r=" << *ret_val << dendl;
 
-  assert(*ret_val == 0);
+  if (*ret_val < 0) {
+    lderr(cct) << "failed to invalidate object map: " << cpp_strerror(*ret_val)
+               << dendl;
+  } else {
+    *ret_val = -EFBIG;
+  }
 
-  *ret_val = -EFBIG;
   m_object_map->clear();
   return m_on_finish;
 }
index 67f9bac11e5d70766831de833740a1db394f0a80..aef8800dd8ace38e683f3a6d78b254539582370e 100644 (file)
@@ -30,8 +30,11 @@ protected:
 
   bool should_complete(int r) override;
   int filter_return_code(int r) const override {
-    // never propagate an error back to the caller
-    return 0;
+    if (m_state == STATE_REQUEST) {
+      // never propagate an error back to the caller
+      return 0;
+    }
+    return r;
   }
   virtual void finish_request() {
   }
index 3efea66b3d14c404a51296a083e8e6bd17b9f738..074987fbe021733ea4fed370e4d4abd3ed5f7e65 100644 (file)
@@ -141,7 +141,15 @@ void SnapshotRemoveRequest::invalidate_next_map() {
 void SnapshotRemoveRequest::handle_invalidate_next_map(int r) {
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 5) << "r=" << r << dendl;
-  assert(r == 0);
+
+  if (r < 0) {
+    std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id,
+                                                 m_next_snap_id));
+    lderr(cct) << "failed to invalidate object map " << oid << ": "
+               << cpp_strerror(r) << dendl;
+    complete(r);
+    return;
+  }
 
   remove_map();
 }
index 697289859215cd5626c46f5bf733153cb90fd1d9..f06bee6a0fc609f7c64afab9b00b173e6fbf4244 100644 (file)
@@ -296,7 +296,12 @@ Context *ResizeRequest<I>::handle_grow_object_map(int *result) {
   CephContext *cct = image_ctx.cct;
   ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl;
 
-  assert(*result == 0);
+  if (*result < 0) {
+    lderr(cct) << this << " " << __func__ << ": failed to resize object map: "
+               << cpp_strerror(*result) << dendl;
+    return this->create_context_finisher(*result);
+  }
+
   send_post_block_writes();
   return nullptr;
 }
@@ -338,8 +343,14 @@ Context *ResizeRequest<I>::handle_shrink_object_map(int *result) {
   CephContext *cct = image_ctx.cct;
   ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl;
 
+  if (*result < 0) {
+    lderr(cct) << this << " " << __func__ << ": failed to resize object map: "
+               << cpp_strerror(*result) << dendl;
+    image_ctx.io_work_queue->unblock_writes();
+    return this->create_context_finisher(*result);
+  }
+
   update_size_and_overlap();
-  assert(*result == 0);
   return this->create_context_finisher(0);
 }
 
index 7072e72ce599fe378b4e564500f8dd6bad06eab1..8a6c2c602a78004ebc3978415d687977279f4d37 100644 (file)
@@ -189,7 +189,15 @@ Context *SnapshotRollbackRequest<I>::handle_rollback_object_map(int *result) {
   CephContext *cct = image_ctx.cct;
   ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl;
 
-  assert(*result == 0);
+  if (*result < 0) {
+    lderr(cct) << this << " " << __func__ << ": failed to roll back object "
+               << "map: " << cpp_strerror(*result) << dendl;
+
+    ceph_assert(m_object_map == nullptr);
+    apply();
+    return this->create_context_finisher(*result);
+  }
+
   send_rollback_objects();
   return nullptr;
 }
@@ -269,7 +277,16 @@ Context *SnapshotRollbackRequest<I>::handle_refresh_object_map(int *result) {
   CephContext *cct = image_ctx.cct;
   ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl;
 
-  assert(*result == 0);
+  if (*result < 0) {
+    lderr(cct) << this << " " << __func__ << ": failed to open object map: "
+               << cpp_strerror(*result) << dendl;
+    delete m_object_map;
+    m_object_map = nullptr;
+    apply();
+
+    return this->create_context_finisher(*result);
+  }
+
   return send_invalidate_cache();
 }
 
index 62154aa1c7f0896cbed6d4e694bf1f5b843ad7f3..ffe733866ad369035de9cf84ff55795b16f492cb 100644 (file)
@@ -852,5 +852,47 @@ TEST_F(TestMockDeepCopyObjectCopyRequest, Remove) {
   ASSERT_EQ(0, compare_objects());
 }
 
+TEST_F(TestMockDeepCopyObjectCopyRequest, ObjectMapUpdateError) {
+  REQUIRE_FEATURE(RBD_FEATURE_OBJECT_MAP);
+
+  // scribble some data
+  interval_set<uint64_t> one;
+  scribble(m_src_image_ctx, 10, 102400, &one);
+
+  ASSERT_EQ(0, create_snap("copy"));
+  librbd::MockTestImageCtx mock_src_image_ctx(*m_src_image_ctx);
+  librbd::MockTestImageCtx mock_dst_image_ctx(*m_dst_image_ctx);
+
+  librbd::MockExclusiveLock mock_exclusive_lock;
+  prepare_exclusive_lock(mock_dst_image_ctx, mock_exclusive_lock);
+
+  librbd::MockObjectMap mock_object_map;
+  mock_dst_image_ctx.object_map = &mock_object_map;
+
+  expect_test_features(mock_dst_image_ctx);
+
+  C_SaferCond ctx;
+  MockObjectCopyRequest *request = create_request(mock_src_image_ctx,
+                                                  mock_dst_image_ctx, &ctx);
+
+  librados::MockTestMemIoCtxImpl &mock_src_io_ctx(get_mock_io_ctx(
+    request->get_src_io_ctx()));
+  librados::MockTestMemIoCtxImpl &mock_dst_io_ctx(get_mock_io_ctx(
+    request->get_dst_io_ctx()));
+
+  InSequence seq;
+  expect_list_snaps(mock_src_image_ctx, mock_src_io_ctx, 0);
+  expect_set_snap_read(mock_src_io_ctx, m_src_snap_ids[0]);
+  expect_sparse_read(mock_src_io_ctx, 0, one.range_end(), 0);
+  expect_start_op(mock_exclusive_lock);
+  expect_write(mock_dst_io_ctx, 0, one.range_end(), {0, {}}, 0);
+  expect_start_op(mock_exclusive_lock);
+  expect_update_object_map(mock_dst_image_ctx, mock_object_map,
+                           m_dst_snap_ids[0], OBJECT_EXISTS, -EBLACKLISTED);
+
+  request->send();
+  ASSERT_EQ(-EBLACKLISTED, ctx.wait());
+}
+
 } // namespace deep_copy
 } // namespace librbd
index 0f47a75043da3da48dbf41c6f11ebe939a6520d2..86cfd5807182b69bd3dcccc603e721913a46a17c 100644 (file)
@@ -443,7 +443,34 @@ TEST_F(TestMockExclusiveLockPostAcquireRequest, AllocateJournalTagError) {
 }
 
 TEST_F(TestMockExclusiveLockPostAcquireRequest, OpenObjectMapError) {
-  REQUIRE_FEATURE(RBD_FEATURE_EXCLUSIVE_LOCK);
+  REQUIRE_FEATURE(RBD_FEATURE_OBJECT_MAP);
+
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  MockTestImageCtx mock_image_ctx(*ictx);
+  expect_op_work_queue(mock_image_ctx);
+
+  InSequence seq;
+  expect_is_refresh_required(mock_image_ctx, false);
+
+  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, -EINVAL);
+
+  C_SaferCond *acquire_ctx = new C_SaferCond();
+  C_SaferCond ctx;
+  MockPostAcquireRequest *req = MockPostAcquireRequest::create(mock_image_ctx,
+                                                               acquire_ctx,
+                                                               &ctx);
+  req->send();
+  ASSERT_EQ(-EINVAL, ctx.wait());
+  ASSERT_EQ(nullptr, mock_image_ctx.object_map);
+}
+
+TEST_F(TestMockExclusiveLockPostAcquireRequest, OpenObjectMapTooBig) {
+  REQUIRE_FEATURE(RBD_FEATURE_OBJECT_MAP);
 
   librbd::ImageCtx *ictx;
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
index ae8736a9dfb0882e53b05fe25af918a317d1e3ef..444077ee40cf3bfa6226eff889240bba1451032f 100644 (file)
@@ -1223,6 +1223,50 @@ TEST_F(TestMockImageRefreshRequest, OpenObjectMapError) {
   expect_test_features(mock_image_ctx);
   expect_is_exclusive_lock_owner(mock_exclusive_lock, true);
 
+  // object map should be immediately opened if exclusive lock owned
+  InSequence seq;
+  expect_get_mutable_metadata(mock_image_ctx, ictx->features, 0);
+  expect_get_metadata(mock_image_ctx, 0);
+  expect_apply_metadata(mock_image_ctx, 0);
+  expect_get_flags(mock_image_ctx, 0);
+  expect_get_group(mock_image_ctx, 0);
+  expect_refresh_parent_is_required(mock_refresh_parent_request, false);
+  expect_open_object_map(mock_image_ctx, mock_object_map, -EBLACKLISTED);
+
+  C_SaferCond ctx;
+  MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, false, false,
+                                                   &ctx);
+  req->send();
+
+  ASSERT_EQ(-EBLACKLISTED, ctx.wait());
+  ASSERT_EQ(nullptr, mock_image_ctx.object_map);
+}
+
+TEST_F(TestMockImageRefreshRequest, OpenObjectMapTooLarge) {
+  REQUIRE_FEATURE(RBD_FEATURE_OBJECT_MAP);
+
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  if (ictx->test_features(RBD_FEATURE_JOURNALING)) {
+    ASSERT_EQ(0, ictx->operations->update_features(RBD_FEATURE_JOURNALING,
+            false));
+  }
+
+  ASSERT_EQ(0, ictx->state->refresh());
+
+  MockRefreshImageCtx mock_image_ctx(*ictx);
+  MockRefreshParentRequest mock_refresh_parent_request;
+
+  MockExclusiveLock mock_exclusive_lock;
+  mock_image_ctx.exclusive_lock = &mock_exclusive_lock;
+
+  MockObjectMap *mock_object_map = new MockObjectMap();
+
+  expect_op_work_queue(mock_image_ctx);
+  expect_test_features(mock_image_ctx);
+  expect_is_exclusive_lock_owner(mock_exclusive_lock, true);
+
   // object map should be immediately opened if exclusive lock owned
   InSequence seq;
   expect_get_mutable_metadata(mock_image_ctx, ictx->features, 0);
@@ -1234,7 +1278,8 @@ TEST_F(TestMockImageRefreshRequest, OpenObjectMapError) {
   expect_open_object_map(mock_image_ctx, mock_object_map, -EFBIG);
 
   C_SaferCond ctx;
-  MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, false, false, &ctx);
+  MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, false, false,
+                                                   &ctx);
   req->send();
 
   ASSERT_EQ(0, ctx.wait());
index 1bd5d60946c5a062048c3acdbc475cf331920ccd..72ae152517264ca3bfb96d32c74fc7be573babab 100644 (file)
@@ -1272,6 +1272,40 @@ TEST_F(TestMockIoObjectRequest, CompareAndWriteMismatch) {
   ASSERT_EQ(1ULL, mismatch_offset);
 }
 
+TEST_F(TestMockIoObjectRequest, ObjectMapError) {
+  REQUIRE_FEATURE(RBD_FEATURE_OBJECT_MAP);
+
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  MockTestImageCtx mock_image_ctx(*ictx);
+  expect_op_work_queue(mock_image_ctx);
+  expect_get_object_size(mock_image_ctx);
+
+  MockExclusiveLock mock_exclusive_lock;
+  mock_image_ctx.exclusive_lock = &mock_exclusive_lock;
+  expect_is_lock_owner(mock_exclusive_lock);
+
+  MockObjectMap mock_object_map;
+  mock_image_ctx.object_map = &mock_object_map;
+
+  bufferlist bl;
+  bl.append(std::string(4096, '1'));
+
+  InSequence seq;
+  expect_get_parent_overlap(mock_image_ctx, CEPH_NOSNAP, 0, 0);
+  expect_object_may_exist(mock_image_ctx, 0, true);
+  expect_object_map_update(mock_image_ctx, 0, 1, OBJECT_EXISTS, {}, true,
+                           -EBLACKLISTED);
+
+  C_SaferCond ctx;
+  auto req = MockObjectWriteRequest::create_write(
+    &mock_image_ctx, ictx->get_object_name(0), 0, 0, std::move(bl),
+    mock_image_ctx.snapc, 0, {}, &ctx);
+  req->send();
+  ASSERT_EQ(-EBLACKLISTED, ctx.wait());
+}
+
 } // namespace io
 } // namespace librbd
 
index 1bae4e72e9fbe7def63f8ac6e097bc8f36003acf..72c44801ce7e35ecfcf14ebbfa61b90077c55601 100644 (file)
@@ -110,9 +110,10 @@ public:
   }
 
   void expect_invalidate_request(MockObjectMapImageCtx &mock_image_ctx,
-                                 MockInvalidateRequest &invalidate_request) {
+                                 MockInvalidateRequest &invalidate_request,
+                                 int r) {
     EXPECT_CALL(invalidate_request, send())
-                  .WillOnce(FinishRequest(&invalidate_request, 0,
+                  .WillOnce(FinishRequest(&invalidate_request, r,
                                           &mock_image_ctx));
   }
 
@@ -223,7 +224,7 @@ TEST_F(TestMockObjectMapRefreshRequest, LoadError) {
   expect_object_map_load(mock_image_ctx, nullptr, TEST_SNAP_ID, -ENOENT);
 
   MockInvalidateRequest invalidate_request;
-  expect_invalidate_request(mock_image_ctx, invalidate_request);
+  expect_invalidate_request(mock_image_ctx, invalidate_request, 0);
   expect_get_image_size(mock_image_ctx, TEST_SNAP_ID,
                         mock_image_ctx.image_ctx->size);
 
@@ -231,6 +232,36 @@ TEST_F(TestMockObjectMapRefreshRequest, LoadError) {
   ASSERT_EQ(0, ctx.wait());
 }
 
+TEST_F(TestMockObjectMapRefreshRequest, LoadInvalidateError) {
+  REQUIRE_FEATURE(RBD_FEATURE_OBJECT_MAP);
+
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  MockObjectMapImageCtx mock_image_ctx(*ictx);
+
+  ceph::BitVector<2> on_disk_object_map;
+  init_object_map(mock_image_ctx, &on_disk_object_map);
+
+  C_SaferCond ctx;
+  ceph::BitVector<2> object_map;
+  MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, &object_map,
+                                                   TEST_SNAP_ID, &ctx);
+
+  InSequence seq;
+  expect_get_image_size(mock_image_ctx, TEST_SNAP_ID,
+                        mock_image_ctx.image_ctx->size);
+  expect_object_map_load(mock_image_ctx, nullptr, TEST_SNAP_ID, -ENOENT);
+
+  MockInvalidateRequest invalidate_request;
+  expect_invalidate_request(mock_image_ctx, invalidate_request, -EPERM);
+  expect_get_image_size(mock_image_ctx, TEST_SNAP_ID,
+                        mock_image_ctx.image_ctx->size);
+
+  req->send();
+  ASSERT_EQ(-EPERM, ctx.wait());
+}
+
 TEST_F(TestMockObjectMapRefreshRequest, LoadCorrupt) {
   REQUIRE_FEATURE(RBD_FEATURE_OBJECT_MAP);
 
@@ -253,7 +284,7 @@ TEST_F(TestMockObjectMapRefreshRequest, LoadCorrupt) {
   expect_object_map_load(mock_image_ctx, nullptr, TEST_SNAP_ID, -EINVAL);
 
   MockInvalidateRequest invalidate_request;
-  expect_invalidate_request(mock_image_ctx, invalidate_request);
+  expect_invalidate_request(mock_image_ctx, invalidate_request, 0);
   expect_truncate_request(mock_image_ctx);
   expect_object_map_resize(mock_image_ctx, on_disk_object_map.size(), 0);
   expect_get_image_size(mock_image_ctx, TEST_SNAP_ID,
@@ -287,7 +318,7 @@ TEST_F(TestMockObjectMapRefreshRequest, TooSmall) {
   expect_object_map_load(mock_image_ctx, &small_object_map, TEST_SNAP_ID, 0);
 
   MockInvalidateRequest invalidate_request;
-  expect_invalidate_request(mock_image_ctx, invalidate_request);
+  expect_invalidate_request(mock_image_ctx, invalidate_request, 0);
   expect_object_map_resize(mock_image_ctx, on_disk_object_map.size(), 0);
   expect_get_image_size(mock_image_ctx, TEST_SNAP_ID,
                         mock_image_ctx.image_ctx->size);
@@ -296,6 +327,38 @@ TEST_F(TestMockObjectMapRefreshRequest, TooSmall) {
   ASSERT_EQ(0, ctx.wait());
 }
 
+TEST_F(TestMockObjectMapRefreshRequest, TooSmallInvalidateError) {
+  REQUIRE_FEATURE(RBD_FEATURE_OBJECT_MAP);
+
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  MockObjectMapImageCtx mock_image_ctx(*ictx);
+
+  ceph::BitVector<2> on_disk_object_map;
+  init_object_map(mock_image_ctx, &on_disk_object_map);
+
+  ceph::BitVector<2> small_object_map;
+
+  C_SaferCond ctx;
+  ceph::BitVector<2> object_map;
+  MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, &object_map,
+                                                   TEST_SNAP_ID, &ctx);
+
+  InSequence seq;
+  expect_get_image_size(mock_image_ctx, TEST_SNAP_ID,
+                        mock_image_ctx.image_ctx->size);
+  expect_object_map_load(mock_image_ctx, &small_object_map, TEST_SNAP_ID, 0);
+
+  MockInvalidateRequest invalidate_request;
+  expect_invalidate_request(mock_image_ctx, invalidate_request, -EPERM);
+  expect_get_image_size(mock_image_ctx, TEST_SNAP_ID,
+                        mock_image_ctx.image_ctx->size);
+
+  req->send();
+  ASSERT_EQ(-EPERM, ctx.wait());
+}
+
 TEST_F(TestMockObjectMapRefreshRequest, TooLarge) {
   REQUIRE_FEATURE(RBD_FEATURE_OBJECT_MAP);
 
@@ -349,7 +412,7 @@ TEST_F(TestMockObjectMapRefreshRequest, ResizeError) {
   expect_object_map_load(mock_image_ctx, &small_object_map, TEST_SNAP_ID, 0);
 
   MockInvalidateRequest invalidate_request;
-  expect_invalidate_request(mock_image_ctx, invalidate_request);
+  expect_invalidate_request(mock_image_ctx, invalidate_request, 0);
   expect_object_map_resize(mock_image_ctx, on_disk_object_map.size(), -ESTALE);
   expect_get_image_size(mock_image_ctx, TEST_SNAP_ID,
                         mock_image_ctx.image_ctx->size);
@@ -379,7 +442,7 @@ TEST_F(TestMockObjectMapRefreshRequest, LargeImageError) {
                         std::numeric_limits<int64_t>::max());
 
   MockInvalidateRequest invalidate_request;
-  expect_invalidate_request(mock_image_ctx, invalidate_request);
+  expect_invalidate_request(mock_image_ctx, invalidate_request, 0);
 
   req->send();
   ASSERT_EQ(-EFBIG, ctx.wait());
index ab2f8acce14cf1c56455c7aa9e58e788d213929f..72a518f43b64b5355696525b276e2766ac21df54 100644 (file)
@@ -188,10 +188,10 @@ public:
   }
 
   void expect_open_object_map(librbd::MockTestImageCtx &mock_image_ctx,
-                              librbd::MockObjectMap &mock_object_map) {
+                              librbd::MockObjectMap &mock_object_map, int r) {
     EXPECT_CALL(mock_object_map, open(_))
-      .WillOnce(Invoke([this](Context *ctx) {
-          m_work_queue->queue(ctx, 0);
+      .WillOnce(Invoke([this, r](Context *ctx) {
+          m_work_queue->queue(ctx, r);
         }));
   }
 
@@ -211,16 +211,17 @@ public:
   }
 
   void expect_copy_object_map(librbd::MockExclusiveLock &mock_exclusive_lock,
-                              librbd::MockObjectMap *mock_object_map) {
+                              librbd::MockObjectMap *mock_object_map, int r) {
     expect_start_op(mock_exclusive_lock);
-    expect_rollback_object_map(*mock_object_map, 0);
+    expect_rollback_object_map(*mock_object_map, r);
   }
 
   void expect_refresh_object_map(librbd::MockTestImageCtx &mock_image_ctx,
-                                 librbd::MockObjectMap *mock_object_map) {
+                                 librbd::MockObjectMap *mock_object_map,
+                                 int r) {
     expect_start_op(*mock_image_ctx.exclusive_lock);
     expect_create_object_map(mock_image_ctx, mock_object_map);
-    expect_open_object_map(mock_image_ctx, *mock_object_map);
+    expect_open_object_map(mock_image_ctx, *mock_object_map, r);
   }
 
   void expect_copy_metadata(MockMetadataCopyRequest &mock_metadata_copy_request,
@@ -253,7 +254,7 @@ TEST_F(TestMockDeepCopyRequest, SimpleCopy) {
   expect_copy_snapshots(mock_snapshot_copy_request, 0);
   expect_copy_image(mock_image_copy_request, 0);
   if (mock_object_map != nullptr) {
-    expect_refresh_object_map(mock_dst_image_ctx, mock_object_map);
+    expect_refresh_object_map(mock_dst_image_ctx, mock_object_map, 0);
   }
   expect_copy_metadata(mock_metadata_copy_request, 0);
 
@@ -285,6 +286,39 @@ TEST_F(TestMockDeepCopyRequest, ErrorOnCopySnapshots) {
   ASSERT_EQ(-EINVAL, ctx.wait());
 }
 
+TEST_F(TestMockDeepCopyRequest, ErrorOnRefreshObjectMap) {
+  REQUIRE_FEATURE(RBD_FEATURE_OBJECT_MAP);
+
+  librbd::MockTestImageCtx mock_src_image_ctx(*m_src_image_ctx);
+  librbd::MockTestImageCtx mock_dst_image_ctx(*m_dst_image_ctx);
+  MockImageCopyRequest mock_image_copy_request;
+  MockSnapshotCopyRequest mock_snapshot_copy_request;
+
+  librbd::MockExclusiveLock mock_exclusive_lock;
+  mock_dst_image_ctx.exclusive_lock = &mock_exclusive_lock;
+
+  librbd::MockObjectMap *mock_object_map = new librbd::MockObjectMap();
+  mock_dst_image_ctx.object_map = mock_object_map;
+
+  expect_test_features(mock_dst_image_ctx);
+
+  InSequence seq;
+  expect_copy_snapshots(mock_snapshot_copy_request, 0);
+  expect_copy_image(mock_image_copy_request, 0);
+  expect_start_op(*mock_dst_image_ctx.exclusive_lock);
+  expect_create_object_map(mock_dst_image_ctx, mock_object_map);
+  expect_open_object_map(mock_dst_image_ctx, *mock_object_map, -EINVAL);
+
+  C_SaferCond ctx;
+  librbd::SnapSeqs snap_seqs;
+  librbd::NoOpProgressContext no_op;
+  auto request = librbd::DeepCopyRequest<librbd::MockTestImageCtx>::create(
+      &mock_src_image_ctx, &mock_dst_image_ctx, 0, CEPH_NOSNAP, false,
+      boost::none, m_work_queue, &snap_seqs, &no_op, &ctx);
+  request->send();
+  ASSERT_EQ(-EINVAL, ctx.wait());
+}
+
 TEST_F(TestMockDeepCopyRequest, ErrorOnCopyImage) {
   librbd::MockTestImageCtx mock_src_image_ctx(*m_src_image_ctx);
   librbd::MockTestImageCtx mock_dst_image_ctx(*m_dst_image_ctx);
@@ -326,7 +360,7 @@ TEST_F(TestMockDeepCopyRequest, ErrorOnCopyMetadata) {
   expect_copy_snapshots(mock_snapshot_copy_request, 0);
   expect_copy_image(mock_image_copy_request, 0);
   if (mock_object_map != nullptr) {
-    expect_refresh_object_map(mock_dst_image_ctx, mock_object_map);
+    expect_refresh_object_map(mock_dst_image_ctx, mock_object_map, 0);
   }
   expect_copy_metadata(mock_metadata_copy_request, -EINVAL);
 
@@ -366,8 +400,8 @@ TEST_F(TestMockDeepCopyRequest, Snap) {
   expect_copy_snapshots(mock_snapshot_copy_request, 0);
   expect_copy_image(mock_image_copy_request, 0);
   if (mock_object_map != nullptr) {
-    expect_copy_object_map(mock_exclusive_lock, mock_object_map);
-    expect_refresh_object_map(mock_dst_image_ctx, mock_object_map);
+    expect_copy_object_map(mock_exclusive_lock, mock_object_map, 0);
+    expect_refresh_object_map(mock_dst_image_ctx, mock_object_map, 0);
   }
   expect_copy_metadata(mock_metadata_copy_request, 0);
 
@@ -380,3 +414,40 @@ TEST_F(TestMockDeepCopyRequest, Snap) {
   request->send();
   ASSERT_EQ(0, ctx.wait());
 }
+
+TEST_F(TestMockDeepCopyRequest, ErrorOnRollbackObjectMap) {
+  REQUIRE_FEATURE(RBD_FEATURE_OBJECT_MAP);
+
+  EXPECT_EQ(0, snap_create(*m_src_image_ctx, "copy"));
+  EXPECT_EQ(0, librbd::api::Image<>::snap_set(m_src_image_ctx,
+                                              cls::rbd::UserSnapshotNamespace(),
+                                              "copy"));
+
+  librbd::MockTestImageCtx mock_src_image_ctx(*m_src_image_ctx);
+  librbd::MockTestImageCtx mock_dst_image_ctx(*m_dst_image_ctx);
+  MockImageCopyRequest mock_image_copy_request;
+  MockMetadataCopyRequest mock_metadata_copy_request;
+  MockSnapshotCopyRequest mock_snapshot_copy_request;
+
+  librbd::MockExclusiveLock mock_exclusive_lock;
+  mock_dst_image_ctx.exclusive_lock = &mock_exclusive_lock;
+
+  librbd::MockObjectMap mock_object_map;
+  mock_dst_image_ctx.object_map = &mock_object_map;
+
+  expect_test_features(mock_dst_image_ctx);
+
+  InSequence seq;
+  expect_copy_snapshots(mock_snapshot_copy_request, 0);
+  expect_copy_image(mock_image_copy_request, 0);
+  expect_copy_object_map(mock_exclusive_lock, &mock_object_map, -EINVAL);
+
+  C_SaferCond ctx;
+  librbd::SnapSeqs snap_seqs = {{m_src_image_ctx->snap_id, 123}};
+  librbd::NoOpProgressContext no_op;
+  auto request = librbd::DeepCopyRequest<librbd::MockTestImageCtx>::create(
+      &mock_src_image_ctx, &mock_dst_image_ctx, 0, m_src_image_ctx->snap_id,
+      false, boost::none, m_work_queue, &snap_seqs, &no_op, &ctx);
+  request->send();
+  ASSERT_EQ(-EINVAL, ctx.wait());
+}