From cc5ac6aa66f4c97cbe1c7d6334b3f710610f6742 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Thu, 2 Mar 2017 10:29:36 -0500 Subject: 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 --- src/librbd/image/CreateRequest.cc | 41 +++++++++++++++++++------------- src/librbd/image/CreateRequest.h | 2 +- src/librbd/journal/CreateRequest.cc | 21 ++++++++-------- src/librbd/journal/CreateRequest.h | 2 +- src/librbd/journal/RemoveRequest.cc | 9 ++++--- src/librbd/journal/RemoveRequest.h | 2 +- src/librbd/operation/ObjectMapIterate.cc | 2 ++ src/librbd/watcher/Notifier.cc | 3 +-- src/librbd/watcher/Notifier.h | 2 +- 9 files changed, 46 insertions(+), 38 deletions(-) (limited to 'src/librbd') diff --git a/src/librbd/image/CreateRequest.cc b/src/librbd/image/CreateRequest.cc index 89d08720153..51a17a92835 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 865fd772268..b481d9ece1a 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 b897fb503b1..033d992c848 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 b8632ad847c..c2b8cd9cf79 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 c9dd020b634..aa2d13e8652 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 e710a131d75..594e62d5eb8 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 5276388d09f..35119e0f5cd 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 e1a3a845c56..f36bee76034 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 609c2ada739..b0173e3f212 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; -- cgit v1.2.3