From 0e318952a5c8dfdb7dcc89cf3fa7bad935accfbd Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Wed, 28 Sep 2022 10:31:45 +0200 Subject: [PATCH] librbd: clip extents to their area instead of DATA area This fixes cases where CRYPTO HEADER area is larger than DATA area. In particular, it was effectively impossible to flatten unformatted clones of such images. Signed-off-by: Ilya Dryomov --- src/librbd/api/DiffIterate.cc | 2 +- src/librbd/api/Io.cc | 15 ++++-- src/librbd/internal.cc | 10 ++-- src/librbd/internal.h | 7 ++- src/librbd/io/ImageDispatcher.cc | 5 +- src/librbd/io/Utils.cc | 6 +-- src/librbd/io/Utils.h | 2 +- src/test/librbd/test_librbd.cc | 91 ++++++++++++++++++++++++++++++++ 8 files changed, 119 insertions(+), 19 deletions(-) diff --git a/src/librbd/api/DiffIterate.cc b/src/librbd/api/DiffIterate.cc index 489763cf6def2..1cc08968a2904 100644 --- a/src/librbd/api/DiffIterate.cc +++ b/src/librbd/api/DiffIterate.cc @@ -198,7 +198,7 @@ int DiffIterate::diff_iterate(I *ictx, } ictx->image_lock.lock_shared(); - r = clip_io(ictx, off, &len); + r = clip_io(ictx, off, &len, io::ImageArea::DATA); ictx->image_lock.unlock_shared(); if (r < 0) { return r; diff --git a/src/librbd/api/Io.cc b/src/librbd/api/Io.cc index fb8501bde91c5..af9d3ebe5cfcc 100644 --- a/src/librbd/api/Io.cc +++ b/src/librbd/api/Io.cc @@ -63,7 +63,8 @@ ssize_t Io::write( << "len = " << len << dendl; image_ctx.image_lock.lock_shared(); - int r = clip_io(util::get_image_ctx(&image_ctx), off, &len); + int r = clip_io(util::get_image_ctx(&image_ctx), off, &len, + io::ImageArea::DATA); image_ctx.image_lock.unlock_shared(); if (r < 0) { lderr(cct) << "invalid IO request: " << cpp_strerror(r) << dendl; @@ -90,7 +91,8 @@ ssize_t Io::discard( << "len = " << len << dendl; image_ctx.image_lock.lock_shared(); - int r = clip_io(util::get_image_ctx(&image_ctx), off, &len); + int r = clip_io(util::get_image_ctx(&image_ctx), off, &len, + io::ImageArea::DATA); image_ctx.image_lock.unlock_shared(); if (r < 0) { lderr(cct) << "invalid IO request: " << cpp_strerror(r) << dendl; @@ -116,7 +118,8 @@ ssize_t Io::write_same( << "len = " << len << ", data_len " << bl.length() << dendl; image_ctx.image_lock.lock_shared(); - int r = clip_io(util::get_image_ctx(&image_ctx), off, &len); + int r = clip_io(util::get_image_ctx(&image_ctx), off, &len, + io::ImageArea::DATA); image_ctx.image_lock.unlock_shared(); if (r < 0) { lderr(cct) << "invalid IO request: " << cpp_strerror(r) << dendl; @@ -142,7 +145,8 @@ ssize_t Io::write_zeroes(I& image_ctx, uint64_t off, uint64_t len, << "len = " << len << dendl; image_ctx.image_lock.lock_shared(); - int r = clip_io(util::get_image_ctx(&image_ctx), off, &len); + int r = clip_io(util::get_image_ctx(&image_ctx), off, &len, + io::ImageArea::DATA); image_ctx.image_lock.unlock_shared(); if (r < 0) { lderr(cct) << "invalid IO request: " << cpp_strerror(r) << dendl; @@ -169,7 +173,8 @@ ssize_t Io::compare_and_write( << off << ", " << "len = " << len << dendl; image_ctx.image_lock.lock_shared(); - int r = clip_io(util::get_image_ctx(&image_ctx), off, &len); + int r = clip_io(util::get_image_ctx(&image_ctx), off, &len, + io::ImageArea::DATA); image_ctx.image_lock.unlock_shared(); if (r < 0) { lderr(cct) << "invalid IO request: " << cpp_strerror(r) << dendl; diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc index 0cefd4b3603d2..297172e2c583c 100644 --- a/src/librbd/internal.cc +++ b/src/librbd/internal.cc @@ -1542,7 +1542,7 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { uint64_t mylen = len; ictx->image_lock.lock_shared(); - r = clip_io(ictx, off, &mylen); + r = clip_io(ictx, off, &mylen, io::ImageArea::DATA); ictx->image_lock.unlock_shared(); if (r < 0) return r; @@ -1595,9 +1595,8 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { return total_read; } - // validate extent against image size; clip to image size if necessary - int clip_io(ImageCtx *ictx, uint64_t off, uint64_t *len) - { + // validate extent against area size; clip to area size if necessary + int clip_io(ImageCtx* ictx, uint64_t off, uint64_t* len, io::ImageArea area) { ceph_assert(ceph_mutex_is_locked(ictx->image_lock)); if (ictx->snap_id != CEPH_NOSNAP && @@ -1609,8 +1608,7 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { if (*len == 0) return 0; - // TODO: pass area - uint64_t area_size = ictx->get_area_size(io::ImageArea::DATA); + uint64_t area_size = ictx->get_area_size(area); // can't start past end if (off >= area_size) diff --git a/src/librbd/internal.h b/src/librbd/internal.h index 2b2400b8c8cfe..50314a6e1780d 100644 --- a/src/librbd/internal.h +++ b/src/librbd/internal.h @@ -20,7 +20,10 @@ namespace librbd { struct ImageCtx; - namespace io { struct AioCompletion; } + namespace io { + struct AioCompletion; + enum class ImageArea; + } class NoOpProgressContext : public ProgressContext { @@ -122,7 +125,7 @@ namespace librbd { void image_info(const ImageCtx *ictx, image_info_t& info, size_t info_size); uint64_t oid_to_object_no(const std::string& oid, const std::string& object_prefix); - int clip_io(ImageCtx *ictx, uint64_t off, uint64_t *len); + int clip_io(ImageCtx* ictx, uint64_t off, uint64_t* len, io::ImageArea area); void init_rbd_header(struct rbd_obj_header_ondisk& ondisk, uint64_t size, int order, uint64_t bid); diff --git a/src/librbd/io/ImageDispatcher.cc b/src/librbd/io/ImageDispatcher.cc index 2dfdc58d4990e..e7e5dda959639 100644 --- a/src/librbd/io/ImageDispatcher.cc +++ b/src/librbd/io/ImageDispatcher.cc @@ -135,8 +135,11 @@ struct ImageDispatcher::PreprocessVisitor } bool clip_request() const { + auto area = (image_dispatch_spec->image_dispatch_flags & + IMAGE_DISPATCH_FLAG_CRYPTO_HEADER ? ImageArea::CRYPTO_HEADER : + ImageArea::DATA); int r = util::clip_request(image_dispatcher->m_image_ctx, - &image_dispatch_spec->image_extents); + &image_dispatch_spec->image_extents, area); if (r < 0) { image_dispatch_spec->fail(r); return true; diff --git a/src/librbd/io/Utils.cc b/src/librbd/io/Utils.cc index 6b9fa5dca1706..a8cef0e41bb93 100644 --- a/src/librbd/io/Utils.cc +++ b/src/librbd/io/Utils.cc @@ -140,12 +140,12 @@ void read_parent(I *image_ctx, uint64_t object_no, ReadExtents* read_extents, } template -int clip_request(I *image_ctx, Extents *image_extents) { +int clip_request(I* image_ctx, Extents* image_extents, ImageArea area) { std::shared_lock image_locker{image_ctx->image_lock}; for (auto &image_extent : *image_extents) { auto clip_len = image_extent.second; int r = clip_io(librbd::util::get_image_ctx(image_ctx), - image_extent.first, &clip_len); + image_extent.first, &clip_len, area); if (r < 0) { return r; } @@ -231,7 +231,7 @@ template void librbd::io::util::read_parent( librbd::ImageCtx *image_ctx, uint64_t object_no, ReadExtents* extents, librados::snap_t snap_id, const ZTracer::Trace &trace, Context* on_finish); template int librbd::io::util::clip_request( - librbd::ImageCtx *image_ctx, Extents *image_extents); + librbd::ImageCtx* image_ctx, Extents* image_extents, ImageArea area); template bool librbd::io::util::trigger_copyup( librbd::ImageCtx *image_ctx, uint64_t object_no, IOContext io_context, Context* on_finish); diff --git a/src/librbd/io/Utils.h b/src/librbd/io/Utils.h index 04e8ba0e04a0e..efb79b6a64da8 100644 --- a/src/librbd/io/Utils.h +++ b/src/librbd/io/Utils.h @@ -36,7 +36,7 @@ void read_parent(ImageCtxT *image_ctx, uint64_t object_no, const ZTracer::Trace &trace, Context* on_finish); template -int clip_request(ImageCtxT *image_ctx, Extents *image_extents); +int clip_request(ImageCtxT* image_ctx, Extents* image_extents, ImageArea area); inline uint64_t get_extents_length(const Extents &extents) { uint64_t total_bytes = 0; diff --git a/src/test/librbd/test_librbd.cc b/src/test/librbd/test_librbd.cc index 543a580568da6..ec31b861b7aed 100644 --- a/src/test/librbd/test_librbd.cc +++ b/src/test/librbd/test_librbd.cc @@ -2804,6 +2804,97 @@ TEST_F(TestLibRBD, EncryptedResize) } } +TEST_F(TestLibRBD, EncryptedFlattenSmallData) +{ + REQUIRE_FEATURE(RBD_FEATURE_LAYERING); + REQUIRE(!is_feature_enabled(RBD_FEATURE_STRIPINGV2)); + REQUIRE(!is_feature_enabled(RBD_FEATURE_JOURNALING)); + + librados::IoCtx ioctx; + ASSERT_EQ(0, _rados.ioctx_create(m_pool_name.c_str(), ioctx)); + + librbd::RBD rbd; + std::string parent_name = get_temp_image_name(); + std::string clone_name = get_temp_image_name(); + uint64_t data_size = 5000; + uint64_t luks2_meta_size = 16 << 20; + std::string passphrase = "some passphrase"; + + { + int order = 22; + ASSERT_EQ(0, create_image_pp(rbd, ioctx, parent_name.c_str(), + luks2_meta_size + data_size, &order)); + librbd::Image parent; + ASSERT_EQ(0, rbd.open(ioctx, parent, parent_name.c_str(), nullptr)); + + librbd::encryption_luks2_format_options_t opts = { + RBD_ENCRYPTION_ALGORITHM_AES256, passphrase}; + ASSERT_EQ(0, parent.encryption_format(RBD_ENCRYPTION_FORMAT_LUKS2, &opts, + sizeof(opts))); + + ceph::bufferlist bl; + bl.append(std::string(data_size, 'a')); + ASSERT_EQ(data_size, parent.write(0, data_size, bl)); + + ASSERT_EQ(0, parent.snap_create("snap")); + ASSERT_EQ(0, parent.snap_protect("snap")); + uint64_t features; + ASSERT_EQ(0, parent.features(&features)); + ASSERT_EQ(0, rbd.clone(ioctx, parent_name.c_str(), "snap", ioctx, + clone_name.c_str(), features, &order)); + } + + { + librbd::Image clone; + ASSERT_EQ(0, rbd.open(ioctx, clone, clone_name.c_str(), nullptr)); + + librbd::encryption_luks_format_options_t opts = {passphrase}; + ASSERT_EQ(0, clone.encryption_load(RBD_ENCRYPTION_FORMAT_LUKS, &opts, + sizeof(opts))); + uint64_t size; + ASSERT_EQ(0, clone.size(&size)); + ASSERT_EQ(data_size, size); + uint64_t overlap; + ASSERT_EQ(0, clone.overlap(&overlap)); + ASSERT_EQ(data_size, overlap); + + ceph::bufferlist expected_bl; + expected_bl.append(std::string(data_size, 'a')); + + ceph::bufferlist read_bl1; + ASSERT_EQ(data_size, clone.read(0, data_size, read_bl1)); + ASSERT_TRUE(expected_bl.contents_equal(read_bl1)); + + ASSERT_EQ(0, clone.flatten()); + + ceph::bufferlist read_bl2; + ASSERT_EQ(data_size, clone.read(0, data_size, read_bl2)); + ASSERT_TRUE(expected_bl.contents_equal(read_bl2)); + } + + { + librbd::Image clone; + ASSERT_EQ(0, rbd.open(ioctx, clone, clone_name.c_str(), nullptr)); + + librbd::encryption_luks_format_options_t opts = {passphrase}; + ASSERT_EQ(0, clone.encryption_load(RBD_ENCRYPTION_FORMAT_LUKS, &opts, + sizeof(opts))); + uint64_t size; + ASSERT_EQ(0, clone.size(&size)); + ASSERT_EQ(data_size, size); + uint64_t overlap; + ASSERT_EQ(0, clone.overlap(&overlap)); + ASSERT_EQ(0, overlap); + + ceph::bufferlist expected_bl; + expected_bl.append(std::string(data_size, 'a')); + + ceph::bufferlist read_bl; + ASSERT_EQ(data_size, clone.read(0, data_size, read_bl)); + ASSERT_TRUE(expected_bl.contents_equal(read_bl)); + } +} + #endif TEST_F(TestLibRBD, TestIOWithIOHint) -- 2.39.5