From fe550f5e52d91eda27747ad539be9b6398f42471 Mon Sep 17 00:00:00 2001 From: Mykola Golub Date: Thu, 28 Nov 2019 11:04:35 +0000 Subject: [PATCH] librbd: refactor CreateNonPrimaryRequest and CreatePrimaryRequest to extract re-usable functions into utils Signed-off-by: Mykola Golub --- src/librbd/CMakeLists.txt | 1 + .../snapshot/CreateNonPrimaryRequest.cc | 44 +---- .../mirror/snapshot/CreateNonPrimaryRequest.h | 2 - .../mirror/snapshot/CreatePrimaryRequest.cc | 57 +----- .../mirror/snapshot/CreatePrimaryRequest.h | 2 - src/librbd/mirror/snapshot/Utils.cc | 182 ++++++++++++++++++ src/librbd/mirror/snapshot/Utils.h | 15 +- src/test/librbd/CMakeLists.txt | 1 + .../test_mock_CreateNonPrimaryRequest.cc | 149 +++++--------- .../test_mock_CreatePrimaryRequest.cc | 112 +++++------ .../librbd/mirror/snapshot/test_mock_Utils.cc | 170 ++++++++++++++++ 11 files changed, 469 insertions(+), 266 deletions(-) create mode 100644 src/librbd/mirror/snapshot/Utils.cc create mode 100644 src/test/librbd/mirror/snapshot/test_mock_Utils.cc diff --git a/src/librbd/CMakeLists.txt b/src/librbd/CMakeLists.txt index f17765b99fb..7376c2ffcd0 100644 --- a/src/librbd/CMakeLists.txt +++ b/src/librbd/CMakeLists.txt @@ -113,6 +113,7 @@ set(librbd_internal_srcs mirror/snapshot/SetImageStateRequest.cc mirror/snapshot/Types.cc mirror/snapshot/UnlinkPeerRequest.cc + mirror/snapshot/Utils.cc mirror/snapshot/WriteImageStateRequest.cc object_map/CreateRequest.cc object_map/InvalidateRequest.cc diff --git a/src/librbd/mirror/snapshot/CreateNonPrimaryRequest.cc b/src/librbd/mirror/snapshot/CreateNonPrimaryRequest.cc index 215499a1d44..426117b69b7 100644 --- a/src/librbd/mirror/snapshot/CreateNonPrimaryRequest.cc +++ b/src/librbd/mirror/snapshot/CreateNonPrimaryRequest.cc @@ -9,6 +9,7 @@ #include "librbd/ImageState.h" #include "librbd/Operations.h" #include "librbd/Utils.h" +#include "librbd/mirror/snapshot/Utils.h" #include "librbd/mirror/snapshot/WriteImageStateRequest.h" #define dout_subsys ceph_subsys_rbd @@ -99,7 +100,7 @@ void CreateNonPrimaryRequest::handle_get_mirror_image(int r) { return; } - if (!validate_snapshot()) { + if (!util::can_create_non_primary_snapshot(m_image_ctx)) { finish(-EINVAL); return; } @@ -190,47 +191,6 @@ void CreateNonPrimaryRequest::finish(int r) { delete this; } -template -bool CreateNonPrimaryRequest::validate_snapshot() { - CephContext *cct = m_image_ctx->cct; - - std::shared_lock image_locker{m_image_ctx->image_lock}; - - for (auto it = m_image_ctx->snap_info.rbegin(); - it != m_image_ctx->snap_info.rend(); it++) { - auto primary = boost::get( - &it->second.snap_namespace); - if (primary != nullptr) { - ldout(cct, 20) << "previous mirror snapshot snap_id=" << it->first << " " - << *primary << dendl; - if (!primary->demoted) { - lderr(cct) << "trying to create non-primary snapshot " - << "when previous primary snapshot is not in demoted state" - << dendl; - return false; - } - return true; - } - auto non_primary = boost::get( - &it->second.snap_namespace); - if (non_primary == nullptr) { - continue; - } - ldout(cct, 20) << "previous snapshot snap_id=" << it->first << " " - << *non_primary << dendl; - if (!non_primary->copied) { - lderr(cct) << "trying to create non-primary snapshot " - << "when previous non-primary snapshot is not copied yet" - << dendl; - return false; - } - return true; - } - - ldout(cct, 20) << "no previous mirror snapshots found" << dendl; - return true; -} - } // namespace snapshot } // namespace mirror } // namespace librbd diff --git a/src/librbd/mirror/snapshot/CreateNonPrimaryRequest.h b/src/librbd/mirror/snapshot/CreateNonPrimaryRequest.h index 60f56f344be..a2ab6adc5c4 100644 --- a/src/librbd/mirror/snapshot/CreateNonPrimaryRequest.h +++ b/src/librbd/mirror/snapshot/CreateNonPrimaryRequest.h @@ -97,8 +97,6 @@ private: void handle_write_image_state(int r); void finish(int r); - - bool validate_snapshot(); }; } // namespace snapshot diff --git a/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc b/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc index d325667373b..b040c407c17 100644 --- a/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc +++ b/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc @@ -10,6 +10,7 @@ #include "librbd/Operations.h" #include "librbd/Utils.h" #include "librbd/mirror/snapshot/UnlinkPeerRequest.h" +#include "librbd/mirror/snapshot/Utils.h" #define dout_subsys ceph_subsys_rbd @@ -100,7 +101,8 @@ void CreatePrimaryRequest::handle_get_mirror_image(int r) { return; } - if (!validate_snapshot()) { + if (!util::can_create_primary_snapshot(m_image_ctx, m_demoted, m_force, + nullptr)) { finish(-EINVAL); return; } @@ -283,59 +285,6 @@ void CreatePrimaryRequest::finish(int r) { delete this; } -template -bool CreatePrimaryRequest::validate_snapshot() { - CephContext *cct = m_image_ctx->cct; - - std::shared_lock image_locker{m_image_ctx->image_lock}; - - for (auto it = m_image_ctx->snap_info.rbegin(); - it != m_image_ctx->snap_info.rend(); it++) { - auto non_primary = boost::get( - &it->second.snap_namespace); - if (non_primary != nullptr) { - ldout(cct, 20) << "previous mirror snapshot snap_id=" << it->first << " " - << *non_primary << dendl; - if (!m_force) { - lderr(cct) << "trying to create primary snapshot without force " - << "when previous snapshot is non-primary" - << dendl; - return false; - } - if (m_demoted) { - lderr(cct) << "trying to create primary demoted snapshot " - << "when previous snapshot is non-primary" - << dendl; - return false; - } - if (!non_primary->copied) { - lderr(cct) << "trying to create primary snapshot " - << "when previous non-primary snapshot is not copied yet" - << dendl; - return false; - } - return true; - } - auto primary = boost::get( - &it->second.snap_namespace); - if (primary == nullptr) { - continue; - } - ldout(cct, 20) << "previous snapshot snap_id=" << it->first << " " - << *primary << dendl; - if (primary->demoted && !m_force) { - lderr(cct) << "trying to create primary snapshot without force " - << "when previous primary snapshot is demoted" - << dendl; - return false; - } - return true; - } - - ldout(cct, 20) << "no previous mirror snapshots found" << dendl; - return true; -} - } // namespace snapshot } // namespace mirror } // namespace librbd diff --git a/src/librbd/mirror/snapshot/CreatePrimaryRequest.h b/src/librbd/mirror/snapshot/CreatePrimaryRequest.h index 9f0b951b723..9291fc45b5d 100644 --- a/src/librbd/mirror/snapshot/CreatePrimaryRequest.h +++ b/src/librbd/mirror/snapshot/CreatePrimaryRequest.h @@ -91,8 +91,6 @@ private: void handle_unlink_peer(int r); void finish(int r); - - bool validate_snapshot(); }; } // namespace snapshot diff --git a/src/librbd/mirror/snapshot/Utils.cc b/src/librbd/mirror/snapshot/Utils.cc new file mode 100644 index 00000000000..6f362f7e0c7 --- /dev/null +++ b/src/librbd/mirror/snapshot/Utils.cc @@ -0,0 +1,182 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#include "common/dout.h" +#include "common/errno.h" +#include "include/stringify.h" +#include "librbd/ImageCtx.h" +#include "librbd/mirror/snapshot/Utils.h" + +#define dout_subsys ceph_subsys_rbd + +#undef dout_prefix +#define dout_prefix *_dout << "librbd::mirror::snapshot::util: " \ + << " " << __func__ << ": " + +namespace librbd { +namespace mirror { +namespace snapshot { +namespace util { + +namespace { + +const std::string IMAGE_STATE_OBJECT_PREFIX = "rbd_mirror_snapshot."; + +bool get_rollback_snap_id( + std::map::reverse_iterator it, + std::map::reverse_iterator end, + uint64_t *rollback_snap_id) { + + for (; it != end; it++) { + auto primary = boost::get( + &it->second.snap_namespace); + if (primary != nullptr) { + break; + } + + auto non_primary = boost::get( + &it->second.snap_namespace); + if (non_primary->copied) { + break; + } + } + + if (it != end) { + *rollback_snap_id = it->first; + return true; + } + + return false; +} + +} // anonymous namespace + +template +bool can_create_primary_snapshot(I *image_ctx, bool demoted, bool force, + uint64_t *rollback_snap_id) { + CephContext *cct = image_ctx->cct; + + if (rollback_snap_id) { + *rollback_snap_id = CEPH_NOSNAP; + } + + std::shared_lock image_locker{image_ctx->image_lock}; + + for (auto it = image_ctx->snap_info.rbegin(); + it != image_ctx->snap_info.rend(); it++) { + auto non_primary = boost::get( + &it->second.snap_namespace); + if (non_primary != nullptr) { + ldout(cct, 20) << "previous mirror snapshot snap_id=" << it->first << " " + << *non_primary << dendl; + if (!force) { + lderr(cct) << "trying to create primary snapshot without force " + << "when previous snapshot is non-primary" + << dendl; + return false; + } + if (demoted) { + lderr(cct) << "trying to create primary demoted snapshot " + << "when previous snapshot is non-primary" + << dendl; + return false; + } + if (!non_primary->primary_mirror_uuid.empty() && !non_primary->copied) { + ldout(cct, 20) << "needs rollback" << dendl; + if (!rollback_snap_id) { + lderr(cct) << "trying to create primary snapshot " + << "when previous non-primary snapshot is not copied yet" + << dendl; + return false; + } + if (!get_rollback_snap_id(++it, image_ctx->snap_info.rend(), + rollback_snap_id)) { + lderr(cct) << "cannot rollback" << dendl; + return false; + } + ldout(cct, 20) << "rollback_snap_id=" << *rollback_snap_id << dendl; + } + return true; + } + auto primary = boost::get( + &it->second.snap_namespace); + if (primary == nullptr) { + continue; + } + ldout(cct, 20) << "previous snapshot snap_id=" << it->first << " " + << *primary << dendl; + if (primary->demoted && !force) { + lderr(cct) << "trying to create primary snapshot without force " + << "when previous primary snapshot is demoted" + << dendl; + return false; + } + return true; + } + + ldout(cct, 20) << "no previous mirror snapshots found" << dendl; + return true; +} + +template +bool can_create_non_primary_snapshot(I *image_ctx) { + CephContext *cct = image_ctx->cct; + + std::shared_lock image_locker{image_ctx->image_lock}; + + for (auto it = image_ctx->snap_info.rbegin(); + it != image_ctx->snap_info.rend(); it++) { + auto primary = boost::get( + &it->second.snap_namespace); + if (primary != nullptr) { + ldout(cct, 20) << "previous mirror snapshot snap_id=" << it->first << " " + << *primary << dendl; + if (!primary->demoted) { + lderr(cct) << "trying to create non-primary snapshot " + << "when previous primary snapshot is not in demoted state" + << dendl; + return false; + } + return true; + } + auto non_primary = boost::get( + &it->second.snap_namespace); + if (non_primary == nullptr) { + continue; + } + ldout(cct, 20) << "previous snapshot snap_id=" << it->first << " " + << *non_primary << dendl; + if (!non_primary->copied) { + lderr(cct) << "trying to create non-primary snapshot " + << "when previous non-primary snapshot is not copied yet" + << dendl; + return false; + } + return true; + } + + ldout(cct, 20) << "no previous mirror snapshots found" << dendl; + return true; +} + +template +std::string image_state_object_name(I *image_ctx, uint64_t snap_id, + uint64_t index) { + return IMAGE_STATE_OBJECT_PREFIX + image_ctx->id + "." + + stringify(snap_id) + "." + stringify(index); +} + +} // namespace util +} // namespace snapshot +} // namespace mirror +} // namespace librbd + +template bool librbd::mirror::snapshot::util::can_create_primary_snapshot( + librbd::ImageCtx *image_ctx, bool demoted, bool force, + uint64_t *rollback_snap_id); + +template bool librbd::mirror::snapshot::util::can_create_non_primary_snapshot( + librbd::ImageCtx *image_ctx); + +template std::string librbd::mirror::snapshot::util::image_state_object_name( + librbd::ImageCtx *image_ctx, uint64_t snap_id, uint64_t index); diff --git a/src/librbd/mirror/snapshot/Utils.h b/src/librbd/mirror/snapshot/Utils.h index daf82f26740..a6fce636ffb 100644 --- a/src/librbd/mirror/snapshot/Utils.h +++ b/src/librbd/mirror/snapshot/Utils.h @@ -7,18 +7,23 @@ #include "include/stringify.h" namespace librbd { + +struct ImageCtx; + namespace mirror { namespace snapshot { namespace util { -const std::string IMAGE_STATE_OBJECT_PREFIX = "rbd_mirror_snapshot."; +template +bool can_create_primary_snapshot(ImageCtxT *image_ctx, bool demoted, bool force, + uint64_t *rollback_snap_id); + +template +bool can_create_non_primary_snapshot(ImageCtxT *image_ctx); template std::string image_state_object_name(ImageCtxT *image_ctx, uint64_t snap_id, - uint64_t index) { - return IMAGE_STATE_OBJECT_PREFIX + image_ctx->id + "." + - stringify(snap_id) + "." + stringify(index); -} + uint64_t index); } // namespace util } // namespace snapshot diff --git a/src/test/librbd/CMakeLists.txt b/src/test/librbd/CMakeLists.txt index 501881a8a8f..19cc88bfb2e 100644 --- a/src/test/librbd/CMakeLists.txt +++ b/src/test/librbd/CMakeLists.txt @@ -88,6 +88,7 @@ set(unittest_librbd_srcs mirror/snapshot/test_mock_CreateNonPrimaryRequest.cc mirror/snapshot/test_mock_CreatePrimaryRequest.cc mirror/snapshot/test_mock_UnlinkPeerRequest.cc + mirror/snapshot/test_mock_Utils.cc mirror/test_mock_DisableRequest.cc object_map/test_mock_InvalidateRequest.cc object_map/test_mock_LockRequest.cc diff --git a/src/test/librbd/mirror/snapshot/test_mock_CreateNonPrimaryRequest.cc b/src/test/librbd/mirror/snapshot/test_mock_CreateNonPrimaryRequest.cc index 03bdac2503a..29918c79316 100644 --- a/src/test/librbd/mirror/snapshot/test_mock_CreateNonPrimaryRequest.cc +++ b/src/test/librbd/mirror/snapshot/test_mock_CreateNonPrimaryRequest.cc @@ -8,6 +8,7 @@ #include "test/librados_test_stub/MockTestMemIoCtxImpl.h" #include "test/librados_test_stub/MockTestMemRadosClient.h" #include "librbd/mirror/snapshot/CreateNonPrimaryRequest.h" +#include "librbd/mirror/snapshot/Utils.h" #include "librbd/mirror/snapshot/WriteImageStateRequest.h" namespace librbd { @@ -23,6 +24,32 @@ struct MockTestImageCtx : public MockImageCtx { namespace mirror { namespace snapshot { +namespace util { + +namespace { + +struct Mock { + static Mock* s_instance; + + Mock() { + s_instance = this; + } + + MOCK_METHOD1(can_create_non_primary_snapshot, + bool(librbd::MockTestImageCtx *)); +}; + +Mock *Mock::s_instance = nullptr; + +} // anonymous namespace + +template<> bool can_create_non_primary_snapshot( + librbd::MockTestImageCtx *image_ctx) { + return Mock::s_instance->can_create_non_primary_snapshot(image_ctx); +} + +} // namespace util + template <> struct WriteImageStateRequest { uint64_t snap_id = CEPH_NOSNAP; @@ -73,16 +100,7 @@ class TestMockMirrorSnapshotCreateNonPrimaryRequest : public TestMockFixture { public: typedef CreateNonPrimaryRequest MockCreateNonPrimaryRequest; typedef WriteImageStateRequest MockWriteImageStateRequest; - - uint64_t m_snap_seq = 0; - - void snap_create(MockTestImageCtx &mock_image_ctx, - const cls::rbd::SnapshotNamespace &ns, - const std::string& snap_name) { - ASSERT_TRUE(mock_image_ctx.snap_info.insert( - {m_snap_seq++, - SnapInfo{snap_name, ns, 0, {}, 0, 0, {}}}).second); - } + typedef util::Mock MockUtils; void expect_refresh_image(MockTestImageCtx &mock_image_ctx, bool refresh_required, int r) { @@ -108,6 +126,12 @@ public: Return(r))); } + void expect_can_create_non_primary_snapshot(MockUtils &mock_utils, + bool result) { + EXPECT_CALL(mock_utils, can_create_non_primary_snapshot(_)) + .WillOnce(Return(result)); + } + void expect_create_snapshot(MockTestImageCtx &mock_image_ctx, int r) { EXPECT_CALL(*mock_image_ctx.operations, snap_create(_, _, _)) .WillOnce(WithArg<2>(CompleteContext( @@ -141,6 +165,8 @@ TEST_F(TestMockMirrorSnapshotCreateNonPrimaryRequest, Success) { 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_non_primary_snapshot(mock_utils, true); expect_create_snapshot(mock_image_ctx, 0); MockWriteImageStateRequest mock_write_image_state_request; expect_write_image_state(mock_image_ctx, mock_write_image_state_request, 0); @@ -193,7 +219,7 @@ TEST_F(TestMockMirrorSnapshotCreateNonPrimaryRequest, GetMirrorImageError) { ASSERT_EQ(-EINVAL, ctx.wait()); } -TEST_F(TestMockMirrorSnapshotCreateNonPrimaryRequest, CreateSnapshotError) { +TEST_F(TestMockMirrorSnapshotCreateNonPrimaryRequest, CanNotError) { REQUIRE_FORMAT_V2(); librbd::ImageCtx *ictx; @@ -203,11 +229,12 @@ TEST_F(TestMockMirrorSnapshotCreateNonPrimaryRequest, CreateSnapshotError) { InSequence seq; - expect_refresh_image(mock_image_ctx, true, 0); + 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}, 0); - expect_create_snapshot(mock_image_ctx, -EINVAL); + MockUtils mock_utils; + expect_can_create_non_primary_snapshot(mock_utils, false); C_SaferCond ctx; auto req = new MockCreateNonPrimaryRequest(&mock_image_ctx, "mirror_uuid", @@ -216,7 +243,7 @@ TEST_F(TestMockMirrorSnapshotCreateNonPrimaryRequest, CreateSnapshotError) { ASSERT_EQ(-EINVAL, ctx.wait()); } -TEST_F(TestMockMirrorSnapshotCreateNonPrimaryRequest, WriteImageStateError) { +TEST_F(TestMockMirrorSnapshotCreateNonPrimaryRequest, CreateSnapshotError) { REQUIRE_FORMAT_V2(); librbd::ImageCtx *ictx; @@ -230,34 +257,9 @@ TEST_F(TestMockMirrorSnapshotCreateNonPrimaryRequest, WriteImageStateError) { expect_get_mirror_image( mock_image_ctx, {cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT, "gid", cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, 0); - expect_create_snapshot(mock_image_ctx, 0); - MockWriteImageStateRequest mock_write_image_state_request; - expect_write_image_state(mock_image_ctx, mock_write_image_state_request, - -EINVAL); - - C_SaferCond ctx; - auto req = new MockCreateNonPrimaryRequest(&mock_image_ctx, "mirror_uuid", - 123, {}, nullptr, &ctx); - req->send(); - ASSERT_EQ(-EINVAL, ctx.wait()); -} - -TEST_F(TestMockMirrorSnapshotCreateNonPrimaryRequest, ValidateErrorPrimary) { - REQUIRE_FORMAT_V2(); - - librbd::ImageCtx *ictx; - ASSERT_EQ(0, open_image(m_image_name, &ictx)); - - MockTestImageCtx mock_image_ctx(*ictx); - cls::rbd::MirrorPrimarySnapshotNamespace ns{false, {"peer_uuid"}}; - snap_create(mock_image_ctx, ns, "mirror_snap"); - - InSequence seq; - - 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}, 0); + MockUtils mock_utils; + expect_can_create_non_primary_snapshot(mock_utils, true); + expect_create_snapshot(mock_image_ctx, -EINVAL); C_SaferCond ctx; auto req = new MockCreateNonPrimaryRequest(&mock_image_ctx, "mirror_uuid", @@ -266,49 +268,26 @@ TEST_F(TestMockMirrorSnapshotCreateNonPrimaryRequest, ValidateErrorPrimary) { ASSERT_EQ(-EINVAL, ctx.wait()); } -TEST_F(TestMockMirrorSnapshotCreateNonPrimaryRequest, ValidatePrimaryDemoted) { +TEST_F(TestMockMirrorSnapshotCreateNonPrimaryRequest, WriteImageStateError) { REQUIRE_FORMAT_V2(); librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); MockTestImageCtx mock_image_ctx(*ictx); - cls::rbd::MirrorPrimarySnapshotNamespace ns{true, {"peer_uuid"}}; - snap_create(mock_image_ctx, ns, "mirror_snap"); InSequence seq; - expect_refresh_image(mock_image_ctx, false, 0); + 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_non_primary_snapshot(mock_utils, true); expect_create_snapshot(mock_image_ctx, 0); MockWriteImageStateRequest mock_write_image_state_request; - expect_write_image_state(mock_image_ctx, mock_write_image_state_request, 0); - - C_SaferCond ctx; - auto req = new MockCreateNonPrimaryRequest(&mock_image_ctx, "mirror_uuid", - 123, {}, nullptr, &ctx); - req->send(); - ASSERT_EQ(0, ctx.wait()); -} - -TEST_F(TestMockMirrorSnapshotCreateNonPrimaryRequest, ValidateErrorNonPrimaryNotCopied) { - REQUIRE_FORMAT_V2(); - - librbd::ImageCtx *ictx; - ASSERT_EQ(0, open_image(m_image_name, &ictx)); - - MockTestImageCtx mock_image_ctx(*ictx); - cls::rbd::MirrorNonPrimarySnapshotNamespace ns{"mirror_uuid", 111}; - snap_create(mock_image_ctx, ns, "mirror_snap"); - - InSequence seq; - - 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}, 0); + expect_write_image_state(mock_image_ctx, mock_write_image_state_request, + -EINVAL); C_SaferCond ctx; auto req = new MockCreateNonPrimaryRequest(&mock_image_ctx, "mirror_uuid", @@ -317,34 +296,6 @@ TEST_F(TestMockMirrorSnapshotCreateNonPrimaryRequest, ValidateErrorNonPrimaryNot ASSERT_EQ(-EINVAL, ctx.wait()); } -TEST_F(TestMockMirrorSnapshotCreateNonPrimaryRequest, ValidateNonPrimaryCopied) { - REQUIRE_FORMAT_V2(); - - librbd::ImageCtx *ictx; - ASSERT_EQ(0, open_image(m_image_name, &ictx)); - - MockTestImageCtx mock_image_ctx(*ictx); - cls::rbd::MirrorNonPrimarySnapshotNamespace ns{"mirror_uuid", 111}; - ns.copied = true; - snap_create(mock_image_ctx, ns, "mirror_snap"); - - InSequence seq; - - 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}, 0); - expect_create_snapshot(mock_image_ctx, 0); - MockWriteImageStateRequest mock_write_image_state_request; - expect_write_image_state(mock_image_ctx, mock_write_image_state_request, 0); - - C_SaferCond ctx; - auto req = new MockCreateNonPrimaryRequest(&mock_image_ctx, "mirror_uuid", - 123, {}, nullptr, &ctx); - req->send(); - ASSERT_EQ(0, ctx.wait()); -} - } // namespace snapshot } // namespace mirror } // namespace librbd diff --git a/src/test/librbd/mirror/snapshot/test_mock_CreatePrimaryRequest.cc b/src/test/librbd/mirror/snapshot/test_mock_CreatePrimaryRequest.cc index 6b8b1b52f3b..97b6a22fea3 100644 --- a/src/test/librbd/mirror/snapshot/test_mock_CreatePrimaryRequest.cc +++ b/src/test/librbd/mirror/snapshot/test_mock_CreatePrimaryRequest.cc @@ -10,6 +10,7 @@ #include "test/librados_test_stub/MockTestMemRadosClient.h" #include "librbd/mirror/snapshot/CreatePrimaryRequest.h" #include "librbd/mirror/snapshot/UnlinkPeerRequest.h" +#include "librbd/mirror/snapshot/Utils.h" namespace librbd { @@ -24,6 +25,29 @@ struct MockTestImageCtx : public MockImageCtx { namespace mirror { namespace snapshot { +namespace util { + +struct Mock { + static Mock* s_instance; + + Mock() { + s_instance = this; + } + + MOCK_METHOD4(can_create_primary_snapshot, + bool(librbd::MockTestImageCtx *, bool, bool, uint64_t *)); +}; + +Mock *Mock::s_instance = nullptr; + +template<> bool can_create_primary_snapshot(librbd::MockTestImageCtx *image_ctx, + bool demoted, bool force, + uint64_t *rollback_snap_id) { + return Mock::s_instance->can_create_primary_snapshot(image_ctx, demoted, + force, rollback_snap_id); +} + +} // namespace util template <> struct UnlinkPeerRequest { @@ -75,6 +99,7 @@ class TestMockMirrorSnapshotCreatePrimaryRequest : public TestMockFixture { public: typedef CreatePrimaryRequest MockCreatePrimaryRequest; typedef UnlinkPeerRequest MockUnlinkPeerRequest; + typedef util::Mock MockUtils; uint64_t m_snap_seq = 0; @@ -110,6 +135,13 @@ public: Return(r))); } + void expect_can_create_primary_snapshot(MockUtils &mock_utils, bool demoted, + bool force, bool result) { + EXPECT_CALL(mock_utils, + can_create_primary_snapshot(_, demoted, force, nullptr)) + .WillOnce(Return(result)); + } + void expect_get_mirror_peers(MockTestImageCtx &mock_image_ctx, const std::vector &peers, int r) { @@ -184,6 +216,8 @@ TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, Success) { 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, {{"uuid", cls::rbd::MIRROR_PEER_DIRECTION_TX, "ceph", "mirror", "fsid"}}, 0); @@ -237,7 +271,7 @@ TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, GetMirrorImageError) { ASSERT_EQ(-EINVAL, ctx.wait()); } -TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, GetMirrorPeersError) { +TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, CanNotError) { REQUIRE_FORMAT_V2(); librbd::ImageCtx *ictx; @@ -251,9 +285,8 @@ TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, GetMirrorPeersError) { expect_get_mirror_image( mock_image_ctx, {cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT, "gid", cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, 0); - expect_get_mirror_peers(mock_image_ctx, - {{"uuid", cls::rbd::MIRROR_PEER_DIRECTION_TX, "ceph", - "mirror", "fsid"}}, -EINVAL); + MockUtils mock_utils; + expect_can_create_primary_snapshot(mock_utils, false, false, false); C_SaferCond ctx; auto req = new MockCreatePrimaryRequest(&mock_image_ctx, false, false, @@ -262,7 +295,7 @@ TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, GetMirrorPeersError) { ASSERT_EQ(-EINVAL, ctx.wait()); } -TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, CreateSnapshotError) { +TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, GetMirrorPeersError) { REQUIRE_FORMAT_V2(); librbd::ImageCtx *ictx; @@ -276,10 +309,11 @@ TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, CreateSnapshotError) { 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, {{"uuid", cls::rbd::MIRROR_PEER_DIRECTION_TX, "ceph", - "mirror", "fsid"}}, 0); - expect_create_snapshot(mock_image_ctx, -EINVAL); + "mirror", "fsid"}}, -EINVAL); C_SaferCond ctx; auto req = new MockCreatePrimaryRequest(&mock_image_ctx, false, false, @@ -288,63 +322,13 @@ TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, CreateSnapshotError) { ASSERT_EQ(-EINVAL, ctx.wait()); } -TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, ValidateErrorNonPrimaryNotCopied) { - REQUIRE_FORMAT_V2(); - - librbd::ImageCtx *ictx; - ASSERT_EQ(0, open_image(m_image_name, &ictx)); - - MockTestImageCtx mock_image_ctx(*ictx); - cls::rbd::MirrorNonPrimarySnapshotNamespace ns{"mirror_uuid", 123}; - snap_create(mock_image_ctx, ns, "mirror_snap"); - - InSequence seq; - - 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}, 0); - - C_SaferCond ctx; - auto req = new MockCreatePrimaryRequest(&mock_image_ctx, false, true, nullptr, - &ctx); - req->send(); - ASSERT_EQ(-EINVAL, ctx.wait()); -} - -TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, ValidateErrorDemotedNonPrimary) { - REQUIRE_FORMAT_V2(); - - librbd::ImageCtx *ictx; - ASSERT_EQ(0, open_image(m_image_name, &ictx)); - - MockTestImageCtx mock_image_ctx(*ictx); - cls::rbd::MirrorNonPrimarySnapshotNamespace ns{"mirror_uuid", 123}; - snap_create(mock_image_ctx, ns, "mirror_snap"); - - InSequence seq; - - 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}, 0); - - C_SaferCond ctx; - auto req = new MockCreatePrimaryRequest(&mock_image_ctx, true, true, nullptr, - &ctx); - req->send(); - ASSERT_EQ(-EINVAL, ctx.wait()); -} - -TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, ValidateDemotedPrimary) { +TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, CreateSnapshotError) { REQUIRE_FORMAT_V2(); librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); MockTestImageCtx mock_image_ctx(*ictx); - cls::rbd::MirrorPrimarySnapshotNamespace ns{false, {"uuid"}}; - snap_create(mock_image_ctx, ns, "mirror_snap"); InSequence seq; @@ -352,16 +336,18 @@ TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, ValidateDemotedPrimary) { 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, {{"uuid", cls::rbd::MIRROR_PEER_DIRECTION_TX, "ceph", "mirror", "fsid"}}, 0); - expect_create_snapshot(mock_image_ctx, 0); + expect_create_snapshot(mock_image_ctx, -EINVAL); C_SaferCond ctx; - auto req = new MockCreatePrimaryRequest(&mock_image_ctx, true, false, nullptr, - &ctx); + auto req = new MockCreatePrimaryRequest(&mock_image_ctx, false, false, + nullptr, &ctx); req->send(); - ASSERT_EQ(0, ctx.wait()); + ASSERT_EQ(-EINVAL, ctx.wait()); } TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, SuccessUnlinkPeer) { @@ -383,6 +369,8 @@ TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, SuccessUnlinkPeer) { 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, {{"uuid", cls::rbd::MIRROR_PEER_DIRECTION_TX, "ceph", "mirror", "fsid"}}, 0); diff --git a/src/test/librbd/mirror/snapshot/test_mock_Utils.cc b/src/test/librbd/mirror/snapshot/test_mock_Utils.cc new file mode 100644 index 00000000000..17752f0e5cd --- /dev/null +++ b/src/test/librbd/mirror/snapshot/test_mock_Utils.cc @@ -0,0 +1,170 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#include "include/stringify.h" +#include "test/librbd/test_mock_fixture.h" +#include "test/librbd/test_support.h" +#include "test/librbd/mock/MockImageCtx.h" +#include "test/librbd/mock/MockOperations.h" +#include "test/librados_test_stub/MockTestMemIoCtxImpl.h" +#include "test/librados_test_stub/MockTestMemRadosClient.h" +#include "librbd/mirror/snapshot/CreatePrimaryRequest.h" +#include "librbd/mirror/snapshot/UnlinkPeerRequest.h" +#include "librbd/mirror/snapshot/Utils.h" + +namespace librbd { + +namespace { + +struct MockTestImageCtx : public MockImageCtx { + explicit MockTestImageCtx(librbd::ImageCtx& image_ctx) : MockImageCtx(image_ctx) { + } +}; + +} // anonymous namespace +} // namespace librbd + +// template definitions +#include "librbd/mirror/snapshot/Utils.cc" +template bool librbd::mirror::snapshot::util::can_create_primary_snapshot( + librbd::MockTestImageCtx *image_ctx, bool demoted, bool force, + uint64_t *rollback_snap_id); +template bool librbd::mirror::snapshot::util::can_create_non_primary_snapshot( + librbd::MockTestImageCtx *image_ctx); + +namespace librbd { +namespace mirror { +namespace snapshot { + +using ::testing::_; +using ::testing::DoAll; +using ::testing::InSequence; +using ::testing::Invoke; +using ::testing::Return; +using ::testing::StrEq; +using ::testing::WithArg; + +class TestMockMirrorSnapshotUtils : public TestMockFixture { +public: + uint64_t m_snap_seq = 0; + + uint64_t snap_create(MockTestImageCtx &mock_image_ctx, + const cls::rbd::SnapshotNamespace &ns, + const std::string& snap_name) { + EXPECT_TRUE(mock_image_ctx.snap_info.insert( + {++m_snap_seq, + SnapInfo{snap_name, ns, 0, {}, 0, 0, {}}}).second); + return m_snap_seq; + } +}; + +TEST_F(TestMockMirrorSnapshotUtils, CanCreatePrimarySnapshot) { + REQUIRE_FORMAT_V2(); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockTestImageCtx mock_image_ctx(*ictx); + + // no previous mirror snapshots found + uint64_t rollback_snap_id; + ASSERT_TRUE(util::can_create_primary_snapshot(&mock_image_ctx, false, false, + &rollback_snap_id)); + ASSERT_EQ(rollback_snap_id, CEPH_NOSNAP); + + cls::rbd::MirrorNonPrimarySnapshotNamespace nns{"mirror_uuid", 123}; + nns.copied = true; + auto copied_snap_id = snap_create(mock_image_ctx, nns, "NPS1"); + + // without force, previous snapshot is non-primary + ASSERT_FALSE(util::can_create_primary_snapshot(&mock_image_ctx, false, false, + nullptr)); + + // demoted, previous snapshot is non-primary + ASSERT_FALSE(util::can_create_primary_snapshot(&mock_image_ctx, true, true, + nullptr)); + + // previous non-primary snapshot is copied + ASSERT_TRUE(util::can_create_primary_snapshot(&mock_image_ctx, false, true, + &rollback_snap_id)); + ASSERT_EQ(rollback_snap_id, CEPH_NOSNAP); + + + nns.copied = false; + snap_create(mock_image_ctx, nns, "NPS2"); + + // previous non-primary snapshot is not copied yet + ASSERT_FALSE(util::can_create_primary_snapshot(&mock_image_ctx, false, true, + nullptr)); + + // can rollback + ASSERT_TRUE(util::can_create_primary_snapshot(&mock_image_ctx, false, true, + &rollback_snap_id)); + ASSERT_EQ(rollback_snap_id, copied_snap_id); + + nns.primary_mirror_uuid.clear(); + snap_create(mock_image_ctx, nns, "NPS3"); + + // previous non-primary snapshot is orphan + ASSERT_TRUE(util::can_create_primary_snapshot(&mock_image_ctx, false, true, + nullptr)); + + cls::rbd::MirrorPrimarySnapshotNamespace pns{true, {"uuid"}}; + snap_create(mock_image_ctx, pns, "PS1"); + + // previous primary snapshot is demoted, no force + ASSERT_FALSE(util::can_create_primary_snapshot(&mock_image_ctx, false, false, + nullptr)); + + // previous primary snapshot is demoted, force + ASSERT_TRUE(util::can_create_primary_snapshot(&mock_image_ctx, false, true, + nullptr)); + + pns.demoted = false; + snap_create(mock_image_ctx, pns, "PS2"); + + // previous snapshot is not demoted primary + ASSERT_TRUE(util::can_create_primary_snapshot(&mock_image_ctx, false, false, + nullptr)); +} + +TEST_F(TestMockMirrorSnapshotUtils, CanCreateNonPrimarySnapshot) { + REQUIRE_FORMAT_V2(); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockTestImageCtx mock_image_ctx(*ictx); + + // no previous mirror snapshots found + ASSERT_TRUE(util::can_create_non_primary_snapshot(&mock_image_ctx)); + + cls::rbd::MirrorNonPrimarySnapshotNamespace nns{"mirror_uuid", 123}; + snap_create(mock_image_ctx, nns, "NPS1"); + + // previous non-primary snapshot is not copied yet + ASSERT_FALSE(util::can_create_non_primary_snapshot(&mock_image_ctx)); + + nns.copied = true; + snap_create(mock_image_ctx, nns, "NPS2"); + + // previous non-primary snapshot is copied + ASSERT_TRUE(util::can_create_non_primary_snapshot(&mock_image_ctx)); + + cls::rbd::MirrorPrimarySnapshotNamespace pns{false, {"uuid"}}; + snap_create(mock_image_ctx, pns, "PS1"); + + // previous primary snapshot is not in demoted state + ASSERT_FALSE(util::can_create_non_primary_snapshot(&mock_image_ctx)); + + pns.demoted = true; + snap_create(mock_image_ctx, pns, "PS2"); + + // previous primary snapshot is in demoted state + ASSERT_TRUE(util::can_create_non_primary_snapshot(&mock_image_ctx)); +} + +} // namespace snapshot +} // namespace mirror +} // namespace librbd + -- 2.39.5