diff options
24 files changed, 531 insertions, 259 deletions
diff --git a/src/librbd/CMakeLists.txt b/src/librbd/CMakeLists.txt index e0fb674d013..af0fdcc8deb 100644 --- a/src/librbd/CMakeLists.txt +++ b/src/librbd/CMakeLists.txt @@ -43,6 +43,7 @@ set(librbd_internal_srcs io/ImageRequest.cc io/ImageRequestWQ.cc io/ObjectRequest.cc + io/ReadResult.cc journal/RemoveRequest.cc journal/CreateRequest.cc journal/OpenRequest.cc diff --git a/src/librbd/cache/ImageWriteback.cc b/src/librbd/cache/ImageWriteback.cc index 1fd167ce793..6867eec2552 100644 --- a/src/librbd/cache/ImageWriteback.cc +++ b/src/librbd/cache/ImageWriteback.cc @@ -7,6 +7,7 @@ #include "librbd/ImageCtx.h" #include "librbd/io/AioCompletion.h" #include "librbd/io/ImageRequest.h" +#include "librbd/io/ReadResult.h" #define dout_subsys ceph_subsys_rbd #undef dout_prefix @@ -29,7 +30,7 @@ void ImageWriteback<I>::aio_read(Extents &&image_extents, bufferlist *bl, auto aio_comp = io::AioCompletion::create_and_start(on_finish, &m_image_ctx, io::AIO_TYPE_READ); io::ImageReadRequest<I> req(m_image_ctx, aio_comp, std::move(image_extents), - nullptr, bl, fadvise_flags); + io::ReadResult{bl}, fadvise_flags); req.set_bypass_image_cache(); req.send(); } diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc index d7125b37dce..8cd4f16efa2 100644 --- a/src/librbd/internal.cc +++ b/src/librbd/internal.cc @@ -42,6 +42,7 @@ #include "librbd/io/ImageRequest.h" #include "librbd/io/ImageRequestWQ.h" #include "librbd/io/ObjectRequest.h" +#include "librbd/io/ReadResult.h" #include "librbd/journal/Types.h" #include "librbd/operation/TrimRequest.h" @@ -2092,7 +2093,7 @@ void filter_out_mirror_watchers(ImageCtx *ictx, // coordinate through AIO WQ to ensure lock is acquired if needed m_dest->io_work_queue->aio_write(comp, m_offset, m_bl->length(), - m_bl->c_str(), + std::move(*m_bl), LIBRADOS_OP_FLAG_FADVISE_DONTNEED); } @@ -2148,8 +2149,8 @@ void filter_out_mirror_watchers(ImageCtx *ictx, Context *ctx = new C_CopyRead(&throttle, dest, offset, bl); auto comp = io::AioCompletion::create_and_start(ctx, src, io::AIO_TYPE_READ); - io::ImageRequest<>::aio_read(src, comp, {{offset, len}}, nullptr, bl, - fadvise_flags); + io::ImageRequest<>::aio_read(src, comp, {{offset, len}}, + io::ReadResult{bl}, fadvise_flags); prog_ctx.update_progress(offset, src_size); } @@ -2374,8 +2375,8 @@ void filter_out_mirror_watchers(ImageCtx *ictx, C_SaferCond ctx; auto c = io::AioCompletion::create_and_start(&ctx, ictx, io::AIO_TYPE_READ); - io::ImageRequest<>::aio_read(ictx, c, {{off, read_len}}, nullptr, &bl, - 0); + io::ImageRequest<>::aio_read(ictx, c, {{off, read_len}}, + io::ReadResult{&bl}, 0); int ret = ctx.wait(); if (ret < 0) { diff --git a/src/librbd/io/AioCompletion.cc b/src/librbd/io/AioCompletion.cc index 8d42e82fa2b..812fbc9e0e3 100644 --- a/src/librbd/io/AioCompletion.cc +++ b/src/librbd/io/AioCompletion.cc @@ -44,30 +44,9 @@ void AioCompletion::finalize(ssize_t rval) assert(ictx != nullptr); CephContext *cct = ictx->cct; - ldout(cct, 20) << this << " " << __func__ << ": r=" << rval << ", " - << "read_buf=" << reinterpret_cast<void*>(read_buf) << ", " - << "real_bl=" << reinterpret_cast<void*>(read_bl) << dendl; + ldout(cct, 20) << this << " " << __func__ << ": r=" << rval << dendl; if (rval >= 0 && aio_type == AIO_TYPE_READ) { - if (read_buf && !read_bl) { - destriper.assemble_result(cct, read_buf, read_buf_len); - } else { - // FIXME: make the destriper write directly into a buffer so - // that we avoid shuffling pointers and copying zeros around. - bufferlist bl; - destriper.assemble_result(cct, bl, true); - - if (read_buf) { - assert(bl.length() == read_buf_len); - bl.copy(0, read_buf_len, read_buf); - ldout(cct, 20) << "copied resulting " << bl.length() - << " bytes to " << (void*)read_buf << dendl; - } - if (read_bl) { - ldout(cct, 20) << " moving resulting " << bl.length() - << " bytes to bl " << (void*)read_bl << dendl; - read_bl->claim(bl); - } - } + read_result.assemble_result(cct); } } diff --git a/src/librbd/io/AioCompletion.h b/src/librbd/io/AioCompletion.h index cce065f9fd8..7c2099cf52e 100644 --- a/src/librbd/io/AioCompletion.h +++ b/src/librbd/io/AioCompletion.h @@ -12,8 +12,8 @@ #include "librbd/AsyncOperation.h" #include "librbd/ImageCtx.h" +#include "librbd/io/ReadResult.h" #include "librbd/io/Types.h" -#include "osdc/Striper.h" class CephContext; @@ -55,10 +55,7 @@ struct AioCompletion { utime_t start_time; aio_type_t aio_type; - Striper::StripedReadResult destriper; - bufferlist *read_bl; - char *read_buf; - size_t read_buf_len; + ReadResult read_result; AsyncOperation async_op; @@ -105,7 +102,6 @@ struct AioCompletion { pending_count(0), blockers(1), ref(1), released(false), ictx(NULL), aio_type(AIO_TYPE_NONE), - read_bl(NULL), read_buf(NULL), read_buf_len(0), journal_tid(0), m_xlist_item(this), event_notify(false) { } diff --git a/src/librbd/io/CopyupRequest.cc b/src/librbd/io/CopyupRequest.cc index b025a1712a1..297d2204fc6 100644 --- a/src/librbd/io/CopyupRequest.cc +++ b/src/librbd/io/CopyupRequest.cc @@ -15,6 +15,7 @@ #include "librbd/io/AioCompletion.h" #include "librbd/io/ImageRequest.h" #include "librbd/io/ObjectRequest.h" +#include "librbd/io/ReadResult.h" #include <boost/bind.hpp> #include <boost/lambda/bind.hpp> @@ -206,7 +207,7 @@ void CopyupRequest::send() << ", extents " << m_image_extents << dendl; ImageRequest<>::aio_read(m_ictx->parent, comp, std::move(m_image_extents), - nullptr, &m_copyup_data, 0); + ReadResult{&m_copyup_data}, 0); } void CopyupRequest::complete(int r) diff --git a/src/librbd/io/ImageRequest.cc b/src/librbd/io/ImageRequest.cc index e11d1949c0f..e2b873d0673 100644 --- a/src/librbd/io/ImageRequest.cc +++ b/src/librbd/io/ImageRequest.cc @@ -81,84 +81,6 @@ struct C_FlushJournalCommit : public Context { }; template <typename ImageCtxT> -class C_AioRead : public C_AioRequest { -public: - C_AioRead(AioCompletion *completion) - : C_AioRequest(completion), m_req(nullptr) { - } - - void finish(int r) override { - m_completion->lock.Lock(); - CephContext *cct = m_completion->ictx->cct; - ldout(cct, 10) << "C_AioRead::finish() " << this << " r = " << r << dendl; - - if (r >= 0 || r == -ENOENT) { // this was a sparse_read operation - ldout(cct, 10) << " got " << m_req->get_extent_map() - << " for " << m_req->get_buffer_extents() - << " bl " << m_req->data().length() << dendl; - // reads from the parent don't populate the m_ext_map and the overlap - // may not be the full buffer. compensate here by filling in m_ext_map - // with the read extent when it is empty. - if (m_req->get_extent_map().empty()) { - m_req->get_extent_map()[m_req->get_offset()] = m_req->data().length(); - } - - m_completion->destriper.add_partial_sparse_result( - cct, m_req->data(), m_req->get_extent_map(), m_req->get_offset(), - m_req->get_buffer_extents()); - r = m_req->get_length(); - } - m_completion->lock.Unlock(); - - C_AioRequest::finish(r); - } - - void set_req(ObjectReadRequest<ImageCtxT> *req) { - m_req = req; - } -private: - ObjectReadRequest<ImageCtxT> *m_req; -}; - -template <typename ImageCtxT> -class C_ImageCacheRead : public C_AioRequest { -public: - typedef std::vector<std::pair<uint64_t,uint64_t> > Extents; - - C_ImageCacheRead(AioCompletion *completion, const Extents &image_extents) - : C_AioRequest(completion), m_image_extents(image_extents) { - } - - inline bufferlist &get_data() { - return m_bl; - } - -protected: - void finish(int r) override { - CephContext *cct = m_completion->ictx->cct; - ldout(cct, 10) << "C_ImageCacheRead::finish() " << this << ": r=" << r - << dendl; - if (r >= 0) { - size_t length = 0; - for (auto &image_extent : m_image_extents) { - length += image_extent.second; - } - assert(length == m_bl.length()); - - m_completion->lock.Lock(); - m_completion->destriper.add_partial_result(cct, m_bl, m_image_extents); - m_completion->lock.Unlock(); - r = length; - } - C_AioRequest::finish(r); - } - -private: - bufferlist m_bl; - Extents m_image_extents; -}; - -template <typename ImageCtxT> class C_ObjectCacheRead : public Context { public: explicit C_ObjectCacheRead(ImageCtxT &ictx, ObjectReadRequest<ImageCtxT> *req) @@ -190,17 +112,10 @@ private: template <typename I> void ImageRequest<I>::aio_read(I *ictx, AioCompletion *c, - Extents &&image_extents, char *buf, - bufferlist *pbl, int op_flags) { - ImageReadRequest<I> req(*ictx, c, std::move(image_extents), buf, pbl, - op_flags); - req.send(); -} - -template <typename I> -void ImageRequest<I>::aio_write(I *ictx, AioCompletion *c, uint64_t off, - size_t len, const char *buf, int op_flags) { - ImageWriteRequest<I> req(*ictx, c, off, len, buf, op_flags); + Extents &&image_extents, + ReadResult &&read_result, int op_flags) { + ImageReadRequest<I> req(*ictx, c, std::move(image_extents), + std::move(read_result), op_flags); req.send(); } @@ -267,6 +182,11 @@ int ImageRequest<I>::clip_request() { } template <typename I> +void ImageRequest<I>::start_op() { + m_aio_comp->start_op(); +} + +template <typename I> void ImageRequest<I>::fail(int r) { AioCompletion *aio_comp = this->m_aio_comp; aio_comp->get(); @@ -274,6 +194,16 @@ void ImageRequest<I>::fail(int r) { } template <typename I> +ImageReadRequest<I>::ImageReadRequest(I &image_ctx, AioCompletion *aio_comp, + Extents &&image_extents, + ReadResult &&read_result, + int op_flags) + : ImageRequest<I>(image_ctx, aio_comp, std::move(image_extents)), + m_op_flags(op_flags) { + aio_comp->read_result = std::move(read_result); +} + +template <typename I> void ImageReadRequest<I>::send_request() { I &image_ctx = this->m_image_ctx; CephContext *cct = image_ctx.cct; @@ -306,10 +236,7 @@ void ImageReadRequest<I>::send_request() { buffer_ofs += extent.second; } } - - aio_comp->read_buf = m_buf; - aio_comp->read_buf_len = buffer_ofs; - aio_comp->read_bl = m_pbl; + aio_comp->read_result.set_clip_length(buffer_ofs); // pre-calculate the expected number of read requests uint32_t request_count = 0; @@ -325,12 +252,13 @@ void ImageReadRequest<I>::send_request() { << extent.length << " from " << extent.buffer_extents << dendl; - C_AioRead<I> *req_comp = new C_AioRead<I>(aio_comp); + auto req_comp = new io::ReadResult::C_SparseReadRequest<I>( + aio_comp); ObjectReadRequest<I> *req = ObjectReadRequest<I>::create( &image_ctx, extent.oid.name, extent.objectno, extent.offset, extent.length, extent.buffer_extents, snap_id, true, req_comp, m_op_flags); - req_comp->set_req(req); + req_comp->request = req; if (image_ctx.object_cacher) { C_ObjectCacheRead<I> *cache_comp = new C_ObjectCacheRead<I>(image_ctx, @@ -357,10 +285,11 @@ void ImageReadRequest<I>::send_image_cache_request() { AioCompletion *aio_comp = this->m_aio_comp; aio_comp->set_request_count(1); - C_ImageCacheRead<I> *req_comp = new C_ImageCacheRead<I>( + + auto *req_comp = new io::ReadResult::C_ImageReadRequest( aio_comp, this->m_image_extents); image_ctx.image_cache->aio_read(std::move(this->m_image_extents), - &req_comp->get_data(), m_op_flags, + &req_comp->bl, m_op_flags, req_comp); } diff --git a/src/librbd/io/ImageRequest.h b/src/librbd/io/ImageRequest.h index 713425b4c67..86ab4bf9aee 100644 --- a/src/librbd/io/ImageRequest.h +++ b/src/librbd/io/ImageRequest.h @@ -8,7 +8,7 @@ #include "include/buffer_fwd.h" #include "common/snap_types.h" #include "osd/osd_types.h" -#include "librbd/io/AioCompletion.h" +#include "librbd/io/Types.h" #include <list> #include <utility> #include <vector> @@ -20,6 +20,7 @@ namespace io { class AioCompletion; class ObjectRequestHandle; +class ReadResult; template <typename ImageCtxT = ImageCtx> class ImageRequest { @@ -29,10 +30,8 @@ public: virtual ~ImageRequest() {} static void aio_read(ImageCtxT *ictx, AioCompletion *c, - Extents &&image_extents, char *buf, bufferlist *pbl, + Extents &&image_extents, ReadResult &&read_result, int op_flags); - static void aio_write(ImageCtxT *ictx, AioCompletion *c, uint64_t off, - size_t len, const char *buf, int op_flags); static void aio_write(ImageCtxT *ictx, AioCompletion *c, Extents &&image_extents, bufferlist &&bl, int op_flags); static void aio_discard(ImageCtxT *ictx, AioCompletion *c, uint64_t off, @@ -43,9 +42,7 @@ public: return false; } - void start_op() { - m_aio_comp->start_op(); - } + void start_op(); void send(); void fail(int r); @@ -82,11 +79,8 @@ public: using typename ImageRequest<ImageCtxT>::Extents; ImageReadRequest(ImageCtxT &image_ctx, AioCompletion *aio_comp, - Extents &&image_extents, char *buf, bufferlist *pbl, - int op_flags) - : ImageRequest<ImageCtxT>(image_ctx, aio_comp, std::move(image_extents)), - m_buf(buf), m_pbl(pbl), m_op_flags(op_flags) { - } + Extents &&image_extents, ReadResult &&read_result, + int op_flags); protected: virtual void send_request() override; @@ -157,12 +151,6 @@ class ImageWriteRequest : public AbstractImageWriteRequest<ImageCtxT> { public: using typename ImageRequest<ImageCtxT>::Extents; - ImageWriteRequest(ImageCtxT &image_ctx, AioCompletion *aio_comp, uint64_t off, - size_t len, const char *buf, int op_flags) - : AbstractImageWriteRequest<ImageCtxT>(image_ctx, aio_comp, {{off, len}}), - m_op_flags(op_flags) { - m_bl.append(buf, len); - } ImageWriteRequest(ImageCtxT &image_ctx, AioCompletion *aio_comp, Extents &&image_extents, bufferlist &&bl, int op_flags) : AbstractImageWriteRequest<ImageCtxT>(image_ctx, aio_comp, diff --git a/src/librbd/io/ImageRequestWQ.cc b/src/librbd/io/ImageRequestWQ.cc index fcc98347aa8..9999503d3e5 100644 --- a/src/librbd/io/ImageRequestWQ.cc +++ b/src/librbd/io/ImageRequestWQ.cc @@ -32,20 +32,20 @@ ImageRequestWQ::ImageRequestWQ(ImageCtx *image_ctx, const string &name, tp->add_work_queue(this); } -ssize_t ImageRequestWQ::read(uint64_t off, uint64_t len, char *buf, - int op_flags) { +ssize_t ImageRequestWQ::read(uint64_t off, uint64_t len, + ReadResult &&read_result, int op_flags) { CephContext *cct = m_image_ctx.cct; ldout(cct, 20) << "read: ictx=" << &m_image_ctx << ", off=" << off << ", " << "len = " << len << dendl; C_SaferCond cond; AioCompletion *c = AioCompletion::create(&cond); - aio_read(c, off, len, buf, NULL, op_flags, false); + aio_read(c, off, len, std::move(read_result), op_flags, false); return cond.wait(); } -ssize_t ImageRequestWQ::write(uint64_t off, uint64_t len, const char *buf, - int op_flags) { +ssize_t ImageRequestWQ::write(uint64_t off, uint64_t len, + bufferlist &&bl, int op_flags) { CephContext *cct = m_image_ctx.cct; ldout(cct, 20) << "write: ictx=" << &m_image_ctx << ", off=" << off << ", " << "len = " << len << dendl; @@ -60,7 +60,7 @@ ssize_t ImageRequestWQ::write(uint64_t off, uint64_t len, const char *buf, C_SaferCond cond; AioCompletion *c = AioCompletion::create(&cond); - aio_write(c, off, len, buf, op_flags, false); + aio_write(c, off, len, std::move(bl), op_flags, false); r = cond.wait(); if (r < 0) { @@ -94,7 +94,7 @@ int ImageRequestWQ::discard(uint64_t off, uint64_t len) { } void ImageRequestWQ::aio_read(AioCompletion *c, uint64_t off, uint64_t len, - char *buf, bufferlist *pbl, int op_flags, + ReadResult &&read_result, int op_flags, bool native_async) { c->init_time(&m_image_ctx, AIO_TYPE_READ); CephContext *cct = m_image_ctx.cct; @@ -122,18 +122,18 @@ void ImageRequestWQ::aio_read(AioCompletion *c, uint64_t off, uint64_t len, if (m_image_ctx.non_blocking_aio || writes_blocked() || !writes_empty() || lock_required) { - queue(new ImageReadRequest<>(m_image_ctx, c, {{off, len}}, buf, pbl, - op_flags)); + queue(new ImageReadRequest<>(m_image_ctx, c, {{off, len}}, + std::move(read_result), op_flags)); } else { c->start_op(); - ImageRequest<>::aio_read(&m_image_ctx, c, {{off, len}}, buf, pbl, - op_flags); + ImageRequest<>::aio_read(&m_image_ctx, c, {{off, len}}, + std::move(read_result), op_flags); finish_in_flight_op(); } } void ImageRequestWQ::aio_write(AioCompletion *c, uint64_t off, uint64_t len, - const char *buf, int op_flags, + bufferlist &&bl, int op_flags, bool native_async) { c->init_time(&m_image_ctx, AIO_TYPE_WRITE); CephContext *cct = m_image_ctx.cct; @@ -151,10 +151,12 @@ void ImageRequestWQ::aio_write(AioCompletion *c, uint64_t off, uint64_t len, RWLock::RLocker owner_locker(m_image_ctx.owner_lock); if (m_image_ctx.non_blocking_aio || writes_blocked()) { - queue(new ImageWriteRequest<>(m_image_ctx, c, off, len, buf, op_flags)); + queue(new ImageWriteRequest<>(m_image_ctx, c, {{off, len}}, + std::move(bl), op_flags)); } else { c->start_op(); - ImageRequest<>::aio_write(&m_image_ctx, c, off, len, buf, op_flags); + ImageRequest<>::aio_write(&m_image_ctx, c, {{off, len}}, + std::move(bl), op_flags); finish_in_flight_op(); } } diff --git a/src/librbd/io/ImageRequestWQ.h b/src/librbd/io/ImageRequestWQ.h index 6e93ed178dd..79f5301a683 100644 --- a/src/librbd/io/ImageRequestWQ.h +++ b/src/librbd/io/ImageRequestWQ.h @@ -18,20 +18,22 @@ namespace io { class AioCompletion; template <typename> class ImageRequest; +class ReadResult; class ImageRequestWQ : protected ThreadPool::PointerWQ<ImageRequest<ImageCtx> > { public: ImageRequestWQ(ImageCtx *image_ctx, const string &name, time_t ti, ThreadPool *tp); - ssize_t read(uint64_t off, uint64_t len, char *buf, int op_flags); - ssize_t write(uint64_t off, uint64_t len, const char *buf, int op_flags); + ssize_t read(uint64_t off, uint64_t len, ReadResult &&read_result, + int op_flags); + ssize_t write(uint64_t off, uint64_t len, bufferlist &&bl, int op_flags); int discard(uint64_t off, uint64_t len); - void aio_read(AioCompletion *c, uint64_t off, uint64_t len, char *buf, - bufferlist *pbl, int op_flags, bool native_async=true); - void aio_write(AioCompletion *c, uint64_t off, uint64_t len, const char *buf, - int op_flags, bool native_async=true); + void aio_read(AioCompletion *c, uint64_t off, uint64_t len, + ReadResult &&read_result, int op_flags, bool native_async=true); + void aio_write(AioCompletion *c, uint64_t off, uint64_t len, + bufferlist &&bl, int op_flags, bool native_async=true); void aio_discard(AioCompletion *c, uint64_t off, uint64_t len, bool native_async=true); void aio_flush(AioCompletion *c, bool native_async=true); diff --git a/src/librbd/io/ObjectRequest.cc b/src/librbd/io/ObjectRequest.cc index 8002c843ed5..8352b2695aa 100644 --- a/src/librbd/io/ObjectRequest.cc +++ b/src/librbd/io/ObjectRequest.cc @@ -17,6 +17,7 @@ #include "librbd/io/AioCompletion.h" #include "librbd/io/CopyupRequest.h" #include "librbd/io/ImageRequest.h" +#include "librbd/io/ReadResult.h" #include <boost/bind.hpp> #include <boost/optional.hpp> @@ -327,7 +328,8 @@ void ObjectReadRequest<I>::read_from_parent(Extents&& parent_extents) << " extents " << parent_extents << dendl; ImageRequest<>::aio_read(image_ctx->parent, parent_completion, - std::move(parent_extents), nullptr, &m_read_data, 0); + std::move(parent_extents), + ReadResult{&m_read_data}, 0); } /** write **/ diff --git a/src/librbd/io/ReadResult.cc b/src/librbd/io/ReadResult.cc new file mode 100644 index 00000000000..433dac73ca8 --- /dev/null +++ b/src/librbd/io/ReadResult.cc @@ -0,0 +1,165 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#include "librbd/io/ReadResult.h" +#include "include/buffer.h" +#include "common/dout.h" +#include "librbd/io/AioCompletion.h" +#include <boost/variant/apply_visitor.hpp> +#include <boost/variant/static_visitor.hpp> + +#define dout_subsys ceph_subsys_rbd +#undef dout_prefix +#define dout_prefix *_dout << "librbd::io::ReadResult: " + +namespace librbd { +namespace io { + +struct ReadResult::SetClipLengthVisitor : public boost::static_visitor<void> { + size_t length; + + SetClipLengthVisitor(size_t length) : length(length) { + } + + void operator()(Linear &linear) const { + assert(length <= linear.buf_len); + linear.buf_len = length; + } + + template <typename T> + void operator()(T &t) const { + } +}; + +struct ReadResult::AssembleResultVisitor : public boost::static_visitor<void> { + CephContext *cct; + Striper::StripedReadResult &destriper; + + AssembleResultVisitor(CephContext *cct, Striper::StripedReadResult &destriper) + : cct(cct), destriper(destriper) { + } + + void operator()(Empty &empty) const { + ldout(cct, 20) << "dropping read result" << dendl; + } + + void operator()(Linear &linear) const { + ldout(cct, 20) << "copying resulting bytes to " + << reinterpret_cast<void*>(linear.buf) << dendl; + destriper.assemble_result(cct, linear.buf, linear.buf_len); + } + + void operator()(Vector &vector) const { + bufferlist bl; + destriper.assemble_result(cct, bl, true); + + ldout(cct, 20) << "copying resulting " << bl.length() << " bytes to iovec " + << reinterpret_cast<const void*>(vector.iov) << dendl; + + bufferlist::iterator it = bl.begin(); + size_t length = bl.length(); + size_t offset = 0; + int idx = 0; + for (; offset < length && idx < vector.iov_count; idx++) { + size_t len = MIN(vector.iov[idx].iov_len, length - offset); + it.copy(len, static_cast<char *>(vector.iov[idx].iov_base)); + offset += len; + } + assert(offset == bl.length()); + } + + void operator()(Bufferlist &bufferlist) const { + bufferlist.bl->clear(); + destriper.assemble_result(cct, *bufferlist.bl, true); + + ldout(cct, 20) << "moved resulting " << bufferlist.bl->length() << " " + << "bytes to bl " << reinterpret_cast<void*>(bufferlist.bl) + << dendl; + } +}; + +ReadResult::C_ReadRequest::C_ReadRequest(AioCompletion *aio_completion) + : aio_completion(aio_completion) { + aio_completion->add_request(); +} + +void ReadResult::C_ReadRequest::finish(int r) { + aio_completion->complete_request(r); +} + +void ReadResult::C_ImageReadRequest::finish(int r) { + CephContext *cct = aio_completion->ictx->cct; + ldout(cct, 10) << "C_ImageReadRequest::finish() " << this << ": r=" << r + << dendl; + if (r >= 0) { + size_t length = 0; + for (auto &image_extent : image_extents) { + length += image_extent.second; + } + assert(length == bl.length()); + + aio_completion->lock.Lock(); + aio_completion->read_result.m_destriper.add_partial_result( + cct, bl, image_extents); + aio_completion->lock.Unlock(); + r = length; + } + + C_ReadRequest::finish(r); +} + +void ReadResult::C_SparseReadRequestBase::finish(ExtentMap &extent_map, + const Extents &buffer_extents, + uint64_t offset, size_t length, + bufferlist &bl, int r) { + aio_completion->lock.Lock(); + CephContext *cct = aio_completion->ictx->cct; + ldout(cct, 10) << "C_SparseReadRequest::finish() " << this << " r = " << r + << dendl; + + if (r >= 0 || r == -ENOENT) { // this was a sparse_read operation + ldout(cct, 10) << " got " << extent_map + << " for " << buffer_extents + << " bl " << bl.length() << dendl; + // reads from the parent don't populate the m_ext_map and the overlap + // may not be the full buffer. compensate here by filling in m_ext_map + // with the read extent when it is empty. + if (extent_map.empty()) { + extent_map[offset] = bl.length(); + } + + aio_completion->read_result.m_destriper.add_partial_sparse_result( + cct, bl, extent_map, offset, buffer_extents); + r = length; + } + aio_completion->lock.Unlock(); + + C_ReadRequest::finish(r); +} + +ReadResult::ReadResult() : m_buffer(Empty()) { +} + +ReadResult::ReadResult(char *buf, size_t buf_len) + : m_buffer(Linear(buf, buf_len)) { +} + +ReadResult::ReadResult(const struct iovec *iov, int iov_count) + : m_buffer(Vector(iov, iov_count)) { +} + +ReadResult::ReadResult(ceph::bufferlist *bl) + : m_buffer(Bufferlist(bl)) { +} + +void ReadResult::set_clip_length(size_t length) { + boost::apply_visitor(SetClipLengthVisitor(length), m_buffer); +} + +void ReadResult::assemble_result(CephContext *cct) { + boost::apply_visitor(AssembleResultVisitor(cct, m_destriper), m_buffer); +} + +} // namespace io +} // namespace librbd + diff --git a/src/librbd/io/ReadResult.h b/src/librbd/io/ReadResult.h new file mode 100644 index 00000000000..24a8f256c59 --- /dev/null +++ b/src/librbd/io/ReadResult.h @@ -0,0 +1,126 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#ifndef CEPH_LIBRBD_IO_READ_RESULT_H +#define CEPH_LIBRBD_IO_READ_RESULT_H + +#include "include/int_types.h" +#include "include/buffer_fwd.h" +#include "include/Context.h" +#include "librbd/io/Types.h" +#include "osdc/Striper.h" +#include <sys/uio.h> +#include <boost/variant/variant.hpp> + +struct CephContext; + +namespace librbd { +namespace io { + +struct AioCompletion; +template <typename> struct ObjectReadRequest; + +class ReadResult { +private: + struct C_ReadRequest : public Context { + AioCompletion *aio_completion; + bufferlist bl; + + C_ReadRequest(AioCompletion *aio_completion); + + void finish(int r) override; + }; + +public: + + struct C_ImageReadRequest : public C_ReadRequest { + Extents image_extents; + + C_ImageReadRequest(AioCompletion *aio_completion, + const Extents image_extents) + : C_ReadRequest(aio_completion), image_extents(image_extents) { + } + + void finish(int r) override; + }; + + struct C_SparseReadRequestBase : public C_ReadRequest { + C_SparseReadRequestBase(AioCompletion *aio_completion) + : C_ReadRequest(aio_completion) { + } + + using C_ReadRequest::finish; + void finish(ExtentMap &extent_map, const Extents &buffer_extents, + uint64_t offset, size_t length, bufferlist &bl, int r); + }; + + template <typename ImageCtxT> + struct C_SparseReadRequest : public C_SparseReadRequestBase { + ObjectReadRequest<ImageCtxT> *request; + + C_SparseReadRequest(AioCompletion *aio_completion) + : C_SparseReadRequestBase(aio_completion) { + } + + void finish(int r) override { + C_SparseReadRequestBase::finish(request->get_extent_map(), + request->get_buffer_extents(), + request->get_offset(), + request->get_length(), request->data(), + r); + } + }; + + ReadResult(); + ReadResult(char *buf, size_t buf_len); + ReadResult(const struct iovec *iov, int iov_count); + ReadResult(ceph::bufferlist *bl); + + void set_clip_length(size_t length); + void assemble_result(CephContext *cct); + +private: + struct Empty { + }; + + struct Linear { + char *buf; + size_t buf_len; + + Linear(char *buf, size_t buf_len) : buf(buf), buf_len(buf_len) { + } + }; + + struct Vector { + const struct iovec *iov; + int iov_count; + + Vector(const struct iovec *iov, int iov_count) + : iov(iov), iov_count(iov_count) { + } + }; + + struct Bufferlist { + ceph::bufferlist *bl; + + Bufferlist(ceph::bufferlist *bl) : bl(bl) { + } + }; + + typedef boost::variant<Empty, + Linear, + Vector, + Bufferlist> Buffer; + struct SetClipLengthVisitor; + struct AssembleResultVisitor; + + Buffer m_buffer; + Striper::StripedReadResult m_destriper; + +}; + +} // namespace io +} // namespace librbd + +#endif // CEPH_LIBRBD_IO_READ_RESULT_H + diff --git a/src/librbd/journal/Replay.cc b/src/librbd/journal/Replay.cc index 3049bb86388..456ec9c0301 100644 --- a/src/librbd/journal/Replay.cc +++ b/src/librbd/journal/Replay.cc @@ -336,8 +336,9 @@ void Replay<I>::handle_event(const journal::AioWriteEvent &event, auto aio_comp = create_aio_modify_completion(on_ready, on_safe, io::AIO_TYPE_WRITE, &flush_required); - io::ImageRequest<I>::aio_write(&m_image_ctx, aio_comp, event.offset, - event.length, data.c_str(), 0); + io::ImageRequest<I>::aio_write(&m_image_ctx, aio_comp, + {{event.offset, event.length}}, + std::move(data), 0); if (flush_required) { m_lock.Lock(); auto flush_comp = create_aio_flush_completion(nullptr); diff --git a/src/librbd/librbd.cc b/src/librbd/librbd.cc index b8936f9af02..35bbcf7db71 100644 --- a/src/librbd/librbd.cc +++ b/src/librbd/librbd.cc @@ -30,6 +30,7 @@ #include "librbd/Operations.h" #include "librbd/io/AioCompletion.h" #include "librbd/io/ImageRequestWQ.h" +#include "librbd/io/ReadResult.h" #include <algorithm> #include <string> #include <vector> @@ -59,6 +60,13 @@ namespace { TracepointProvider::Traits tracepoint_traits("librbd_tp.so", "rbd_tracing"); +buffer::raw* create_write_raw(librbd::ImageCtx *ictx, const char *buf, + size_t len) { + // TODO: until librados can guarantee memory won't be referenced after + // it ACKs a request, always make a copy of the user-provided memory + return buffer::copy(buf, len); +} + CephContext* get_cct(IoCtx &io_ctx) { return reinterpret_cast<CephContext*>(io_ctx.cct()); } @@ -1218,7 +1226,7 @@ namespace librbd { tracepoint(librbd, read_enter, ictx, ictx->name.c_str(), ictx->snap_name.c_str(), ictx->read_only, ofs, len); bufferptr ptr(len); bl.push_back(std::move(ptr)); - int r = ictx->io_work_queue->read(ofs, len, bl.c_str(), 0); + int r = ictx->io_work_queue->read(ofs, len, io::ReadResult{&bl}, 0); tracepoint(librbd, read_exit, r); return r; } @@ -1230,7 +1238,7 @@ namespace librbd { ictx->read_only, ofs, len, op_flags); bufferptr ptr(len); bl.push_back(std::move(ptr)); - int r = ictx->io_work_queue->read(ofs, len, bl.c_str(), op_flags); + int r = ictx->io_work_queue->read(ofs, len, io::ReadResult{&bl}, op_flags); tracepoint(librbd, read_exit, r); return r; } @@ -1296,7 +1304,7 @@ namespace librbd { tracepoint(librbd, write_exit, -EINVAL); return -EINVAL; } - int r = ictx->io_work_queue->write(ofs, len, bl.c_str(), 0); + int r = ictx->io_work_queue->write(ofs, len, bufferlist{bl}, 0); tracepoint(librbd, write_exit, r); return r; } @@ -1310,7 +1318,7 @@ namespace librbd { tracepoint(librbd, write_exit, -EINVAL); return -EINVAL; } - int r = ictx->io_work_queue->write(ofs, len, bl.c_str(), op_flags); + int r = ictx->io_work_queue->write(ofs, len, bufferlist{bl}, op_flags); tracepoint(librbd, write_exit, r); return r; } @@ -1337,8 +1345,8 @@ namespace librbd { tracepoint(librbd, aio_write_exit, -EINVAL); return -EINVAL; } - ictx->io_work_queue->aio_write(get_aio_completion(c), off, len, bl.c_str(), - 0); + ictx->io_work_queue->aio_write(get_aio_completion(c), off, len, + bufferlist{bl}, 0); tracepoint(librbd, aio_write_exit, 0); return 0; } @@ -1353,8 +1361,8 @@ namespace librbd { tracepoint(librbd, aio_write_exit, -EINVAL); return -EINVAL; } - ictx->io_work_queue->aio_write(get_aio_completion(c), off, len, bl.c_str(), - op_flags); + ictx->io_work_queue->aio_write(get_aio_completion(c), off, len, + bufferlist{bl}, op_flags); tracepoint(librbd, aio_write_exit, 0); return 0; } @@ -1375,8 +1383,8 @@ namespace librbd { tracepoint(librbd, aio_read_enter, ictx, ictx->name.c_str(), ictx->snap_name.c_str(), ictx->read_only, off, len, bl.c_str(), c->pc); ldout(ictx->cct, 10) << "Image::aio_read() buf=" << (void *)bl.c_str() << "~" << (void *)(bl.c_str() + len - 1) << dendl; - ictx->io_work_queue->aio_read(get_aio_completion(c), off, len, NULL, &bl, - 0); + ictx->io_work_queue->aio_read(get_aio_completion(c), off, len, + io::ReadResult{&bl}, 0); tracepoint(librbd, aio_read_exit, 0); return 0; } @@ -1389,8 +1397,8 @@ namespace librbd { ictx->read_only, off, len, bl.c_str(), c->pc, op_flags); ldout(ictx->cct, 10) << "Image::aio_read() buf=" << (void *)bl.c_str() << "~" << (void *)(bl.c_str() + len - 1) << dendl; - ictx->io_work_queue->aio_read(get_aio_completion(c), off, len, NULL, &bl, - op_flags); + ictx->io_work_queue->aio_read(get_aio_completion(c), off, len, + io::ReadResult{&bl}, op_flags); tracepoint(librbd, aio_read_exit, 0); return 0; } @@ -2782,7 +2790,8 @@ extern "C" ssize_t rbd_read(rbd_image_t image, uint64_t ofs, size_t len, { librbd::ImageCtx *ictx = (librbd::ImageCtx *)image; tracepoint(librbd, read_enter, ictx, ictx->name.c_str(), ictx->snap_name.c_str(), ictx->read_only, ofs, len); - int r = ictx->io_work_queue->read(ofs, len, buf, 0); + int r = ictx->io_work_queue->read(ofs, len, librbd::io::ReadResult{buf, len}, + 0); tracepoint(librbd, read_exit, r); return r; } @@ -2793,7 +2802,8 @@ extern "C" ssize_t rbd_read2(rbd_image_t image, uint64_t ofs, size_t len, librbd::ImageCtx *ictx = (librbd::ImageCtx *)image; tracepoint(librbd, read2_enter, ictx, ictx->name.c_str(), ictx->snap_name.c_str(), ictx->read_only, ofs, len, op_flags); - int r = ictx->io_work_queue->read(ofs, len, buf, op_flags); + int r = ictx->io_work_queue->read(ofs, len, librbd::io::ReadResult{buf, len}, + op_flags); tracepoint(librbd, read_exit, r); return r; } @@ -2860,7 +2870,10 @@ extern "C" ssize_t rbd_write(rbd_image_t image, uint64_t ofs, size_t len, { librbd::ImageCtx *ictx = (librbd::ImageCtx *)image; tracepoint(librbd, write_enter, ictx, ictx->name.c_str(), ictx->snap_name.c_str(), ictx->read_only, ofs, len, buf); - int r = ictx->io_work_queue->write(ofs, len, buf, 0); + + bufferlist bl; + bl.push_back(create_write_raw(ictx, buf, len)); + int r = ictx->io_work_queue->write(ofs, len, std::move(bl), 0); tracepoint(librbd, write_exit, r); return r; } @@ -2871,7 +2884,10 @@ extern "C" ssize_t rbd_write2(rbd_image_t image, uint64_t ofs, size_t len, librbd::ImageCtx *ictx = (librbd::ImageCtx *)image; tracepoint(librbd, write2_enter, ictx, ictx->name.c_str(), ictx->snap_name.c_str(), ictx->read_only, ofs, len, buf, op_flags); - int r = ictx->io_work_queue->write(ofs, len, buf, op_flags); + + bufferlist bl; + bl.push_back(create_write_raw(ictx, buf, len)); + int r = ictx->io_work_queue->write(ofs, len, std::move(bl), op_flags); tracepoint(librbd, write_exit, r); return r; } @@ -2902,7 +2918,11 @@ extern "C" int rbd_aio_write(rbd_image_t image, uint64_t off, size_t len, librbd::ImageCtx *ictx = (librbd::ImageCtx *)image; librbd::RBD::AioCompletion *comp = (librbd::RBD::AioCompletion *)c; tracepoint(librbd, aio_write_enter, ictx, ictx->name.c_str(), ictx->snap_name.c_str(), ictx->read_only, off, len, buf, comp->pc); - ictx->io_work_queue->aio_write(get_aio_completion(comp), off, len, buf, 0); + + bufferlist bl; + bl.push_back(create_write_raw(ictx, buf, len)); + ictx->io_work_queue->aio_write(get_aio_completion(comp), off, len, + std::move(bl), 0); tracepoint(librbd, aio_write_exit, 0); return 0; } @@ -2914,8 +2934,11 @@ extern "C" int rbd_aio_write2(rbd_image_t image, uint64_t off, size_t len, librbd::RBD::AioCompletion *comp = (librbd::RBD::AioCompletion *)c; tracepoint(librbd, aio_write2_enter, ictx, ictx->name.c_str(), ictx->snap_name.c_str(), ictx->read_only, off, len, buf, comp->pc, op_flags); - ictx->io_work_queue->aio_write(get_aio_completion(comp), off, len, buf, - op_flags); + + bufferlist bl; + bl.push_back(create_write_raw(ictx, buf, len)); + ictx->io_work_queue->aio_write(get_aio_completion(comp), off, len, + std::move(bl), op_flags); tracepoint(librbd, aio_write_exit, 0); return 0; } @@ -2938,8 +2961,8 @@ extern "C" int rbd_aio_read(rbd_image_t image, uint64_t off, size_t len, librbd::ImageCtx *ictx = (librbd::ImageCtx *)image; librbd::RBD::AioCompletion *comp = (librbd::RBD::AioCompletion *)c; tracepoint(librbd, aio_read_enter, ictx, ictx->name.c_str(), ictx->snap_name.c_str(), ictx->read_only, off, len, buf, comp->pc); - ictx->io_work_queue->aio_read(get_aio_completion(comp), off, len, buf, NULL, - 0); + ictx->io_work_queue->aio_read(get_aio_completion(comp), off, len, + librbd::io::ReadResult{buf, len}, 0); tracepoint(librbd, aio_read_exit, 0); return 0; } @@ -2951,8 +2974,8 @@ extern "C" int rbd_aio_read2(rbd_image_t image, uint64_t off, size_t len, librbd::RBD::AioCompletion *comp = (librbd::RBD::AioCompletion *)c; tracepoint(librbd, aio_read2_enter, ictx, ictx->name.c_str(), ictx->snap_name.c_str(), ictx->read_only, off, len, buf, comp->pc, op_flags); - ictx->io_work_queue->aio_read(get_aio_completion(comp), off, len, buf, NULL, - op_flags); + ictx->io_work_queue->aio_read(get_aio_completion(comp), off, len, + librbd::io::ReadResult{buf, len},op_flags); tracepoint(librbd, aio_read_exit, 0); return 0; } diff --git a/src/test/librbd/io/test_mock_ImageRequest.cc b/src/test/librbd/io/test_mock_ImageRequest.cc index 55dd7549c0f..de27ef8b1ed 100644 --- a/src/test/librbd/io/test_mock_ImageRequest.cc +++ b/src/test/librbd/io/test_mock_ImageRequest.cc @@ -202,8 +202,11 @@ TEST_F(TestMockIoImageRequest, AioWriteJournalAppendDisabled) { C_SaferCond aio_comp_ctx; AioCompletion *aio_comp = AioCompletion::create_and_start( &aio_comp_ctx, ictx, AIO_TYPE_WRITE); - MockImageWriteRequest mock_aio_image_write(mock_image_ctx, aio_comp, 0, 1, "1", - 0); + + bufferlist bl; + bl.append("1"); + MockImageWriteRequest mock_aio_image_write(mock_image_ctx, aio_comp, + {{0, 1}}, std::move(bl), 0); { RWLock::RLocker owner_locker(mock_image_ctx.owner_lock); mock_aio_image_write.send(); diff --git a/src/test/librbd/journal/test_Entries.cc b/src/test/librbd/journal/test_Entries.cc index 8506a6f7e4e..3345394dda8 100644 --- a/src/test/librbd/journal/test_Entries.cc +++ b/src/test/librbd/journal/test_Entries.cc @@ -126,10 +126,13 @@ TEST_F(TestJournalEntries, AioWrite) { ASSERT_TRUE(journaler != NULL); std::string buffer(512, '1'); + bufferlist write_bl; + write_bl.append(buffer); + C_SaferCond cond_ctx; auto c = librbd::io::AioCompletion::create(&cond_ctx); c->get(); - ictx->io_work_queue->aio_write(c, 123, buffer.size(), buffer.c_str(), 0); + ictx->io_work_queue->aio_write(c, 123, buffer.size(), std::move(write_bl), 0); ASSERT_EQ(0, c->wait_for_complete()); c->put(); diff --git a/src/test/librbd/journal/test_Replay.cc b/src/test/librbd/journal/test_Replay.cc index e2d4425f7b8..19ec52e5e8a 100644 --- a/src/test/librbd/journal/test_Replay.cc +++ b/src/test/librbd/journal/test_Replay.cc @@ -18,6 +18,7 @@ #include "librbd/io/AioCompletion.h" #include "librbd/io/ImageRequest.h" #include "librbd/io/ImageRequestWQ.h" +#include "librbd/io/ReadResult.h" #include "librbd/journal/Types.h" void register_test_journal_replay() { @@ -109,9 +110,11 @@ TEST_F(TestJournalReplay, AioDiscardEvent) { ictx->features &= ~RBD_FEATURE_JOURNALING; std::string payload(4096, '1'); + bufferlist payload_bl; + payload_bl.append(payload); auto aio_comp = new librbd::io::AioCompletion(); - ictx->io_work_queue->aio_write(aio_comp, 0, payload.size(), payload.c_str(), - 0); + ictx->io_work_queue->aio_write(aio_comp, 0, payload.size(), + std::move(payload_bl), 0); ASSERT_EQ(0, aio_comp->wait_for_complete()); aio_comp->release(); @@ -121,9 +124,10 @@ TEST_F(TestJournalReplay, AioDiscardEvent) { aio_comp->release(); std::string read_payload(4096, '\0'); + librbd::io::ReadResult read_result{&read_payload[0], read_payload.size()}; aio_comp = new librbd::io::AioCompletion(); ictx->io_work_queue->aio_read(aio_comp, 0, read_payload.size(), - &read_payload[0], NULL, 0); + librbd::io::ReadResult{read_result}, 0); ASSERT_EQ(0, aio_comp->wait_for_complete()); aio_comp->release(); ASSERT_EQ(payload, read_payload); @@ -148,7 +152,7 @@ TEST_F(TestJournalReplay, AioDiscardEvent) { aio_comp = new librbd::io::AioCompletion(); ictx->io_work_queue->aio_read(aio_comp, 0, read_payload.size(), - &read_payload[0], NULL, 0); + librbd::io::ReadResult{read_result}, 0); ASSERT_EQ(0, aio_comp->wait_for_complete()); aio_comp->release(); if (ictx->cct->_conf->rbd_skip_partial_discard) { @@ -209,9 +213,10 @@ TEST_F(TestJournalReplay, AioWriteEvent) { ASSERT_EQ(0, when_acquired_lock(ictx)); std::string read_payload(4096, '\0'); + librbd::io::ReadResult read_result{&read_payload[0], read_payload.size()}; auto aio_comp = new librbd::io::AioCompletion(); ictx->io_work_queue->aio_read(aio_comp, 0, read_payload.size(), - &read_payload[0], NULL, 0); + std::move(read_result), 0); ASSERT_EQ(0, aio_comp->wait_for_complete()); aio_comp->release(); ASSERT_EQ(payload, read_payload); @@ -238,8 +243,8 @@ TEST_F(TestJournalReplay, AioWriteEvent) { // verify lock ordering constraints aio_comp = new librbd::io::AioCompletion(); - ictx->io_work_queue->aio_write(aio_comp, 0, payload.size(), payload.c_str(), - 0); + ictx->io_work_queue->aio_write(aio_comp, 0, payload.size(), + bufferlist{payload_bl}, 0); ASSERT_EQ(0, aio_comp->wait_for_complete()); aio_comp->release(); } @@ -782,9 +787,11 @@ TEST_F(TestJournalReplay, ObjectPosition) { get_journal_commit_position(ictx, &initial_tag, &initial_entry); std::string payload(4096, '1'); + bufferlist payload_bl; + payload_bl.append(payload); auto aio_comp = new librbd::io::AioCompletion(); - ictx->io_work_queue->aio_write(aio_comp, 0, payload.size(), payload.c_str(), - 0); + ictx->io_work_queue->aio_write(aio_comp, 0, payload.size(), + bufferlist{payload_bl}, 0); ASSERT_EQ(0, aio_comp->wait_for_complete()); aio_comp->release(); @@ -803,8 +810,8 @@ TEST_F(TestJournalReplay, ObjectPosition) { // write again aio_comp = new librbd::io::AioCompletion(); - ictx->io_work_queue->aio_write(aio_comp, 0, payload.size(), payload.c_str(), - 0); + ictx->io_work_queue->aio_write(aio_comp, 0, payload.size(), + bufferlist{payload_bl}, 0); ASSERT_EQ(0, aio_comp->wait_for_complete()); aio_comp->release(); diff --git a/src/test/librbd/journal/test_mock_Replay.cc b/src/test/librbd/journal/test_mock_Replay.cc index a3847e3a69e..4a3d7199c82 100644 --- a/src/test/librbd/journal/test_mock_Replay.cc +++ b/src/test/librbd/journal/test_mock_Replay.cc @@ -28,18 +28,18 @@ template <> struct ImageRequest<MockReplayImageCtx> { static ImageRequest *s_instance; - MOCK_METHOD5(aio_write, void(AioCompletion *c, uint64_t off, size_t len, - const char *buf, int op_flags)); + MOCK_METHOD4(aio_write, void(AioCompletion *c, const Extents &image_extents, + const bufferlist &bl, int op_flags)); static void aio_write(MockReplayImageCtx *ictx, AioCompletion *c, - uint64_t off, size_t len, const char *buf, + Extents &&image_extents, bufferlist &&bl, int op_flags) { assert(s_instance != nullptr); - s_instance->aio_write(c, off, len, buf, op_flags); + s_instance->aio_write(c, image_extents, bl, op_flags); } MOCK_METHOD3(aio_discard, void(AioCompletion *c, uint64_t off, uint64_t len)); - static void aio_discard(MockReplayImageCtx *ictx, AioCompletion *c, uint64_t off, - uint64_t len) { + static void aio_discard(MockReplayImageCtx *ictx, AioCompletion *c, + uint64_t off, uint64_t len) { assert(s_instance != nullptr); s_instance->aio_discard(c, off, len); } @@ -81,6 +81,11 @@ using ::testing::SaveArg; using ::testing::StrEq; using ::testing::WithArgs; +MATCHER_P(BufferlistEqual, str, "") { + bufferlist bl(arg); + return (strncmp(bl.c_str(), str, strlen(str)) == 0); +} + MATCHER_P(CStrEq, str, "") { return (strncmp(arg, str, strlen(str)) == 0); } @@ -133,7 +138,7 @@ public: io::AioCompletion **aio_comp, uint64_t off, uint64_t len, const char *data) { EXPECT_CALL(mock_io_image_request, - aio_write(_, off, len, CStrEq(data), _)) + aio_write(_, io::Extents{{off, len}}, BufferlistEqual(data), _)) .WillOnce(SaveArg<0>(aio_comp)); } diff --git a/src/test/librbd/test_fixture.cc b/src/test/librbd/test_fixture.cc index bf3e149dfc8..38614c1432b 100644 --- a/src/test/librbd/test_fixture.cc +++ b/src/test/librbd/test_fixture.cc @@ -123,7 +123,7 @@ int TestFixture::unlock_image() { } int TestFixture::acquire_exclusive_lock(librbd::ImageCtx &ictx) { - int r = ictx.io_work_queue->write(0, 0, "", 0); + int r = ictx.io_work_queue->write(0, 0, {}, 0); if (r != 0) { return r; } diff --git a/src/test/librbd/test_internal.cc b/src/test/librbd/test_internal.cc index 743559be78d..b9eb60db11c 100644 --- a/src/test/librbd/test_internal.cc +++ b/src/test/librbd/test_internal.cc @@ -274,7 +274,10 @@ TEST_F(TestInternal, AioWriteRequestsLock) { Context *ctx = new DummyContext(); auto c = librbd::io::AioCompletion::create(ctx); c->get(); - ictx->io_work_queue->aio_write(c, 0, buffer.size(), buffer.c_str(), 0); + + bufferlist bl; + bl.append(buffer); + ictx->io_work_queue->aio_write(c, 0, buffer.size(), std::move(bl), 0); bool is_owner; ASSERT_EQ(0, librbd::is_exclusive_lock_owner(ictx, &is_owner)); @@ -493,7 +496,7 @@ TEST_F(TestInternal, SnapshotCopyup) bufferlist bl; bl.append(std::string(256, '1')); - ASSERT_EQ(256, ictx->io_work_queue->write(0, bl.length(), bl.c_str(), 0)); + ASSERT_EQ(256, ictx->io_work_queue->write(0, bl.length(), bufferlist{bl}, 0)); ASSERT_EQ(0, snap_create(*ictx, "snap1")); ASSERT_EQ(0, ictx->operations->snap_protect("snap1")); @@ -512,7 +515,8 @@ TEST_F(TestInternal, SnapshotCopyup) ASSERT_EQ(0, snap_create(*ictx2, "snap1")); ASSERT_EQ(0, snap_create(*ictx2, "snap2")); - ASSERT_EQ(256, ictx2->io_work_queue->write(256, bl.length(), bl.c_str(), 0)); + ASSERT_EQ(256, ictx2->io_work_queue->write(256, bl.length(), bufferlist{bl}, + 0)); librados::IoCtx snap_ctx; snap_ctx.dup(ictx2->data_ctx); @@ -536,15 +540,22 @@ TEST_F(TestInternal, SnapshotCopyup) read_bl.push_back(read_ptr); std::list<std::string> snaps = {"snap1", "snap2", ""}; + librbd::io::ReadResult read_result{&read_bl}; for (std::list<std::string>::iterator it = snaps.begin(); it != snaps.end(); ++it) { const char *snap_name = it->empty() ? NULL : it->c_str(); ASSERT_EQ(0, librbd::snap_set(ictx2, snap_name)); - ASSERT_EQ(256, ictx2->io_work_queue->read(0, 256, read_bl.c_str(), 0)); + ASSERT_EQ(256, + ictx2->io_work_queue->read(0, 256, + librbd::io::ReadResult{read_result}, + 0)); ASSERT_TRUE(bl.contents_equal(read_bl)); - ASSERT_EQ(256, ictx2->io_work_queue->read(256, 256, read_bl.c_str(), 0)); + ASSERT_EQ(256, + ictx2->io_work_queue->read(256, 256, + librbd::io::ReadResult{read_result}, + 0)); if (snap_name == NULL) { ASSERT_TRUE(bl.contents_equal(read_bl)); } else { @@ -590,7 +601,7 @@ TEST_F(TestInternal, ResizeCopyup) bl.append(std::string(4096, '1')); for (size_t i = 0; i < m_image_size; i += bl.length()) { ASSERT_EQ(bl.length(), ictx->io_work_queue->write(i, bl.length(), - bl.c_str(), 0)); + bufferlist{bl}, 0)); } ASSERT_EQ(0, snap_create(*ictx, "snap1")); @@ -622,9 +633,12 @@ TEST_F(TestInternal, ResizeCopyup) ictx2->snap_info.begin()->second.parent = librbd::parent_info(); } + librbd::io::ReadResult read_result{&read_bl}; for (size_t i = 2 << order; i < m_image_size; i += bl.length()) { - ASSERT_EQ(bl.length(), ictx2->io_work_queue->read(i, bl.length(), - read_bl.c_str(), 0)); + ASSERT_EQ(bl.length(), + ictx2->io_work_queue->read(i, bl.length(), + librbd::io::ReadResult{read_result}, + 0)); ASSERT_TRUE(bl.contents_equal(read_bl)); } } @@ -652,7 +666,7 @@ TEST_F(TestInternal, DiscardCopyup) bl.append(std::string(4096, '1')); for (size_t i = 0; i < m_image_size; i += bl.length()) { ASSERT_EQ(bl.length(), ictx->io_work_queue->write(i, bl.length(), - bl.c_str(), 0)); + bufferlist{bl}, 0)); } ASSERT_EQ(0, snap_create(*ictx, "snap1")); @@ -681,9 +695,12 @@ TEST_F(TestInternal, DiscardCopyup) ictx2->snap_info.begin()->second.parent = librbd::parent_info(); } + librbd::io::ReadResult read_result{&read_bl}; for (size_t i = 0; i < m_image_size; i += bl.length()) { - ASSERT_EQ(bl.length(), ictx2->io_work_queue->read(i, bl.length(), - read_bl.c_str(), 0)); + ASSERT_EQ(bl.length(), + ictx2->io_work_queue->read(i, bl.length(), + librbd::io::ReadResult{read_result}, + 0)); ASSERT_TRUE(bl.contents_equal(read_bl)); } } @@ -695,12 +712,14 @@ TEST_F(TestInternal, ShrinkFlushesCache) { std::string buffer(4096, '1'); // ensure write-path is initialized - ictx->io_work_queue->write(0, buffer.size(), buffer.c_str(), 0); + bufferlist write_bl; + write_bl.append(buffer); + ictx->io_work_queue->write(0, buffer.size(), bufferlist{write_bl}, 0); C_SaferCond cond_ctx; auto c = librbd::io::AioCompletion::create(&cond_ctx); c->get(); - ictx->io_work_queue->aio_write(c, 0, buffer.size(), buffer.c_str(), 0); + ictx->io_work_queue->aio_write(c, 0, buffer.size(), bufferlist{write_bl}, 0); librbd::NoOpProgressContext no_op; ASSERT_EQ(0, ictx->operations->resize(m_image_size >> 1, true, no_op)); @@ -790,7 +809,7 @@ TEST_F(TestInternal, WriteFullCopyup) { bufferlist bl; bl.append(std::string(1 << ictx->order, '1')); ASSERT_EQ(bl.length(), - ictx->io_work_queue->write(0, bl.length(), bl.c_str(), 0)); + ictx->io_work_queue->write(0, bl.length(), bufferlist{bl}, 0)); ASSERT_EQ(0, librbd::flush(ictx)); ASSERT_EQ(0, create_snapshot("snap1", true)); @@ -820,7 +839,7 @@ TEST_F(TestInternal, WriteFullCopyup) { write_full_bl.append(std::string(1 << ictx2->order, '2')); ASSERT_EQ(write_full_bl.length(), ictx2->io_work_queue->write(0, write_full_bl.length(), - write_full_bl.c_str(), 0)); + bufferlist{write_full_bl}, 0)); ASSERT_EQ(0, ictx2->operations->flatten(no_op)); @@ -828,13 +847,16 @@ TEST_F(TestInternal, WriteFullCopyup) { bufferlist read_bl; read_bl.push_back(read_ptr); - ASSERT_EQ(read_bl.length(), ictx2->io_work_queue->read(0, read_bl.length(), - read_bl.c_str(), 0)); + librbd::io::ReadResult read_result{&read_bl}; + ASSERT_EQ(read_bl.length(), + ictx2->io_work_queue->read(0, read_bl.length(), + librbd::io::ReadResult{read_result}, 0)); ASSERT_TRUE(write_full_bl.contents_equal(read_bl)); ASSERT_EQ(0, librbd::snap_set(ictx2, "snap1")); - ASSERT_EQ(read_bl.length(), ictx2->io_work_queue->read(0, read_bl.length(), - read_bl.c_str(), 0)); + ASSERT_EQ(read_bl.length(), + ictx2->io_work_queue->read(0, read_bl.length(), + librbd::io::ReadResult{read_result}, 0)); ASSERT_TRUE(bl.contents_equal(read_bl)); } diff --git a/src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc b/src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc index 8c5e92e3f21..fe77cb01bc3 100644 --- a/src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc +++ b/src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc @@ -10,6 +10,7 @@ #include "librbd/internal.h" #include "librbd/Operations.h" #include "librbd/io/ImageRequestWQ.h" +#include "librbd/io/ReadResult.h" #include "test/librados_test_stub/MockTestMemIoCtxImpl.h" #include "test/librbd/mock/MockImageCtx.h" #include "tools/rbd_mirror/Threads.h" @@ -55,7 +56,7 @@ void scribble(librbd::ImageCtx *image_ctx, int num_ops, size_t max_size, bufferlist bl; bl.append(std::string(len, '1')); - int r = image_ctx->io_work_queue->write(off, len, bl.c_str(), 0); + int r = image_ctx->io_work_queue->write(off, len, std::move(bl), 0); ASSERT_EQ(static_cast<int>(len), r); interval_set<uint64_t> w; @@ -265,16 +266,16 @@ public: bufferlist remote_bl; remote_bl.append(std::string(object_size, '1')); - r = m_remote_image_ctx->io_work_queue->read(0, object_size, - remote_bl.c_str(), 0); + r = m_remote_image_ctx->io_work_queue->read( + 0, object_size, librbd::io::ReadResult{&remote_bl}, 0); if (r < 0) { return r; } bufferlist local_bl; local_bl.append(std::string(object_size, '1')); - r = m_local_image_ctx->io_work_queue->read(0, object_size, - local_bl.c_str(), 0); + r = m_local_image_ctx->io_work_queue->read( + 0, object_size, librbd::io::ReadResult{&local_bl}, 0); if (r < 0) { return r; } diff --git a/src/test/rbd_mirror/test_ImageReplayer.cc b/src/test/rbd_mirror/test_ImageReplayer.cc index 29ab47ff63a..e7668a07a7b 100644 --- a/src/test/rbd_mirror/test_ImageReplayer.cc +++ b/src/test/rbd_mirror/test_ImageReplayer.cc @@ -32,6 +32,7 @@ #include "librbd/internal.h" #include "librbd/io/AioCompletion.h" #include "librbd/io/ImageRequestWQ.h" +#include "librbd/io/ReadResult.h" #include "tools/rbd_mirror/types.h" #include "tools/rbd_mirror/ImageReplayer.h" #include "tools/rbd_mirror/ImageSyncThrottler.h" @@ -306,7 +307,9 @@ public: size_t len) { size_t written; - written = ictx->io_work_queue->write(off, len, test_data, 0); + bufferlist bl; + bl.append(std::string(test_data, len)); + written = ictx->io_work_queue->write(off, len, std::move(bl), 0); printf("wrote: %d\n", (int)written); ASSERT_EQ(len, written); } @@ -318,7 +321,8 @@ public: char *result = (char *)malloc(len + 1); ASSERT_NE(static_cast<char *>(NULL), result); - read = ictx->io_work_queue->read(off, len, result, 0); + read = ictx->io_work_queue->read( + off, len, librbd::io::ReadResult{result, len}, 0); printf("read: %d\n", (int)read); ASSERT_EQ(len, static_cast<size_t>(read)); result[len] = '\0'; diff --git a/src/test/rbd_mirror/test_ImageSync.cc b/src/test/rbd_mirror/test_ImageSync.cc index de68bddfd12..51d48357b5e 100644 --- a/src/test/rbd_mirror/test_ImageSync.cc +++ b/src/test/rbd_mirror/test_ImageSync.cc @@ -12,6 +12,7 @@ #include "librbd/internal.h" #include "librbd/Operations.h" #include "librbd/io/ImageRequestWQ.h" +#include "librbd/io/ReadResult.h" #include "librbd/journal/Types.h" #include "tools/rbd_mirror/ImageSync.h" #include "tools/rbd_mirror/Threads.h" @@ -34,9 +35,10 @@ void scribble(librbd::ImageCtx *image_ctx, int num_ops, size_t max_size) if (rand() % 4 == 0) { ASSERT_EQ((int)len, image_ctx->io_work_queue->discard(off, len)); } else { - std::string str(len, '1'); + bufferlist bl; + bl.append(std::string(len, '1')); ASSERT_EQ((int)len, image_ctx->io_work_queue->write(off, len, - str.c_str(), 0)); + std::move(bl), 0)); } } @@ -129,9 +131,11 @@ TEST_F(TestImageSync, Simple) { for (uint64_t offset = 0; offset < m_remote_image_ctx->size; offset += object_size) { ASSERT_LE(0, m_remote_image_ctx->io_work_queue->read( - offset, object_size, read_remote_bl.c_str(), 0)); + offset, object_size, + librbd::io::ReadResult{&read_remote_bl}, 0)); ASSERT_LE(0, m_local_image_ctx->io_work_queue->read( - offset, object_size, read_local_bl.c_str(), 0)); + offset, object_size, + librbd::io::ReadResult{&read_local_bl}, 0)); ASSERT_TRUE(read_remote_bl.contents_equal(read_local_bl)); } } @@ -143,9 +147,11 @@ TEST_F(TestImageSync, Resize) { uint64_t off = 0; uint64_t len = object_size / 10; - std::string str(len, '1'); + bufferlist bl; + bl.append(std::string(len, '1')); ASSERT_EQ((int)len, m_remote_image_ctx->io_work_queue->write(off, len, - str.c_str(), 0)); + std::move(bl), + 0)); { RWLock::RLocker owner_locker(m_remote_image_ctx->owner_lock); ASSERT_EQ(0, m_remote_image_ctx->flush()); @@ -169,9 +175,9 @@ TEST_F(TestImageSync, Resize) { read_local_bl.append(std::string(len, '\0')); ASSERT_LE(0, m_remote_image_ctx->io_work_queue->read( - off, len, read_remote_bl.c_str(), 0)); + off, len, librbd::io::ReadResult{&read_remote_bl}, 0)); ASSERT_LE(0, m_local_image_ctx->io_work_queue->read( - off, len, read_local_bl.c_str(), 0)); + off, len, librbd::io::ReadResult{&read_local_bl}, 0)); ASSERT_TRUE(read_remote_bl.contents_equal(read_local_bl)); } @@ -183,9 +189,11 @@ TEST_F(TestImageSync, Discard) { uint64_t off = 0; uint64_t len = object_size / 10; - std::string str(len, '1'); + bufferlist bl; + bl.append(std::string(len, '1')); ASSERT_EQ((int)len, m_remote_image_ctx->io_work_queue->write(off, len, - str.c_str(), 0)); + std::move(bl), + 0)); { RWLock::RLocker owner_locker(m_remote_image_ctx->owner_lock); ASSERT_EQ(0, m_remote_image_ctx->flush()); @@ -211,9 +219,9 @@ TEST_F(TestImageSync, Discard) { read_local_bl.append(std::string(object_size, '\0')); ASSERT_LE(0, m_remote_image_ctx->io_work_queue->read( - off, len, read_remote_bl.c_str(), 0)); + off, len, librbd::io::ReadResult{&read_remote_bl}, 0)); ASSERT_LE(0, m_local_image_ctx->io_work_queue->read( - off, len, read_local_bl.c_str(), 0)); + off, len, librbd::io::ReadResult{&read_local_bl}, 0)); ASSERT_TRUE(read_remote_bl.contents_equal(read_local_bl)); } @@ -281,9 +289,11 @@ TEST_F(TestImageSync, SnapshotStress) { for (uint64_t offset = 0; offset < remote_size; offset += object_size) { ASSERT_LE(0, m_remote_image_ctx->io_work_queue->read( - offset, object_size, read_remote_bl.c_str(), 0)); + offset, object_size, + librbd::io::ReadResult{&read_remote_bl}, 0)); ASSERT_LE(0, m_local_image_ctx->io_work_queue->read( - offset, object_size, read_local_bl.c_str(), 0)); + offset, object_size, + librbd::io::ReadResult{&read_local_bl}, 0)); ASSERT_TRUE(read_remote_bl.contents_equal(read_local_bl)); } } |