Skip to content

Commit 49d666a

Browse files
idryomovidryomov
authored
Merge pull request ceph#57973 from idryomov/wip-66418
librbd: diff-iterate shouldn't crash on an empty byte range Reviewed-by: Ramana Raja <[email protected]> Reviewed-by: Mykola Golub <[email protected]>
2 parents b5ed96c + 4f8aca2 commit 49d666a

File tree

2 files changed

+134
-15
lines changed

2 files changed

+134
-15
lines changed

‎src/librbd/api/DiffIterate.cc‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,7 @@ int DiffIterate<I>::diff_iterate(I *ictx,
266266

267267
template <typename I>
268268
std::pair<uint64_t, uint64_t> DiffIterate<I>::calc_object_diff_range() {
269+
ceph_assert(m_length > 0);
269270
uint64_t period = m_image_ctx.get_stripe_period();
270271
uint64_t first_period_off = round_down_to(m_offset, period);
271272
uint64_t last_period_off = round_down_to(m_offset + m_length - 1, period);
@@ -311,13 +312,13 @@ int DiffIterate<I>::execute() {
311312
if (from_snap_id == CEPH_NOSNAP) {
312313
return -ENOENT;
313314
}
314-
if (from_snap_id == end_snap_id) {
315+
if (from_snap_id > end_snap_id) {
316+
return -EINVAL;
317+
}
318+
if (from_snap_id == end_snap_id || m_length == 0) {
315319
// no diff.
316320
return 0;
317321
}
318-
if (from_snap_id >= end_snap_id) {
319-
return -EINVAL;
320-
}
321322

322323
int r;
323324
bool fast_diff_enabled = false;

‎src/test/librbd/test_librbd.cc‎

Lines changed: 129 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7545,6 +7545,10 @@ TYPED_TEST(DiffIterateTest, DiffIterateDeterministic)
75457545
vector_iterate_cb, &extents));
75467546
ASSERT_EQ(0u, extents.size());
75477547

7548+
ASSERT_EQ(-ENOENT, rbd_diff_iterate2(image, "snap1", 0, size, true,
7549+
this->whole_object, vector_iterate_cb,
7550+
&extents));
7551+
75487552
ASSERT_EQ(0, rbd_snap_create(image, "snap1"));
75497553

75507554
std::string buf(256, '1');
@@ -7621,31 +7625,61 @@ TYPED_TEST(DiffIterateTest, DiffIterateDeterministic)
76217625
ASSERT_EQ(diff_extent(1 << order, 256, true, object_size), extents[0]);
76227626
extents.clear();
76237627

7628+
// 8. snap3 -> snap3
7629+
ASSERT_EQ(0, rbd_diff_iterate2(image, "snap3", 0, size, true, this->whole_object,
7630+
vector_iterate_cb, &extents));
7631+
ASSERT_EQ(0u, extents.size());
7632+
76247633
ASSERT_PASSED(this->validate_object_map, image);
76257634
ASSERT_EQ(0, rbd_snap_set(image, "snap2"));
76267635

7627-
// 8. beginning of time -> snap2
7636+
// 9. beginning of time -> snap2
76287637
ASSERT_EQ(0, rbd_diff_iterate2(image, NULL, 0, size, true, this->whole_object,
76297638
vector_iterate_cb, &extents));
76307639
ASSERT_EQ(1u, extents.size());
76317640
ASSERT_EQ(diff_extent(0, 256, true, object_size), extents[0]);
76327641
extents.clear();
76337642

7634-
// 9. snap1 -> snap2
7643+
// 10. snap1 -> snap2
76357644
ASSERT_EQ(0, rbd_diff_iterate2(image, "snap1", 0, size, true, this->whole_object,
76367645
vector_iterate_cb, &extents));
76377646
ASSERT_EQ(1u, extents.size());
76387647
ASSERT_EQ(diff_extent(0, 256, true, object_size), extents[0]);
76397648
extents.clear();
76407649

7650+
// 11. snap2 -> snap2
7651+
ASSERT_EQ(0, rbd_diff_iterate2(image, "snap2", 0, size, true, this->whole_object,
7652+
vector_iterate_cb, &extents));
7653+
ASSERT_EQ(0u, extents.size());
7654+
7655+
// 12. snap3 -> snap2
7656+
ASSERT_EQ(-EINVAL, rbd_diff_iterate2(image, "snap3", 0, size, true,
7657+
this->whole_object, vector_iterate_cb,
7658+
&extents));
7659+
76417660
ASSERT_PASSED(this->validate_object_map, image);
76427661
ASSERT_EQ(0, rbd_snap_set(image, "snap1"));
76437662

7644-
// 10. beginning of time -> snap1
7663+
// 13. beginning of time -> snap1
76457664
ASSERT_EQ(0, rbd_diff_iterate2(image, NULL, 0, size, true, this->whole_object,
76467665
vector_iterate_cb, &extents));
76477666
ASSERT_EQ(0u, extents.size());
76487667

7668+
// 14. snap1 -> snap1
7669+
ASSERT_EQ(0, rbd_diff_iterate2(image, "snap1", 0, size, true, this->whole_object,
7670+
vector_iterate_cb, &extents));
7671+
ASSERT_EQ(0u, extents.size());
7672+
7673+
// 15. snap2 -> snap1
7674+
ASSERT_EQ(-EINVAL, rbd_diff_iterate2(image, "snap2", 0, size, true,
7675+
this->whole_object, vector_iterate_cb,
7676+
&extents));
7677+
7678+
// 16. snap3 -> snap1
7679+
ASSERT_EQ(-EINVAL, rbd_diff_iterate2(image, "snap3", 0, size, true,
7680+
this->whole_object, vector_iterate_cb,
7681+
&extents));
7682+
76497683
ASSERT_PASSED(this->validate_object_map, image);
76507684

76517685
ASSERT_EQ(0, rbd_close(image));
@@ -7678,6 +7712,10 @@ TYPED_TEST(DiffIterateTest, DiffIterateDeterministicPP)
76787712
vector_iterate_cb, &extents));
76797713
ASSERT_EQ(0u, extents.size());
76807714

7715+
ASSERT_EQ(-ENOENT, image.diff_iterate2("snap1", 0, size, true,
7716+
this->whole_object, vector_iterate_cb,
7717+
&extents));
7718+
76817719
ASSERT_EQ(0, image.snap_create("snap1"));
76827720

76837721
ceph::bufferlist bl;
@@ -7755,31 +7793,61 @@ TYPED_TEST(DiffIterateTest, DiffIterateDeterministicPP)
77557793
ASSERT_EQ(diff_extent(1 << order, 256, true, object_size), extents[0]);
77567794
extents.clear();
77577795

7796+
// 8. snap3 -> snap3
7797+
ASSERT_EQ(0, image.diff_iterate2("snap3", 0, size, true, this->whole_object,
7798+
vector_iterate_cb, &extents));
7799+
ASSERT_EQ(0u, extents.size());
7800+
77587801
ASSERT_PASSED(this->validate_object_map, image);
77597802
ASSERT_EQ(0, image.snap_set("snap2"));
77607803

7761-
// 8. beginning of time -> snap2
7804+
// 9. beginning of time -> snap2
77627805
ASSERT_EQ(0, image.diff_iterate2(NULL, 0, size, true, this->whole_object,
77637806
vector_iterate_cb, &extents));
77647807
ASSERT_EQ(1u, extents.size());
77657808
ASSERT_EQ(diff_extent(0, 256, true, object_size), extents[0]);
77667809
extents.clear();
77677810

7768-
// 9. snap1 -> snap2
7811+
// 10. snap1 -> snap2
77697812
ASSERT_EQ(0, image.diff_iterate2("snap1", 0, size, true, this->whole_object,
77707813
vector_iterate_cb, &extents));
77717814
ASSERT_EQ(1u, extents.size());
77727815
ASSERT_EQ(diff_extent(0, 256, true, object_size), extents[0]);
77737816
extents.clear();
77747817

7818+
// 11. snap2 -> snap2
7819+
ASSERT_EQ(0, image.diff_iterate2("snap2", 0, size, true, this->whole_object,
7820+
vector_iterate_cb, &extents));
7821+
ASSERT_EQ(0u, extents.size());
7822+
7823+
// 12. snap3 -> snap2
7824+
ASSERT_EQ(-EINVAL, image.diff_iterate2("snap3", 0, size, true,
7825+
this->whole_object, vector_iterate_cb,
7826+
&extents));
7827+
77757828
ASSERT_PASSED(this->validate_object_map, image);
77767829
ASSERT_EQ(0, image.snap_set("snap1"));
77777830

7778-
// 10. beginning of time -> snap1
7831+
// 13. beginning of time -> snap1
77797832
ASSERT_EQ(0, image.diff_iterate2(NULL, 0, size, true, this->whole_object,
77807833
vector_iterate_cb, &extents));
77817834
ASSERT_EQ(0u, extents.size());
77827835

7836+
// 14. snap1 -> snap1
7837+
ASSERT_EQ(0, image.diff_iterate2("snap1", 0, size, true, this->whole_object,
7838+
vector_iterate_cb, &extents));
7839+
ASSERT_EQ(0u, extents.size());
7840+
7841+
// 15. snap2 -> snap1
7842+
ASSERT_EQ(-EINVAL, image.diff_iterate2("snap2", 0, size, true,
7843+
this->whole_object, vector_iterate_cb,
7844+
&extents));
7845+
7846+
// 16. snap3 -> snap1
7847+
ASSERT_EQ(-EINVAL, image.diff_iterate2("snap3", 0, size, true,
7848+
this->whole_object, vector_iterate_cb,
7849+
&extents));
7850+
77837851
ASSERT_PASSED(this->validate_object_map, image);
77847852
}
77857853

@@ -8529,23 +8597,59 @@ TYPED_TEST(DiffIterateTest, DiffIterateUnalignedSmall)
85298597
{
85308598
librbd::RBD rbd;
85318599
librbd::Image image;
8532-
int order = 0;
8600+
int order = 22;
85338601
std::string name = this->get_temp_image_name();
8534-
ssize_t size = 10 << 20;
8602+
ssize_t data_end = 8 << 20;
85358603

8536-
ASSERT_EQ(0, create_image_pp(rbd, ioctx, name.c_str(), size, &order));
8604+
ASSERT_EQ(0, create_image_pp(rbd, ioctx, name.c_str(),
8605+
data_end + (2 << 20), &order));
85378606
ASSERT_EQ(0, rbd.open(ioctx, image, name.c_str(), NULL));
85388607

85398608
ceph::bufferlist bl;
8540-
bl.append(std::string(size, '1'));
8541-
ASSERT_EQ(size, image.write(0, size, bl));
8609+
bl.append(std::string(data_end, '1'));
8610+
ASSERT_EQ(data_end, image.write(0, data_end, bl));
85428611

85438612
std::vector<diff_extent> extents;
8613+
ASSERT_EQ(0, image.diff_iterate2(NULL, 0, 0, true,
8614+
this->whole_object, vector_iterate_cb,
8615+
&extents));
8616+
ASSERT_EQ(0u, extents.size());
8617+
85448618
ASSERT_EQ(0, image.diff_iterate2(NULL, 5000005, 1234, true,
85458619
this->whole_object, vector_iterate_cb,
85468620
&extents));
85478621
ASSERT_EQ(1u, extents.size());
85488622
ASSERT_EQ(diff_extent(5000005, 1234, true, 0), extents[0]);
8623+
extents.clear();
8624+
8625+
ASSERT_EQ(0, image.diff_iterate2(NULL, data_end - 1, 0, true,
8626+
this->whole_object, vector_iterate_cb,
8627+
&extents));
8628+
ASSERT_EQ(0u, extents.size());
8629+
8630+
ASSERT_EQ(0, image.diff_iterate2(NULL, data_end - 1, 1, true,
8631+
this->whole_object, vector_iterate_cb,
8632+
&extents));
8633+
ASSERT_EQ(1u, extents.size());
8634+
ASSERT_EQ(diff_extent(data_end - 1, 1, true, 0), extents[0]);
8635+
extents.clear();
8636+
8637+
ASSERT_EQ(0, image.diff_iterate2(NULL, data_end - 1, 2, true,
8638+
this->whole_object, vector_iterate_cb,
8639+
&extents));
8640+
ASSERT_EQ(1u, extents.size());
8641+
ASSERT_EQ(diff_extent(data_end - 1, 1, true, 0), extents[0]);
8642+
extents.clear();
8643+
8644+
ASSERT_EQ(0, image.diff_iterate2(NULL, data_end, 0, true,
8645+
this->whole_object, vector_iterate_cb,
8646+
&extents));
8647+
ASSERT_EQ(0u, extents.size());
8648+
8649+
ASSERT_EQ(0, image.diff_iterate2(NULL, data_end, 1, true,
8650+
this->whole_object, vector_iterate_cb,
8651+
&extents));
8652+
ASSERT_EQ(0u, extents.size());
85498653

85508654
ASSERT_PASSED(this->validate_object_map, image);
85518655
}
@@ -8580,6 +8684,20 @@ TYPED_TEST(DiffIterateTest, DiffIterateUnaligned)
85808684
ASSERT_EQ(diff_extent(8376263, 12345, true, 0), extents[0]);
85818685
ASSERT_EQ(diff_extent(8388608, 4194304, true, 0), extents[1]);
85828686
ASSERT_EQ(diff_extent(12582912, 54321, true, 0), extents[2]);
8687+
extents.clear();
8688+
8689+
// length is clipped up to end
8690+
ASSERT_EQ(0, image.diff_iterate2(NULL, size - 1, size, true,
8691+
this->whole_object, vector_iterate_cb,
8692+
&extents));
8693+
ASSERT_EQ(1u, extents.size());
8694+
ASSERT_EQ(diff_extent(size - 1, 1, true, 0), extents[0]);
8695+
extents.clear();
8696+
8697+
// offset past end
8698+
ASSERT_EQ(-EINVAL, image.diff_iterate2(NULL, size, size, true,
8699+
this->whole_object,
8700+
vector_iterate_cb, &extents));
85838701

85848702
ASSERT_PASSED(this->validate_object_map, image);
85858703
}

0 commit comments

Comments
 (0)