From bed4857487b041de50227a685179cfd997b36874 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Fri, 17 Aug 2018 10:17:55 -0400 Subject: [PATCH] librbd: always open first parent image if it exists for a snapshot The deep-copy and migration features required force-opening the parent image just in case the deep-flatten feature wasn't enabled on an image. This change simplies the code by always opening the direct parent image, which really only matters if a cloned image has snapshots w/o the deep-flatten feature. Signed-off-by: Jason Dillaman --- src/librbd/ImageCtx.h | 1 - src/librbd/Types.h | 4 +- src/librbd/deep_copy/ImageCopyRequest.cc | 102 +------------- src/librbd/deep_copy/ImageCopyRequest.h | 14 -- src/librbd/deep_copy/ObjectCopyRequest.cc | 46 ++++--- src/librbd/deep_copy/ObjectCopyRequest.h | 13 +- src/librbd/image/CloseRequest.cc | 30 ----- src/librbd/image/CloseRequest.h | 6 - src/librbd/image/RefreshParentRequest.cc | 125 ++---------------- src/librbd/image/RefreshParentRequest.h | 39 ++---- src/librbd/io/CopyupRequest.cc | 8 +- src/librbd/operation/MigrateRequest.cc | 5 +- .../deep_copy/test_mock_ImageCopyRequest.cc | 11 -- .../deep_copy/test_mock_ObjectCopyRequest.cc | 7 +- 14 files changed, 69 insertions(+), 342 deletions(-) diff --git a/src/librbd/ImageCtx.h b/src/librbd/ImageCtx.h index f623baffb65c8..62e76cff4f58a 100644 --- a/src/librbd/ImageCtx.h +++ b/src/librbd/ImageCtx.h @@ -125,7 +125,6 @@ namespace librbd { ImageCtx *parent; ImageCtx *child = nullptr; MigrationInfo migration_info; - ImageCtx *migration_parent = nullptr; cls::rbd::GroupSpec group_spec; uint64_t stripe_unit, stripe_count; uint64_t flags; diff --git a/src/librbd/Types.h b/src/librbd/Types.h index 901bc56dee10a..b1658ee69cbe0 100644 --- a/src/librbd/Types.h +++ b/src/librbd/Types.h @@ -117,8 +117,8 @@ struct SnapInfo { enum { OPEN_FLAG_SKIP_OPEN_PARENT = 1 << 0, - OPEN_FLAG_OLD_FORMAT = 1 << 1, - OPEN_FLAG_IGNORE_MIGRATING = 1 << 2, + OPEN_FLAG_OLD_FORMAT = 1 << 1, + OPEN_FLAG_IGNORE_MIGRATING = 1 << 2 }; struct MigrationInfo { diff --git a/src/librbd/deep_copy/ImageCopyRequest.cc b/src/librbd/deep_copy/ImageCopyRequest.cc index 14dd45235b12a..1f6ce40227454 100644 --- a/src/librbd/deep_copy/ImageCopyRequest.cc +++ b/src/librbd/deep_copy/ImageCopyRequest.cc @@ -48,7 +48,7 @@ void ImageCopyRequest::send() { return; } - send_open_parent(); + send_object_copies(); } template @@ -59,69 +59,6 @@ void ImageCopyRequest::cancel() { m_canceled = true; } -template -void ImageCopyRequest::send_open_parent() { - ParentSpec parent_spec; - { - RWLock::RLocker snap_locker(m_src_image_ctx->snap_lock); - RWLock::RLocker parent_locker(m_src_image_ctx->parent_lock); - - auto snap_id = m_snap_map.begin()->first; - auto parent_info = m_src_image_ctx->get_parent_info(snap_id); - if (parent_info == nullptr) { - ldout(m_cct, 20) << "could not find parent info for snap id " << snap_id - << dendl; - } else { - parent_spec = parent_info->spec; - } - } - - if (parent_spec.pool_id == -1) { - send_object_copies(); - return; - } - - ldout(m_cct, 20) << "pool_id=" << parent_spec.pool_id << ", image_id=" - << parent_spec.image_id << ", snap_id=" - << parent_spec.snap_id << dendl; - - librados::Rados rados(m_src_image_ctx->md_ctx); - librados::IoCtx parent_io_ctx; - int r = rados.ioctx_create2(parent_spec.pool_id, parent_io_ctx); - if (r < 0) { - lderr(m_cct) << "failed to access parent pool (id=" << parent_spec.pool_id - << "): " << cpp_strerror(r) << dendl; - finish(r); - return; - } - - // TODO support clone v2 parent namespaces - parent_io_ctx.set_namespace(m_src_image_ctx->md_ctx.get_namespace()); - - m_src_parent_image_ctx = I::create("", parent_spec.image_id, - parent_spec.snap_id, parent_io_ctx, true); - auto ctx = create_context_callback< - ImageCopyRequest, &ImageCopyRequest::handle_open_parent>(this); - - auto req = image::OpenRequest::create(m_src_parent_image_ctx, false, ctx); - req->send(); -} - -template -void ImageCopyRequest::handle_open_parent(int r) { - ldout(m_cct, 20) << "r=" << r << dendl; - - if (r < 0) { - lderr(m_cct) << "failed to open parent: " << cpp_strerror(r) << dendl; - m_src_parent_image_ctx->destroy(); - m_src_parent_image_ctx = nullptr; - finish(r); - return; - } - - send_object_copies(); -} - template void ImageCopyRequest::send_object_copies() { m_object_no = 0; @@ -157,7 +94,7 @@ void ImageCopyRequest::send_object_copies() { } if (complete) { - send_close_parent(); + finish(m_ret_val); } } @@ -185,8 +122,7 @@ void ImageCopyRequest::send_next_object_copy() { handle_object_copy(ono, r); }); ObjectCopyRequest *req = ObjectCopyRequest::create( - m_src_image_ctx, m_src_parent_image_ctx, m_dst_image_ctx, m_snap_map, ono, - m_flatten, ctx); + m_src_image_ctx, m_dst_image_ctx, m_snap_map, ono, m_flatten, ctx); req->send(); } @@ -226,40 +162,8 @@ void ImageCopyRequest::handle_object_copy(uint64_t object_no, int r) { } if (complete) { - send_close_parent(); - } -} - -template -void ImageCopyRequest::send_close_parent() { - if (m_src_parent_image_ctx == nullptr) { finish(m_ret_val); - return; - } - - ldout(m_cct, 20) << dendl; - - auto ctx = create_context_callback< - ImageCopyRequest, &ImageCopyRequest::handle_close_parent>(this); - auto req = image::CloseRequest::create(m_src_parent_image_ctx, ctx); - req->send(); -} - -template -void ImageCopyRequest::handle_close_parent(int r) { - ldout(m_cct, 20) << "r=" << r << dendl; - - if (r < 0) { - lderr(m_cct) << "failed to close parent: " << cpp_strerror(r) << dendl; - if (m_ret_val == 0) { - m_ret_val = r; - } } - - m_src_parent_image_ctx->destroy(); - m_src_parent_image_ctx = nullptr; - - finish(m_ret_val); } template diff --git a/src/librbd/deep_copy/ImageCopyRequest.h b/src/librbd/deep_copy/ImageCopyRequest.h index c7d020785673a..e50507e884859 100644 --- a/src/librbd/deep_copy/ImageCopyRequest.h +++ b/src/librbd/deep_copy/ImageCopyRequest.h @@ -55,19 +55,12 @@ private: * @verbatim * * - * | - * v - * OPEN_PARENT (skip if not needed) - * | * | . . . . . * | . . (parallel execution of * v v . multiple objects at once) * COPY_OBJECT . . . . * | * v - * CLOSE_PARENT (skip if not needed) - * | - * v * * * @endverbatim @@ -95,18 +88,11 @@ private: bool m_updating_progress = false; SnapMap m_snap_map; int m_ret_val = 0; - ImageCtxT *m_src_parent_image_ctx = nullptr; - - void send_open_parent(); - void handle_open_parent(int r); void send_object_copies(); void send_next_object_copy(); void handle_object_copy(uint64_t object_no, int r); - void send_close_parent(); - void handle_close_parent(int r); - void finish(int r); }; diff --git a/src/librbd/deep_copy/ObjectCopyRequest.cc b/src/librbd/deep_copy/ObjectCopyRequest.cc index 2c72fbeb7c8b9..9bbd21b4eaf32 100644 --- a/src/librbd/deep_copy/ObjectCopyRequest.cc +++ b/src/librbd/deep_copy/ObjectCopyRequest.cc @@ -41,13 +41,11 @@ using librbd::util::create_rados_callback; template ObjectCopyRequest::ObjectCopyRequest(I *src_image_ctx, - I *src_parent_image_ctx, I *dst_image_ctx, const SnapMap &snap_map, uint64_t dst_object_number, bool flatten, Context *on_finish) : m_src_image_ctx(src_image_ctx), - m_src_parent_image_ctx(src_parent_image_ctx), m_dst_image_ctx(dst_image_ctx), m_cct(dst_image_ctx->cct), m_snap_map(snap_map), m_dst_object_number(dst_object_number), m_flatten(flatten), m_on_finish(on_finish) { @@ -229,27 +227,35 @@ void ObjectCopyRequest::handle_read_object(int r) { template void ObjectCopyRequest::send_read_from_parent() { + m_src_image_ctx->snap_lock.get_read(); + m_src_image_ctx->parent_lock.get_read(); io::Extents image_extents; compute_read_from_parent_ops(&image_extents); + m_src_image_ctx->snap_lock.put_read(); + if (image_extents.empty()) { + m_src_image_ctx->parent_lock.put_read(); handle_read_from_parent(0); return; } ldout(m_cct, 20) << dendl; - ceph_assert(m_src_parent_image_ctx != nullptr); + ceph_assert(m_src_image_ctx->parent != nullptr); auto ctx = create_context_callback< ObjectCopyRequest, &ObjectCopyRequest::handle_read_from_parent>(this); auto comp = io::AioCompletion::create_and_start( - ctx, util::get_image_ctx(m_src_image_ctx), io::AIO_TYPE_READ); + ctx, util::get_image_ctx(m_src_image_ctx->parent), io::AIO_TYPE_READ); ldout(m_cct, 20) << "completion " << comp << ", extents " << image_extents << dendl; - io::ImageRequest::aio_read(m_src_parent_image_ctx, comp, + + auto src_image_ctx = m_src_image_ctx; + io::ImageRequest::aio_read(src_image_ctx->parent, comp, std::move(image_extents), io::ReadResult{&m_read_from_parent_data}, 0, ZTracer::Trace()); + src_image_ctx->parent_lock.put_read(); } template @@ -564,8 +570,12 @@ void ObjectCopyRequest::compute_read_ops() { m_read_snaps = {}; m_zero_interval = {}; + m_src_image_ctx->parent_lock.get_read(); + bool hide_parent = (m_src_image_ctx->parent != nullptr); + m_src_image_ctx->parent_lock.put_read(); + librados::snap_t src_copy_point_snap_id = m_snap_map.rbegin()->first; - bool prev_exists = m_src_parent_image_ctx != nullptr; + bool prev_exists = hide_parent; uint64_t prev_end_size = prev_exists ? m_src_image_ctx->layout.object_size : 0; librados::snap_t start_src_snap_id = 0; @@ -589,12 +599,10 @@ void ObjectCopyRequest::compute_read_ops() { exists = true; end_size = m_src_image_ctx->layout.object_size; clone_end_snap_id = end_src_snap_id; - } - - if (!exists) { + } else if (!exists) { end_size = 0; - if (end_src_snap_id == m_snap_map.begin()->first && - m_src_parent_image_ctx != nullptr && m_snap_set.clones.empty()) { + if (hide_parent && end_src_snap_id == m_snap_map.begin()->first && + m_snap_set.clones.empty()) { ldout(m_cct, 20) << "no clones for existing object" << dendl; exists = true; diff.insert(0, m_src_image_ctx->layout.object_size); @@ -686,8 +694,7 @@ void ObjectCopyRequest::compute_read_ops() { prev_end_size = end_size; prev_exists = exists; - if (prev_exists && prev_end_size == 0 && - m_src_parent_image_ctx != nullptr) { + if (hide_parent && prev_exists && prev_end_size == 0) { // hide parent prev_end_size = m_src_image_ctx->layout.object_size; } @@ -702,11 +709,14 @@ void ObjectCopyRequest::compute_read_ops() { template void ObjectCopyRequest::compute_read_from_parent_ops( io::Extents *parent_image_extents) { + assert(m_src_image_ctx->snap_lock.is_locked()); + assert(m_src_image_ctx->parent_lock.is_locked()); + m_read_ops = {}; m_zero_interval = {}; parent_image_extents->clear(); - if (m_src_parent_image_ctx == nullptr) { + if (m_src_image_ctx->parent == nullptr) { ldout(m_cct, 20) << "no parent" << dendl; return; } @@ -733,9 +743,6 @@ void ObjectCopyRequest::compute_read_from_parent_ops( auto src_snap_seq = m_snap_map.begin()->first; - RWLock::RLocker snap_locker(m_src_image_ctx->snap_lock); - RWLock::RLocker parent_locker(m_src_image_ctx->parent_lock); - uint64_t parent_overlap; int r = m_src_image_ctx->get_parent_overlap(src_snap_seq, &parent_overlap); if (r < 0) { @@ -835,7 +842,10 @@ void ObjectCopyRequest::compute_zero_ops() { bool fast_diff = m_dst_image_ctx->test_features(RBD_FEATURE_FAST_DIFF); uint64_t prev_end_size = 0; - bool hide_parent = m_src_parent_image_ctx != nullptr; + + m_src_image_ctx->parent_lock.get_read(); + bool hide_parent = (m_src_image_ctx->parent != nullptr); + m_src_image_ctx->parent_lock.put_read(); for (auto &it : m_dst_zero_interval) { auto src_snap_seq = it.first; diff --git a/src/librbd/deep_copy/ObjectCopyRequest.h b/src/librbd/deep_copy/ObjectCopyRequest.h index e514d539876af..4f53826117b76 100644 --- a/src/librbd/deep_copy/ObjectCopyRequest.h +++ b/src/librbd/deep_copy/ObjectCopyRequest.h @@ -25,19 +25,17 @@ template class ObjectCopyRequest { public: static ObjectCopyRequest* create(ImageCtxT *src_image_ctx, - ImageCtxT *src_parent_image_ctx, ImageCtxT *dst_image_ctx, const SnapMap &snap_map, uint64_t object_number, bool flatten, Context *on_finish) { - return new ObjectCopyRequest(src_image_ctx, src_parent_image_ctx, - dst_image_ctx, snap_map, object_number, - flatten, on_finish); + return new ObjectCopyRequest(src_image_ctx, dst_image_ctx, snap_map, + object_number, flatten, on_finish); } - ObjectCopyRequest(ImageCtxT *src_image_ctx, ImageCtxT *src_parent_image_ctx, - ImageCtxT *dst_image_ctx, const SnapMap &snap_map, - uint64_t object_number, bool flatten, Context *on_finish); + ObjectCopyRequest(ImageCtxT *src_image_ctx, ImageCtxT *dst_image_ctx, + const SnapMap &snap_map, uint64_t object_number, + bool flatten, Context *on_finish); void send(); @@ -133,7 +131,6 @@ private: typedef std::map> SnapObjectSizes; ImageCtxT *m_src_image_ctx; - ImageCtxT *m_src_parent_image_ctx; ImageCtxT *m_dst_image_ctx; CephContext *m_cct; const SnapMap &m_snap_map; diff --git a/src/librbd/image/CloseRequest.cc b/src/librbd/image/CloseRequest.cc index 57a1eb7d97c37..d114c401862b2 100644 --- a/src/librbd/image/CloseRequest.cc +++ b/src/librbd/image/CloseRequest.cc @@ -266,36 +266,6 @@ template void CloseRequest::handle_flush_op_work_queue(int r) { CephContext *cct = m_image_ctx->cct; ldout(cct, 10) << this << " " << __func__ << ": r=" << r << dendl; - send_close_migration_parent(); -} - -template -void CloseRequest::send_close_migration_parent() { - if (m_image_ctx->migration_parent == nullptr) { - send_close_parent(); - return; - } - - CephContext *cct = m_image_ctx->cct; - ldout(cct, 10) << this << " " << __func__ << dendl; - - m_image_ctx->migration_parent->state->close(create_async_context_callback( - *m_image_ctx, create_context_callback< - CloseRequest, &CloseRequest::handle_close_migration_parent>(this))); -} - -template -void CloseRequest::handle_close_migration_parent(int r) { - CephContext *cct = m_image_ctx->cct; - ldout(cct, 10) << this << " " << __func__ << ": r=" << r << dendl; - - delete m_image_ctx->migration_parent; - m_image_ctx->migration_parent = nullptr; - save_result(r); - if (r < 0) { - lderr(cct) << "error closing migration parent image: " << cpp_strerror(r) - << dendl; - } send_close_parent(); } diff --git a/src/librbd/image/CloseRequest.h b/src/librbd/image/CloseRequest.h index 44916f781f13e..fd101beedb331 100644 --- a/src/librbd/image/CloseRequest.h +++ b/src/librbd/image/CloseRequest.h @@ -55,9 +55,6 @@ private: * v * FLUSH_OP_WORK_QUEUE * | - * v (skip if no migration parent) - * CLOSE_MIGRATION_PARENT - * | * v (skip if no parent) * CLOSE_PARENT * | @@ -106,9 +103,6 @@ private: void send_flush_op_work_queue(); void handle_flush_op_work_queue(int r); - void send_close_migration_parent(); - void handle_close_migration_parent(int r); - void send_close_parent(); void handle_close_parent(int r); diff --git a/src/librbd/image/RefreshParentRequest.cc b/src/librbd/image/RefreshParentRequest.cc index 233b189c486c0..925d3d8c95bda 100644 --- a/src/librbd/image/RefreshParentRequest.cc +++ b/src/librbd/image/RefreshParentRequest.cc @@ -65,8 +65,15 @@ template bool RefreshParentRequest::does_parent_exist( I &child_image_ctx, const ParentInfo &parent_md, const MigrationInfo &migration_info) { + if (child_image_ctx.child != nullptr && + child_image_ctx.child->migration_info.empty() && parent_md.overlap == 0) { + // intermediate, non-migrating images should only open their parent if they + // overlap + return false; + } + return (parent_md.spec.pool_id > -1 && parent_md.overlap > 0) || - !migration_info.empty(); + !migration_info.empty(); } template @@ -84,7 +91,6 @@ void RefreshParentRequest::apply() { ceph_assert(m_child_image_ctx.snap_lock.is_wlocked()); ceph_assert(m_child_image_ctx.parent_lock.is_wlocked()); std::swap(m_child_image_ctx.parent, m_parent_image_ctx); - std::swap(m_child_image_ctx.migration_parent, m_migration_parent_image_ctx); } template @@ -94,7 +100,7 @@ void RefreshParentRequest::finalize(Context *on_finish) { m_on_finish = on_finish; if (m_parent_image_ctx != nullptr) { - send_close_migration_parent(); + send_close_parent(); } else { send_complete(0); } @@ -159,124 +165,11 @@ Context *RefreshParentRequest::handle_open_parent(int *result) { // image already closed by open state machine delete m_parent_image_ctx; m_parent_image_ctx = nullptr; - return m_on_finish; - } - - if (m_migration_info.empty()) { - return m_on_finish; - } - - send_open_migration_parent(); - return nullptr; -} - -template -void RefreshParentRequest::send_open_migration_parent() { - ceph_assert(m_parent_image_ctx != nullptr); - ceph_assert(!m_migration_info.empty()); - - CephContext *cct = m_child_image_ctx.cct; - ParentSpec parent_spec; - { - RWLock::RLocker snap_locker(m_parent_image_ctx->snap_lock); - RWLock::RLocker parent_locker(m_parent_image_ctx->parent_lock); - - auto snap_id = m_migration_info.snap_map.begin()->first; - auto parent_info = m_parent_image_ctx->get_parent_info(snap_id); - if (parent_info == nullptr) { - lderr(cct) << "could not find parent info for snap id " << snap_id - << dendl; - } else { - parent_spec = parent_info->spec; - } - } - - if (parent_spec.pool_id == -1) { - send_complete(0); - return; - } - - ldout(cct, 10) << this << " " << __func__ << dendl; - - librados::Rados rados(m_child_image_ctx.md_ctx); - - librados::IoCtx parent_io_ctx; - int r = rados.ioctx_create2(parent_spec.pool_id, parent_io_ctx); - if (r < 0) { - lderr(cct) << "failed to access parent pool (id=" << parent_spec.pool_id - << "): " << cpp_strerror(r) << dendl; - save_result(&r); - send_close_parent(); - return; - } - - m_migration_parent_image_ctx = new I("", parent_spec.image_id, - parent_spec.snap_id, parent_io_ctx, - true); - - using klass = RefreshParentRequest; - Context *ctx = create_async_context_callback( - m_child_image_ctx, create_context_callback< - klass, &klass::handle_open_migration_parent, false>(this)); - OpenRequest *req = OpenRequest::create(m_migration_parent_image_ctx, 0, - ctx); - req->send(); -} - -template -Context *RefreshParentRequest::handle_open_migration_parent(int *result) { - CephContext *cct = m_child_image_ctx.cct; - ldout(cct, 10) << this << " " << __func__ << " r=" << *result << dendl; - - save_result(result); - if (*result < 0) { - lderr(cct) << "failed to open migration parent image: " - << cpp_strerror(*result) << dendl; - - // image already closed by open state machine - delete m_migration_parent_image_ctx; - m_migration_parent_image_ctx = nullptr; } return m_on_finish; } -template -void RefreshParentRequest::send_close_migration_parent() { - if (m_migration_parent_image_ctx == nullptr) { - send_close_parent(); - return; - } - - CephContext *cct = m_child_image_ctx.cct; - ldout(cct, 10) << this << " " << __func__ << dendl; - - using klass = RefreshParentRequest; - Context *ctx = create_async_context_callback( - m_child_image_ctx, create_context_callback< - klass, &klass::handle_close_migration_parent, false>(this)); - CloseRequest *req = CloseRequest::create(m_migration_parent_image_ctx, - ctx); - req->send(); -} - -template -Context *RefreshParentRequest::handle_close_migration_parent(int *result) { - CephContext *cct = m_child_image_ctx.cct; - ldout(cct, 10) << this << " " << __func__ << " r=" << *result << dendl; - - delete m_migration_parent_image_ctx; - m_migration_parent_image_ctx = nullptr; - - if (*result < 0) { - lderr(cct) << "failed to close migration parent image: " - << cpp_strerror(*result) << dendl; - } - - send_close_parent(); - return nullptr; -} - template void RefreshParentRequest::send_close_parent() { ceph_assert(m_parent_image_ctx != nullptr); diff --git a/src/librbd/image/RefreshParentRequest.h b/src/librbd/image/RefreshParentRequest.h index 2211da01373b6..39abc1b789c7d 100644 --- a/src/librbd/image/RefreshParentRequest.h +++ b/src/librbd/image/RefreshParentRequest.h @@ -41,25 +41,19 @@ private: * * | * | (open required) - * |----------------> OPEN_PARENT * * * * * * * * * * * * * * * * * * * * * - * | | * - * | v * - * | OPEN_MIGRATION_PARENT * * * * * * * * * * * * * * * - * | | (skip if not * * - * | v needed) * * - * \----------------> * * - * | (skip if not * * - * | (close required) needed) * * - * |-----------------> CLOSE_MIGRATION_PARENT * * - * | | * * - * | v * * - * | CLOSE_PARENT <* * * * * * * - * | | (on error) * - * | v * - * | RESET_EXISTENCE * - * | | * - * | v * - * \-----------------> < * * * * * * * * * * + * |----------------> OPEN_PARENT * * * * * * * * * * * * * * * + * | | * + * | v (on error) * + * \----------------> * + * | * + * | (close required) * + * |-----------------> CLOSE_PARENT * + * | | * + * | v * + * | RESET_EXISTENCE * + * | | * + * | v * + * \-----------------> < * * * * * * @endverbatim */ @@ -73,7 +67,6 @@ private: Context *m_on_finish; ImageCtxT *m_parent_image_ctx; - ImageCtxT *m_migration_parent_image_ctx = nullptr; uint64_t m_parent_snap_id; int m_error_result; @@ -91,12 +84,6 @@ private: void send_open_parent(); Context *handle_open_parent(int *result); - void send_open_migration_parent(); - Context *handle_open_migration_parent(int *result); - - void send_close_migration_parent(); - Context *handle_close_migration_parent(int *result); - void send_close_parent(); Context *handle_close_parent(int *result); diff --git a/src/librbd/io/CopyupRequest.cc b/src/librbd/io/CopyupRequest.cc index 09be635016d46..c3026b1e4dd1a 100644 --- a/src/librbd/io/CopyupRequest.cc +++ b/src/librbd/io/CopyupRequest.cc @@ -238,9 +238,8 @@ void CopyupRequest::send() if (is_deep_copy()) { m_flatten = is_copyup_required() ? true : m_ictx->migration_info.flatten; auto req = deep_copy::ObjectCopyRequest::create( - m_ictx->parent, m_ictx->migration_parent, m_ictx, - m_ictx->migration_info.snap_map, m_object_no, m_flatten, - util::create_context_callback(this)); + m_ictx->parent, m_ictx, m_ictx->migration_info.snap_map, m_object_no, + m_flatten, util::create_context_callback(this)); ldout(m_ictx->cct, 20) << "deep copy object req " << req << ", object_no " << m_object_no << ", flatten " << m_flatten @@ -280,8 +279,7 @@ bool CopyupRequest::should_complete(int *r) { case STATE_READ_FROM_PARENT: ldout(cct, 20) << "READ_FROM_PARENT" << dendl; m_ictx->copyup_list_lock.Lock(); - if (*r == -ENOENT && is_deep_copy() && m_ictx->migration_parent && - !m_flatten && is_copyup_required()) { + if (*r == -ENOENT && is_deep_copy() && !m_flatten && is_copyup_required()) { ldout(cct, 5) << "restart deep copy with flatten" << dendl; m_ictx->copyup_list_lock.Unlock(); send(); diff --git a/src/librbd/operation/MigrateRequest.cc b/src/librbd/operation/MigrateRequest.cc index 5cf3388790005..7bcff805a5e13 100644 --- a/src/librbd/operation/MigrateRequest.cc +++ b/src/librbd/operation/MigrateRequest.cc @@ -131,9 +131,8 @@ private: ceph_assert(image_ctx.parent != nullptr); auto req = deep_copy::ObjectCopyRequest::create( - image_ctx.parent, image_ctx.migration_parent, &image_ctx, - image_ctx.migration_info.snap_map, m_object_no, - image_ctx.migration_info.flatten, ctx); + image_ctx.parent, &image_ctx, image_ctx.migration_info.snap_map, + m_object_no, image_ctx.migration_info.flatten, ctx); ldout(cct, 20) << "deep copy object req " << req << ", object_no " << m_object_no << dendl; diff --git a/src/test/librbd/deep_copy/test_mock_ImageCopyRequest.cc b/src/test/librbd/deep_copy/test_mock_ImageCopyRequest.cc index 06750906bbbb5..a3e5effd9989e 100644 --- a/src/test/librbd/deep_copy/test_mock_ImageCopyRequest.cc +++ b/src/test/librbd/deep_copy/test_mock_ImageCopyRequest.cc @@ -49,7 +49,6 @@ struct ObjectCopyRequest { static ObjectCopyRequest* s_instance; static ObjectCopyRequest* create( librbd::MockTestImageCtx *src_image_ctx, - librbd::MockTestImageCtx *src_parent_image_ctx, librbd::MockTestImageCtx *dst_image_ctx, const SnapMap &snap_map, uint64_t object_number, bool flatten, Context *on_finish) { ceph_assert(s_instance != nullptr); @@ -165,10 +164,6 @@ public: .WillOnce(Return(size)).RetiresOnSaturation(); } - void expect_get_parent_info(librbd::MockTestImageCtx &mock_image_ctx) { - EXPECT_CALL(mock_image_ctx, get_parent_info(_)).WillOnce(Return(nullptr)); - } - void expect_object_copy_send(MockObjectCopyRequest &mock_object_copy_request) { EXPECT_CALL(mock_object_copy_request, send()); } @@ -268,7 +263,6 @@ TEST_F(TestMockDeepCopyImageCopyRequest, SimpleImage) { MockObjectCopyRequest mock_object_copy_request; InSequence seq; - expect_get_parent_info(mock_src_image_ctx); expect_get_image_size(mock_src_image_ctx, 1 << m_src_image_ctx->order); expect_get_image_size(mock_src_image_ctx, 0); expect_object_copy_send(mock_object_copy_request); @@ -304,7 +298,6 @@ TEST_F(TestMockDeepCopyImageCopyRequest, OutOfOrder) { librbd::MockTestImageCtx mock_dst_image_ctx(*m_dst_image_ctx); MockObjectCopyRequest mock_object_copy_request; - expect_get_parent_info(mock_src_image_ctx); expect_get_image_size(mock_src_image_ctx, object_count * (1 << m_src_image_ctx->order)); expect_get_image_size(mock_src_image_ctx, 0); @@ -372,7 +365,6 @@ TEST_F(TestMockDeepCopyImageCopyRequest, SnapshotSubset) { MockObjectCopyRequest mock_object_copy_request; InSequence seq; - expect_get_parent_info(mock_src_image_ctx); expect_get_image_size(mock_src_image_ctx, 1 << m_src_image_ctx->order); expect_get_image_size(mock_src_image_ctx, 0); expect_get_image_size(mock_src_image_ctx, 0); @@ -405,7 +397,6 @@ TEST_F(TestMockDeepCopyImageCopyRequest, RestartPartialSync) { MockObjectCopyRequest mock_object_copy_request; InSequence seq; - expect_get_parent_info(mock_src_image_ctx); expect_get_image_size(mock_src_image_ctx, 2 * (1 << m_src_image_ctx->order)); expect_get_image_size(mock_src_image_ctx, 0); expect_object_copy_send(mock_object_copy_request); @@ -440,7 +431,6 @@ TEST_F(TestMockDeepCopyImageCopyRequest, Cancel) { MockObjectCopyRequest mock_object_copy_request; InSequence seq; - expect_get_parent_info(mock_src_image_ctx); expect_get_image_size(mock_src_image_ctx, 1 << m_src_image_ctx->order); expect_get_image_size(mock_src_image_ctx, 0); expect_object_copy_send(mock_object_copy_request); @@ -477,7 +467,6 @@ TEST_F(TestMockDeepCopyImageCopyRequest, Cancel_Inflight_Sync) { MockObjectCopyRequest mock_object_copy_request; InSequence seq; - expect_get_parent_info(mock_src_image_ctx); expect_get_image_size(mock_src_image_ctx, 6 * (1 << m_src_image_ctx->order)); expect_get_image_size(mock_src_image_ctx, m_image_size); diff --git a/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc b/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc index 20a8b9c6c72f7..bc0f4a6b17bd4 100644 --- a/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc +++ b/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc @@ -25,6 +25,8 @@ struct MockTestImageCtx : public librbd::MockImageCtx { explicit MockTestImageCtx(librbd::ImageCtx &image_ctx) : librbd::MockImageCtx(image_ctx) { } + + MockTestImageCtx *parent = nullptr; }; } // anonymous namespace @@ -206,9 +208,8 @@ public: librbd::MockTestImageCtx &mock_dst_image_ctx, Context *on_finish) { expect_get_object_name(mock_dst_image_ctx); expect_get_object_count(mock_dst_image_ctx); - return new MockObjectCopyRequest(&mock_src_image_ctx, nullptr, - &mock_dst_image_ctx, m_snap_map, 0, false, - on_finish); + return new MockObjectCopyRequest(&mock_src_image_ctx, &mock_dst_image_ctx, + m_snap_map, 0, false, on_finish); } void expect_set_snap_read(librados::MockTestMemIoCtxImpl &mock_io_ctx, -- 2.39.5