From fccbc70fe15e4c92c22467b204362881b12468f1 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Wed, 5 Feb 2020 10:42:27 -0500 Subject: [PATCH] librbd: deep-copy should accept a lower-bound for the destination snap_id For snapshot-based mirroring, we will want to prevent the modification of snapshots below the last sync snapshot and to prevent the copying of data below that lower-bound as well. This commit just adds the new parameter and future commits will update the snapshot and object copy behavior. Signed-off-by: Jason Dillaman --- src/librbd/DeepCopyRequest.cc | 44 ++++++++++---------- src/librbd/DeepCopyRequest.h | 22 ++++++---- src/librbd/api/Image.cc | 2 +- src/test/librbd/test_mock_DeepCopyRequest.cc | 14 +++---- src/test/rbd_mirror/test_mock_ImageSync.cc | 5 ++- src/tools/rbd_mirror/ImageSync.cc | 2 +- 6 files changed, 49 insertions(+), 40 deletions(-) diff --git a/src/librbd/DeepCopyRequest.cc b/src/librbd/DeepCopyRequest.cc index a8955a769b5..3d93699dc6e 100644 --- a/src/librbd/DeepCopyRequest.cc +++ b/src/librbd/DeepCopyRequest.cc @@ -27,18 +27,20 @@ using librbd::util::unique_lock_name; template DeepCopyRequest::DeepCopyRequest(I *src_image_ctx, I *dst_image_ctx, - librados::snap_t snap_id_start, - librados::snap_t snap_id_end, bool flatten, + librados::snap_t src_snap_id_start, + librados::snap_t src_snap_id_end, + librados::snap_t dst_snap_id_start, + bool flatten, const ObjectNumber &object_number, ContextWQ *work_queue, SnapSeqs *snap_seqs, ProgressContext *prog_ctx, Context *on_finish) : RefCountedObject(dst_image_ctx->cct), m_src_image_ctx(src_image_ctx), - m_dst_image_ctx(dst_image_ctx), m_snap_id_start(snap_id_start), - m_snap_id_end(snap_id_end), m_flatten(flatten), - m_object_number(object_number), m_work_queue(work_queue), - m_snap_seqs(snap_seqs), m_prog_ctx(prog_ctx), m_on_finish(on_finish), - m_cct(dst_image_ctx->cct), + m_dst_image_ctx(dst_image_ctx), m_src_snap_id_start(src_snap_id_start), + m_src_snap_id_end(src_snap_id_end), m_dst_snap_id_start(dst_snap_id_start), + m_flatten(flatten), m_object_number(object_number), + m_work_queue(work_queue), m_snap_seqs(snap_seqs), m_prog_ctx(prog_ctx), + m_on_finish(on_finish), m_cct(dst_image_ctx->cct), m_lock(ceph::make_mutex(unique_lock_name("DeepCopyRequest::m_lock", this))) { } @@ -102,8 +104,8 @@ void DeepCopyRequest::send_copy_snapshots() { Context *ctx = create_context_callback< DeepCopyRequest, &DeepCopyRequest::handle_copy_snapshots>(this); m_snapshot_copy_request = SnapshotCopyRequest::create( - m_src_image_ctx, m_dst_image_ctx, m_snap_id_end, m_flatten, m_work_queue, - m_snap_seqs, ctx); + m_src_image_ctx, m_dst_image_ctx, m_src_snap_id_end, m_flatten, + m_work_queue, m_snap_seqs, ctx); m_snapshot_copy_request->get(); m_lock.unlock(); @@ -134,7 +136,7 @@ void DeepCopyRequest::handle_copy_snapshots(int r) { return; } - if (m_snap_id_end == CEPH_NOSNAP) { + if (m_src_snap_id_end == CEPH_NOSNAP) { (*m_snap_seqs)[CEPH_NOSNAP] = CEPH_NOSNAP; } @@ -155,8 +157,8 @@ void DeepCopyRequest::send_copy_image() { Context *ctx = create_context_callback< DeepCopyRequest, &DeepCopyRequest::handle_copy_image>(this); m_image_copy_request = ImageCopyRequest::create( - m_src_image_ctx, m_dst_image_ctx, m_snap_id_start, m_snap_id_end, - m_flatten, m_object_number, *m_snap_seqs, m_prog_ctx, ctx); + m_src_image_ctx, m_dst_image_ctx, m_src_snap_id_start, m_src_snap_id_end, + m_flatten, m_object_number, *m_snap_seqs, m_prog_ctx, ctx); m_image_copy_request->get(); m_lock.unlock(); @@ -201,7 +203,7 @@ void DeepCopyRequest::send_copy_object_map() { send_copy_metadata(); return; } - if (m_snap_id_end == CEPH_NOSNAP) { + if (m_src_snap_id_end == CEPH_NOSNAP) { m_dst_image_ctx->image_lock.unlock_shared(); m_dst_image_ctx->owner_lock.unlock_shared(); send_refresh_object_map(); @@ -230,8 +232,8 @@ void DeepCopyRequest::send_copy_object_map() { handle_copy_object_map(r); finish_op_ctx->complete(0); }); - ceph_assert(m_snap_seqs->count(m_snap_id_end) > 0); - librados::snap_t copy_snap_id = (*m_snap_seqs)[m_snap_id_end]; + ceph_assert(m_snap_seqs->count(m_src_snap_id_end) > 0); + librados::snap_t copy_snap_id = (*m_snap_seqs)[m_src_snap_id_end]; m_dst_image_ctx->object_map->rollback(copy_snap_id, ctx); m_dst_image_ctx->image_lock.unlock_shared(); m_dst_image_ctx->owner_lock.unlock_shared(); @@ -327,17 +329,17 @@ template int DeepCopyRequest::validate_copy_points() { std::shared_lock image_locker{m_src_image_ctx->image_lock}; - if (m_snap_id_start != 0 && - m_src_image_ctx->snap_info.find(m_snap_id_start) == + if (m_src_snap_id_start != 0 && + m_src_image_ctx->snap_info.find(m_src_snap_id_start) == m_src_image_ctx->snap_info.end()) { - lderr(m_cct) << "invalid start snap_id " << m_snap_id_start << dendl; + lderr(m_cct) << "invalid start snap_id " << m_src_snap_id_start << dendl; return -EINVAL; } - if (m_snap_id_end != CEPH_NOSNAP && - m_src_image_ctx->snap_info.find(m_snap_id_end) == + if (m_src_snap_id_end != CEPH_NOSNAP && + m_src_image_ctx->snap_info.find(m_src_snap_id_end) == m_src_image_ctx->snap_info.end()) { - lderr(m_cct) << "invalid end snap_id " << m_snap_id_end << dendl; + lderr(m_cct) << "invalid end snap_id " << m_src_snap_id_end << dendl; return -EINVAL; } diff --git a/src/librbd/DeepCopyRequest.h b/src/librbd/DeepCopyRequest.h index 77b87ddc94f..0348246e722 100644 --- a/src/librbd/DeepCopyRequest.h +++ b/src/librbd/DeepCopyRequest.h @@ -32,20 +32,25 @@ class DeepCopyRequest : public RefCountedObject { public: static DeepCopyRequest* create(ImageCtxT *src_image_ctx, ImageCtxT *dst_image_ctx, - librados::snap_t snap_id_start, - librados::snap_t snap_id_end, bool flatten, + librados::snap_t src_snap_id_start, + librados::snap_t src_snap_id_end, + librados::snap_t dst_snap_id_start, + bool flatten, const deep_copy::ObjectNumber &object_number, ContextWQ *work_queue, SnapSeqs *snap_seqs, ProgressContext *prog_ctx, Context *on_finish) { - return new DeepCopyRequest(src_image_ctx, dst_image_ctx, snap_id_start, - snap_id_end, flatten, object_number, work_queue, - snap_seqs, prog_ctx, on_finish); + return new DeepCopyRequest(src_image_ctx, dst_image_ctx, src_snap_id_start, + src_snap_id_end, dst_snap_id_start, flatten, + object_number, work_queue, snap_seqs, prog_ctx, + on_finish); } DeepCopyRequest(ImageCtxT *src_image_ctx, ImageCtxT *dst_image_ctx, - librados::snap_t snap_id_start, librados::snap_t snap_id_end, + librados::snap_t src_snap_id_start, + librados::snap_t src_snap_id_end, + librados::snap_t dst_snap_id_start, bool flatten, const deep_copy::ObjectNumber &object_number, ContextWQ *work_queue, SnapSeqs *snap_seqs, ProgressContext *prog_ctx, Context *on_finish); @@ -86,8 +91,9 @@ private: ImageCtxT *m_src_image_ctx; ImageCtxT *m_dst_image_ctx; - librados::snap_t m_snap_id_start; - librados::snap_t m_snap_id_end; + librados::snap_t m_src_snap_id_start; + librados::snap_t m_src_snap_id_end; + librados::snap_t m_dst_snap_id_start; bool m_flatten; deep_copy::ObjectNumber m_object_number; ContextWQ *m_work_queue; diff --git a/src/librbd/api/Image.cc b/src/librbd/api/Image.cc index c0bacaf675e..7b7be14acf1 100644 --- a/src/librbd/api/Image.cc +++ b/src/librbd/api/Image.cc @@ -680,7 +680,7 @@ int Image::deep_copy(I *src, I *dest, bool flatten, C_SaferCond cond; SnapSeqs snap_seqs; auto req = DeepCopyRequest::create(src, dest, snap_id_start, snap_id_end, - flatten, boost::none, op_work_queue, + 0U, flatten, boost::none, op_work_queue, &snap_seqs, &prog_ctx, &cond); req->send(); int r = cond.wait(); diff --git a/src/test/librbd/test_mock_DeepCopyRequest.cc b/src/test/librbd/test_mock_DeepCopyRequest.cc index d198620ba72..0329b057323 100644 --- a/src/test/librbd/test_mock_DeepCopyRequest.cc +++ b/src/test/librbd/test_mock_DeepCopyRequest.cc @@ -264,7 +264,7 @@ TEST_F(TestMockDeepCopyRequest, SimpleCopy) { librbd::SnapSeqs snap_seqs; librbd::NoOpProgressContext no_op; auto request = librbd::DeepCopyRequest::create( - &mock_src_image_ctx, &mock_dst_image_ctx, 0, CEPH_NOSNAP, false, + &mock_src_image_ctx, &mock_dst_image_ctx, 0, CEPH_NOSNAP, 0, false, boost::none, m_work_queue, &snap_seqs, &no_op, &ctx); request->send(); ASSERT_EQ(0, ctx.wait()); @@ -282,7 +282,7 @@ TEST_F(TestMockDeepCopyRequest, ErrorOnCopySnapshots) { librbd::SnapSeqs snap_seqs; librbd::NoOpProgressContext no_op; auto request = librbd::DeepCopyRequest::create( - &mock_src_image_ctx, &mock_dst_image_ctx, 0, CEPH_NOSNAP, false, + &mock_src_image_ctx, &mock_dst_image_ctx, 0, CEPH_NOSNAP, 0, false, boost::none, m_work_queue, &snap_seqs, &no_op, &ctx); request->send(); ASSERT_EQ(-EINVAL, ctx.wait()); @@ -315,7 +315,7 @@ TEST_F(TestMockDeepCopyRequest, ErrorOnRefreshObjectMap) { librbd::SnapSeqs snap_seqs; librbd::NoOpProgressContext no_op; auto request = librbd::DeepCopyRequest::create( - &mock_src_image_ctx, &mock_dst_image_ctx, 0, CEPH_NOSNAP, false, + &mock_src_image_ctx, &mock_dst_image_ctx, 0, CEPH_NOSNAP, 0, false, boost::none, m_work_queue, &snap_seqs, &no_op, &ctx); request->send(); ASSERT_EQ(-EINVAL, ctx.wait()); @@ -335,7 +335,7 @@ TEST_F(TestMockDeepCopyRequest, ErrorOnCopyImage) { librbd::SnapSeqs snap_seqs; librbd::NoOpProgressContext no_op; auto request = librbd::DeepCopyRequest::create( - &mock_src_image_ctx, &mock_dst_image_ctx, 0, CEPH_NOSNAP, false, + &mock_src_image_ctx, &mock_dst_image_ctx, 0, CEPH_NOSNAP, 0, false, boost::none, m_work_queue, &snap_seqs, &no_op, &ctx); request->send(); ASSERT_EQ(-EINVAL, ctx.wait()); @@ -372,7 +372,7 @@ TEST_F(TestMockDeepCopyRequest, ErrorOnCopyMetadata) { librbd::SnapSeqs snap_seqs; librbd::NoOpProgressContext no_op; auto request = librbd::DeepCopyRequest::create( - &mock_src_image_ctx, &mock_dst_image_ctx, 0, CEPH_NOSNAP, false, + &mock_src_image_ctx, &mock_dst_image_ctx, 0, CEPH_NOSNAP, 0, false, boost::none, m_work_queue, &snap_seqs, &no_op, &ctx); request->send(); ASSERT_EQ(-EINVAL, ctx.wait()); @@ -414,7 +414,7 @@ TEST_F(TestMockDeepCopyRequest, Snap) { librbd::NoOpProgressContext no_op; auto request = librbd::DeepCopyRequest::create( &mock_src_image_ctx, &mock_dst_image_ctx, 0, m_src_image_ctx->snap_id, - false, boost::none, m_work_queue, &snap_seqs, &no_op, &ctx); + 0, false, boost::none, m_work_queue, &snap_seqs, &no_op, &ctx); request->send(); ASSERT_EQ(0, ctx.wait()); } @@ -451,7 +451,7 @@ TEST_F(TestMockDeepCopyRequest, ErrorOnRollbackObjectMap) { librbd::NoOpProgressContext no_op; auto request = librbd::DeepCopyRequest::create( &mock_src_image_ctx, &mock_dst_image_ctx, 0, m_src_image_ctx->snap_id, - false, boost::none, m_work_queue, &snap_seqs, &no_op, &ctx); + 0, false, boost::none, m_work_queue, &snap_seqs, &no_op, &ctx); request->send(); ASSERT_EQ(-EINVAL, ctx.wait()); } diff --git a/src/test/rbd_mirror/test_mock_ImageSync.cc b/src/test/rbd_mirror/test_mock_ImageSync.cc index b86d4bee88c..743418325ce 100644 --- a/src/test/rbd_mirror/test_mock_ImageSync.cc +++ b/src/test/rbd_mirror/test_mock_ImageSync.cc @@ -33,8 +33,9 @@ public: static DeepCopyRequest* create( librbd::MockTestImageCtx *src_image_ctx, librbd::MockTestImageCtx *dst_image_ctx, - librados::snap_t snap_id_start, librados::snap_t snap_id_end, - bool flatten, const librbd::deep_copy::ObjectNumber &object_number, + librados::snap_t src_snap_id_start, librados::snap_t src_snap_id_end, + librados::snap_t dst_snap_id_start, bool flatten, + const librbd::deep_copy::ObjectNumber &object_number, ContextWQ *work_queue, SnapSeqs *snap_seqs, ProgressContext *prog_ctx, Context *on_finish) { ceph_assert(s_instance != nullptr); diff --git a/src/tools/rbd_mirror/ImageSync.cc b/src/tools/rbd_mirror/ImageSync.cc index 3ec9c6295aa..2765b66bf29 100644 --- a/src/tools/rbd_mirror/ImageSync.cc +++ b/src/tools/rbd_mirror/ImageSync.cc @@ -255,7 +255,7 @@ void ImageSync::send_copy_image() { m_image_copy_prog_ctx = new ImageCopyProgressContext(this); m_image_copy_request = librbd::DeepCopyRequest::create( m_remote_image_ctx, m_local_image_ctx, snap_id_start, snap_id_end, - false, object_number, m_threads->work_queue, &m_snap_seqs_copy, + 0, false, object_number, m_threads->work_queue, &m_snap_seqs_copy, m_image_copy_prog_ctx, ctx); m_image_copy_request->get(); m_lock.unlock(); -- 2.39.5