From f94c276accb8c9f4096b3bffc95362a921f2a2a5 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Wed, 5 Aug 2020 09:12:41 -0400 Subject: [PATCH] librbd: migration abort should revert data back to the original image If the migration destination image was modified and then the migration was aborted, we need to copy the data back to the source image to avoid losing data. For simplicity we will only revert the HEAD revision state and will not attempt to copy new snapshots on the destination image back to the source. Fixes: https://tracker.ceph.com/issues/41394 Signed-off-by: Jason Dillaman (cherry picked from commit 5bd15da8be09a4e7644d411a0b0c132e5b795393) Conflicts: src/librbd/api/Migration.cc: trivial resolution --- src/librbd/api/Migration.cc | 89 ++++++++++++++++++++++++++++++- src/librbd/api/Migration.h | 3 ++ src/test/librbd/test_Migration.cc | 22 ++++++++ 3 files changed, 113 insertions(+), 1 deletion(-) diff --git a/src/librbd/api/Migration.cc b/src/librbd/api/Migration.cc index a2cdabe1d589d..7d6e29647374b 100644 --- a/src/librbd/api/Migration.cc +++ b/src/librbd/api/Migration.cc @@ -18,6 +18,8 @@ #include "librbd/api/Image.h" #include "librbd/api/Snapshot.h" #include "librbd/api/Trash.h" +#include "librbd/deep_copy/Handler.h" +#include "librbd/deep_copy/ImageCopyRequest.h" #include "librbd/deep_copy/MetadataCopyRequest.h" #include "librbd/deep_copy/SnapshotCopyRequest.h" #include "librbd/exclusive_lock/Policy.h" @@ -337,6 +339,30 @@ int open_source_image(librados::IoCtx& io_ctx, const std::string &image_name, return 0; } +class SteppedProgressContext : public ProgressContext { +public: + SteppedProgressContext(ProgressContext* progress_ctx, size_t total_steps) + : m_progress_ctx(progress_ctx), m_total_steps(total_steps) { + } + + void next_step() { + ceph_assert(m_current_step < m_total_steps); + ++m_current_step; + } + + int update_progress(uint64_t object_number, + uint64_t object_count) override { + return m_progress_ctx->update_progress( + object_number + (object_count * (m_current_step - 1)), + object_count * m_total_steps); + } + +private: + ProgressContext* m_progress_ctx; + size_t m_total_steps; + size_t m_current_step = 1; +}; + } // anonymous namespace template @@ -820,6 +846,11 @@ int Migration::abort() { return r; } + // copy dst HEAD -> src HEAD + SteppedProgressContext progress_ctx(m_prog_ctx, 2); + revert_data(dst_image_ctx, m_src_image_ctx, &progress_ctx); + progress_ctx.next_step(); + ldout(m_cct, 10) << "relinking children" << dendl; r = relink_children(dst_image_ctx, m_src_image_ctx); if (r < 0) { @@ -863,7 +894,7 @@ int Migration::abort() { ImageCtx::get_thread_pool_instance(m_cct, &thread_pool, &op_work_queue); C_SaferCond on_remove; auto req = librbd::image::RemoveRequest<>::create( - m_dst_io_ctx, dst_image_ctx, false, false, *m_prog_ctx, op_work_queue, + m_dst_io_ctx, dst_image_ctx, false, false, progress_ctx, op_work_queue, &on_remove); req->send(); r = on_remove.wait(); @@ -1815,6 +1846,62 @@ int Migration::remove_src_image() { return 0; } +template +int Migration::revert_data(I* src_image_ctx, I* dst_image_ctx, + ProgressContext* prog_ctx) { + ldout(m_cct, 10) << dendl; + + cls::rbd::MigrationSpec migration_spec; + int r = cls_client::migration_get(&src_image_ctx->md_ctx, + src_image_ctx->header_oid, + &migration_spec); + + if (r < 0) { + lderr(m_cct) << "failed retrieving migration header: " << cpp_strerror(r) + << dendl; + return r; + } + + if (migration_spec.header_type != cls::rbd::MIGRATION_HEADER_TYPE_DST) { + lderr(m_cct) << "unexpected migration header type: " + << migration_spec.header_type << dendl; + return -EINVAL; + } + + uint64_t src_snap_id_start = 0; + uint64_t src_snap_id_end = CEPH_NOSNAP; + uint64_t dst_snap_id_start = 0; + if (!migration_spec.snap_seqs.empty()) { + src_snap_id_start = migration_spec.snap_seqs.rbegin()->second; + } + + // we only care about the HEAD revision so only add a single mapping to + // represent the most recent state + SnapSeqs snap_seqs; + snap_seqs[CEPH_NOSNAP] = CEPH_NOSNAP; + + ldout(m_cct, 20) << "src_snap_id_start=" << src_snap_id_start << ", " + << "src_snap_id_end=" << src_snap_id_end << ", " + << "dst_snap_id_start=" << dst_snap_id_start << ", " + << "snap_seqs=" << snap_seqs << dendl; + + C_SaferCond ctx; + deep_copy::ProgressHandler progress_handler(prog_ctx); + auto request = deep_copy::ImageCopyRequest::create( + src_image_ctx, dst_image_ctx, src_snap_id_start, src_snap_id_end, + dst_snap_id_start, false, {}, snap_seqs, &progress_handler, &ctx); + request->send(); + + r = ctx.wait(); + if (r < 0) { + lderr(m_cct) << "error reverting destination image data blocks back to " + << "source image: " << cpp_strerror(r) << dendl; + return r; + } + + return 0; +} + } // namespace api } // namespace librbd diff --git a/src/librbd/api/Migration.h b/src/librbd/api/Migration.h index d87f494076508..8d24e310d8136 100644 --- a/src/librbd/api/Migration.h +++ b/src/librbd/api/Migration.h @@ -96,6 +96,9 @@ private: const librbd::snap_info_t &src_snap, const librbd::linked_image_spec_t &child_image, bool migration_abort, bool reattach_child); + + int revert_data(ImageCtxT* src_image_ctx, ImageCtxT* dst_image_ctx, + ProgressContext *prog_ctx); }; } // namespace api diff --git a/src/test/librbd/test_Migration.cc b/src/test/librbd/test_Migration.cc index 38fdabe52ecfd..69182ddcea5f6 100644 --- a/src/test/librbd/test_Migration.cc +++ b/src/test/librbd/test_Migration.cc @@ -1116,6 +1116,28 @@ TEST_F(TestMigration, AbortInUseImage) { no_op)); } +TEST_F(TestMigration, AbortWithoutSnapshots) { + test_no_snaps(); + migration_prepare(m_ioctx, m_image_name); + migration_status(RBD_IMAGE_MIGRATION_STATE_PREPARED); + test_no_snaps(); + migration_abort(m_ioctx, m_image_name); +} + +TEST_F(TestMigration, AbortWithSnapshots) { + test_snaps(); + migration_prepare(m_ioctx, m_image_name); + migration_status(RBD_IMAGE_MIGRATION_STATE_PREPARED); + + test_no_snaps(); + flush(); + ASSERT_EQ(0, TestFixture::snap_create(*m_ictx, "dst-only-snap")); + + test_no_snaps(); + + migration_abort(m_ioctx, m_image_name); +} + TEST_F(TestMigration, CloneV1Parent) { const uint32_t CLONE_FORMAT = 1; -- 2.39.5