From: Jason Dillaman Date: Wed, 4 May 2016 19:16:17 +0000 (-0400) Subject: librbd: properly handle object map open returning error code X-Git-Tag: v10.2.1~17^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=7448d29027dafe1abd1e6ebff6f0fe5bb15e4b07;p=ceph.git librbd: properly handle object map open returning error code Signed-off-by: Jason Dillaman (cherry picked from commit 570de56bfc7fd2edd5cf1cf5b7b92084cd3352a3) --- diff --git a/src/librbd/exclusive_lock/AcquireRequest.cc b/src/librbd/exclusive_lock/AcquireRequest.cc index 4008c827a1d2..4a4ee6d05112 100644 --- a/src/librbd/exclusive_lock/AcquireRequest.cc +++ b/src/librbd/exclusive_lock/AcquireRequest.cc @@ -250,8 +250,15 @@ Context *AcquireRequest::handle_open_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); + if (*ret_val < 0) { + lderr(cct) << "failed to open object map: " << cpp_strerror(*ret_val) + << dendl; + + *ret_val = 0; + delete m_object_map; + m_object_map = nullptr; + } + return send_open_journal(); } diff --git a/src/librbd/image/RefreshRequest.cc b/src/librbd/image/RefreshRequest.cc index b499b0c86953..dedf3073832a 100644 --- a/src/librbd/image/RefreshRequest.cc +++ b/src/librbd/image/RefreshRequest.cc @@ -607,7 +607,13 @@ Context *RefreshRequest::handle_v2_open_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 open object map: " << cpp_strerror(*result) + << dendl; + delete m_object_map; + m_object_map = nullptr; + } + send_v2_open_journal(); return nullptr; } diff --git a/src/librbd/image/SetSnapRequest.cc b/src/librbd/image/SetSnapRequest.cc index f30f4a97fe75..44da6730bd7b 100644 --- a/src/librbd/image/SetSnapRequest.cc +++ b/src/librbd/image/SetSnapRequest.cc @@ -272,8 +272,12 @@ Context *SetSnapRequest::handle_open_object_map(int *result) { CephContext *cct = m_image_ctx.cct; ldout(cct, 10) << __func__ << ": r=" << *result << dendl; - // object map should never report errors - assert(*result == 0); + if (*result < 0) { + lderr(cct) << "failed to open object map: " << cpp_strerror(*result) + << dendl; + delete m_object_map; + m_object_map = nullptr; + } *result = apply(); if (*result < 0) { diff --git a/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc b/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc index b5eef00b650f..e4ba0ec81e71 100644 --- a/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc +++ b/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc @@ -65,9 +65,9 @@ public: } void expect_open_object_map(MockImageCtx &mock_image_ctx, - MockObjectMap &mock_object_map) { + MockObjectMap &mock_object_map, int r) { EXPECT_CALL(mock_object_map, open(_)) - .WillOnce(CompleteContext(0, mock_image_ctx.image_ctx->op_work_queue)); + .WillOnce(CompleteContext(r, mock_image_ctx.image_ctx->op_work_queue)); } void expect_close_object_map(MockImageCtx &mock_image_ctx, @@ -192,7 +192,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, Success) { 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_open_object_map(mock_image_ctx, mock_object_map, 0); MockJournal mock_journal; MockJournalPolicy mock_journal_policy; @@ -228,7 +228,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, SuccessJournalDisabled) { 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_open_object_map(mock_image_ctx, mock_object_map, 0); expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, false); @@ -291,7 +291,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, JournalError) { 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_open_object_map(mock_image_ctx, *mock_object_map, 0); MockJournal *mock_journal = new MockJournal(); expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, true); @@ -326,7 +326,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, AllocateJournalTagError) { 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_open_object_map(mock_image_ctx, *mock_object_map, 0); MockJournal *mock_journal = new MockJournal(); MockJournalPolicy mock_journal_policy; @@ -653,5 +653,42 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BreakLockError) { ASSERT_EQ(-EINVAL, ctx.wait()); } +TEST_F(TestMockExclusiveLockAcquireRequest, OpenObjectMapError) { + 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_flush_notifies(mock_image_ctx); + 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, -EFBIG); + + MockJournal mock_journal; + MockJournalPolicy mock_journal_policy; + 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_get_journal_policy(mock_image_ctx, mock_journal_policy); + expect_allocate_journal_tag(mock_image_ctx, mock_journal_policy, 0); + + C_SaferCond acquire_ctx; + C_SaferCond ctx; + MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx, + TEST_COOKIE, + &acquire_ctx, &ctx); + req->send(); + ASSERT_EQ(0, acquire_ctx.wait()); + ASSERT_EQ(0, ctx.wait()); + ASSERT_EQ(nullptr, mock_image_ctx.object_map); +} + } // namespace exclusive_lock } // namespace librbd diff --git a/src/test/librbd/image/test_mock_RefreshRequest.cc b/src/test/librbd/image/test_mock_RefreshRequest.cc index 6af5f46f46c1..01a738106462 100644 --- a/src/test/librbd/image/test_mock_RefreshRequest.cc +++ b/src/test/librbd/image/test_mock_RefreshRequest.cc @@ -261,10 +261,10 @@ public: } void expect_open_object_map(MockRefreshImageCtx &mock_image_ctx, - MockObjectMap &mock_object_map, int r) { + MockObjectMap *mock_object_map, int r) { EXPECT_CALL(mock_image_ctx, create_object_map(_)) - .WillOnce(Return(&mock_object_map)); - EXPECT_CALL(mock_object_map, open(_)) + .WillOnce(Return(mock_object_map)); + EXPECT_CALL(*mock_object_map, open(_)) .WillOnce(CompleteContext(r, mock_image_ctx.image_ctx->op_work_queue)); } @@ -419,7 +419,7 @@ TEST_F(TestMockImageRefreshRequest, SuccessSetSnapshotV2) { expect_get_snapshots(mock_image_ctx, 0); expect_refresh_parent_is_required(mock_refresh_parent_request, false); if (ictx->test_features(RBD_FEATURE_OBJECT_MAP)) { - expect_open_object_map(mock_image_ctx, mock_object_map, 0); + expect_open_object_map(mock_image_ctx, &mock_object_map, 0); } expect_add_snap(mock_image_ctx, "snap", ictx->snap_ids.begin()->second); expect_get_snap_id(mock_image_ctx, "snap", ictx->snap_ids.begin()->second); @@ -664,7 +664,7 @@ TEST_F(TestMockImageRefreshRequest, EnableObjectMapWithExclusiveLock) { expect_get_mutable_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); expect_refresh_parent_is_required(mock_refresh_parent_request, false); - expect_open_object_map(mock_image_ctx, mock_object_map, 0); + expect_open_object_map(mock_image_ctx, &mock_object_map, 0); C_SaferCond ctx; MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, &ctx); @@ -745,5 +745,40 @@ TEST_F(TestMockImageRefreshRequest, DisableObjectMap) { ASSERT_EQ(0, ctx.wait()); } +TEST_F(TestMockImageRefreshRequest, OpenObjectMapError) { + REQUIRE_FEATURE(RBD_FEATURE_OBJECT_MAP); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_JOURNALING, false)); + + 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, 0); + expect_get_flags(mock_image_ctx, 0); + expect_refresh_parent_is_required(mock_refresh_parent_request, false); + expect_open_object_map(mock_image_ctx, mock_object_map, -EFBIG); + + C_SaferCond ctx; + MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, &ctx); + req->send(); + + ASSERT_EQ(0, ctx.wait()); + ASSERT_EQ(nullptr, mock_image_ctx.object_map); +} + } // namespace image } // namespace librbd