From 5fb1277eeca6314ffc2e864ac1f24badf58f4b4d Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Tue, 28 Feb 2017 11:05:51 -0500 Subject: [PATCH] librbd: eliminate context switch from remove state machine Also cleaned up formatting issues within the state machine. Signed-off-by: Jason Dillaman --- src/librbd/image/RemoveRequest.cc | 144 ++++++++++++++---------------- src/librbd/image/RemoveRequest.h | 71 ++++++++------- 2 files changed, 109 insertions(+), 106 deletions(-) diff --git a/src/librbd/image/RemoveRequest.cc b/src/librbd/image/RemoveRequest.cc index 24bc22beefe..f835b180a9b 100644 --- a/src/librbd/image/RemoveRequest.cc +++ b/src/librbd/image/RemoveRequest.cc @@ -27,14 +27,14 @@ using util::create_context_callback; using util::create_async_context_callback; using util::create_rados_ack_callback; -static const std::string IMAGE_CLIENT_ID(""); - template -RemoveRequest::RemoveRequest(IoCtx &ioctx, const std::string &image_name, const std::string &image_id, - bool force, ProgressContext &prog_ctx, ContextWQ *op_work_queue, - Context *on_finish) : - m_ioctx(ioctx), m_image_name(image_name), m_image_id(image_id), m_force(force), - m_prog_ctx(prog_ctx), m_op_work_queue(op_work_queue), m_on_finish(on_finish) { +RemoveRequest::RemoveRequest(IoCtx &ioctx, const std::string &image_name, + const std::string &image_id, bool force, + ProgressContext &prog_ctx, + ContextWQ *op_work_queue, Context *on_finish) + : m_ioctx(ioctx), m_image_name(image_name), m_image_id(image_id), + m_force(force), m_prog_ctx(prog_ctx), m_op_work_queue(op_work_queue), + m_on_finish(on_finish) { m_cct = reinterpret_cast(m_ioctx.cct()); m_image_ctx = I::create((m_image_id.empty() ? m_image_name : std::string()), @@ -53,7 +53,8 @@ void RemoveRequest::open_image() { ldout(m_cct, 20) << dendl; using klass = RemoveRequest; - Context *ctx = create_context_callback(this); + Context *ctx = create_context_callback( + this); m_image_ctx->state->open(true, ctx); } @@ -63,11 +64,15 @@ Context *RemoveRequest::handle_open_image(int *result) { ldout(m_cct, 20) << ": r=" << *result << dendl; if (*result < 0) { + m_image_ctx->destroy(); + m_image_ctx = nullptr; + if (*result != -ENOENT) { lderr(m_cct) << "error opening image: " << cpp_strerror(*result) << dendl; + return m_on_finish; } - m_ret_val = *result; - switch_thread_context(); + + remove_image(); return nullptr; } @@ -98,10 +103,12 @@ void RemoveRequest::acquire_exclusive_lock() { using klass = RemoveRequest; if (m_force) { - Context *ctx = create_context_callback(this); + Context *ctx = create_context_callback< + klass, &klass::handle_exclusive_lock_force>(this); m_image_ctx->exclusive_lock->shut_down(ctx); } else { - Context *ctx = create_context_callback(this); + Context *ctx = create_context_callback< + klass, &klass::handle_exclusive_lock>(this); RWLock::WLocker owner_lock(m_image_ctx->owner_lock); m_image_ctx->exclusive_lock->try_acquire_lock(ctx); } @@ -112,8 +119,8 @@ Context *RemoveRequest::handle_exclusive_lock_force(int *result) { ldout(m_cct, 20) << ": r=" << *result << dendl; if (*result < 0) { - lderr(m_cct) << "error shutting down exclusive lock" << cpp_strerror(*result) - << dendl; + lderr(m_cct) << "error shutting down exclusive lock: " + << cpp_strerror(*result) << dendl; send_close_image(*result); return nullptr; } @@ -168,8 +175,8 @@ void RemoveRequest::check_image_watchers() { librados::AioCompletion *rados_completion = create_rados_ack_callback(this); - int r = m_image_ctx->md_ctx.aio_operate(m_header_oid, - rados_completion, &op, &m_out_bl); + int r = m_image_ctx->md_ctx.aio_operate(m_header_oid, rados_completion, + &op, &m_out_bl); assert(r == 0); rados_completion->release(); } @@ -185,7 +192,8 @@ void RemoveRequest::filter_out_mirror_watchers() { } cls::rbd::MirrorImage mirror_image; - int r = cls_client::mirror_image_get(&m_image_ctx->md_ctx, m_image_ctx->id, &mirror_image); + int r = cls_client::mirror_image_get(&m_image_ctx->md_ctx, m_image_ctx->id, + &mirror_image); if (r < 0) { if (r != -ENOENT) { lderr(m_cct) << "failed to retrieve mirroring state: " @@ -219,10 +227,12 @@ Context *RemoveRequest::handle_check_image_watchers(int *result) { ldout(m_cct, 20) << ": r=" << *result << dendl; if (*result < 0) { - lderr(m_cct) << "error listing image watchers: " << cpp_strerror(*result) << dendl; + lderr(m_cct) << "error listing image watchers: " << cpp_strerror(*result) + << dendl; send_close_image(*result); return nullptr; } + // If an image is being bootstrapped by rbd-mirror, it implies // that the rbd-mirror daemon currently has the image open. // Permit removal if this is the case. @@ -246,11 +256,11 @@ void RemoveRequest::check_image_consistency_group() { librbd::cls_client::image_get_group_start(&op); using klass = RemoveRequest; - librados::AioCompletion *rados_completion = - create_rados_ack_callback(this); + librados::AioCompletion *rados_completion = create_rados_ack_callback< + klass, &klass::handle_check_image_consistency_group>(this); m_out_bl.clear(); - int r = m_image_ctx->md_ctx.aio_operate(m_header_oid, - rados_completion, &op, &m_out_bl); + int r = m_image_ctx->md_ctx.aio_operate(m_header_oid, rados_completion, &op, + &m_out_bl); assert(r == 0); rados_completion->release(); } @@ -260,8 +270,8 @@ Context *RemoveRequest::handle_check_image_consistency_group(int *result) { ldout(m_cct, 20) << ": r=" << *result << dendl; if (*result < 0) { - lderr(m_cct) << "error fetching consistency group for image" << cpp_strerror(*result) - << dendl; + lderr(m_cct) << "error fetching consistency group for image: " + << cpp_strerror(*result) << dendl; send_close_image(*result); return nullptr; } @@ -274,7 +284,7 @@ Context *RemoveRequest::handle_check_image_consistency_group(int *result) { return nullptr; } if (s.is_valid()) { - lderr(m_cct) << "image is in a consistency group - not removing" << dendl; + lderr(m_cct) << "image is in a group - not removing" << dendl; send_close_image(-EMLINK); return nullptr; } @@ -290,10 +300,11 @@ void RemoveRequest::trim_image() { using klass = RemoveRequest; Context *ctx = create_async_context_callback( - *m_image_ctx, create_context_callback(this)); + *m_image_ctx, create_context_callback< + klass, &klass::handle_trim_image>(this)); RWLock::RLocker owner_lock(m_image_ctx->owner_lock); - librbd::operation::TrimRequest *req = librbd::operation::TrimRequest::create( + auto req = librbd::operation::TrimRequest::create( *m_image_ctx, ctx, m_image_ctx->size, 0, m_prog_ctx); req->send(); } @@ -303,8 +314,8 @@ Context *RemoveRequest::handle_trim_image(int *result) { ldout(m_cct, 20) << ": r=" << *result << dendl; if (*result < 0) { - lderr(m_cct) << "warning: failed to remove some object(s): " << cpp_strerror(*result) - << dendl; + lderr(m_cct) << "warning: failed to remove some object(s): " + << cpp_strerror(*result) << dendl; } if (m_old_format) { @@ -342,8 +353,8 @@ Context *RemoveRequest::handle_remove_child(int *result) { if (*result == -ENOENT) { *result = 0; } else if (*result < 0) { - lderr(m_cct) << "error removing child from children list" << cpp_strerror(*result) - << dendl; + lderr(m_cct) << "error removing child from children list: " + << cpp_strerror(*result) << dendl; send_close_image(*result); return nullptr; } @@ -358,7 +369,8 @@ void RemoveRequest::send_disable_mirror() { ldout(m_cct, 20) << dendl; using klass = RemoveRequest; - Context *ctx = create_context_callback(this); + Context *ctx = create_context_callback< + klass, &klass::handle_disable_mirror>(this); mirror::DisableRequest *req = mirror::DisableRequest::create(m_image_ctx, m_force, !m_force, ctx); @@ -369,12 +381,11 @@ template Context *RemoveRequest::handle_disable_mirror(int *result) { ldout(m_cct, 20) << ": r=" << *result << dendl; - if (*result == -EOPNOTSUPP) { *result = 0; } else if (*result < 0) { - lderr(m_cct) << "error disabling image mirroring: " << cpp_strerror(*result) - << dendl; + lderr(m_cct) << "error disabling image mirroring: " + << cpp_strerror(*result) << dendl; } send_close_image(*result); @@ -387,7 +398,8 @@ void RemoveRequest::send_close_image(int r) { m_ret_val = r; using klass = RemoveRequest; - Context *ctx = create_context_callback(this); + Context *ctx = create_context_callback< + klass, &klass::handle_send_close_image>(this); m_image_ctx->state->close(ctx); } @@ -397,15 +409,18 @@ Context *RemoveRequest::handle_send_close_image(int *result) { ldout(m_cct, 20) << ": r=" << *result << dendl; if (*result < 0) { - lderr(m_cct) << "error encountered while closing image: " << cpp_strerror(*result) - << dendl; + lderr(m_cct) << "error encountered while closing image: " + << cpp_strerror(*result) << dendl; } + m_image_ctx->destroy(); + m_image_ctx = nullptr; if (m_ret_val < 0) { - switch_thread_context(); - } else { - remove_header(); + *result = m_ret_val; + return m_on_finish; } + + remove_header(); return nullptr; } @@ -430,34 +445,8 @@ Context *RemoveRequest::handle_remove_header(int *result) { m_ret_val = *result; } - switch_thread_context(); - return nullptr; -} - -template -void RemoveRequest::switch_thread_context() { - ldout(m_cct, 20) << dendl; - - using klass = RemoveRequest; - - Context *ctx = create_context_callback(this); - m_op_work_queue->queue(ctx, 0); -} - -template -void RemoveRequest::handle_switch_thread_context(int r) { - ldout(m_cct, 20) << ": r=" << r << dendl; - - m_image_ctx->destroy(); - m_image_ctx = nullptr; - - if (m_ret_val < 0 && m_ret_val != -ENOENT) { - m_on_finish->complete(m_ret_val); - return; - } - remove_image(); - return; + return nullptr; } template @@ -494,10 +483,11 @@ void RemoveRequest::send_journal_remove() { ldout(m_cct, 20) << dendl; using klass = RemoveRequest; - Context *ctx = create_context_callback(this); + Context *ctx = create_context_callback< + klass, &klass::handle_journal_remove>(this); journal::RemoveRequest *req = journal::RemoveRequest::create( - m_ioctx, m_image_id, IMAGE_CLIENT_ID, m_op_work_queue, ctx); + m_ioctx, m_image_id, Journal<>::IMAGE_CLIENT_ID, m_op_work_queue, ctx); req->send(); } @@ -568,8 +558,8 @@ Context *RemoveRequest::handle_mirror_image_remove(int *result) { ldout(m_cct, 20) << ": r=" << *result << dendl; if (*result < 0 && *result != -ENOENT && *result != -EOPNOTSUPP) { - lderr(m_cct) << "failed to remove mirror image state: " << cpp_strerror(*result) - << dendl; + lderr(m_cct) << "failed to remove mirror image state: " + << cpp_strerror(*result) << dendl; return m_on_finish; } else { *result = 0; @@ -614,6 +604,7 @@ void RemoveRequest::handle_remove_v1_image(int r) { } m_on_finish->complete(r); + delete this; return; } @@ -659,7 +650,8 @@ Context *RemoveRequest::handle_dir_get_image_id(int *result) { ldout(m_cct, 20) << ": r=" << *result << dendl; if ((*result < 0) && (*result != -ENOENT)) { - lderr(m_cct) << "error fetching image id: " << cpp_strerror(*result) << dendl; + lderr(m_cct) << "error fetching image id: " << cpp_strerror(*result) + << dendl; return m_on_finish; } @@ -696,7 +688,8 @@ Context *RemoveRequest::handle_dir_get_image_name(int *result) { ldout(m_cct, 20) << ": r=" << *result << dendl; if ((*result < 0) && (*result != -ENOENT)) { - lderr(m_cct) << "error fetching image name: " << cpp_strerror(*result) << dendl; + lderr(m_cct) << "error fetching image name: " << cpp_strerror(*result) + << dendl; return m_on_finish; } @@ -729,7 +722,8 @@ Context *RemoveRequest::handle_remove_id_object(int *result) { ldout(m_cct, 20) << ": r=" << *result << dendl; if ((*result < 0) && (*result != -ENOENT)) { - lderr(m_cct) << "error removing id object: " << cpp_strerror(*result) << dendl; + lderr(m_cct) << "error removing id object: " << cpp_strerror(*result) + << dendl; return m_on_finish; } diff --git a/src/librbd/image/RemoveRequest.h b/src/librbd/image/RemoveRequest.h index 10ae7bf2756..6fe50b9a266 100644 --- a/src/librbd/image/RemoveRequest.h +++ b/src/librbd/image/RemoveRequest.h @@ -24,10 +24,14 @@ private: typedef ::librbd::image::TypeTraits TypeTraits; typedef typename TypeTraits::ContextWQ ContextWQ; public: - static RemoveRequest *create(librados::IoCtx &ioctx, const std::string &image_name, const std::string &image_id, - bool force, ProgressContext &prog_ctx, ContextWQ *op_work_queue, + static RemoveRequest *create(librados::IoCtx &ioctx, + const std::string &image_name, + const std::string &image_id, + bool force, ProgressContext &prog_ctx, + ContextWQ *op_work_queue, Context *on_finish) { - return new RemoveRequest(ioctx, image_name, image_id, force, prog_ctx, op_work_queue, on_finish); + return new RemoveRequest(ioctx, image_name, image_id, force, prog_ctx, + op_work_queue, on_finish); } void send(); @@ -38,56 +42,64 @@ private: * * * | - * v - * OPEN IMAGE------------------- - * | | + * v + * OPEN IMAGE------------------\ + * | | * v | - * error CHECK EXCLUSIVE LOCK---- | - * _______<_______ | | | - * | | v (aquired) | - * | | AQUIRE EXCLUSIVE LOCK | | - * | | / | | | - * | |------<-------/ v | | - * | | VALIDATE IMAGE REMOVAL<- | + * error CHECK EXCLUSIVE LOCK---\ | + * /-------<-------\ | | | + * | | | (acquired)| + * | | v | | + * | | AQUIRE EXCLUSIVE LOCK | | + * | | / | | | + * | |------<-------/ | | | + * | | v | | + * | | VALIDATE IMAGE REMOVAL<-/ | * | | / | v - * | |------<--------/ v | + * | \------<--------/ | | + * | v | * | TRIM IMAGE | * | | | * v v | * | REMOVE CHILD | * | | | * | v v - * |--------->------------------>CLOSE IMAGE | + * \--------->------------------>CLOSE IMAGE | * | | * error v | - * |------<--------| SWITCH THREAD CONTEXT<----------- - * | | / | - * | |-------<-------/ v - * | | REMOVE HEADER + * /------<--------\ REMOVE HEADER<--------------/ * | | / | - * | |-------<-------/ v + * | |-------<-------/ | + * | | v * | | REMOVE JOURNAL * | | / | - * | |-------<-------/ v + * | |-------<-------/ | + * | | v * v ^ REMOVE OBJECTMAP * | | / | - * | |-------<-------/ v + * | |-------<-------/ | + * | | v * | | REMOVE MIRROR IMAGE * | | / | - * | |-------<-------/ v + * | |-------<-------/ | + * | | v * | | REMOVE ID OBJECT * | | / | - * | |-------<-------/ v + * | |-------<-------/ | + * | | v * | | REMOVE IMAGE * | | / | - * | |-------<-------/ v - * ------------------->------------ + * | \-------<-------/ | + * | v + * \------------------>------------ * * @endverbatim */ - RemoveRequest(librados::IoCtx &ioctx, const std::string &image_name, const std::string &image_id, - bool force, ProgressContext &prog_ctx, ContextWQ *op_work_queue, Context *on_finish); + RemoveRequest(librados::IoCtx &ioctx, const std::string &image_name, + const std::string &image_id, bool force, + ProgressContext &prog_ctx, ContextWQ *op_work_queue, + Context *on_finish); librados::IoCtx &m_ioctx; std::string m_image_name; @@ -147,9 +159,6 @@ private: void send_close_image(int r); Context *handle_send_close_image(int *result); - void switch_thread_context(); - void handle_switch_thread_context(int r); - void remove_header(); Context *handle_remove_header(int *result); -- 2.39.5