From: Jason Dillaman Date: Wed, 31 May 2017 14:22:26 +0000 (-0400) Subject: rbd-mirror: ensure exclusive lock cannot be released with in-flight IO X-Git-Tag: ses5-milestone6~8^2~5^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F15140%2Fhead;p=ceph.git rbd-mirror: ensure exclusive lock cannot be released with in-flight IO Signed-off-by: Jason Dillaman --- diff --git a/src/test/rbd_mirror/image_sync/test_mock_ImageCopyRequest.cc b/src/test/rbd_mirror/image_sync/test_mock_ImageCopyRequest.cc index 81c0ea931b15..4fbf13c6b4fa 100644 --- a/src/test/rbd_mirror/image_sync/test_mock_ImageCopyRequest.cc +++ b/src/test/rbd_mirror/image_sync/test_mock_ImageCopyRequest.cc @@ -87,6 +87,7 @@ using ::testing::_; using ::testing::InSequence; using ::testing::Invoke; using ::testing::Return; +using ::testing::ReturnNew; using ::testing::WithArg; using ::testing::InvokeWithoutArgs; @@ -106,6 +107,11 @@ public: ASSERT_EQ(0, open_image(m_local_io_ctx, m_image_name, &m_local_image_ctx)); } + void expect_start_op(librbd::MockExclusiveLock &mock_exclusive_lock) { + EXPECT_CALL(mock_exclusive_lock, start_op()).WillOnce( + ReturnNew([](int) {})); + } + void expect_get_snap_id(librbd::MockTestImageCtx &mock_image_ctx) { EXPECT_CALL(mock_image_ctx, get_snap_id(_, _)) .WillRepeatedly(Invoke([&mock_image_ctx](cls::rbd::SnapshotNamespace snap_namespace, @@ -216,12 +222,16 @@ TEST_F(TestMockImageSyncImageCopyRequest, SimpleImage) { journal::MockJournaler mock_journaler; MockObjectCopyRequest mock_object_copy_request; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + expect_get_snap_id(mock_remote_image_ctx); InSequence seq; expect_get_object_count(mock_remote_image_ctx, 1); expect_get_object_count(mock_remote_image_ctx, 0); expect_update_client(mock_journaler, 0); + expect_start_op(mock_exclusive_lock); expect_object_copy_send(mock_object_copy_request); expect_update_client(mock_journaler, 0); @@ -263,11 +273,16 @@ TEST_F(TestMockImageSyncImageCopyRequest, Throttled) { journal::MockJournaler mock_journaler; MockObjectCopyRequest mock_object_copy_request; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + expect_get_snap_id(mock_remote_image_ctx); expect_get_object_count(mock_remote_image_ctx, object_count); expect_get_object_count(mock_remote_image_ctx, 0); + EXPECT_CALL(mock_exclusive_lock, start_op()) + .Times(object_count).WillRepeatedly(ReturnNew([](int) {})); EXPECT_CALL(mock_object_copy_request, send()).Times(object_count); boost::optional expected_object_number(boost::none); @@ -326,6 +341,9 @@ TEST_F(TestMockImageSyncImageCopyRequest, SnapshotSubset) { journal::MockJournaler mock_journaler; MockObjectCopyRequest mock_object_copy_request; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + expect_get_snap_id(mock_remote_image_ctx); InSequence seq; @@ -334,6 +352,7 @@ TEST_F(TestMockImageSyncImageCopyRequest, SnapshotSubset) { expect_get_object_count(mock_remote_image_ctx, 1); expect_get_object_count(mock_remote_image_ctx, 1); expect_update_client(mock_journaler, 0); + expect_start_op(mock_exclusive_lock); expect_object_copy_send(mock_object_copy_request); expect_update_client(mock_journaler, 0); @@ -364,6 +383,9 @@ TEST_F(TestMockImageSyncImageCopyRequest, RestartCatchup) { journal::MockJournaler mock_journaler; MockObjectCopyRequest mock_object_copy_request; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + expect_get_snap_id(mock_remote_image_ctx); InSequence seq; @@ -371,6 +393,7 @@ TEST_F(TestMockImageSyncImageCopyRequest, RestartCatchup) { expect_get_object_count(mock_remote_image_ctx, 0); expect_get_object_count(mock_remote_image_ctx, 0); expect_update_client(mock_journaler, 0); + expect_start_op(mock_exclusive_lock); expect_object_copy_send(mock_object_copy_request); expect_update_client(mock_journaler, 0); @@ -398,12 +421,16 @@ TEST_F(TestMockImageSyncImageCopyRequest, RestartPartialSync) { journal::MockJournaler mock_journaler; MockObjectCopyRequest mock_object_copy_request; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + expect_get_snap_id(mock_remote_image_ctx); InSequence seq; expect_get_object_count(mock_remote_image_ctx, 1); expect_get_object_count(mock_remote_image_ctx, 2); expect_update_client(mock_journaler, 0); + expect_start_op(mock_exclusive_lock); expect_object_copy_send(mock_object_copy_request); expect_update_client(mock_journaler, 0); @@ -435,12 +462,16 @@ TEST_F(TestMockImageSyncImageCopyRequest, Cancel) { journal::MockJournaler mock_journaler; MockObjectCopyRequest mock_object_copy_request; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + expect_get_snap_id(mock_remote_image_ctx); InSequence seq; expect_get_object_count(mock_remote_image_ctx, 2); expect_get_object_count(mock_remote_image_ctx, 2); expect_update_client(mock_journaler, 0); + expect_start_op(mock_exclusive_lock); expect_object_copy_send(mock_object_copy_request); C_SaferCond ctx; @@ -481,11 +512,16 @@ TEST_F(TestMockImageSyncImageCopyRequest, Cancel_Inflight_Sync) { journal::MockJournaler mock_journaler; MockObjectCopyRequest mock_object_copy_request; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + expect_get_snap_id(mock_remote_image_ctx); expect_get_object_count(mock_remote_image_ctx, 10); expect_get_object_count(mock_remote_image_ctx, 0); + EXPECT_CALL(mock_exclusive_lock, start_op()) + .Times(6).WillRepeatedly(ReturnNew([](int) {})); EXPECT_CALL(mock_object_copy_request, send()).Times(6); EXPECT_CALL(mock_journaler, update_client(_, _)) @@ -529,6 +565,9 @@ TEST_F(TestMockImageSyncImageCopyRequest, Cancel1) { journal::MockJournaler mock_journaler; MockObjectCopyRequest mock_object_copy_request; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + C_SaferCond ctx; MockImageCopyRequest *request = create_request(mock_remote_image_ctx, mock_local_image_ctx, @@ -559,6 +598,9 @@ TEST_F(TestMockImageSyncImageCopyRequest, MissingSnap) { librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); journal::MockJournaler mock_journaler; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + expect_get_snap_id(mock_remote_image_ctx); C_SaferCond ctx; @@ -582,6 +624,9 @@ TEST_F(TestMockImageSyncImageCopyRequest, MissingFromSnap) { librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); journal::MockJournaler mock_journaler; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + expect_get_snap_id(mock_remote_image_ctx); C_SaferCond ctx; @@ -607,6 +652,9 @@ TEST_F(TestMockImageSyncImageCopyRequest, EmptySnapMap) { librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); journal::MockJournaler mock_journaler; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + expect_get_snap_id(mock_remote_image_ctx); C_SaferCond ctx; @@ -632,6 +680,9 @@ TEST_F(TestMockImageSyncImageCopyRequest, EmptySnapSeqs) { librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); journal::MockJournaler mock_journaler; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + expect_get_snap_id(mock_remote_image_ctx); C_SaferCond ctx; diff --git a/src/test/rbd_mirror/image_sync/test_mock_SnapshotCopyRequest.cc b/src/test/rbd_mirror/image_sync/test_mock_SnapshotCopyRequest.cc index 0d019e5ce096..7801b66bd9ce 100644 --- a/src/test/rbd_mirror/image_sync/test_mock_SnapshotCopyRequest.cc +++ b/src/test/rbd_mirror/image_sync/test_mock_SnapshotCopyRequest.cc @@ -85,6 +85,7 @@ using ::testing::InSequence; using ::testing::Invoke; using ::testing::InvokeWithoutArgs; using ::testing::Return; +using ::testing::ReturnNew; using ::testing::SetArgPointee; using ::testing::StrEq; using ::testing::WithArg; @@ -105,6 +106,11 @@ public: ASSERT_EQ(0, open_image(m_local_io_ctx, m_image_name, &m_local_image_ctx)); } + void expect_start_op(librbd::MockExclusiveLock &mock_exclusive_lock) { + EXPECT_CALL(mock_exclusive_lock, start_op()).WillOnce( + ReturnNew([](int) {})); + } + void expect_get_snap_namespace(librbd::MockTestImageCtx &mock_image_ctx, uint64_t snap_id) { EXPECT_CALL(mock_image_ctx, get_snap_namespace(snap_id, _)) @@ -226,6 +232,9 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, Empty) { librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); journal::MockJournaler mock_journaler; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + InSequence seq; expect_update_client(mock_journaler, 0); @@ -245,6 +254,9 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, UpdateClientError) { librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); journal::MockJournaler mock_journaler; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + InSequence seq; expect_update_client(mock_journaler, -EINVAL); @@ -261,6 +273,9 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, UpdateClientCancel) { librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); journal::MockJournaler mock_journaler; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + C_SaferCond ctx; MockSnapshotCopyRequest *request = create_request(mock_remote_image_ctx, mock_local_image_ctx, @@ -280,20 +295,25 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapCreate) { ASSERT_EQ(0, create_snap(m_remote_image_ctx, "snap1")); ASSERT_EQ(0, create_snap(m_remote_image_ctx, "snap2")); - uint64_t remote_snap_id1 = m_remote_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}]; - uint64_t remote_snap_id2 = m_remote_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap2"}]; + uint64_t remote_snap_id1 = m_remote_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}]; + uint64_t remote_snap_id2 = m_remote_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap2"}]; librbd::MockTestImageCtx mock_remote_image_ctx(*m_remote_image_ctx); librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); MockSnapshotCreateRequest mock_snapshot_create_request; journal::MockJournaler mock_journaler; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + InSequence seq; expect_get_snap_namespace(mock_remote_image_ctx, remote_snap_id1); + expect_start_op(mock_exclusive_lock); expect_snap_create(mock_local_image_ctx, mock_snapshot_create_request, "snap1", 12, 0); expect_get_snap_namespace(mock_remote_image_ctx, remote_snap_id2); + expect_start_op(mock_exclusive_lock); expect_snap_create(mock_local_image_ctx, mock_snapshot_create_request, "snap2", 14, 0); expect_snap_is_protected(mock_remote_image_ctx, remote_snap_id1, false, 0); expect_snap_is_protected(mock_remote_image_ctx, remote_snap_id2, false, 0); @@ -318,10 +338,14 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapCreateError) { MockSnapshotCreateRequest mock_snapshot_create_request; journal::MockJournaler mock_journaler; - uint64_t remote_snap_id1 = mock_remote_image_ctx.snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}]; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + + uint64_t remote_snap_id1 = mock_remote_image_ctx.snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}]; InSequence seq; expect_get_snap_namespace(mock_remote_image_ctx, remote_snap_id1); + expect_start_op(mock_exclusive_lock); expect_snap_create(mock_local_image_ctx, mock_snapshot_create_request, "snap1", 12, -EINVAL); C_SaferCond ctx; @@ -340,8 +364,11 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapCreateCancel) { MockSnapshotCreateRequest mock_snapshot_create_request; journal::MockJournaler mock_journaler; - uint64_t remote_snap_id1 = m_remote_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}]; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + + uint64_t remote_snap_id1 = m_remote_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}]; expect_get_snap_namespace(mock_remote_image_ctx, remote_snap_id1); C_SaferCond ctx; @@ -349,6 +376,7 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapCreateCancel) { mock_local_image_ctx, mock_journaler, &ctx); InSequence seq; + expect_start_op(mock_exclusive_lock); EXPECT_CALL(mock_snapshot_create_request, send()) .WillOnce(DoAll(InvokeWithoutArgs([request]() { request->cancel(); @@ -365,23 +393,29 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapRemoveAndCreate) { ASSERT_EQ(0, create_snap(m_remote_image_ctx, "snap1")); ASSERT_EQ(0, create_snap(m_local_image_ctx, "snap1")); - uint64_t remote_snap_id1 = m_remote_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}]; - uint64_t local_snap_id1 = m_local_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}]; + uint64_t remote_snap_id1 = m_remote_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}]; + uint64_t local_snap_id1 = m_local_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}]; librbd::MockTestImageCtx mock_remote_image_ctx(*m_remote_image_ctx); librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); MockSnapshotCreateRequest mock_snapshot_create_request; journal::MockJournaler mock_journaler; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + InSequence seq; expect_snap_is_unprotected(mock_local_image_ctx, - m_local_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}], true, 0); + m_local_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}], + true, 0); expect_get_snap_namespace(mock_local_image_ctx, local_snap_id1); + expect_start_op(mock_exclusive_lock); expect_snap_remove(mock_local_image_ctx, "snap1", 0); expect_get_snap_namespace(mock_remote_image_ctx, remote_snap_id1); + expect_start_op(mock_exclusive_lock); expect_snap_create(mock_local_image_ctx, mock_snapshot_create_request, "snap1", 12, 0); expect_snap_is_protected(mock_remote_image_ctx, remote_snap_id1, false, 0); expect_update_client(mock_journaler, 0); @@ -404,13 +438,18 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapRemoveError) { librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); journal::MockJournaler mock_journaler; - uint64_t local_snap_id1 = m_local_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}]; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + + uint64_t local_snap_id1 = m_local_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}]; InSequence seq; expect_snap_is_unprotected(mock_local_image_ctx, - m_local_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}], true, 0); + m_local_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}], + true, 0); expect_get_snap_namespace(mock_local_image_ctx, local_snap_id1); + expect_start_op(mock_exclusive_lock); expect_snap_remove(mock_local_image_ctx, "snap1", -EINVAL); C_SaferCond ctx; @@ -425,19 +464,23 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapUnprotect) { ASSERT_EQ(0, create_snap(m_remote_image_ctx, "snap1", true)); ASSERT_EQ(0, create_snap(m_local_image_ctx, "snap1", true)); - uint64_t remote_snap_id1 = m_remote_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}]; - uint64_t local_snap_id1 = m_local_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}]; + uint64_t remote_snap_id1 = m_remote_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}]; + uint64_t local_snap_id1 = m_local_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}]; m_client_meta.snap_seqs[remote_snap_id1] = local_snap_id1; librbd::MockTestImageCtx mock_remote_image_ctx(*m_remote_image_ctx); librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); journal::MockJournaler mock_journaler; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + InSequence seq; expect_snap_is_unprotected(mock_local_image_ctx, local_snap_id1, false, 0); expect_snap_is_unprotected(mock_remote_image_ctx, remote_snap_id1, true, 0); + expect_start_op(mock_exclusive_lock); expect_snap_unprotect(mock_local_image_ctx, "snap1", 0); expect_get_snap_namespace(mock_local_image_ctx, local_snap_id1); expect_get_snap_namespace(mock_remote_image_ctx, remote_snap_id1); @@ -459,19 +502,23 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapUnprotectError) { ASSERT_EQ(0, create_snap(m_remote_image_ctx, "snap1", true)); ASSERT_EQ(0, create_snap(m_local_image_ctx, "snap1", true)); - uint64_t remote_snap_id1 = m_remote_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}]; - uint64_t local_snap_id1 = m_local_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}]; + uint64_t remote_snap_id1 = m_remote_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}]; + uint64_t local_snap_id1 = m_local_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}]; m_client_meta.snap_seqs[remote_snap_id1] = local_snap_id1; librbd::MockTestImageCtx mock_remote_image_ctx(*m_remote_image_ctx); librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); journal::MockJournaler mock_journaler; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + InSequence seq; expect_snap_is_unprotected(mock_local_image_ctx, local_snap_id1, false, 0); expect_snap_is_unprotected(mock_remote_image_ctx, remote_snap_id1, true, 0); + expect_start_op(mock_exclusive_lock); expect_snap_unprotect(mock_local_image_ctx, "snap1", -EBUSY); C_SaferCond ctx; @@ -486,16 +533,19 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapUnprotectCancel) { ASSERT_EQ(0, create_snap(m_remote_image_ctx, "snap1", true)); ASSERT_EQ(0, create_snap(m_local_image_ctx, "snap1", true)); - uint64_t remote_snap_id1 = m_remote_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}]; - uint64_t local_snap_id1 = m_local_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}]; + uint64_t remote_snap_id1 = m_remote_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}]; + uint64_t local_snap_id1 = m_local_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}]; m_client_meta.snap_seqs[remote_snap_id1] = local_snap_id1; librbd::MockTestImageCtx mock_remote_image_ctx(*m_remote_image_ctx); librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); journal::MockJournaler mock_journaler; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + C_SaferCond ctx; MockSnapshotCopyRequest *request = create_request(mock_remote_image_ctx, mock_local_image_ctx, @@ -503,6 +553,7 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapUnprotectCancel) { InSequence seq; expect_snap_is_unprotected(mock_local_image_ctx, local_snap_id1, false, 0); expect_snap_is_unprotected(mock_remote_image_ctx, remote_snap_id1, true, 0); + expect_start_op(mock_exclusive_lock); EXPECT_CALL(*mock_local_image_ctx.operations, execute_snap_unprotect(_, StrEq("snap1"), _)) .WillOnce(DoAll(InvokeWithoutArgs([request]() { @@ -520,24 +571,31 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapUnprotectRemove) { ASSERT_EQ(0, create_snap(m_remote_image_ctx, "snap1", true)); ASSERT_EQ(0, create_snap(m_local_image_ctx, "snap1", true)); - uint64_t remote_snap_id1 = m_remote_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}]; - uint64_t local_snap_id1 = m_local_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}]; + uint64_t remote_snap_id1 = m_remote_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}]; + uint64_t local_snap_id1 = m_local_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}]; librbd::MockTestImageCtx mock_remote_image_ctx(*m_remote_image_ctx); librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); MockSnapshotCreateRequest mock_snapshot_create_request; journal::MockJournaler mock_journaler; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + InSequence seq; expect_snap_is_unprotected(mock_local_image_ctx, - m_local_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}], false, 0); + m_local_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}], + false, 0); + expect_start_op(mock_exclusive_lock); expect_snap_unprotect(mock_local_image_ctx, "snap1", 0); expect_get_snap_namespace(mock_local_image_ctx, local_snap_id1); + expect_start_op(mock_exclusive_lock); expect_snap_remove(mock_local_image_ctx, "snap1", 0); expect_get_snap_namespace(mock_remote_image_ctx, remote_snap_id1); + expect_start_op(mock_exclusive_lock); expect_snap_create(mock_local_image_ctx, mock_snapshot_create_request, "snap1", 12, 0); expect_snap_is_protected(mock_remote_image_ctx, remote_snap_id1, false, 0); expect_update_client(mock_journaler, 0); @@ -556,19 +614,24 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapUnprotectRemove) { TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapCreateProtect) { ASSERT_EQ(0, create_snap(m_remote_image_ctx, "snap1", true)); - uint64_t remote_snap_id1 = m_remote_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}]; + uint64_t remote_snap_id1 = m_remote_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}]; librbd::MockTestImageCtx mock_remote_image_ctx(*m_remote_image_ctx); librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); MockSnapshotCreateRequest mock_snapshot_create_request; journal::MockJournaler mock_journaler; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + InSequence seq; expect_get_snap_namespace(mock_remote_image_ctx, remote_snap_id1); + expect_start_op(mock_exclusive_lock); expect_snap_create(mock_local_image_ctx, mock_snapshot_create_request, "snap1", 12, 0); expect_snap_is_protected(mock_remote_image_ctx, remote_snap_id1, true, 0); expect_snap_is_protected(mock_local_image_ctx, 12, false, 0); + expect_start_op(mock_exclusive_lock); expect_snap_protect(mock_local_image_ctx, "snap1", 0); expect_update_client(mock_journaler, 0); @@ -587,22 +650,26 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapProtect) { ASSERT_EQ(0, create_snap(m_remote_image_ctx, "snap1", true)); ASSERT_EQ(0, create_snap(m_local_image_ctx, "snap1", true)); - uint64_t remote_snap_id1 = m_remote_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}]; - uint64_t local_snap_id1 = m_local_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}]; + uint64_t remote_snap_id1 = m_remote_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}]; + uint64_t local_snap_id1 = m_local_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}]; m_client_meta.snap_seqs[remote_snap_id1] = local_snap_id1; librbd::MockTestImageCtx mock_remote_image_ctx(*m_remote_image_ctx); librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); journal::MockJournaler mock_journaler; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + InSequence seq; expect_snap_is_unprotected(mock_local_image_ctx, local_snap_id1, true, 0); expect_get_snap_namespace(mock_local_image_ctx, local_snap_id1); expect_get_snap_namespace(mock_remote_image_ctx, remote_snap_id1); expect_snap_is_protected(mock_remote_image_ctx, remote_snap_id1, true, 0); expect_snap_is_protected(mock_local_image_ctx, local_snap_id1, false, 0); + expect_start_op(mock_exclusive_lock); expect_snap_protect(mock_local_image_ctx, "snap1", 0); expect_update_client(mock_journaler, 0); @@ -621,22 +688,26 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapProtectError) { ASSERT_EQ(0, create_snap(m_remote_image_ctx, "snap1", true)); ASSERT_EQ(0, create_snap(m_local_image_ctx, "snap1", true)); - uint64_t remote_snap_id1 = m_remote_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}]; - uint64_t local_snap_id1 = m_local_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}]; + uint64_t remote_snap_id1 = m_remote_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}]; + uint64_t local_snap_id1 = m_local_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}]; m_client_meta.snap_seqs[remote_snap_id1] = local_snap_id1; librbd::MockTestImageCtx mock_remote_image_ctx(*m_remote_image_ctx); librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); journal::MockJournaler mock_journaler; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + InSequence seq; expect_snap_is_unprotected(mock_local_image_ctx, local_snap_id1, true, 0); expect_get_snap_namespace(mock_local_image_ctx, local_snap_id1); expect_get_snap_namespace(mock_remote_image_ctx, remote_snap_id1); expect_snap_is_protected(mock_remote_image_ctx, remote_snap_id1, true, 0); expect_snap_is_protected(mock_local_image_ctx, local_snap_id1, false, 0); + expect_start_op(mock_exclusive_lock); expect_snap_protect(mock_local_image_ctx, "snap1", -EINVAL); C_SaferCond ctx; @@ -651,16 +722,19 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapProtectCancel) { ASSERT_EQ(0, create_snap(m_remote_image_ctx, "snap1", true)); ASSERT_EQ(0, create_snap(m_local_image_ctx, "snap1", true)); - uint64_t remote_snap_id1 = m_remote_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}]; - uint64_t local_snap_id1 = m_local_image_ctx->snap_ids[{cls::rbd::UserSnapshotNamespace(), - "snap1"}]; + uint64_t remote_snap_id1 = m_remote_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}]; + uint64_t local_snap_id1 = m_local_image_ctx->snap_ids[ + {cls::rbd::UserSnapshotNamespace(), "snap1"}]; m_client_meta.snap_seqs[remote_snap_id1] = local_snap_id1; librbd::MockTestImageCtx mock_remote_image_ctx(*m_remote_image_ctx); librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); journal::MockJournaler mock_journaler; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + C_SaferCond ctx; MockSnapshotCopyRequest *request = create_request(mock_remote_image_ctx, mock_local_image_ctx, @@ -671,6 +745,7 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapProtectCancel) { expect_get_snap_namespace(mock_remote_image_ctx, remote_snap_id1); expect_snap_is_protected(mock_remote_image_ctx, remote_snap_id1, true, 0); expect_snap_is_protected(mock_local_image_ctx, local_snap_id1, false, 0); + expect_start_op(mock_exclusive_lock); EXPECT_CALL(*mock_local_image_ctx.operations, execute_snap_protect(_, StrEq("snap1"), _)) .WillOnce(DoAll(InvokeWithoutArgs([request]() { diff --git a/src/test/rbd_mirror/image_sync/test_mock_SnapshotCreateRequest.cc b/src/test/rbd_mirror/image_sync/test_mock_SnapshotCreateRequest.cc index 91893abe2b54..4a8ab1fc73cf 100644 --- a/src/test/rbd_mirror/image_sync/test_mock_SnapshotCreateRequest.cc +++ b/src/test/rbd_mirror/image_sync/test_mock_SnapshotCreateRequest.cc @@ -39,6 +39,7 @@ using ::testing::InSequence; using ::testing::Invoke; using ::testing::InvokeWithoutArgs; using ::testing::Return; +using ::testing::ReturnNew; using ::testing::StrEq; using ::testing::WithArg; @@ -54,6 +55,11 @@ public: ASSERT_EQ(0, open_image(m_local_io_ctx, m_image_name, &m_local_image_ctx)); } + void expect_start_op(librbd::MockExclusiveLock &mock_exclusive_lock) { + EXPECT_CALL(mock_exclusive_lock, start_op()).WillOnce( + ReturnNew([](int) {})); + } + void expect_test_features(librbd::MockTestImageCtx &mock_image_ctx, uint64_t features, bool enabled) { EXPECT_CALL(mock_image_ctx, test_features(features)) @@ -120,9 +126,13 @@ public: TEST_F(TestMockImageSyncSnapshotCreateRequest, Resize) { librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; InSequence seq; + expect_start_op(mock_exclusive_lock); expect_set_size(mock_local_image_ctx, 0); + expect_start_op(mock_exclusive_lock); expect_snap_create(mock_local_image_ctx, "snap1", 10, 0); expect_test_features(mock_local_image_ctx, RBD_FEATURE_OBJECT_MAP, false); @@ -138,8 +148,11 @@ TEST_F(TestMockImageSyncSnapshotCreateRequest, Resize) { TEST_F(TestMockImageSyncSnapshotCreateRequest, ResizeError) { librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; InSequence seq; + expect_start_op(mock_exclusive_lock); expect_set_size(mock_local_image_ctx, -EINVAL); C_SaferCond ctx; @@ -154,10 +167,15 @@ TEST_F(TestMockImageSyncSnapshotCreateRequest, ResizeError) { TEST_F(TestMockImageSyncSnapshotCreateRequest, RemoveParent) { librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + mock_local_image_ctx.parent_md.spec.pool_id = 213; InSequence seq; + expect_start_op(mock_exclusive_lock); expect_remove_parent(mock_local_image_ctx, 0); + expect_start_op(mock_exclusive_lock); expect_snap_create(mock_local_image_ctx, "snap1", 10, 0); expect_test_features(mock_local_image_ctx, RBD_FEATURE_OBJECT_MAP, false); @@ -173,9 +191,13 @@ TEST_F(TestMockImageSyncSnapshotCreateRequest, RemoveParent) { TEST_F(TestMockImageSyncSnapshotCreateRequest, RemoveParentError) { librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + mock_local_image_ctx.parent_md.spec.pool_id = 213; InSequence seq; + expect_start_op(mock_exclusive_lock); expect_remove_parent(mock_local_image_ctx, -EINVAL); C_SaferCond ctx; @@ -190,11 +212,17 @@ TEST_F(TestMockImageSyncSnapshotCreateRequest, RemoveParentError) { TEST_F(TestMockImageSyncSnapshotCreateRequest, RemoveSetParent) { librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + mock_local_image_ctx.parent_md.spec.pool_id = 213; InSequence seq; + expect_start_op(mock_exclusive_lock); expect_remove_parent(mock_local_image_ctx, 0); + expect_start_op(mock_exclusive_lock); expect_set_parent(mock_local_image_ctx, 0); + expect_start_op(mock_exclusive_lock); expect_snap_create(mock_local_image_ctx, "snap1", 10, 0); expect_test_features(mock_local_image_ctx, RBD_FEATURE_OBJECT_MAP, false); @@ -211,9 +239,13 @@ TEST_F(TestMockImageSyncSnapshotCreateRequest, RemoveSetParent) { TEST_F(TestMockImageSyncSnapshotCreateRequest, SetParentSpec) { librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; InSequence seq; + expect_start_op(mock_exclusive_lock); expect_set_parent(mock_local_image_ctx, 0); + expect_start_op(mock_exclusive_lock); expect_snap_create(mock_local_image_ctx, "snap1", 10, 0); expect_test_features(mock_local_image_ctx, RBD_FEATURE_OBJECT_MAP, false); @@ -230,10 +262,15 @@ TEST_F(TestMockImageSyncSnapshotCreateRequest, SetParentSpec) { TEST_F(TestMockImageSyncSnapshotCreateRequest, SetParentOverlap) { librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + mock_local_image_ctx.parent_md.spec = {123, "test", 0}; InSequence seq; + expect_start_op(mock_exclusive_lock); expect_set_parent(mock_local_image_ctx, 0); + expect_start_op(mock_exclusive_lock); expect_snap_create(mock_local_image_ctx, "snap1", 10, 0); expect_test_features(mock_local_image_ctx, RBD_FEATURE_OBJECT_MAP, false); @@ -250,8 +287,11 @@ TEST_F(TestMockImageSyncSnapshotCreateRequest, SetParentOverlap) { TEST_F(TestMockImageSyncSnapshotCreateRequest, SetParentError) { librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; InSequence seq; + expect_start_op(mock_exclusive_lock); expect_set_parent(mock_local_image_ctx, -ESTALE); C_SaferCond ctx; @@ -267,8 +307,11 @@ TEST_F(TestMockImageSyncSnapshotCreateRequest, SetParentError) { TEST_F(TestMockImageSyncSnapshotCreateRequest, SnapCreate) { librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; InSequence seq; + expect_start_op(mock_exclusive_lock); expect_snap_create(mock_local_image_ctx, "snap1", 10, 0); expect_test_features(mock_local_image_ctx, RBD_FEATURE_OBJECT_MAP, false); @@ -284,8 +327,11 @@ TEST_F(TestMockImageSyncSnapshotCreateRequest, SnapCreate) { TEST_F(TestMockImageSyncSnapshotCreateRequest, SnapCreateError) { librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; InSequence seq; + expect_start_op(mock_exclusive_lock); expect_snap_create(mock_local_image_ctx, "snap1", 10, -EINVAL); C_SaferCond ctx; @@ -300,10 +346,14 @@ TEST_F(TestMockImageSyncSnapshotCreateRequest, SnapCreateError) { TEST_F(TestMockImageSyncSnapshotCreateRequest, ResizeObjectMap) { librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; InSequence seq; + expect_start_op(mock_exclusive_lock); expect_snap_create(mock_local_image_ctx, "snap1", 10, 0); expect_test_features(mock_local_image_ctx, RBD_FEATURE_OBJECT_MAP, true); + expect_start_op(mock_exclusive_lock); expect_object_map_resize(mock_local_image_ctx, 10, 0); C_SaferCond ctx; @@ -318,10 +368,14 @@ TEST_F(TestMockImageSyncSnapshotCreateRequest, ResizeObjectMap) { TEST_F(TestMockImageSyncSnapshotCreateRequest, ResizeObjectMapError) { librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; InSequence seq; + expect_start_op(mock_exclusive_lock); expect_snap_create(mock_local_image_ctx, "snap1", 10, 0); expect_test_features(mock_local_image_ctx, RBD_FEATURE_OBJECT_MAP, true); + expect_start_op(mock_exclusive_lock); expect_object_map_resize(mock_local_image_ctx, 10, -EINVAL); C_SaferCond ctx; diff --git a/src/test/rbd_mirror/test_mock_ImageSync.cc b/src/test/rbd_mirror/test_mock_ImageSync.cc index 676615d870c7..914338b9eb97 100644 --- a/src/test/rbd_mirror/test_mock_ImageSync.cc +++ b/src/test/rbd_mirror/test_mock_ImageSync.cc @@ -169,6 +169,7 @@ using ::testing::_; using ::testing::InSequence; using ::testing::Invoke; using ::testing::Return; +using ::testing::ReturnNew; using ::testing::WithArg; using ::testing::InvokeWithoutArgs; @@ -191,6 +192,11 @@ public: ASSERT_EQ(0, open_image(m_local_io_ctx, m_image_name, &m_local_image_ctx)); } + void expect_start_op(librbd::MockExclusiveLock &mock_exclusive_lock) { + EXPECT_CALL(mock_exclusive_lock, start_op()).WillOnce( + ReturnNew([](int) {})); + } + void expect_create_sync_point(librbd::MockTestImageCtx &mock_local_image_ctx, MockSyncPointCreateRequest &mock_sync_point_create_request, int r) { @@ -286,6 +292,9 @@ TEST_F(TestMockImageSync, SimpleSync) { MockSyncPointCreateRequest mock_sync_point_create_request; MockSyncPointPruneRequest mock_sync_point_prune_request; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + librbd::MockObjectMap *mock_object_map = new librbd::MockObjectMap(); mock_local_image_ctx.object_map = mock_object_map; expect_test_features(mock_local_image_ctx); @@ -294,6 +303,7 @@ TEST_F(TestMockImageSync, SimpleSync) { expect_create_sync_point(mock_local_image_ctx, mock_sync_point_create_request, 0); expect_copy_snapshots(mock_snapshot_copy_request, 0); expect_copy_image(mock_image_copy_request, 0); + expect_start_op(mock_exclusive_lock); expect_rollback_object_map(*mock_object_map, 0); expect_create_object_map(mock_local_image_ctx, mock_object_map); expect_open_object_map(mock_local_image_ctx, *mock_object_map); @@ -321,6 +331,9 @@ TEST_F(TestMockImageSync, RestartSync) { mock_local_image_ctx.snap_ids[{cls::rbd::UserSnapshotNamespace(), "snap1"}] = 123; mock_local_image_ctx.snap_ids[{cls::rbd::UserSnapshotNamespace(), "snap2"}] = 234; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + librbd::MockObjectMap *mock_object_map = new librbd::MockObjectMap(); mock_local_image_ctx.object_map = mock_object_map; expect_test_features(mock_local_image_ctx); @@ -329,6 +342,7 @@ TEST_F(TestMockImageSync, RestartSync) { expect_prune_sync_point(mock_sync_point_prune_request, false, 0); expect_copy_snapshots(mock_snapshot_copy_request, 0); expect_copy_image(mock_image_copy_request, 0); + expect_start_op(mock_exclusive_lock); expect_rollback_object_map(*mock_object_map, 0); expect_create_object_map(mock_local_image_ctx, mock_object_map); expect_open_object_map(mock_local_image_ctx, *mock_object_map); @@ -351,6 +365,9 @@ TEST_F(TestMockImageSync, CancelImageCopy) { MockSyncPointCreateRequest mock_sync_point_create_request; MockSyncPointPruneRequest mock_sync_point_prune_request; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + m_client_meta.sync_points = {{cls::rbd::UserSnapshotNamespace(), "snap1", boost::none}}; InSequence seq; @@ -387,6 +404,9 @@ TEST_F(TestMockImageSync, CancelAfterCopySnapshots) { MockSnapshotCopyRequest mock_snapshot_copy_request; MockSyncPointCreateRequest mock_sync_point_create_request; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + librbd::MockObjectMap *mock_object_map = new librbd::MockObjectMap(); mock_local_image_ctx.object_map = mock_object_map; expect_test_features(mock_local_image_ctx); @@ -419,6 +439,9 @@ TEST_F(TestMockImageSync, CancelAfterCopyImage) { MockSyncPointCreateRequest mock_sync_point_create_request; MockSyncPointPruneRequest mock_sync_point_prune_request; + librbd::MockExclusiveLock mock_exclusive_lock; + mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock; + librbd::MockObjectMap *mock_object_map = new librbd::MockObjectMap(); mock_local_image_ctx.object_map = mock_object_map; expect_test_features(mock_local_image_ctx); diff --git a/src/tools/rbd_mirror/ImageSync.cc b/src/tools/rbd_mirror/ImageSync.cc index 9c898fdbb628..c0ea6f599788 100644 --- a/src/tools/rbd_mirror/ImageSync.cc +++ b/src/tools/rbd_mirror/ImageSync.cc @@ -5,6 +5,7 @@ #include "ProgressContext.h" #include "common/errno.h" #include "journal/Journaler.h" +#include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/ObjectMap.h" #include "librbd/Utils.h" @@ -244,10 +245,12 @@ template void ImageSync::send_copy_object_map() { update_progress("COPY_OBJECT_MAP"); + m_local_image_ctx->owner_lock.get_read(); m_local_image_ctx->snap_lock.get_read(); if (!m_local_image_ctx->test_features(RBD_FEATURE_OBJECT_MAP, m_local_image_ctx->snap_lock)) { m_local_image_ctx->snap_lock.put_read(); + m_local_image_ctx->owner_lock.put_read(); send_prune_sync_points(); return; } @@ -257,20 +260,35 @@ void ImageSync::send_copy_object_map() { assert(!m_client_meta->sync_points.empty()); librbd::journal::MirrorPeerSyncPoint &sync_point = m_client_meta->sync_points.front(); - auto snap_id_it = m_local_image_ctx->snap_ids.find({cls::rbd::UserSnapshotNamespace(), - sync_point.snap_name}); + auto snap_id_it = m_local_image_ctx->snap_ids.find( + {cls::rbd::UserSnapshotNamespace(), sync_point.snap_name}); assert(snap_id_it != m_local_image_ctx->snap_ids.end()); librados::snap_t snap_id = snap_id_it->second; dout(20) << ": snap_id=" << snap_id << ", " << "snap_name=" << sync_point.snap_name << dendl; + Context *finish_op_ctx = nullptr; + if (m_local_image_ctx->exclusive_lock != nullptr) { + finish_op_ctx = m_local_image_ctx->exclusive_lock->start_op(); + } + if (finish_op_ctx == nullptr) { + derr << ": lost exclusive lock" << dendl; + m_local_image_ctx->snap_lock.put_read(); + m_local_image_ctx->owner_lock.put_read(); + finish(-EROFS); + return; + } + // rollback the object map (copy snapshot object map to HEAD) RWLock::WLocker object_map_locker(m_local_image_ctx->object_map_lock); - Context *ctx = create_context_callback< - ImageSync, &ImageSync::handle_copy_object_map>(this); + auto ctx = new FunctionContext([this, finish_op_ctx](int r) { + handle_copy_object_map(r); + finish_op_ctx->complete(0); + }); m_local_image_ctx->object_map->rollback(snap_id, ctx); m_local_image_ctx->snap_lock.put_read(); + m_local_image_ctx->owner_lock.put_read(); } template diff --git a/src/tools/rbd_mirror/image_sync/ImageCopyRequest.cc b/src/tools/rbd_mirror/image_sync/ImageCopyRequest.cc index dbc2560acd02..6d3ec7af36c0 100644 --- a/src/tools/rbd_mirror/image_sync/ImageCopyRequest.cc +++ b/src/tools/rbd_mirror/image_sync/ImageCopyRequest.cc @@ -7,6 +7,7 @@ #include "common/errno.h" #include "common/Timer.h" #include "journal/Journaler.h" +#include "librbd/ExclusiveLock.h" #include "librbd/Utils.h" #include "tools/rbd_mirror/ProgressContext.h" @@ -141,6 +142,7 @@ void ImageCopyRequest::send_object_copies() { bool complete; { + RWLock::RLocker owner_locker(m_local_image_ctx->owner_lock); Mutex::Locker locker(m_lock); for (int i = 0; i < cct->_conf->rbd_concurrent_management_ops; ++i) { send_next_object_copy(); @@ -172,6 +174,7 @@ void ImageCopyRequest::send_object_copies() { template void ImageCopyRequest::send_next_object_copy() { + assert(m_local_image_ctx->owner_lock.is_locked()); assert(m_lock.is_locked()); if (m_canceled && m_ret_val == 0) { @@ -189,8 +192,20 @@ void ImageCopyRequest::send_next_object_copy() { ++m_current_ops; - Context *ctx = create_context_callback< - ImageCopyRequest, &ImageCopyRequest::handle_object_copy>(this); + Context *finish_op_ctx = nullptr; + if (m_local_image_ctx->exclusive_lock != nullptr) { + finish_op_ctx = m_local_image_ctx->exclusive_lock->start_op(); + } + if (finish_op_ctx == nullptr) { + derr << ": lost exclusive lock" << dendl; + finish(-EROFS); + return; + } + + auto ctx = new FunctionContext([this, finish_op_ctx](int r) { + handle_object_copy(r); + finish_op_ctx->complete(0); + }); ObjectCopyRequest *req = ObjectCopyRequest::create( m_local_image_ctx, m_remote_image_ctx, &m_snap_map, ono, ctx); req->send(); @@ -203,6 +218,7 @@ void ImageCopyRequest::handle_object_copy(int r) { int percent; bool complete; { + RWLock::RLocker owner_locker(m_local_image_ctx->owner_lock); Mutex::Locker locker(m_lock); assert(m_current_ops > 0); --m_current_ops; diff --git a/src/tools/rbd_mirror/image_sync/SnapshotCopyRequest.cc b/src/tools/rbd_mirror/image_sync/SnapshotCopyRequest.cc index 25967b9e3395..f5f0701d1134 100644 --- a/src/tools/rbd_mirror/image_sync/SnapshotCopyRequest.cc +++ b/src/tools/rbd_mirror/image_sync/SnapshotCopyRequest.cc @@ -6,6 +6,7 @@ #include "common/errno.h" #include "common/WorkQueue.h" #include "journal/Journaler.h" +#include "librbd/ExclusiveLock.h" #include "librbd/Operations.h" #include "librbd/Utils.h" #include "librbd/journal/Types.h" @@ -169,13 +170,20 @@ void SnapshotCopyRequest::send_snap_unprotect() { << "snap_name=" << m_snap_name << ", " << "snap_id=" << m_prev_snap_id << dendl; - Context *ctx = create_context_callback< - SnapshotCopyRequest, &SnapshotCopyRequest::handle_snap_unprotect>( - this); + auto finish_op_ctx = start_local_op(); + if (finish_op_ctx == nullptr) { + derr << ": lost exclusive lock" << dendl; + finish(-EROFS); + return; + } + + auto ctx = new FunctionContext([this, finish_op_ctx](int r) { + handle_snap_unprotect(r); + finish_op_ctx->complete(0); + }); RWLock::RLocker owner_locker(m_local_image_ctx->owner_lock); - m_local_image_ctx->operations->execute_snap_unprotect(cls::rbd::UserSnapshotNamespace(), - m_snap_name.c_str(), - ctx); + m_local_image_ctx->operations->execute_snap_unprotect( + cls::rbd::UserSnapshotNamespace(), m_snap_name.c_str(), ctx); } template @@ -208,7 +216,8 @@ void SnapshotCopyRequest::send_snap_remove() { cls::rbd::SnapshotNamespace snap_namespace; m_local_image_ctx->snap_lock.get_read(); - int r = m_local_image_ctx->get_snap_namespace(local_snap_id, &snap_namespace); + int r = m_local_image_ctx->get_snap_namespace(local_snap_id, + &snap_namespace); m_local_image_ctx->snap_lock.put_read(); if (r < 0) { derr << ": failed to retrieve local snap namespace: " << m_snap_name @@ -217,7 +226,8 @@ void SnapshotCopyRequest::send_snap_remove() { return; } - if (boost::get(&snap_namespace) == nullptr) { + if (boost::get(&snap_namespace) == + nullptr) { continue; } @@ -247,13 +257,20 @@ void SnapshotCopyRequest::send_snap_remove() { << "snap_name=" << m_snap_name << ", " << "snap_id=" << m_prev_snap_id << dendl; - Context *ctx = create_context_callback< - SnapshotCopyRequest, &SnapshotCopyRequest::handle_snap_remove>( - this); + auto finish_op_ctx = start_local_op(); + if (finish_op_ctx == nullptr) { + derr << ": lost exclusive lock" << dendl; + finish(-EROFS); + return; + } + + auto ctx = new FunctionContext([this, finish_op_ctx](int r) { + handle_snap_remove(r); + finish_op_ctx->complete(0); + }); RWLock::RLocker owner_locker(m_local_image_ctx->owner_lock); - m_local_image_ctx->operations->execute_snap_remove(cls::rbd::UserSnapshotNamespace(), - m_snap_name.c_str(), - ctx); + m_local_image_ctx->operations->execute_snap_remove( + cls::rbd::UserSnapshotNamespace(), m_snap_name.c_str(), ctx); } template @@ -343,11 +360,20 @@ void SnapshotCopyRequest::send_snap_create() { << "snap_id=" << parent_spec.snap_id << ", " << "overlap=" << parent_overlap << "]" << dendl; - Context *ctx = create_context_callback< - SnapshotCopyRequest, &SnapshotCopyRequest::handle_snap_create>( - this); + Context *finish_op_ctx = start_local_op(); + if (finish_op_ctx == nullptr) { + derr << ": lost exclusive lock" << dendl; + finish(-EROFS); + return; + } + + auto ctx = new FunctionContext([this, finish_op_ctx](int r) { + handle_snap_create(r); + finish_op_ctx->complete(0); + }); SnapshotCreateRequest *req = SnapshotCreateRequest::create( - m_local_image_ctx, m_snap_name, m_snap_namespace, size, parent_spec, parent_overlap, ctx); + m_local_image_ctx, m_snap_name, m_snap_namespace, size, parent_spec, + parent_overlap, ctx); req->send(); } @@ -445,13 +471,20 @@ void SnapshotCopyRequest::send_snap_protect() { << "snap_name=" << m_snap_name << ", " << "snap_id=" << m_prev_snap_id << dendl; - Context *ctx = create_context_callback< - SnapshotCopyRequest, &SnapshotCopyRequest::handle_snap_protect>( - this); + auto finish_op_ctx = start_local_op(); + if (finish_op_ctx == nullptr) { + derr << ": lost exclusive lock" << dendl; + finish(-EROFS); + return; + } + + auto ctx = new FunctionContext([this, finish_op_ctx](int r) { + handle_snap_protect(r); + finish_op_ctx->complete(0); + }); RWLock::RLocker owner_locker(m_local_image_ctx->owner_lock); - m_local_image_ctx->operations->execute_snap_protect(cls::rbd::UserSnapshotNamespace(), - m_snap_name.c_str(), - ctx); + m_local_image_ctx->operations->execute_snap_protect( + cls::rbd::UserSnapshotNamespace(), m_snap_name.c_str(), ctx); } template @@ -565,6 +598,15 @@ int SnapshotCopyRequest::validate_parent(I *image_ctx, return 0; } +template +Context *SnapshotCopyRequest::start_local_op() { + RWLock::RLocker owner_locker(m_local_image_ctx->owner_lock); + if (m_local_image_ctx->exclusive_lock == nullptr) { + return nullptr; + } + return m_local_image_ctx->exclusive_lock->start_op(); +} + } // namespace image_sync } // namespace mirror } // namespace rbd diff --git a/src/tools/rbd_mirror/image_sync/SnapshotCopyRequest.h b/src/tools/rbd_mirror/image_sync/SnapshotCopyRequest.h index 1714ad2dd90d..85ae4d41305a 100644 --- a/src/tools/rbd_mirror/image_sync/SnapshotCopyRequest.h +++ b/src/tools/rbd_mirror/image_sync/SnapshotCopyRequest.h @@ -134,6 +134,8 @@ private: int validate_parent(ImageCtxT *image_ctx, librbd::ParentSpec *spec); + Context *start_local_op(); + }; } // namespace image_sync diff --git a/src/tools/rbd_mirror/image_sync/SnapshotCreateRequest.cc b/src/tools/rbd_mirror/image_sync/SnapshotCreateRequest.cc index e5cb9eda008e..238417934354 100644 --- a/src/tools/rbd_mirror/image_sync/SnapshotCreateRequest.cc +++ b/src/tools/rbd_mirror/image_sync/SnapshotCreateRequest.cc @@ -5,6 +5,7 @@ #include "common/errno.h" #include "cls/rbd/cls_rbd_client.h" #include "cls/rbd/cls_rbd_types.h" +#include "librbd/ExclusiveLock.h" #include "librbd/ObjectMap.h" #include "librbd/Operations.h" #include "librbd/Utils.h" @@ -62,8 +63,18 @@ void SnapshotCreateRequest::send_set_size() { librados::ObjectWriteOperation op; librbd::cls_client::set_size(&op, m_size); - librados::AioCompletion *comp = create_rados_callback< - SnapshotCreateRequest, &SnapshotCreateRequest::handle_set_size>(this); + auto finish_op_ctx = start_local_op(); + if (finish_op_ctx == nullptr) { + derr << ": lost exclusive lock" << dendl; + finish(-EROFS); + return; + } + + auto ctx = new FunctionContext([this, finish_op_ctx](int r) { + handle_set_size(r); + finish_op_ctx->complete(0); + }); + librados::AioCompletion *comp = create_rados_callback(ctx); int r = m_local_image_ctx->md_ctx.aio_operate(m_local_image_ctx->header_oid, comp, &op); assert(r == 0); @@ -106,9 +117,18 @@ void SnapshotCreateRequest::send_remove_parent() { librados::ObjectWriteOperation op; librbd::cls_client::remove_parent(&op); - librados::AioCompletion *comp = create_rados_callback< - SnapshotCreateRequest, - &SnapshotCreateRequest::handle_remove_parent>(this); + auto finish_op_ctx = start_local_op(); + if (finish_op_ctx == nullptr) { + derr << ": lost exclusive lock" << dendl; + finish(-EROFS); + return; + } + + auto ctx = new FunctionContext([this, finish_op_ctx](int r) { + handle_remove_parent(r); + finish_op_ctx->complete(0); + }); + librados::AioCompletion *comp = create_rados_callback(ctx); int r = m_local_image_ctx->md_ctx.aio_operate(m_local_image_ctx->header_oid, comp, &op); assert(r == 0); @@ -152,9 +172,18 @@ void SnapshotCreateRequest::send_set_parent() { librados::ObjectWriteOperation op; librbd::cls_client::set_parent(&op, m_parent_spec, m_parent_overlap); - librados::AioCompletion *comp = create_rados_callback< - SnapshotCreateRequest, - &SnapshotCreateRequest::handle_set_parent>(this); + auto finish_op_ctx = start_local_op(); + if (finish_op_ctx == nullptr) { + derr << ": lost exclusive lock" << dendl; + finish(-EROFS); + return; + } + + auto ctx = new FunctionContext([this, finish_op_ctx](int r) { + handle_set_parent(r); + finish_op_ctx->complete(0); + }); + librados::AioCompletion *comp = create_rados_callback(ctx); int r = m_local_image_ctx->md_ctx.aio_operate(m_local_image_ctx->header_oid, comp, &op); assert(r == 0); @@ -186,9 +215,17 @@ template void SnapshotCreateRequest::send_snap_create() { dout(20) << ": snap_name=" << m_snap_name << dendl; - Context *ctx = create_context_callback< - SnapshotCreateRequest, &SnapshotCreateRequest::handle_snap_create>( - this); + auto finish_op_ctx = start_local_op(); + if (finish_op_ctx == nullptr) { + derr << ": lost exclusive lock" << dendl; + finish(-EROFS); + return; + } + + auto ctx = new FunctionContext([this, finish_op_ctx](int r) { + handle_snap_create(r); + finish_op_ctx->complete(0); + }); RWLock::RLocker owner_locker(m_local_image_ctx->owner_lock); m_local_image_ctx->operations->execute_snap_create(m_snap_namespace, m_snap_name.c_str(), @@ -218,8 +255,8 @@ void SnapshotCreateRequest::send_create_object_map() { } m_local_image_ctx->snap_lock.get_read(); - auto snap_it = m_local_image_ctx->snap_ids.find({cls::rbd::UserSnapshotNamespace(), - m_snap_name}); + auto snap_it = m_local_image_ctx->snap_ids.find( + {cls::rbd::UserSnapshotNamespace(), m_snap_name}); if (snap_it == m_local_image_ctx->snap_ids.end()) { derr << ": failed to locate snap: " << m_snap_name << dendl; m_local_image_ctx->snap_lock.put_read(); @@ -242,9 +279,18 @@ void SnapshotCreateRequest::send_create_object_map() { librados::ObjectWriteOperation op; librbd::cls_client::object_map_resize(&op, object_count, OBJECT_NONEXISTENT); - librados::AioCompletion *comp = create_rados_callback< - SnapshotCreateRequest, - &SnapshotCreateRequest::handle_create_object_map>(this); + auto finish_op_ctx = start_local_op(); + if (finish_op_ctx == nullptr) { + derr << ": lost exclusive lock" << dendl; + finish(-EROFS); + return; + } + + auto ctx = new FunctionContext([this, finish_op_ctx](int r) { + handle_create_object_map(r); + finish_op_ctx->complete(0); + }); + librados::AioCompletion *comp = create_rados_callback(ctx); int r = m_local_image_ctx->md_ctx.aio_operate(object_map_oid, comp, &op); assert(r == 0); comp->release(); @@ -264,6 +310,15 @@ void SnapshotCreateRequest::handle_create_object_map(int r) { finish(0); } +template +Context *SnapshotCreateRequest::start_local_op() { + RWLock::RLocker owner_locker(m_local_image_ctx->owner_lock); + if (m_local_image_ctx->exclusive_lock == nullptr) { + return nullptr; + } + return m_local_image_ctx->exclusive_lock->start_op(); +} + template void SnapshotCreateRequest::finish(int r) { dout(20) << ": r=" << r << dendl; diff --git a/src/tools/rbd_mirror/image_sync/SnapshotCreateRequest.h b/src/tools/rbd_mirror/image_sync/SnapshotCreateRequest.h index ecfe414a2d2e..503a164a0d59 100644 --- a/src/tools/rbd_mirror/image_sync/SnapshotCreateRequest.h +++ b/src/tools/rbd_mirror/image_sync/SnapshotCreateRequest.h @@ -94,6 +94,8 @@ private: void send_create_object_map(); void handle_create_object_map(int r); + Context *start_local_op(); + void finish(int r); };