From 830b9198405ca7efda93ac3d5f1e880c50aa3a49 Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Mon, 12 Sep 2022 20:24:29 +0200 Subject: [PATCH] librbd: introduce ImageArea and split remap_extents() into two methods Since remap in either direction can really be done only once, iterating through image dispatch layers in ImageDispatcher::remap_extents() makes no sense. For now, just replace the iteration with CryptoImageDispatch lookup. Signed-off-by: Ilya Dryomov --- src/librbd/ImageCtx.cc | 6 +-- src/librbd/crypto/CryptoImageDispatch.cc | 39 ++++++++++++++----- src/librbd/crypto/CryptoImageDispatch.h | 7 +++- src/librbd/io/ImageDispatchInterface.h | 4 -- src/librbd/io/ImageDispatcher.cc | 34 +++++++++------- src/librbd/io/ImageDispatcher.h | 4 +- src/librbd/io/ImageDispatcherInterface.h | 5 ++- src/librbd/io/Types.cc | 11 ++++++ src/librbd/io/Types.h | 12 +++--- src/librbd/io/Utils.cc | 32 ++++++++++++--- src/librbd/io/Utils.h | 8 ++++ .../crypto/test_mock_CryptoObjectDispatch.cc | 23 ++++++----- src/test/librbd/mock/io/MockImageDispatcher.h | 3 +- 13 files changed, 128 insertions(+), 60 deletions(-) diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc index b990d8adc76a2..ab74d6a07818a 100644 --- a/src/librbd/ImageCtx.cc +++ b/src/librbd/ImageCtx.cc @@ -36,6 +36,7 @@ #include "librbd/io/ObjectDispatcher.h" #include "librbd/io/QosImageDispatch.h" #include "librbd/io/IoOperations.h" +#include "librbd/io/Utils.h" #include "librbd/journal/StandardPolicy.h" #include "librbd/operation/ResizeRequest.h" @@ -542,10 +543,7 @@ librados::IoCtx duplicate_io_ctx(librados::IoCtx& io_ctx) { return 0; } - io::Extents extents = {{raw_size, 0}}; - io_image_dispatcher->remap_extents( - extents, io::IMAGE_EXTENTS_MAP_TYPE_PHYSICAL_TO_LOGICAL); - return extents.front().first; + return io::util::raw_to_area_offset(*this, raw_size).first; } uint64_t ImageCtx::get_object_count(snap_t in_snap_id) const { diff --git a/src/librbd/crypto/CryptoImageDispatch.cc b/src/librbd/crypto/CryptoImageDispatch.cc index 15513bf55be36..4d4c360dc5ef9 100644 --- a/src/librbd/crypto/CryptoImageDispatch.cc +++ b/src/librbd/crypto/CryptoImageDispatch.cc @@ -10,18 +10,39 @@ CryptoImageDispatch::CryptoImageDispatch( uint64_t data_offset) : m_data_offset(data_offset) { } - -void CryptoImageDispatch::remap_extents( - io::Extents& image_extents, io::ImageExtentsMapType type) { - if (type == io::IMAGE_EXTENTS_MAP_TYPE_LOGICAL_TO_PHYSICAL) { - for (auto& extent: image_extents) { - extent.first += m_data_offset; +void CryptoImageDispatch::remap_to_physical(io::Extents& image_extents, + io::ImageArea area) { + switch (area) { + case io::ImageArea::DATA: + for (auto& [off, _] : image_extents) { + off += m_data_offset; } - } else if (type == io::IMAGE_EXTENTS_MAP_TYPE_PHYSICAL_TO_LOGICAL) { - for (auto& extent: image_extents) { - extent.first -= m_data_offset; + break; + case io::ImageArea::CRYPTO_HEADER: + // direct mapping + break; + default: + ceph_abort(); + } +} + +io::ImageArea CryptoImageDispatch::remap_to_logical( + io::Extents& image_extents) { + bool saw_data = false; + bool saw_crypto_header = false; + for (auto& [off, _] : image_extents) { + if (off >= m_data_offset) { + off -= m_data_offset; + saw_data = true; + } else { + saw_crypto_header = true; } } + if (saw_crypto_header) { + ceph_assert(!saw_data); + return io::ImageArea::CRYPTO_HEADER; + } + return io::ImageArea::DATA; } } // namespace crypto diff --git a/src/librbd/crypto/CryptoImageDispatch.h b/src/librbd/crypto/CryptoImageDispatch.h index dae3dac853b1d..737104006af2a 100644 --- a/src/librbd/crypto/CryptoImageDispatch.h +++ b/src/librbd/crypto/CryptoImageDispatch.h @@ -97,8 +97,11 @@ public: return false; } - void remap_extents(io::Extents& image_extents, - io::ImageExtentsMapType type) override; + // called directly by ImageDispatcher + // TODO: hoist these out and remove CryptoImageDispatch since it's + // just a placeholder + void remap_to_physical(io::Extents& image_extents, io::ImageArea area); + io::ImageArea remap_to_logical(io::Extents& image_extents); private: uint64_t m_data_offset; diff --git a/src/librbd/io/ImageDispatchInterface.h b/src/librbd/io/ImageDispatchInterface.h index 64cea861286d9..205c18c474fa7 100644 --- a/src/librbd/io/ImageDispatchInterface.h +++ b/src/librbd/io/ImageDispatchInterface.h @@ -80,10 +80,6 @@ struct ImageDispatchInterface { Context* on_dispatched) = 0; virtual bool invalidate_cache(Context* on_finish) = 0; - - virtual void remap_extents(Extents& image_extents, - ImageExtentsMapType type) {} - }; } // namespace io diff --git a/src/librbd/io/ImageDispatcher.cc b/src/librbd/io/ImageDispatcher.cc index 413218ad3b463..2dfdc58d4990e 100644 --- a/src/librbd/io/ImageDispatcher.cc +++ b/src/librbd/io/ImageDispatcher.cc @@ -6,6 +6,7 @@ #include "common/AsyncOpTracker.h" #include "common/dout.h" #include "librbd/ImageCtx.h" +#include "librbd/crypto/CryptoImageDispatch.h" #include "librbd/io/ImageDispatch.h" #include "librbd/io/ImageDispatchInterface.h" #include "librbd/io/ImageDispatchSpec.h" @@ -266,22 +267,29 @@ void ImageDispatcher::wait_on_writes_unblocked(Context *on_unblocked) { } template -void ImageDispatcher::remap_extents(Extents& image_extents, - ImageExtentsMapType type) { - auto loop = [&image_extents, type](auto begin, auto end) { - for (auto it = begin; it != end; ++it) { - auto& image_dispatch_meta = it->second; - auto image_dispatch = image_dispatch_meta.dispatch; - image_dispatch->remap_extents(image_extents, type); - } - }; +void ImageDispatcher::remap_to_physical(Extents& image_extents, + ImageArea area) { + std::shared_lock locker{this->m_lock}; + auto it = this->m_dispatches.find(IMAGE_DISPATCH_LAYER_CRYPTO); + if (it == this->m_dispatches.end()) { + ceph_assert(area == ImageArea::DATA); + return; + } + auto crypto_image_dispatch = static_cast( + it->second.dispatch); + crypto_image_dispatch->remap_to_physical(image_extents, area); +} +template +ImageArea ImageDispatcher::remap_to_logical(Extents& image_extents) { std::shared_lock locker{this->m_lock}; - if (type == IMAGE_EXTENTS_MAP_TYPE_LOGICAL_TO_PHYSICAL) { - loop(this->m_dispatches.cbegin(), this->m_dispatches.cend()); - } else if (type == IMAGE_EXTENTS_MAP_TYPE_PHYSICAL_TO_LOGICAL) { - loop(this->m_dispatches.crbegin(), this->m_dispatches.crend()); + auto it = this->m_dispatches.find(IMAGE_DISPATCH_LAYER_CRYPTO); + if (it == this->m_dispatches.end()) { + return ImageArea::DATA; } + auto crypto_image_dispatch = static_cast( + it->second.dispatch); + return crypto_image_dispatch->remap_to_logical(image_extents); } template diff --git a/src/librbd/io/ImageDispatcher.h b/src/librbd/io/ImageDispatcher.h index f8eeeacda57f5..5d5fb053521be 100644 --- a/src/librbd/io/ImageDispatcher.h +++ b/src/librbd/io/ImageDispatcher.h @@ -46,8 +46,8 @@ public: void unblock_writes() override; void wait_on_writes_unblocked(Context *on_unblocked) override; - void remap_extents(Extents& image_extents, - ImageExtentsMapType type) override; + void remap_to_physical(Extents& image_extents, ImageArea area) override; + ImageArea remap_to_logical(Extents& image_extents) override; protected: bool send_dispatch( diff --git a/src/librbd/io/ImageDispatcherInterface.h b/src/librbd/io/ImageDispatcherInterface.h index 9737fb3b0af2c..dcff3d96acd48 100644 --- a/src/librbd/io/ImageDispatcherInterface.h +++ b/src/librbd/io/ImageDispatcherInterface.h @@ -30,8 +30,9 @@ public: virtual void wait_on_writes_unblocked(Context *on_unblocked) = 0; virtual void invalidate_cache(Context* on_finish) = 0; - virtual void remap_extents(Extents& image_extents, - ImageExtentsMapType type) = 0; + + virtual void remap_to_physical(Extents& image_extents, ImageArea area) = 0; + virtual ImageArea remap_to_logical(Extents& image_extents) = 0; }; } // namespace io diff --git a/src/librbd/io/Types.cc b/src/librbd/io/Types.cc index 223c7728320fe..19fcc6b8905d8 100644 --- a/src/librbd/io/Types.cc +++ b/src/librbd/io/Types.cc @@ -34,5 +34,16 @@ std::ostream& operator<<(std::ostream& os, const SparseExtent& se) { return os; } +std::ostream& operator<<(std::ostream& os, ImageArea area) { + switch (area) { + case ImageArea::DATA: + return os << "data"; + case ImageArea::CRYPTO_HEADER: + return os << "crypto_header"; + default: + ceph_abort(); + } +} + } // namespace io } // namespace librbd diff --git a/src/librbd/io/Types.h b/src/librbd/io/Types.h index 12fea2bb3262e..8ca9ccb1b4332 100644 --- a/src/librbd/io/Types.h +++ b/src/librbd/io/Types.h @@ -112,11 +112,6 @@ enum { RBD_IO_OPERATION_COMPARE_AND_WRITE) }; -enum ImageExtentsMapType { - IMAGE_EXTENTS_MAP_TYPE_LOGICAL_TO_PHYSICAL, - IMAGE_EXTENTS_MAP_TYPE_PHYSICAL_TO_LOGICAL, -}; - enum ObjectDispatchLayer { OBJECT_DISPATCH_LAYER_NONE = 0, OBJECT_DISPATCH_LAYER_CACHE, @@ -275,6 +270,13 @@ using striper::LightweightObjectExtents; typedef std::pair Extent; typedef std::vector Extents; +enum class ImageArea { + DATA, + CRYPTO_HEADER +}; + +std::ostream& operator<<(std::ostream& os, ImageArea area); + struct ReadExtent { const uint64_t offset; const uint64_t length; diff --git a/src/librbd/io/Utils.cc b/src/librbd/io/Utils.cc index 2ce9dd11fc67f..5b786ff8f52fd 100644 --- a/src/librbd/io/Utils.cc +++ b/src/librbd/io/Utils.cc @@ -186,8 +186,8 @@ void file_to_extents(I* image_ctx, uint64_t offset, uint64_t length, uint64_t buffer_offset, striper::LightweightObjectExtents* object_extents) { Extents extents = {{offset, length}}; - image_ctx->io_image_dispatcher->remap_extents( - extents, IMAGE_EXTENTS_MAP_TYPE_LOGICAL_TO_PHYSICAL); + // TODO: pass area + image_ctx->io_image_dispatcher->remap_to_physical(extents, ImageArea::DATA); for (auto [off, len] : extents) { Striper::file_to_extents(image_ctx->cct, &image_ctx->layout, off, len, 0, buffer_offset, object_extents); @@ -200,8 +200,24 @@ void extent_to_file(I* image_ctx, uint64_t object_no, uint64_t offset, std::vector >& extents) { Striper::extent_to_file(image_ctx->cct, &image_ctx->layout, object_no, offset, length, extents); - image_ctx->io_image_dispatcher->remap_extents( - extents, IMAGE_EXTENTS_MAP_TYPE_PHYSICAL_TO_LOGICAL); + // TODO: return area + image_ctx->io_image_dispatcher->remap_to_logical(extents); +} + +template +uint64_t area_to_raw_offset(const I& image_ctx, uint64_t offset, + ImageArea area) { + Extents extents = {{offset, 0}}; + image_ctx.io_image_dispatcher->remap_to_physical(extents, area); + return extents[0].first; +} + +template +std::pair raw_to_area_offset(const I& image_ctx, + uint64_t offset) { + Extents extents = {{offset, 0}}; + auto area = image_ctx.io_image_dispatcher->remap_to_logical(extents); + return {extents[0].first, area}; } template @@ -209,8 +225,7 @@ uint64_t get_file_offset(I* image_ctx, uint64_t object_no, uint64_t offset) { auto off = Striper::get_file_offset(image_ctx->cct, &image_ctx->layout, object_no, offset); Extents extents = {{off, 0}}; - image_ctx->io_image_dispatcher->remap_extents( - extents, IMAGE_EXTENTS_MAP_TYPE_PHYSICAL_TO_LOGICAL); + image_ctx->io_image_dispatcher->remap_to_logical(extents); return extents[0].first; } @@ -234,6 +249,11 @@ template void librbd::io::util::extent_to_file( librbd::ImageCtx *image_ctx, uint64_t object_no, uint64_t offset, uint64_t length, std::vector >& extents); +template uint64_t librbd::io::util::area_to_raw_offset( + const librbd::ImageCtx& image_ctx, uint64_t offset, ImageArea area); +template auto librbd::io::util::raw_to_area_offset( + const librbd::ImageCtx& image_ctx, uint64_t offset) + -> std::pair; template uint64_t librbd::io::util::get_file_offset( librbd::ImageCtx *image_ctx, uint64_t object_no, uint64_t offset); diff --git a/src/librbd/io/Utils.h b/src/librbd/io/Utils.h index 9f7e0b94668ba..21218a8eb5be4 100644 --- a/src/librbd/io/Utils.h +++ b/src/librbd/io/Utils.h @@ -64,6 +64,14 @@ void extent_to_file(ImageCtxT *image_ctx, uint64_t object_no, uint64_t offset, uint64_t length, std::vector >& extents); +template +uint64_t area_to_raw_offset(const ImageCtxT& image_ctx, uint64_t offset, + ImageArea area); + +template +std::pair raw_to_area_offset(const ImageCtxT& image_ctx, + uint64_t offset); + template uint64_t get_file_offset(ImageCtxT *image_ctx, uint64_t object_no, uint64_t offset); diff --git a/src/test/librbd/crypto/test_mock_CryptoObjectDispatch.cc b/src/test/librbd/crypto/test_mock_CryptoObjectDispatch.cc index c0d25d62b0bdf..f0920f09d354d 100644 --- a/src/test/librbd/crypto/test_mock_CryptoObjectDispatch.cc +++ b/src/test/librbd/crypto/test_mock_CryptoObjectDispatch.cc @@ -224,10 +224,9 @@ struct TestMockCryptoCryptoObjectDispatch : public TestMockFixture { mock_image_ctx->layout.object_size)); } - void expect_remap_extents(uint64_t offset, uint64_t length) { - EXPECT_CALL(*mock_image_ctx->io_image_dispatcher, remap_extents( - ElementsAre(Pair(offset, length)), - io::IMAGE_EXTENTS_MAP_TYPE_PHYSICAL_TO_LOGICAL)); + void expect_remap_to_logical(uint64_t offset, uint64_t length) { + EXPECT_CALL(*mock_image_ctx->io_image_dispatcher, remap_to_logical( + ElementsAre(Pair(offset, length)))); } void expect_get_parent_overlap(uint64_t overlap) { @@ -517,7 +516,7 @@ TEST_F(TestMockCryptoCryptoObjectDispatch, UnalignedWriteCopyup) { expect_get_object_size(); expect_get_parent_overlap(mock_image_ctx->layout.object_size); - expect_remap_extents(0, mock_image_ctx->layout.object_size); + expect_remap_to_logical(0, mock_image_ctx->layout.object_size); expect_prune_parent_extents(mock_image_ctx->layout.object_size); EXPECT_CALL(mock_exclusive_lock, is_lock_owner()).WillRepeatedly( Return(true)); @@ -562,7 +561,7 @@ TEST_F(TestMockCryptoCryptoObjectDispatch, UnalignedWriteEmptyCopyup) { expect_get_object_size(); expect_get_parent_overlap(mock_image_ctx->layout.object_size); - expect_remap_extents(0, mock_image_ctx->layout.object_size); + expect_remap_to_logical(0, mock_image_ctx->layout.object_size); expect_prune_parent_extents(mock_image_ctx->layout.object_size); EXPECT_CALL(mock_exclusive_lock, is_lock_owner()).WillRepeatedly( Return(true)); @@ -751,12 +750,12 @@ TEST_F(TestMockCryptoCryptoObjectDispatch, PrepareCopyup) { expect_get_object_size(); expect_encrypt(6); InSequence seq; - expect_remap_extents(0, 4096); - expect_remap_extents(4096, 4096); - expect_remap_extents(8192, 4096); - expect_remap_extents(0, 4096); - expect_remap_extents(4096, 8192); - expect_remap_extents(16384, 4096); + expect_remap_to_logical(0, 4096); + expect_remap_to_logical(4096, 4096); + expect_remap_to_logical(8192, 4096); + expect_remap_to_logical(0, 4096); + expect_remap_to_logical(4096, 8192); + expect_remap_to_logical(16384, 4096); ASSERT_EQ(0, mock_crypto_object_dispatch->prepare_copyup( 0, &snapshot_sparse_bufferlist)); diff --git a/src/test/librbd/mock/io/MockImageDispatcher.h b/src/test/librbd/mock/io/MockImageDispatcher.h index 43ceb25265477..92cddd501f2dc 100644 --- a/src/test/librbd/mock/io/MockImageDispatcher.h +++ b/src/test/librbd/mock/io/MockImageDispatcher.h @@ -40,7 +40,8 @@ public: MOCK_METHOD0(unblock_writes, void()); MOCK_METHOD1(wait_on_writes_unblocked, void(Context*)); - MOCK_METHOD2(remap_extents, void(Extents&, ImageExtentsMapType)); + MOCK_METHOD2(remap_to_physical, void(Extents&, ImageArea)); + MOCK_METHOD1(remap_to_logical, ImageArea(Extents&)); }; } // namespace io -- 2.39.5