Skip to content

Commit

Permalink
librbd/object_map: fix diff from snapshot when image is grown
Browse files Browse the repository at this point in the history 
Commit 399a45e ("librbd/object_map: rbd diff between two
snapshots lists entire image content") fixed most of the damage caused
by commit b81cd24 ("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>
  • Loading branch information
@idryomov
idryomov committed Jan 20, 2024
1 parent 4e036d6 commit 34386d2
Show file tree
Hide file tree
Showing 3 changed files with 305 additions and 7 deletions.
7 changes: 1 addition & 6 deletions src/librbd/object_map/DiffRequest.cc
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Expand Down
1 change: 0 additions & 1 deletion src/librbd/object_map/DiffRequest.h
View file
Expand Up @@ -69,7 +69,6 @@ class DiffRequest {
uint64_t m_current_size = 0;

BitVector<2> m_object_map;
bool m_object_diff_state_valid = false;

bufferlist m_out_bl;

Expand Down
304 changes: 304 additions & 0 deletions src/test/librbd/object_map/test_mock_DiffRequest.cc
View file
Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down

0 comments on commit 34386d2

Please sign in to comment.