]> git-server-git.apps.pok.os.sepia.ceph.com Git - ceph.git/commitdiff
librbd: make read_parent reusable
authorMykola Golub <mgolub@suse.com>
Mon, 20 Jul 2020 09:53:57 +0000 (10:53 +0100)
committerJason Dillaman <dillaman@redhat.com>
Thu, 27 Aug 2020 17:56:04 +0000 (13:56 -0400)
Signed-off-by: Mykola Golub <mgolub@suse.com>
(cherry picked from commit 187c92ff3dfe0f171de203c7820042f88c79b3d3)

Conflicts:
src/librbd/io/Utils.h: missing Context forward declaration
src/librbd/io/ObjectRequest.cc: ASIO refactor
src/librbd/io/Utils.cc: ASIO refactor
src/test/librbd/io/test_mock_ObjectRequest.cc: ASIO refactor

src/librbd/io/ImageRequest.h
src/librbd/io/ObjectRequest.cc
src/librbd/io/Utils.cc
src/librbd/io/Utils.h
src/test/librbd/io/test_mock_ObjectRequest.cc

index e5a04597d3e7df46e3a05edebd03119950c636c1..0fe10f2c9fe41c45c97145cc77df05ac52f644af 100644 (file)
@@ -79,7 +79,7 @@ protected:
               const ZTracer::Trace &parent_trace)
     : m_image_ctx(image_ctx), m_aio_comp(aio_comp),
       m_image_extents(std::move(image_extents)),
-      m_trace(util::create_trace(image_ctx, trace_name, parent_trace)) {
+      m_trace(librbd::util::create_trace(image_ctx, trace_name, parent_trace)) {
     m_trace.event("start");
   }
 
index 3966dd2e725b5f01a8ca0fbdf16b6e4169eccfa0..ffd21dfc617e6b44b65d1609ae04f6a776fdfa7a 100644 (file)
@@ -17,8 +17,7 @@
 #include "librbd/Utils.h"
 #include "librbd/io/AioCompletion.h"
 #include "librbd/io/CopyupRequest.h"
-#include "librbd/io/ImageRequest.h"
-#include "librbd/io/ReadResult.h"
+#include "librbd/io/Utils.h"
 
 #include <boost/bind.hpp>
 #include <boost/optional.hpp>
@@ -34,6 +33,9 @@ namespace librbd {
 namespace io {
 
 using librbd::util::data_object_name;
+using librbd::util::create_context_callback;
+using librbd::util::create_rados_callback;
+using librbd::util::create_trace;
 
 namespace {
 
@@ -102,7 +104,7 @@ ObjectRequest<I>::ObjectRequest(
     const ZTracer::Trace &trace, Context *completion)
   : m_ictx(ictx), m_object_no(objectno), m_object_off(off),
     m_object_len(len), m_snap_id(snap_id), m_completion(completion),
-    m_trace(util::create_trace(*ictx, "", trace)) {
+    m_trace(create_trace(*ictx, "", trace)) {
   ceph_assert(m_ictx->data_ctx.is_valid());
   if (m_trace.valid()) {
     m_trace.copy_name(trace_name + std::string(" ") +
@@ -165,7 +167,7 @@ bool ObjectRequest<I>::compute_parent_extents(Extents *parent_extents,
 template <typename I>
 void ObjectRequest<I>::async_finish(int r) {
   ldout(m_ictx->cct, 20) << "r=" << r << dendl;
-  m_ictx->op_work_queue->queue(util::create_context_callback<
+  m_ictx->op_work_queue->queue(create_context_callback<
     ObjectRequest<I>, &ObjectRequest<I>::finish>(this), r);
 }
 
@@ -221,7 +223,7 @@ void ObjectReadRequest<I>::read_object() {
   }
   op.set_op_flags2(m_op_flags);
 
-  librados::AioCompletion *rados_completion = util::create_rados_callback<
+  librados::AioCompletion *rados_completion = create_rados_callback<
     ObjectReadRequest<I>, &ObjectReadRequest<I>::handle_read_object>(this);
   int flags = image_ctx->get_read_flags(this->m_snap_id);
   int r = image_ctx->data_ctx.aio_operate(
@@ -254,38 +256,14 @@ void ObjectReadRequest<I>::handle_read_object(int r) {
 template <typename I>
 void ObjectReadRequest<I>::read_parent() {
   I *image_ctx = this->m_ictx;
-
-  std::shared_lock image_locker{image_ctx->image_lock};
-
-  // calculate reverse mapping onto the image
-  Extents parent_extents;
-  Striper::extent_to_file(image_ctx->cct, &image_ctx->layout,
-                          this->m_object_no, this->m_object_off,
-                          this->m_object_len, parent_extents);
-
-  uint64_t parent_overlap = 0;
-  uint64_t object_overlap = 0;
-  int r = image_ctx->get_parent_overlap(this->m_snap_id, &parent_overlap);
-  if (r == 0) {
-    object_overlap = image_ctx->prune_parent_extents(parent_extents,
-                                                     parent_overlap);
-  }
-
-  if (object_overlap == 0) {
-    image_locker.unlock();
-
-    this->finish(-ENOENT);
-    return;
-  }
-
   ldout(image_ctx->cct, 20) << dendl;
 
-  auto parent_completion = AioCompletion::create_and_start<
-    ObjectReadRequest<I>, &ObjectReadRequest<I>::handle_read_parent>(
-      this, util::get_image_ctx(image_ctx->parent), AIO_TYPE_READ);
-  ImageRequest<I>::aio_read(image_ctx->parent, parent_completion,
-                            std::move(parent_extents), ReadResult{m_read_data},
-                            0, this->m_trace);
+  auto ctx = create_context_callback<
+    ObjectReadRequest<I>, &ObjectReadRequest<I>::handle_read_parent>(this);
+
+  io::util::read_parent<I>(image_ctx, this->m_object_no, this->m_object_off,
+                           this->m_object_len, this->m_snap_id, this->m_trace,
+                           m_read_data, ctx);
 }
 
 template <typename I>
@@ -495,7 +473,7 @@ void AbstractObjectWriteRequest<I>::write_object() {
   add_write_ops(&write);
   ceph_assert(write.size() != 0);
 
-  librados::AioCompletion *rados_completion = util::create_rados_callback<
+  librados::AioCompletion *rados_completion = create_rados_callback<
     AbstractObjectWriteRequest<I>,
     &AbstractObjectWriteRequest<I>::handle_write_object>(this);
   int r = image_ctx->data_ctx.aio_operate(
index bf06663388c8e710cb315500db0fa79df9855cfb..92394e4f7aa3536d31e799f536f420de8a70b355 100644 (file)
@@ -2,8 +2,17 @@
 // vim: ts=8 sw=2 smarttab
 
 #include "librbd/io/Utils.h"
+#include "common/dout.h"
 #include "include/buffer.h"
+#include "include/rados/librados.hpp"
+#include "librbd/io/AioCompletion.h"
+#include "librbd/io/ImageRequest.h"
 #include "osd/osd_types.h"
+#include "osdc/Striper.h"
+
+#define dout_subsys ceph_subsys_rbd
+#undef dout_prefix
+#define dout_prefix *_dout << "librbd::io::util: " << __func__ << ": "
 
 namespace librbd {
 namespace io {
@@ -51,7 +60,52 @@ bool assemble_write_same_extent(
   return false;
 }
 
+template <typename I>
+void read_parent(I *image_ctx, uint64_t object_no, uint64_t off,
+                 uint64_t len, librados::snap_t snap_id,
+                 const ZTracer::Trace &trace, ceph::bufferlist* data,
+                 Context* on_finish) {
+
+  auto cct = image_ctx->cct;
+
+  std::shared_lock image_locker{image_ctx->image_lock};
+
+  // calculate reverse mapping onto the image
+  Extents parent_extents;
+  Striper::extent_to_file(cct, &image_ctx->layout, object_no, off, len,
+                          parent_extents);
+
+  uint64_t parent_overlap = 0;
+  uint64_t object_overlap = 0;
+  int r = image_ctx->get_parent_overlap(snap_id, &parent_overlap);
+  if (r == 0) {
+    object_overlap = image_ctx->prune_parent_extents(parent_extents,
+                                                     parent_overlap);
+  }
+
+  if (object_overlap == 0) {
+    image_locker.unlock();
+
+    on_finish->complete(-ENOENT);
+    return;
+  }
+
+  ldout(cct, 20) << dendl;
+
+  auto comp = AioCompletion::create_and_start(on_finish, image_ctx->parent,
+                                              AIO_TYPE_READ);
+  ldout(cct, 20) << "completion " << comp << ", extents " << parent_extents
+                 << dendl;
+
+  ImageRequest<I>::aio_read(image_ctx->parent, comp, std::move(parent_extents),
+                            ReadResult{data}, 0, trace);
+}
+
 } // namespace util
 } // namespace io
 } // namespace librbd
 
+template void librbd::io::util::read_parent(
+    librbd::ImageCtx *image_ctx, uint64_t object_no, uint64_t off, uint64_t len,
+    librados::snap_t snap_id, const ZTracer::Trace &trace,
+    ceph::bufferlist* data, Context* on_finish);
index 285985036de280e44bdb090f41c9fa70fb7d7026..13237435a1569e980a9388f5af68142c81b951aa 100644 (file)
@@ -6,12 +6,18 @@
 
 #include "include/int_types.h"
 #include "include/buffer_fwd.h"
+#include "include/rados/rados_types.hpp"
+#include "common/zipkin_trace.h"
 #include "librbd/io/Types.h"
 #include <map>
 
+class Context;
 class ObjectExtent;
 
 namespace librbd {
+
+struct ImageCtx;
+
 namespace io {
 namespace util {
 
@@ -20,6 +26,12 @@ bool assemble_write_same_extent(const LightweightObjectExtent &object_extent,
                                 ceph::bufferlist *ws_data,
                                 bool force_write);
 
+template <typename ImageCtxT = librbd::ImageCtx>
+void read_parent(ImageCtxT *image_ctx, uint64_t object_no, uint64_t off,
+                 uint64_t len, librados::snap_t snap_id,
+                 const ZTracer::Trace &trace, ceph::bufferlist* data,
+                 Context* on_finish);
+
 } // namespace util
 } // namespace io
 } // namespace librbd
index b963d737704da4547c7eab00c39b92ea2ca6bcd3..e6fd5b2ee03cd822e7e3b30b28a34478a449175d 100644 (file)
@@ -10,8 +10,8 @@
 #include "test/librados_test_stub/MockTestMemRadosClient.h"
 #include "include/rbd/librbd.hpp"
 #include "librbd/io/CopyupRequest.h"
-#include "librbd/io/ImageRequest.h"
 #include "librbd/io/ObjectRequest.h"
+#include "librbd/io/Utils.h"
 
 namespace librbd {
 namespace {
@@ -53,24 +53,38 @@ struct CopyupRequest<librbd::MockTestImageCtx> : public CopyupRequest<librbd::Mo
   }
 };
 
-template <>
-struct ImageRequest<librbd::MockTestImageCtx> {
-  static ImageRequest *s_instance;
-  static void aio_read(librbd::MockImageCtx *ictx, AioCompletion *c,
-                       Extents &&image_extents, ReadResult &&read_result,
-                       int op_flags, const ZTracer::Trace &parent_trace) {
-    s_instance->aio_read(c, image_extents);
-  }
+CopyupRequest<librbd::MockTestImageCtx>* CopyupRequest<librbd::MockTestImageCtx>::s_instance = nullptr;
 
-  MOCK_METHOD2(aio_read, void(AioCompletion *, const Extents&));
+namespace util {
+
+namespace {
 
-  ImageRequest() {
+struct Mock {
+  static Mock* s_instance;
+
+  Mock() {
     s_instance = this;
   }
+
+  MOCK_METHOD8(read_parent,
+               void(librbd::MockTestImageCtx *, uint64_t, uint64_t, uint64_t,
+                    librados::snap_t, const ZTracer::Trace &, ceph::bufferlist*,
+                    Context*));
 };
 
-CopyupRequest<librbd::MockTestImageCtx>* CopyupRequest<librbd::MockTestImageCtx>::s_instance = nullptr;
-ImageRequest<librbd::MockTestImageCtx>* ImageRequest<librbd::MockTestImageCtx>::s_instance = nullptr;
+Mock *Mock::s_instance = nullptr;
+
+} // anonymous namespace
+
+template<> void read_parent(
+    librbd::MockTestImageCtx *image_ctx, uint64_t object_no, uint64_t off,
+    uint64_t len, librados::snap_t snap_id, const ZTracer::Trace &trace,
+    ceph::bufferlist* data, Context* on_finish) {
+  Mock::s_instance->read_parent(image_ctx, object_no, off, len, snap_id, trace,
+                                data, on_finish);
+}
+
+} // namespace util
 
 } // namespace io
 } // namespace librbd
@@ -97,7 +111,7 @@ struct TestMockIoObjectRequest : public TestMockFixture {
   typedef ObjectCompareAndWriteRequest<librbd::MockTestImageCtx> MockObjectCompareAndWriteRequest;
   typedef AbstractObjectWriteRequest<librbd::MockTestImageCtx> MockAbstractObjectWriteRequest;
   typedef CopyupRequest<librbd::MockTestImageCtx> MockCopyupRequest;
-  typedef ImageRequest<librbd::MockTestImageCtx> MockImageRequest;
+  typedef util::Mock MockUtils;
 
   void expect_object_may_exist(MockTestImageCtx &mock_image_ctx,
                                uint64_t object_no, bool exists) {
@@ -179,14 +193,12 @@ struct TestMockIoObjectRequest : public TestMockFixture {
     }
   }
 
-  void expect_aio_read(MockTestImageCtx &mock_image_ctx,
-                       MockImageRequest& mock_image_request,
-                       Extents&& extents, int r) {
-    EXPECT_CALL(mock_image_request, aio_read(_, extents))
-      .WillOnce(WithArg<0>(Invoke([&mock_image_ctx, r](AioCompletion* aio_comp) {
-                             aio_comp->set_request_count(1);
-                             mock_image_ctx.image_ctx->op_work_queue->queue(new C_AioRequest(aio_comp), r);
-                           })));
+  void expect_read_parent(MockUtils &mock_utils, uint64_t object_no,
+                          uint64_t off, uint64_t len, librados::snap_t snap_id,
+                          int r) {
+    EXPECT_CALL(mock_utils,
+                read_parent(_, object_no, off, len, snap_id, _, _, _))
+      .WillOnce(WithArg<7>(CompleteContext(r, static_cast<ContextWQ*>(nullptr))));
   }
 
   void expect_copyup(MockCopyupRequest& mock_copyup_request, int r) {
@@ -423,10 +435,8 @@ TEST_F(TestMockIoObjectRequest, ParentRead) {
   expect_get_read_flags(mock_image_ctx, CEPH_NOSNAP, 0);
   expect_read(mock_image_ctx, ictx->get_object_name(0), 0, 4096, "", -ENOENT);
 
-  MockImageRequest mock_image_request;
-  expect_get_parent_overlap(mock_image_ctx, CEPH_NOSNAP, 4096, 0);
-  expect_prune_parent_extents(mock_image_ctx, {{0, 4096}}, 4096, 4096);
-  expect_aio_read(mock_image_ctx, mock_image_request, {{0, 4096}}, 0);
+  MockUtils mock_utils;
+  expect_read_parent(mock_utils, 0, 0, 4096, CEPH_NOSNAP, 0);
 
   bufferlist bl;
   ExtentMap extent_map;
@@ -470,10 +480,8 @@ TEST_F(TestMockIoObjectRequest, ParentReadError) {
   expect_get_read_flags(mock_image_ctx, CEPH_NOSNAP, 0);
   expect_read(mock_image_ctx, ictx->get_object_name(0), 0, 4096, "", -ENOENT);
 
-  MockImageRequest mock_image_request;
-  expect_get_parent_overlap(mock_image_ctx, CEPH_NOSNAP, 4096, 0);
-  expect_prune_parent_extents(mock_image_ctx, {{0, 4096}}, 4096, 4096);
-  expect_aio_read(mock_image_ctx, mock_image_request, {{0, 4096}}, -EPERM);
+  MockUtils mock_utils;
+  expect_read_parent(mock_utils, 0, 0, 4096, CEPH_NOSNAP, -EPERM);
 
   bufferlist bl;
   ExtentMap extent_map;
@@ -517,10 +525,8 @@ TEST_F(TestMockIoObjectRequest, CopyOnRead) {
   expect_get_read_flags(mock_image_ctx, CEPH_NOSNAP, 0);
   expect_read(mock_image_ctx, ictx->get_object_name(0), 0, 4096, "", -ENOENT);
 
-  MockImageRequest mock_image_request;
-  expect_get_parent_overlap(mock_image_ctx, CEPH_NOSNAP, 4096, 0);
-  expect_prune_parent_extents(mock_image_ctx, {{0, 4096}}, 4096, 4096);
-  expect_aio_read(mock_image_ctx, mock_image_request, {{0, 4096}}, 0);
+  MockUtils mock_utils;
+  expect_read_parent(mock_utils, 0, 0, 4096, CEPH_NOSNAP, 0);
 
   MockCopyupRequest mock_copyup_request;
   expect_get_parent_overlap(mock_image_ctx, CEPH_NOSNAP, 4096, 0);