From: Jason Dillaman Date: Tue, 18 Sep 2018 18:37:12 +0000 (-0400) Subject: librbd: properly handle potential object map failures X-Git-Tag: v14.0.1~210^2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=765f8ce2536b315a046d0aceff234e9e3c66271f;p=ceph.git librbd: properly handle potential object map failures 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 --- diff --git a/src/librbd/DeepCopyRequest.cc b/src/librbd/DeepCopyRequest.cc index a522ceee246aa..45191158e38c4 100644 --- a/src/librbd/DeepCopyRequest.cc +++ b/src/librbd/DeepCopyRequest.cc @@ -230,7 +230,13 @@ template void DeepCopyRequest::handle_copy_object_map(int r) { ldout(m_cct, 20) << dendl; - ceph_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(); } @@ -264,9 +270,18 @@ template void DeepCopyRequest::handle_refresh_object_map(int r) { ldout(m_cct, 20) << "r=" << r << dendl; - ceph_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; diff --git a/src/librbd/deep_copy/ObjectCopyRequest.cc b/src/librbd/deep_copy/ObjectCopyRequest.cc index 9f1ff8ac43343..78ef97fb47163 100644 --- a/src/librbd/deep_copy/ObjectCopyRequest.cc +++ b/src/librbd/deep_copy/ObjectCopyRequest.cc @@ -487,7 +487,12 @@ template void ObjectCopyRequest::handle_update_object_map(int r) { ldout(m_cct, 20) << "r=" << r << dendl; - ceph_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; diff --git a/src/librbd/deep_copy/SnapshotCopyRequest.cc b/src/librbd/deep_copy/SnapshotCopyRequest.cc index b8947d733f74f..db24ca72ecf3b 100644 --- a/src/librbd/deep_copy/SnapshotCopyRequest.cc +++ b/src/librbd/deep_copy/SnapshotCopyRequest.cc @@ -598,7 +598,13 @@ template void SnapshotCopyRequest::handle_resize_object_map(int r) { ldout(m_cct, 20) << "r=" << r << dendl; - ceph_assert(r == 0); + if (r < 0) { + lderr(m_cct) << "failed to resize object map: " << cpp_strerror(r) + << dendl; + finish(r); + return; + } + finish(0); } diff --git a/src/librbd/exclusive_lock/PostAcquireRequest.cc b/src/librbd/exclusive_lock/PostAcquireRequest.cc index bea877427acc7..7e67e41c2b6e5 100644 --- a/src/librbd/exclusive_lock/PostAcquireRequest.cc +++ b/src/librbd/exclusive_lock/PostAcquireRequest.cc @@ -225,10 +225,15 @@ void PostAcquireRequest::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::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 - ceph_assert(r == 0); + if (r < 0) { + lderr(cct) << "failed to close object map: " << cpp_strerror(r) << dendl; + } + revert(); finish(); } diff --git a/src/librbd/exclusive_lock/PreReleaseRequest.cc b/src/librbd/exclusive_lock/PreReleaseRequest.cc index 7c663823e6a7a..7dbae6c5992e4 100644 --- a/src/librbd/exclusive_lock/PreReleaseRequest.cc +++ b/src/librbd/exclusive_lock/PreReleaseRequest.cc @@ -271,10 +271,11 @@ void PreReleaseRequest::handle_close_object_map(int r) { CephContext *cct = m_image_ctx.cct; ldout(cct, 10) << "r=" << r << dendl; - // object map shouldn't return errors - ceph_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(); } diff --git a/src/librbd/image/RefreshRequest.cc b/src/librbd/image/RefreshRequest.cc index 3c4212f005fb0..7fc57c06fae41 100644 --- a/src/librbd/image/RefreshRequest.cc +++ b/src/librbd/image/RefreshRequest.cc @@ -993,6 +993,10 @@ Context *RefreshRequest::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(); @@ -1151,7 +1155,11 @@ Context *RefreshRequest::handle_v2_close_object_map(int *result) { CephContext *cct = m_image_ctx.cct; ldout(cct, 10) << this << " " << __func__ << ": r=" << *result << dendl; - ceph_assert(*result == 0); + if (*result < 0) { + lderr(cct) << "failed to close object map: " << cpp_strerror(*result) + << dendl; + } + ceph_assert(m_object_map != nullptr); delete m_object_map; m_object_map = nullptr; diff --git a/src/librbd/io/CopyupRequest.cc b/src/librbd/io/CopyupRequest.cc index c3026b1e4dd1a..dbb3f0f89e5d6 100644 --- a/src/librbd/io/CopyupRequest.cc +++ b/src/librbd/io/CopyupRequest.cc @@ -302,12 +302,22 @@ bool CopyupRequest::should_complete(int *r) { case STATE_OBJECT_MAP_HEAD: ldout(cct, 20) << "OBJECT_MAP_HEAD" << dendl; - ceph_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; - ceph_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; diff --git a/src/librbd/io/ObjectRequest.cc b/src/librbd/io/ObjectRequest.cc index b1cb5f6764043..1250e03cdff7c 100644 --- a/src/librbd/io/ObjectRequest.cc +++ b/src/librbd/io/ObjectRequest.cc @@ -474,8 +474,13 @@ template void AbstractObjectWriteRequest::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; + } - ceph_assert(r == 0); write_object(); } @@ -621,8 +626,13 @@ template void AbstractObjectWriteRequest::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; + } - ceph_assert(r == 0); this->finish(0); } diff --git a/src/librbd/object_map/InvalidateRequest.cc b/src/librbd/object_map/InvalidateRequest.cc index 754b090201861..9f7a2fa10140f 100644 --- a/src/librbd/object_map/InvalidateRequest.cc +++ b/src/librbd/object_map/InvalidateRequest.cc @@ -53,7 +53,7 @@ void InvalidateRequest::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::send() { librados::AioCompletion *rados_completion = this->create_callback_completion(); r = image_ctx.md_ctx.aio_operate(image_ctx.header_oid, rados_completion, - &op); + &op); ceph_assert(r == 0); rados_completion->release(); } diff --git a/src/librbd/object_map/RefreshRequest.cc b/src/librbd/object_map/RefreshRequest.cc index c668fc2305e65..c4fc2539c05dd 100644 --- a/src/librbd/object_map/RefreshRequest.cc +++ b/src/librbd/object_map/RefreshRequest.cc @@ -181,7 +181,11 @@ Context *RefreshRequest::handle_invalidate(int *ret_val) { CephContext *cct = m_image_ctx.cct; ldout(cct, 10) << this << " " << __func__ << ": r=" << *ret_val << dendl; - ceph_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::handle_resize_invalidate(int *ret_val) { CephContext *cct = m_image_ctx.cct; ldout(cct, 10) << this << " " << __func__ << ": r=" << *ret_val << dendl; - ceph_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::handle_resize(int *ret_val) { << dendl; *ret_val = 0; } + apply(); return m_on_finish; } @@ -275,9 +286,13 @@ Context *RefreshRequest::handle_invalidate_and_close(int *ret_val) { CephContext *cct = m_image_ctx.cct; ldout(cct, 10) << this << " " << __func__ << ": r=" << *ret_val << dendl; - ceph_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; } diff --git a/src/librbd/object_map/Request.h b/src/librbd/object_map/Request.h index 67f9bac11e5d7..aef8800dd8ace 100644 --- a/src/librbd/object_map/Request.h +++ b/src/librbd/object_map/Request.h @@ -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() { } diff --git a/src/librbd/object_map/SnapshotRemoveRequest.cc b/src/librbd/object_map/SnapshotRemoveRequest.cc index 932b7095bf879..54f67c8d76d0a 100644 --- a/src/librbd/object_map/SnapshotRemoveRequest.cc +++ b/src/librbd/object_map/SnapshotRemoveRequest.cc @@ -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; - ceph_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(); } diff --git a/src/librbd/operation/ResizeRequest.cc b/src/librbd/operation/ResizeRequest.cc index 69e50e5d7c73d..9cf9c648ba8e7 100644 --- a/src/librbd/operation/ResizeRequest.cc +++ b/src/librbd/operation/ResizeRequest.cc @@ -295,7 +295,12 @@ Context *ResizeRequest::handle_grow_object_map(int *result) { CephContext *cct = image_ctx.cct; ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl; - ceph_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; } @@ -337,8 +342,14 @@ Context *ResizeRequest::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(); - ceph_assert(*result == 0); return this->create_context_finisher(0); } diff --git a/src/librbd/operation/SnapshotCreateRequest.cc b/src/librbd/operation/SnapshotCreateRequest.cc index e4cbd4000bc09..ab53d8ed0e9d4 100644 --- a/src/librbd/operation/SnapshotCreateRequest.cc +++ b/src/librbd/operation/SnapshotCreateRequest.cc @@ -238,9 +238,13 @@ Context *SnapshotCreateRequest::handle_create_object_map(int *result) { CephContext *cct = image_ctx.cct; ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl; - ceph_assert(*result == 0); - image_ctx.io_work_queue->unblock_writes(); + if (*result < 0) { + lderr(cct) << this << " " << __func__ << ": failed to snapshot object map: " + << cpp_strerror(*result) << dendl; + return this->create_context_finisher(*result); + } + return this->create_context_finisher(0); } diff --git a/src/librbd/operation/SnapshotRollbackRequest.cc b/src/librbd/operation/SnapshotRollbackRequest.cc index 0cd85463bf7e5..006dc4bb761c2 100644 --- a/src/librbd/operation/SnapshotRollbackRequest.cc +++ b/src/librbd/operation/SnapshotRollbackRequest.cc @@ -222,7 +222,13 @@ Context *SnapshotRollbackRequest::handle_get_snap_object_map(int *result) { CephContext *cct = image_ctx.cct; ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl; - ceph_assert(*result == 0); + if (*result < 0) { + lderr(cct) << this << " " << __func__ << ": failed to open object map: " + << cpp_strerror(*result) << dendl; + delete m_snap_object_map; + m_snap_object_map = nullptr; + } + send_rollback_object_map(); return nullptr; } @@ -256,7 +262,15 @@ Context *SnapshotRollbackRequest::handle_rollback_object_map(int *result) { CephContext *cct = image_ctx.cct; ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl; - ceph_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; } @@ -337,7 +351,16 @@ Context *SnapshotRollbackRequest::handle_refresh_object_map(int *result) { CephContext *cct = image_ctx.cct; ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl; - ceph_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(); } diff --git a/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc b/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc index aefac1279dddb..f004291b820a9 100644 --- a/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc +++ b/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc @@ -853,5 +853,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 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 diff --git a/src/test/librbd/exclusive_lock/test_mock_PostAcquireRequest.cc b/src/test/librbd/exclusive_lock/test_mock_PostAcquireRequest.cc index b9f43a0c9b352..82e079c7c9a6f 100644 --- a/src/test/librbd/exclusive_lock/test_mock_PostAcquireRequest.cc +++ b/src/test/librbd/exclusive_lock/test_mock_PostAcquireRequest.cc @@ -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)); diff --git a/src/test/librbd/image/test_mock_RefreshRequest.cc b/src/test/librbd/image/test_mock_RefreshRequest.cc index 24cb22602f9f1..a76f8acb8f902 100644 --- a/src/test/librbd/image/test_mock_RefreshRequest.cc +++ b/src/test/librbd/image/test_mock_RefreshRequest.cc @@ -1276,6 +1276,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_parent(mock_image_ctx, 0); + expect_get_metadata(mock_image_ctx, 0); + expect_apply_metadata(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); @@ -1287,7 +1331,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()); diff --git a/src/test/librbd/io/test_mock_ObjectRequest.cc b/src/test/librbd/io/test_mock_ObjectRequest.cc index 27eff02bd95aa..0a4aeda733f14 100644 --- a/src/test/librbd/io/test_mock_ObjectRequest.cc +++ b/src/test/librbd/io/test_mock_ObjectRequest.cc @@ -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 diff --git a/src/test/librbd/object_map/test_mock_RefreshRequest.cc b/src/test/librbd/object_map/test_mock_RefreshRequest.cc index 39bfab6756405..60cc579a8cf53 100644 --- a/src/test/librbd/object_map/test_mock_RefreshRequest.cc +++ b/src/test/librbd/object_map/test_mock_RefreshRequest.cc @@ -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::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()); diff --git a/src/test/librbd/test_mock_DeepCopyRequest.cc b/src/test/librbd/test_mock_DeepCopyRequest.cc index fd45e9f0e5d4f..00c457bf0a8cb 100644 --- a/src/test/librbd/test_mock_DeepCopyRequest.cc +++ b/src/test/librbd/test_mock_DeepCopyRequest.cc @@ -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::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::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()); +}