From 82b296e3495799ce45421d722bb60e05d0dc0145 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Tue, 21 Aug 2018 21:41:52 -0400 Subject: [PATCH] librbd: refactor flatten state machine to new code style Signed-off-by: Jason Dillaman --- src/librbd/operation/FlattenRequest.cc | 173 +++++++++++++++---------- src/librbd/operation/FlattenRequest.h | 48 +++---- 2 files changed, 127 insertions(+), 94 deletions(-) diff --git a/src/librbd/operation/FlattenRequest.cc b/src/librbd/operation/FlattenRequest.cc index 2f3225c64ae..0bfdfcd09fa 100644 --- a/src/librbd/operation/FlattenRequest.cc +++ b/src/librbd/operation/FlattenRequest.cc @@ -15,11 +15,15 @@ #define dout_subsys ceph_subsys_rbd #undef dout_prefix -#define dout_prefix *_dout << "librbd::FlattenRequest: " +#define dout_prefix *_dout << "librbd::operation::FlattenRequest: " << this \ + << " " << __func__ << ": " namespace librbd { namespace operation { +using util::create_context_callback; +using util::create_rados_callback; + template class C_FlattenObject : public C_AsyncObjectThrottle { public: @@ -74,117 +78,152 @@ template bool FlattenRequest::should_complete(int r) { I &image_ctx = this->m_image_ctx; CephContext *cct = image_ctx.cct; - ldout(cct, 5) << this << " should_complete: " << " r=" << r << dendl; - if (r == -ERESTART) { - ldout(cct, 5) << "flatten operation interrupted" << dendl; - return true; - } else if (r < 0 && r != -ENOENT) { - lderr(cct) << "flatten encountered an error: " << cpp_strerror(r) << dendl; - return true; + ldout(cct, 5) << "r=" << r << dendl; + if (r < 0) { + lderr(cct) << "encountered error: " << cpp_strerror(r) << dendl; } - - RWLock::RLocker owner_locker(image_ctx.owner_lock); - switch (m_state) { - case STATE_FLATTEN_OBJECTS: - ldout(cct, 5) << "FLATTEN_OBJECTS" << dendl; - return send_detach_child(); - - case STATE_DETACH_CHILD: - ldout(cct, 5) << "DETACH_CHILD" << dendl; - return send_update_header(); - - case STATE_UPDATE_HEADER: - ldout(cct, 5) << "UPDATE_HEADER" << dendl; - return true; - - default: - lderr(cct) << "invalid state: " << m_state << dendl; - ceph_abort(); - break; - } - return false; + return true; } template void FlattenRequest::send_op() { + flatten_objects(); +} + +template +void FlattenRequest::flatten_objects() { I &image_ctx = this->m_image_ctx; ceph_assert(image_ctx.owner_lock.is_locked()); - CephContext *cct = image_ctx.cct; - ldout(cct, 5) << this << " send" << dendl; - m_state = STATE_FLATTEN_OBJECTS; + CephContext *cct = image_ctx.cct; + ldout(cct, 5) << dendl; + + assert(image_ctx.owner_lock.is_locked()); + auto ctx = create_context_callback< + FlattenRequest, + &FlattenRequest::handle_flatten_objects>(this); typename AsyncObjectThrottle::ContextFactory context_factory( boost::lambda::bind(boost::lambda::new_ptr >(), boost::lambda::_1, &image_ctx, m_snapc, boost::lambda::_2)); AsyncObjectThrottle *throttle = new AsyncObjectThrottle( - this, image_ctx, context_factory, this->create_callback_context(), &m_prog_ctx, - 0, m_overlap_objects); + this, image_ctx, context_factory, ctx, &m_prog_ctx, 0, m_overlap_objects); throttle->start_ops(image_ctx.concurrent_management_ops); } template -bool FlattenRequest::send_detach_child() { +void FlattenRequest::handle_flatten_objects(int r) { + I &image_ctx = this->m_image_ctx; + CephContext *cct = image_ctx.cct; + ldout(cct, 5) << "r=" << r << dendl; + + if (r == -ERESTART) { + ldout(cct, 5) << "flatten operation interrupted" << dendl; + this->complete(r); + return; + } else if (r < 0) { + lderr(cct) << "flatten encountered an error: " << cpp_strerror(r) << dendl; + this->complete(r); + return; + } + + detach_child(); +} + +template +void FlattenRequest::detach_child() { I &image_ctx = this->m_image_ctx; - ceph_assert(image_ctx.owner_lock.is_locked()); CephContext *cct = image_ctx.cct; // should have been canceled prior to releasing lock + image_ctx.owner_lock.get_read(); ceph_assert(image_ctx.exclusive_lock == nullptr || - image_ctx.exclusive_lock->is_lock_owner()); + image_ctx.exclusive_lock->is_lock_owner()); // if there are no snaps, remove from the children object as well // (if snapshots remain, they have their own parent info, and the child // will be removed when the last snap goes away) - { - RWLock::RLocker snap_locker(image_ctx.snap_lock); - if ((image_ctx.features & RBD_FEATURE_DEEP_FLATTEN) == 0 && - !image_ctx.snaps.empty()) { - return send_update_header(); - } + image_ctx.snap_lock.get_read(); + if ((image_ctx.features & RBD_FEATURE_DEEP_FLATTEN) == 0 && + !image_ctx.snaps.empty()) { + image_ctx.snap_lock.put_read(); + image_ctx.owner_lock.put_read(); + remove_parent(); + return; } + image_ctx.snap_lock.put_read(); - ldout(cct, 2) << "detaching child" << dendl; - m_state = STATE_DETACH_CHILD; - - auto req = image::DetachChildRequest::create( - image_ctx, this->create_callback_context()); + ldout(cct, 5) << dendl; + auto ctx = create_context_callback< + FlattenRequest, + &FlattenRequest::handle_detach_child>(this); + auto req = image::DetachChildRequest::create(image_ctx, ctx); req->send(); - return false; + image_ctx.owner_lock.put_read(); } template -bool FlattenRequest::send_update_header() { +void FlattenRequest::handle_detach_child(int r) { I &image_ctx = this->m_image_ctx; - ceph_assert(image_ctx.owner_lock.is_locked()); CephContext *cct = image_ctx.cct; + ldout(cct, 5) << "r=" << r << dendl; - ldout(cct, 5) << this << " send_update_header" << dendl; - m_state = STATE_UPDATE_HEADER; + if (r < 0 && r != -ENOENT) { + lderr(cct) << "detach encountered an error: " << cpp_strerror(r) << dendl; + this->complete(r); + return; + } + + remove_parent(); +} + +template +void FlattenRequest::remove_parent() { + I &image_ctx = this->m_image_ctx; + CephContext *cct = image_ctx.cct; + ldout(cct, 5) << dendl; // should have been canceled prior to releasing lock + image_ctx.owner_lock.get_read(); ceph_assert(image_ctx.exclusive_lock == nullptr || - image_ctx.exclusive_lock->is_lock_owner()); + image_ctx.exclusive_lock->is_lock_owner()); - { - RWLock::RLocker parent_locker(image_ctx.parent_lock); - // stop early if the parent went away - it just means - // another flatten finished first, so this one is useless. - if (image_ctx.parent_md.spec.pool_id == -1) { - ldout(cct, 5) << "image already flattened" << dendl; - return true; - } + // stop early if the parent went away - it just means + // another flatten finished first, so this one is useless. + image_ctx.parent_lock.get_read(); + if (!image_ctx.parent) { + ldout(cct, 5) << "image already flattened" << dendl; + image_ctx.parent_lock.put_read(); + image_ctx.owner_lock.put_read(); + this->complete(0); + return; } + image_ctx.parent_lock.put_read(); // remove parent from this (base) image librados::ObjectWriteOperation op; cls_client::remove_parent(&op); - librados::AioCompletion *rados_completion = this->create_callback_completion(); - int r = image_ctx.md_ctx.aio_operate(image_ctx.header_oid, - rados_completion, &op); + auto aio_comp = create_rados_callback< + FlattenRequest, + &FlattenRequest::handle_remove_parent>(this); + int r = image_ctx.md_ctx.aio_operate(image_ctx.header_oid, aio_comp, &op); ceph_assert(r == 0); - rados_completion->release(); - return false; + aio_comp->release(); + image_ctx.owner_lock.put_read(); +} + +template +void FlattenRequest::handle_remove_parent(int r) { + I &image_ctx = this->m_image_ctx; + CephContext *cct = image_ctx.cct; + ldout(cct, 5) << "r=" << r << dendl; + + if (r < 0 && r != -ENOENT) { + lderr(cct) << "remove parent encountered an error: " << cpp_strerror(r) + << dendl; + } + + this->complete(r); } } // namespace operation diff --git a/src/librbd/operation/FlattenRequest.h b/src/librbd/operation/FlattenRequest.h index 59494277d95..171ebae0075 100644 --- a/src/librbd/operation/FlattenRequest.h +++ b/src/librbd/operation/FlattenRequest.h @@ -20,7 +20,8 @@ public: FlattenRequest(ImageCtxT &image_ctx, Context *on_finish, uint64_t overlap_objects, const ::SnapContext &snapc, ProgressContext &prog_ctx) - : Request(image_ctx, on_finish), m_overlap_objects(overlap_objects), + : Request(image_ctx, on_finish), + m_overlap_objects(overlap_objects), m_snapc(snapc), m_prog_ctx(prog_ctx) { } @@ -34,45 +35,38 @@ protected: private: /** - * Flatten goes through the following state machine to copyup objects - * from the parent image: - * * @verbatim * * * | * v - * STATE_FLATTEN_OBJECTS ---> STATE_DETACH_CHILD . . . . . - * . | . - * . | . - * . v . - * . STATE_UPDATE_HEADER . - * . | . - * . | . - * . \---> < . . - * . ^ - * . . - * . . . . . . . . . . . . . . . . . . . + * FLATTEN_OBJECTS + * | + * v + * DETACH_CHILD + * | + * v + * REMOVE_PARENT + * | + * v + * * * @endverbatim - * - * The _DETACH_CHILD state will be skipped if the image has one or - * more snapshots. The _UPDATE_HEADER state will be skipped if the - * image was concurrently flattened by another client. */ - enum State { - STATE_FLATTEN_OBJECTS, - STATE_DETACH_CHILD, - STATE_UPDATE_HEADER - }; uint64_t m_overlap_objects; ::SnapContext m_snapc; ProgressContext &m_prog_ctx; - State m_state = STATE_FLATTEN_OBJECTS; - bool send_detach_child(); - bool send_update_header(); + void flatten_objects(); + void handle_flatten_objects(int r); + + void detach_child(); + void handle_detach_child(int r); + + void remove_parent(); + void handle_remove_parent(int r); + }; } // namespace operation