From: Ilya Dryomov <idryomov@gmail.com>
Date: Tue, 28 Jun 2022 18:47:25 +0000 (+0200)
Subject: librbd: make ImageCopyRequest::send_next_object_copy() return void
X-Git-Tag: v15.2.17~9^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=76503a9fa6919b8e38171e43c6d3a4e12bd6fd8c;p=ceph.git

librbd: make ImageCopyRequest::send_next_object_copy() return void

Make send_object_copies() consistent with handle_object_copy() wrt
calling send_next_object_copy().

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
(cherry picked from commit 155fabd994f8a2052482d216df1ec0dfb40cb5b7)
---

diff --git a/src/librbd/deep_copy/ImageCopyRequest.cc b/src/librbd/deep_copy/ImageCopyRequest.cc
index 3a177d13b82..70ff6b98f6e 100644
--- a/src/librbd/deep_copy/ImageCopyRequest.cc
+++ b/src/librbd/deep_copy/ImageCopyRequest.cc
@@ -149,11 +149,8 @@ void ImageCopyRequest<I>::send_object_copies() {
 
     // attempt to schedule at least 'max_ops' initial requests where
     // some objects might be skipped if fast-diff notes no change
-    while (m_current_ops < max_ops) {
-      int r = send_next_object_copy();
-      if (r < 0) {
-        break;
-      }
+    for (uint64_t i = 0; i < max_ops; i++) {
+      send_next_object_copy();
     }
 
     complete = (m_current_ops == 0) && !m_updating_progress;
@@ -165,7 +162,7 @@ void ImageCopyRequest<I>::send_object_copies() {
 }
 
 template <typename I>
-int ImageCopyRequest<I>::send_next_object_copy() {
+void ImageCopyRequest<I>::send_next_object_copy() {
   ceph_assert(ceph_mutex_is_locked(m_lock));
 
   if (m_canceled && m_ret_val == 0) {
@@ -173,10 +170,8 @@ int ImageCopyRequest<I>::send_next_object_copy() {
     m_ret_val = -ECANCELED;
   }
 
-  if (m_ret_val < 0) {
-    return m_ret_val;
-  } else if (m_object_no >= m_end_object_no) {
-    return -ENODATA;
+  if (m_ret_val < 0 || m_object_no >= m_end_object_no) {
+    return;
   }
 
   uint64_t ono = m_object_no++;
@@ -204,7 +199,7 @@ int ImageCopyRequest<I>::send_next_object_copy() {
     if (skip) {
       ldout(m_cct, 20) << "skipping clean object " << ono << dendl;
       create_async_context_callback(*m_src_image_ctx, ctx)->complete(0);
-      return 0;
+      return;
     }
   }
 
@@ -217,7 +212,6 @@ int ImageCopyRequest<I>::send_next_object_copy() {
     m_src_image_ctx, m_dst_image_ctx, m_src_snap_id_start, m_dst_snap_id_start,
     m_snap_map, ono, flags, m_handler, ctx);
   req->send();
-  return 0;
 }
 
 template <typename I>
diff --git a/src/librbd/deep_copy/ImageCopyRequest.h b/src/librbd/deep_copy/ImageCopyRequest.h
index 9b7934dd35a..cb8b83781a5 100644
--- a/src/librbd/deep_copy/ImageCopyRequest.h
+++ b/src/librbd/deep_copy/ImageCopyRequest.h
@@ -109,7 +109,7 @@ private:
   void handle_compute_diff(int r);
 
   void send_object_copies();
-  int send_next_object_copy();
+  void send_next_object_copy();
   void handle_object_copy(uint64_t object_no, int r);
 
   void finish(int r);
diff --git a/src/test/librbd/deep_copy/test_mock_ImageCopyRequest.cc b/src/test/librbd/deep_copy/test_mock_ImageCopyRequest.cc
index f296e3cd832..1b05fa1ad06 100644
--- a/src/test/librbd/deep_copy/test_mock_ImageCopyRequest.cc
+++ b/src/test/librbd/deep_copy/test_mock_ImageCopyRequest.cc
@@ -669,6 +669,32 @@ TEST_F(TestMockDeepCopyImageCopyRequest, Cancel_Inflight_Sync) {
   ASSERT_EQ(5u, handler.object_number.get());
 }
 
+TEST_F(TestMockDeepCopyImageCopyRequest, CancelBeforeSend) {
+  librados::snap_t snap_id_end;
+  ASSERT_EQ(0, create_snap("copy", &snap_id_end));
+
+  librbd::MockTestImageCtx mock_src_image_ctx(*m_src_image_ctx);
+  librbd::MockTestImageCtx mock_dst_image_ctx(*m_dst_image_ctx);
+
+  InSequence seq;
+
+  MockDiffRequest mock_diff_request;
+  expect_diff_send(mock_diff_request, {}, -EINVAL);
+  expect_get_image_size(mock_src_image_ctx, 2 * (1 << m_src_image_ctx->order));
+  expect_get_image_size(mock_src_image_ctx, 0);
+
+  librbd::deep_copy::NoOpHandler no_op;
+  C_SaferCond ctx;
+  auto request = new MockImageCopyRequest(&mock_src_image_ctx,
+                                          &mock_dst_image_ctx,
+                                          0, snap_id_end, 0, false, boost::none,
+                                          m_snap_seqs, &no_op, &ctx);
+  request->cancel();
+  request->send();
+
+  ASSERT_EQ(-ECANCELED, ctx.wait());
+}
+
 TEST_F(TestMockDeepCopyImageCopyRequest, MissingSnap) {
   librbd::MockTestImageCtx mock_src_image_ctx(*m_src_image_ctx);
   librbd::MockTestImageCtx mock_dst_image_ctx(*m_dst_image_ctx);