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>
(cherry picked from commit cc5ac6aa66f4c97cbe1c7d6334b3f710610f6742)

diff --git a/src/librbd/image/CreateRequest.cc b/src/librbd/image/CreateRequest.cc
index 2a9dfac2fc1f26cc3a7aad38407b771513c2abf1..cfb65461974fad95c49fe63245fffe4f36189b3c 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();
@@ -498,7 +500,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;
@@ -534,7 +537,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();
@@ -571,7 +575,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();
@@ -616,7 +621,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 :
@@ -624,9 +630,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();
 }
 
@@ -635,7 +641,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();
@@ -701,11 +708,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/object_watcher/Notifier.cc b/src/librbd/object_watcher/Notifier.cc
index 019c786ee62c4c2686b40ed207d7d69b60878969..b1b62c5eb45d18e9006fdbe99291a6a37d6dca10 100644 (file)
--- a/src/librbd/object_watcher/Notifier.cc
+++ b/src/librbd/object_watcher/Notifier.cc
@@ -16,10 +16,9 @@ namespace object_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/object_watcher/Notifier.h b/src/librbd/object_watcher/Notifier.h
index bfe996eee74df494f58f82100357dee4fc6e118a..3c219afdaa0ec2cbbc6dbc239ae3cf97bf9cf2ab 100644 (file)
--- a/src/librbd/object_watcher/Notifier.h
+++ b/src/librbd/object_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/librbd/operation/ObjectMapIterate.cc b/src/librbd/operation/ObjectMapIterate.cc
index 229a585d2a0789169e9bd035ddcfcbf131a387ec..9cdc3bce686b6ebf96b81adab321fb71db2ced11 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/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc
index 9bf4381d826cee58c7f7fcc2595e454ee6ce18c4..7a999a88d6d62f7f7d376996b6df73a0cbffb555 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