Merge pull request ceph#38494 from dillaman/wip-45694

librbd: fix sporadic failures in TestMigration.StressLive Reviewed-by: Mykola Golub <[email protected]>
JiYou · Dec 11, 2020 · cbbfd1c · cbbfd1c
2 parents 286c6b4 + 1baba64
commit cbbfd1c
Show file tree

Hide file tree

Showing 9 changed files with 40 additions and 16 deletions.
diff --git a/src/librbd/deep_copy/ImageCopyRequest.cc b/src/librbd/deep_copy/ImageCopyRequest.cc
@@ -199,13 +199,18 @@ int ImageCopyRequest<I>::send_next_object_copy() {
   ldout(m_cct, 20) << "object_num=" << ono << dendl;
   ++m_current_ops;
 
+  uint32_t flags = 0;
+  if (m_flatten) {
+    flags |= OBJECT_COPY_REQUEST_FLAG_FLATTEN;
+  }
+
   Context *ctx = new LambdaContext(
     [this, ono](int r) {
       handle_object_copy(ono, r);
     });
   auto req = ObjectCopyRequest<I>::create(
     m_src_image_ctx, m_dst_image_ctx, m_src_snap_id_start, m_dst_snap_id_start,
-    m_snap_map, ono, m_flatten, m_handler, ctx);
+    m_snap_map, ono, flags, m_handler, ctx);
   req->send();
   return 0;
 }

diff --git a/src/librbd/deep_copy/ObjectCopyRequest.cc b/src/librbd/deep_copy/ObjectCopyRequest.cc
@@ -37,13 +37,13 @@ ObjectCopyRequest<I>::ObjectCopyRequest(I *src_image_ctx,
                                         librados::snap_t dst_snap_id_start,
                                         const SnapMap &snap_map,
                                         uint64_t dst_object_number,
-                                        bool flatten, Handler* handler,
+                                        uint32_t flags, Handler* handler,
                                         Context *on_finish)
   : m_src_image_ctx(src_image_ctx),
     m_dst_image_ctx(dst_image_ctx), m_cct(dst_image_ctx->cct),
     m_src_snap_id_start(src_snap_id_start),
     m_dst_snap_id_start(dst_snap_id_start), m_snap_map(snap_map),
-    m_dst_object_number(dst_object_number), m_flatten(flatten),
+    m_dst_object_number(dst_object_number), m_flags(flags),
     m_handler(handler), m_on_finish(on_finish) {
   ceph_assert(src_image_ctx->data_ctx.is_valid());
   ceph_assert(dst_image_ctx->data_ctx.is_valid());
@@ -339,7 +339,9 @@ void ObjectCopyRequest<I>::send_write_object() {
                    << "dst_snaps=" << dst_snap_ids << dendl;
 
   librados::ObjectWriteOperation op;
-  if (!m_dst_image_ctx->migration_info.empty()) {
+
+  bool migration = ((m_flags & OBJECT_COPY_REQUEST_FLAG_MIGRATION) != 0);
+  if (migration) {
     ldout(m_cct, 20) << "assert_snapc_seq=" << dst_snap_seq << dendl;
     cls_client::assert_snapc_seq(&op, dst_snap_seq,
                                  cls::rbd::ASSERT_SNAPC_SEQ_GT_SNAPSET_SEQ);
@@ -381,7 +383,7 @@ void ObjectCopyRequest<I>::send_write_object() {
     }
   }
 
-  if (op.size() == (m_dst_image_ctx->migration_info.empty() ? 0 : 1)) {
+  if (op.size() == (migration ? 1 : 0)) {
     handle_write_object(0);
     return;
   }
@@ -513,7 +515,8 @@ void ObjectCopyRequest<I>::compute_read_ops() {
     }
   }
 
-  if (!dne_image_interval.empty() && (!only_dne_extents || m_flatten)) {
+  bool flatten = ((m_flags & OBJECT_COPY_REQUEST_FLAG_FLATTEN) != 0);
+  if (!dne_image_interval.empty() && (!only_dne_extents || flatten)) {
     auto snap_map_it = m_snap_map.begin();
     ceph_assert(snap_map_it != m_snap_map.end());
 

diff --git a/src/librbd/deep_copy/ObjectCopyRequest.h b/src/librbd/deep_copy/ObjectCopyRequest.h
@@ -34,17 +34,17 @@ class ObjectCopyRequest {
                                    librados::snap_t src_snap_id_start,
                                    librados::snap_t dst_snap_id_start,
                                    const SnapMap &snap_map,
-                                   uint64_t object_number, bool flatten,
+                                   uint64_t object_number, uint32_t flags,
                                    Handler* handler, Context *on_finish) {
     return new ObjectCopyRequest(src_image_ctx, dst_image_ctx,
                                  src_snap_id_start, dst_snap_id_start, snap_map,
-                                 object_number, flatten, handler, on_finish);
+                                 object_number, flags, handler, on_finish);
   }
 
   ObjectCopyRequest(ImageCtxT *src_image_ctx, ImageCtxT *dst_image_ctx,
                     librados::snap_t src_snap_id_start,
                     librados::snap_t dst_snap_id_start, const SnapMap &snap_map,
-                    uint64_t object_number, bool flatten, Handler* handler,
+                    uint64_t object_number, uint32_t flags, Handler* handler,
                     Context *on_finish);
 
   void send();
@@ -107,7 +107,7 @@ class ObjectCopyRequest {
   librados::snap_t m_dst_snap_id_start;
   SnapMap m_snap_map;
   uint64_t m_dst_object_number;
-  bool m_flatten;
+  uint32_t m_flags;
   Handler* m_handler;
   Context *m_on_finish;
 

diff --git a/src/librbd/deep_copy/Types.h b/src/librbd/deep_copy/Types.h
@@ -11,6 +11,11 @@
 namespace librbd {
 namespace deep_copy {
 
+enum {
+  OBJECT_COPY_REQUEST_FLAG_FLATTEN   = 1U << 0,
+  OBJECT_COPY_REQUEST_FLAG_MIGRATION = 1U << 1,
+};
+
 typedef std::vector<librados::snap_t> SnapIds;
 typedef std::map<librados::snap_t, SnapIds> SnapMap;
 

diff --git a/src/librbd/io/CopyupRequest.cc b/src/librbd/io/CopyupRequest.cc
@@ -258,11 +258,16 @@ void CopyupRequest<I>::deep_copy() {
 
   ldout(cct, 20) << "flatten=" << m_flatten << dendl;
 
+  uint32_t flags = deep_copy::OBJECT_COPY_REQUEST_FLAG_MIGRATION;
+  if (m_flatten) {
+    flags |= deep_copy::OBJECT_COPY_REQUEST_FLAG_FLATTEN;
+  }
+
   auto ctx = librbd::util::create_context_callback<
     CopyupRequest<I>, &CopyupRequest<I>::handle_deep_copy>(this);
   auto req = deep_copy::ObjectCopyRequest<I>::create(
     m_image_ctx->parent, m_image_ctx, 0, 0,
-    m_image_ctx->migration_info.snap_map, m_object_no, m_flatten, nullptr, ctx);
+    m_image_ctx->migration_info.snap_map, m_object_no, flags, nullptr, ctx);
 
   req->send();
 }

diff --git a/src/librbd/operation/MigrateRequest.cc b/src/librbd/operation/MigrateRequest.cc
@@ -128,9 +128,14 @@ class C_MigrateObject : public C_AsyncObjectThrottle<I> {
     } else {
       ceph_assert(image_ctx.parent != nullptr);
 
+      uint32_t flags = deep_copy::OBJECT_COPY_REQUEST_FLAG_MIGRATION;
+      if (image_ctx.migration_info.flatten) {
+        flags |= deep_copy::OBJECT_COPY_REQUEST_FLAG_FLATTEN;
+      }
+
       auto req = deep_copy::ObjectCopyRequest<I>::create(
         image_ctx.parent, &image_ctx, 0, 0, image_ctx.migration_info.snap_map,
-        m_object_no, image_ctx.migration_info.flatten, nullptr, ctx);
+        m_object_no, flags, nullptr, ctx);
 
       ldout(cct, 20) << "deep copy object req " << req << ", object_no "
                      << m_object_no << dendl;

diff --git a/src/test/librbd/deep_copy/test_mock_ImageCopyRequest.cc b/src/test/librbd/deep_copy/test_mock_ImageCopyRequest.cc
@@ -54,7 +54,7 @@ struct ObjectCopyRequest<librbd::MockTestImageCtx> {
       librados::snap_t src_snap_id_start,
       librados::snap_t dst_snap_id_start,
       const SnapMap &snap_map,
-      uint64_t object_number, bool flatten, Handler* handler,
+      uint64_t object_number, uint32_t flags, Handler* handler,
       Context *on_finish) {
     ceph_assert(s_instance != nullptr);
     std::lock_guard locker{s_instance->lock};

diff --git a/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc b/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc
@@ -232,7 +232,7 @@ class TestMockDeepCopyObjectCopyRequest : public TestMockFixture {
     expect_get_object_name(mock_dst_image_ctx);
     return new MockObjectCopyRequest(&mock_src_image_ctx, &mock_dst_image_ctx,
                                      src_snap_id_start, dst_snap_id_start,
-                                     m_snap_map, 0, false, nullptr, on_finish);
+                                     m_snap_map, 0, 0, nullptr, on_finish);
   }
 
   void expect_read(librbd::MockTestImageCtx& mock_image_ctx,

diff --git a/src/test/librbd/io/test_mock_CopyupRequest.cc b/src/test/librbd/io/test_mock_CopyupRequest.cc
@@ -55,11 +55,12 @@ struct ObjectCopyRequest<librbd::MockTestImageCtx> {
                                    librados::snap_t src_snap_id_start,
                                    librados::snap_t dst_snap_id_start,
                                    const SnapMap &snap_map,
-                                   uint64_t object_number, bool flatten,
+                                   uint64_t object_number, uint32_t flags,
                                    Handler*, Context *on_finish) {
     ceph_assert(s_instance != nullptr);
     s_instance->object_number = object_number;
-    s_instance->flatten = flatten;
+    s_instance->flatten = (
+      (flags & deep_copy::OBJECT_COPY_REQUEST_FLAG_FLATTEN) != 0);
     s_instance->on_finish = on_finish;
     return s_instance;
   }