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 <dillaman@redhat.com>

diff --git a/src/librbd/image/CreateRequest.cc b/src/librbd/image/CreateRequest.cc
index 89d08720153dfd3e01590b091cffa8ab4fd3d5a2..51a17a92835a7aa83b01be5f70e680b392d95c39 100644 (file)
--- a/src/librbd/image/CreateRequest.cc
+++ b/src/librbd/image/CreateRequest.cc
@@ -125,12 +125,11 @@ CreateRequest<I>::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<CephContext *>(m_ioctx.cct());
 
   m_id_obj = util::id_obj_name(m_image_name);
@@ -294,7 +293,8 @@ Context* CreateRequest<I>::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<I>::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<I>::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<I>::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<I>::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<I>::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<I>::journal_create() {
   ldout(m_cct, 20) << this << " " << __func__ << dendl;
 
   using klass = CreateRequest<I>;
-  Context *ctx = create_context_callback<klass, &klass::handle_journal_create>(this);
+  Context *ctx = create_context_callback<klass, &klass::handle_journal_create>(
+    this);
 
   librbd::journal::TagData tag_data;
   tag_data.mirror_uuid = (m_force_non_primary ? m_primary_mirror_uuid :
@@ -629,9 +635,9 @@ void CreateRequest<I>::journal_create() {
 
   librbd::journal::CreateRequest<I> *req =
     librbd::journal::CreateRequest<I>::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<I>::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<I>::IMAGE_CLIENT_ID, m_op_work_queue, ctx);
   req->send();
 }
 
@@ -640,7 +646,8 @@ Context* CreateRequest<I>::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<I>::journal_remove() {
   ldout(m_cct, 20) << this << " " <<__func__ << dendl;
 
   using klass = CreateRequest<I>;
-  Context *ctx = create_context_callback<klass, &klass::handle_journal_remove>(this);
+  Context *ctx = create_context_callback<klass, &klass::handle_journal_remove>(
+    this);
 
   librbd::journal::RemoveRequest<I> *req =
     librbd::journal::RemoveRequest<I>::create(
-      m_ioctx, m_image_id, librbd::Journal<I>::IMAGE_CLIENT_ID, m_op_work_queue, ctx);
+      m_ioctx, m_image_id, librbd::Journal<I>::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 865fd772268274f10c27125b50e3eb0aa72e7494..b481d9ece1a256e2d8d1cc1444aa2dd9da86e5fc 100644 (file)
--- 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 b897fb503b1ef982f7015804396bc27bc0f3e9e4..033d992c848250b5b8c251aca6c4c19ff87c85a0 100644 (file)
--- a/src/librbd/journal/CreateRequest.cc
+++ b/src/librbd/journal/CreateRequest.cc
@@ -23,17 +23,16 @@ namespace journal {
 
 template<typename I>
 CreateRequest<I>::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<CephContext *>(m_ioctx.cct());
 }
 

diff --git a/src/librbd/journal/CreateRequest.h b/src/librbd/journal/CreateRequest.h
index b8632ad847c7a371c24413478c59c9fd6e805a8f..c2b8cd9cf796e7d14366b5883c532edc30626877 100644 (file)
--- 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 c9dd020b63412602f12490c9724cf60671bb1f57..aa2d13e8652c416dd6886b3fc7c0fa2e50e075df 100644 (file)
--- a/src/librbd/journal/RemoveRequest.cc
+++ b/src/librbd/journal/RemoveRequest.cc
@@ -22,12 +22,11 @@ namespace journal {
 
 template<typename I>
 RemoveRequest<I>::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<CephContext *>(m_ioctx.cct());
 }
 

diff --git a/src/librbd/journal/RemoveRequest.h b/src/librbd/journal/RemoveRequest.h
index e710a131d753e0ed4c4b44de18f190ec8ebec631..594e62d5eb89a6c44bd432b2229c581ab8c89b65 100644 (file)
--- 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 5276388d09f2fddea5f326f5afeaec1a9113c633..35119e0f5cd2dc3e91ef5eed5cda288f28154c69 100644 (file)
--- 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 e1a3a845c56bd10acbe450369210e3cac25903d1..f36bee7603453db542ef50a39c32e92ee1fc68c8 100644 (file)
--- 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<CephContext *>(m_ioctx.cct());
 }
 

diff --git a/src/librbd/watcher/Notifier.h b/src/librbd/watcher/Notifier.h
index 609c2ada739d7b8f35a4d4581989db9373dd7c2b..b0173e3f212e21fd8e0c5a704d1fcc4c66009316 100644 (file)
--- 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 64f843bb9f23dff4f7b1368c2821c6d12b4358ea..2d5f1470e8a2a4e74cf621d4b891b40b51313e13 100644 (file)
--- a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc
+++ b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc
@@ -451,8 +451,11 @@ template <typename I>
 void ObjectCopyRequest<I>::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