librbd: cleanup Context and librados AIO completion callbacks

Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/librbd/AioObjectRequest.cc b/src/librbd/AioObjectRequest.cc
index 48e98371f9a5af007566ea9550cc69a171230642..aef2f1814734d5c66eb538fa98b4adc4b81dc39c 100644 (file)
--- a/src/librbd/AioObjectRequest.cc
+++ b/src/librbd/AioObjectRequest.cc
@@ -15,6 +15,7 @@
 
 #include "librbd/AioObjectRequest.h"
 #include "librbd/CopyupRequest.h"
+#include "librbd/Utils.h"
 
 #include <boost/bind.hpp>
 #include <boost/optional.hpp>
@@ -217,9 +218,6 @@ namespace librbd {
       return;
     }
 
-    librados::AioCompletion *rados_completion =
-      librados::Rados::aio_create_completion(this, rados_req_cb, NULL);
-    int r;
     librados::ObjectReadOperation op;
     int flags = m_ictx->get_read_flags(m_snap_id);
     if (m_sparse) {
@@ -230,7 +228,10 @@ namespace librbd {
     }
     op.set_op_flags2(m_op_flags);
 
-    r = m_ictx->data_ctx.aio_operate(m_oid, rados_completion, &op, flags, NULL);
+    librados::AioCompletion *rados_completion =
+      util::create_rados_ack_callback(this);
+    int r = m_ictx->data_ctx.aio_operate(m_oid, rados_completion, &op, flags,
+                                         NULL);
     assert(r == 0);
 
     rados_completion->release();
@@ -406,8 +407,7 @@ namespace librbd {
 
         RWLock::WLocker object_map_locker(m_ictx->object_map_lock);
         if (m_ictx->object_map[m_object_no] != new_state) {
-          FunctionContext *ctx = new FunctionContext(
-            boost::bind(&AioObjectRequest::complete, this, _1));
+          Context *ctx = util::create_context_callback<AioObjectRequest>(this);
           bool updated = m_ictx->object_map.aio_update(m_object_no, new_state,
                                                        current_state, ctx);
           assert(updated);
@@ -445,8 +445,7 @@ namespace librbd {
       return true;
     }
 
-    FunctionContext *ctx = new FunctionContext(
-      boost::bind(&AioObjectRequest::complete, this, _1));
+    Context *ctx = util::create_context_callback<AioObjectRequest>(this);
     bool updated = m_ictx->object_map.aio_update(m_object_no,
                                                  OBJECT_NONEXISTENT,
                                                 OBJECT_PENDING, ctx);
@@ -501,7 +500,7 @@ namespace librbd {
     assert(m_write.size() != 0);
 
     librados::AioCompletion *rados_completion =
-      librados::Rados::aio_create_completion(this, NULL, rados_req_cb);
+      util::create_rados_safe_callback(this);
     int r = m_ictx->data_ctx.aio_operate(m_oid, rados_completion, &m_write,
                                         m_snap_seq, m_snaps);
     assert(r == 0);

diff --git a/src/librbd/AsyncRequest.cc b/src/librbd/AsyncRequest.cc
index b8974b024e7e3acfa93f89053bc9682f408dd0aa..ae0650f33e7d693d91d453e0eec919cf9f7c6048 100644 (file)
--- a/src/librbd/AsyncRequest.cc
+++ b/src/librbd/AsyncRequest.cc
@@ -3,6 +3,7 @@
 #include "librbd/AsyncRequest.h"
 #include "librbd/ImageCtx.h"
 #include "librbd/internal.h"
+#include "librbd/Utils.h"
 #include "common/WorkQueue.h"
 #include <boost/bind.hpp>
 
@@ -28,19 +29,18 @@ void AsyncRequest<T>::async_complete(int r) {
 
 template <typename T>
 librados::AioCompletion *AsyncRequest<T>::create_callback_completion() {
-  return librados::Rados::aio_create_completion(create_callback_context(),
-                                               NULL, rados_ctx_cb);
+  return util::create_rados_safe_callback(this);
 }
 
 template <typename T>
 Context *AsyncRequest<T>::create_callback_context() {
-  return new FunctionContext(boost::bind(&AsyncRequest<T>::complete, this, _1));
+  return util::create_context_callback(this);
 }
 
 template <typename T>
 Context *AsyncRequest<T>::create_async_callback_context() {
-  return new FunctionContext(boost::bind(&AsyncRequest<T>::async_complete, this,
-                                         _1));;
+  return util::create_context_callback<AsyncRequest<T>,
+                                       &AsyncRequest<T>::async_complete>(this);
 }
 
 template <typename T>

diff --git a/src/librbd/CopyupRequest.cc b/src/librbd/CopyupRequest.cc
index fdb3d62708003aa02b8585866b59f17e02b196d1..1c1beaa5d701dd0c28b1b13ff0a1d7031b2e7257 100644 (file)
--- a/src/librbd/CopyupRequest.cc
+++ b/src/librbd/CopyupRequest.cc
@@ -15,6 +15,7 @@
 #include "librbd/ImageWatcher.h"
 #include "librbd/internal.h"
 #include "librbd/ObjectMap.h"
+#include "librbd/Utils.h"
 
 #include <boost/bind.hpp>
 #include <boost/lambda/bind.hpp>
@@ -142,9 +143,7 @@ private:
 
       ldout(m_ictx->cct, 20) << __func__ << " " << this << " copyup with "
                              << "empty snapshot context" << dendl;
-      librados::AioCompletion *comp =
-        librados::Rados::aio_create_completion(create_callback_context(), NULL,
-                                               rados_ctx_cb);
+      librados::AioCompletion *comp = util::create_rados_safe_callback(this);
       r = m_ictx->md_ctx.aio_operate(m_oid, comp, &copyup_op, 0, snaps);
       assert(r == 0);
       comp->release();
@@ -167,9 +166,7 @@ private:
       assert(write_op.size() != 0);
 
       snaps.insert(snaps.end(), snapc.snaps.begin(), snapc.snaps.end());
-      librados::AioCompletion *comp =
-        librados::Rados::aio_create_completion(create_callback_context(), NULL,
-                                               rados_ctx_cb);
+      librados::AioCompletion *comp = util::create_rados_safe_callback(this);
       r = m_ictx->data_ctx.aio_operate(m_oid, comp, &write_op);
       assert(r == 0);
       comp->release();
@@ -181,7 +178,7 @@ private:
   {
     m_state = STATE_READ_FROM_PARENT;
     AioCompletion *comp = aio_create_completion_internal(
-      create_callback_context(), rbd_ctx_cb);
+      util::create_context_callback(this), rbd_ctx_cb);
 
     ldout(m_ictx->cct, 20) << __func__ << " " << this
                            << ": completion " << comp
@@ -311,15 +308,10 @@ private:
         boost::lambda::_1, m_ictx, m_object_no, &m_snap_ids,
         boost::lambda::_2));
       AsyncObjectThrottle<> *throttle = new AsyncObjectThrottle<>(
-        NULL, *m_ictx, context_factory, create_callback_context(), NULL, 0,
-        m_snap_ids.size());
+        NULL, *m_ictx, context_factory, util::create_context_callback(this),
+        NULL, 0, m_snap_ids.size());
       throttle->start_ops(m_ictx->concurrent_management_ops);
     }
     return false;
   }
-
-  Context *CopyupRequest::create_callback_context()
-  {
-    return new FunctionContext(boost::bind(&CopyupRequest::complete, this, _1));
-  }
 }

diff --git a/src/librbd/CopyupRequest.h b/src/librbd/CopyupRequest.h
index e3a7cdb9c695bd800cfb61382327aa2b982500d9..cbf7d9fa2964da64803413ae054638f1d643b5e1 100644 (file)
--- a/src/librbd/CopyupRequest.h
+++ b/src/librbd/CopyupRequest.h
@@ -25,6 +25,8 @@ namespace librbd {
     void send();
     void queue_send();
 
+    void complete(int r);
+
   private:
     /**
      * Copyup requests go through the following state machine to read from the
@@ -74,15 +76,12 @@ namespace librbd {
 
     void complete_requests(int r);
 
-    void complete(int r);
     bool should_complete(int r);
 
     void remove_from_list();
 
     bool send_object_map();
     bool send_copyup();
-
-    Context *create_callback_context();
   };
 }
 

diff --git a/src/librbd/DiffIterate.cc b/src/librbd/DiffIterate.cc
index ae52db36970a44d7ee82c4f5f6e2c366ad27d546..8982764ffc37d8252128d6e4232d95819c99de9c 100644 (file)
--- a/src/librbd/DiffIterate.cc
+++ b/src/librbd/DiffIterate.cc
@@ -4,6 +4,7 @@
 #include "librbd/DiffIterate.h"
 #include "librbd/ImageCtx.h"
 #include "librbd/internal.h"
+#include "librbd/Utils.h"
 #include "include/rados/librados.hpp"
 #include "include/interval_set.h"
 #include "common/errno.h"
@@ -61,7 +62,7 @@ public:
   void send() {
     C_OrderedThrottle *ctx = m_diff_context.throttle.start_op(this);
     librados::AioCompletion *rados_completion =
-      librados::Rados::aio_create_completion(ctx, NULL, rados_ctx_cb);
+      util::create_rados_safe_callback(ctx);
 
     librados::ObjectReadOperation op;
     op.list_snaps(&m_snap_set, &m_snap_ret);

diff --git a/src/librbd/LibrbdWriteback.cc b/src/librbd/LibrbdWriteback.cc
index b25dfca28ea822fbca27424a5b48e1e4f5096936..949f75a8b9a37f4ddc4cbae7e8d8f9637cd97f9d 100644 (file)
--- a/src/librbd/LibrbdWriteback.cc
+++ b/src/librbd/LibrbdWriteback.cc
@@ -18,6 +18,7 @@
 #include "librbd/AioCompletion.h"
 #include "librbd/ObjectMap.h"
 #include "librbd/Journal.h"
+#include "librbd/Utils.h"
 
 #include "include/assert.h"
 
@@ -193,12 +194,13 @@ namespace librbd {
       }
     }
 
-    librados::AioCompletion *rados_completion =
-      librados::Rados::aio_create_completion(req, context_cb, NULL);
     librados::ObjectReadOperation op;
     op.read(off, len, pbl, NULL);
     op.set_op_flags2(op_flags);
     int flags = m_ictx->get_read_flags(snapid);
+
+    librados::AioCompletion *rados_completion =
+      util::create_rados_ack_callback(req);
     int r = m_ictx->data_ctx.aio_operate(oid.name, rados_completion, &op,
                                         flags, NULL);
     rados_completion->release();

diff --git a/src/librbd/Makefile.am b/src/librbd/Makefile.am
index 3bf4122cfd0e74de0e56059f18de2dbaf16f4b2b..5a3bd9462f9e94853d744c9520316b7f2cafbd38 100644 (file)
--- a/src/librbd/Makefile.am
+++ b/src/librbd/Makefile.am
@@ -90,6 +90,7 @@ noinst_HEADERS += \
        librbd/parent_types.h \
        librbd/SnapInfo.h \
        librbd/TaskFinisher.h \
+       librbd/Utils.h \
        librbd/WatchNotifyTypes.h \
        librbd/object_map/InvalidateRequest.h \
        librbd/object_map/Request.h \

diff --git a/src/librbd/ObjectMap.cc b/src/librbd/ObjectMap.cc
index 469f4e74289a0283f8dff9c093e15e993a5a7a20..62a9c35d8eba05904fdf5cd07989420298b8b08e 100644 (file)
--- a/src/librbd/ObjectMap.cc
+++ b/src/librbd/ObjectMap.cc
@@ -10,6 +10,7 @@
 #include "librbd/object_map/SnapshotRemoveRequest.h"
 #include "librbd/object_map/SnapshotRollbackRequest.h"
 #include "librbd/object_map/UpdateRequest.h"
+#include "librbd/Utils.h"
 #include "common/dout.h"
 #include "common/errno.h"
 #include "include/stringify.h"
@@ -297,8 +298,7 @@ void ObjectMap::aio_save(Context *on_finish)
   cls_client::object_map_save(&op, m_object_map);
 
   std::string oid(object_map_name(m_image_ctx.id, m_snap_id));
-  librados::AioCompletion *comp = librados::Rados::aio_create_completion(
-    on_finish, NULL, rados_ctx_cb);
+  librados::AioCompletion *comp = util::create_rados_safe_callback(on_finish);
 
   int r = m_image_ctx.md_ctx.aio_operate(oid, comp, &op);
   assert(r == 0);

diff --git a/src/librbd/Utils.h b/src/librbd/Utils.h
new file mode 100644 (file)
index 0000000..bbb30b1
--- /dev/null
+++ b/src/librbd/Utils.h
@@ -0,0 +1,134 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+
+#ifndef CEPH_LIBRBD_UTILS_H
+#define CEPH_LIBRBD_UTILS_H
+
+#include "include/rados/librados.hpp"
+#include "include/Context.h"
+#include <type_traits>
+
+class Context;
+
+namespace librbd {
+
+class ImageCtx;
+
+namespace util {
+
+namespace detail {
+
+template <typename T>
+void rados_callback(rados_completion_t c, void *arg) {
+  reinterpret_cast<T*>(arg)->complete(rados_aio_get_return_value(c));
+}
+
+template <typename T, Context*(T::*MF)(int*), bool destroy>
+void rados_state_callback(rados_completion_t c, void *arg) {
+  T *obj = reinterpret_cast<T*>(arg);
+  int r = rados_aio_get_return_value(c);
+  Context *on_finish = (obj->*MF)(&r);
+  if (on_finish != nullptr) {
+    on_finish->complete(r);
+    if (destroy) {
+      delete obj;
+    }
+  }
+}
+
+template <typename T, void (T::*MF)(int)>
+class C_CallbackAdapter : public Context {
+  T *obj;
+public:
+  C_CallbackAdapter(T *obj) : obj(obj) {
+  }
+
+protected:
+  virtual void finish(int r) {
+    (obj->*MF)(r);
+  }
+};
+
+template <typename T, Context*(T::*MF)(int*), bool destroy>
+class C_StateCallbackAdapter : public Context {
+  T *obj;
+public:
+  C_StateCallbackAdapter(T *obj) : obj(obj){
+  }
+
+protected:
+  virtual void complete(int r) override {
+    Context *on_finish = (obj->*MF)(&r);
+    if (on_finish != nullptr) {
+      on_finish->complete(r);
+      if (destroy) {
+        delete obj;
+      }
+    }
+    Context::complete(r);
+  }
+  virtual void finish(int r) override {
+  }
+};
+
+template <typename WQ>
+struct C_AsyncCallback : public Context {
+  WQ *op_work_queue;
+  Context *on_finish;
+
+  C_AsyncCallback(WQ *op_work_queue, Context *on_finish)
+    : op_work_queue(op_work_queue), on_finish(on_finish) {
+  }
+  virtual void finish(int r) {
+    op_work_queue->queue(on_finish, r);
+  }
+};
+
+} // namespace detail
+
+template <typename T>
+librados::AioCompletion *create_rados_ack_callback(T *obj) {
+  return librados::Rados::aio_create_completion(
+    obj, &detail::rados_callback<T>, nullptr);
+}
+
+template <typename T, Context*(T::*MF)(int*), bool destroy=true>
+librados::AioCompletion *create_rados_ack_callback(T *obj) {
+  return librados::Rados::aio_create_completion(
+    obj, &detail::rados_state_callback<T, MF, destroy>, nullptr);
+}
+
+template <typename T>
+librados::AioCompletion *create_rados_safe_callback(T *obj) {
+  return librados::Rados::aio_create_completion(
+    obj, nullptr, &detail::rados_callback<T>);
+}
+
+template <typename T, Context*(T::*MF)(int*), bool destroy=true>
+librados::AioCompletion *create_rados_safe_callback(T *obj) {
+  return librados::Rados::aio_create_completion(
+    obj, nullptr, &detail::rados_state_callback<T, MF, destroy>);
+}
+
+template <typename T, void(T::*MF)(int) = &T::complete>
+Context *create_context_callback(T *obj) {
+  return new detail::C_CallbackAdapter<T, MF>(obj);
+}
+
+template <typename T, Context*(T::*MF)(int*), bool destroy=true>
+Context *create_context_callback(T *obj) {
+  return new detail::C_StateCallbackAdapter<T, MF, destroy>(obj);
+}
+
+template <typename I>
+Context *create_async_context_callback(I &image_ctx, Context *on_finish) {
+  // use async callback to acquire a clean lock context
+  return new detail::C_AsyncCallback<
+    typename std::decay<decltype(*image_ctx.op_work_queue)>::type>(
+      image_ctx.op_work_queue, on_finish);
+}
+
+} // namespace util
+} // namespace librbd
+
+#endif // CEPH_LIBRBD_UTILS_H

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index fbc9a533f2b51e6533b0c713c9f92789dc6c0432..ca514c8086a697024c5ff386a2f6a937f2c2dbc1 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -42,6 +42,7 @@
 #include "librbd/operation/SnapshotRollbackRequest.h"
 #include "librbd/operation/SnapshotUnprotectRequest.h"
 #include "librbd/operation/TrimRequest.h"
+#include "librbd/Utils.h"
 #include "include/util.h"
 
 #include <boost/bind.hpp>
@@ -3503,18 +3504,6 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
     return r;
   }
 
-  void rados_req_cb(rados_completion_t c, void *arg)
-  {
-    AioObjectRequest *req = reinterpret_cast<AioObjectRequest *>(arg);
-    req->complete(rados_aio_get_return_value(c));
-  }
-
-  void rados_ctx_cb(rados_completion_t c, void *arg)
-  {
-    Context *comp = reinterpret_cast<Context *>(arg);
-    comp->complete(rados_aio_get_return_value(c));
-  }
-
   // validate extent against image size; clip to image size if necessary
   int clip_io(ImageCtx *ictx, uint64_t off, uint64_t *len)
   {

diff --git a/src/librbd/internal.h b/src/librbd/internal.h
index 99a8b44c1410362cfa7ffdf717bcd3ea168fdf31..b562807e32b70b6e3c777acfffefc1a88c61533b 100644 (file)
--- a/src/librbd/internal.h
+++ b/src/librbd/internal.h
@@ -235,8 +235,6 @@ namespace librbd {
                                                callback_t cb_complete);
 
   // raw callbacks
-  void rados_req_cb(rados_completion_t cb, void *arg);
-  void rados_ctx_cb(rados_completion_t cb, void *arg);
   void rbd_req_cb(completion_t cb, void *arg);
   void rbd_ctx_cb(completion_t cb, void *arg);
 }

diff --git a/src/librbd/operation/RebuildObjectMapRequest.cc b/src/librbd/operation/RebuildObjectMapRequest.cc
index 2ed0cbecf9f5d00ae161e5dcbe5798b95661f844..c820da0bd8fb3d74a5e10c17b855781d1ee10cc6 100644 (file)
--- a/src/librbd/operation/RebuildObjectMapRequest.cc
+++ b/src/librbd/operation/RebuildObjectMapRequest.cc
@@ -11,6 +11,7 @@
 #include "librbd/ObjectMap.h"
 #include "librbd/operation/ResizeRequest.h"
 #include "librbd/operation/TrimRequest.h"
+#include "librbd/Utils.h"
 #include <boost/lambda/bind.hpp>
 #include <boost/lambda/construct.hpp>
 
@@ -83,12 +84,10 @@ private:
     ldout(image_ctx.cct, 5) << m_oid << " C_VerifyObject::send_list_snaps"
                             << dendl;
 
-    librados::AioCompletion *comp = librados::Rados::aio_create_completion(
-      this, NULL, rados_ctx_cb);
-
     librados::ObjectReadOperation op;
     op.list_snaps(&m_snap_set, &m_snap_list_ret);
 
+    librados::AioCompletion *comp = util::create_rados_safe_callback(this);
     int r = m_io_ctx.aio_operate(m_oid, comp, &op, NULL);
     assert(r == 0);
     comp->release();

diff --git a/src/librbd/operation/SnapshotRollbackRequest.cc b/src/librbd/operation/SnapshotRollbackRequest.cc
index 2acd2aef0421f5dc0683514a4e2844d2c6e81e6c..39770142d7cb6c2ba11548ef0c397ba8626f04e9 100644 (file)
--- a/src/librbd/operation/SnapshotRollbackRequest.cc
+++ b/src/librbd/operation/SnapshotRollbackRequest.cc
@@ -8,6 +8,7 @@
 #include "librbd/AsyncObjectThrottle.h"
 #include "librbd/ImageCtx.h"
 #include "librbd/ObjectMap.h"
+#include "librbd/Utils.h"
 #include "librbd/operation/ResizeRequest.h"
 #include "osdc/Striper.h"
 #include <boost/lambda/bind.hpp>
@@ -62,10 +63,11 @@ public:
 
     std::string oid = image_ctx.get_object_name(m_object_num);
 
-    librados::AioCompletion *rados_completion =
-      librados::Rados::aio_create_completion(this, NULL, rados_ctx_cb);
     librados::ObjectWriteOperation op;
     op.selfmanaged_snap_rollback(m_snap_id);
+
+    librados::AioCompletion *rados_completion =
+      util::create_rados_safe_callback(this);
     image_ctx.data_ctx.aio_operate(oid, rados_completion, &op);
     rados_completion->release();
     return 0;

diff --git a/src/librbd/operation/TrimRequest.cc b/src/librbd/operation/TrimRequest.cc
index da5a3bb4bc3fa1b870b0d8a53e13e71ebe973963..d2825775084ce5e68d30713e0b8b0e77b1e91135 100644 (file)
--- a/src/librbd/operation/TrimRequest.cc
+++ b/src/librbd/operation/TrimRequest.cc
@@ -8,6 +8,7 @@
 #include "librbd/ImageWatcher.h"
 #include "librbd/internal.h"
 #include "librbd/ObjectMap.h"
+#include "librbd/Utils.h"
 #include "common/ContextCompletion.h"
 #include "common/dout.h"
 #include "common/errno.h"
@@ -76,7 +77,7 @@ public:
     ldout(image_ctx.cct, 10) << "removing " << oid << dendl;
 
     librados::AioCompletion *rados_completion =
-      librados::Rados::aio_create_completion(this, NULL, rados_ctx_cb);
+      util::create_rados_safe_callback(this);
     int r = image_ctx.data_ctx.aio_remove(oid, rados_completion);
     assert(r == 0);
     rados_completion->release();