From: Jason Dillaman Date: Thu, 24 Sep 2020 19:15:23 +0000 (-0400) Subject: librbd: support preprocessing parent data prior to copyup X-Git-Tag: v16.1.0~833^2~3 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=0b53cdf93a33c78398b64b9a72f2640d00700217;p=ceph.git librbd: support preprocessing parent data prior to copyup Let object dispatch layers potentially mutate the copyup data read from the parent prior to issuing the actual copyup operation. This can allow for a layer like the crypto layer to re-encrypt the parent image data using the child image's encryption keys, for example. Signed-off-by: Jason Dillaman --- diff --git a/src/librbd/io/CopyupRequest.cc b/src/librbd/io/CopyupRequest.cc index fda4d9da616..f4cadd3c394 100644 --- a/src/librbd/io/CopyupRequest.cc +++ b/src/librbd/io/CopyupRequest.cc @@ -18,6 +18,7 @@ #include "librbd/deep_copy/ObjectCopyRequest.h" #include "librbd/io/AioCompletion.h" #include "librbd/io/ImageRequest.h" +#include "librbd/io/ObjectDispatcherInterface.h" #include "librbd/io/ObjectRequest.h" #include "librbd/io/ReadResult.h" @@ -190,21 +191,26 @@ void CopyupRequest::handle_read_from_parent(int r) { auto cct = m_image_ctx->cct; ldout(cct, 20) << "r=" << r << dendl; - m_image_ctx->image_lock.lock_shared(); - m_lock.lock(); - m_copyup_is_zero = m_copyup_data.is_zero(); - m_copyup_required = is_copyup_required(); - disable_append_requests(); - if (r < 0 && r != -ENOENT) { + m_lock.lock(); + disable_append_requests(); m_lock.unlock(); - m_image_ctx->image_lock.unlock_shared(); lderr(cct) << "error reading from parent: " << cpp_strerror(r) << dendl; finish(r); return; } + convert_copyup_extent_map(); + + m_image_ctx->image_lock.lock_shared(); + m_lock.lock(); + disable_append_requests(); + + prepare_copyup_data(); + + m_copyup_is_zero = m_copyup_data.is_zero(); + m_copyup_required = is_copyup_required(); if (!m_copyup_required) { m_lock.unlock(); m_image_ctx->image_lock.unlock_shared(); @@ -214,22 +220,6 @@ void CopyupRequest::handle_read_from_parent(int r) { return; } - // convert the image-extent extent map to object-extents - Extents image_extent_map; - image_extent_map.swap(m_copyup_extent_map); - m_copyup_extent_map.reserve(m_copyup_extent_map.size()); - - for (auto [image_offset, image_length] : image_extent_map) { - striper::LightweightObjectExtents object_extents; - Striper::file_to_extents( - cct, &m_image_ctx->layout, image_offset, image_length, 0, 0, - &object_extents); - for (auto& object_extent : object_extents) { - m_copyup_extent_map.emplace_back( - object_extent.offset, object_extent.length); - } - } - // copyup() will affect snapshots only if parent data is not all // zeros. if (!m_copyup_is_zero) { @@ -661,6 +651,104 @@ void CopyupRequest::compute_deep_copy_snap_ids() { }); } +template +void CopyupRequest::convert_copyup_extent_map() { + auto cct = m_image_ctx->cct; + + Extents image_extent_map; + image_extent_map.swap(m_copyup_extent_map); + m_copyup_extent_map.reserve(image_extent_map.size()); + + // convert the image-extent extent map to object-extents + for (auto [image_offset, image_length] : image_extent_map) { + striper::LightweightObjectExtents object_extents; + Striper::file_to_extents( + cct, &m_image_ctx->layout, image_offset, image_length, 0, 0, + &object_extents); + for (auto& object_extent : object_extents) { + m_copyup_extent_map.emplace_back( + object_extent.offset, object_extent.length); + } + } + + ldout(cct, 20) << "image_extents=" << image_extent_map << ", " + << "object_extents=" << m_copyup_extent_map << dendl; +} + +template +void CopyupRequest::prepare_copyup_data() { + ceph_assert(ceph_mutex_is_locked(m_image_ctx->image_lock)); + auto cct = m_image_ctx->cct; + + SnapshotSparseBufferlist snapshot_sparse_bufferlist; + auto& sparse_bufferlist = snapshot_sparse_bufferlist[0]; + + bool copy_on_read = m_pending_requests.empty(); + bool maybe_deep_copyup = !m_image_ctx->snapc.snaps.empty(); + if (copy_on_read || maybe_deep_copyup) { + // stand-alone copyup that will not be overwritten until HEAD revision + ldout(cct, 20) << "processing full copy-up" << dendl; + + uint64_t buffer_offset = 0; + for (auto [object_offset, object_length] : m_copyup_extent_map) { + bufferlist sub_bl; + sub_bl.substr_of(m_copyup_data, buffer_offset, object_length); + buffer_offset += object_length; + + sparse_bufferlist.insert( + object_offset, object_length, + {SPARSE_EXTENT_STATE_DATA, object_length, std::move(sub_bl)}); + } + } else { + // copyup that will concurrently written to the HEAD revision with the + // associated write-ops so only process partial extents + uint64_t buffer_offset = 0; + for (auto [object_offset, object_length] : m_copyup_extent_map) { + interval_set copyup_object_extents; + copyup_object_extents.insert(object_offset, object_length); + + interval_set intersection; + intersection.intersection_of(copyup_object_extents, + m_write_object_extents); + + // extract only portions of the parent copyup data that have not + // been overwritten by write-ops + copyup_object_extents.subtract(intersection); + for (auto [copyup_offset, copyup_length] : copyup_object_extents) { + bufferlist sub_bl; + sub_bl.substr_of( + m_copyup_data, buffer_offset + (copyup_offset - object_offset), + copyup_length); + ceph_assert(sub_bl.length() == copyup_length); + + sparse_bufferlist.insert( + copyup_offset, copyup_length, + {SPARSE_EXTENT_STATE_DATA, copyup_length, std::move(sub_bl)}); + } + buffer_offset += object_length; + } + + ldout(cct, 20) << "processing partial copy-up: " << sparse_bufferlist + << dendl; + } + + // Let dispatch layers have a chance to process the data + m_image_ctx->io_object_dispatcher->prepare_copyup( + m_object_no, &snapshot_sparse_bufferlist); + + // Convert sparse extents back to extent map + m_copyup_data.clear(); + m_copyup_extent_map.clear(); + m_copyup_extent_map.reserve(sparse_bufferlist.ext_count()); + for (auto& extent : sparse_bufferlist) { + auto& sbe = extent.get_val(); + if (sbe.state == SPARSE_EXTENT_STATE_DATA) { + m_copyup_extent_map.emplace_back(extent.get_off(), extent.get_len()); + m_copyup_data.append(sbe.bl); + } + } +} + } // namespace io } // namespace librbd diff --git a/src/librbd/io/CopyupRequest.h b/src/librbd/io/CopyupRequest.h index 9724184fa38..127fc4a00bf 100644 --- a/src/librbd/io/CopyupRequest.h +++ b/src/librbd/io/CopyupRequest.h @@ -131,6 +131,8 @@ private: bool is_deep_copy() const; void compute_deep_copy_snap_ids(); + void convert_copyup_extent_map(); + void prepare_copyup_data(); }; } // namespace io diff --git a/src/test/librbd/io/test_mock_CopyupRequest.cc b/src/test/librbd/io/test_mock_CopyupRequest.cc index 93ee1c61c15..716c2796924 100644 --- a/src/test/librbd/io/test_mock_CopyupRequest.cc +++ b/src/test/librbd/io/test_mock_CopyupRequest.cc @@ -342,6 +342,25 @@ struct TestMockIoCopyupRequest : public TestMockFixture { })); } + void expect_prepare_copyup(MockTestImageCtx& mock_image_ctx) { + EXPECT_CALL(*mock_image_ctx.io_object_dispatcher, prepare_copyup(_, _)); + } + + void expect_prepare_copyup(MockTestImageCtx& mock_image_ctx, + const SparseBufferlist& in_sparse_bl, + const SparseBufferlist& out_sparse_bl) { + EXPECT_CALL(*mock_image_ctx.io_object_dispatcher, + prepare_copyup(_, _)) + .WillOnce(WithArg<1>(Invoke( + [in_sparse_bl, out_sparse_bl] + (SnapshotSparseBufferlist* snap_sparse_bl) { + auto& sparse_bl = (*snap_sparse_bl)[0]; + EXPECT_EQ(in_sparse_bl, sparse_bl); + + sparse_bl = out_sparse_bl; + }))); + } + void flush_async_operations(librbd::ImageCtx* ictx) { api::Io<>::flush(*ictx); } @@ -373,6 +392,7 @@ TEST_F(TestMockIoCopyupRequest, Standard) { std::string data(4096, '1'); expect_read_parent(mock_parent_image_ctx, mock_image_request, {{0, 4096}}, data, 0); + expect_prepare_copyup(mock_image_ctx); MockAbstractObjectWriteRequest mock_write_request; expect_get_pre_write_object_map_state(mock_image_ctx, mock_write_request, @@ -429,6 +449,7 @@ TEST_F(TestMockIoCopyupRequest, StandardWithSnaps) { std::string data(4096, '1'); expect_read_parent(mock_parent_image_ctx, mock_image_request, {{0, 4096}}, data, 0); + expect_prepare_copyup(mock_image_ctx); MockAbstractObjectWriteRequest mock_write_request; expect_get_pre_write_object_map_state(mock_image_ctx, mock_write_request, @@ -477,6 +498,7 @@ TEST_F(TestMockIoCopyupRequest, CopyOnRead) { std::string data(4096, '1'); expect_read_parent(mock_parent_image_ctx, mock_image_request, {{0, 4096}}, data, 0); + expect_prepare_copyup(mock_image_ctx); expect_object_map_at(mock_image_ctx, 0, OBJECT_NONEXISTENT); expect_object_map_update(mock_image_ctx, CEPH_NOSNAP, 0, OBJECT_EXISTS, true, @@ -523,6 +545,7 @@ TEST_F(TestMockIoCopyupRequest, CopyOnReadWithSnaps) { std::string data(4096, '1'); expect_read_parent(mock_parent_image_ctx, mock_image_request, {{0, 4096}}, data, 0); + expect_prepare_copyup(mock_image_ctx); expect_object_map_at(mock_image_ctx, 0, OBJECT_NONEXISTENT); expect_object_map_update(mock_image_ctx, 1, 0, OBJECT_EXISTS, true, 0); @@ -783,6 +806,7 @@ TEST_F(TestMockIoCopyupRequest, ZeroedCopyup) { InSequence seq; MockAbstractObjectWriteRequest mock_write_request; + expect_prepare_copyup(mock_image_ctx); expect_is_empty_write_op(mock_write_request, false); expect_get_pre_write_object_map_state(mock_image_ctx, mock_write_request, OBJECT_EXISTS); @@ -828,6 +852,7 @@ TEST_F(TestMockIoCopyupRequest, ZeroedCopyOnRead) { std::string data(4096, '\0'); expect_read_parent(mock_parent_image_ctx, mock_image_request, {{0, 4096}}, data, 0); + expect_prepare_copyup(mock_image_ctx); expect_object_map_at(mock_image_ctx, 0, OBJECT_NONEXISTENT); expect_object_map_update(mock_image_ctx, CEPH_NOSNAP, 0, OBJECT_EXISTS, true, @@ -867,6 +892,8 @@ TEST_F(TestMockIoCopyupRequest, NoOpCopyup) { expect_read_parent(mock_parent_image_ctx, mock_image_request, {{0, 4096}}, "", -ENOENT); + expect_prepare_copyup(mock_image_ctx); + MockAbstractObjectWriteRequest mock_write_request; expect_is_empty_write_op(mock_write_request, true); @@ -903,6 +930,7 @@ TEST_F(TestMockIoCopyupRequest, RestartWrite) { std::string data(4096, '1'); expect_read_parent(mock_parent_image_ctx, mock_image_request, {{0, 4096}}, data, 0); + expect_prepare_copyup(mock_image_ctx); MockAbstractObjectWriteRequest mock_write_request1; expect_get_pre_write_object_map_state(mock_image_ctx, mock_write_request1, @@ -958,12 +986,10 @@ TEST_F(TestMockIoCopyupRequest, ReadFromParentError) { expect_read_parent(mock_parent_image_ctx, mock_image_request, {{0, 4096}}, "", -EPERM); - MockAbstractObjectWriteRequest mock_write_request; - expect_is_empty_write_op(mock_write_request, false); - auto req = new MockCopyupRequest(&mock_image_ctx, 0, {{0, 4096}}, {}); mock_image_ctx.copyup_list[0] = req; + MockAbstractObjectWriteRequest mock_write_request; req->append_request(&mock_write_request, {}); req->send(); @@ -1031,6 +1057,7 @@ TEST_F(TestMockIoCopyupRequest, UpdateObjectMapError) { std::string data(4096, '1'); expect_read_parent(mock_parent_image_ctx, mock_image_request, {{0, 4096}}, data, 0); + expect_prepare_copyup(mock_image_ctx); MockAbstractObjectWriteRequest mock_write_request; expect_get_pre_write_object_map_state(mock_image_ctx, mock_write_request, @@ -1079,6 +1106,7 @@ TEST_F(TestMockIoCopyupRequest, CopyupError) { std::string data(4096, '1'); expect_read_parent(mock_parent_image_ctx, mock_image_request, {{0, 4096}}, data, 0); + expect_prepare_copyup(mock_image_ctx); MockAbstractObjectWriteRequest mock_write_request; expect_get_pre_write_object_map_state(mock_image_ctx, mock_write_request, @@ -1128,6 +1156,7 @@ TEST_F(TestMockIoCopyupRequest, SparseCopyupNotSupported) { std::string data(4096, '1'); expect_read_parent(mock_parent_image_ctx, mock_image_request, {{0, 4096}}, data, 0); + expect_prepare_copyup(mock_image_ctx); MockAbstractObjectWriteRequest mock_write_request; expect_get_pre_write_object_map_state(mock_image_ctx, mock_write_request, @@ -1148,5 +1177,128 @@ TEST_F(TestMockIoCopyupRequest, SparseCopyupNotSupported) { ASSERT_EQ(0, mock_write_request.ctx.wait()); } +TEST_F(TestMockIoCopyupRequest, ProcessCopyup) { + REQUIRE_FEATURE(RBD_FEATURE_LAYERING); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockTestImageCtx mock_parent_image_ctx(*ictx->parent); + MockTestImageCtx mock_image_ctx(*ictx, &mock_parent_image_ctx); + + MockExclusiveLock mock_exclusive_lock; + MockJournal mock_journal; + MockObjectMap mock_object_map; + initialize_features(ictx, mock_image_ctx, mock_exclusive_lock, mock_journal, + mock_object_map); + + expect_op_work_queue(mock_image_ctx); + expect_is_lock_owner(mock_image_ctx); + + InSequence seq; + + MockImageRequest mock_image_request; + std::string data(4096, '1'); + expect_read_parent(mock_parent_image_ctx, mock_image_request, {{0, 4096}}, + data, 0); + + bufferlist in_prepare_bl; + in_prepare_bl.append(std::string(3072, '1')); + bufferlist out_prepare_bl; + out_prepare_bl.substr_of(in_prepare_bl, 0, 1024); + expect_prepare_copyup( + mock_image_ctx, + {{1024U, {3072U, {SPARSE_EXTENT_STATE_DATA, 3072, + std::move(in_prepare_bl)}}}}, + {{2048U, {1024U, {SPARSE_EXTENT_STATE_DATA, 1024, + std::move(out_prepare_bl)}}}}); + + MockAbstractObjectWriteRequest mock_write_request; + expect_get_pre_write_object_map_state(mock_image_ctx, mock_write_request, + OBJECT_EXISTS); + expect_object_map_at(mock_image_ctx, 0, OBJECT_NONEXISTENT); + expect_object_map_update(mock_image_ctx, CEPH_NOSNAP, 0, OBJECT_EXISTS, true, + 0); + + expect_add_copyup_ops(mock_write_request); + expect_sparse_copyup(mock_image_ctx, CEPH_NOSNAP, ictx->get_object_name(0), + {{2048, 1024}}, data.substr(0, 1024), 0); + expect_write(mock_image_ctx, CEPH_NOSNAP, ictx->get_object_name(0), 0); + + auto req = new MockCopyupRequest(&mock_image_ctx, 0, + {{0, 4096}}, {}); + mock_image_ctx.copyup_list[0] = req; + req->append_request(&mock_write_request, {{0, 1024}}); + req->send(); + + ASSERT_EQ(0, mock_write_request.ctx.wait()); +} + +TEST_F(TestMockIoCopyupRequest, ProcessCopyupOverwrite) { + REQUIRE_FEATURE(RBD_FEATURE_LAYERING); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ictx->image_lock.lock(); + ictx->add_snap(cls::rbd::UserSnapshotNamespace(), "1", 1, ictx->size, + ictx->parent_md, RBD_PROTECTION_STATUS_UNPROTECTED, + 0, {}); + ictx->snapc = {1, {1}}; + ictx->image_lock.unlock(); + + MockTestImageCtx mock_parent_image_ctx(*ictx->parent); + MockTestImageCtx mock_image_ctx(*ictx, &mock_parent_image_ctx); + + MockExclusiveLock mock_exclusive_lock; + MockJournal mock_journal; + MockObjectMap mock_object_map; + initialize_features(ictx, mock_image_ctx, mock_exclusive_lock, mock_journal, + mock_object_map); + + expect_test_features(mock_image_ctx); + expect_op_work_queue(mock_image_ctx); + expect_is_lock_owner(mock_image_ctx); + + InSequence seq; + + MockImageRequest mock_image_request; + std::string data(4096, '1'); + expect_read_parent(mock_parent_image_ctx, mock_image_request, {{0, 4096}}, + data, 0); + + bufferlist in_prepare_bl; + in_prepare_bl.append(data); + bufferlist out_prepare_bl; + out_prepare_bl.substr_of(in_prepare_bl, 0, 1024); + expect_prepare_copyup( + mock_image_ctx, + {{0, {4096, {SPARSE_EXTENT_STATE_DATA, 4096, + std::move(in_prepare_bl)}}}}, + {{0, {1024, {SPARSE_EXTENT_STATE_DATA, 1024, bufferlist{out_prepare_bl}}}}, + {2048, {1024, {SPARSE_EXTENT_STATE_DATA, 1024, + bufferlist{out_prepare_bl}}}}}); + + MockAbstractObjectWriteRequest mock_write_request; + expect_get_pre_write_object_map_state(mock_image_ctx, mock_write_request, + OBJECT_EXISTS); + expect_object_map_at(mock_image_ctx, 0, OBJECT_NONEXISTENT); + expect_object_map_update(mock_image_ctx, 1, 0, OBJECT_EXISTS, true, 0); + expect_object_map_update(mock_image_ctx, CEPH_NOSNAP, 0, OBJECT_EXISTS, true, + 0); + + expect_add_copyup_ops(mock_write_request); + expect_sparse_copyup(mock_image_ctx, 0, ictx->get_object_name(0), + {{0, 1024}, {2048, 1024}}, data.substr(0, 2048), 0); + expect_write(mock_image_ctx, CEPH_NOSNAP, ictx->get_object_name(0), 0); + + auto req = new MockCopyupRequest(&mock_image_ctx, 0, + {{0, 4096}}, {}); + mock_image_ctx.copyup_list[0] = req; + req->append_request(&mock_write_request, {{0, 1024}}); + req->send(); + + ASSERT_EQ(0, mock_write_request.ctx.wait()); +} + } // namespace io } // namespace librbd