librbd: templatize io::CopyupRequest and io::ObjectRequest

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

diff --git a/src/librbd/ImageCtx.h b/src/librbd/ImageCtx.h
index 910182a70dd6412a1e71e713e1505d730d0a2ea8..9479bae30a9a566ce0e7f7a42bdb72ec5b039927 100644 (file)
--- a/src/librbd/ImageCtx.h
+++ b/src/librbd/ImageCtx.h
@@ -52,8 +52,8 @@ namespace librbd {
   namespace io {
   class AioCompletion;
   class AsyncOperation;
+  template <typename> class CopyupRequest;
   template <typename> class ImageRequestWQ;
-  class CopyupRequest;
   }
   namespace journal { struct Policy; }
 
@@ -140,7 +140,7 @@ namespace librbd {
     Readahead readahead;
     uint64_t total_bytes_read;
 
-    std::map<uint64_t, io::CopyupRequest*> copyup_list;
+    std::map<uint64_t, io::CopyupRequest<ImageCtx>*> copyup_list;
 
     xlist<io::AsyncOperation*> async_ops;
     xlist<AsyncRequest<>*> async_requests;

diff --git a/src/librbd/io/CopyupRequest.cc b/src/librbd/io/CopyupRequest.cc
index 5f01b45e259ffb3c29480d665c5875eb53a476e9..ac3cffb43b8678c36a824472773b8a7a04be4eed 100644 (file)
--- a/src/librbd/io/CopyupRequest.cc
+++ b/src/librbd/io/CopyupRequest.cc
@@ -80,11 +80,11 @@ private:
 
 } // anonymous namespace
 
-
-CopyupRequest::CopyupRequest(ImageCtx *ictx, const std::string &oid,
-                             uint64_t objectno, Extents &&image_extents,
-                            const ZTracer::Trace &parent_trace)
-  : m_ictx(ictx), m_oid(oid), m_object_no(objectno),
+template <typename I>
+CopyupRequest<I>::CopyupRequest(I *ictx, const std::string &oid,
+                                uint64_t objectno, Extents &&image_extents,
+                                const ZTracer::Trace &parent_trace)
+  : m_ictx(util::get_image_ctx(ictx)), m_oid(oid), m_object_no(objectno),
     m_image_extents(image_extents),
     m_trace(util::create_trace(*m_ictx, "copy-up", parent_trace)),
     m_state(STATE_READ_FROM_PARENT)
@@ -92,17 +92,20 @@ CopyupRequest::CopyupRequest(ImageCtx *ictx, const std::string &oid,
   m_async_op.start_op(*m_ictx);
 }
 
-CopyupRequest::~CopyupRequest() {
+template <typename I>
+CopyupRequest<I>::~CopyupRequest() {
   assert(m_pending_requests.empty());
   m_async_op.finish_op();
 }
 
-void CopyupRequest::append_request(ObjectRequest<> *req) {
+template <typename I>
+void CopyupRequest<I>::append_request(ObjectRequest<I> *req) {
   ldout(m_ictx->cct, 20) << req << dendl;
   m_pending_requests.push_back(req);
 }
 
-void CopyupRequest::complete_requests(int r) {
+template <typename I>
+void CopyupRequest<I>::complete_requests(int r) {
   while (!m_pending_requests.empty()) {
     vector<ObjectRequest<> *>::iterator it = m_pending_requests.begin();
     ObjectRequest<> *req = *it;
@@ -112,7 +115,8 @@ void CopyupRequest::complete_requests(int r) {
   }
 }
 
-bool CopyupRequest::send_copyup() {
+template <typename I>
+bool CopyupRequest<I>::send_copyup() {
   bool add_copyup_op = !m_copyup_data.is_zero();
   bool copy_on_read = m_pending_requests.empty();
   if (!add_copyup_op && copy_on_read) {
@@ -189,7 +193,8 @@ bool CopyupRequest::send_copyup() {
   return false;
 }
 
-bool CopyupRequest::is_copyup_required() {
+template <typename I>
+bool CopyupRequest<I>::is_copyup_required() {
   bool noop = true;
   for (const ObjectRequest<> *req : m_pending_requests) {
     if (!req->is_op_payload_empty()) {
@@ -201,7 +206,8 @@ bool CopyupRequest::is_copyup_required() {
   return (m_copyup_data.is_zero() && noop);
 }
 
-void CopyupRequest::send()
+template <typename I>
+void CopyupRequest<I>::send()
 {
   m_state = STATE_READ_FROM_PARENT;
   AioCompletion *comp = AioCompletion::create_and_start(
@@ -215,7 +221,8 @@ void CopyupRequest::send()
                            ReadResult{&m_copyup_data}, 0, m_trace);
 }
 
-void CopyupRequest::complete(int r)
+template <typename I>
+void CopyupRequest<I>::complete(int r)
 {
   if (should_complete(r)) {
     complete_requests(r);
@@ -223,7 +230,8 @@ void CopyupRequest::complete(int r)
   }
 }
 
-bool CopyupRequest::should_complete(int r)
+template <typename I>
+bool CopyupRequest<I>::should_complete(int r)
 {
   CephContext *cct = m_ictx->cct;
   ldout(cct, 20) << "oid " << m_oid
@@ -277,17 +285,18 @@ bool CopyupRequest::should_complete(int r)
   return (r < 0);
 }
 
-void CopyupRequest::remove_from_list()
+template <typename I>
+void CopyupRequest<I>::remove_from_list()
 {
   Mutex::Locker l(m_ictx->copyup_list_lock);
 
-  map<uint64_t, CopyupRequest*>::iterator it =
-    m_ictx->copyup_list.find(m_object_no);
+  auto it = m_ictx->copyup_list.find(m_object_no);
   assert(it != m_ictx->copyup_list.end());
   m_ictx->copyup_list.erase(it);
 }
 
-bool CopyupRequest::send_object_map_head() {
+template <typename I>
+bool CopyupRequest<I>::send_object_map_head() {
   CephContext *cct = m_ictx->cct;
   ldout(cct, 20) << dendl;
 
@@ -346,7 +355,8 @@ bool CopyupRequest::send_object_map_head() {
   return send_object_map();
 }
 
-bool CopyupRequest::send_object_map() {
+template <typename I>
+bool CopyupRequest<I>::send_object_map() {
   // avoid possible recursive lock attempts
   if (m_snap_ids.empty()) {
     // no object map update required
@@ -371,3 +381,5 @@ bool CopyupRequest::send_object_map() {
 
 } // namespace io
 } // namespace librbd
+
+template class librbd::io::CopyupRequest<librbd::ImageCtx>;

diff --git a/src/librbd/io/CopyupRequest.h b/src/librbd/io/CopyupRequest.h
index c4aa6a25956a54653abe66f458133cd2b606eee3..6b0413bb4e9af98a67ebc49a02df15692682a06e 100644 (file)
--- a/src/librbd/io/CopyupRequest.h
+++ b/src/librbd/io/CopyupRequest.h
@@ -26,13 +26,21 @@ namespace io {
 struct AioCompletion;
 template <typename I> class ObjectRequest;
 
+template <typename ImageCtxT = librbd::ImageCtx>
 class CopyupRequest {
 public:
-  CopyupRequest(ImageCtx *ictx, const std::string &oid, uint64_t objectno,
+  static CopyupRequest* create(ImageCtxT *ictx, const std::string &oid,
+                               uint64_t objectno, Extents &&image_extents,
+                               const ZTracer::Trace &parent_trace) {
+    return new CopyupRequest(ictx, oid, objectno, std::move(image_extents),
+                             parent_trace);
+  }
+
+  CopyupRequest(ImageCtxT *ictx, const std::string &oid, uint64_t objectno,
                 Extents &&image_extents, const ZTracer::Trace &parent_trace);
   ~CopyupRequest();
 
-  void append_request(ObjectRequest<ImageCtx> *req);
+  void append_request(ObjectRequest<ImageCtxT> *req);
 
   void send();
 
@@ -84,7 +92,7 @@ private:
 
   State m_state;
   ceph::bufferlist m_copyup_data;
-  std::vector<ObjectRequest<ImageCtx> *> m_pending_requests;
+  std::vector<ObjectRequest<ImageCtxT> *> m_pending_requests;
   std::atomic<unsigned> m_pending_copyups { 0 };
 
   AsyncOperation m_async_op;
@@ -107,4 +115,6 @@ private:
 } // namespace io
 } // namespace librbd
 
+extern template class librbd::io::CopyupRequest<librbd::ImageCtx>;
+
 #endif // CEPH_LIBRBD_IO_COPYUP_REQUEST_H

diff --git a/src/librbd/io/ObjectRequest.cc b/src/librbd/io/ObjectRequest.cc
index d6a3905deabafb24a2c1ed6c70cd33c3eb780d23..16e8aabf20d1382671f20c726a5e90cacd865fb3 100644 (file)
--- a/src/librbd/io/ObjectRequest.cc
+++ b/src/librbd/io/ObjectRequest.cc
@@ -31,6 +31,19 @@
 namespace librbd {
 namespace io {
 
+namespace {
+
+template <typename I>
+inline bool is_copy_on_read(I *ictx, librados::snap_t snap_id) {
+  assert(ictx->snap_lock.is_locked());
+  return (ictx->clone_copy_on_read &&
+          !ictx->read_only && snap_id == CEPH_NOSNAP &&
+          (ictx->exclusive_lock == nullptr ||
+           ictx->exclusive_lock->is_lock_owner()));
+}
+
+} // anonymous namespace
+
 template <typename I>
 ObjectRequest<I>*
 ObjectRequest<I>::create_remove(I *ictx, const std::string &oid,
@@ -121,7 +134,7 @@ ObjectRequest<I>::create_compare_and_write(I *ictx, const std::string &oid,
 }
 
 template <typename I>
-ObjectRequest<I>::ObjectRequest(ImageCtx *ictx, const std::string &oid,
+ObjectRequest<I>::ObjectRequest(I *ictx, const std::string &oid,
                                 uint64_t objectno, uint64_t off,
                                 uint64_t len, librados::snap_t snap_id,
                                 bool hide_enoent, const char *trace_name,
@@ -186,14 +199,6 @@ bool ObjectRequest<I>::compute_parent_extents() {
   return false;
 }
 
-static inline bool is_copy_on_read(ImageCtx *ictx, librados::snap_t snap_id) {
-  assert(ictx->snap_lock.is_locked());
-  return (ictx->clone_copy_on_read &&
-          !ictx->read_only && snap_id == CEPH_NOSNAP &&
-          (ictx->exclusive_lock == nullptr ||
-           ictx->exclusive_lock->is_lock_owner()));
-}
-
 /** read **/
 
 template <typename I>
@@ -204,8 +209,8 @@ ObjectReadRequest<I>::ObjectReadRequest(I *ictx, const std::string &oid,
                                        int op_flags,
                                        const ZTracer::Trace &parent_trace,
                                         Context *completion)
-  : ObjectRequest<I>(util::get_image_ctx(ictx), oid, objectno, offset, len,
-                     snap_id, false, "read", parent_trace, completion),
+  : ObjectRequest<I>(ictx, oid, objectno, offset, len, snap_id, false, "read",
+                     parent_trace, completion),
     m_buffer_extents(be), m_tried_parent(false), m_sparse(sparse),
     m_op_flags(op_flags), m_state(LIBRBD_AIO_READ_FLAT) {
   guard_read();
@@ -214,7 +219,7 @@ ObjectReadRequest<I>::ObjectReadRequest(I *ictx, const std::string &oid,
 template <typename I>
 void ObjectReadRequest<I>::guard_read()
 {
-  ImageCtx *image_ctx = this->m_ictx;
+  I *image_ctx = this->m_ictx;
   RWLock::RLocker snap_locker(image_ctx->snap_lock);
   RWLock::RLocker parent_locker(image_ctx->parent_lock);
 
@@ -227,7 +232,7 @@ void ObjectReadRequest<I>::guard_read()
 template <typename I>
 bool ObjectReadRequest<I>::should_complete(int r)
 {
-  ImageCtx *image_ctx = this->m_ictx;
+  I *image_ctx = this->m_ictx;
   ldout(image_ctx->cct, 20) << this->m_oid << " "
                             << this->m_object_off << "~" << this->m_object_len
                             << " r = " << r << dendl;
@@ -302,7 +307,7 @@ bool ObjectReadRequest<I>::should_complete(int r)
 
 template <typename I>
 void ObjectReadRequest<I>::send() {
-  ImageCtx *image_ctx = this->m_ictx;
+  I *image_ctx = this->m_ictx;
   ldout(image_ctx->cct, 20) << this->m_oid << " " << this->m_object_off
                             << "~" << this->m_object_len
                             << dendl;
@@ -342,7 +347,7 @@ void ObjectReadRequest<I>::send() {
 template <typename I>
 void ObjectReadRequest<I>::send_copyup()
 {
-  ImageCtx *image_ctx = this->m_ictx;
+  I *image_ctx = this->m_ictx;
   ldout(image_ctx->cct, 20) << this->m_oid << " " << this->m_object_off
                             << "~" << this->m_object_len << dendl;
 
@@ -357,11 +362,10 @@ void ObjectReadRequest<I>::send_copyup()
   }
 
   Mutex::Locker copyup_locker(image_ctx->copyup_list_lock);
-  map<uint64_t, CopyupRequest*>::iterator it =
-    image_ctx->copyup_list.find(this->m_object_no);
+  auto it = image_ctx->copyup_list.find(this->m_object_no);
   if (it == image_ctx->copyup_list.end()) {
     // create and kick off a CopyupRequest
-    CopyupRequest *new_req = new CopyupRequest(
+    auto new_req = CopyupRequest<I>::create(
       image_ctx, this->m_oid, this->m_object_no,
       std::move(this->m_parent_extents), this->m_trace);
     this->m_parent_extents.clear();
@@ -374,15 +378,15 @@ void ObjectReadRequest<I>::send_copyup()
 template <typename I>
 void ObjectReadRequest<I>::read_from_parent(Extents&& parent_extents)
 {
-  ImageCtx *image_ctx = this->m_ictx;
+  I *image_ctx = this->m_ictx;
   AioCompletion *parent_completion = AioCompletion::create_and_start<
-    ObjectRequest<I> >(this, image_ctx, AIO_TYPE_READ);
+    ObjectRequest<I> >(this, util::get_image_ctx(image_ctx), AIO_TYPE_READ);
 
   ldout(image_ctx->cct, 20) << "parent completion " << parent_completion
                             << " extents " << parent_extents << dendl;
-  ImageRequest<>::aio_read(image_ctx->parent, parent_completion,
-                           std::move(parent_extents),
-                           ReadResult{&m_read_data}, 0, this->m_trace);
+  ImageRequest<I>::aio_read(image_ctx->parent, parent_completion,
+                            std::move(parent_extents),
+                            ReadResult{&m_read_data}, 0, this->m_trace);
 }
 
 /** write **/
@@ -568,13 +572,10 @@ void AbstractObjectWriteRequest::send_copyup()
   m_state = LIBRBD_AIO_WRITE_COPYUP;
 
   m_ictx->copyup_list_lock.Lock();
-  map<uint64_t, CopyupRequest*>::iterator it =
-    m_ictx->copyup_list.find(m_object_no);
+  auto it = m_ictx->copyup_list.find(m_object_no);
   if (it == m_ictx->copyup_list.end()) {
-    CopyupRequest *new_req = new CopyupRequest(m_ictx, m_oid,
-                                               m_object_no,
-                                               std::move(m_parent_extents),
-                                              this->m_trace);
+    auto new_req = CopyupRequest<>::create(
+      m_ictx, m_oid, m_object_no, std::move(m_parent_extents), this->m_trace);
     m_parent_extents.clear();
 
     // make sure to wait on this CopyupRequest

diff --git a/src/librbd/io/ObjectRequest.h b/src/librbd/io/ObjectRequest.h
index 9b6b5a9bc6d6ac89f56c7317c14e806c547ce20d..9c3551db31046295781757bba0fff779efa23b4e 100644 (file)
--- a/src/librbd/io/ObjectRequest.h
+++ b/src/librbd/io/ObjectRequest.h
@@ -21,7 +21,7 @@ struct ImageCtx;
 namespace io {
 
 struct AioCompletion;
-class CopyupRequest;
+template <typename> class CopyupRequest;
 class ObjectRemoveRequest;
 class ObjectTruncateRequest;
 class ObjectWriteRequest;
@@ -97,7 +97,7 @@ public:
                                                  const ZTracer::Trace &parent_trace,
                                                  Context *completion);
 
-  ObjectRequest(ImageCtx *ictx, const std::string &oid,
+  ObjectRequest(ImageCtxT *ictx, const std::string &oid,
                 uint64_t objectno, uint64_t off, uint64_t len,
                 librados::snap_t snap_id, bool hide_enoent,
                const char *trace_name, const ZTracer::Trace &parent_trace,
@@ -129,7 +129,7 @@ public:
 protected:
   bool compute_parent_extents();
 
-  ImageCtx *m_ictx;
+  ImageCtxT *m_ictx;
   std::string m_oid;
   uint64_t m_object_no, m_object_off, m_object_len;
   librados::snap_t m_snap_id;

diff --git a/src/test/librbd/mock/MockImageCtx.h b/src/test/librbd/mock/MockImageCtx.h
index dca86c17d2b510c7e637f59e969b1f76577a1981..3b9228194594d37fcfc9ec5bc60fd7733d0a5f34 100644 (file)
--- a/src/test/librbd/mock/MockImageCtx.h
+++ b/src/test/librbd/mock/MockImageCtx.h
@@ -66,6 +66,7 @@ struct MockImageCtx {
       parent_lock(image_ctx.parent_lock),
       object_map_lock(image_ctx.object_map_lock),
       async_ops_lock(image_ctx.async_ops_lock),
+      copyup_list_lock(image_ctx.copyup_list_lock),
       order(image_ctx.order),
       size(image_ctx.size),
       features(image_ctx.features),
@@ -92,6 +93,7 @@ struct MockImageCtx {
       concurrent_management_ops(image_ctx.concurrent_management_ops),
       blacklist_on_break_lock(image_ctx.blacklist_on_break_lock),
       blacklist_expire_seconds(image_ctx.blacklist_expire_seconds),
+      sparse_read_threshold_bytes(image_ctx.sparse_read_threshold_bytes),
       journal_order(image_ctx.journal_order),
       journal_splay_width(image_ctx.journal_splay_width),
       journal_commit_age(image_ctx.journal_commit_age),
@@ -148,6 +150,7 @@ struct MockImageCtx {
   MOCK_CONST_METHOD0(get_current_size, uint64_t());
   MOCK_CONST_METHOD1(get_image_size, uint64_t(librados::snap_t));
   MOCK_CONST_METHOD1(get_object_count, uint64_t(librados::snap_t));
+  MOCK_CONST_METHOD1(get_read_flags, int(librados::snap_t));
   MOCK_CONST_METHOD2(get_snap_id,
                     librados::snap_t(cls::rbd::SnapshotNamespace snap_namespace,
                                      std::string in_snap_name));
@@ -158,6 +161,8 @@ struct MockImageCtx {
                                           ParentSpec *pspec));
   MOCK_CONST_METHOD2(get_parent_overlap, int(librados::snap_t in_snap_id,
                                              uint64_t *overlap));
+  MOCK_CONST_METHOD2(prune_parent_extents, uint64_t(vector<pair<uint64_t,uint64_t> >& ,
+                                                    uint64_t));
 
   MOCK_CONST_METHOD2(is_snap_protected, int(librados::snap_t in_snap_id,
                                             bool *is_protected));
@@ -247,6 +252,7 @@ struct MockImageCtx {
   RWLock &parent_lock;
   RWLock &object_map_lock;
   Mutex &async_ops_lock;
+  Mutex &copyup_list_lock;
 
   uint8_t order;
   uint64_t size;
@@ -268,6 +274,8 @@ struct MockImageCtx {
   xlist<AsyncRequest<MockImageCtx>*> async_requests;
   std::list<Context*> async_requests_waiters;
 
+  std::map<uint64_t, io::CopyupRequest<MockImageCtx>*> copyup_list;
+
   io::MockImageRequestWQ *io_work_queue;
   MockContextWQ *op_work_queue;
 
@@ -292,6 +300,7 @@ struct MockImageCtx {
   int concurrent_management_ops;
   bool blacklist_on_break_lock;
   uint32_t blacklist_expire_seconds;
+  uint64_t sparse_read_threshold_bytes;
   uint8_t journal_order;
   uint8_t journal_splay_width;
   double journal_commit_age;