From 5ddc15a34b3bc2726ea15cfbfbe2bd28a329db99 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Wed, 5 Feb 2020 15:27:39 -0500 Subject: [PATCH] librbd: ensure deep-copy snapshot map includes all destination snap ids When deep-copying from an arbitrary start snapshot id, the snap sequence will be missing all older snapshots. Additionally, snapshot types that are not deep-copied still need to be included in the destination snap map. Signed-off-by: Jason Dillaman --- src/librbd/deep_copy/ImageCopyRequest.cc | 14 +++++--- src/librbd/deep_copy/ImageCopyRequest.h | 18 +++++----- src/librbd/deep_copy/Utils.cc | 42 +++++++++++++++++++++--- src/librbd/deep_copy/Utils.h | 8 +++-- src/librbd/image/RefreshRequest.cc | 4 +-- 5 files changed, 64 insertions(+), 22 deletions(-) diff --git a/src/librbd/deep_copy/ImageCopyRequest.cc b/src/librbd/deep_copy/ImageCopyRequest.cc index 2fd2ed4a40a51..75cdae00b8ac3 100644 --- a/src/librbd/deep_copy/ImageCopyRequest.cc +++ b/src/librbd/deep_copy/ImageCopyRequest.cc @@ -23,16 +23,16 @@ using librbd::util::unique_lock_name; template ImageCopyRequest::ImageCopyRequest(I *src_image_ctx, I *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, bool flatten, const ObjectNumber &object_number, const 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_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_flatten(flatten), m_object_number(object_number), 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("ImageCopyRequest::m_lock", this))) { @@ -40,8 +40,12 @@ ImageCopyRequest::ImageCopyRequest(I *src_image_ctx, I *dst_image_ctx, template void ImageCopyRequest::send() { - util::compute_snap_map(m_snap_id_start, m_snap_id_end, m_snap_seqs, + m_dst_image_ctx->image_lock.lock_shared(); + util::compute_snap_map(m_dst_image_ctx->cct, m_src_snap_id_start, + m_src_snap_id_end, m_dst_image_ctx->snaps, m_snap_seqs, &m_snap_map); + m_dst_image_ctx->image_lock.unlock_shared(); + if (m_snap_map.empty()) { lderr(m_cct) << "failed to map snapshots within boundary" << dendl; finish(-EINVAL); diff --git a/src/librbd/deep_copy/ImageCopyRequest.h b/src/librbd/deep_copy/ImageCopyRequest.h index 15c3cd2951d22..8b0860087229b 100644 --- a/src/librbd/deep_copy/ImageCopyRequest.h +++ b/src/librbd/deep_copy/ImageCopyRequest.h @@ -30,19 +30,21 @@ class ImageCopyRequest : public RefCountedObject { public: static ImageCopyRequest* 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, + bool flatten, const ObjectNumber &object_number, const SnapSeqs &snap_seqs, ProgressContext *prog_ctx, Context *on_finish) { - return new ImageCopyRequest(src_image_ctx, dst_image_ctx, snap_id_start, - snap_id_end, flatten, object_number, snap_seqs, - prog_ctx, on_finish); + return new ImageCopyRequest(src_image_ctx, dst_image_ctx, src_snap_id_start, + src_snap_id_end, flatten, object_number, + snap_seqs, prog_ctx, on_finish); } ImageCopyRequest(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, bool flatten, const ObjectNumber &object_number, const SnapSeqs &snap_seqs, ProgressContext *prog_ctx, Context *on_finish); @@ -68,8 +70,8 @@ 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; bool m_flatten; ObjectNumber m_object_number; SnapSeqs m_snap_seqs; diff --git a/src/librbd/deep_copy/Utils.cc b/src/librbd/deep_copy/Utils.cc index 32b96587d194c..c2dd25020e43b 100644 --- a/src/librbd/deep_copy/Utils.cc +++ b/src/librbd/deep_copy/Utils.cc @@ -1,29 +1,61 @@ // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- // vim: ts=8 sw=2 smarttab +#include "common/debug.h" #include "Utils.h" +#include namespace librbd { namespace deep_copy { namespace util { -void compute_snap_map(librados::snap_t snap_id_start, - librados::snap_t snap_id_end, +#define dout_subsys ceph_subsys_rbd +#undef dout_prefix +#define dout_prefix *_dout << "librbd::deep_copy::util::" << __func__ << ": " + +void compute_snap_map(CephContext* cct, + librados::snap_t src_snap_id_start, + librados::snap_t src_snap_id_end, + const SnapIds& dst_snap_ids, const SnapSeqs &snap_seqs, SnapMap *snap_map) { + std::set ordered_dst_snap_ids{ + dst_snap_ids.begin(), dst_snap_ids.end()}; + auto dst_snap_id_it = ordered_dst_snap_ids.begin(); + SnapIds snap_ids; for (auto &it : snap_seqs) { + // ensure all dst snap ids are included in the mapping table since + // deep copy will skip non-user snapshots + while (dst_snap_id_it != ordered_dst_snap_ids.end()) { + if (*dst_snap_id_it < it.second) { + snap_ids.insert(snap_ids.begin(), *dst_snap_id_it); + } else if (*dst_snap_id_it > it.second) { + break; + } + ++dst_snap_id_it; + } + + // we should only have the HEAD revision in the the last snap seq + ceph_assert(snap_ids.empty() || snap_ids[0] != CEPH_NOSNAP); snap_ids.insert(snap_ids.begin(), it.second); - if (it.first < snap_id_start) { + + if (it.first < src_snap_id_start) { continue; - } else if (it.first > snap_id_end) { + } else if (it.first > src_snap_id_end) { break; } (*snap_map)[it.first] = snap_ids; } + + ldout(cct, 10) << "src_snap_id_start=" << src_snap_id_start << ", " + << "src_snap_id_end=" << src_snap_id_end << ", " + << "dst_snap_ids=" << dst_snap_ids << ", " + << "snap_seqs=" << snap_seqs << ", " + << "snap_map=" << *snap_map << dendl; } -} // util +} // namespace util } // namespace deep_copy } // namespace librbd diff --git a/src/librbd/deep_copy/Utils.h b/src/librbd/deep_copy/Utils.h index 28e1d6d7ea475..0681548db0f07 100644 --- a/src/librbd/deep_copy/Utils.h +++ b/src/librbd/deep_copy/Utils.h @@ -10,12 +10,16 @@ #include +struct CephContext; + namespace librbd { namespace deep_copy { namespace util { -void compute_snap_map(librados::snap_t snap_id_start, - librados::snap_t snap_id_end, +void compute_snap_map(CephContext* cct, + librados::snap_t src_snap_id_start, + librados::snap_t src_snap_id_end, + const SnapIds& dst_snap_ids, const SnapSeqs &snap_seqs, SnapMap *snap_map); diff --git a/src/librbd/image/RefreshRequest.cc b/src/librbd/image/RefreshRequest.cc index eaccf7f36fd78..adc0afb4b8faf 100644 --- a/src/librbd/image/RefreshRequest.cc +++ b/src/librbd/image/RefreshRequest.cc @@ -1494,8 +1494,8 @@ bool RefreshRequest::get_migration_info(ParentImageInfo *parent_md, m_migration_spec.image_name, m_migration_spec.image_id, {}, overlap, m_migration_spec.flatten}; - deep_copy::util::compute_snap_map(0, CEPH_NOSNAP, snap_seqs, - &migration_info->snap_map); + deep_copy::util::compute_snap_map(m_image_ctx.cct, 0, CEPH_NOSNAP, {}, + snap_seqs, &migration_info->snap_map); return true; } -- 2.39.5