From 69394af46b0df8a54317558f0a36c9ba5b177b74 Mon Sep 17 00:00:00 2001
From: Ilya Dryomov <idryomov@gmail.com>
Date: Tue, 28 Jun 2022 20:47:25 +0200
Subject: [PATCH] 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)
---
 src/librbd/deep_copy/ImageCopyRequest.cc      | 18 +++++--------
 src/librbd/deep_copy/ImageCopyRequest.h       |  2 +-
 .../deep_copy/test_mock_ImageCopyRequest.cc   | 26 +++++++++++++++++++
 3 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/src/librbd/deep_copy/ImageCopyRequest.cc b/src/librbd/deep_copy/ImageCopyRequest.cc
index e880071118730..08e959dd57232 100644
--- a/src/librbd/deep_copy/ImageCopyRequest.cc
+++ b/src/librbd/deep_copy/ImageCopyRequest.cc
@@ -147,11 +147,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;
@@ -163,7 +160,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) {
@@ -171,10 +168,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++;
@@ -209,7 +204,7 @@ int ImageCopyRequest<I>::send_next_object_copy() {
     if (object_diff_state == object_map::DIFF_STATE_HOLE) {
       ldout(m_cct, 20) << "skipping non-existent object " << ono << dendl;
       create_async_context_callback(*m_src_image_ctx, ctx)->complete(0);
-      return 0;
+      return;
     }
   }
 
@@ -226,7 +221,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 9b7934dd35adb..cb8b83781a5c5 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 2763554b94667..e38ffffdbe493 100644
--- a/src/test/librbd/deep_copy/test_mock_ImageCopyRequest.cc
+++ b/src/test/librbd/deep_copy/test_mock_ImageCopyRequest.cc
@@ -707,6 +707,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);
-- 
2.39.5