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 e880071118730422f7f2371f5c2f266e974fcee7..08e959dd572325f74beda92c334167a29836e0ab 100644 (file)
--- 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 9b7934dd35adb9ae98ee3f0b7beb2015d7b2bd66..cb8b83781a5c55908df397fe4beb1444bf8fd1bb 100644 (file)
--- 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 2763554b94667aafd7f8a7d4eefefad063208975..e38ffffdbe49380a015de99ebc9b7ee1adcd8ffe 100644 (file)
--- 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);