From 90174e72c614bbcc77c082b8c7ad12f5bb4ff456 Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Mon, 12 Feb 2024 13:07:22 +0100 Subject: [PATCH] librbd: refactor merge() for SparseBufferlistExtent - pass left.length + right.length instead of bl.length() for consistency and to avoid circumventing the assert in SparseBufferlistExtent constructor - claim_append() takes an lvalue reference, no need to move - follow the pattern used in split() Signed-off-by: Ilya Dryomov (cherry picked from commit 69c18cfd291e63fbcd261d3cf20be8dfa0c7488d) --- src/librbd/io/Types.h | 11 ++--- src/test/librbd/io/test_mock_ObjectRequest.cc | 49 +++++++++++++++++++ 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/src/librbd/io/Types.h b/src/librbd/io/Types.h index c11342141d8..03e9ffa3b7c 100644 --- a/src/librbd/io/Types.h +++ b/src/librbd/io/Types.h @@ -248,14 +248,13 @@ struct SparseBufferlistExtentSplitMerge { SparseBufferlistExtent merge(SparseBufferlistExtent&& left, SparseBufferlistExtent&& right) const { + ceph::bufferlist bl; if (left.state == SPARSE_EXTENT_STATE_DATA) { - ceph::bufferlist bl{std::move(left.bl)}; - bl.claim_append(std::move(right.bl)); - return SparseBufferlistExtent(SPARSE_EXTENT_STATE_DATA, - bl.length(), std::move(bl)); - } else { - return SparseBufferlistExtent(left.state, left.length + right.length, {}); + bl.claim_append(left.bl); + bl.claim_append(right.bl); } + return SparseBufferlistExtent(left.state, left.length + right.length, + std::move(bl)); } uint64_t length(const SparseBufferlistExtent& sbe) const { diff --git a/src/test/librbd/io/test_mock_ObjectRequest.cc b/src/test/librbd/io/test_mock_ObjectRequest.cc index 3073fee64d2..91e8827b951 100644 --- a/src/test/librbd/io/test_mock_ObjectRequest.cc +++ b/src/test/librbd/io/test_mock_ObjectRequest.cc @@ -2089,6 +2089,25 @@ TEST(SparseExtents, Split) { EXPECT_EQ(expected_extents, extents); } +TEST(SparseExtents, Merge) { + SparseExtents extents; + extents.insert(50, 100, {SPARSE_EXTENT_STATE_DATA, 100}); + extents.insert(30, 15, {SPARSE_EXTENT_STATE_ZEROED, 15}); + extents.insert(45, 10, {SPARSE_EXTENT_STATE_DATA, 10}); + extents.insert(200, 40, {SPARSE_EXTENT_STATE_DNE, 40}); + extents.insert(160, 25, {SPARSE_EXTENT_STATE_DNE, 25}); + extents.insert(140, 20, {SPARSE_EXTENT_STATE_DATA, 20}); + extents.insert(25, 5, {SPARSE_EXTENT_STATE_ZEROED, 5}); + extents.insert(185, 15, {SPARSE_EXTENT_STATE_DNE, 15}); + + SparseExtents expected_extents = { + {25, {20, {SPARSE_EXTENT_STATE_ZEROED, 20}}}, + {45, {115, {SPARSE_EXTENT_STATE_DATA, 115}}}, + {160, {80, {SPARSE_EXTENT_STATE_DNE, 80}}} + }; + EXPECT_EQ(expected_extents, extents); +} + TEST(SparseBufferlist, Split) { bufferlist bl; bl.append(std::string(5, '1')); @@ -2161,6 +2180,36 @@ TEST(SparseBufferlist, SplitData) { EXPECT_EQ(expected_extents, extents); } +TEST(SparseBufferlist, Merge) { + bufferlist bl1; + bl1.append(std::string(100, '1')); + bufferlist bl2; + bl2.append(std::string(10, '2')); + bufferlist bl3; + bl3.append(std::string(20, '3')); + bufferlist expected_bl; + expected_bl.append(std::string(10, '2')); + expected_bl.append(std::string(85, '1')); + expected_bl.append(std::string(20, '3')); + + SparseBufferlist extents; + extents.insert(50, 100, {SPARSE_EXTENT_STATE_DATA, 100, std::move(bl1)}); + extents.insert(30, 15, {SPARSE_EXTENT_STATE_ZEROED, 15}); + extents.insert(45, 10, {SPARSE_EXTENT_STATE_DATA, 10, std::move(bl2)}); + extents.insert(200, 40, {SPARSE_EXTENT_STATE_DNE, 40}); + extents.insert(160, 25, {SPARSE_EXTENT_STATE_DNE, 25}); + extents.insert(140, 20, {SPARSE_EXTENT_STATE_DATA, 20, std::move(bl3)}); + extents.insert(25, 5, {SPARSE_EXTENT_STATE_ZEROED, 5}); + extents.insert(185, 15, {SPARSE_EXTENT_STATE_DNE, 15}); + + SparseBufferlist expected_extents = { + {25, {20, {SPARSE_EXTENT_STATE_ZEROED, 20}}}, + {45, {115, {SPARSE_EXTENT_STATE_DATA, 115, std::move(expected_bl)}}}, + {160, {80, {SPARSE_EXTENT_STATE_DNE, 80}}} + }; + EXPECT_EQ(expected_extents, extents); +} + } // namespace io } // namespace librbd -- 2.39.5