librbd: fix read_whole_object handling in ObjectListSnapsRequest

Originally, in commit 2be4840afd4f ("librados/snap_set_diff: don't
assert on empty snapset"), exists was set to true.  This didn't make
ObjectListSnapsRequest, causing the following deep-copy tests to fail
when run against calc_snap_set_diff() rigged to return "whole object"
as described in [1]:

    TestDeepCopy.Snaps
    TestDeepCopy.SnapDiscard
    TestDeepCopy.CloneHideParent
    TestDeepCopy.Snaps_LargerDstObjSize
    TestDeepCopy.Snaps_SmallerDstObjSize

This is a regression introduced in commit cc87a8bd697e ("librbd:
deep-copy object utilizes image-extent IO methods") by way of commit
11923e234efc ("librbd: generic object list snapshot request").

[1] https://github.com/ceph/ceph/pull/20648#issuecomment-369292309

Fixes: https://tracker.ceph.com/issues/63654
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
(cherry picked from commit 0a1f633e0240b4a7cfbcddd96d53fbf4b17f0b28)

diff --git a/src/librbd/io/ObjectRequest.cc b/src/librbd/io/ObjectRequest.cc
index bae58839034641d6089d9a399bc5a3dadd60fb34..68ae5e27e4d6e022233a2781cacf5289a65ba974 100644 (file)
--- a/src/librbd/io/ObjectRequest.cc
+++ b/src/librbd/io/ObjectRequest.cc
@@ -837,6 +837,7 @@ void ObjectListSnapsRequest<I>::handle_list_snaps(int r) {
     if (read_whole_object) {
       ldout(cct, 1) << "need to read full object" << dendl;
       diff.insert(0, image_ctx->layout.object_size);
+      exists = true;
       end_size = image_ctx->layout.object_size;
       clone_end_snap_id = end_snap_id;
     } else if ((m_list_snaps_flags & LIST_SNAPS_FLAG_WHOLE_OBJECT) != 0 &&
@@ -884,7 +885,7 @@ void ObjectListSnapsRequest<I>::handle_list_snaps(int r) {
                    << "end_size=" << end_size << ", "
                    << "prev_end_size=" << prev_end_size << ", "
                    << "exists=" << exists << ", "
-                   << "whole_object=" << read_whole_object << dendl;
+                   << "read_whole_object=" << read_whole_object << dendl;
 
     // check if object exists prior to start of incremental snap delta so that
     // we don't DNE the object if no additional deltas exist

diff --git a/src/test/librbd/io/test_mock_ObjectRequest.cc b/src/test/librbd/io/test_mock_ObjectRequest.cc
index 885eac36f16c6f2b098215bbeaef736b1f593baa..f1dd79e604f7d49371c323c9e89a051f8569a077 100644 (file)
--- a/src/test/librbd/io/test_mock_ObjectRequest.cc
+++ b/src/test/librbd/io/test_mock_ObjectRequest.cc
@@ -2027,6 +2027,42 @@ TEST_F(TestMockIoObjectRequest, ListSnapsWholeObjectEndSize) {
   }
 }
 
+TEST_F(TestMockIoObjectRequest, ListSnapsNoSnapsInSnapSet) {
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  MockTestImageCtx mock_image_ctx(*ictx);
+  mock_image_ctx.snaps = {3};
+
+  InSequence seq;
+
+  librados::snap_set_t snap_set;
+  snap_set.seq = 3;
+  librados::clone_info_t clone_info;
+
+  clone_info.cloneid = 3;
+  clone_info.snaps = {};
+  clone_info.overlap = {};
+  clone_info.size = 0;
+  snap_set.clones.push_back(clone_info);
+
+  expect_list_snaps(mock_image_ctx, snap_set, 0);
+
+  SnapshotDelta snapshot_delta;
+  C_SaferCond ctx;
+  auto req = MockObjectListSnapsRequest::create(
+    &mock_image_ctx, 0, {{0, mock_image_ctx.layout.object_size - 1}},
+    {0, CEPH_NOSNAP}, 0, {}, &snapshot_delta, &ctx);
+  req->send();
+  ASSERT_EQ(0, ctx.wait());
+
+  SnapshotDelta expected_snapshot_delta;
+  expected_snapshot_delta[{CEPH_NOSNAP,CEPH_NOSNAP}].insert(
+    0, mock_image_ctx.layout.object_size - 1,
+    {SPARSE_EXTENT_STATE_DATA, mock_image_ctx.layout.object_size - 1});
+  EXPECT_EQ(expected_snapshot_delta, snapshot_delta);
+}
+
 } // namespace io
 } // namespace librbd