RepoMirrors/ceph
1
0
Fork 0
mirror of https://github.com/ceph/ceph synced 2025-02-21 01:47:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #44714 from idryomov/wip-rbd-mirror-delprop-races

Browse Source 
rbd-mirror: fix races in snapshot-based mirroring deletion propagation

Reviewed-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@cern.ch>
Reviewed-by: Mykola Golub <mgolub@suse.com>
This commit is contained in:
Ilya Dryomov 2022-01-24 11:43:56 +01:00 committed by GitHub
commit 315e186124
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 254 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

228
src/test/rbd_mirror/image_replayer/test_mock_PrepareRemoteImageRequest.cc
View File

@ -331,7 +331,6 @@ TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, SuccessJournal) {
}
TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, SuccessSnapshot) {
::journal::MockJournaler mock_remote_journaler;
MockThreads mock_threads(m_threads);
InSequence seq;
@ -434,8 +433,7 @@ TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, SuccessNotRegistered) {
mock_journal_state_builder.remote_client_state);
}
TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, MirrorImageIdError) {
::journal::MockJournaler mock_remote_journaler;
TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, GetMirrorImageIdError) {
MockThreads mock_threads(m_threads);
InSequence seq;
@ -461,7 +459,6 @@ TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, MirrorImageIdError) {
}
TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, GetMirrorInfoError) {
::journal::MockJournaler mock_remote_journaler;
MockThreads mock_threads(m_threads);
InSequence seq;
@ -586,6 +583,229 @@ TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, RegisterClientError) {
ASSERT_EQ(-EINVAL, ctx.wait());
}
TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, MirrorImageIdDNEJournal) {
MockThreads mock_threads(m_threads);
InSequence seq;
MockGetMirrorImageIdRequest mock_get_mirror_image_id_request;
expect_get_mirror_image_id(mock_get_mirror_image_id_request, "", -ENOENT);
MockJournalStateBuilder mock_journal_state_builder;
MockStateBuilder* mock_state_builder = &mock_journal_state_builder;
C_SaferCond ctx;
auto req = MockPrepareRemoteImageRequest::create(&mock_threads,
m_local_io_ctx,
m_remote_io_ctx,
"global image id",
"local mirror uuid",
{"remote mirror uuid", ""},
nullptr,
&mock_state_builder,
&ctx);
req->send();
ASSERT_EQ(-ENOENT, ctx.wait());
ASSERT_EQ(cls::rbd::MIRROR_IMAGE_MODE_JOURNAL,
mock_journal_state_builder.mirror_image_mode);
ASSERT_EQ("remote mirror uuid",
mock_journal_state_builder.remote_mirror_uuid);
ASSERT_EQ("", mock_journal_state_builder.remote_image_id);
}
TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, MirrorImageIdDNESnapshot) {
MockThreads mock_threads(m_threads);
InSequence seq;
MockGetMirrorImageIdRequest mock_get_mirror_image_id_request;
expect_get_mirror_image_id(mock_get_mirror_image_id_request, "", -ENOENT);
MockSnapshotStateBuilder mock_snapshot_state_builder;
MockStateBuilder* mock_state_builder = &mock_snapshot_state_builder;
C_SaferCond ctx;
auto req = MockPrepareRemoteImageRequest::create(&mock_threads,
m_local_io_ctx,
m_remote_io_ctx,
"global image id",
"local mirror uuid",
{"remote mirror uuid",
"remote mirror peer uuid"},
nullptr,
&mock_state_builder,
&ctx);
req->send();
ASSERT_EQ(-ENOENT, ctx.wait());
ASSERT_EQ(cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT,
mock_snapshot_state_builder.mirror_image_mode);
ASSERT_EQ("remote mirror uuid",
mock_snapshot_state_builder.remote_mirror_uuid);
ASSERT_EQ("remote mirror peer uuid",
mock_snapshot_state_builder.remote_mirror_peer_uuid);
ASSERT_EQ("", mock_snapshot_state_builder.remote_image_id);
}
TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, MirrorInfoDNEJournal) {
MockThreads mock_threads(m_threads);
InSequence seq;
MockGetMirrorImageIdRequest mock_get_mirror_image_id_request;
expect_get_mirror_image_id(mock_get_mirror_image_id_request,
"remote image id", 0);
MockGetMirrorInfoRequest mock_get_mirror_info_request;
expect_get_mirror_info(mock_get_mirror_info_request,
{cls::rbd::MIRROR_IMAGE_MODE_JOURNAL,
"global image id",
cls::rbd::MIRROR_IMAGE_STATE_ENABLED},
librbd::mirror::PROMOTION_STATE_PRIMARY,
"remote mirror uuid", -ENOENT);
MockJournalStateBuilder mock_journal_state_builder;
MockStateBuilder* mock_state_builder = &mock_journal_state_builder;
C_SaferCond ctx;
auto req = MockPrepareRemoteImageRequest::create(&mock_threads,
m_local_io_ctx,
m_remote_io_ctx,
"global image id",
"local mirror uuid",
{"remote mirror uuid", ""},
nullptr,
&mock_state_builder,
&ctx);
req->send();
ASSERT_EQ(-ENOENT, ctx.wait());
ASSERT_EQ(cls::rbd::MIRROR_IMAGE_MODE_JOURNAL,
mock_journal_state_builder.mirror_image_mode);
ASSERT_EQ("remote mirror uuid",
mock_journal_state_builder.remote_mirror_uuid);
ASSERT_EQ("", mock_journal_state_builder.remote_image_id);
}
TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, MirrorInfoDNESnapshot) {
MockThreads mock_threads(m_threads);
InSequence seq;
MockGetMirrorImageIdRequest mock_get_mirror_image_id_request;
expect_get_mirror_image_id(mock_get_mirror_image_id_request,
"remote image id", 0);
MockGetMirrorInfoRequest mock_get_mirror_info_request;
expect_get_mirror_info(mock_get_mirror_info_request,
{cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT,
"global image id",
cls::rbd::MIRROR_IMAGE_STATE_ENABLED},
librbd::mirror::PROMOTION_STATE_PRIMARY,
"remote mirror uuid", -ENOENT);
MockSnapshotStateBuilder mock_snapshot_state_builder;
MockStateBuilder* mock_state_builder = &mock_snapshot_state_builder;
C_SaferCond ctx;
auto req = MockPrepareRemoteImageRequest::create(&mock_threads,
m_local_io_ctx,
m_remote_io_ctx,
"global image id",
"local mirror uuid",
{"remote mirror uuid",
"remote mirror peer uuid"},
nullptr,
&mock_state_builder,
&ctx);
req->send();
ASSERT_EQ(-ENOENT, ctx.wait());
ASSERT_EQ(cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT,
mock_snapshot_state_builder.mirror_image_mode);
ASSERT_EQ("remote mirror uuid",
mock_snapshot_state_builder.remote_mirror_uuid);
ASSERT_EQ("remote mirror peer uuid",
mock_snapshot_state_builder.remote_mirror_peer_uuid);
ASSERT_EQ("", mock_snapshot_state_builder.remote_image_id);
}
TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, MirrorInfoDisablingJournal) {
MockThreads mock_threads(m_threads);
InSequence seq;
MockGetMirrorImageIdRequest mock_get_mirror_image_id_request;
expect_get_mirror_image_id(mock_get_mirror_image_id_request,
"remote image id", 0);
MockGetMirrorInfoRequest mock_get_mirror_info_request;
expect_get_mirror_info(mock_get_mirror_info_request,
{cls::rbd::MIRROR_IMAGE_MODE_JOURNAL,
"global image id",
cls::rbd::MIRROR_IMAGE_STATE_DISABLING},
librbd::mirror::PROMOTION_STATE_PRIMARY,
"remote mirror uuid", 0);
MockJournalStateBuilder mock_journal_state_builder;
expect_get_mirror_image_mode(mock_journal_state_builder,
cls::rbd::MIRROR_IMAGE_MODE_JOURNAL);
MockStateBuilder* mock_state_builder = &mock_journal_state_builder;
C_SaferCond ctx;
auto req = MockPrepareRemoteImageRequest::create(&mock_threads,
m_local_io_ctx,
m_remote_io_ctx,
"global image id",
"local mirror uuid",
{"remote mirror uuid", ""},
nullptr,
&mock_state_builder,
&ctx);
req->send();
ASSERT_EQ(-ENOENT, ctx.wait());
ASSERT_EQ(cls::rbd::MIRROR_IMAGE_MODE_JOURNAL,
mock_journal_state_builder.mirror_image_mode);
ASSERT_EQ("remote mirror uuid",
mock_journal_state_builder.remote_mirror_uuid);
ASSERT_EQ("", mock_journal_state_builder.remote_image_id);
}
TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, MirrorInfoDisablingSnapshot) {
MockThreads mock_threads(m_threads);
InSequence seq;
MockGetMirrorImageIdRequest mock_get_mirror_image_id_request;
expect_get_mirror_image_id(mock_get_mirror_image_id_request,
"remote image id", 0);
MockGetMirrorInfoRequest mock_get_mirror_info_request;
expect_get_mirror_info(mock_get_mirror_info_request,
{cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT,
"global image id",
cls::rbd::MIRROR_IMAGE_STATE_DISABLING},
librbd::mirror::PROMOTION_STATE_PRIMARY,
"remote mirror uuid", 0);
MockSnapshotStateBuilder mock_snapshot_state_builder;
expect_get_mirror_image_mode(mock_snapshot_state_builder,
cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT);
MockStateBuilder* mock_state_builder = &mock_snapshot_state_builder;
C_SaferCond ctx;
auto req = MockPrepareRemoteImageRequest::create(&mock_threads,
m_local_io_ctx,
m_remote_io_ctx,
"global image id",
"local mirror uuid",
{"remote mirror uuid",
"remote mirror peer uuid"},
nullptr,
&mock_state_builder,
&ctx);
req->send();
ASSERT_EQ(-ENOENT, ctx.wait());
ASSERT_EQ(cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT,
mock_snapshot_state_builder.mirror_image_mode);
ASSERT_EQ("remote mirror uuid",
mock_snapshot_state_builder.remote_mirror_uuid);
ASSERT_EQ("remote mirror peer uuid",
mock_snapshot_state_builder.remote_mirror_peer_uuid);
ASSERT_EQ("", mock_snapshot_state_builder.remote_image_id);
}
} // namespace image_replayer
} // namespace mirror
} // namespace rbd

19
src/tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.cc
View File

@ -38,6 +38,10 @@ template <typename I>
void PrepareRemoteImageRequest<I>::send() {
if (*m_state_builder != nullptr) {
(*m_state_builder)->remote_mirror_uuid = m_remote_pool_meta.mirror_uuid;
auto state_builder = dynamic_cast<snapshot::StateBuilder<I>*>(*m_state_builder);
if (state_builder) {
state_builder->remote_mirror_peer_uuid = m_remote_pool_meta.mirror_peer_uuid;
}
}
get_remote_image_id();
@ -62,7 +66,6 @@ void PrepareRemoteImageRequest<I>::handle_get_remote_image_id(int r) {
<< "remote_image_id=" << m_remote_image_id << dendl;
if (r == -ENOENT) {
finalize_snapshot_state_builder(r);
finish(r);
return;
} else if (r < 0) {
@ -93,7 +96,6 @@ void PrepareRemoteImageRequest<I>::handle_get_mirror_info(int r) {
if (r == -ENOENT) {
dout(10) << "image " << m_global_image_id << " not mirrored" << dendl;
finalize_snapshot_state_builder(r);
finish(r);
return;
} else if (r < 0) {
@ -130,7 +132,7 @@ void PrepareRemoteImageRequest<I>::handle_get_mirror_info(int r) {
get_client();
break;
case cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT:
finalize_snapshot_state_builder(0);
finalize_snapshot_state_builder();
finish(0);
break;
default:
@ -252,26 +254,21 @@ void PrepareRemoteImageRequest<I>::finalize_journal_state_builder(
}
template <typename I>
void PrepareRemoteImageRequest<I>::finalize_snapshot_state_builder(int r) {
void PrepareRemoteImageRequest<I>::finalize_snapshot_state_builder() {
snapshot::StateBuilder<I>* state_builder = nullptr;
if (*m_state_builder != nullptr) {
state_builder = dynamic_cast<snapshot::StateBuilder<I>*>(*m_state_builder);
} else if (r >= 0) {
ceph_assert(state_builder != nullptr);
} else {
state_builder = snapshot::StateBuilder<I>::create(m_global_image_id);
*m_state_builder = state_builder;
}
if (state_builder == nullptr) {
// local image prepare failed or is using journal-based mirroring
return;
}
dout(10) << "remote_mirror_uuid=" << m_remote_pool_meta.mirror_uuid << ", "
<< "remote_mirror_peer_uuid="
<< m_remote_pool_meta.mirror_peer_uuid << ", "
<< "remote_image_id=" << m_remote_image_id << ", "
<< "remote_promotion_state=" << m_promotion_state << dendl;
ceph_assert(state_builder != nullptr);
state_builder->remote_mirror_uuid = m_remote_pool_meta.mirror_uuid;
state_builder->remote_mirror_peer_uuid = m_remote_pool_meta.mirror_peer_uuid;
state_builder->remote_image_id = m_remote_image_id;

2
src/tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.h
View File

@ -139,7 +139,7 @@ private:
void finalize_journal_state_builder(cls::journal::ClientState client_state,
const MirrorPeerClientMeta& client_meta);
void finalize_snapshot_state_builder(int r);
void finalize_snapshot_state_builder();
void finish(int r);
};

5
src/tools/rbd_mirror/image_replayer/StateBuilder.cc
View File

@ -43,8 +43,9 @@ bool StateBuilder<I>::is_local_primary() const {
template <typename I>
bool StateBuilder<I>::is_linked() const {
return (local_promotion_state ==
librbd::mirror::PROMOTION_STATE_NON_PRIMARY);
return ((local_promotion_state ==
librbd::mirror::PROMOTION_STATE_NON_PRIMARY) &&
is_linked_impl());
}
template <typename I>

12
src/tools/rbd_mirror/image_replayer/StateBuilder.h
View File

@ -44,16 +44,14 @@ public:
virtual bool is_disconnected() const = 0;
bool is_local_primary() const;
virtual bool is_linked() const;
bool is_linked() const;
virtual cls::rbd::MirrorImageMode get_mirror_image_mode() const = 0;
virtual image_sync::SyncPointHandler* create_sync_point_handler() = 0;
void destroy_sync_point_handler();
virtual bool replay_requires_remote_image() const {
return false;
}
virtual bool replay_requires_remote_image() const = 0;
void close_remote_image(Context* on_finish);
@ -81,13 +79,13 @@ public:
std::string global_image_id;
std::string local_image_id{};
std::string local_image_id;
librbd::mirror::PromotionState local_promotion_state =
librbd::mirror::PROMOTION_STATE_PRIMARY;
ImageCtxT* local_image_ctx = nullptr;
std::string remote_mirror_uuid;
std::string remote_image_id{};
std::string remote_image_id;
librbd::mirror::PromotionState remote_promotion_state =
librbd::mirror::PROMOTION_STATE_NON_PRIMARY;
ImageCtxT* remote_image_ctx = nullptr;
@ -100,6 +98,8 @@ protected:
void close_local_image(Context* on_finish);
private:
virtual bool is_linked_impl() const = 0;
void handle_close_local_image(int r, Context* on_finish);
void handle_close_remote_image(int r, Context* on_finish);
};

5
src/tools/rbd_mirror/image_replayer/journal/StateBuilder.cc
View File

@ -58,10 +58,9 @@ bool StateBuilder<I>::is_disconnected() const {
}
template <typename I>
bool StateBuilder<I>::is_linked() const {
bool StateBuilder<I>::is_linked_impl() const {
ceph_assert(!this->remote_mirror_uuid.empty());
return (image_replayer::StateBuilder<I>::is_linked() &&
local_primary_mirror_uuid == this->remote_mirror_uuid);
return (local_primary_mirror_uuid == this->remote_mirror_uuid);
}
template <typename I>

6
src/tools/rbd_mirror/image_replayer/journal/StateBuilder.h
View File

@ -37,12 +37,15 @@ public:
void close(Context* on_finish) override;
bool is_disconnected() const override;
bool is_linked() const override;
cls::rbd::MirrorImageMode get_mirror_image_mode() const override;
image_sync::SyncPointHandler* create_sync_point_handler() override;
bool replay_requires_remote_image() const override {
return false;
}
BaseRequest* create_local_image_request(
Threads<ImageCtxT>* threads,
librados::IoCtx& local_io_ctx,
@ -75,6 +78,7 @@ public:
SyncPointHandler<ImageCtxT>* sync_point_handler = nullptr;
private:
bool is_linked_impl() const override;
void shut_down_remote_journaler(Context* on_finish);
void handle_shut_down_remote_journaler(int r, Context* on_finish);

5
src/tools/rbd_mirror/image_replayer/snapshot/StateBuilder.cc
View File

@ -56,10 +56,9 @@ bool StateBuilder<I>::is_disconnected() const {
}
template <typename I>
bool StateBuilder<I>::is_linked() const {
bool StateBuilder<I>::is_linked_impl() const {
// the remote has to have us registered as a peer
return (image_replayer::StateBuilder<I>::is_linked() &&
!remote_mirror_peer_uuid.empty());
return !remote_mirror_peer_uuid.empty();
}
template <typename I>

4
src/tools/rbd_mirror/image_replayer/snapshot/StateBuilder.h
View File

@ -42,7 +42,6 @@ public:
void close(Context* on_finish) override;
bool is_disconnected() const override;
bool is_linked() const override;
cls::rbd::MirrorImageMode get_mirror_image_mode() const override;
@ -79,6 +78,9 @@ public:
std::string remote_mirror_peer_uuid;
librbd::mirror::snapshot::ImageMeta<ImageCtxT>* local_image_meta = nullptr;
private:
bool is_linked_impl() const override;
};
} // namespace snapshot