From: Jason Dillaman Date: Thu, 2 Mar 2017 15:29:36 +0000 (-0500) Subject: librbd: avoid duplicating librados IoCtx objects if not needed X-Git-Tag: v12.0.1~109^2~3 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=cc5ac6aa66f4c97cbe1c7d6334b3f710610f6742;p=ceph.git librbd: avoid duplicating librados IoCtx objects if not needed This introduces the potential for shutdown race conditions within the unit tests. Signed-off-by: Jason Dillaman --- diff --git a/src/librbd/image/CreateRequest.cc b/src/librbd/image/CreateRequest.cc index 89d08720153d..51a17a92835a 100644 --- a/src/librbd/image/CreateRequest.cc +++ b/src/librbd/image/CreateRequest.cc @@ -125,12 +125,11 @@ CreateRequest::CreateRequest(IoCtx &ioctx, const std::string &image_name, const std::string &primary_mirror_uuid, bool skip_mirror_enable, ContextWQ *op_work_queue, Context *on_finish) - : m_image_name(image_name), m_image_id(image_id), m_size(size), - m_non_primary_global_image_id(non_primary_global_image_id), + : m_ioctx(ioctx), m_image_name(image_name), m_image_id(image_id), + m_size(size), m_non_primary_global_image_id(non_primary_global_image_id), m_primary_mirror_uuid(primary_mirror_uuid), m_skip_mirror_enable(skip_mirror_enable), m_op_work_queue(op_work_queue), m_on_finish(on_finish) { - m_ioctx.dup(ioctx); m_cct = reinterpret_cast(m_ioctx.cct()); m_id_obj = util::id_obj_name(m_image_name); @@ -294,7 +293,8 @@ Context* CreateRequest::handle_validate_pool(int *result) { create_id_object(); return nullptr; } else if ((*result < 0) && (*result != -ENOENT)) { - lderr(m_cct) << "failed to stat RBD directory: " << cpp_strerror(*result) << dendl; + lderr(m_cct) << "failed to stat RBD directory: " << cpp_strerror(*result) + << dendl; return m_on_finish; } @@ -351,7 +351,8 @@ Context *CreateRequest::handle_create_id_object(int *result) { ldout(m_cct, 20) << __func__ << ": r=" << *result << dendl; if (*result < 0) { - lderr(m_cct) << "error creating RBD id object: " << cpp_strerror(*result) << dendl; + lderr(m_cct) << "error creating RBD id object: " << cpp_strerror(*result) + << dendl; return m_on_finish; } @@ -379,7 +380,8 @@ Context *CreateRequest::handle_add_image_to_directory(int *result) { ldout(m_cct, 20) << __func__ << ": r=" << *result << dendl; if (*result < 0) { - lderr(m_cct) << "error adding image to directory: " << cpp_strerror(*result) << dendl; + lderr(m_cct) << "error adding image to directory: " << cpp_strerror(*result) + << dendl; m_r_saved = *result; remove_id_object(); @@ -503,7 +505,8 @@ Context *CreateRequest::handle_set_stripe_unit_count(int *result) { ldout(m_cct, 20) << __func__ << ": r=" << *result << dendl; if (*result < 0) { - lderr(m_cct) << "error setting stripe unit/count: " << cpp_strerror(*result) << dendl; + lderr(m_cct) << "error setting stripe unit/count: " + << cpp_strerror(*result) << dendl; m_r_saved = *result; remove_header_object(); return nullptr; @@ -539,7 +542,8 @@ Context *CreateRequest::handle_object_map_resize(int *result) { ldout(m_cct, 20) << __func__ << ": r=" << *result << dendl; if (*result < 0) { - lderr(m_cct) << "error creating initial object map: " << cpp_strerror(*result) << dendl; + lderr(m_cct) << "error creating initial object map: " + << cpp_strerror(*result) << dendl; m_r_saved = *result; remove_header_object(); @@ -576,7 +580,8 @@ Context *CreateRequest::handle_fetch_mirror_mode(int *result) { ldout(m_cct, 20) << __func__ << ": r=" << *result << dendl; if ((*result < 0) && (*result != -ENOENT)) { - lderr(m_cct) << "failed to retrieve mirror mode: " << cpp_strerror(*result) << dendl; + lderr(m_cct) << "failed to retrieve mirror mode: " << cpp_strerror(*result) + << dendl; m_r_saved = *result; remove_object_map(); @@ -621,7 +626,8 @@ void CreateRequest::journal_create() { ldout(m_cct, 20) << this << " " << __func__ << dendl; using klass = CreateRequest; - Context *ctx = create_context_callback(this); + Context *ctx = create_context_callback( + this); librbd::journal::TagData tag_data; tag_data.mirror_uuid = (m_force_non_primary ? m_primary_mirror_uuid : @@ -629,9 +635,9 @@ void CreateRequest::journal_create() { librbd::journal::CreateRequest *req = librbd::journal::CreateRequest::create( - m_ioctx, m_image_id, m_journal_order, m_journal_splay_width, m_journal_pool, - cls::journal::Tag::TAG_CLASS_NEW, tag_data, librbd::Journal::IMAGE_CLIENT_ID, - m_op_work_queue, ctx); + m_ioctx, m_image_id, m_journal_order, m_journal_splay_width, + m_journal_pool, cls::journal::Tag::TAG_CLASS_NEW, tag_data, + librbd::Journal::IMAGE_CLIENT_ID, m_op_work_queue, ctx); req->send(); } @@ -640,7 +646,8 @@ Context* CreateRequest::handle_journal_create(int *result) { ldout(m_cct, 20) << __func__ << ": r=" << *result << dendl; if (*result < 0) { - lderr(m_cct) << "error creating journal: " << cpp_strerror(*result) << dendl; + lderr(m_cct) << "error creating journal: " << cpp_strerror(*result) + << dendl; m_r_saved = *result; remove_object_map(); @@ -706,11 +713,13 @@ void CreateRequest::journal_remove() { ldout(m_cct, 20) << this << " " <<__func__ << dendl; using klass = CreateRequest; - Context *ctx = create_context_callback(this); + Context *ctx = create_context_callback( + this); librbd::journal::RemoveRequest *req = librbd::journal::RemoveRequest::create( - m_ioctx, m_image_id, librbd::Journal::IMAGE_CLIENT_ID, m_op_work_queue, ctx); + m_ioctx, m_image_id, librbd::Journal::IMAGE_CLIENT_ID, m_op_work_queue, + ctx); req->send(); } diff --git a/src/librbd/image/CreateRequest.h b/src/librbd/image/CreateRequest.h index 865fd7722682..b481d9ece1a2 100644 --- a/src/librbd/image/CreateRequest.h +++ b/src/librbd/image/CreateRequest.h @@ -98,7 +98,7 @@ private: bool skip_mirror_enable, ContextWQ *op_work_queue, Context *on_finish); - IoCtx m_ioctx; + IoCtx &m_ioctx; std::string m_image_name; std::string m_image_id; uint64_t m_size; diff --git a/src/librbd/journal/CreateRequest.cc b/src/librbd/journal/CreateRequest.cc index b897fb503b1e..033d992c8482 100644 --- a/src/librbd/journal/CreateRequest.cc +++ b/src/librbd/journal/CreateRequest.cc @@ -23,17 +23,16 @@ namespace journal { template CreateRequest::CreateRequest(IoCtx &ioctx, const std::string &imageid, - uint8_t order, uint8_t splay_width, - const std::string &object_pool, - uint64_t tag_class, TagData &tag_data, - const std::string &client_id, - ContextWQ *op_work_queue, - Context *on_finish) - : m_image_id(imageid), m_order(order), m_splay_width(splay_width), - m_object_pool(object_pool), m_tag_class(tag_class), m_tag_data(tag_data), - m_image_client_id(client_id), m_op_work_queue(op_work_queue), - m_on_finish(on_finish) { - m_ioctx.dup(ioctx); + uint8_t order, uint8_t splay_width, + const std::string &object_pool, + uint64_t tag_class, TagData &tag_data, + const std::string &client_id, + ContextWQ *op_work_queue, + Context *on_finish) + : m_ioctx(ioctx), m_image_id(imageid), m_order(order), + m_splay_width(splay_width), m_object_pool(object_pool), + m_tag_class(tag_class), m_tag_data(tag_data), m_image_client_id(client_id), + m_op_work_queue(op_work_queue), m_on_finish(on_finish) { m_cct = reinterpret_cast(m_ioctx.cct()); } diff --git a/src/librbd/journal/CreateRequest.h b/src/librbd/journal/CreateRequest.h index b8632ad847c7..c2b8cd9cf796 100644 --- a/src/librbd/journal/CreateRequest.h +++ b/src/librbd/journal/CreateRequest.h @@ -57,7 +57,7 @@ private: const std::string &client_id, ContextWQ *op_work_queue, Context *on_finish); - IoCtx m_ioctx; + IoCtx &m_ioctx; std::string m_image_id; uint8_t m_order; uint8_t m_splay_width; diff --git a/src/librbd/journal/RemoveRequest.cc b/src/librbd/journal/RemoveRequest.cc index c9dd020b6341..aa2d13e8652c 100644 --- a/src/librbd/journal/RemoveRequest.cc +++ b/src/librbd/journal/RemoveRequest.cc @@ -22,12 +22,11 @@ namespace journal { template RemoveRequest::RemoveRequest(IoCtx &ioctx, const std::string &image_id, - const std::string &client_id, - ContextWQ *op_work_queue, - Context *on_finish) - : m_image_id(image_id), m_image_client_id(client_id), + const std::string &client_id, + ContextWQ *op_work_queue, + Context *on_finish) + : m_ioctx(ioctx), m_image_id(image_id), m_image_client_id(client_id), m_op_work_queue(op_work_queue), m_on_finish(on_finish) { - m_ioctx.dup(ioctx); m_cct = reinterpret_cast(m_ioctx.cct()); } diff --git a/src/librbd/journal/RemoveRequest.h b/src/librbd/journal/RemoveRequest.h index e710a131d753..594e62d5eb89 100644 --- a/src/librbd/journal/RemoveRequest.h +++ b/src/librbd/journal/RemoveRequest.h @@ -49,7 +49,7 @@ private: const std::string &client_id, ContextWQ *op_work_queue, Context *on_finish); - IoCtx m_ioctx; + IoCtx &m_ioctx; std::string m_image_id; std::string m_image_client_id; ContextWQ *m_op_work_queue; diff --git a/src/librbd/operation/ObjectMapIterate.cc b/src/librbd/operation/ObjectMapIterate.cc index 5276388d09f2..35119e0f5cd2 100644 --- a/src/librbd/operation/ObjectMapIterate.cc +++ b/src/librbd/operation/ObjectMapIterate.cc @@ -47,6 +47,8 @@ public: if (should_complete(r)) { ldout(image_ctx.cct, 20) << m_oid << " C_VerifyObjectCallback completed " << dendl; + m_io_ctx.close(); + this->finish(r); delete this; } diff --git a/src/librbd/watcher/Notifier.cc b/src/librbd/watcher/Notifier.cc index e1a3a845c56b..f36bee760345 100644 --- a/src/librbd/watcher/Notifier.cc +++ b/src/librbd/watcher/Notifier.cc @@ -16,10 +16,9 @@ namespace watcher { const uint64_t Notifier::NOTIFY_TIMEOUT = 5000; Notifier::Notifier(ContextWQ *work_queue, IoCtx &ioctx, const std::string &oid) - : m_work_queue(work_queue), m_oid(oid), + : m_work_queue(work_queue), m_ioctx(ioctx), m_oid(oid), m_aio_notify_lock(util::unique_lock_name( "librbd::object_watcher::Notifier::m_aio_notify_lock", this)) { - m_ioctx.dup(ioctx); m_cct = reinterpret_cast(m_ioctx.cct()); } diff --git a/src/librbd/watcher/Notifier.h b/src/librbd/watcher/Notifier.h index 609c2ada739d..b0173e3f212e 100644 --- a/src/librbd/watcher/Notifier.h +++ b/src/librbd/watcher/Notifier.h @@ -43,7 +43,7 @@ private: }; ContextWQ *m_work_queue; - librados::IoCtx m_ioctx; + librados::IoCtx &m_ioctx; CephContext *m_cct; std::string m_oid; diff --git a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc index 64f843bb9f23..2d5f1470e8a2 100644 --- a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc +++ b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc @@ -451,8 +451,11 @@ template void ObjectCopyRequest::finish(int r) { dout(20) << ": r=" << r << dendl; - m_on_finish->complete(r); + // ensure IoCtxs are closed prior to proceeding + auto on_finish = m_on_finish; delete this; + + on_finish->complete(r); } } // namespace image_sync