librbd: the first post-migration snapshot isn't always dirty

Currently, the first post-migration snapshot is always marked EXISTS
(i.e. dirty).  This is wrong, because the data can be inherited from
a pre-migration snapshot, handled by deep copy.

Mark all post-migration snapshots EXISTS_CLEAN in this case.

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

diff --git a/src/librbd/io/CopyupRequest.cc b/src/librbd/io/CopyupRequest.cc
index 8ce262720bdf563e2dad9022aabf06b7bce8c5bd..3ab44d0ec8599be139c9b4313fb45056e5951fa3 100644 (file)
--- a/src/librbd/io/CopyupRequest.cc
+++ b/src/librbd/io/CopyupRequest.cc
@@ -38,10 +38,12 @@ public:
   C_UpdateObjectMap(AsyncObjectThrottle<I> &throttle, I *image_ctx,
                     uint64_t object_no, uint8_t head_object_map_state,
                     const std::vector<uint64_t> *snap_ids,
-                    const ZTracer::Trace &trace, size_t snap_id_idx)
+                    bool first_snap_is_clean, const ZTracer::Trace &trace,
+                    size_t snap_id_idx)
     : C_AsyncObjectThrottle<I>(throttle, *image_ctx), m_object_no(object_no),
       m_head_object_map_state(head_object_map_state), m_snap_ids(*snap_ids),
-      m_trace(trace), m_snap_id_idx(snap_id_idx)
+      m_first_snap_is_clean(first_snap_is_clean), m_trace(trace),
+      m_snap_id_idx(snap_id_idx)
   {
   }
 
@@ -79,7 +81,7 @@ public:
     auto& image_ctx = this->m_image_ctx;
     uint8_t state = OBJECT_EXISTS;
     if (image_ctx.test_features(RBD_FEATURE_FAST_DIFF, image_ctx.snap_lock) &&
-        m_snap_id_idx > 0) {
+        (m_snap_id_idx > 0 || m_first_snap_is_clean)) {
       // first snapshot should be exists+dirty since it contains
       // the copyup data -- later snapshots inherit the data.
       state = OBJECT_EXISTS_CLEAN;
@@ -96,6 +98,7 @@ private:
   uint64_t m_object_no;
   uint8_t m_head_object_map_state;
   const std::vector<uint64_t> &m_snap_ids;
+  bool m_first_snap_is_clean;
   const ZTracer::Trace &m_trace;
   size_t m_snap_id_idx;
 };
@@ -335,7 +338,7 @@ void CopyupRequest<I>::update_object_maps() {
   typename AsyncObjectThrottle<I>::ContextFactory context_factory(
     boost::lambda::bind(boost::lambda::new_ptr<C_UpdateObjectMap<I>>(),
     boost::lambda::_1, m_image_ctx, m_object_no, head_object_map_state,
-    &m_snap_ids, m_trace, boost::lambda::_2));
+    &m_snap_ids, m_first_snap_is_clean, m_trace, boost::lambda::_2));
   auto ctx = util::create_context_callback<
     CopyupRequest<I>, &CopyupRequest<I>::handle_update_object_maps>(this);
   auto throttle = new AsyncObjectThrottle<I>(
@@ -591,6 +594,7 @@ void CopyupRequest<I>::compute_deep_copy_snap_ids() {
                std::back_inserter(m_snap_ids),
                [this, cct=m_image_ctx->cct, &deep_copied](uint64_t snap_id) {
       if (deep_copied.count(snap_id)) {
+        m_first_snap_is_clean = true;
         return false;
       }
 

diff --git a/src/librbd/io/CopyupRequest.h b/src/librbd/io/CopyupRequest.h
index e16b89bfd871b181174df87650586f5ede3c70ab..e4b3a2e7ffceabf1b122099456c667d2081cdd9d 100644 (file)
--- a/src/librbd/io/CopyupRequest.h
+++ b/src/librbd/io/CopyupRequest.h
@@ -93,6 +93,7 @@ private:
   AsyncOperation m_async_op;
 
   std::vector<uint64_t> m_snap_ids;
+  bool m_first_snap_is_clean = false;
 
   Mutex m_lock;
   WriteRequests m_pending_requests;

diff --git a/src/test/librbd/io/test_mock_CopyupRequest.cc b/src/test/librbd/io/test_mock_CopyupRequest.cc
index 53cdff8684e8cb8176babccab4d94a390fcec822..c6a28595ae87aa9e678d0acb4278fb7382320f94 100644 (file)
--- a/src/test/librbd/io/test_mock_CopyupRequest.cc
+++ b/src/test/librbd/io/test_mock_CopyupRequest.cc
@@ -588,7 +588,7 @@ TEST_F(TestMockIoCopyupRequest, DeepCopyOnRead) {
   flush_async_operations(ictx);
 }
 
-TEST_F(TestMockIoCopyupRequest, DeepCopyWithSnaps) {
+TEST_F(TestMockIoCopyupRequest, DeepCopyWithPostSnaps) {
   REQUIRE_FEATURE(RBD_FEATURE_LAYERING);
 
   librbd::ImageCtx *ictx;
@@ -621,6 +621,77 @@ TEST_F(TestMockIoCopyupRequest, DeepCopyWithSnaps) {
 
   InSequence seq;
 
+  MockAbstractObjectWriteRequest mock_write_request;
+  MockObjectCopyRequest mock_object_copy_request;
+  mock_image_ctx.migration_info = {1, "", "", "image id",
+                                   {{CEPH_NOSNAP, {2, 1}}},
+                                   ictx->size, true};
+  expect_is_empty_write_op(mock_write_request, false);
+  expect_object_copy(mock_image_ctx, mock_object_copy_request, true, 0);
+
+  expect_is_empty_write_op(mock_write_request, false);
+  expect_get_parent_overlap(mock_image_ctx, 1, 0, 0);
+  expect_get_parent_overlap(mock_image_ctx, 2, 1, 0);
+  expect_prune_parent_extents(mock_image_ctx, 1, 1);
+  expect_get_parent_overlap(mock_image_ctx, 3, 1, 0);
+  expect_prune_parent_extents(mock_image_ctx, 1, 1);
+  expect_get_pre_write_object_map_state(mock_image_ctx, mock_write_request,
+                                        OBJECT_EXISTS);
+  expect_object_map_at(mock_image_ctx, 0, OBJECT_NONEXISTENT);
+  expect_object_map_update(mock_image_ctx, 2, 0, OBJECT_EXISTS, true, 0);
+  expect_object_map_update(mock_image_ctx, 3, 0, OBJECT_EXISTS_CLEAN, true, 0);
+  expect_object_map_update(mock_image_ctx, CEPH_NOSNAP, 0, OBJECT_EXISTS, true,
+                           0);
+
+  expect_add_copyup_ops(mock_write_request);
+  expect_copyup(mock_image_ctx, CEPH_NOSNAP, "oid", "", 0);
+  expect_write(mock_image_ctx, CEPH_NOSNAP, "oid", 0);
+
+  auto req = new MockCopyupRequest(&mock_image_ctx, "oid", 0,
+                                   {{0, 4096}}, {});
+  mock_image_ctx.copyup_list[0] = req;
+  req->append_request(&mock_write_request);
+  req->send();
+
+  ASSERT_EQ(0, mock_write_request.ctx.wait());
+}
+
+TEST_F(TestMockIoCopyupRequest, DeepCopyWithPreAndPostSnaps) {
+  REQUIRE_FEATURE(RBD_FEATURE_LAYERING);
+
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+  ictx->snap_lock.get_write();
+  ictx->add_snap(cls::rbd::UserSnapshotNamespace(), "4", 4, ictx->size,
+                 ictx->parent_md, RBD_PROTECTION_STATUS_UNPROTECTED,
+                 0, {});
+  ictx->add_snap(cls::rbd::UserSnapshotNamespace(), "3", 3, ictx->size,
+                 ictx->parent_md, RBD_PROTECTION_STATUS_UNPROTECTED,
+                 0, {});
+  ictx->add_snap(cls::rbd::UserSnapshotNamespace(), "2", 2, ictx->size,
+                 ictx->parent_md, RBD_PROTECTION_STATUS_UNPROTECTED,
+                 0, {});
+  ictx->add_snap(cls::rbd::UserSnapshotNamespace(), "1", 1, ictx->size,
+                 ictx->parent_md, RBD_PROTECTION_STATUS_UNPROTECTED,
+                 0, {});
+  ictx->snapc = {4, {4, 3, 2, 1}};
+  ictx->snap_lock.put_write();
+
+  MockTestImageCtx mock_parent_image_ctx(*ictx->parent);
+  MockTestImageCtx mock_image_ctx(*ictx, &mock_parent_image_ctx);
+
+  MockExclusiveLock mock_exclusive_lock;
+  MockJournal mock_journal;
+  MockObjectMap mock_object_map;
+  initialize_features(ictx, mock_image_ctx, mock_exclusive_lock, mock_journal,
+                      mock_object_map);
+
+  expect_test_features(mock_image_ctx);
+  expect_op_work_queue(mock_image_ctx);
+  expect_is_lock_owner(mock_image_ctx);
+
+  InSequence seq;
+
   MockAbstractObjectWriteRequest mock_write_request;
   MockObjectCopyRequest mock_object_copy_request;
   mock_image_ctx.migration_info = {1, "", "", "image id",
@@ -633,10 +704,13 @@ TEST_F(TestMockIoCopyupRequest, DeepCopyWithSnaps) {
   expect_get_parent_overlap(mock_image_ctx, 2, 0, 0);
   expect_get_parent_overlap(mock_image_ctx, 3, 1, 0);
   expect_prune_parent_extents(mock_image_ctx, 1, 1);
+  expect_get_parent_overlap(mock_image_ctx, 4, 1, 0);
+  expect_prune_parent_extents(mock_image_ctx, 1, 1);
   expect_get_pre_write_object_map_state(mock_image_ctx, mock_write_request,
                                         OBJECT_EXISTS);
   expect_object_map_at(mock_image_ctx, 0, OBJECT_NONEXISTENT);
-  expect_object_map_update(mock_image_ctx, 3, 0, OBJECT_EXISTS, true, 0);
+  expect_object_map_update(mock_image_ctx, 3, 0, OBJECT_EXISTS_CLEAN, true, 0);
+  expect_object_map_update(mock_image_ctx, 4, 0, OBJECT_EXISTS_CLEAN, true, 0);
   expect_object_map_update(mock_image_ctx, CEPH_NOSNAP, 0, OBJECT_EXISTS, true,
                            0);