From: Mykola Golub Date: Tue, 31 May 2016 12:00:30 +0000 (+0300) Subject: rbd-mirror: support bootstrap canceling X-Git-Tag: v10.2.2~17^2~2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=c94e5b740628b98008e5342b8a3b2b888ff7b794;p=ceph.git rbd-mirror: support bootstrap canceling Signed-off-by: Mykola Golub (cherry picked from commit 21f895f48498e518c3675a9e851559f4d071f6dd) --- diff --git a/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc b/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc index 79b7eff13496..7ca0436a6320 100644 --- a/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc +++ b/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc @@ -58,7 +58,14 @@ struct ImageSync { s_instance = this; } - MOCK_METHOD0(start, void()); + void put() { + } + + void get() { + } + + MOCK_METHOD0(send, void()); + MOCK_METHOD0(cancel, void()); }; ImageSync* ImageSync::s_instance = nullptr; 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 9c697363fd34..abc6cc036736 100644 --- a/src/test/rbd_mirror/image_sync/test_mock_ImageCopyRequest.cc +++ b/src/test/rbd_mirror/image_sync/test_mock_ImageCopyRequest.cc @@ -77,6 +77,7 @@ using ::testing::InSequence; using ::testing::Invoke; using ::testing::Return; using ::testing::WithArg; +using ::testing::InvokeWithoutArgs; class TestMockImageSyncImageCopyRequest : public TestMockFixture { public: @@ -379,7 +380,6 @@ TEST_F(TestMockImageSyncImageCopyRequest, Cancel) { expect_get_object_count(mock_remote_image_ctx, 2); expect_update_client(mock_journaler, 0); expect_object_copy_send(mock_object_copy_request); - expect_update_client(mock_journaler, 0); C_SaferCond ctx; MockImageCopyRequest *request = create_request(mock_remote_image_ctx, @@ -393,7 +393,38 @@ TEST_F(TestMockImageSyncImageCopyRequest, Cancel) { request->cancel(); ASSERT_TRUE(complete_object_copy(mock_object_copy_request, 0, 0)); - ASSERT_EQ(0, ctx.wait()); + ASSERT_EQ(-ECANCELED, ctx.wait()); +} + +TEST_F(TestMockImageSyncImageCopyRequest, Cancel1) { + ASSERT_EQ(0, create_snap("snap1")); + m_client_meta.sync_points = {{"snap1", boost::none}}; + + librbd::MockImageCtx mock_remote_image_ctx(*m_remote_image_ctx); + librbd::MockImageCtx mock_local_image_ctx(*m_local_image_ctx); + journal::MockJournaler mock_journaler; + MockObjectCopyRequest mock_object_copy_request; + + C_SaferCond ctx; + MockImageCopyRequest *request = create_request(mock_remote_image_ctx, + mock_local_image_ctx, + mock_journaler, + m_client_meta.sync_points.front(), + &ctx); + + 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_CALL(mock_journaler, update_client(_, _)) + .WillOnce(DoAll(InvokeWithoutArgs([request]() { + request->cancel(); + }), + WithArg<1>(CompleteContext(0)))); + + request->send(); + ASSERT_EQ(-ECANCELED, ctx.wait()); } TEST_F(TestMockImageSyncImageCopyRequest, MissingSnap) { 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 35252f9177e0..bcb9fd69d146 100644 --- a/src/test/rbd_mirror/image_sync/test_mock_SnapshotCopyRequest.cc +++ b/src/test/rbd_mirror/image_sync/test_mock_SnapshotCopyRequest.cc @@ -234,6 +234,26 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, UpdateClientError) { ASSERT_EQ(-EINVAL, ctx.wait()); } +TEST_F(TestMockImageSyncSnapshotCopyRequest, UpdateClientCancel) { + librbd::MockImageCtx mock_remote_image_ctx(*m_remote_image_ctx); + librbd::MockImageCtx mock_local_image_ctx(*m_local_image_ctx); + journal::MockJournaler mock_journaler; + + C_SaferCond ctx; + MockSnapshotCopyRequest *request = create_request(mock_remote_image_ctx, + mock_local_image_ctx, + mock_journaler, &ctx); + InSequence seq; + EXPECT_CALL(mock_journaler, update_client(_, _)) + .WillOnce(DoAll(InvokeWithoutArgs([request]() { + request->cancel(); + }), + WithArg<1>(CompleteContext(0)))); + + request->send(); + ASSERT_EQ(-ECANCELED, ctx.wait()); +} + TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapCreate) { ASSERT_EQ(0, create_snap(m_remote_image_ctx, "snap1")); ASSERT_EQ(0, create_snap(m_remote_image_ctx, "snap2")); @@ -283,6 +303,31 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapCreateError) { ASSERT_EQ(-EINVAL, ctx.wait()); } +TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapCreateCancel) { + ASSERT_EQ(0, create_snap(m_remote_image_ctx, "snap1")); + + librbd::MockImageCtx mock_remote_image_ctx(*m_remote_image_ctx); + librbd::MockImageCtx mock_local_image_ctx(*m_local_image_ctx); + MockSnapshotCreateRequest mock_snapshot_create_request; + journal::MockJournaler mock_journaler; + + C_SaferCond ctx; + MockSnapshotCopyRequest *request = create_request(mock_remote_image_ctx, + mock_local_image_ctx, + mock_journaler, &ctx); + InSequence seq; + EXPECT_CALL(mock_snapshot_create_request, send()) + .WillOnce(DoAll(InvokeWithoutArgs([request]() { + request->cancel(); + }), + Invoke([this, &mock_snapshot_create_request]() { + m_threads->work_queue->queue(mock_snapshot_create_request.on_finish, 0); + }))); + + request->send(); + ASSERT_EQ(-ECANCELED, ctx.wait()); +} + TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapRemoveAndCreate) { ASSERT_EQ(0, create_snap(m_remote_image_ctx, "snap1")); ASSERT_EQ(0, create_snap(m_local_image_ctx, "snap1")); @@ -388,6 +433,38 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapUnprotectError) { ASSERT_EQ(-EBUSY, ctx.wait()); } +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["snap1"]; + uint64_t local_snap_id1 = m_local_image_ctx->snap_ids["snap1"]; + m_client_meta.snap_seqs[remote_snap_id1] = local_snap_id1; + + librbd::MockImageCtx mock_remote_image_ctx(*m_remote_image_ctx); + librbd::MockImageCtx mock_local_image_ctx(*m_local_image_ctx); + journal::MockJournaler mock_journaler; + + C_SaferCond ctx; + MockSnapshotCopyRequest *request = create_request(mock_remote_image_ctx, + mock_local_image_ctx, + mock_journaler, &ctx); + 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_CALL(*mock_local_image_ctx.operations, + execute_snap_unprotect(StrEq("snap1"), _)) + .WillOnce(DoAll(InvokeWithoutArgs([request]() { + request->cancel(); + }), + WithArg<1>(Invoke([this](Context *ctx) { + m_threads->work_queue->queue(ctx, 0); + })))); + + request->send(); + ASSERT_EQ(-ECANCELED, ctx.wait()); +} + 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)); @@ -503,6 +580,39 @@ TEST_F(TestMockImageSyncSnapshotCopyRequest, SnapProtectError) { ASSERT_EQ(-EINVAL, ctx.wait()); } +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["snap1"]; + uint64_t local_snap_id1 = m_local_image_ctx->snap_ids["snap1"]; + m_client_meta.snap_seqs[remote_snap_id1] = local_snap_id1; + + librbd::MockImageCtx mock_remote_image_ctx(*m_remote_image_ctx); + librbd::MockImageCtx mock_local_image_ctx(*m_local_image_ctx); + journal::MockJournaler mock_journaler; + + C_SaferCond ctx; + MockSnapshotCopyRequest *request = create_request(mock_remote_image_ctx, + mock_local_image_ctx, + mock_journaler, &ctx); + InSequence seq; + expect_snap_is_unprotected(mock_local_image_ctx, local_snap_id1, true, 0); + 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_CALL(*mock_local_image_ctx.operations, + execute_snap_protect(StrEq("snap1"), _)) + .WillOnce(DoAll(InvokeWithoutArgs([request]() { + request->cancel(); + }), + WithArg<1>(Invoke([this](Context *ctx) { + m_threads->work_queue->queue(ctx, 0); + })))); + + request->send(); + ASSERT_EQ(-ECANCELED, ctx.wait()); +} + } // namespace image_sync } // namespace mirror } // namespace rbd diff --git a/src/test/rbd_mirror/test_ImageSync.cc b/src/test/rbd_mirror/test_ImageSync.cc index db6e26f544d8..922d49933174 100644 --- a/src/test/rbd_mirror/test_ImageSync.cc +++ b/src/test/rbd_mirror/test_ImageSync.cc @@ -100,7 +100,7 @@ public: TEST_F(TestImageSync, Empty) { C_SaferCond ctx; ImageSync<> *request = create_request(&ctx); - request->start(); + request->send(); ASSERT_EQ(0, ctx.wait()); ASSERT_EQ(0U, m_client_meta.sync_points.size()); @@ -115,7 +115,7 @@ TEST_F(TestImageSync, Simple) { C_SaferCond ctx; ImageSync<> *request = create_request(&ctx); - request->start(); + request->send(); ASSERT_EQ(0, ctx.wait()); int64_t object_size = std::min( @@ -159,7 +159,7 @@ TEST_F(TestImageSync, SnapshotStress) { C_SaferCond ctx; ImageSync<> *request = create_request(&ctx); - request->start(); + request->send(); ASSERT_EQ(0, ctx.wait()); int64_t object_size = std::min( diff --git a/src/test/rbd_mirror/test_mock_ImageReplayer.cc b/src/test/rbd_mirror/test_mock_ImageReplayer.cc index 85c8f7a452a1..b5cfcd9d356e 100644 --- a/src/test/rbd_mirror/test_mock_ImageReplayer.cc +++ b/src/test/rbd_mirror/test_mock_ImageReplayer.cc @@ -80,7 +80,14 @@ struct BootstrapRequest { s_instance = this; } + void put() { + } + + void get() { + } + MOCK_METHOD0(send, void()); + MOCK_METHOD0(cancel, void()); }; template<> diff --git a/src/test/rbd_mirror/test_mock_ImageSync.cc b/src/test/rbd_mirror/test_mock_ImageSync.cc index 38d2cb00e10d..868a030f0999 100644 --- a/src/test/rbd_mirror/test_mock_ImageSync.cc +++ b/src/test/rbd_mirror/test_mock_ImageSync.cc @@ -58,6 +58,13 @@ public: ImageCopyRequest() { s_instance = this; } + + void put() { + } + + void get() { + } + MOCK_METHOD0(cancel, void()); MOCK_METHOD0(send, void()); }; @@ -83,7 +90,15 @@ public: SnapshotCopyRequest() { s_instance = this; } + + void put() { + } + + void get() { + } + MOCK_METHOD0(send, void()); + MOCK_METHOD0(cancel, void()); }; template <> @@ -144,6 +159,7 @@ using ::testing::InSequence; using ::testing::Invoke; using ::testing::Return; using ::testing::WithArg; +using ::testing::InvokeWithoutArgs; class TestMockImageSync : public TestMockFixture { public: @@ -273,7 +289,7 @@ TEST_F(TestMockImageSync, SimpleSync) { MockImageSync *request = create_request(mock_remote_image_ctx, mock_local_image_ctx, mock_journaler, &ctx); - request->start(); + request->send(); ASSERT_EQ(0, ctx.wait()); } @@ -308,7 +324,7 @@ TEST_F(TestMockImageSync, RestartSync) { MockImageSync *request = create_request(mock_remote_image_ctx, mock_local_image_ctx, mock_journaler, &ctx); - request->start(); + request->send(); ASSERT_EQ(0, ctx.wait()); } @@ -337,15 +353,80 @@ TEST_F(TestMockImageSync, CancelImageCopy) { MockImageSync *request = create_request(mock_remote_image_ctx, mock_local_image_ctx, mock_journaler, &ctx); - request->start(); + request->get(); + request->send(); // cancel the image copy once it starts ASSERT_EQ(0, image_copy_ctx.wait()); request->cancel(); + request->put(); m_threads->work_queue->queue(mock_image_copy_request.on_finish, 0); ASSERT_EQ(-ECANCELED, ctx.wait()); } +TEST_F(TestMockImageSync, CancelAfterCopySnapshots) { + librbd::MockImageCtx mock_remote_image_ctx(*m_remote_image_ctx); + librbd::MockImageCtx mock_local_image_ctx(*m_local_image_ctx); + journal::MockJournaler mock_journaler; + MockSnapshotCopyRequest mock_snapshot_copy_request; + MockSyncPointCreateRequest mock_sync_point_create_request; + + librbd::MockObjectMap *mock_object_map = new librbd::MockObjectMap(); + mock_local_image_ctx.object_map = mock_object_map; + expect_test_features(mock_local_image_ctx); + + C_SaferCond ctx; + MockImageSync *request = create_request(mock_remote_image_ctx, + mock_local_image_ctx, + mock_journaler, &ctx); + InSequence seq; + expect_create_sync_point(mock_local_image_ctx, mock_sync_point_create_request, 0); + EXPECT_CALL(mock_snapshot_copy_request, send()) + .WillOnce((DoAll(InvokeWithoutArgs([request]() { + request->cancel(); + }), + Invoke([this, &mock_snapshot_copy_request]() { + m_threads->work_queue->queue(mock_snapshot_copy_request.on_finish, 0); + })))); + EXPECT_CALL(mock_snapshot_copy_request, cancel()); + + request->send(); + ASSERT_EQ(-ECANCELED, ctx.wait()); +} + +TEST_F(TestMockImageSync, CancelAfterCopyImage) { + librbd::MockImageCtx mock_remote_image_ctx(*m_remote_image_ctx); + librbd::MockImageCtx mock_local_image_ctx(*m_local_image_ctx); + journal::MockJournaler mock_journaler; + MockImageCopyRequest mock_image_copy_request; + MockSnapshotCopyRequest mock_snapshot_copy_request; + MockSyncPointCreateRequest mock_sync_point_create_request; + MockSyncPointPruneRequest mock_sync_point_prune_request; + + librbd::MockObjectMap *mock_object_map = new librbd::MockObjectMap(); + mock_local_image_ctx.object_map = mock_object_map; + expect_test_features(mock_local_image_ctx); + + C_SaferCond ctx; + MockImageSync *request = create_request(mock_remote_image_ctx, + mock_local_image_ctx, + mock_journaler, &ctx); + InSequence seq; + expect_create_sync_point(mock_local_image_ctx, mock_sync_point_create_request, 0); + expect_copy_snapshots(mock_snapshot_copy_request, 0); + EXPECT_CALL(mock_image_copy_request, send()) + .WillOnce((DoAll(InvokeWithoutArgs([request]() { + request->cancel(); + }), + Invoke([this, &mock_image_copy_request]() { + m_threads->work_queue->queue(mock_image_copy_request.on_finish, 0); + })))); + EXPECT_CALL(mock_image_copy_request, cancel()); + + request->send(); + ASSERT_EQ(-ECANCELED, ctx.wait()); +} + } // namespace mirror } // namespace rbd diff --git a/src/tools/Makefile-client.am b/src/tools/Makefile-client.am index 716ce4fb485f..7762c8b5e27b 100644 --- a/src/tools/Makefile-client.am +++ b/src/tools/Makefile-client.am @@ -112,6 +112,7 @@ librbd_mirror_internal_la_SOURCES = \ tools/rbd_mirror/image_sync/SyncPointPruneRequest.cc noinst_LTLIBRARIES += librbd_mirror_internal.la noinst_HEADERS += \ + tools/rbd_mirror/BaseRequest.h \ tools/rbd_mirror/ClusterWatcher.h \ tools/rbd_mirror/ImageReplayer.h \ tools/rbd_mirror/ImageSync.h \ diff --git a/src/tools/rbd_mirror/BaseRequest.h b/src/tools/rbd_mirror/BaseRequest.h new file mode 100644 index 000000000000..91e4fbaa4e0d --- /dev/null +++ b/src/tools/rbd_mirror/BaseRequest.h @@ -0,0 +1,42 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#ifndef CEPH_RBD_MIRROR_BASE_REQUEST_H +#define CEPH_RBD_MIRROR_BASE_REQUEST_H + +#include "common/RefCountedObj.h" + +namespace rbd { +namespace mirror { + +class BaseRequest : public RefCountedObject { +public: + BaseRequest(const std::string& name, CephContext *cct, Context *on_finish) + : RefCountedObject(cct, 1), m_name(name), m_cct(cct), + m_on_finish(on_finish) { + } + + virtual void send() = 0; + virtual void cancel() {} + +protected: + void finish(int r) { + if (m_cct) { + lsubdout(m_cct, rbd_mirror, 20) << m_name << "::finish: r=" << r << dendl; + } + if (m_on_finish) { + m_on_finish->complete(r); + } + put(); + } + +private: + const std::string m_name; + CephContext *m_cct; + Context *m_on_finish; +}; + +} // namespace mirror +} // namespace rbd + +#endif // CEPH_RBD_MIRROR_BASE_REQUEST_H diff --git a/src/tools/rbd_mirror/ImageReplayer.cc b/src/tools/rbd_mirror/ImageReplayer.cc index 5c4920c10ddc..8d503516890b 100644 --- a/src/tools/rbd_mirror/ImageReplayer.cc +++ b/src/tools/rbd_mirror/ImageReplayer.cc @@ -51,7 +51,7 @@ template struct ReplayHandler : public ::journal::ReplayHandler { ImageReplayer *replayer; ReplayHandler(ImageReplayer *replayer) : replayer(replayer) {} -virtual void get() {} + virtual void get() {} virtual void put() {} virtual void handle_entries_available() { @@ -282,6 +282,7 @@ ImageReplayer::~ImageReplayer() assert(m_replay_handler == nullptr); assert(m_on_start_finish == nullptr); assert(m_on_stop_finish == nullptr); + assert(m_bootstrap_request == nullptr); assert(m_in_flight_status_updates == 0); delete m_asok_hook; } @@ -366,7 +367,6 @@ void ImageReplayer::bootstrap() { dout(20) << "bootstrap params: " << "local_image_name=" << m_local_image_name << dendl; - // TODO: add a new bootstrap state and support canceling Context *ctx = create_context_callback< ImageReplayer, &ImageReplayer::handle_bootstrap>(this); @@ -379,6 +379,7 @@ void ImageReplayer::bootstrap() { { Mutex::Locker locker(m_lock); + request->get(); m_bootstrap_request = request; } @@ -394,6 +395,7 @@ void ImageReplayer::handle_bootstrap(int r) { { Mutex::Locker locker(m_lock); + m_bootstrap_request->put(); m_bootstrap_request = nullptr; if (m_local_image_ctx) { m_local_image_id = m_local_image_ctx->id; @@ -555,7 +557,10 @@ void ImageReplayer::stop(Context *on_finish, bool manual) } else { if (!is_stopped_()) { if (m_state == STATE_STARTING) { - dout(20) << "interrupting start" << dendl; + dout(20) << "canceling start" << dendl; + if (m_bootstrap_request) { + m_bootstrap_request->cancel(); + } } else { dout(20) << "interrupting replay" << dendl; shut_down_replay = true; diff --git a/src/tools/rbd_mirror/ImageSync.cc b/src/tools/rbd_mirror/ImageSync.cc index ff2c7bb1561a..629d1ba62c2b 100644 --- a/src/tools/rbd_mirror/ImageSync.cc +++ b/src/tools/rbd_mirror/ImageSync.cc @@ -33,16 +33,22 @@ ImageSync::ImageSync(I *local_image_ctx, I *remote_image_ctx, MirrorPeerClientMeta *client_meta, ContextWQ *work_queue, Context *on_finish, ProgressContext *progress_ctx) - : m_local_image_ctx(local_image_ctx), m_remote_image_ctx(remote_image_ctx), + : BaseRequest("rbd::mirror::ImageSync", local_image_ctx->cct, on_finish), + m_local_image_ctx(local_image_ctx), m_remote_image_ctx(remote_image_ctx), m_timer(timer), m_timer_lock(timer_lock), m_mirror_uuid(mirror_uuid), m_journaler(journaler), m_client_meta(client_meta), - m_work_queue(work_queue), m_on_finish(on_finish), - m_progress_ctx(progress_ctx), + m_work_queue(work_queue), m_progress_ctx(progress_ctx), m_lock(unique_lock_name("ImageSync::m_lock", this)) { } template -void ImageSync::start() { +ImageSync::~ImageSync() { + assert(m_snapshot_copy_request == nullptr); + assert(m_image_copy_request == nullptr); +} + +template +void ImageSync::send() { send_prune_catch_up_sync_point(); } @@ -54,6 +60,11 @@ void ImageSync::cancel() { ldout(cct, 20) << dendl; m_canceled = true; + + if (m_snapshot_copy_request != nullptr) { + m_snapshot_copy_request->cancel(); + } + if (m_image_copy_request != nullptr) { m_image_copy_request->cancel(); } @@ -131,17 +142,27 @@ void ImageSync::handle_create_sync_point(int r) { template void ImageSync::send_copy_snapshots() { - update_progress("COPY_SNAPSHOTS"); + m_lock.Lock(); + if (m_canceled) { + m_lock.Unlock(); + finish(-ECANCELED); + return; + } CephContext *cct = m_local_image_ctx->cct; ldout(cct, 20) << dendl; Context *ctx = create_context_callback< ImageSync, &ImageSync::handle_copy_snapshots>(this); - SnapshotCopyRequest *request = SnapshotCopyRequest::create( + m_snapshot_copy_request = SnapshotCopyRequest::create( m_local_image_ctx, m_remote_image_ctx, &m_snap_map, m_journaler, m_client_meta, m_work_queue, ctx); - request->send(); + m_snapshot_copy_request->get(); + m_lock.Unlock(); + + update_progress("COPY_SNAPSHOTS"); + + m_snapshot_copy_request->send(); } template @@ -149,7 +170,20 @@ void ImageSync::handle_copy_snapshots(int r) { CephContext *cct = m_local_image_ctx->cct; ldout(cct, 20) << ": r=" << r << dendl; - if (r < 0) { + { + Mutex::Locker locker(m_lock); + m_snapshot_copy_request->put(); + m_snapshot_copy_request = nullptr; + if (r == 0 && m_canceled) { + r = -ECANCELED; + } + } + + if (r == -ECANCELED) { + ldout(cct, 10) << ": snapshot copy canceled" << dendl; + finish(r); + return; + } else if (r < 0) { lderr(cct) << ": failed to copy snapshot metadata: " << cpp_strerror(r) << dendl; finish(r); @@ -177,6 +211,7 @@ void ImageSync::send_copy_image() { m_local_image_ctx, m_remote_image_ctx, m_timer, m_timer_lock, m_journaler, m_client_meta, &m_client_meta->sync_points.front(), ctx, m_progress_ctx); + m_image_copy_request->get(); m_lock.Unlock(); update_progress("COPY_IMAGE"); @@ -186,17 +221,18 @@ void ImageSync::send_copy_image() { template void ImageSync::handle_copy_image(int r) { + CephContext *cct = m_local_image_ctx->cct; + ldout(cct, 20) << ": r=" << r << dendl; + { Mutex::Locker locker(m_lock); + m_image_copy_request->put(); m_image_copy_request = nullptr; if (r == 0 && m_canceled) { r = -ECANCELED; } } - CephContext *cct = m_local_image_ctx->cct; - ldout(cct, 20) << ": r=" << r << dendl; - if (r == -ECANCELED) { ldout(cct, 10) << ": image copy canceled" << dendl; finish(r); @@ -314,15 +350,6 @@ void ImageSync::handle_prune_sync_points(int r) { finish(0); } -template -void ImageSync::finish(int r) { - CephContext *cct = m_local_image_ctx->cct; - ldout(cct, 20) << ": r=" << r << dendl; - - m_on_finish->complete(r); - delete this; -} - template void ImageSync::update_progress(const std::string &description) { dout(20) << ": " << description << dendl; diff --git a/src/tools/rbd_mirror/ImageSync.h b/src/tools/rbd_mirror/ImageSync.h index abe0f4648ddf..97bad5471c4a 100644 --- a/src/tools/rbd_mirror/ImageSync.h +++ b/src/tools/rbd_mirror/ImageSync.h @@ -8,6 +8,7 @@ #include "librbd/ImageCtx.h" #include "librbd/journal/TypeTraits.h" #include "common/Mutex.h" +#include "tools/rbd_mirror/BaseRequest.h" #include #include @@ -24,9 +25,10 @@ namespace mirror { class ProgressContext; namespace image_sync { template class ImageCopyRequest; } +namespace image_sync { template class SnapshotCopyRequest; } template -class ImageSync { +class ImageSync : public BaseRequest { public: typedef librbd::journal::TypeTraits TypeTraits; typedef typename TypeTraits::Journaler Journaler; @@ -49,8 +51,9 @@ public: Journaler *journaler, MirrorPeerClientMeta *client_meta, ContextWQ *work_queue, Context *on_finish, ProgressContext *progress_ctx = nullptr); + ~ImageSync(); - void start(); + void send(); void cancel(); private: @@ -97,7 +100,6 @@ private: Journaler *m_journaler; MirrorPeerClientMeta *m_client_meta; ContextWQ *m_work_queue; - Context *m_on_finish; ProgressContext *m_progress_ctx; SnapMap m_snap_map; @@ -105,7 +107,8 @@ private: Mutex m_lock; bool m_canceled = false; - image_sync::ImageCopyRequest *m_image_copy_request; + image_sync::SnapshotCopyRequest *m_snapshot_copy_request = nullptr; + image_sync::ImageCopyRequest *m_image_copy_request = nullptr; decltype(ImageCtxT::object_map) m_object_map = nullptr; void send_prune_catch_up_sync_point(); @@ -129,8 +132,6 @@ private: void send_prune_sync_points(); void handle_prune_sync_points(int r); - void finish(int r); - void update_progress(const std::string &description); }; diff --git a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc index b90b60856f35..b42894243117 100644 --- a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc +++ b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc @@ -32,6 +32,7 @@ namespace image_replayer { using librbd::util::create_context_callback; using librbd::util::create_rados_ack_callback; +using librbd::util::unique_lock_name; template BootstrapRequest::BootstrapRequest(librados::IoCtx &local_io_ctx, @@ -48,18 +49,21 @@ BootstrapRequest::BootstrapRequest(librados::IoCtx &local_io_ctx, MirrorPeerClientMeta *client_meta, Context *on_finish, rbd::mirror::ProgressContext *progress_ctx) - : m_local_io_ctx(local_io_ctx), m_remote_io_ctx(remote_io_ctx), + : BaseRequest("rbd::mirror::image_replayer::BootstrapRequest", + reinterpret_cast(local_io_ctx.cct()), on_finish), + m_local_io_ctx(local_io_ctx), m_remote_io_ctx(remote_io_ctx), m_local_image_ctx(local_image_ctx), m_local_image_name(local_image_name), m_remote_image_id(remote_image_id), m_global_image_id(global_image_id), m_work_queue(work_queue), m_timer(timer), m_timer_lock(timer_lock), m_local_mirror_uuid(local_mirror_uuid), m_remote_mirror_uuid(remote_mirror_uuid), m_journaler(journaler), - m_client_meta(client_meta), m_on_finish(on_finish), - m_progress_ctx(progress_ctx) { + m_client_meta(client_meta), m_progress_ctx(progress_ctx), + m_lock(unique_lock_name("BootstrapRequest::m_lock", this)) { } template BootstrapRequest::~BootstrapRequest() { + assert(m_image_sync_request == nullptr); assert(m_remote_image_ctx == nullptr); } @@ -68,6 +72,18 @@ void BootstrapRequest::send() { get_local_image_id(); } +template +void BootstrapRequest::cancel() { + dout(20) << dendl; + + Mutex::Locker locker(m_lock); + m_canceled = true; + + if (m_image_sync_request) { + m_image_sync_request->cancel(); + } +} + template void BootstrapRequest::get_local_image_id() { dout(20) << dendl; @@ -406,6 +422,13 @@ void BootstrapRequest::handle_update_client(int r) { return; } + if (m_canceled) { + dout(10) << ": request canceled" << dendl; + m_ret_val = -ECANCELED; + close_local_image(); + return; + } + m_client_meta->image_id = m_local_image_id; get_remote_tags(); } @@ -441,6 +464,13 @@ void BootstrapRequest::handle_get_remote_tags(int r) { return; } + if (m_canceled) { + dout(10) << ": request canceled" << dendl; + m_ret_val = -ECANCELED; + close_local_image(); + return; + } + // decode the remote tags librbd::journal::TagData remote_tag_data; for (auto &tag : m_remote_tags) { @@ -517,13 +547,30 @@ void BootstrapRequest::image_sync() { m_local_mirror_uuid, m_journaler, m_client_meta, m_work_queue, ctx, m_progress_ctx); - request->start(); + { + Mutex::Locker locker(m_lock); + request->get(); + m_image_sync_request = request; + } + + request->send(); } template void BootstrapRequest::handle_image_sync(int r) { dout(20) << ": r=" << r << dendl; + { + Mutex::Locker locker(m_lock); + m_image_sync_request->put(); + m_image_sync_request = nullptr; + } + + if (m_canceled) { + dout(10) << ": request canceled" << dendl; + m_ret_val = -ECANCELED; + } + if (r < 0) { derr << ": failed to sync remote image: " << cpp_strerror(r) << dendl; m_ret_val = r; @@ -584,14 +631,6 @@ void BootstrapRequest::handle_close_remote_image(int r) { finish(m_ret_val); } -template -void BootstrapRequest::finish(int r) { - dout(20) << ": r=" << r << dendl; - - m_on_finish->complete(r); - delete this; -} - template bool BootstrapRequest::decode_client_meta() { dout(20) << dendl; @@ -617,7 +656,8 @@ bool BootstrapRequest::decode_client_meta() { *m_client_meta = *client_meta; - dout(20) << ": client found: image_id=" << m_local_image_id << dendl; + dout(20) << ": client found: image_id=" << m_local_image_id + << ", client_meta=" << *m_client_meta << dendl; return true; } diff --git a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.h b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.h index 0f6a79e5e661..35ca88395fa7 100644 --- a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.h +++ b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.h @@ -6,8 +6,10 @@ #include "include/int_types.h" #include "include/rados/librados.hpp" +#include "common/Mutex.h" #include "cls/journal/cls_journal_types.h" #include "librbd/journal/TypeTraits.h" +#include "tools/rbd_mirror/BaseRequest.h" #include #include @@ -22,12 +24,13 @@ namespace librbd { namespace journal { struct MirrorPeerClientMeta; } } namespace rbd { namespace mirror { +template class ImageSync; class ProgressContext; namespace image_replayer { template -class BootstrapRequest { +class BootstrapRequest : public BaseRequest { public: typedef librbd::journal::TypeTraits TypeTraits; typedef typename TypeTraits::Journaler Journaler; @@ -70,6 +73,7 @@ public: ~BootstrapRequest(); void send(); + void cancel(); private: /** @@ -142,8 +146,10 @@ private: std::string m_remote_mirror_uuid; Journaler *m_journaler; MirrorPeerClientMeta *m_client_meta; - Context *m_on_finish; ProgressContext *m_progress_ctx; + Mutex m_lock; + ImageSync *m_image_sync_request = nullptr; + bool m_canceled = false; Tags m_remote_tags; cls::journal::Client m_client; @@ -193,8 +199,6 @@ private: void close_remote_image(); void handle_close_remote_image(int r); - void finish(int r); - bool decode_client_meta(); void update_progress(const std::string &description); diff --git a/src/tools/rbd_mirror/image_sync/ImageCopyRequest.cc b/src/tools/rbd_mirror/image_sync/ImageCopyRequest.cc index df41d8181de6..1cd582ab1fdb 100644 --- a/src/tools/rbd_mirror/image_sync/ImageCopyRequest.cc +++ b/src/tools/rbd_mirror/image_sync/ImageCopyRequest.cc @@ -29,10 +29,12 @@ ImageCopyRequest::ImageCopyRequest(I *local_image_ctx, I *remote_image_ctx, MirrorPeerSyncPoint *sync_point, Context *on_finish, ProgressContext *progress_ctx) - : m_local_image_ctx(local_image_ctx), m_remote_image_ctx(remote_image_ctx), + : BaseRequest("rbd::mirror::image_sync::ImageCopyRequest", + local_image_ctx->cct, on_finish), + m_local_image_ctx(local_image_ctx), m_remote_image_ctx(remote_image_ctx), m_timer(timer), m_timer_lock(timer_lock), m_journaler(journaler), m_client_meta(client_meta), m_sync_point(sync_point), - m_on_finish(on_finish), m_progress_ctx(progress_ctx), + m_progress_ctx(progress_ctx), m_lock(unique_lock_name("ImageCopyRequest::m_lock", this)), m_client_meta_copy(*client_meta) { assert(!m_client_meta_copy.sync_points.empty()); @@ -100,9 +102,19 @@ void ImageCopyRequest::handle_update_max_object_count(int r) { CephContext *cct = m_local_image_ctx->cct; ldout(cct, 20) << ": r=" << r << dendl; + if (r == 0) { + Mutex::Locker locker(m_lock); + if (m_canceled) { + ldout(cct, 10) << ": image copy canceled" << dendl; + r = -ECANCELED; + } + } + if (r < 0) { - lderr(cct) << ": failed to update client data: " << cpp_strerror(r) - << dendl; + if (r != -ECANCELED) { + lderr(cct) << ": failed to update client data: " << cpp_strerror(r) + << dendl; + } finish(r); return; } @@ -147,15 +159,19 @@ void ImageCopyRequest::send_object_copies() { template void ImageCopyRequest::send_next_object_copy() { assert(m_lock.is_locked()); - if (m_canceled) { - return; - } else if (m_ret_val < 0 || m_object_no >= m_end_object_no) { + CephContext *cct = m_local_image_ctx->cct; + + if (m_canceled && m_ret_val == 0) { + ldout(cct, 10) << ": image copy canceled" << dendl; + m_ret_val = -ECANCELED; + } + + if (m_ret_val < 0 || m_object_no >= m_end_object_no) { return; } uint64_t ono = m_object_no++; - CephContext *cct = m_local_image_ctx->cct; ldout(cct, 20) << ": object_num=" << ono << dendl; ++m_current_ops; @@ -178,6 +194,7 @@ void ImageCopyRequest::handle_object_copy(int r) { Mutex::Locker locker(m_lock); assert(m_current_ops > 0); --m_current_ops; + percent = 100 * m_object_no / m_end_object_no; if (r < 0) { @@ -244,15 +261,6 @@ void ImageCopyRequest::handle_flush_sync_point(int r) { finish(0); } -template -void ImageCopyRequest::finish(int r) { - CephContext *cct = m_local_image_ctx->cct; - ldout(cct, 20) << ": r=" << r << dendl; - - m_on_finish->complete(r); - delete this; -} - template int ImageCopyRequest::compute_snap_map() { CephContext *cct = m_local_image_ctx->cct; diff --git a/src/tools/rbd_mirror/image_sync/ImageCopyRequest.h b/src/tools/rbd_mirror/image_sync/ImageCopyRequest.h index 7198aaefac55..118b48d80235 100644 --- a/src/tools/rbd_mirror/image_sync/ImageCopyRequest.h +++ b/src/tools/rbd_mirror/image_sync/ImageCopyRequest.h @@ -9,6 +9,7 @@ #include "common/Mutex.h" #include "librbd/journal/Types.h" #include "librbd/journal/TypeTraits.h" +#include "tools/rbd_mirror/BaseRequest.h" #include #include @@ -25,7 +26,7 @@ class ProgressContext; namespace image_sync { template -class ImageCopyRequest { +class ImageCopyRequest : public BaseRequest { public: typedef std::vector SnapIds; typedef std::map SnapMap; @@ -87,7 +88,6 @@ private: Journaler *m_journaler; MirrorPeerClientMeta *m_client_meta; MirrorPeerSyncPoint *m_sync_point; - Context *m_on_finish; ProgressContext *m_progress_ctx; SnapMap m_snap_map; @@ -112,8 +112,6 @@ private: void send_flush_sync_point(); void handle_flush_sync_point(int r); - void finish(int r); - int compute_snap_map(); void update_progress(const std::string &description, bool flush = true); diff --git a/src/tools/rbd_mirror/image_sync/SnapshotCopyRequest.cc b/src/tools/rbd_mirror/image_sync/SnapshotCopyRequest.cc index bee21c019e66..671bc0a06058 100644 --- a/src/tools/rbd_mirror/image_sync/SnapshotCopyRequest.cc +++ b/src/tools/rbd_mirror/image_sync/SnapshotCopyRequest.cc @@ -36,6 +36,7 @@ const std::string &get_snapshot_name(I *image_ctx, librados::snap_t snap_id) { } // anonymous namespace using librbd::util::create_context_callback; +using librbd::util::unique_lock_name; template SnapshotCopyRequest::SnapshotCopyRequest(I *local_image_ctx, @@ -45,10 +46,12 @@ SnapshotCopyRequest::SnapshotCopyRequest(I *local_image_ctx, librbd::journal::MirrorPeerClientMeta *meta, ContextWQ *work_queue, Context *on_finish) - : m_local_image_ctx(local_image_ctx), m_remote_image_ctx(remote_image_ctx), + : BaseRequest("rbd::mirror::image_sync::SnapshotCopyRequest", + local_image_ctx->cct, on_finish), + m_local_image_ctx(local_image_ctx), m_remote_image_ctx(remote_image_ctx), m_snap_map(snap_map), m_journaler(journaler), m_client_meta(meta), - m_work_queue(work_queue), m_on_finish(on_finish), - m_snap_seqs(meta->snap_seqs) { + m_work_queue(work_queue), m_snap_seqs(meta->snap_seqs), + m_lock(unique_lock_name("SnapshotCopyRequest::m_lock", this)) { m_snap_map->clear(); // snap ids ordered from oldest to newest @@ -78,6 +81,15 @@ void SnapshotCopyRequest::send() { send_snap_unprotect(); } +template +void SnapshotCopyRequest::cancel() { + Mutex::Locker locker(m_lock); + + CephContext *cct = m_local_image_ctx->cct; + ldout(cct, 20) << dendl; + m_canceled = true; +} + template void SnapshotCopyRequest::send_snap_unprotect() { CephContext *cct = m_local_image_ctx->cct; @@ -173,6 +185,10 @@ void SnapshotCopyRequest::handle_snap_unprotect(int r) { finish(r); return; } + if (handle_cancellation()) + { + return; + } send_snap_unprotect(); } @@ -233,6 +249,10 @@ void SnapshotCopyRequest::handle_snap_remove(int r) { finish(r); return; } + if (handle_cancellation()) + { + return; + } send_snap_remove(); } @@ -314,6 +334,10 @@ void SnapshotCopyRequest::handle_snap_create(int r) { finish(r); return; } + if (handle_cancellation()) + { + return; + } assert(m_prev_snap_id != CEPH_NOSNAP); @@ -412,6 +436,10 @@ void SnapshotCopyRequest::handle_snap_protect(int r) { finish(r); return; } + if (handle_cancellation()) + { + return; + } send_snap_protect(); } @@ -447,6 +475,10 @@ void SnapshotCopyRequest::handle_update_client(int r) { finish(r); return; } + if (handle_cancellation()) + { + return; + } m_client_meta->snap_seqs = m_snap_seqs; @@ -454,24 +486,24 @@ void SnapshotCopyRequest::handle_update_client(int r) { } template -void SnapshotCopyRequest::error(int r) { - dout(20) << ": r=" << r << dendl; - - m_work_queue->queue(create_context_callback< - SnapshotCopyRequest, &SnapshotCopyRequest::finish>(this), r); +bool SnapshotCopyRequest::handle_cancellation() { + { + Mutex::Locker locker(m_lock); + if (!m_canceled) { + return false; + } + } + CephContext *cct = m_local_image_ctx->cct; + ldout(cct, 10) << ": snapshot copy canceled" << dendl; + finish(-ECANCELED); + return true; } template -void SnapshotCopyRequest::finish(int r) { - CephContext *cct = m_local_image_ctx->cct; - ldout(cct, 20) << ": r=" << r << dendl; - - if (r >= 0) { - m_client_meta->snap_seqs = m_snap_seqs; - } +void SnapshotCopyRequest::error(int r) { + dout(20) << ": r=" << r << dendl; - m_on_finish->complete(r); - delete this; + m_work_queue->queue(new FunctionContext([this, r](int r1) { finish(r); })); } template diff --git a/src/tools/rbd_mirror/image_sync/SnapshotCopyRequest.h b/src/tools/rbd_mirror/image_sync/SnapshotCopyRequest.h index 02b5ce74a39d..2be105e9d065 100644 --- a/src/tools/rbd_mirror/image_sync/SnapshotCopyRequest.h +++ b/src/tools/rbd_mirror/image_sync/SnapshotCopyRequest.h @@ -10,6 +10,7 @@ #include "librbd/ImageCtx.h" #include "librbd/parent_types.h" #include "librbd/journal/TypeTraits.h" +#include "tools/rbd_mirror/BaseRequest.h" #include #include #include @@ -25,7 +26,7 @@ namespace mirror { namespace image_sync { template -class SnapshotCopyRequest { +class SnapshotCopyRequest : public BaseRequest { public: typedef librbd::journal::TypeTraits TypeTraits; typedef typename TypeTraits::Journaler Journaler; @@ -50,6 +51,7 @@ public: ContextWQ *work_queue, Context *on_finish); void send(); + void cancel(); private: /** @@ -95,7 +97,6 @@ private: Journaler *m_journaler; librbd::journal::MirrorPeerClientMeta *m_client_meta; ContextWQ *m_work_queue; - Context *m_on_finish; SnapIdSet m_local_snap_ids; SnapIdSet m_remote_snap_ids; @@ -106,6 +107,9 @@ private: librbd::parent_spec m_local_parent_spec; + Mutex m_lock; + bool m_canceled = false; + void send_snap_unprotect(); void handle_snap_unprotect(int r); @@ -121,8 +125,9 @@ private: void send_update_client(); void handle_update_client(int r); + bool handle_cancellation(); + void error(int r); - void finish(int r); void compute_snap_map();