From: Mykola Golub Date: Mon, 20 Jul 2020 09:53:57 +0000 (+0100) Subject: librbd: make read_parent reusable X-Git-Tag: v15.2.9~122^2~92^2~4 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=0ff49db4738d2ce2604ed1d3b2882ee4b9316a00;p=ceph.git librbd: make read_parent reusable Signed-off-by: Mykola Golub (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 --- diff --git a/src/librbd/io/ImageRequest.h b/src/librbd/io/ImageRequest.h index e5a04597d3e7..0fe10f2c9fe4 100644 --- a/src/librbd/io/ImageRequest.h +++ b/src/librbd/io/ImageRequest.h @@ -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"); } diff --git a/src/librbd/io/ObjectRequest.cc b/src/librbd/io/ObjectRequest.cc index 3966dd2e725b..ffd21dfc617e 100644 --- a/src/librbd/io/ObjectRequest.cc +++ b/src/librbd/io/ObjectRequest.cc @@ -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 #include @@ -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::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::compute_parent_extents(Extents *parent_extents, template void ObjectRequest::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, &ObjectRequest::finish>(this), r); } @@ -221,7 +223,7 @@ void ObjectReadRequest::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, &ObjectReadRequest::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::handle_read_object(int r) { template void ObjectReadRequest::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, &ObjectReadRequest::handle_read_parent>( - this, util::get_image_ctx(image_ctx->parent), AIO_TYPE_READ); - ImageRequest::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, &ObjectReadRequest::handle_read_parent>(this); + + io::util::read_parent(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 @@ -495,7 +473,7 @@ void AbstractObjectWriteRequest::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, &AbstractObjectWriteRequest::handle_write_object>(this); int r = image_ctx->data_ctx.aio_operate( diff --git a/src/librbd/io/Utils.cc b/src/librbd/io/Utils.cc index bf06663388c8..92394e4f7aa3 100644 --- a/src/librbd/io/Utils.cc +++ b/src/librbd/io/Utils.cc @@ -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 +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::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); diff --git a/src/librbd/io/Utils.h b/src/librbd/io/Utils.h index 285985036de2..13237435a156 100644 --- a/src/librbd/io/Utils.h +++ b/src/librbd/io/Utils.h @@ -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 +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 +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 diff --git a/src/test/librbd/io/test_mock_ObjectRequest.cc b/src/test/librbd/io/test_mock_ObjectRequest.cc index b963d737704d..e6fd5b2ee03c 100644 --- a/src/test/librbd/io/test_mock_ObjectRequest.cc +++ b/src/test/librbd/io/test_mock_ObjectRequest.cc @@ -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 : public CopyupRequest -struct ImageRequest { - 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* CopyupRequest::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* CopyupRequest::s_instance = nullptr; -ImageRequest* ImageRequest::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 MockObjectCompareAndWriteRequest; typedef AbstractObjectWriteRequest MockAbstractObjectWriteRequest; typedef CopyupRequest MockCopyupRequest; - typedef ImageRequest 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(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);