From 399a335de4d9e9b8c9395c16c346b62c8a35d5c9 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Thu, 30 Jan 2020 17:45:40 -0500 Subject: [PATCH] librbd: avoid refreshing within the mirror state machines The refresh has been moved up to the API layer. Also avoid querying the MirrorImage struct when creating mirror primary snapshots since (1) when enabling the record won't exist yet and (2) in the demote/ promote cases it would have just been read by the previous state machine. Signed-off-by: Jason Dillaman --- src/librbd/api/Mirror.cc | 57 +++++++++- src/librbd/mirror/DemoteRequest.cc | 3 +- src/librbd/mirror/PromoteRequest.cc | 4 +- .../mirror/snapshot/CreatePrimaryRequest.cc | 89 ++-------------- .../mirror/snapshot/CreatePrimaryRequest.h | 26 ++--- src/librbd/mirror/snapshot/DemoteRequest.cc | 31 +----- src/librbd/mirror/snapshot/DemoteRequest.h | 19 ++-- src/librbd/mirror/snapshot/PromoteRequest.cc | 30 +----- src/librbd/mirror/snapshot/PromoteRequest.h | 19 ++-- .../test_mock_CreatePrimaryRequest.cc | 100 ++---------------- .../snapshot/test_mock_PromoteRequest.cc | 38 +------ 11 files changed, 110 insertions(+), 306 deletions(-) diff --git a/src/librbd/api/Mirror.cc b/src/librbd/api/Mirror.cc index 81694d84347..88e3bf80e66 100644 --- a/src/librbd/api/Mirror.cc +++ b/src/librbd/api/Mirror.cc @@ -582,8 +582,21 @@ void Mirror::image_promote(I *ictx, bool force, Context *on_finish) { ldout(cct, 20) << "ictx=" << ictx << ", " << "force=" << force << dendl; - auto req = mirror::PromoteRequest<>::create(*ictx, force, on_finish); - req->send(); + auto on_refresh = new LambdaContext([ictx, force, on_finish](int r) { + if (r < 0) { + lderr(ictx->cct) << "refresh failed: " << cpp_strerror(r) << dendl; + on_finish->complete(r); + return; + } + + auto req = mirror::PromoteRequest<>::create(*ictx, force, on_finish); + req->send(); + }); + if (ictx->state->is_refresh_required()) { + ictx->state->refresh(on_refresh); + } else { + on_refresh->complete(0); + } } template @@ -606,8 +619,21 @@ void Mirror::image_demote(I *ictx, Context *on_finish) { CephContext *cct = ictx->cct; ldout(cct, 20) << "ictx=" << ictx << dendl; - auto req = mirror::DemoteRequest<>::create(*ictx, on_finish); - req->send(); + auto on_refresh = new LambdaContext([ictx, on_finish](int r) { + if (r < 0) { + lderr(ictx->cct) << "refresh failed: " << cpp_strerror(r) << dendl; + on_finish->complete(r); + return; + } + + auto req = mirror::DemoteRequest<>::create(*ictx, on_finish); + req->send(); + }); + if (ictx->state->is_refresh_required()) { + ictx->state->refresh(on_refresh); + } else { + on_refresh->complete(0); + } } template @@ -1816,9 +1842,30 @@ int Mirror::image_snapshot_create(I *ictx, uint64_t *snap_id) { CephContext *cct = ictx->cct; ldout(cct, 20) << "ictx=" << ictx << dendl; + int r = ictx->state->refresh_if_required(); + if (r < 0) { + return r; + } + + cls::rbd::MirrorImage mirror_image; + r = cls_client::mirror_image_get(&ictx->md_ctx, ictx->id, + &mirror_image); + if (r == -ENOENT) { + return -EINVAL; + } else if (r < 0) { + lderr(cct) << "failed to retrieve mirror image" << dendl; + return r; + } + + if (mirror_image.mode != cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT || + mirror_image.state != cls::rbd::MIRROR_IMAGE_STATE_ENABLED) { + lderr(cct) << "snapshot based mirroring is not enabled" << dendl; + return r; + } + C_SaferCond on_finish; auto req = mirror::snapshot::CreatePrimaryRequest::create( - ictx, 0U, snap_id, &on_finish); + ictx, mirror_image.global_image_id, 0U, snap_id, &on_finish); req->send(); return on_finish.wait(); } diff --git a/src/librbd/mirror/DemoteRequest.cc b/src/librbd/mirror/DemoteRequest.cc index f70c2020c29..350a76d8342 100644 --- a/src/librbd/mirror/DemoteRequest.cc +++ b/src/librbd/mirror/DemoteRequest.cc @@ -135,7 +135,8 @@ void DemoteRequest::demote() { if (m_mirror_image.mode == cls::rbd::MIRROR_IMAGE_MODE_JOURNAL) { Journal::demote(&m_image_ctx, ctx); } else if (m_mirror_image.mode == cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT) { - auto req = mirror::snapshot::DemoteRequest::create(&m_image_ctx, ctx); + auto req = mirror::snapshot::DemoteRequest::create( + &m_image_ctx, m_mirror_image.global_image_id, ctx); req->send(); } else { lderr(cct) << "unknown image mirror mode: " << m_mirror_image.mode << dendl; diff --git a/src/librbd/mirror/PromoteRequest.cc b/src/librbd/mirror/PromoteRequest.cc index 2b829fb8a28..dc70e6af085 100644 --- a/src/librbd/mirror/PromoteRequest.cc +++ b/src/librbd/mirror/PromoteRequest.cc @@ -77,8 +77,8 @@ void PromoteRequest::promote() { if (m_mirror_image.mode == cls::rbd::MIRROR_IMAGE_MODE_JOURNAL) { Journal::promote(&m_image_ctx, ctx); } else if (m_mirror_image.mode == cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT) { - auto req = mirror::snapshot::PromoteRequest::create(&m_image_ctx, - m_force, ctx); + auto req = mirror::snapshot::PromoteRequest::create( + &m_image_ctx, m_mirror_image.global_image_id, m_force, ctx); req->send(); } else { lderr(cct) << "unknown image mirror mode: " << m_mirror_image.mode << dendl; diff --git a/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc b/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc index 4ec7967f98b..c4e85475468 100644 --- a/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc +++ b/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc @@ -26,91 +26,17 @@ using librbd::util::create_context_callback; using librbd::util::create_rados_callback; template -CreatePrimaryRequest::CreatePrimaryRequest(I *image_ctx, uint32_t flags, - uint64_t *snap_id, - Context *on_finish) - : m_image_ctx(image_ctx), m_flags(flags), - m_snap_id(snap_id), m_on_finish(on_finish) { +CreatePrimaryRequest::CreatePrimaryRequest( + I *image_ctx, const std::string& global_image_id, uint32_t flags, + uint64_t *snap_id, Context *on_finish) + : m_image_ctx(image_ctx), m_global_image_id(global_image_id), + m_flags(flags), m_snap_id(snap_id), m_on_finish(on_finish) { m_default_ns_ctx.dup(m_image_ctx->md_ctx); m_default_ns_ctx.set_namespace(""); } template void CreatePrimaryRequest::send() { - refresh_image(); -} - -template -void CreatePrimaryRequest::refresh_image() { - if (!m_image_ctx->state->is_refresh_required()) { - get_mirror_image(); - return; - } - - CephContext *cct = m_image_ctx->cct; - ldout(cct, 20) << dendl; - - auto ctx = create_context_callback< - CreatePrimaryRequest, - &CreatePrimaryRequest::handle_refresh_image>(this); - m_image_ctx->state->refresh(ctx); -} - -template -void CreatePrimaryRequest::handle_refresh_image(int r) { - CephContext *cct = m_image_ctx->cct; - ldout(cct, 20) << "r=" << r << dendl; - - if (r < 0) { - lderr(cct) << "failed to refresh image: " << cpp_strerror(r) << dendl; - finish(r); - return; - } - - get_mirror_image(); -} - -template -void CreatePrimaryRequest::get_mirror_image() { - CephContext *cct = m_image_ctx->cct; - ldout(cct, 20) << dendl; - - librados::ObjectReadOperation op; - cls_client::mirror_image_get_start(&op, m_image_ctx->id); - - librados::AioCompletion *comp = create_rados_callback< - CreatePrimaryRequest, - &CreatePrimaryRequest::handle_get_mirror_image>(this); - int r = m_image_ctx->md_ctx.aio_operate(RBD_MIRRORING, comp, &op, &m_out_bl); - ceph_assert(r == 0); - comp->release(); -} - -template -void CreatePrimaryRequest::handle_get_mirror_image(int r) { - CephContext *cct = m_image_ctx->cct; - ldout(cct, 20) << "r=" << r << dendl; - - cls::rbd::MirrorImage mirror_image; - if (r == 0) { - auto iter = m_out_bl.cbegin(); - r = cls_client::mirror_image_get_finish(&iter, &mirror_image); - } - - if (r < 0 && r != -ENOENT) { - lderr(cct) << "failed to retrieve mirroring state: " << cpp_strerror(r) - << dendl; - finish(r); - return; - } - - if (mirror_image.mode != cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT || - mirror_image.state != cls::rbd::MIRROR_IMAGE_STATE_ENABLED) { - lderr(cct) << "snapshot based mirroring is not enabled" << dendl; - finish(-EINVAL); - return; - } - if (!util::can_create_primary_snapshot( m_image_ctx, ((m_flags & CREATE_PRIMARY_FLAG_DEMOTED) != 0), @@ -121,7 +47,7 @@ void CreatePrimaryRequest::handle_get_mirror_image(int r) { uuid_d uuid_gen; uuid_gen.generate_random(); - m_snap_name = ".mirror.primary." + mirror_image.global_image_id + "." + + m_snap_name = ".mirror.primary." + m_global_image_id + "." + uuid_gen.to_string(); get_mirror_peers(); @@ -136,7 +62,8 @@ void CreatePrimaryRequest::get_mirror_peers() { cls_client::mirror_peer_list_start(&op); librados::AioCompletion *comp = create_rados_callback< - CreatePrimaryRequest, &CreatePrimaryRequest::handle_get_mirror_peers>(this); + CreatePrimaryRequest, + &CreatePrimaryRequest::handle_get_mirror_peers>(this); m_out_bl.clear(); int r = m_default_ns_ctx.aio_operate(RBD_MIRRORING, comp, &op, &m_out_bl); ceph_assert(r == 0); diff --git a/src/librbd/mirror/snapshot/CreatePrimaryRequest.h b/src/librbd/mirror/snapshot/CreatePrimaryRequest.h index 022cc95efc3..aa00abe0284 100644 --- a/src/librbd/mirror/snapshot/CreatePrimaryRequest.h +++ b/src/librbd/mirror/snapshot/CreatePrimaryRequest.h @@ -24,14 +24,17 @@ namespace snapshot { template class CreatePrimaryRequest { public: - static CreatePrimaryRequest *create(ImageCtxT *image_ctx, uint32_t flags, - uint64_t *snap_id, + static CreatePrimaryRequest *create(ImageCtxT *image_ctx, + const std::string& global_image_id, + uint32_t flags, uint64_t *snap_id, Context *on_finish) { - return new CreatePrimaryRequest(image_ctx, flags, snap_id, on_finish); + return new CreatePrimaryRequest(image_ctx, global_image_id, flags, snap_id, + on_finish); } - CreatePrimaryRequest(ImageCtxT *image_ctx, uint32_t flags, uint64_t *snap_id, - Context *on_finish); + CreatePrimaryRequest(ImageCtxT *image_ctx, + const std::string& global_image_id, + uint32_t flags, uint64_t *snap_id, Context *on_finish); void send(); @@ -42,12 +45,6 @@ private: * * | * v - * REFRESH_IMAGE - * | - * v - * GET_MIRROR_IMAGE - * | - * v * GET_MIRROR_PEERS * | * v @@ -63,6 +60,7 @@ private: */ ImageCtxT *m_image_ctx; + std::string m_global_image_id; const uint32_t m_flags; uint64_t *m_snap_id; Context *m_on_finish; @@ -73,12 +71,6 @@ private: bufferlist m_out_bl; - void refresh_image(); - void handle_refresh_image(int r); - - void get_mirror_image(); - void handle_get_mirror_image(int r); - void get_mirror_peers(); void handle_get_mirror_peers(int r); diff --git a/src/librbd/mirror/snapshot/DemoteRequest.cc b/src/librbd/mirror/snapshot/DemoteRequest.cc index 62fd7f45d74..43d555990b1 100644 --- a/src/librbd/mirror/snapshot/DemoteRequest.cc +++ b/src/librbd/mirror/snapshot/DemoteRequest.cc @@ -24,35 +24,6 @@ using librbd::util::create_context_callback; template void DemoteRequest::send() { - refresh_image(); -} - -template -void DemoteRequest::refresh_image() { - if (!m_image_ctx->state->is_refresh_required()) { - create_snapshot(); - return; - } - - CephContext *cct = m_image_ctx->cct; - ldout(cct, 20) << dendl; - - auto ctx = create_context_callback< - DemoteRequest, &DemoteRequest::handle_refresh_image>(this); - m_image_ctx->state->refresh(ctx); -} - -template -void DemoteRequest::handle_refresh_image(int r) { - CephContext *cct = m_image_ctx->cct; - ldout(cct, 20) << "r=" << r << dendl; - - if (r < 0) { - lderr(cct) << "failed to refresh image: " << cpp_strerror(r) << dendl; - finish(r); - return; - } - create_snapshot(); } @@ -65,7 +36,7 @@ void DemoteRequest::create_snapshot() { DemoteRequest, &DemoteRequest::handle_create_snapshot>(this); auto req = CreatePrimaryRequest::create( - m_image_ctx, + m_image_ctx, m_global_image_id, (snapshot::CREATE_PRIMARY_FLAG_IGNORE_EMPTY_PEERS | snapshot::CREATE_PRIMARY_FLAG_DEMOTED), nullptr, ctx); req->send(); diff --git a/src/librbd/mirror/snapshot/DemoteRequest.h b/src/librbd/mirror/snapshot/DemoteRequest.h index f075561e3ef..94a2eb333e0 100644 --- a/src/librbd/mirror/snapshot/DemoteRequest.h +++ b/src/librbd/mirror/snapshot/DemoteRequest.h @@ -21,12 +21,16 @@ namespace snapshot { template class DemoteRequest { public: - static DemoteRequest *create(ImageCtxT *image_ctx, Context *on_finish) { - return new DemoteRequest(image_ctx, on_finish); + static DemoteRequest *create(ImageCtxT *image_ctx, + const std::string& global_image_id, + Context *on_finish) { + return new DemoteRequest(image_ctx, global_image_id, on_finish); } - DemoteRequest(ImageCtxT *image_ctx, Context *on_finish) - : m_image_ctx(image_ctx), m_on_finish(on_finish) { + DemoteRequest(ImageCtxT *image_ctx, const std::string& global_image_id, + Context *on_finish) + : m_image_ctx(image_ctx), m_global_image_id(global_image_id), + m_on_finish(on_finish) { } void send(); @@ -38,9 +42,6 @@ private: * * | * v - * REFRESH_IMAGE - * | - * v * CREATE_SNAPSHOT * | * v @@ -50,11 +51,9 @@ private: */ ImageCtxT *m_image_ctx; + std::string m_global_image_id; Context *m_on_finish; - void refresh_image(); - void handle_refresh_image(int r); - void create_snapshot(); void handle_create_snapshot(int r); diff --git a/src/librbd/mirror/snapshot/PromoteRequest.cc b/src/librbd/mirror/snapshot/PromoteRequest.cc index 3fb28cf16ea..347813f6a5c 100644 --- a/src/librbd/mirror/snapshot/PromoteRequest.cc +++ b/src/librbd/mirror/snapshot/PromoteRequest.cc @@ -30,35 +30,7 @@ using librbd::util::create_context_callback; template void PromoteRequest::send() { - refresh_image(); -} - -template -void PromoteRequest::refresh_image() { - if (!m_image_ctx->state->is_refresh_required()) { - handle_refresh_image(0); - return; - } - - CephContext *cct = m_image_ctx->cct; - ldout(cct, 20) << dendl; - - auto ctx = create_context_callback< - PromoteRequest, &PromoteRequest::handle_refresh_image>(this); - m_image_ctx->state->refresh(ctx); -} - -template -void PromoteRequest::handle_refresh_image(int r) { CephContext *cct = m_image_ctx->cct; - ldout(cct, 20) << "r=" << r << dendl; - - if (r < 0) { - lderr(cct) << "failed to refresh image: " << cpp_strerror(r) << dendl; - finish(r); - return; - } - if (!util::can_create_primary_snapshot(m_image_ctx, false, true, &m_rollback_snap_id)) { lderr(cct) << "cannot promote" << dendl; @@ -323,7 +295,7 @@ void PromoteRequest::create_promote_snapshot() { &PromoteRequest::handle_create_promote_snapshot>(this); auto req = CreatePrimaryRequest::create( - m_image_ctx, + m_image_ctx, m_global_image_id, (snapshot::CREATE_PRIMARY_FLAG_IGNORE_EMPTY_PEERS | snapshot::CREATE_PRIMARY_FLAG_FORCE), nullptr, ctx); req->send(); diff --git a/src/librbd/mirror/snapshot/PromoteRequest.h b/src/librbd/mirror/snapshot/PromoteRequest.h index f6eb92b61da..7555842f2ce 100644 --- a/src/librbd/mirror/snapshot/PromoteRequest.h +++ b/src/librbd/mirror/snapshot/PromoteRequest.h @@ -24,13 +24,17 @@ namespace snapshot { template class PromoteRequest { public: - static PromoteRequest *create(ImageCtxT *image_ctx, bool force, - Context *on_finish) { - return new PromoteRequest(image_ctx, force, on_finish); + static PromoteRequest *create(ImageCtxT *image_ctx, + const std::string& global_image_id, + bool force, Context *on_finish) { + return new PromoteRequest(image_ctx, global_image_id, force, on_finish); } - PromoteRequest(ImageCtxT *image_ctx, bool force, Context *on_finish) - : m_image_ctx(image_ctx), m_force(force), m_on_finish(on_finish) { + PromoteRequest(ImageCtxT *image_ctx, + const std::string& global_image_id, bool force, + Context *on_finish) + : m_image_ctx(image_ctx), m_global_image_id(global_image_id), + m_force(force), m_on_finish(on_finish) { } void send(); @@ -41,8 +45,8 @@ private: * * * | - * v (can promote) - * REFRESH_IMAGE -------------------------------\ + * | (can promote) + * |\----------------------------------------\ * | | * | (needs rollback) | * v | @@ -72,6 +76,7 @@ private: */ ImageCtxT *m_image_ctx; + std::string m_global_image_id; bool m_force; Context *m_on_finish; diff --git a/src/test/librbd/mirror/snapshot/test_mock_CreatePrimaryRequest.cc b/src/test/librbd/mirror/snapshot/test_mock_CreatePrimaryRequest.cc index 6fda820caeb..279cf858a89 100644 --- a/src/test/librbd/mirror/snapshot/test_mock_CreatePrimaryRequest.cc +++ b/src/test/librbd/mirror/snapshot/test_mock_CreatePrimaryRequest.cc @@ -123,30 +123,6 @@ public: })); } - void expect_refresh_image(MockTestImageCtx &mock_image_ctx, - bool refresh_required, int r) { - EXPECT_CALL(*mock_image_ctx.state, is_refresh_required()) - .WillOnce(Return(refresh_required)); - if (refresh_required) { - EXPECT_CALL(*mock_image_ctx.state, refresh(_)) - .WillOnce(CompleteContext(r, mock_image_ctx.image_ctx->op_work_queue)); - } - } - - void expect_get_mirror_image(MockTestImageCtx &mock_image_ctx, - const cls::rbd::MirrorImage &mirror_image, - int r) { - using ceph::encode; - bufferlist bl; - encode(mirror_image, bl); - - EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), - exec(RBD_MIRRORING, _, StrEq("rbd"), StrEq("mirror_image_get"), - _, _, _)) - .WillOnce(DoAll(WithArg<5>(CopyInBufferlist(bl)), - Return(r))); - } - void expect_can_create_primary_snapshot(MockUtils &mock_utils, bool demoted, bool force, bool result) { EXPECT_CALL(mock_utils, @@ -225,10 +201,6 @@ TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, Success) { InSequence seq; expect_clone_md_ctx(mock_image_ctx); - expect_refresh_image(mock_image_ctx, true, 0); - expect_get_mirror_image( - mock_image_ctx, {cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT, "gid", - cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, 0); MockUtils mock_utils; expect_can_create_primary_snapshot(mock_utils, false, false, true); expect_get_mirror_peers(mock_image_ctx, @@ -237,52 +209,12 @@ TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, Success) { expect_create_snapshot(mock_image_ctx, 0); C_SaferCond ctx; - auto req = new MockCreatePrimaryRequest(&mock_image_ctx, 0U, nullptr, &ctx); + auto req = new MockCreatePrimaryRequest(&mock_image_ctx, "gid", 0U, nullptr, + &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } -TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, RefreshError) { - REQUIRE_FORMAT_V2(); - - librbd::ImageCtx *ictx; - ASSERT_EQ(0, open_image(m_image_name, &ictx)); - - MockTestImageCtx mock_image_ctx(*ictx); - - InSequence seq; - - expect_clone_md_ctx(mock_image_ctx); - expect_refresh_image(mock_image_ctx, true, -EINVAL); - - C_SaferCond ctx; - auto req = new MockCreatePrimaryRequest(&mock_image_ctx, 0U, nullptr, &ctx); - req->send(); - ASSERT_EQ(-EINVAL, ctx.wait()); -} - -TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, GetMirrorImageError) { - REQUIRE_FORMAT_V2(); - - librbd::ImageCtx *ictx; - ASSERT_EQ(0, open_image(m_image_name, &ictx)); - - MockTestImageCtx mock_image_ctx(*ictx); - - InSequence seq; - - expect_clone_md_ctx(mock_image_ctx); - expect_refresh_image(mock_image_ctx, false, 0); - expect_get_mirror_image( - mock_image_ctx, {cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT, "gid", - cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, -EINVAL); - - C_SaferCond ctx; - auto req = new MockCreatePrimaryRequest(&mock_image_ctx, 0U, nullptr, &ctx); - req->send(); - ASSERT_EQ(-EINVAL, ctx.wait()); -} - TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, CanNotError) { REQUIRE_FORMAT_V2(); @@ -294,15 +226,12 @@ TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, CanNotError) { InSequence seq; expect_clone_md_ctx(mock_image_ctx); - expect_refresh_image(mock_image_ctx, true, 0); - expect_get_mirror_image( - mock_image_ctx, {cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT, "gid", - cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, 0); MockUtils mock_utils; expect_can_create_primary_snapshot(mock_utils, false, false, false); C_SaferCond ctx; - auto req = new MockCreatePrimaryRequest(&mock_image_ctx, 0U, nullptr, &ctx); + auto req = new MockCreatePrimaryRequest(&mock_image_ctx, "gid", 0U, nullptr, + &ctx); req->send(); ASSERT_EQ(-EINVAL, ctx.wait()); } @@ -318,10 +247,6 @@ TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, GetMirrorPeersError) { InSequence seq; expect_clone_md_ctx(mock_image_ctx); - expect_refresh_image(mock_image_ctx, true, 0); - expect_get_mirror_image( - mock_image_ctx, {cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT, "gid", - cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, 0); MockUtils mock_utils; expect_can_create_primary_snapshot(mock_utils, false, false, true); expect_get_mirror_peers(mock_image_ctx, @@ -329,7 +254,8 @@ TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, GetMirrorPeersError) { "mirror", "fsid"}}, -EINVAL); C_SaferCond ctx; - auto req = new MockCreatePrimaryRequest(&mock_image_ctx, 0U, nullptr, &ctx); + auto req = new MockCreatePrimaryRequest(&mock_image_ctx, "gid", 0U, nullptr, + &ctx); req->send(); ASSERT_EQ(-EINVAL, ctx.wait()); } @@ -345,10 +271,6 @@ TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, CreateSnapshotError) { InSequence seq; expect_clone_md_ctx(mock_image_ctx); - expect_refresh_image(mock_image_ctx, true, 0); - expect_get_mirror_image( - mock_image_ctx, {cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT, "gid", - cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, 0); MockUtils mock_utils; expect_can_create_primary_snapshot(mock_utils, false, false, true); expect_get_mirror_peers(mock_image_ctx, @@ -357,7 +279,8 @@ TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, CreateSnapshotError) { expect_create_snapshot(mock_image_ctx, -EINVAL); C_SaferCond ctx; - auto req = new MockCreatePrimaryRequest(&mock_image_ctx, 0U, nullptr, &ctx); + auto req = new MockCreatePrimaryRequest(&mock_image_ctx, "gid", 0U, nullptr, + &ctx); req->send(); ASSERT_EQ(-EINVAL, ctx.wait()); } @@ -378,10 +301,6 @@ TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, SuccessUnlinkPeer) { InSequence seq; expect_clone_md_ctx(mock_image_ctx); - expect_refresh_image(mock_image_ctx, true, 0); - expect_get_mirror_image( - mock_image_ctx, {cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT, "gid", - cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, 0); MockUtils mock_utils; expect_can_create_primary_snapshot(mock_utils, false, false, true); expect_get_mirror_peers(mock_image_ctx, @@ -394,7 +313,8 @@ TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, SuccessUnlinkPeer) { expect_unlink_peer(mock_image_ctx, mock_unlink_peer_request, snap_id, "uuid", 0); C_SaferCond ctx; - auto req = new MockCreatePrimaryRequest(&mock_image_ctx, 0U, nullptr, &ctx); + auto req = new MockCreatePrimaryRequest(&mock_image_ctx, "gid", 0U, nullptr, + &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } diff --git a/src/test/librbd/mirror/snapshot/test_mock_PromoteRequest.cc b/src/test/librbd/mirror/snapshot/test_mock_PromoteRequest.cc index f0a49971317..76f73e5e0a0 100644 --- a/src/test/librbd/mirror/snapshot/test_mock_PromoteRequest.cc +++ b/src/test/librbd/mirror/snapshot/test_mock_PromoteRequest.cc @@ -117,6 +117,7 @@ struct CreatePrimaryRequest { Context* on_finish = nullptr; static CreatePrimaryRequest* s_instance; static CreatePrimaryRequest *create(MockTestImageCtx *image_ctx, + const std::string& global_image_id, uint32_t flags, uint64_t *snap_id, Context *on_finish) { ceph_assert(s_instance != nullptr); @@ -163,16 +164,6 @@ public: typedef CreatePrimaryRequest MockCreatePrimaryRequest; typedef util::Mock MockUtils; - void expect_refresh_image(MockTestImageCtx &mock_image_ctx, - bool refresh_required, int r) { - EXPECT_CALL(*mock_image_ctx.state, is_refresh_required()) - .WillOnce(Return(refresh_required)); - if (refresh_required) { - EXPECT_CALL(*mock_image_ctx.state, refresh(_)) - .WillOnce(CompleteContext(r, mock_image_ctx.image_ctx->op_work_queue)); - } - } - void expect_can_create_primary_snapshot(MockUtils &mock_utils, bool force, uint64_t rollback_snap_id, bool result) { @@ -273,14 +264,13 @@ TEST_F(TestMockMirrorSnapshotPromoteRequest, Success) { InSequence seq; - expect_refresh_image(mock_image_ctx, true, 0); MockUtils mock_utils; expect_can_create_primary_snapshot(mock_utils, true, CEPH_NOSNAP, true); MockCreatePrimaryRequest mock_create_primary_request; expect_create_promote_snapshot(mock_image_ctx, mock_create_primary_request, 0); C_SaferCond ctx; - auto req = new MockPromoteRequest(&mock_image_ctx, force, &ctx); + auto req = new MockPromoteRequest(&mock_image_ctx, "gid", force, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } @@ -303,7 +293,6 @@ TEST_F(TestMockMirrorSnapshotPromoteRequest, SuccessRollback) { InSequence seq; - expect_refresh_image(mock_image_ctx, true, 0); MockUtils mock_utils; expect_can_create_primary_snapshot(mock_utils, true, 123, true); MockCreateNonPrimaryRequest mock_create_non_primary_request; @@ -322,29 +311,11 @@ TEST_F(TestMockMirrorSnapshotPromoteRequest, SuccessRollback) { expect_release_lock(mock_image_ctx, 0); C_SaferCond ctx; - auto req = new MockPromoteRequest(&mock_image_ctx, force, &ctx); + auto req = new MockPromoteRequest(&mock_image_ctx, "gid", force, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } -TEST_F(TestMockMirrorSnapshotPromoteRequest, RefreshError) { - REQUIRE_FORMAT_V2(); - - librbd::ImageCtx *ictx; - ASSERT_EQ(0, open_image(m_image_name, &ictx)); - - MockTestImageCtx mock_image_ctx(*ictx); - - InSequence seq; - - expect_refresh_image(mock_image_ctx, true, -EINVAL); - - C_SaferCond ctx; - auto req = new MockPromoteRequest(&mock_image_ctx, false, &ctx); - req->send(); - ASSERT_EQ(-EINVAL, ctx.wait()); -} - TEST_F(TestMockMirrorSnapshotPromoteRequest, ErrorCannotRollback) { REQUIRE_FORMAT_V2(); @@ -358,12 +329,11 @@ TEST_F(TestMockMirrorSnapshotPromoteRequest, ErrorCannotRollback) { InSequence seq; - expect_refresh_image(mock_image_ctx, true, 0); MockUtils mock_utils; expect_can_create_primary_snapshot(mock_utils, true, CEPH_NOSNAP, false); C_SaferCond ctx; - auto req = new MockPromoteRequest(&mock_image_ctx, force, &ctx); + auto req = new MockPromoteRequest(&mock_image_ctx, "gid", force, &ctx); req->send(); ASSERT_EQ(-EINVAL, ctx.wait()); } -- 2.39.5