From 4f36d3e936e9d1e94a4821f3fc5e407dec20695d Mon Sep 17 00:00:00 2001 From: Or Ozeri Date: Wed, 16 Sep 2020 11:42:45 +0300 Subject: [PATCH] librbd: support assembling sparse results of multiple object extents Currently, Striper supports assembling results representing a single object extent. Recently, the object dispatch API was extended allowing to read multiple object extents per rados operation. This commit enables the Striper to correctly un-sparsify the results of the new read extents API. Signed-off-by: Or Ozeri --- .../cache/ObjectCacherObjectDispatch.cc | 45 ++++++--- src/librbd/cache/ObjectCacherObjectDispatch.h | 3 +- src/librbd/cache/ObjectCacherWriteback.cc | 7 +- src/librbd/cache/ParentCacheObjectDispatch.cc | 63 ++++++------ src/librbd/cache/ParentCacheObjectDispatch.h | 8 +- src/librbd/cache/WriteAroundObjectDispatch.cc | 17 ++-- src/librbd/cache/WriteAroundObjectDispatch.h | 3 +- src/librbd/crypto/CryptoObjectDispatch.cc | 76 +++++++-------- src/librbd/crypto/CryptoObjectDispatch.h | 7 +- src/librbd/io/ImageRequest.cc | 21 ++-- src/librbd/io/ObjectDispatch.cc | 13 ++- src/librbd/io/ObjectDispatch.h | 3 +- src/librbd/io/ObjectDispatchInterface.h | 7 +- src/librbd/io/ObjectDispatchSpec.h | 19 ++-- src/librbd/io/ObjectDispatcher.cc | 3 +- src/librbd/io/ObjectRequest.cc | 56 +++-------- src/librbd/io/ObjectRequest.h | 24 ++--- src/librbd/io/ReadResult.cc | 49 +++++++--- src/librbd/io/ReadResult.h | 23 +++-- .../io/SimpleSchedulerObjectDispatch.cc | 12 +-- src/librbd/io/SimpleSchedulerObjectDispatch.h | 3 +- src/librbd/io/Types.h | 39 ++++++++ src/librbd/io/Utils.cc | 40 ++++++-- src/librbd/io/Utils.h | 10 +- src/librbd/journal/ObjectDispatch.h | 3 +- src/librbd/plugin/Api.cc | 6 +- src/librbd/plugin/Api.h | 4 +- src/osdc/Striper.cc | 14 ++- src/osdc/Striper.h | 2 +- .../test_mock_ParentCacheObjectDispatch.cc | 26 ++--- .../crypto/test_mock_CryptoObjectDispatch.cc | 18 ++-- src/test/librbd/io/test_mock_ObjectRequest.cc | 95 ++++++++----------- ...test_mock_SimpleSchedulerObjectDispatch.cc | 5 +- src/test/librbd/mock/io/MockObjectDispatch.h | 17 ++-- 34 files changed, 397 insertions(+), 344 deletions(-) diff --git a/src/librbd/cache/ObjectCacherObjectDispatch.cc b/src/librbd/cache/ObjectCacherObjectDispatch.cc index 92539478d764f..2fefb26af73b1 100644 --- a/src/librbd/cache/ObjectCacherObjectDispatch.cc +++ b/src/librbd/cache/ObjectCacherObjectDispatch.cc @@ -11,6 +11,7 @@ #include "librbd/cache/ObjectCacherWriteback.h" #include "librbd/io/ObjectDispatchSpec.h" #include "librbd/io/ObjectDispatcherInterface.h" +#include "librbd/io/ReadResult.h" #include "librbd/io/Types.h" #include "librbd/io/Utils.h" #include "osd/osd_types.h" @@ -182,21 +183,25 @@ void ObjectCacherObjectDispatch::shut_down(Context* on_finish) { template bool ObjectCacherObjectDispatch::read( - uint64_t object_no, const io::Extents &extents, IOContext io_context, + uint64_t object_no, io::ReadExtents* extents, IOContext io_context, int op_flags, int read_flags, const ZTracer::Trace &parent_trace, - ceph::bufferlist* read_data, io::Extents* extent_map, uint64_t* version, - int* object_dispatch_flags, io::DispatchResult* dispatch_result, - Context** on_finish, Context* on_dispatched) { + uint64_t* version, int* object_dispatch_flags, + io::DispatchResult* dispatch_result, Context** on_finish, + Context* on_dispatched) { // IO chained in reverse order auto cct = m_image_ctx->cct; - ldout(cct, 20) << "object_no=" << object_no << " " << extents << dendl; + ldout(cct, 20) << "object_no=" << object_no << " " << *extents << dendl; + + if (extents->size() == 0) { + ldout(cct, 20) << "no extents to read" << dendl; + return false; + } - if (version != nullptr || extents.size() != 1) { + if (version != nullptr) { // we currently don't cache read versions // and don't support reading more than one extent return false; } - auto [object_off, object_len] = extents.front(); // ensure we aren't holding the cache lock post-read on_dispatched = util::create_async_context_callback(*m_image_ctx, @@ -207,16 +212,30 @@ bool ObjectCacherObjectDispatch::read( ((read_flags << ObjectCacherWriteback::READ_FLAGS_SHIFT) & ObjectCacherWriteback::READ_FLAGS_MASK)); + ceph::bufferlist* bl; + if (extents->size() > 1) { + auto req = new io::ReadResult::C_ObjectReadMergedExtents( + cct, extents, on_dispatched); + on_dispatched = req; + bl = &req->bl; + } else { + bl = &extents->front().bl; + } + m_image_ctx->image_lock.lock_shared(); auto rd = m_object_cacher->prepare_read( - io_context->read_snap().value_or(CEPH_NOSNAP), read_data, op_flags); + io_context->read_snap().value_or(CEPH_NOSNAP), bl, op_flags); m_image_ctx->image_lock.unlock_shared(); - ObjectExtent extent(data_object_name(m_image_ctx, object_no), object_no, - object_off, object_len, 0); - extent.oloc.pool = m_image_ctx->data_ctx.get_id(); - extent.buffer_extents.push_back({0, object_len}); - rd->extents.push_back(extent); + uint64_t off = 0; + for (auto& read_extent: *extents) { + ObjectExtent extent(data_object_name(m_image_ctx, object_no), object_no, + read_extent.offset, read_extent.length, 0); + extent.oloc.pool = m_image_ctx->data_ctx.get_id(); + extent.buffer_extents.push_back({off, read_extent.length}); + rd->extents.push_back(extent); + off += read_extent.length; + } ZTracer::Trace trace(parent_trace); *dispatch_result = io::DISPATCH_RESULT_COMPLETE; diff --git a/src/librbd/cache/ObjectCacherObjectDispatch.h b/src/librbd/cache/ObjectCacherObjectDispatch.h index ca60b04b1d8fb..ab0561cfdd357 100644 --- a/src/librbd/cache/ObjectCacherObjectDispatch.h +++ b/src/librbd/cache/ObjectCacherObjectDispatch.h @@ -42,9 +42,8 @@ public: void shut_down(Context* on_finish) override; bool read( - uint64_t object_no, const io::Extents &extents, IOContext io_context, + uint64_t object_no, io::ReadExtents* extents, IOContext io_context, int op_flags, int read_flags, const ZTracer::Trace &parent_trace, - ceph::bufferlist* read_data, io::Extents* extent_map, uint64_t* version, int* object_dispatch_flags, io::DispatchResult* dispatch_result, Context** on_finish, Context* on_dispatched) override; diff --git a/src/librbd/cache/ObjectCacherWriteback.cc b/src/librbd/cache/ObjectCacherWriteback.cc index b99438a65d6b9..50655b6a111bd 100644 --- a/src/librbd/cache/ObjectCacherWriteback.cc +++ b/src/librbd/cache/ObjectCacherWriteback.cc @@ -135,7 +135,7 @@ void ObjectCacherWriteback::read(const object_t& oid, uint64_t object_no, aio_comp->set_request_count(1); auto req_comp = new io::ReadResult::C_ObjectReadRequest( - aio_comp, off, len, {{0, len}}); + aio_comp, {{off, len, {{0, len}}}}); auto io_context = m_ictx->duplicate_data_io_context(); if (snapid != CEPH_NOSNAP) { @@ -147,9 +147,8 @@ void ObjectCacherWriteback::read(const object_t& oid, uint64_t object_no, op_flags &= ~READ_FLAGS_MASK; auto req = io::ObjectDispatchSpec::create_read( - m_ictx, io::OBJECT_DISPATCH_LAYER_CACHE, object_no, {{off, len}}, - io_context, op_flags, read_flags, trace, &req_comp->bl, - &req_comp->extent_map, nullptr, req_comp); + m_ictx, io::OBJECT_DISPATCH_LAYER_CACHE, object_no, &req_comp->extents, + io_context, op_flags, read_flags, trace, nullptr, req_comp); req->send(); } diff --git a/src/librbd/cache/ParentCacheObjectDispatch.cc b/src/librbd/cache/ParentCacheObjectDispatch.cc index 6b81198b96137..843bc845fcd58 100644 --- a/src/librbd/cache/ParentCacheObjectDispatch.cc +++ b/src/librbd/cache/ParentCacheObjectDispatch.cc @@ -65,22 +65,19 @@ void ParentCacheObjectDispatch::init(Context* on_finish) { template bool ParentCacheObjectDispatch::read( - uint64_t object_no, const io::Extents &extents, IOContext io_context, + uint64_t object_no, io::ReadExtents* extents, IOContext io_context, int op_flags, int read_flags, const ZTracer::Trace &parent_trace, - ceph::bufferlist* read_data, io::Extents* extent_map, uint64_t* version, - int* object_dispatch_flags, io::DispatchResult* dispatch_result, - Context** on_finish, Context* on_dispatched) { + uint64_t* version, int* object_dispatch_flags, + io::DispatchResult* dispatch_result, Context** on_finish, + Context* on_dispatched) { auto cct = m_image_ctx->cct; - ldout(cct, 20) << "object_no=" << object_no << " " << extents << dendl; + ldout(cct, 20) << "object_no=" << object_no << " " << *extents << dendl; - if (version != nullptr || extents.size() != 1) { + if (version != nullptr) { // we currently don't cache read versions - // and don't support reading more than one extent return false; } - auto [object_off, object_len] = extents.front(); - string oid = data_object_name(m_image_ctx, object_no); /* if RO daemon still don't startup, or RO daemon crash, @@ -95,13 +92,11 @@ bool ParentCacheObjectDispatch::read( CacheGenContextURef ctx = make_gen_lambda_context> - ([this, read_data, dispatch_result, on_dispatched, object_no, - object_off = object_off, object_len = object_len, io_context, - &parent_trace] + ([this, extents, dispatch_result, on_dispatched, object_no, io_context, + &parent_trace] (ObjectCacheRequest* ack) { - handle_read_cache(ack, object_no, object_off, object_len, io_context, - parent_trace, read_data, dispatch_result, - on_dispatched); + handle_read_cache(ack, object_no, extents, io_context, parent_trace, + dispatch_result, on_dispatched); }); m_cache_client->lookup_object(m_image_ctx->data_ctx.get_namespace(), @@ -113,9 +108,8 @@ bool ParentCacheObjectDispatch::read( template void ParentCacheObjectDispatch::handle_read_cache( - ObjectCacheRequest* ack, uint64_t object_no, uint64_t read_off, - uint64_t read_len, IOContext io_context, - const ZTracer::Trace &parent_trace, ceph::bufferlist* read_data, + ObjectCacheRequest* ack, uint64_t object_no, io::ReadExtents* extents, + IOContext io_context, const ZTracer::Trace &parent_trace, io::DispatchResult* dispatch_result, Context* on_dispatched) { auto cct = m_image_ctx->cct; ldout(cct, 20) << dendl; @@ -139,23 +133,36 @@ void ParentCacheObjectDispatch::handle_read_cache( *dispatch_result = io::DISPATCH_RESULT_COMPLETE; on_dispatched->complete(r); }); - m_plugin_api.read_parent(m_image_ctx, object_no, {{read_off, read_len}}, + m_plugin_api.read_parent(m_image_ctx, object_no, extents, io_context->read_snap().value_or(CEPH_NOSNAP), - parent_trace, read_data, ctx); + parent_trace, ctx); return; } - // try to read from parent image cache - int r = read_object(file_path, read_data, read_off, read_len, on_dispatched); - if(r < 0) { - // cache read error, fall back to read rados - *dispatch_result = io::DISPATCH_RESULT_CONTINUE; - on_dispatched->complete(0); - return; + int read_len = 0; + for (auto& extent: *extents) { + // try to read from parent image cache + int r = read_object(file_path, &extent.bl, extent.offset, extent.length, + on_dispatched); + if (r < 0) { + // cache read error, fall back to read rados + for (auto& read_extent: *extents) { + // clear read bufferlists + if (&read_extent == &extent) { + break; + } + read_extent.bl.clear(); + } + *dispatch_result = io::DISPATCH_RESULT_CONTINUE; + on_dispatched->complete(0); + return; + } + + read_len += r; } *dispatch_result = io::DISPATCH_RESULT_COMPLETE; - on_dispatched->complete(r); + on_dispatched->complete(read_len); } template diff --git a/src/librbd/cache/ParentCacheObjectDispatch.h b/src/librbd/cache/ParentCacheObjectDispatch.h index 910c5e32b15a1..6fbe44a447e78 100644 --- a/src/librbd/cache/ParentCacheObjectDispatch.h +++ b/src/librbd/cache/ParentCacheObjectDispatch.h @@ -44,9 +44,8 @@ public: } bool read( - uint64_t object_no, const io::Extents &extents, IOContext io_context, + uint64_t object_no, io::ReadExtents* extents, IOContext io_context, int op_flags, int read_flags, const ZTracer::Trace &parent_trace, - ceph::bufferlist* read_data, io::Extents* extent_map, uint64_t* version, int* object_dispatch_flags, io::DispatchResult* dispatch_result, Context** on_finish, Context* on_dispatched) override; @@ -132,10 +131,9 @@ private: int read_object(std::string file_path, ceph::bufferlist* read_data, uint64_t offset, uint64_t length, Context *on_finish); void handle_read_cache(ceph::immutable_obj_cache::ObjectCacheRequest* ack, - uint64_t object_no, uint64_t read_off, - uint64_t read_len, IOContext io_context, + uint64_t object_no, io::ReadExtents* extents, + IOContext io_context, const ZTracer::Trace &parent_trace, - ceph::bufferlist* read_data, io::DispatchResult* dispatch_result, Context* on_dispatched); int handle_register_client(bool reg); diff --git a/src/librbd/cache/WriteAroundObjectDispatch.cc b/src/librbd/cache/WriteAroundObjectDispatch.cc index e202b08b6c068..adffeacfb0fa3 100644 --- a/src/librbd/cache/WriteAroundObjectDispatch.cc +++ b/src/librbd/cache/WriteAroundObjectDispatch.cc @@ -57,14 +57,17 @@ void WriteAroundObjectDispatch::shut_down(Context* on_finish) { template bool WriteAroundObjectDispatch::read( - uint64_t object_no, const io::Extents &extents, IOContext io_context, + uint64_t object_no, io::ReadExtents* extents, IOContext io_context, int op_flags, int read_flags, const ZTracer::Trace &parent_trace, - ceph::bufferlist* read_data, io::Extents* extent_map, uint64_t* version, - int* object_dispatch_flags, io::DispatchResult* dispatch_result, - Context** on_finish, Context* on_dispatched) { - auto [object_off, object_len] = extents.front(); - return dispatch_unoptimized_io(object_no, object_off, object_len, - dispatch_result, on_dispatched); + uint64_t* version, int* object_dispatch_flags, + io::DispatchResult* dispatch_result, Context** on_finish, + Context* on_dispatched) { + bool handled = false; + for (auto& extent: *extents) { + handled |= dispatch_unoptimized_io(object_no, extent.offset, extent.length, + dispatch_result, on_dispatched); + } + return handled; } template diff --git a/src/librbd/cache/WriteAroundObjectDispatch.h b/src/librbd/cache/WriteAroundObjectDispatch.h index 8086ddc546b58..53bb902e74025 100644 --- a/src/librbd/cache/WriteAroundObjectDispatch.h +++ b/src/librbd/cache/WriteAroundObjectDispatch.h @@ -42,9 +42,8 @@ public: void shut_down(Context* on_finish) override; bool read( - uint64_t object_no, const io::Extents &extents, IOContext io_context, + uint64_t object_no, io::ReadExtents* extents, IOContext io_context, int op_flags, int read_flags, const ZTracer::Trace &parent_trace, - ceph::bufferlist* read_data, io::Extents* extent_map, uint64_t* version, int* object_dispatch_flags, io::DispatchResult* dispatch_result, Context** on_finish, Context* on_dispatched) override; diff --git a/src/librbd/crypto/CryptoObjectDispatch.cc b/src/librbd/crypto/CryptoObjectDispatch.cc index 57d761811c04e..c5c02a372bf27 100644 --- a/src/librbd/crypto/CryptoObjectDispatch.cc +++ b/src/librbd/crypto/CryptoObjectDispatch.cc @@ -29,41 +29,28 @@ struct C_EncryptedObjectReadRequest : public Context { I* image_ctx; CryptoInterface* crypto; uint64_t object_no; - uint64_t object_off; - ceph::bufferlist* read_data; + io::ReadExtents* extents; Context* onfinish; - io::ReadResult::C_ObjectReadRequest* req_comp; io::ObjectDispatchSpec* req; C_EncryptedObjectReadRequest( I* image_ctx, CryptoInterface* crypto, uint64_t object_no, - uint64_t object_off, uint64_t object_len, IOContext io_context, + io::ReadExtents* extents, IOContext io_context, int op_flags, int read_flags, const ZTracer::Trace &parent_trace, - ceph::bufferlist* read_data, int* object_dispatch_flags, + uint64_t* version, int* object_dispatch_flags, Context** on_finish, Context* on_dispatched) : image_ctx(image_ctx), crypto(crypto), object_no(object_no), - object_off(object_off), - read_data(read_data), + extents(extents), onfinish(on_dispatched) { *on_finish = util::create_context_callback< Context, &Context::complete>(*on_finish, crypto); - auto aio_comp = io::AioCompletion::create_and_start( - (Context*)this, util::get_image_ctx(image_ctx), - io::AIO_TYPE_READ); - aio_comp->read_result = io::ReadResult{read_data}; - aio_comp->set_request_count(1); - - auto req_comp = new io::ReadResult::C_ObjectReadRequest( - aio_comp, object_off, object_len, {{0, object_len}}); - req = io::ObjectDispatchSpec::create_read( image_ctx, io::OBJECT_DISPATCH_LAYER_CRYPTO, object_no, - {{object_off, object_len}}, io_context, op_flags, read_flags, - parent_trace, &req_comp->bl, &req_comp->extent_map, nullptr, - req_comp); + extents, io_context, op_flags, read_flags, parent_trace, + version, this); } void send() { @@ -71,16 +58,25 @@ struct C_EncryptedObjectReadRequest : public Context { } void finish(int r) override { - ldout(image_ctx->cct, 20) << "r=" << r << dendl; - if (r > 0) { - auto crypto_ret = crypto->decrypt( - read_data, - Striper::get_file_offset( - image_ctx->cct, &image_ctx->layout, object_no, - object_off)); - if (crypto_ret != 0) { - ceph_assert(crypto_ret < 0); - r = crypto_ret; + auto cct = image_ctx->cct; + ldout(cct, 20) << "r=" << r << dendl; + if (r == 0) { + for (auto& extent: *extents) { + io::util::unsparsify(cct, &extent.bl, extent.extent_map, + extent.offset, extent.length); + + auto crypto_ret = crypto->decrypt( + &extent.bl, + Striper::get_file_offset( + cct, &image_ctx->layout, object_no, + extent.offset)); + if (crypto_ret != 0) { + ceph_assert(crypto_ret < 0); + r = crypto_ret; + break; + } + + r += extent.length; } } onfinish->complete(r); @@ -116,28 +112,20 @@ void CryptoObjectDispatch::shut_down(Context* on_finish) { template bool CryptoObjectDispatch::read( - uint64_t object_no, const io::Extents &extents, IOContext io_context, + uint64_t object_no, io::ReadExtents* extents, IOContext io_context, int op_flags, int read_flags, const ZTracer::Trace &parent_trace, - ceph::bufferlist* read_data, io::Extents* extent_map, uint64_t* version, - int* object_dispatch_flags, io::DispatchResult* dispatch_result, - Context** on_finish, Context* on_dispatched) { + uint64_t* version, int* object_dispatch_flags, + io::DispatchResult* dispatch_result, Context** on_finish, + Context* on_dispatched) { auto cct = m_image_ctx->cct; ldout(cct, 20) << data_object_name(m_image_ctx, object_no) << " " - << extents << dendl; + << *extents << dendl; ceph_assert(m_crypto != nullptr); - if (version != nullptr || extents.size() != 1) { - // there's currently no need to support multiple extents - // as well as returning object version - return false; - } - - auto [object_off, object_len] = extents.front(); - *dispatch_result = io::DISPATCH_RESULT_COMPLETE; auto req = new C_EncryptedObjectReadRequest( - m_image_ctx, m_crypto, object_no, object_off, object_len, io_context, - op_flags, read_flags, parent_trace, read_data, object_dispatch_flags, + m_image_ctx, m_crypto, object_no, extents, io_context, + op_flags, read_flags, parent_trace, version, object_dispatch_flags, on_finish, on_dispatched); req->send(); return true; diff --git a/src/librbd/crypto/CryptoObjectDispatch.h b/src/librbd/crypto/CryptoObjectDispatch.h index 68f1acf48d06f..aad11af641a85 100644 --- a/src/librbd/crypto/CryptoObjectDispatch.h +++ b/src/librbd/crypto/CryptoObjectDispatch.h @@ -32,10 +32,9 @@ public: void shut_down(Context* on_finish) override; bool read( - uint64_t object_no, const io::Extents &extents, - IOContext io_context, int op_flags, int read_flags, - const ZTracer::Trace &parent_trace, ceph::bufferlist* read_data, - io::Extents* extent_map, uint64_t* version, int* object_dispatch_flags, + uint64_t object_no, io::ReadExtents* extents, IOContext io_context, + int op_flags, int read_flags, const ZTracer::Trace &parent_trace, + uint64_t* version, int* object_dispatch_flags, io::DispatchResult* dispatch_result, Context** on_finish, Context* on_dispatched) override; diff --git a/src/librbd/io/ImageRequest.cc b/src/librbd/io/ImageRequest.cc index 3b109da0d2686..6953d21fad0de 100644 --- a/src/librbd/io/ImageRequest.cc +++ b/src/librbd/io/ImageRequest.cc @@ -114,21 +114,19 @@ template struct C_RBD_Readahead : public Context { I *ictx; uint64_t object_no; - uint64_t offset; - uint64_t length; - - bufferlist read_data; - io::Extents extent_map; + io::ReadExtents extents; C_RBD_Readahead(I *ictx, uint64_t object_no, uint64_t offset, uint64_t length) - : ictx(ictx), object_no(object_no), offset(offset), length(length) { + : ictx(ictx), object_no(object_no), extents({{offset, length}}) { ictx->readahead.inc_pending(); } void finish(int r) override { + ceph_assert(extents.size() == 1); + auto& extent = extents.front(); ldout(ictx->cct, 20) << "C_RBD_Readahead on " << data_object_name(ictx, object_no) << ": " - << offset << "~" << length << dendl; + << extent.offset << "~" << extent.length << dendl; ictx->readahead.dec_pending(); } }; @@ -176,8 +174,7 @@ void readahead(I *ictx, const Extents& image_extents, IOContext io_context) { object_extent.length); auto req = io::ObjectDispatchSpec::create_read( ictx, io::OBJECT_DISPATCH_LAYER_NONE, object_extent.object_no, - {{object_extent.offset, object_extent.length}}, io_context, 0, 0, {}, - &req_comp->read_data, &req_comp->extent_map, nullptr, req_comp); + &req_comp->extents, io_context, 0, 0, {}, nullptr, req_comp); req->send(); } @@ -417,11 +414,11 @@ void ImageReadRequest::send_request() { << oe.buffer_extents << dendl; auto req_comp = new io::ReadResult::C_ObjectReadRequest( - aio_comp, oe.offset, oe.length, std::move(oe.buffer_extents)); + aio_comp, {{oe.offset, oe.length, std::move(oe.buffer_extents)}}); auto req = ObjectDispatchSpec::create_read( &image_ctx, OBJECT_DISPATCH_LAYER_NONE, oe.object_no, - {{oe.offset, oe.length}}, this->m_io_context, m_op_flags, m_read_flags, - this->m_trace, &req_comp->bl, &req_comp->extent_map, nullptr, req_comp); + &req_comp->extents, this->m_io_context, m_op_flags, m_read_flags, + this->m_trace, nullptr, req_comp); req->send(); } diff --git a/src/librbd/io/ObjectDispatch.cc b/src/librbd/io/ObjectDispatch.cc index fde16482b5c37..a31cc74eac36a 100644 --- a/src/librbd/io/ObjectDispatch.cc +++ b/src/librbd/io/ObjectDispatch.cc @@ -33,19 +33,18 @@ void ObjectDispatch::shut_down(Context* on_finish) { template bool ObjectDispatch::read( - uint64_t object_no, const Extents &extents, IOContext io_context, + uint64_t object_no, ReadExtents* extents, IOContext io_context, int op_flags, int read_flags, const ZTracer::Trace &parent_trace, - ceph::bufferlist* read_data, Extents* extent_map, uint64_t* version, - int* object_dispatch_flags, DispatchResult* dispatch_result, - Context** on_finish, Context* on_dispatched) { + uint64_t* version, int* object_dispatch_flags, + DispatchResult* dispatch_result, Context** on_finish, + Context* on_dispatched) { auto cct = m_image_ctx->cct; - ldout(cct, 20) << "object_no=" << object_no << " " << extents << dendl; + ldout(cct, 20) << "object_no=" << object_no << " " << *extents << dendl; *dispatch_result = DISPATCH_RESULT_COMPLETE; auto req = new ObjectReadRequest(m_image_ctx, object_no, extents, io_context, op_flags, read_flags, - parent_trace, read_data, extent_map, - version, on_dispatched); + parent_trace, version, on_dispatched); req->send(); return true; } diff --git a/src/librbd/io/ObjectDispatch.h b/src/librbd/io/ObjectDispatch.h index 3d7f9eba9a335..a45bb9a0ff320 100644 --- a/src/librbd/io/ObjectDispatch.h +++ b/src/librbd/io/ObjectDispatch.h @@ -33,9 +33,8 @@ public: void shut_down(Context* on_finish) override; bool read( - uint64_t object_no, const Extents &extents, IOContext io_context, + uint64_t object_no, ReadExtents* extents, IOContext io_context, int op_flags, int read_flags, const ZTracer::Trace &parent_trace, - ceph::bufferlist* read_data, Extents* extent_map, uint64_t* version, int* object_dispatch_flags, DispatchResult* dispatch_result, Context** on_finish, Context* on_dispatched) override; diff --git a/src/librbd/io/ObjectDispatchInterface.h b/src/librbd/io/ObjectDispatchInterface.h index 92fd13de7f15f..c61dc533ad64b 100644 --- a/src/librbd/io/ObjectDispatchInterface.h +++ b/src/librbd/io/ObjectDispatchInterface.h @@ -34,10 +34,9 @@ struct ObjectDispatchInterface { virtual void shut_down(Context* on_finish) = 0; virtual bool read( - uint64_t object_no, const Extents &extents, - IOContext io_context, int op_flags, int read_flags, - const ZTracer::Trace &parent_trace, ceph::bufferlist* read_data, - Extents* extent_map, uint64_t* version, int* object_dispatch_flags, + uint64_t object_no, ReadExtents* extents, IOContext io_context, + int op_flags, int read_flags, const ZTracer::Trace &parent_trace, + uint64_t* version, int* object_dispatch_flags, DispatchResult* dispatch_result, Context** on_finish, Context* on_dispatched) = 0; diff --git a/src/librbd/io/ObjectDispatchSpec.h b/src/librbd/io/ObjectDispatchSpec.h index c9daae6c08282..a0d4b49a46345 100644 --- a/src/librbd/io/ObjectDispatchSpec.h +++ b/src/librbd/io/ObjectDispatchSpec.h @@ -43,17 +43,14 @@ public: }; struct ReadRequest : public RequestBase { - const Extents extents; + ReadExtents* extents; int read_flags; - ceph::bufferlist* read_data; - Extents* extent_map; uint64_t* version; - ReadRequest(uint64_t object_no, const Extents &extents, - int read_flags, ceph::bufferlist* read_data, - Extents* extent_map, uint64_t* version) + ReadRequest(uint64_t object_no, ReadExtents* extents, int read_flags, + uint64_t* version) : RequestBase(object_no), extents(extents), read_flags(read_flags), - read_data(read_data), extent_map(extent_map), version(version) { + version(version) { } }; @@ -170,15 +167,13 @@ public: template static ObjectDispatchSpec* create_read( ImageCtxT* image_ctx, ObjectDispatchLayer object_dispatch_layer, - uint64_t object_no, const Extents &extents, IOContext io_context, + uint64_t object_no, ReadExtents* extents, IOContext io_context, int op_flags, int read_flags, const ZTracer::Trace &parent_trace, - ceph::bufferlist* read_data, Extents* extent_map, uint64_t* version, - Context* on_finish) { + uint64_t* version, Context* on_finish) { return new ObjectDispatchSpec(image_ctx->io_object_dispatcher, object_dispatch_layer, ReadRequest{object_no, extents, - read_flags, read_data, extent_map, - version}, + read_flags, version}, io_context, op_flags, parent_trace, on_finish); } diff --git a/src/librbd/io/ObjectDispatcher.cc b/src/librbd/io/ObjectDispatcher.cc index 432573c132be6..52500275c0856 100644 --- a/src/librbd/io/ObjectDispatcher.cc +++ b/src/librbd/io/ObjectDispatcher.cc @@ -109,8 +109,7 @@ struct ObjectDispatcher::SendVisitor : public boost::static_visitor { return object_dispatch->read( read.object_no, read.extents, object_dispatch_spec->io_context, object_dispatch_spec->op_flags, read.read_flags, - object_dispatch_spec->parent_trace, - read.read_data, read.extent_map, read.version, + object_dispatch_spec->parent_trace, read.version, &object_dispatch_spec->object_dispatch_flags, &object_dispatch_spec->dispatch_result, &object_dispatch_spec->dispatcher_ctx.on_finish, diff --git a/src/librbd/io/ObjectRequest.cc b/src/librbd/io/ObjectRequest.cc index 703d125688844..1e7280d3a03eb 100644 --- a/src/librbd/io/ObjectRequest.cc +++ b/src/librbd/io/ObjectRequest.cc @@ -200,14 +200,14 @@ void ObjectRequest::finish(int r) { template ObjectReadRequest::ObjectReadRequest( - I *ictx, uint64_t objectno, const Extents &extents, + I *ictx, uint64_t objectno, ReadExtents* extents, IOContext io_context, int op_flags, int read_flags, - const ZTracer::Trace &parent_trace, ceph::bufferlist* read_data, - Extents* extent_map, uint64_t* version, Context *completion) - : ObjectRequest(ictx, objectno, io_context, "read", - parent_trace, completion), - m_extents(extents), m_op_flags(op_flags), m_read_flags(read_flags), - m_read_data(read_data), m_extent_map(extent_map), m_version(version) { + const ZTracer::Trace &parent_trace, uint64_t* version, + Context *completion) + : ObjectRequest(ictx, objectno, io_context, "read", parent_trace, + completion), + m_extents(extents), m_op_flags(op_flags),m_read_flags(read_flags), + m_version(version) { } template @@ -235,15 +235,12 @@ void ObjectReadRequest::read_object() { ldout(image_ctx->cct, 20) << dendl; neorados::ReadOp read_op; - m_extent_results.reserve(this->m_extents.size()); - for (auto [object_off, object_len]: this->m_extents) { - m_extent_results.emplace_back(); - auto& extent_result = m_extent_results.back(); - if (object_len >= image_ctx->sparse_read_threshold_bytes) { - read_op.sparse_read(object_off, object_len, &(extent_result.first), - &(extent_result.second)); + for (auto& extent: *this->m_extents) { + if (extent.length >= image_ctx->sparse_read_threshold_bytes) { + read_op.sparse_read(extent.offset, extent.length, &extent.bl, + &extent.extent_map); } else { - read_op.read(object_off, object_len, &(extent_result.first)); + read_op.read(extent.offset, extent.length, &extent.bl); } } util::apply_op_flags( @@ -272,28 +269,6 @@ void ObjectReadRequest::handle_read_object(int r) { return; } - // merge ExtentResults to a single sparse bufferlist - int pos = 0; - uint64_t object_off = 0; - for (auto& [read_data, extent_map] : m_extent_results) { - if (extent_map.size() == 0) { - extent_map.push_back(std::make_pair(m_extents[pos].first, - read_data.length())); - } - - uint64_t total_extents_len = 0; - for (auto& [extent_off, extent_len] : extent_map) { - ceph_assert(extent_off >= object_off); - object_off = extent_off + extent_len; - m_extent_map->push_back(std::make_pair(extent_off, extent_len)); - total_extents_len += extent_len; - } - ceph_assert(total_extents_len == read_data.length()); - - m_read_data->claim_append(read_data); - ++pos; - } - this->finish(0); } @@ -313,7 +288,7 @@ void ObjectReadRequest::read_parent() { io::util::read_parent( image_ctx, this->m_object_no, this->m_extents, this->m_io_context->read_snap().value_or(CEPH_NOSNAP), this->m_trace, - m_read_data, ctx); + ctx); } template @@ -331,11 +306,6 @@ void ObjectReadRequest::handle_read_parent(int r) { return; } - if (this->m_extents.size() > 1) { - m_extent_map->insert(m_extent_map->end(), - m_extents.begin(), m_extents.end()); - } - copyup(); } diff --git a/src/librbd/io/ObjectRequest.h b/src/librbd/io/ObjectRequest.h index 1f96834e32935..711b440bd3207 100644 --- a/src/librbd/io/ObjectRequest.h +++ b/src/librbd/io/ObjectRequest.h @@ -93,20 +93,19 @@ template class ObjectReadRequest : public ObjectRequest { public: static ObjectReadRequest* create( - ImageCtxT *ictx, uint64_t objectno, const Extents &extents, + ImageCtxT *ictx, uint64_t objectno, ReadExtents* extents, IOContext io_context, int op_flags, int read_flags, - const ZTracer::Trace &parent_trace, ceph::bufferlist* read_data, - Extents* extent_map, uint64_t* version, Context *completion) { + const ZTracer::Trace &parent_trace, uint64_t* version, + Context *completion) { return new ObjectReadRequest(ictx, objectno, extents, io_context, op_flags, - read_flags, parent_trace, read_data, - extent_map, version, completion); + read_flags, parent_trace, version, completion); } ObjectReadRequest( - ImageCtxT *ictx, uint64_t objectno, const Extents &extents, + ImageCtxT *ictx, uint64_t objectno, ReadExtents* extents, IOContext io_context, int op_flags, int read_flags, - const ZTracer::Trace &parent_trace, ceph::bufferlist* read_data, - Extents* extent_map, uint64_t* version, Context *completion); + const ZTracer::Trace &parent_trace, uint64_t* version, + Context *completion); void send() override; @@ -136,16 +135,9 @@ private: * @endverbatim */ - const Extents m_extents; - - typedef std::pair ExtentResult; - typedef std::vector ExtentResults; - ExtentResults m_extent_results; + ReadExtents* m_extents; int m_op_flags; int m_read_flags; - - ceph::bufferlist* m_read_data; - Extents* m_extent_map; uint64_t* m_version; void read_object(); diff --git a/src/librbd/io/ReadResult.cc b/src/librbd/io/ReadResult.cc index 3b5913573ed70..26e27943d2c37 100644 --- a/src/librbd/io/ReadResult.cc +++ b/src/librbd/io/ReadResult.cc @@ -175,10 +175,8 @@ void ReadResult::C_ImageReadRequest::finish(int r) { } ReadResult::C_ObjectReadRequest::C_ObjectReadRequest( - AioCompletion *aio_completion, uint64_t object_off, uint64_t object_len, - LightweightBufferExtents&& buffer_extents) - : aio_completion(aio_completion), object_off(object_off), - object_len(object_len), buffer_extents(std::move(buffer_extents)) { + AioCompletion *aio_completion, ReadExtents&& extents) + : aio_completion(aio_completion), extents(std::move(extents)) { aio_completion->add_request(); } @@ -191,26 +189,49 @@ void ReadResult::C_ObjectReadRequest::finish(int r) { r = 0; } if (r >= 0) { - ldout(cct, 10) << " got " << extent_map - << " for " << buffer_extents - << " bl " << bl.length() << dendl; + uint64_t object_len = 0; aio_completion->lock.lock(); - if (!extent_map.empty()) { + for (auto& extent: extents) { + ldout(cct, 10) << " got " << extent.extent_map + << " for " << extent.buffer_extents + << " bl " << extent.bl.length() << dendl; + aio_completion->read_result.m_destriper.add_partial_sparse_result( - cct, bl, extent_map, object_off, buffer_extents); - } else { - // handle the case where a sparse-read wasn't issued - aio_completion->read_result.m_destriper.add_partial_result( - cct, std::move(bl), buffer_extents); + cct, std::move(extent.bl), extent.extent_map, extent.offset, + extent.buffer_extents); + + object_len += extent.length; } aio_completion->lock.unlock(); - r = object_len; } aio_completion->complete_request(r); } +ReadResult::C_ObjectReadMergedExtents::C_ObjectReadMergedExtents( + CephContext* cct, ReadExtents* extents, Context* on_finish) + : cct(cct), extents(extents), on_finish(on_finish) { +} + +void ReadResult::C_ObjectReadMergedExtents::finish(int r) { + if (r >= 0) { + for (auto& extent: *extents) { + if (bl.length() < extent.length) { + lderr(cct) << "Merged extents length is less than expected" << dendl; + r = -EIO; + break; + } + bl.splice(0, extent.length, &extent.bl); + } + if (bl.length() != 0) { + lderr(cct) << "Merged extents length is greater than expected" << dendl; + r = -EIO; + } + } + on_finish->complete(r); +} + ReadResult::ReadResult() : m_buffer(Empty()) { } diff --git a/src/librbd/io/ReadResult.h b/src/librbd/io/ReadResult.h index ad10f36c1126f..365d79f83ca73 100644 --- a/src/librbd/io/ReadResult.h +++ b/src/librbd/io/ReadResult.h @@ -38,20 +38,25 @@ public: struct C_ObjectReadRequest : public Context { AioCompletion *aio_completion; - uint64_t object_off; - uint64_t object_len; - LightweightBufferExtents buffer_extents; + ReadExtents extents; - bufferlist bl; - Extents extent_map; - - C_ObjectReadRequest(AioCompletion *aio_completion, uint64_t object_off, - uint64_t object_len, - LightweightBufferExtents&& buffer_extents); + C_ObjectReadRequest(AioCompletion *aio_completion, ReadExtents&& extents); void finish(int r) override; }; + struct C_ObjectReadMergedExtents : public Context { + CephContext* cct; + ReadExtents* extents; + Context *on_finish; + bufferlist bl; + + C_ObjectReadMergedExtents(CephContext* cct, ReadExtents* extents, + Context* on_finish); + + void finish(int r) override; + }; + ReadResult(); ReadResult(char *buf, size_t buf_len); ReadResult(const struct iovec *iov, int iov_count); diff --git a/src/librbd/io/SimpleSchedulerObjectDispatch.cc b/src/librbd/io/SimpleSchedulerObjectDispatch.cc index 2886de0aba080..3f2b5bbe2b46f 100644 --- a/src/librbd/io/SimpleSchedulerObjectDispatch.cc +++ b/src/librbd/io/SimpleSchedulerObjectDispatch.cc @@ -219,18 +219,18 @@ void SimpleSchedulerObjectDispatch::shut_down(Context* on_finish) { template bool SimpleSchedulerObjectDispatch::read( - uint64_t object_no, const Extents &extents, IOContext io_context, + uint64_t object_no, ReadExtents* extents, IOContext io_context, int op_flags, int read_flags, const ZTracer::Trace &parent_trace, - ceph::bufferlist* read_data, Extents* extent_map, uint64_t* version, - int* object_dispatch_flags, DispatchResult* dispatch_result, - Context** on_finish, Context* on_dispatched) { + uint64_t* version, int* object_dispatch_flags, + DispatchResult* dispatch_result, Context** on_finish, + Context* on_dispatched) { auto cct = m_image_ctx->cct; ldout(cct, 20) << data_object_name(m_image_ctx, object_no) << " " << extents << dendl; std::lock_guard locker{m_lock}; - for (auto [object_off, object_len] : extents) { - if (intersects(object_no, object_off, object_len)) { + for (auto& extent : *extents) { + if (intersects(object_no, extent.offset, extent.length)) { dispatch_delayed_requests(object_no); break; } diff --git a/src/librbd/io/SimpleSchedulerObjectDispatch.h b/src/librbd/io/SimpleSchedulerObjectDispatch.h index faf965dc14992..d68cda3ef2800 100644 --- a/src/librbd/io/SimpleSchedulerObjectDispatch.h +++ b/src/librbd/io/SimpleSchedulerObjectDispatch.h @@ -49,9 +49,8 @@ public: void shut_down(Context* on_finish) override; bool read( - uint64_t object_no, const Extents &extents, IOContext io_context, + uint64_t object_no, ReadExtents* extents, IOContext io_context, int op_flags, int read_flags, const ZTracer::Trace &parent_trace, - ceph::bufferlist* read_data, Extents* extent_map, uint64_t* version, int* object_dispatch_flags, DispatchResult* dispatch_result, Context** on_finish, Context* on_dispatched) override; diff --git a/src/librbd/io/Types.h b/src/librbd/io/Types.h index 03d5f528baba4..9d473fd413158 100644 --- a/src/librbd/io/Types.h +++ b/src/librbd/io/Types.h @@ -197,6 +197,45 @@ using striper::LightweightObjectExtents; typedef std::pair Extent; typedef std::vector Extents; +struct ReadExtent { + const uint64_t offset; + const uint64_t length; + const LightweightBufferExtents buffer_extents; + ceph::bufferlist bl; + Extents extent_map; + + ReadExtent(uint64_t offset, + uint64_t length) : offset(offset), length(length) {}; + ReadExtent(uint64_t offset, + uint64_t length, + const LightweightBufferExtents&& buffer_extents) + : offset(offset), + length(length), + buffer_extents(buffer_extents) {} + ReadExtent(uint64_t offset, + uint64_t length, + const LightweightBufferExtents&& buffer_extents, + ceph::bufferlist&& bl, + Extents&& extent_map) : offset(offset), + length(length), + buffer_extents(buffer_extents), + bl(bl), + extent_map(extent_map) {}; + + friend inline std::ostream& operator<<( + std::ostream& os, + const ReadExtent &extent) { + os << "offset=" << extent.offset << ", " + << "length=" << extent.length << ", " + << "buffer_extents=" << extent.buffer_extents << ", " + << "bl.length=" << extent.bl.length() << ", " + << "extent_map=" << extent.extent_map; + return os; + } +}; + +typedef std::vector ReadExtents; + typedef std::map ExtentMap; } // namespace io diff --git a/src/librbd/io/Utils.cc b/src/librbd/io/Utils.cc index ffa5de47b9311..bc5f9644ff9e6 100644 --- a/src/librbd/io/Utils.cc +++ b/src/librbd/io/Utils.cc @@ -82,9 +82,9 @@ bool assemble_write_same_extent( } template -void read_parent(I *image_ctx, uint64_t object_no, const Extents &extents, +void read_parent(I *image_ctx, uint64_t object_no, ReadExtents* extents, librados::snap_t snap_id, const ZTracer::Trace &trace, - ceph::bufferlist* data, Context* on_finish) { + Context* on_finish) { auto cct = image_ctx->cct; @@ -92,9 +92,9 @@ void read_parent(I *image_ctx, uint64_t object_no, const Extents &extents, // calculate reverse mapping onto the image Extents parent_extents; - for (auto [object_off, object_len]: extents) { - Striper::extent_to_file(cct, &image_ctx->layout, object_no, object_off, - object_len, parent_extents); + for (auto& extent: *extents) { + Striper::extent_to_file(cct, &image_ctx->layout, object_no, extent.offset, + extent.length, parent_extents); } uint64_t parent_overlap = 0; @@ -114,13 +114,24 @@ void read_parent(I *image_ctx, uint64_t object_no, const Extents &extents, ldout(cct, 20) << dendl; + ceph::bufferlist* parent_read_bl; + if (extents->size() > 1) { + auto parent_comp = new ReadResult::C_ObjectReadMergedExtents( + cct, extents, on_finish); + parent_read_bl = &parent_comp->bl; + on_finish = parent_comp; + } else { + parent_read_bl = &extents->front().bl; + } + 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}, + image_ctx->parent, comp, std::move(parent_extents), + ReadResult{parent_read_bl}, image_ctx->parent->get_data_io_context(), 0, 0, trace); } @@ -148,13 +159,24 @@ uint64_t extents_length(Extents &extents) { return total_bytes; } +void unsparsify(CephContext* cct, ceph::bufferlist* bl, + const Extents& extent_map, uint64_t bl_off, + uint64_t out_bl_len) { + Striper::StripedReadResult destriper; + bufferlist out_bl; + + destriper.add_partial_sparse_result(cct, std::move(*bl), extent_map, bl_off, + {{0, out_bl_len}}); + destriper.assemble_result(cct, out_bl, true); + *bl = out_bl; +} + } // namespace util } // namespace io } // namespace librbd template void librbd::io::util::read_parent( - librbd::ImageCtx *image_ctx, uint64_t object_no, const Extents &extents, - librados::snap_t snap_id, const ZTracer::Trace &trace, - ceph::bufferlist* data, Context* on_finish); + librbd::ImageCtx *image_ctx, uint64_t object_no, ReadExtents* extents, + librados::snap_t snap_id, const ZTracer::Trace &trace, Context* on_finish); template int librbd::io::util::clip_request( librbd::ImageCtx *image_ctx, Extents *image_extents); diff --git a/src/librbd/io/Utils.h b/src/librbd/io/Utils.h index 63e0affcd1b42..c58649bb975db 100644 --- a/src/librbd/io/Utils.h +++ b/src/librbd/io/Utils.h @@ -30,15 +30,19 @@ bool assemble_write_same_extent(const LightweightObjectExtent &object_extent, bool force_write); template -void read_parent(ImageCtxT *image_ctx, uint64_t object_no, const Extents &extents, - librados::snap_t snap_id, const ZTracer::Trace &trace, - ceph::bufferlist* data, Context* on_finish); +void read_parent(ImageCtxT *image_ctx, uint64_t object_no, + ReadExtents* extents, librados::snap_t snap_id, + const ZTracer::Trace &trace, Context* on_finish); template int clip_request(ImageCtxT *image_ctx, Extents *image_extents); uint64_t extents_length(Extents &extents); +void unsparsify(CephContext* cct, ceph::bufferlist* bl, + const Extents& extent_map, uint64_t bl_off, + uint64_t out_bl_len); + } // namespace util } // namespace io } // namespace librbd diff --git a/src/librbd/journal/ObjectDispatch.h b/src/librbd/journal/ObjectDispatch.h index a523fa390de12..006f323d40b72 100644 --- a/src/librbd/journal/ObjectDispatch.h +++ b/src/librbd/journal/ObjectDispatch.h @@ -37,9 +37,8 @@ public: void shut_down(Context* on_finish) override; bool read( - uint64_t object_no, const io::Extents &extents, IOContext io_context, + uint64_t object_no, io::ReadExtents* extents, IOContext io_context, int op_flags, int read_flags, const ZTracer::Trace &parent_trace, - ceph::bufferlist* read_data, io::Extents* extent_map, uint64_t* version, int* object_dispatch_flags, io::DispatchResult* dispatch_result, Context** on_finish, Context* on_dispatched) { diff --git a/src/librbd/plugin/Api.cc b/src/librbd/plugin/Api.cc index 5e96c97c50dc9..76a9859f79dd5 100644 --- a/src/librbd/plugin/Api.cc +++ b/src/librbd/plugin/Api.cc @@ -10,10 +10,10 @@ namespace plugin { template void Api::read_parent( - I *image_ctx, uint64_t object_no, const Extents &extents, + I *image_ctx, uint64_t object_no, io::ReadExtents* extents, librados::snap_t snap_id, const ZTracer::Trace &trace, - ceph::bufferlist* data, Context* on_finish) { - io::util::read_parent(image_ctx, object_no, extents, snap_id, trace, data, + Context* on_finish) { + io::util::read_parent(image_ctx, object_no, extents, snap_id, trace, on_finish); } diff --git a/src/librbd/plugin/Api.h b/src/librbd/plugin/Api.h index 1cba7b870d2ca..f2dd5c8224915 100644 --- a/src/librbd/plugin/Api.h +++ b/src/librbd/plugin/Api.h @@ -25,9 +25,9 @@ struct Api { virtual ~Api() {} virtual void read_parent( - ImageCtxT *image_ctx, uint64_t object_no, const Extents &extents, + ImageCtxT *image_ctx, uint64_t object_no, io::ReadExtents* extents, librados::snap_t snap_id, const ZTracer::Trace &trace, - ceph::bufferlist* data, Context* on_finish); + Context* on_finish); }; diff --git a/src/osdc/Striper.cc b/src/osdc/Striper.cc index e653fe43f9ee9..6f162e901fe7f 100644 --- a/src/osdc/Striper.cc +++ b/src/osdc/Striper.cc @@ -425,6 +425,12 @@ void Striper::StripedReadResult::add_partial_sparse_result( ldout(cct, 10) << "add_partial_sparse_result(" << this << ") " << bl.length() << " covering " << bl_map << " (offset " << bl_off << ")" << " to " << buffer_extents << dendl; + + if (bl_map.empty()) { + add_partial_result(cct, bl, buffer_extents); + return; + } + auto s = bl_map.cbegin(); for (auto& be : buffer_extents) { ::add_partial_sparse_result(cct, &partial, &total_intended_len, bl, &s, @@ -433,12 +439,18 @@ void Striper::StripedReadResult::add_partial_sparse_result( } void Striper::StripedReadResult::add_partial_sparse_result( - CephContext *cct, ceph::buffer::list& bl, + CephContext *cct, ceph::buffer::list&& bl, const std::vector>& bl_map, uint64_t bl_off, const striper::LightweightBufferExtents& buffer_extents) { ldout(cct, 10) << "add_partial_sparse_result(" << this << ") " << bl.length() << " covering " << bl_map << " (offset " << bl_off << ")" << " to " << buffer_extents << dendl; + + if (bl_map.empty()) { + add_partial_result(cct, std::move(bl), buffer_extents); + return; + } + auto s = bl_map.cbegin(); for (auto& be : buffer_extents) { ::add_partial_sparse_result(cct, &partial, &total_intended_len, bl, &s, diff --git a/src/osdc/Striper.h b/src/osdc/Striper.h index c7822361a4a4c..0761cd6c76376 100644 --- a/src/osdc/Striper.h +++ b/src/osdc/Striper.h @@ -106,7 +106,7 @@ const std::map& bl_map, uint64_t bl_off, const std::vector >& buffer_extents); void add_partial_sparse_result( - CephContext *cct, ceph::buffer::list& bl, + CephContext *cct, ceph::buffer::list&& bl, const std::vector>& bl_map, uint64_t bl_off, const striper::LightweightBufferExtents& buffer_extents); diff --git a/src/test/librbd/cache/test_mock_ParentCacheObjectDispatch.cc b/src/test/librbd/cache/test_mock_ParentCacheObjectDispatch.cc index f3947ebe84d80..6609f1f02e4a3 100644 --- a/src/test/librbd/cache/test_mock_ParentCacheObjectDispatch.cc +++ b/src/test/librbd/cache/test_mock_ParentCacheObjectDispatch.cc @@ -42,10 +42,9 @@ namespace plugin { template <> struct Api { - MOCK_METHOD7(read_parent, void(MockParentImageCacheImageCtx*, uint64_t, - const librbd::io::Extents &, librados::snap_t, - const ZTracer::Trace &, ceph::bufferlist*, - Context*)); + MOCK_METHOD6(read_parent, void(MockParentImageCacheImageCtx*, uint64_t, + librbd::io::ReadExtents*, librados::snap_t, + const ZTracer::Trace &, Context*)); }; } // namespace plugin @@ -114,11 +113,11 @@ public : } void expect_read_parent(MockPluginApi &mock_plugin_api, uint64_t object_no, - const io::Extents &extents, librados::snap_t snap_id, + io::ReadExtents* extents, librados::snap_t snap_id, int r) { EXPECT_CALL(mock_plugin_api, - read_parent(_, object_no, extents, snap_id, _, _, _)) - .WillOnce(WithArg<6>(CompleteContext(r, static_cast(nullptr)))); + read_parent(_, object_no, extents, snap_id, _, _)) + .WillOnce(WithArg<5>(CompleteContext(r, static_cast(nullptr)))); } void expect_cache_close(MockParentImageCache& mparent_image_cache, int ret_val) { @@ -360,10 +359,10 @@ TEST_F(TestMockParentCacheObjectDispatch, test_read) { C_SaferCond on_dispatched; io::DispatchResult dispatch_result; - ceph::bufferlist read_data; + io::ReadExtents extents = {{0, 4096}, {8192, 4096}}; mock_parent_image_cache->read( - 0, {{0, 4096}}, mock_image_ctx.get_data_io_context(), 0, 0, {}, &read_data, - nullptr, nullptr, nullptr, &dispatch_result, nullptr, &on_dispatched); + 0, &extents, mock_image_ctx.get_data_io_context(), 0, 0, {}, nullptr, + nullptr, &dispatch_result, nullptr, &on_dispatched); ASSERT_EQ(0, on_dispatched.wait()); mock_parent_image_cache->get_cache_client()->close(); @@ -410,13 +409,14 @@ TEST_F(TestMockParentCacheObjectDispatch, test_read_dne) { expect_cache_lookup_object(*mock_parent_image_cache, ""); - expect_read_parent(mock_plugin_api, 0, {{0, 4096}}, CEPH_NOSNAP, 0); + io::ReadExtents extents = {{0, 4096}}; + expect_read_parent(mock_plugin_api, 0, &extents, CEPH_NOSNAP, 0); C_SaferCond on_dispatched; io::DispatchResult dispatch_result; mock_parent_image_cache->read( - 0, {{0, 4096}}, mock_image_ctx.get_data_io_context(), 0, 0, {}, nullptr, - nullptr, nullptr, nullptr, &dispatch_result, nullptr, &on_dispatched); + 0, &extents, mock_image_ctx.get_data_io_context(), 0, 0, {}, nullptr, + nullptr, &dispatch_result, nullptr, &on_dispatched); ASSERT_EQ(0, on_dispatched.wait()); mock_parent_image_cache->get_cache_client()->close(); diff --git a/src/test/librbd/crypto/test_mock_CryptoObjectDispatch.cc b/src/test/librbd/crypto/test_mock_CryptoObjectDispatch.cc index af5932130512d..658b442f46679 100644 --- a/src/test/librbd/crypto/test_mock_CryptoObjectDispatch.cc +++ b/src/test/librbd/crypto/test_mock_CryptoObjectDispatch.cc @@ -107,8 +107,8 @@ struct TestMockCryptoCryptoObjectDispatch : public TestMockFixture { EXPECT_CALL(*crypto, encrypt(_, _)).Times(count); } - void expect_decrypt() { - EXPECT_CALL(*crypto, decrypt(_, _)); + void expect_decrypt(int count = 1) { + EXPECT_CALL(*crypto, decrypt(_, _)).Times(count); } }; @@ -135,9 +135,10 @@ TEST_F(TestMockCryptoCryptoObjectDispatch, Discard) { TEST_F(TestMockCryptoCryptoObjectDispatch, ReadFail) { expect_object_read(); + io::ReadExtents extents = {{0, 4096}}; ASSERT_TRUE(mock_crypto_object_dispatch->read( - 0, {{0, 4096}}, mock_image_ctx->get_data_io_context(), 0, 0, {}, &data, - &extent_map, nullptr, &object_dispatch_flags, &dispatch_result, + 0, &extents, mock_image_ctx->get_data_io_context(), 0, 0, {}, + nullptr, &object_dispatch_flags, &dispatch_result, &on_finish, on_dispatched)); ASSERT_EQ(dispatch_result, io::DISPATCH_RESULT_COMPLETE); ASSERT_NE(on_finish, &finished_cond); @@ -149,17 +150,18 @@ TEST_F(TestMockCryptoCryptoObjectDispatch, ReadFail) { TEST_F(TestMockCryptoCryptoObjectDispatch, Read) { expect_object_read(); + io::ReadExtents extents = {{0, 4096}, {8192, 4096}}; ASSERT_TRUE(mock_crypto_object_dispatch->read( - 0, {{0, 4096}}, mock_image_ctx->get_data_io_context(), 0, 0, {}, - &data, &extent_map, nullptr, &object_dispatch_flags, &dispatch_result, + 0, &extents, mock_image_ctx->get_data_io_context(), 0, 0, {}, + nullptr, &object_dispatch_flags, &dispatch_result, &on_finish, on_dispatched)); ASSERT_EQ(dispatch_result, io::DISPATCH_RESULT_COMPLETE); ASSERT_NE(on_finish, &finished_cond); ASSERT_EQ(ETIMEDOUT, dispatched_cond.wait_for(0)); - expect_decrypt(); + expect_decrypt(2); dispatcher_ctx->complete(0); - ASSERT_EQ(4096, dispatched_cond.wait()); + ASSERT_EQ(8192, dispatched_cond.wait()); } TEST_F(TestMockCryptoCryptoObjectDispatch, Write) { diff --git a/src/test/librbd/io/test_mock_ObjectRequest.cc b/src/test/librbd/io/test_mock_ObjectRequest.cc index 3e2fbbdbeb849..dad8005731a7d 100644 --- a/src/test/librbd/io/test_mock_ObjectRequest.cc +++ b/src/test/librbd/io/test_mock_ObjectRequest.cc @@ -98,10 +98,9 @@ struct Mock { s_instance = this; } - MOCK_METHOD7(read_parent, - void(librbd::MockTestImageCtx *, uint64_t, const Extents &, - librados::snap_t, const ZTracer::Trace &, ceph::bufferlist*, - Context*)); + MOCK_METHOD6(read_parent, + void(librbd::MockTestImageCtx *, uint64_t, ReadExtents*, + librados::snap_t, const ZTracer::Trace &, Context*)); }; Mock *Mock::s_instance = nullptr; @@ -110,11 +109,10 @@ Mock *Mock::s_instance = nullptr; template<> void read_parent( librbd::MockTestImageCtx *image_ctx, uint64_t object_no, - const Extents &extents, librados::snap_t snap_id, - const ZTracer::Trace &trace, ceph::bufferlist* data, - Context* on_finish) { + ReadExtents* extents, librados::snap_t snap_id, + const ZTracer::Trace &trace, Context* on_finish) { Mock::s_instance->read_parent(image_ctx, object_no, extents, snap_id, trace, - data, on_finish); + on_finish); } } // namespace util @@ -232,11 +230,11 @@ struct TestMockIoObjectRequest : public TestMockFixture { } void expect_read_parent(MockUtils &mock_utils, uint64_t object_no, - const Extents &extents, librados::snap_t snap_id, + ReadExtents* extents, librados::snap_t snap_id, int r) { EXPECT_CALL(mock_utils, - read_parent(_, object_no, extents, snap_id, _, _, _)) - .WillOnce(WithArg<6>(CompleteContext(r, static_cast(nullptr)))); + read_parent(_, object_no, extents, snap_id, _, _)) + .WillOnce(WithArg<5>(CompleteContext(r, static_cast(nullptr)))); } void expect_copyup(MockCopyupRequest& mock_copyup_request, int r) { @@ -421,26 +419,25 @@ TEST_F(TestMockIoObjectRequest, Read) { expect_read(mock_image_ctx, ictx->get_object_name(0), 8192, 4096, std::string(4096, '2'), 0); - bufferlist bl; - Extents extent_map; C_SaferCond ctx; uint64_t version; + ReadExtents extents = {{0, 4096}, {8192, 4096}}; auto req = MockObjectReadRequest::create( - &mock_image_ctx, 0, {{0, 4096}, {8192, 4096}}, + &mock_image_ctx, 0, &extents, mock_image_ctx.get_data_io_context(), 0, 0, {}, - &bl, &extent_map, &version, &ctx); + &version, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); - bufferlist expected_bl; - expected_bl.append(std::string(4096, '1')); - expected_bl.append(std::string(4096, '2')); - Extents exepected_extent_map = {{0, 4096}, {8192, 4096}}; + bufferlist expected_bl1; + expected_bl1.append(std::string(4096, '1')); + bufferlist expected_bl2; + expected_bl2.append(std::string(4096, '2')); - ASSERT_TRUE(expected_bl.contents_equal(bl)); - ASSERT_EQ(exepected_extent_map.size(), extent_map.size()); - ASSERT_TRUE(std::equal(extent_map.begin(), extent_map.end(), - exepected_extent_map.begin())); + ASSERT_EQ(extents[0].extent_map.size(), 0); + ASSERT_EQ(extents[1].extent_map.size(), 0); + ASSERT_TRUE(extents[0].bl.contents_equal(expected_bl1)); + ASSERT_TRUE(extents[1].bl.contents_equal(expected_bl2)); } TEST_F(TestMockIoObjectRequest, SparseReadThreshold) { @@ -462,14 +459,12 @@ TEST_F(TestMockIoObjectRequest, SparseReadThreshold) { ictx->sparse_read_threshold_bytes, std::string(ictx->sparse_read_threshold_bytes, '1'), 0); - bufferlist bl; - Extents extent_map; C_SaferCond ctx; + + ReadExtents extents = {{0, ictx->sparse_read_threshold_bytes}}; auto req = MockObjectReadRequest::create( - &mock_image_ctx, 0, - {{0, ictx->sparse_read_threshold_bytes}}, - mock_image_ctx.get_data_io_context(), 0, 0, {}, &bl, - &extent_map, nullptr, &ctx); + &mock_image_ctx, 0, &extents, + mock_image_ctx.get_data_io_context(), 0, 0, {}, nullptr, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } @@ -491,12 +486,11 @@ TEST_F(TestMockIoObjectRequest, ReadError) { expect_get_read_flags(mock_image_ctx, CEPH_NOSNAP, 0); expect_read(mock_image_ctx, ictx->get_object_name(0), 0, 4096, "", -EPERM); - bufferlist bl; - Extents extent_map; C_SaferCond ctx; + ReadExtents extents = {{0, 4096}}; auto req = MockObjectReadRequest::create( - &mock_image_ctx, 0, {{0, 4096}}, - mock_image_ctx.get_data_io_context(), 0, 0, {}, &bl, &extent_map, + &mock_image_ctx, 0, &extents, + mock_image_ctx.get_data_io_context(), 0, 0, {}, nullptr, &ctx); req->send(); ASSERT_EQ(-EPERM, ctx.wait()); @@ -536,14 +530,13 @@ TEST_F(TestMockIoObjectRequest, ParentRead) { expect_read(mock_image_ctx, ictx->get_object_name(0), 0, 4096, "", -ENOENT); MockUtils mock_utils; - expect_read_parent(mock_utils, 0, {{0, 4096}}, CEPH_NOSNAP, 0); + ReadExtents extents = {{0, 4096}}; + expect_read_parent(mock_utils, 0, &extents, CEPH_NOSNAP, 0); - bufferlist bl; - Extents extent_map; C_SaferCond ctx; auto req = MockObjectReadRequest::create( - &mock_image_ctx, 0, {{0, 4096}}, - mock_image_ctx.get_data_io_context(), 0, 0, {}, &bl, &extent_map, + &mock_image_ctx, 0, &extents, + mock_image_ctx.get_data_io_context(), 0, 0, {}, nullptr, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); @@ -583,14 +576,13 @@ TEST_F(TestMockIoObjectRequest, ParentReadError) { expect_read(mock_image_ctx, ictx->get_object_name(0), 0, 4096, "", -ENOENT); MockUtils mock_utils; - expect_read_parent(mock_utils, 0, {{0, 4096}}, CEPH_NOSNAP, -EPERM); + ReadExtents extents = {{0, 4096}}; + expect_read_parent(mock_utils, 0, &extents, CEPH_NOSNAP, -EPERM); - bufferlist bl; - Extents extent_map; C_SaferCond ctx; auto req = MockObjectReadRequest::create( - &mock_image_ctx, 0, {{0, 4096}}, - mock_image_ctx.get_data_io_context(), 0, 0, {}, &bl, &extent_map, + &mock_image_ctx, 0, &extents, + mock_image_ctx.get_data_io_context(), 0, 0, {}, nullptr, &ctx); req->send(); ASSERT_EQ(-EPERM, ctx.wait()); @@ -629,13 +621,11 @@ TEST_F(TestMockIoObjectRequest, SkipParentRead) { expect_get_read_flags(mock_image_ctx, CEPH_NOSNAP, 0); expect_read(mock_image_ctx, ictx->get_object_name(0), 0, 4096, "", -ENOENT); - bufferlist bl; - Extents extent_map; + ReadExtents extents = {{0, 4096}}; C_SaferCond ctx; auto req = MockObjectReadRequest::create( - &mock_image_ctx, 0, {{0, 4096}}, mock_image_ctx.get_data_io_context(), 0, - READ_FLAG_DISABLE_READ_FROM_PARENT, {}, &bl, &extent_map, - nullptr, &ctx); + &mock_image_ctx, 0, &extents, mock_image_ctx.get_data_io_context(), 0, + READ_FLAG_DISABLE_READ_FROM_PARENT, {}, nullptr, &ctx); req->send(); ASSERT_EQ(-ENOENT, ctx.wait()); } @@ -674,19 +664,18 @@ TEST_F(TestMockIoObjectRequest, CopyOnRead) { expect_read(mock_image_ctx, ictx->get_object_name(0), 0, 4096, "", -ENOENT); MockUtils mock_utils; - expect_read_parent(mock_utils, 0, {{0, 4096}}, CEPH_NOSNAP, 0); + ReadExtents extents = {{0, 4096}}; + expect_read_parent(mock_utils, 0, &extents, CEPH_NOSNAP, 0); MockCopyupRequest mock_copyup_request; expect_get_parent_overlap(mock_image_ctx, CEPH_NOSNAP, 4096, 0); expect_prune_parent_extents(mock_image_ctx, {{0, 4096}}, 4096, 4096); expect_copyup(mock_copyup_request, 0); - bufferlist bl; - Extents extent_map; C_SaferCond ctx; auto req = MockObjectReadRequest::create( - &mock_image_ctx, 0, {{0, 4096}}, - mock_image_ctx.get_data_io_context(), 0, 0, {}, &bl, &extent_map, + &mock_image_ctx, 0, &extents, + mock_image_ctx.get_data_io_context(), 0, 0, {}, nullptr, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); diff --git a/src/test/librbd/io/test_mock_SimpleSchedulerObjectDispatch.cc b/src/test/librbd/io/test_mock_SimpleSchedulerObjectDispatch.cc index dd7625cecb8a4..a3d823d234f9e 100644 --- a/src/test/librbd/io/test_mock_SimpleSchedulerObjectDispatch.cc +++ b/src/test/librbd/io/test_mock_SimpleSchedulerObjectDispatch.cc @@ -131,9 +131,10 @@ TEST_F(TestMockIoSimpleSchedulerObjectDispatch, Read) { C_SaferCond cond; Context *on_finish = &cond; + io::ReadExtents extents = {{0, 4096}, {8192, 4096}}; ASSERT_FALSE(mock_simple_scheduler_object_dispatch.read( - 0, {{0, 4096}}, mock_image_ctx.get_data_io_context(), 0, 0, {}, nullptr, - nullptr, nullptr, nullptr, nullptr, &on_finish, nullptr)); + 0, &extents, mock_image_ctx.get_data_io_context(), 0, 0, {}, nullptr, + nullptr, nullptr, &on_finish, nullptr)); ASSERT_EQ(on_finish, &cond); // not modified on_finish->complete(0); ASSERT_EQ(0, cond.wait()); diff --git a/src/test/librbd/mock/io/MockObjectDispatch.h b/src/test/librbd/mock/io/MockObjectDispatch.h index ccdde9f74b160..fd0a5ac78c527 100644 --- a/src/test/librbd/mock/io/MockObjectDispatch.h +++ b/src/test/librbd/mock/io/MockObjectDispatch.h @@ -24,18 +24,17 @@ public: MOCK_METHOD1(shut_down, void(Context*)); - MOCK_METHOD8(execute_read, - bool(uint64_t, const Extents&, IOContext io_context, - ceph::bufferlist*, Extents*, uint64_t*, + MOCK_METHOD6(execute_read, + bool(uint64_t, ReadExtents*, IOContext io_context, uint64_t*, DispatchResult*, Context*)); bool read( - uint64_t object_no, const Extents& extents, IOContext io_context, + uint64_t object_no, ReadExtents* extents, IOContext io_context, int op_flags, int read_flags, const ZTracer::Trace& parent_trace, - ceph::bufferlist* read_data, Extents* extent_map, uint64_t* version, - int* dispatch_flags, DispatchResult* dispatch_result, - Context** on_finish, Context* on_dispatched) { - return execute_read(object_no, extents, io_context, read_data, extent_map, - version, dispatch_result, on_dispatched); + uint64_t* version, int* dispatch_flags, + DispatchResult* dispatch_result, Context** on_finish, + Context* on_dispatched) { + return execute_read(object_no, extents, io_context, version, + dispatch_result, on_dispatched); } MOCK_METHOD9(execute_discard, -- 2.39.5