librbd/object_map: fix diff from snapshot when image is grown

Commit 399a45e11332 ("librbd/object_map: rbd diff between two
snapshots lists entire image content") fixed most of the damage caused
by commit b81cd2460de7 ("librbd/object_map: diff state machine should
track object existence"), but the case of a "resize diff" when diffing
from snapshot was missed.  An area that was freshly allocated in image
resize is the same in principle as a freshly created image and objects
marked OBJECT_EXISTS_CLEAN are no exception.  Diff for such objects in
such an area should be set to DIFF_STATE_DATA_UPDATED, however
currently when diffing from snapshot, it's set to DIFF_STATE_DATA.

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

diff --git a/src/librbd/object_map/DiffRequest.cc b/src/librbd/object_map/DiffRequest.cc
index 7fc3c900c14d449f65f616ff87dab16683dcbe74..08a25b026527c7dcb5db453051d1a2428bedbcaf 100644 (file)
--- a/src/librbd/object_map/DiffRequest.cc
+++ b/src/librbd/object_map/DiffRequest.cc
@@ -233,15 +233,12 @@ void DiffRequest<I>::handle_load_object_map(int r) {
   }
   ldout(cct, 20) << "computed overlap diffs" << dendl;
 
-  bool diff_from_start = (m_snap_id_start == 0);
   auto end_it = m_object_map.end();
   for (; it != end_it; ++it, ++diff_it, ++i) {
     uint8_t object_map_state = *it;
     if (object_map_state == OBJECT_NONEXISTENT) {
       *diff_it = DIFF_STATE_HOLE;
-    } else if (diff_from_start ||
-               (m_object_diff_state_valid &&
-                object_map_state != OBJECT_EXISTS_CLEAN)) {
+    } else if (m_current_snap_id != m_snap_id_start) {
       // diffing against the beginning of time or image was grown
       // (implicit) starting state is HOLE, this is the first object
       // map after
@@ -278,8 +275,6 @@ void DiffRequest<I>::handle_load_object_map(int r) {
   }
   ldout(cct, 20) << "computed resize diffs" << dendl;
 
-  m_object_diff_state_valid = true;
-
   std::shared_lock image_locker{m_image_ctx->image_lock};
   load_object_map(&image_locker);
 }

diff --git a/src/librbd/object_map/DiffRequest.h b/src/librbd/object_map/DiffRequest.h
index b02ef34ba347e34dd9fcf757bff2992d7bd3de27..f2fb9975f751024bb49aa7f8630b63de5d97fe6c 100644 (file)
--- a/src/librbd/object_map/DiffRequest.h
+++ b/src/librbd/object_map/DiffRequest.h
@@ -69,7 +69,6 @@ private:
   uint64_t m_current_size = 0;
 
   BitVector<2> m_object_map;
-  bool m_object_diff_state_valid = false;
 
   bufferlist m_out_bl;
 

diff --git a/src/test/librbd/object_map/test_mock_DiffRequest.cc b/src/test/librbd/object_map/test_mock_DiffRequest.cc
index fb52327e88cb97d02fe865e4d5b90ae1ec40406e..74dd2215ff389f2e278aa9ece1a7cd1b69d2ce24 100644 (file)
--- a/src/test/librbd/object_map/test_mock_DiffRequest.cc
+++ b/src/test/librbd/object_map/test_mock_DiffRequest.cc
@@ -354,6 +354,86 @@ TEST_P(TestMockObjectMapDiffRequest, FromBeginningToSnapIntermediateSnap) {
   }
 }
 
+TEST_P(TestMockObjectMapDiffRequest, FromBeginningToSnapIntermediateSnapGrow) {
+  REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF);
+
+  uint32_t object_count_1 = std::size(from_beginning_intermediate_table);
+  uint32_t object_count_2 = object_count_1 + std::size(from_beginning_table);
+  m_image_ctx->size = object_count_2 * (1 << m_image_ctx->order);
+  m_image_ctx->snap_info = {
+    {1U, {"snap1", {cls::rbd::UserSnapshotNamespace{}},
+          object_count_1 * (1 << m_image_ctx->order), {}, {}, {}, {}}},
+    {2U, {"snap2", {cls::rbd::UserSnapshotNamespace{}},
+          object_count_2 * (1 << m_image_ctx->order), {}, {}, {}, {}}}
+  };
+
+  BitVector<2> object_map_1;
+  object_map_1.resize(object_count_1);
+  BitVector<2> object_map_2;
+  object_map_2.resize(object_count_2);
+  BitVector<2> expected_diff_state;
+  expected_diff_state.resize(object_count_2);
+  for (uint32_t i = 0; i < object_count_1; i++) {
+    object_map_1[i] = from_beginning_intermediate_table[i][0];
+    object_map_2[i] = from_beginning_intermediate_table[i][1];
+    if (is_diff_iterate()) {
+      expected_diff_state[i] = from_beginning_intermediate_table[i][2];
+    } else {
+      expected_diff_state[i] = from_beginning_intermediate_table[i][3];
+    }
+  }
+  for (uint32_t i = object_count_1; i < object_count_2; i++) {
+    object_map_2[i] = from_beginning_table[i - object_count_1][0];
+    expected_diff_state[i] = from_beginning_table[i - object_count_1][1];
+  }
+
+  auto load = [&](MockTestImageCtx& mock_image_ctx) {
+    expect_get_flags(mock_image_ctx, 1, 0, 0);
+    expect_load_map(mock_image_ctx, 1, object_map_1, 0);
+    expect_get_flags(mock_image_ctx, 2, 0, 0);
+    expect_load_map(mock_image_ctx, 2, object_map_2, 0);
+  };
+  if (is_diff_iterate()) {
+    test_diff_iterate(load, 0, 2, expected_diff_state);
+  } else {
+    test_deep_copy(load, 0, 2, expected_diff_state);
+  }
+}
+
+TEST_P(TestMockObjectMapDiffRequest, FromBeginningToSnapIntermediateSnapGrowFromZero) {
+  REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF);
+
+  uint32_t object_count_2 = std::size(from_beginning_table);
+  m_image_ctx->size = object_count_2 * (1 << m_image_ctx->order);
+  m_image_ctx->snap_info = {
+    {1U, {"snap1", {cls::rbd::UserSnapshotNamespace{}}, {}, {}, {}, {}, {}}},
+    {2U, {"snap2", {cls::rbd::UserSnapshotNamespace{}},
+          object_count_2 * (1 << m_image_ctx->order), {}, {}, {}, {}}}
+  };
+
+  BitVector<2> object_map_1;
+  BitVector<2> object_map_2;
+  object_map_2.resize(object_count_2);
+  BitVector<2> expected_diff_state;
+  expected_diff_state.resize(object_count_2);
+  for (uint32_t i = 0; i < object_count_2; i++) {
+    object_map_2[i] = from_beginning_table[i][0];
+    expected_diff_state[i] = from_beginning_table[i][1];
+  }
+
+  auto load = [&](MockTestImageCtx& mock_image_ctx) {
+    expect_get_flags(mock_image_ctx, 1, 0, 0);
+    expect_load_map(mock_image_ctx, 1, object_map_1, 0);
+    expect_get_flags(mock_image_ctx, 2, 0, 0);
+    expect_load_map(mock_image_ctx, 2, object_map_2, 0);
+  };
+  if (is_diff_iterate()) {
+    test_diff_iterate(load, 0, 2, expected_diff_state);
+  } else {
+    test_deep_copy(load, 0, 2, expected_diff_state);
+  }
+}
+
 TEST_P(TestMockObjectMapDiffRequest, FromBeginningToHead) {
   REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF);
 
@@ -419,6 +499,82 @@ TEST_P(TestMockObjectMapDiffRequest, FromBeginningToHeadIntermediateSnap) {
   }
 }
 
+TEST_P(TestMockObjectMapDiffRequest, FromBeginningToHeadIntermediateSnapGrow) {
+  REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF);
+
+  uint32_t object_count_1 = std::size(from_beginning_intermediate_table);
+  uint32_t object_count_head = object_count_1 + std::size(from_beginning_table);
+  m_image_ctx->size = object_count_head * (1 << m_image_ctx->order);
+  m_image_ctx->snap_info = {
+    {1U, {"snap1", {cls::rbd::UserSnapshotNamespace{}},
+          object_count_1 * (1 << m_image_ctx->order), {}, {}, {}, {}}}
+  };
+
+  BitVector<2> object_map_1;
+  object_map_1.resize(object_count_1);
+  BitVector<2> object_map_head;
+  object_map_head.resize(object_count_head);
+  BitVector<2> expected_diff_state;
+  expected_diff_state.resize(object_count_head);
+  for (uint32_t i = 0; i < object_count_1; i++) {
+    object_map_1[i] = from_beginning_intermediate_table[i][0];
+    object_map_head[i] = from_beginning_intermediate_table[i][1];
+    if (is_diff_iterate()) {
+      expected_diff_state[i] = from_beginning_intermediate_table[i][2];
+    } else {
+      expected_diff_state[i] = from_beginning_intermediate_table[i][3];
+    }
+  }
+  for (uint32_t i = object_count_1; i < object_count_head; i++) {
+    object_map_head[i] = from_beginning_table[i - object_count_1][0];
+    expected_diff_state[i] = from_beginning_table[i - object_count_1][1];
+  }
+
+  auto load = [&](MockTestImageCtx& mock_image_ctx) {
+    expect_get_flags(mock_image_ctx, 1, 0, 0);
+    expect_load_map(mock_image_ctx, 1, object_map_1, 0);
+    expect_get_flags(mock_image_ctx, CEPH_NOSNAP, 0, 0);
+    expect_load_map(mock_image_ctx, CEPH_NOSNAP, object_map_head, 0);
+  };
+  if (is_diff_iterate()) {
+    test_diff_iterate(load, 0, CEPH_NOSNAP, expected_diff_state);
+  } else {
+    test_deep_copy(load, 0, CEPH_NOSNAP, expected_diff_state);
+  }
+}
+
+TEST_P(TestMockObjectMapDiffRequest, FromBeginningToHeadIntermediateSnapGrowFromZero) {
+  REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF);
+
+  uint32_t object_count_head = std::size(from_beginning_table);
+  m_image_ctx->size = object_count_head * (1 << m_image_ctx->order);
+  m_image_ctx->snap_info = {
+    {1U, {"snap1", {cls::rbd::UserSnapshotNamespace{}}, {}, {}, {}, {}, {}}}
+  };
+
+  BitVector<2> object_map_1;
+  BitVector<2> object_map_head;
+  object_map_head.resize(object_count_head);
+  BitVector<2> expected_diff_state;
+  expected_diff_state.resize(object_count_head);
+  for (uint32_t i = 0; i < object_count_head; i++) {
+    object_map_head[i] = from_beginning_table[i][0];
+    expected_diff_state[i] = from_beginning_table[i][1];
+  }
+
+  auto load = [&](MockTestImageCtx& mock_image_ctx) {
+    expect_get_flags(mock_image_ctx, 1, 0, 0);
+    expect_load_map(mock_image_ctx, 1, object_map_1, 0);
+    expect_get_flags(mock_image_ctx, CEPH_NOSNAP, 0, 0);
+    expect_load_map(mock_image_ctx, CEPH_NOSNAP, object_map_head, 0);
+  };
+  if (is_diff_iterate()) {
+    test_diff_iterate(load, 0, CEPH_NOSNAP, expected_diff_state);
+  } else {
+    test_deep_copy(load, 0, CEPH_NOSNAP, expected_diff_state);
+  }
+}
+
 TEST_P(TestMockObjectMapDiffRequest, FromSnapToSnap) {
   REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF);
 
@@ -456,6 +612,82 @@ TEST_P(TestMockObjectMapDiffRequest, FromSnapToSnap) {
   }
 }
 
+TEST_P(TestMockObjectMapDiffRequest, FromSnapToSnapGrow) {
+  REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF);
+
+  uint32_t object_count_1 = std::size(from_snap_table);
+  uint32_t object_count_2 = object_count_1 + std::size(from_beginning_table);
+  m_image_ctx->size = object_count_2 * (1 << m_image_ctx->order);
+  m_image_ctx->snap_info = {
+    {1U, {"snap1", {cls::rbd::UserSnapshotNamespace{}},
+          object_count_1 * (1 << m_image_ctx->order), {}, {}, {}, {}}},
+    {2U, {"snap2", {cls::rbd::UserSnapshotNamespace{}},
+          object_count_2 * (1 << m_image_ctx->order), {}, {}, {}, {}}}
+  };
+
+  BitVector<2> object_map_1;
+  object_map_1.resize(object_count_1);
+  BitVector<2> object_map_2;
+  object_map_2.resize(object_count_2);
+  BitVector<2> expected_diff_state;
+  expected_diff_state.resize(object_count_2);
+  for (uint32_t i = 0; i < object_count_1; i++) {
+    object_map_1[i] = from_snap_table[i][0];
+    object_map_2[i] = from_snap_table[i][1];
+    expected_diff_state[i] = from_snap_table[i][2];
+  }
+  for (uint32_t i = object_count_1; i < object_count_2; i++) {
+    object_map_2[i] = from_beginning_table[i - object_count_1][0];
+    expected_diff_state[i] = from_beginning_table[i - object_count_1][1];
+  }
+
+  auto load = [&](MockTestImageCtx& mock_image_ctx) {
+    expect_get_flags(mock_image_ctx, 1, 0, 0);
+    expect_load_map(mock_image_ctx, 1, object_map_1, 0);
+    expect_get_flags(mock_image_ctx, 2, 0, 0);
+    expect_load_map(mock_image_ctx, 2, object_map_2, 0);
+  };
+  if (is_diff_iterate()) {
+    test_diff_iterate(load, 1, 2, expected_diff_state);
+  } else {
+    test_deep_copy(load, 1, 2, expected_diff_state);
+  }
+}
+
+TEST_P(TestMockObjectMapDiffRequest, FromSnapToSnapGrowFromZero) {
+  REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF);
+
+  uint32_t object_count_2 = std::size(from_beginning_table);
+  m_image_ctx->size = object_count_2 * (1 << m_image_ctx->order);
+  m_image_ctx->snap_info = {
+    {1U, {"snap1", {cls::rbd::UserSnapshotNamespace{}}, {}, {}, {}, {}, {}}},
+    {2U, {"snap2", {cls::rbd::UserSnapshotNamespace{}},
+          object_count_2 * (1 << m_image_ctx->order), {}, {}, {}, {}}}
+  };
+
+  BitVector<2> object_map_1;
+  BitVector<2> object_map_2;
+  object_map_2.resize(object_count_2);
+  BitVector<2> expected_diff_state;
+  expected_diff_state.resize(object_count_2);
+  for (uint32_t i = 0; i < object_count_2; i++) {
+    object_map_2[i] = from_beginning_table[i][0];
+    expected_diff_state[i] = from_beginning_table[i][1];
+  }
+
+  auto load = [&](MockTestImageCtx& mock_image_ctx) {
+    expect_get_flags(mock_image_ctx, 1, 0, 0);
+    expect_load_map(mock_image_ctx, 1, object_map_1, 0);
+    expect_get_flags(mock_image_ctx, 2, 0, 0);
+    expect_load_map(mock_image_ctx, 2, object_map_2, 0);
+  };
+  if (is_diff_iterate()) {
+    test_diff_iterate(load, 1, 2, expected_diff_state);
+  } else {
+    test_deep_copy(load, 1, 2, expected_diff_state);
+  }
+}
+
 TEST_P(TestMockObjectMapDiffRequest, FromSnapToSnapIntermediateSnap) {
   REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF);
 
@@ -539,6 +771,78 @@ TEST_P(TestMockObjectMapDiffRequest, FromSnapToHead) {
   }
 }
 
+TEST_P(TestMockObjectMapDiffRequest, FromSnapToHeadGrow) {
+  REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF);
+
+  uint32_t object_count_1 = std::size(from_snap_table);
+  uint32_t object_count_head = object_count_1 + std::size(from_beginning_table);
+  m_image_ctx->size = object_count_head * (1 << m_image_ctx->order);
+  m_image_ctx->snap_info = {
+    {1U, {"snap1", {cls::rbd::UserSnapshotNamespace{}},
+          object_count_1 * (1 << m_image_ctx->order), {}, {}, {}, {}}}
+  };
+
+  BitVector<2> object_map_1;
+  object_map_1.resize(object_count_1);
+  BitVector<2> object_map_head;
+  object_map_head.resize(object_count_head);
+  BitVector<2> expected_diff_state;
+  expected_diff_state.resize(object_count_head);
+  for (uint32_t i = 0; i < object_count_1; i++) {
+    object_map_1[i] = from_snap_table[i][0];
+    object_map_head[i] = from_snap_table[i][1];
+    expected_diff_state[i] = from_snap_table[i][2];
+  }
+  for (uint32_t i = object_count_1; i < object_count_head; i++) {
+    object_map_head[i] = from_beginning_table[i - object_count_1][0];
+    expected_diff_state[i] = from_beginning_table[i - object_count_1][1];
+  }
+
+  auto load = [&](MockTestImageCtx& mock_image_ctx) {
+    expect_get_flags(mock_image_ctx, 1, 0, 0);
+    expect_load_map(mock_image_ctx, 1, object_map_1, 0);
+    expect_get_flags(mock_image_ctx, CEPH_NOSNAP, 0, 0);
+    expect_load_map(mock_image_ctx, CEPH_NOSNAP, object_map_head, 0);
+  };
+  if (is_diff_iterate()) {
+    test_diff_iterate(load, 1, CEPH_NOSNAP, expected_diff_state);
+  } else {
+    test_deep_copy(load, 1, CEPH_NOSNAP, expected_diff_state);
+  }
+}
+
+TEST_P(TestMockObjectMapDiffRequest, FromSnapToHeadGrowFromZero) {
+  REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF);
+
+  uint32_t object_count_head = std::size(from_beginning_table);
+  m_image_ctx->size = object_count_head * (1 << m_image_ctx->order);
+  m_image_ctx->snap_info = {
+    {1U, {"snap1", {cls::rbd::UserSnapshotNamespace{}}, {}, {}, {}, {}, {}}}
+  };
+
+  BitVector<2> object_map_1;
+  BitVector<2> object_map_head;
+  object_map_head.resize(object_count_head);
+  BitVector<2> expected_diff_state;
+  expected_diff_state.resize(object_count_head);
+  for (uint32_t i = 0; i < object_count_head; i++) {
+    object_map_head[i] = from_beginning_table[i][0];
+    expected_diff_state[i] = from_beginning_table[i][1];
+  }
+
+  auto load = [&](MockTestImageCtx& mock_image_ctx) {
+    expect_get_flags(mock_image_ctx, 1, 0, 0);
+    expect_load_map(mock_image_ctx, 1, object_map_1, 0);
+    expect_get_flags(mock_image_ctx, CEPH_NOSNAP, 0, 0);
+    expect_load_map(mock_image_ctx, CEPH_NOSNAP, object_map_head, 0);
+  };
+  if (is_diff_iterate()) {
+    test_diff_iterate(load, 1, CEPH_NOSNAP, expected_diff_state);
+  } else {
+    test_deep_copy(load, 1, CEPH_NOSNAP, expected_diff_state);
+  }
+}
+
 TEST_P(TestMockObjectMapDiffRequest, FromSnapToHeadIntermediateSnap) {
   REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF);