librbd: non-pruning parent overlap handling fixes

Apply similar "reduce overlap and respect area" logic to places
that don't use prune_parent_extents().  Changes to FlattenRequest
and TrimRequest here should complete the long tail of encrypted
I/O path and flatten fixes.

Signed-off-by: Ilya Dryomov <[email protected]>

src/librbd/Operations.cc

@@ -541,19 +541,24 @@ void Operations<I>::execute_flatten(ProgressContext &prog_ctx,
     return;
   }
 
-  uint64_t overlap;
-  int r = m_image_ctx.get_parent_overlap(CEPH_NOSNAP, &overlap);
-  ceph_assert(r == 0);
-  ceph_assert(overlap <= m_image_ctx.size);
+  uint64_t crypto_header_objects = Striper::get_num_objects(
+      m_image_ctx.layout,
+      m_image_ctx.get_area_size(io::ImageArea::CRYPTO_HEADER));
 
-  uint64_t overlap_objects = Striper::get_num_objects(m_image_ctx.layout,
-                                                      overlap);
+  uint64_t raw_overlap;
+  int r = m_image_ctx.get_parent_overlap(CEPH_NOSNAP, &raw_overlap);
+  ceph_assert(r == 0);
+  auto overlap = m_image_ctx.reduce_parent_overlap(raw_overlap, false);
+  uint64_t data_overlap_objects = Striper::get_num_objects(
+      m_image_ctx.layout,
+      (overlap.second == io::ImageArea::DATA ? overlap.first : 0));
 
   m_image_ctx.image_lock.unlock_shared();
 
+  // leave encryption header flattening to format-specific handler
   operation::FlattenRequest<I> *req = new operation::FlattenRequest<I>(
-    m_image_ctx, new C_NotifyUpdate<I>(m_image_ctx, on_finish), overlap_objects,
-    prog_ctx);
+      m_image_ctx, new C_NotifyUpdate<I>(m_image_ctx, on_finish),
+      crypto_header_objects, data_overlap_objects, prog_ctx);
   req->send();
 }
 

src/librbd/api/DiffIterate.cc

@@ -263,12 +263,14 @@ int DiffIterate<I>::execute() {
       // check parent overlap only if we are comparing to the beginning of time
       if (m_include_parent && from_snap_id == 0) {
         std::shared_lock image_locker{m_image_ctx.image_lock};
-        uint64_t overlap = 0;
-        m_image_ctx.get_parent_overlap(m_image_ctx.snap_id, &overlap);
-        if (m_image_ctx.parent && overlap > 0) {
+        uint64_t raw_overlap = 0;
+        m_image_ctx.get_parent_overlap(m_image_ctx.snap_id, &raw_overlap);
+        auto overlap = m_image_ctx.reduce_parent_overlap(raw_overlap, false);
+        if (overlap.first > 0 && overlap.second == io::ImageArea::DATA) {
           ldout(cct, 10) << " first getting parent diff" << dendl;
-          DiffIterate diff_parent(*m_image_ctx.parent, {}, nullptr, 0, overlap,
-                                  true, true, &simple_diff_cb, &parent_diff);
+          DiffIterate diff_parent(*m_image_ctx.parent, {}, nullptr, 0,
+                                  overlap.first, true, true, &simple_diff_cb,
+                                  &parent_diff);
           r = diff_parent.execute();
           if (r < 0) {
             return r;

src/librbd/operation/FlattenRequest.cc

@@ -97,15 +97,6 @@ void FlattenRequest<I>::flatten_objects() {
   CephContext *cct = image_ctx.cct;
   ldout(cct, 5) << dendl;
 
-  auto encryption_format = image_ctx.encryption_format.get();
-  uint64_t start_object_no = 0;
-  if (encryption_format != nullptr) {
-    // leave encryption header flattening to format-specific handler
-    start_object_no = Striper::get_num_objects(
-            image_ctx.layout,
-            encryption_format->get_crypto()->get_data_offset());
-  }
-
   assert(ceph_mutex_is_locked(image_ctx.owner_lock));
   auto ctx = create_context_callback<
     FlattenRequest<I>,
@@ -115,8 +106,8 @@ void FlattenRequest<I>::flatten_objects() {
       boost::lambda::_1, &image_ctx, image_ctx.get_data_io_context(),
       boost::lambda::_2));
   AsyncObjectThrottle<I> *throttle = new AsyncObjectThrottle<I>(
-    this, image_ctx, context_factory, ctx, &m_prog_ctx, start_object_no,
-    m_overlap_objects);
+      this, image_ctx, context_factory, ctx, &m_prog_ctx, m_start_object_no,
+      m_start_object_no + m_overlap_objects);
   throttle->start_ops(
     image_ctx.config.template get_val<uint64_t>("rbd_concurrent_management_ops"));
 }

src/librbd/operation/FlattenRequest.h

@@ -17,10 +17,12 @@ class FlattenRequest : public Request<ImageCtxT>
 {
 public:
   FlattenRequest(ImageCtxT &image_ctx, Context *on_finish,
-                 uint64_t overlap_objects, ProgressContext &prog_ctx)
-    : Request<ImageCtxT>(image_ctx, on_finish),
-      m_overlap_objects(overlap_objects), m_prog_ctx(prog_ctx) {
-  }
+                 uint64_t start_object_no, uint64_t overlap_objects,
+                 ProgressContext& prog_ctx)
+      : Request<ImageCtxT>(image_ctx, on_finish),
+        m_start_object_no(start_object_no),
+        m_overlap_objects(overlap_objects),
+        m_prog_ctx(prog_ctx) {}
 
 protected:
   void send_op() override;
@@ -54,6 +56,7 @@ class FlattenRequest : public Request<ImageCtxT>
    * @endverbatim
    */
 
+  uint64_t m_start_object_no;
   uint64_t m_overlap_objects;
   ProgressContext &m_prog_ctx;
 

src/librbd/operation/SparsifyRequest.cc

@@ -11,6 +11,7 @@
 #include "librbd/ImageCtx.h"
 #include "librbd/Types.h"
 #include "librbd/io/ObjectRequest.h"
+#include "librbd/io/Utils.h"
 #include "osdc/Striper.h"
 #include <boost/lambda/bind.hpp>
 #include <boost/lambda/construct.hpp>
@@ -143,13 +144,17 @@ class C_SparsifyObject : public C_AsyncObjectThrottle<I> {
         return 1;
       }
 
-      uint64_t overlap_objects = 0;
-      uint64_t overlap;
-      int r = image_ctx.get_parent_overlap(CEPH_NOSNAP, &overlap);
-      if (r == 0 && overlap > 0) {
-        overlap_objects = Striper::get_num_objects(image_ctx.layout, overlap);
+      uint64_t raw_overlap = 0;
+      uint64_t object_overlap = 0;
+      int r = image_ctx.get_parent_overlap(CEPH_NOSNAP, &raw_overlap);
+      ceph_assert(r == 0);
+      if (raw_overlap > 0) {
+        auto [parent_extents, area] = io::util::object_to_area_extents(
+            &image_ctx, m_object_no, {{0, image_ctx.layout.object_size}});
+        object_overlap = image_ctx.prune_parent_extents(parent_extents, area,
+                                                        raw_overlap, false);
       }
-      m_remove_empty = (m_object_no >= overlap_objects);
+      m_remove_empty = object_overlap == 0;
     }
 
     send_sparsify();

src/librbd/operation/TrimRequest.cc

@@ -223,20 +223,30 @@ void TrimRequest<I>::send_copyup_objects() {
 
   IOContext io_context;
   bool has_snapshots;
-  uint64_t parent_overlap;
+  uint64_t copyup_end;
   {
     std::shared_lock image_locker{image_ctx.image_lock};
 
     io_context = image_ctx.get_data_io_context();
     has_snapshots = !image_ctx.snaps.empty();
-    int r = image_ctx.get_parent_overlap(CEPH_NOSNAP, &parent_overlap);
+
+    uint64_t crypto_header_objects = Striper::get_num_objects(
+        image_ctx.layout,
+        image_ctx.get_area_size(io::ImageArea::CRYPTO_HEADER));
+
+    uint64_t raw_overlap;
+    int r = image_ctx.get_parent_overlap(CEPH_NOSNAP, &raw_overlap);
     ceph_assert(r == 0);
+    auto overlap = image_ctx.reduce_parent_overlap(raw_overlap, false);
+    uint64_t data_overlap_objects = Striper::get_num_objects(
+        image_ctx.layout,
+        (overlap.second == io::ImageArea::DATA ? overlap.first : 0));
+
+    // copyup is only required for portion of image that overlaps parent
+    ceph_assert(m_delete_start >= crypto_header_objects);
+    copyup_end = crypto_header_objects + data_overlap_objects;
   }
 
-  // copyup is only required for portion of image that overlaps parent
-  uint64_t copyup_end = Striper::get_num_objects(image_ctx.layout,
-                                                 parent_overlap);
-
   // TODO: protect against concurrent shrink and snap create?
   // skip to remove if no copyup is required.
   if (copyup_end <= m_delete_start || !has_snapshots) {

src/test/librbd/operation/test_mock_TrimRequest.cc

@@ -156,6 +156,17 @@ class TestMockOperationTrimRequest : public TestMockFixture {
                            })));
   }
 
+  void expect_reduce_parent_overlap(MockTestImageCtx& mock_image_ctx,
+                                    uint64_t overlap) {
+    EXPECT_CALL(mock_image_ctx, reduce_parent_overlap(overlap, false))
+      .WillOnce(Return(std::make_pair(overlap, io::ImageArea::DATA)));
+  }
+
+  void expect_get_area_size(MockTestImageCtx& mock_image_ctx) {
+    EXPECT_CALL(mock_image_ctx, get_area_size(io::ImageArea::CRYPTO_HEADER))
+      .WillOnce(Return(0));
+  }
+
   void expect_object_may_exist(MockTestImageCtx &mock_image_ctx,
                                uint64_t object_no, bool exists) {
     if (mock_image_ctx.object_map != nullptr) {
@@ -221,7 +232,9 @@ TEST_F(TestMockOperationTrimRequest, SuccessRemove) {
                            true, 0);
 
   // copy-up
+  expect_get_area_size(mock_image_ctx);
   expect_get_parent_overlap(mock_image_ctx, 0);
+  expect_reduce_parent_overlap(mock_image_ctx, 0);
 
   // remove
   expect_object_may_exist(mock_image_ctx, 0, true);
@@ -277,7 +290,9 @@ TEST_F(TestMockOperationTrimRequest, SuccessCopyUp) {
 
   // copy-up
   io::MockObjectDispatch mock_io_object_dispatch;
+  expect_get_area_size(mock_image_ctx);
   expect_get_parent_overlap(mock_image_ctx, ictx->get_object_size());
+  expect_reduce_parent_overlap(mock_image_ctx, ictx->get_object_size());
   expect_get_object_name(mock_image_ctx, 0, "object0");
   expect_object_discard(mock_image_ctx, mock_io_object_dispatch, 0,
                         ictx->get_object_size(), false, 0);
@@ -369,7 +384,9 @@ TEST_F(TestMockOperationTrimRequest, RemoveError) {
                            false, 0);
 
   // copy-up
+  expect_get_area_size(mock_image_ctx);
   expect_get_parent_overlap(mock_image_ctx, 0);
+  expect_reduce_parent_overlap(mock_image_ctx, 0);
 
   // remove
   expect_object_may_exist(mock_image_ctx, 0, true);
@@ -421,7 +438,9 @@ TEST_F(TestMockOperationTrimRequest, CopyUpError) {
 
   // copy-up
   io::MockObjectDispatch mock_io_object_dispatch;
+  expect_get_area_size(mock_image_ctx);
   expect_get_parent_overlap(mock_image_ctx, ictx->get_object_size());
+  expect_reduce_parent_overlap(mock_image_ctx, ictx->get_object_size());
   expect_get_object_name(mock_image_ctx, 0, "object0");
   expect_object_discard(mock_image_ctx, mock_io_object_dispatch, 0,
                         ictx->get_object_size(), false, -EINVAL);