From: Or Ozeri Date: Thu, 16 Jul 2020 11:14:36 +0000 (+0300) Subject: librbd: support reading multiple extents in the object dispatch interface X-Git-Tag: v16.1.0~1515^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=01045545b6f61c381a221c9da7f6a927e997cd2b;p=ceph.git librbd: support reading multiple extents in the object dispatch interface This commit extends the object dispatch read function to support multiple object extents (using a single librados ObjectOperation). In addition, we add an option to return the version number for that read. This new function will be used by the new crypto object dispatch layer to support read-modify-write operation required for encrypting and writing unaligned data. Signed-off-by: Or Ozeri --- diff --git a/src/librbd/cache/ObjectCacherObjectDispatch.cc b/src/librbd/cache/ObjectCacherObjectDispatch.cc index ffa0fc0a840ea..db0c71f186a42 100644 --- a/src/librbd/cache/ObjectCacherObjectDispatch.cc +++ b/src/librbd/cache/ObjectCacherObjectDispatch.cc @@ -181,15 +181,21 @@ void ObjectCacherObjectDispatch::shut_down(Context* on_finish) { template bool ObjectCacherObjectDispatch::read( - uint64_t object_no, uint64_t object_off, uint64_t object_len, - librados::snap_t snap_id, int op_flags, const ZTracer::Trace &parent_trace, - ceph::bufferlist* read_data, io::Extents* extent_map, + uint64_t object_no, const io::Extents &extents, librados::snap_t snap_id, + int op_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) { // IO chained in reverse order auto cct = m_image_ctx->cct; - ldout(cct, 20) << "object_no=" << object_no << " " << object_off << "~" - << object_len << dendl; + ldout(cct, 20) << "object_no=" << object_no << " " << extents << dendl; + + if (version != nullptr || extents.size() != 1) { + // 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, diff --git a/src/librbd/cache/ObjectCacherObjectDispatch.h b/src/librbd/cache/ObjectCacherObjectDispatch.h index cb5e389126446..6e4ab23ae77e6 100644 --- a/src/librbd/cache/ObjectCacherObjectDispatch.h +++ b/src/librbd/cache/ObjectCacherObjectDispatch.h @@ -42,10 +42,10 @@ public: void shut_down(Context* on_finish) override; bool read( - uint64_t object_no, uint64_t object_off, uint64_t object_len, - librados::snap_t snap_id, int op_flags, - const ZTracer::Trace &parent_trace, ceph::bufferlist* read_data, - io::Extents* extent_map, int* object_dispatch_flags, + uint64_t object_no, const io::Extents &extents, librados::snap_t snap_id, + int op_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 c05d9e737f651..529b6c54ad105 100644 --- a/src/librbd/cache/ObjectCacherWriteback.cc +++ b/src/librbd/cache/ObjectCacherWriteback.cc @@ -137,8 +137,8 @@ void ObjectCacherWriteback::read(const object_t& oid, uint64_t object_no, aio_comp, off, len, {{0, len}}); auto req = io::ObjectDispatchSpec::create_read( - m_ictx, io::OBJECT_DISPATCH_LAYER_CACHE, object_no, off, len, snapid, - op_flags, trace, &req_comp->bl, &req_comp->extent_map, req_comp); + m_ictx, io::OBJECT_DISPATCH_LAYER_CACHE, object_no, {{off, len}}, snapid, + op_flags, trace, &req_comp->bl, &req_comp->extent_map, nullptr, req_comp); req->send(); } diff --git a/src/librbd/cache/ParentCacheObjectDispatch.cc b/src/librbd/cache/ParentCacheObjectDispatch.cc index 17a601d7a126c..7e3823481fd46 100644 --- a/src/librbd/cache/ParentCacheObjectDispatch.cc +++ b/src/librbd/cache/ParentCacheObjectDispatch.cc @@ -64,15 +64,22 @@ void ParentCacheObjectDispatch::init(Context* on_finish) { template bool ParentCacheObjectDispatch::read( - uint64_t object_no, uint64_t object_off, - uint64_t object_len, librados::snap_t snap_id, int op_flags, - const ZTracer::Trace &parent_trace, ceph::bufferlist* read_data, - io::Extents* extent_map, int* object_dispatch_flags, - io::DispatchResult* dispatch_result, Context** on_finish, - Context* on_dispatched) { + uint64_t object_no, const io::Extents &extents, librados::snap_t snap_id, + int op_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 cct = m_image_ctx->cct; - ldout(cct, 20) << "object_no=" << object_no << " " << object_off << "~" - << object_len << dendl; + ldout(cct, 20) << "object_no=" << object_no << " " << extents << dendl; + + if (version != nullptr || extents.size() != 1) { + // 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, @@ -128,7 +135,7 @@ void ParentCacheObjectDispatch::handle_read_cache( *dispatch_result = io::DISPATCH_RESULT_COMPLETE; on_dispatched->complete(r); }); - io::util::read_parent(m_image_ctx, object_no, read_off, read_len, + io::util::read_parent(m_image_ctx, object_no, {{read_off, read_len}}, snap_id, parent_trace, read_data, ctx); return; } diff --git a/src/librbd/cache/ParentCacheObjectDispatch.h b/src/librbd/cache/ParentCacheObjectDispatch.h index d3265619669b6..588d284f2fd10 100644 --- a/src/librbd/cache/ParentCacheObjectDispatch.h +++ b/src/librbd/cache/ParentCacheObjectDispatch.h @@ -40,10 +40,10 @@ public: } bool read( - uint64_t object_no, uint64_t object_off, uint64_t object_len, - librados::snap_t snap_id, int op_flags, - const ZTracer::Trace &parent_trace, ceph::bufferlist* read_data, - io::Extents* extent_map, int* object_dispatch_flags, + uint64_t object_no, const io::Extents &extents, librados::snap_t snap_id, + int op_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/WriteAroundObjectDispatch.cc b/src/librbd/cache/WriteAroundObjectDispatch.cc index 3d4fd1fc31e0b..bbcf4a65b24f1 100644 --- a/src/librbd/cache/WriteAroundObjectDispatch.cc +++ b/src/librbd/cache/WriteAroundObjectDispatch.cc @@ -57,11 +57,12 @@ void WriteAroundObjectDispatch::shut_down(Context* on_finish) { template bool WriteAroundObjectDispatch::read( - uint64_t object_no, uint64_t object_off, uint64_t object_len, - librados::snap_t snap_id, int op_flags, const ZTracer::Trace &parent_trace, - ceph::bufferlist* read_data, io::Extents* extent_map, + uint64_t object_no, const io::Extents &extents, librados::snap_t snap_id, + int op_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); } diff --git a/src/librbd/cache/WriteAroundObjectDispatch.h b/src/librbd/cache/WriteAroundObjectDispatch.h index 873feb724493e..bf1e1d985513f 100644 --- a/src/librbd/cache/WriteAroundObjectDispatch.h +++ b/src/librbd/cache/WriteAroundObjectDispatch.h @@ -42,10 +42,10 @@ public: void shut_down(Context* on_finish) override; bool read( - uint64_t object_no, uint64_t object_off, uint64_t object_len, - librados::snap_t snap_id, int op_flags, - const ZTracer::Trace &parent_trace, ceph::bufferlist* read_data, - io::Extents* extent_map, int* object_dispatch_flags, + uint64_t object_no, const io::Extents &extents, librados::snap_t snap_id, + int op_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/io/ImageRequest.cc b/src/librbd/io/ImageRequest.cc index e8b824f0ee6f7..48a3decfcbcaf 100644 --- a/src/librbd/io/ImageRequest.cc +++ b/src/librbd/io/ImageRequest.cc @@ -101,8 +101,8 @@ void readahead(I *ictx, const Extents& image_extents) { 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, snap_id, 0, {}, - &req_comp->read_data, &req_comp->extent_map, req_comp); + {{object_extent.offset, object_extent.length}}, snap_id, 0, {}, + &req_comp->read_data, &req_comp->extent_map, nullptr, req_comp); req->send(); } @@ -405,9 +405,9 @@ void ImageReadRequest::send_request() { auto req_comp = new io::ReadResult::C_ObjectReadRequest( 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, snap_id, m_op_flags, this->m_trace, &req_comp->bl, - &req_comp->extent_map, req_comp); + &image_ctx, OBJECT_DISPATCH_LAYER_NONE, oe.object_no, + {{oe.offset, oe.length}}, snap_id, m_op_flags, this->m_trace, + &req_comp->bl, &req_comp->extent_map, nullptr, req_comp); req->send(); } diff --git a/src/librbd/io/ObjectDispatch.cc b/src/librbd/io/ObjectDispatch.cc index 4e0e626cfd3cb..c6099316685a8 100644 --- a/src/librbd/io/ObjectDispatch.cc +++ b/src/librbd/io/ObjectDispatch.cc @@ -33,20 +33,18 @@ void ObjectDispatch::shut_down(Context* on_finish) { template bool ObjectDispatch::read( - uint64_t object_no, uint64_t object_off, uint64_t object_len, - librados::snap_t snap_id, int op_flags, const ZTracer::Trace &parent_trace, - ceph::bufferlist* read_data, Extents* extent_map, + uint64_t object_no, const Extents &extents, librados::snap_t snap_id, + int op_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) { auto cct = m_image_ctx->cct; - ldout(cct, 20) << data_object_name(m_image_ctx, object_no) << " " - << object_off << "~" << object_len << dendl; + ldout(cct, 20) << "object_no=" << object_no << " " << extents << dendl; *dispatch_result = DISPATCH_RESULT_COMPLETE; - auto req = new ObjectReadRequest(m_image_ctx, object_no, object_off, - object_len, snap_id, op_flags, - parent_trace, read_data, extent_map, - on_dispatched); + auto req = new ObjectReadRequest(m_image_ctx, object_no, extents, snap_id, + op_flags, parent_trace, read_data, + extent_map, version, on_dispatched); req->send(); return true; } diff --git a/src/librbd/io/ObjectDispatch.h b/src/librbd/io/ObjectDispatch.h index a7945ed0ce0db..4a65ec417beef 100644 --- a/src/librbd/io/ObjectDispatch.h +++ b/src/librbd/io/ObjectDispatch.h @@ -34,10 +34,10 @@ public: void shut_down(Context* on_finish) override; bool read( - uint64_t object_no, uint64_t object_off, uint64_t object_len, - librados::snap_t snap_id, int op_flags, - const ZTracer::Trace &parent_trace, ceph::bufferlist* read_data, - Extents* extent_map, int* object_dispatch_flags, + uint64_t object_no, const Extents &extents, librados::snap_t snap_id, + int op_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 9397d1ead1471..1ade6adbd5186 100644 --- a/src/librbd/io/ObjectDispatchInterface.h +++ b/src/librbd/io/ObjectDispatchInterface.h @@ -34,9 +34,9 @@ struct ObjectDispatchInterface { virtual void shut_down(Context* on_finish) = 0; virtual bool read( - uint64_t object_no, uint64_t object_off, uint64_t object_len, - librados::snap_t snap_id, int op_flags,const ZTracer::Trace &parent_trace, - ceph::bufferlist* read_data, Extents* extent_map, + uint64_t object_no, const Extents &extents, + librados::snap_t snap_id, int op_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) = 0; diff --git a/src/librbd/io/ObjectDispatchSpec.h b/src/librbd/io/ObjectDispatchSpec.h index 74ff4208f7a30..942b1d719cf86 100644 --- a/src/librbd/io/ObjectDispatchSpec.h +++ b/src/librbd/io/ObjectDispatchSpec.h @@ -36,35 +36,35 @@ private: public: struct RequestBase { uint64_t object_no; - uint64_t object_off; - RequestBase(uint64_t object_no, uint64_t object_off) - : object_no(object_no), object_off(object_off) { + RequestBase(uint64_t object_no) + : object_no(object_no) { } }; struct ReadRequest : public RequestBase { - uint64_t object_len; + const Extents extents; librados::snap_t snap_id; ceph::bufferlist* read_data; Extents* extent_map; + uint64_t* version; - ReadRequest(uint64_t object_no, uint64_t object_off, uint64_t object_len, + ReadRequest(uint64_t object_no, const Extents &extents, librados::snap_t snap_id, ceph::bufferlist* read_data, - Extents* extent_map) - : RequestBase(object_no, object_off), - object_len(object_len), snap_id(snap_id), read_data(read_data), - extent_map(extent_map) { + Extents* extent_map, uint64_t* version) + : RequestBase(object_no), extents(extents), snap_id(snap_id), + read_data(read_data), extent_map(extent_map), version(version) { } }; struct WriteRequestBase : public RequestBase { + uint64_t object_off; ::SnapContext snapc; uint64_t journal_tid; WriteRequestBase(uint64_t object_no, uint64_t object_off, const ::SnapContext& snapc, uint64_t journal_tid) - : RequestBase(object_no, object_off), snapc(snapc), + : RequestBase(object_no), object_off(object_off), snapc(snapc), journal_tid(journal_tid) { } }; @@ -153,15 +153,14 @@ public: template static ObjectDispatchSpec* create_read( ImageCtxT* image_ctx, ObjectDispatchLayer object_dispatch_layer, - uint64_t object_no, uint64_t object_off, uint64_t object_len, - librados::snap_t snap_id, int op_flags, - const ZTracer::Trace &parent_trace, ceph::bufferlist* read_data, - Extents* extent_map, Context* on_finish) { + uint64_t object_no, const Extents &extents, librados::snap_t snap_id, + int op_flags, const ZTracer::Trace &parent_trace, + ceph::bufferlist* read_data, Extents* extent_map, uint64_t* version, + Context* on_finish) { return new ObjectDispatchSpec(image_ctx->io_object_dispatcher, object_dispatch_layer, - ReadRequest{object_no, object_off, - object_len, snap_id, read_data, - extent_map}, + ReadRequest{object_no, extents, snap_id, + read_data, extent_map, version}, op_flags, parent_trace, on_finish); } diff --git a/src/librbd/io/ObjectDispatcher.cc b/src/librbd/io/ObjectDispatcher.cc index 7c46fdff75d2d..4732aa68e27c8 100644 --- a/src/librbd/io/ObjectDispatcher.cc +++ b/src/librbd/io/ObjectDispatcher.cc @@ -107,9 +107,9 @@ struct ObjectDispatcher::SendVisitor : public boost::static_visitor { bool operator()(ObjectDispatchSpec::ReadRequest& read) const { return object_dispatch->read( - read.object_no, read.object_off, read.object_len, read.snap_id, + read.object_no, read.extents, read.snap_id, object_dispatch_spec->op_flags, object_dispatch_spec->parent_trace, - read.read_data, read.extent_map, + read.read_data, read.extent_map, 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 1bae9099d24ce..87879eb8607a0 100644 --- a/src/librbd/io/ObjectRequest.cc +++ b/src/librbd/io/ObjectRequest.cc @@ -99,11 +99,10 @@ ObjectRequest::create_compare_and_write( template ObjectRequest::ObjectRequest( - I *ictx, uint64_t objectno, uint64_t off, uint64_t len, - librados::snap_t snap_id, const char *trace_name, - 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), + I *ictx, uint64_t objectno, librados::snap_t snap_id, + const char *trace_name, const ZTracer::Trace &trace, Context *completion) + : m_ictx(ictx), m_object_no(objectno), m_snap_id(snap_id), + m_completion(completion), m_trace(create_trace(*ictx, "", trace)) { ceph_assert(m_ictx->data_ctx.is_valid()); if (m_trace.valid()) { @@ -182,12 +181,14 @@ void ObjectRequest::finish(int r) { template ObjectReadRequest::ObjectReadRequest( - I *ictx, uint64_t objectno, uint64_t offset, uint64_t len, + I *ictx, uint64_t objectno, const Extents &extents, librados::snap_t snap_id, int op_flags, const ZTracer::Trace &parent_trace, - bufferlist* read_data, Extents* extent_map, Context *completion) - : ObjectRequest(ictx, objectno, offset, len, snap_id, "read", + ceph::bufferlist* read_data, Extents* extent_map, uint64_t* version, + Context *completion) + : ObjectRequest(ictx, objectno, snap_id, "read", parent_trace, completion), - m_op_flags(op_flags), m_read_data(read_data), m_extent_map(extent_map) { + m_extents(extents), m_op_flags(op_flags), m_read_data(read_data), + m_extent_map(extent_map), m_version(version) { } template @@ -213,11 +214,16 @@ void ObjectReadRequest::read_object() { ldout(image_ctx->cct, 20) << dendl; neorados::ReadOp read_op; - if (this->m_object_len >= image_ctx->sparse_read_threshold_bytes) { - read_op.sparse_read(this->m_object_off, this->m_object_len, m_read_data, - m_extent_map); - } else { - read_op.read(this->m_object_off, this->m_object_len, m_read_data); + 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)); + } else { + read_op.read(object_off, object_len, &(extent_result.first)); + } } util::apply_op_flags(m_op_flags, image_ctx->get_read_flags(this->m_snap_id), &read_op); @@ -226,7 +232,7 @@ void ObjectReadRequest::read_object() { {data_object_name(this->m_ictx, this->m_object_no)}, *image_ctx->get_data_io_context(), std::move(read_op), nullptr, librbd::asio::util::get_callback_adapter( - [this](int r) { handle_read_object(r); }), nullptr, + [this](int r) { handle_read_object(r); }), m_version, (this->m_trace.valid() ? this->m_trace.get_info() : nullptr)); } @@ -245,6 +251,28 @@ 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); } @@ -256,9 +284,8 @@ void ObjectReadRequest::read_parent() { 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); + io::util::read_parent(image_ctx, this->m_object_no, this->m_extents, + this->m_snap_id, this->m_trace, m_read_data, ctx); } template @@ -276,6 +303,11 @@ 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(); } @@ -328,9 +360,9 @@ AbstractObjectWriteRequest::AbstractObjectWriteRequest( I *ictx, uint64_t object_no, uint64_t object_off, uint64_t len, const ::SnapContext &snapc, const char *trace_name, const ZTracer::Trace &parent_trace, Context *completion) - : ObjectRequest(ictx, object_no, object_off, len, CEPH_NOSNAP, trace_name, - parent_trace, completion), - m_snap_seq(snapc.seq.val) + : ObjectRequest(ictx, object_no, CEPH_NOSNAP, trace_name, parent_trace, + completion), + m_object_off(object_off), m_object_len(len), m_snap_seq(snapc.seq.val) { m_snaps.insert(m_snaps.end(), snapc.snaps.begin(), snapc.snaps.end()); diff --git a/src/librbd/io/ObjectRequest.h b/src/librbd/io/ObjectRequest.h index f696e13f71c47..c34a1f7f3efc2 100644 --- a/src/librbd/io/ObjectRequest.h +++ b/src/librbd/io/ObjectRequest.h @@ -53,9 +53,9 @@ public: const ::SnapContext &snapc, uint64_t *mismatch_offset, int op_flags, const ZTracer::Trace &parent_trace, Context *completion); - ObjectRequest(ImageCtxT *ictx, uint64_t objectno, uint64_t off, uint64_t len, - librados::snap_t snap_id, const char *trace_name, - const ZTracer::Trace &parent_trace, Context *completion); + ObjectRequest(ImageCtxT *ictx, uint64_t objectno, librados::snap_t snap_id, + const char *trace_name, const ZTracer::Trace &parent_trace, + Context *completion); virtual ~ObjectRequest() { m_trace.event("finish"); } @@ -75,7 +75,7 @@ protected: bool compute_parent_extents(Extents *parent_extents, bool read_request); ImageCtxT *m_ictx; - uint64_t m_object_no, m_object_off, m_object_len; + uint64_t m_object_no; librados::snap_t m_snap_id; Context *m_completion; ZTracer::Trace m_trace; @@ -91,20 +91,20 @@ template class ObjectReadRequest : public ObjectRequest { public: static ObjectReadRequest* create( - ImageCtxT *ictx, uint64_t objectno, uint64_t offset, uint64_t len, + ImageCtxT *ictx, uint64_t objectno, const Extents &extents, librados::snap_t snap_id, int op_flags, const ZTracer::Trace &parent_trace, ceph::bufferlist* read_data, - Extents* extent_map, Context *completion) { - return new ObjectReadRequest(ictx, objectno, offset, len, - snap_id, op_flags, parent_trace, read_data, - extent_map, completion); + Extents* extent_map, uint64_t* version, Context *completion) { + return new ObjectReadRequest(ictx, objectno, extents, snap_id, op_flags, + parent_trace, read_data, extent_map, version, + completion); } ObjectReadRequest( - ImageCtxT *ictx, uint64_t objectno, uint64_t offset, uint64_t len, + ImageCtxT *ictx, uint64_t objectno, const Extents &extents, librados::snap_t snap_id, int op_flags, const ZTracer::Trace &parent_trace, ceph::bufferlist* read_data, - Extents* extent_map, Context *completion); + Extents* extent_map, uint64_t* version, Context *completion); void send() override; @@ -134,10 +134,16 @@ private: * @endverbatim */ + const Extents m_extents; + + typedef std::pair ExtentResult; + typedef std::vector ExtentResults; + ExtentResults m_extent_results; int m_op_flags; ceph::bufferlist* m_read_data; Extents* m_extent_map; + uint64_t* m_version; void read_object(); void handle_read_object(int r); @@ -173,6 +179,8 @@ public: void send() override; protected: + uint64_t m_object_off; + uint64_t m_object_len; bool m_full_object = false; bool m_copyup_enabled = true; diff --git a/src/librbd/io/SimpleSchedulerObjectDispatch.cc b/src/librbd/io/SimpleSchedulerObjectDispatch.cc index 66669ec4d03d3..8b83468fe89af 100644 --- a/src/librbd/io/SimpleSchedulerObjectDispatch.cc +++ b/src/librbd/io/SimpleSchedulerObjectDispatch.cc @@ -215,18 +215,21 @@ void SimpleSchedulerObjectDispatch::shut_down(Context* on_finish) { template bool SimpleSchedulerObjectDispatch::read( - uint64_t object_no, uint64_t object_off, uint64_t object_len, - librados::snap_t snap_id, int op_flags, const ZTracer::Trace &parent_trace, - ceph::bufferlist* read_data, Extents* extent_map, + uint64_t object_no, const Extents &extents, librados::snap_t snap_id, + int op_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) { auto cct = m_image_ctx->cct; - ldout(cct, 20) << data_object_name(m_image_ctx, object_no) << " " - << object_off << "~" << object_len << dendl; + ldout(cct, 20) << data_object_name(m_image_ctx, object_no) << " " << extents + << dendl; std::lock_guard locker{m_lock}; - if (intersects(object_no, object_off, object_len)) { - dispatch_delayed_requests(object_no); + for (auto [object_off, object_len] : extents) { + if (intersects(object_no, object_off, object_len)) { + dispatch_delayed_requests(object_no); + break; + } } return false; diff --git a/src/librbd/io/SimpleSchedulerObjectDispatch.h b/src/librbd/io/SimpleSchedulerObjectDispatch.h index ca134ef385c57..def70af5e906c 100644 --- a/src/librbd/io/SimpleSchedulerObjectDispatch.h +++ b/src/librbd/io/SimpleSchedulerObjectDispatch.h @@ -50,10 +50,10 @@ public: void shut_down(Context* on_finish) override; bool read( - uint64_t object_no, uint64_t object_off, uint64_t object_len, - librados::snap_t snap_id, int op_flags, - const ZTracer::Trace &parent_trace, ceph::bufferlist* read_data, - Extents* extent_map, int* object_dispatch_flags, + uint64_t object_no, const Extents &extents, librados::snap_t snap_id, + int op_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/Utils.cc b/src/librbd/io/Utils.cc index 78d8a5a83fa88..50334c063d52b 100644 --- a/src/librbd/io/Utils.cc +++ b/src/librbd/io/Utils.cc @@ -80,10 +80,9 @@ bool assemble_write_same_extent( } 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) { +void read_parent(I *image_ctx, uint64_t object_no, const Extents &extents, + librados::snap_t snap_id, const ZTracer::Trace &trace, + ceph::bufferlist* data, Context* on_finish) { auto cct = image_ctx->cct; @@ -91,8 +90,10 @@ void read_parent(I *image_ctx, uint64_t object_no, uint64_t off, // calculate reverse mapping onto the image Extents parent_extents; - Striper::extent_to_file(cct, &image_ctx->layout, object_no, off, len, - 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); + } uint64_t parent_overlap = 0; uint64_t object_overlap = 0; @@ -125,6 +126,6 @@ void read_parent(I *image_ctx, uint64_t object_no, uint64_t off, } // namespace librbd template void librbd::io::util::read_parent( - librbd::ImageCtx *image_ctx, uint64_t object_no, uint64_t off, uint64_t len, + 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); diff --git a/src/librbd/io/Utils.h b/src/librbd/io/Utils.h index 295bf0469cac8..81161adc8313b 100644 --- a/src/librbd/io/Utils.h +++ b/src/librbd/io/Utils.h @@ -30,10 +30,9 @@ bool assemble_write_same_extent(const LightweightObjectExtent &object_extent, 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); +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); } // namespace util } // namespace io diff --git a/src/librbd/journal/ObjectDispatch.h b/src/librbd/journal/ObjectDispatch.h index 48176e6434512..cd03389c5c134 100644 --- a/src/librbd/journal/ObjectDispatch.h +++ b/src/librbd/journal/ObjectDispatch.h @@ -38,10 +38,10 @@ public: void shut_down(Context* on_finish) override; bool read( - uint64_t object_no, uint64_t object_off, uint64_t object_len, - librados::snap_t snap_id, int op_flags, - const ZTracer::Trace &parent_trace, ceph::bufferlist* read_data, - io::Extents* extent_map, int* object_dispatch_flags, + uint64_t object_no, const io::Extents &extents, librados::snap_t snap_id, + int op_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) { return false; diff --git a/src/test/librbd/cache/test_mock_ParentCacheObjectDispatch.cc b/src/test/librbd/cache/test_mock_ParentCacheObjectDispatch.cc index d649b58d999c0..c65c4974dcd92 100644 --- a/src/test/librbd/cache/test_mock_ParentCacheObjectDispatch.cc +++ b/src/test/librbd/cache/test_mock_ParentCacheObjectDispatch.cc @@ -50,9 +50,9 @@ struct Mock { s_instance = this; } - MOCK_METHOD8(read_parent, - void(librbd::MockParentImageCacheImageCtx *, uint64_t, uint64_t, - uint64_t, librados::snap_t, const ZTracer::Trace &, + MOCK_METHOD7(read_parent, + void(librbd::MockParentImageCacheImageCtx *, uint64_t, + const io::Extents &, librados::snap_t, const ZTracer::Trace &, ceph::bufferlist*, Context*)); }; @@ -62,9 +62,9 @@ Mock *Mock::s_instance = nullptr; template<> void read_parent( librbd::MockParentImageCacheImageCtx *image_ctx, uint64_t object_no, - uint64_t off, uint64_t len, librados::snap_t snap_id, + const io::Extents &extents, 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, + Mock::s_instance->read_parent(image_ctx, object_no, extents, snap_id, trace, data, on_finish); } @@ -136,11 +136,11 @@ public : } void expect_read_parent(MockUtils &mock_utils, uint64_t object_no, - uint64_t off, uint64_t len, librados::snap_t snap_id, + const io::Extents &extents, 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)))); + read_parent(_, object_no, extents, snap_id, _, _, _)) + .WillOnce(WithArg<6>(CompleteContext(r, static_cast(nullptr)))); } void expect_cache_close(MockParentImageCache& mparent_image_cache, int ret_val) { @@ -370,9 +370,9 @@ TEST_F(TestMockParentCacheObjectDispatch, test_read) { C_SaferCond on_dispatched; io::DispatchResult dispatch_result; ceph::bufferlist read_data; - mock_parent_image_cache->read(0, 0, 4096, CEPH_NOSNAP, 0, {}, &read_data, - nullptr, nullptr, &dispatch_result, nullptr, - &on_dispatched); + mock_parent_image_cache->read(0, {{0, 4096}}, CEPH_NOSNAP, 0, {}, &read_data, + nullptr, nullptr, nullptr, &dispatch_result, + nullptr, &on_dispatched); ASSERT_EQ(0, on_dispatched.wait()); mock_parent_image_cache->get_cache_client()->close(); @@ -418,13 +418,13 @@ TEST_F(TestMockParentCacheObjectDispatch, test_read_dne) { expect_cache_lookup_object(*mock_parent_image_cache, ""); MockUtils mock_utils; - expect_read_parent(mock_utils, 0, 0, 4096, CEPH_NOSNAP, 0); + expect_read_parent(mock_utils, 0, {{0, 4096}}, CEPH_NOSNAP, 0); C_SaferCond on_dispatched; io::DispatchResult dispatch_result; - mock_parent_image_cache->read(0, 0, 4096, CEPH_NOSNAP, 0, {}, nullptr, - nullptr, nullptr, &dispatch_result, nullptr, - &on_dispatched); + mock_parent_image_cache->read(0, {{0, 4096}}, CEPH_NOSNAP, 0, {}, nullptr, + nullptr, 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/io/test_mock_ObjectRequest.cc b/src/test/librbd/io/test_mock_ObjectRequest.cc index a5ab6dba533db..123bf52c149d9 100644 --- a/src/test/librbd/io/test_mock_ObjectRequest.cc +++ b/src/test/librbd/io/test_mock_ObjectRequest.cc @@ -66,8 +66,8 @@ struct Mock { s_instance = this; } - MOCK_METHOD8(read_parent, - void(librbd::MockTestImageCtx *, uint64_t, uint64_t, uint64_t, + MOCK_METHOD7(read_parent, + void(librbd::MockTestImageCtx *, uint64_t, const Extents &, librados::snap_t, const ZTracer::Trace &, ceph::bufferlist*, Context*)); }; @@ -77,10 +77,11 @@ 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, + 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) { + Mock::s_instance->read_parent(image_ctx, object_no, extents, snap_id, trace, data, on_finish); } @@ -197,11 +198,11 @@ struct TestMockIoObjectRequest : public TestMockFixture { } void expect_read_parent(MockUtils &mock_utils, uint64_t object_no, - uint64_t off, uint64_t len, librados::snap_t snap_id, + const Extents &extents, 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)))); + read_parent(_, object_no, extents, snap_id, _, _, _)) + .WillOnce(WithArg<6>(CompleteContext(r, static_cast(nullptr)))); } void expect_copyup(MockCopyupRequest& mock_copyup_request, int r) { @@ -352,14 +353,28 @@ TEST_F(TestMockIoObjectRequest, Read) { expect_get_read_flags(mock_image_ctx, CEPH_NOSNAP, 0); expect_read(mock_image_ctx, ictx->get_object_name(0), 0, 4096, std::string(4096, '1'), 0); + 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; auto req = MockObjectReadRequest::create( - &mock_image_ctx, 0, 0, 4096, CEPH_NOSNAP, 0, {}, &bl, &extent_map, &ctx); + &mock_image_ctx, 0, {{0, 4096}, {8192, 4096}}, CEPH_NOSNAP, 0, {}, + &bl, &extent_map, &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}}; + + 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())); } TEST_F(TestMockIoObjectRequest, SparseReadThreshold) { @@ -385,8 +400,9 @@ TEST_F(TestMockIoObjectRequest, SparseReadThreshold) { Extents extent_map; C_SaferCond ctx; auto req = MockObjectReadRequest::create( - &mock_image_ctx, 0, 0, ictx->sparse_read_threshold_bytes, CEPH_NOSNAP, 0, - {}, &bl, &extent_map, &ctx); + &mock_image_ctx, 0, + {{0, ictx->sparse_read_threshold_bytes}}, CEPH_NOSNAP, 0, {}, &bl, + &extent_map, nullptr, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } @@ -412,7 +428,8 @@ TEST_F(TestMockIoObjectRequest, ReadError) { Extents extent_map; C_SaferCond ctx; auto req = MockObjectReadRequest::create( - &mock_image_ctx, 0, 0, 4096, CEPH_NOSNAP, 0, {}, &bl, &extent_map, &ctx); + &mock_image_ctx, 0, {{0, 4096}}, CEPH_NOSNAP, 0, {}, &bl, &extent_map, + nullptr, &ctx); req->send(); ASSERT_EQ(-EPERM, ctx.wait()); } @@ -451,13 +468,14 @@ 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); + expect_read_parent(mock_utils, 0, {{0, 4096}}, CEPH_NOSNAP, 0); bufferlist bl; Extents extent_map; C_SaferCond ctx; auto req = MockObjectReadRequest::create( - &mock_image_ctx, 0, 0, 4096, CEPH_NOSNAP, 0, {}, &bl, &extent_map, &ctx); + &mock_image_ctx, 0, {{0, 4096}}, CEPH_NOSNAP, 0, {}, &bl, &extent_map, + nullptr, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } @@ -496,13 +514,14 @@ 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); + expect_read_parent(mock_utils, 0, {{0, 4096}}, CEPH_NOSNAP, -EPERM); bufferlist bl; Extents extent_map; C_SaferCond ctx; auto req = MockObjectReadRequest::create( - &mock_image_ctx, 0, 0, 4096, CEPH_NOSNAP, 0, {}, &bl, &extent_map, &ctx); + &mock_image_ctx, 0, {{0, 4096}}, CEPH_NOSNAP, 0, {}, &bl, &extent_map, + nullptr, &ctx); req->send(); ASSERT_EQ(-EPERM, ctx.wait()); } @@ -541,7 +560,7 @@ 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); + 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); @@ -552,7 +571,8 @@ TEST_F(TestMockIoObjectRequest, CopyOnRead) { Extents extent_map; C_SaferCond ctx; auto req = MockObjectReadRequest::create( - &mock_image_ctx, 0, 0, 4096, CEPH_NOSNAP, 0, {}, &bl, &extent_map, &ctx); + &mock_image_ctx, 0, {{0, 4096}}, CEPH_NOSNAP, 0, {}, &bl, &extent_map, + 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 ab2b0ba1d7c02..9ce77ad0fb068 100644 --- a/src/test/librbd/io/test_mock_SimpleSchedulerObjectDispatch.cc +++ b/src/test/librbd/io/test_mock_SimpleSchedulerObjectDispatch.cc @@ -132,8 +132,8 @@ TEST_F(TestMockIoSimpleSchedulerObjectDispatch, Read) { C_SaferCond cond; Context *on_finish = &cond; ASSERT_FALSE(mock_simple_scheduler_object_dispatch.read( - 0, 0, 4096, CEPH_NOSNAP, 0, {}, nullptr, nullptr, nullptr, nullptr, - &on_finish, nullptr)); + 0, {{0, 4096}}, CEPH_NOSNAP, 0, {}, nullptr, nullptr, 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 5cfe98f480688..943b3a06a1e08 100644 --- a/src/test/librbd/mock/io/MockObjectDispatch.h +++ b/src/test/librbd/mock/io/MockObjectDispatch.h @@ -25,17 +25,17 @@ public: MOCK_METHOD1(shut_down, void(Context*)); MOCK_METHOD8(execute_read, - bool(uint64_t, uint64_t, uint64_t, librados::snap_t, - ceph::bufferlist*, Extents*, DispatchResult*, Context*)); + bool(uint64_t, const Extents&, librados::snap_t, + ceph::bufferlist*, Extents*, uint64_t*, + DispatchResult*, Context*)); bool read( - uint64_t object_no, uint64_t object_off, uint64_t object_len, - librados::snap_t snap_id, int op_flags, - const ZTracer::Trace& parent_trace, ceph::bufferlist* read_data, - Extents* extent_map, int* dispatch_flags, - DispatchResult* dispatch_result, Context** on_finish, - Context* on_dispatched) { - return execute_read(object_no, object_off, object_len, snap_id, read_data, - extent_map, dispatch_result, on_dispatched); + uint64_t object_no, const Extents& extents, librados::snap_t snap_id, + int op_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, snap_id, read_data, extent_map, + version, dispatch_result, on_dispatched); } MOCK_METHOD9(execute_discard,