From acd1ba8423c6bec116b087a591a027ca5ce4c9a4 Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Mon, 27 Nov 2023 11:59:26 +0100 Subject: [PATCH] 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 (cherry picked from commit 0a1f633e0240b4a7cfbcddd96d53fbf4b17f0b28) --- src/librbd/io/ObjectRequest.cc | 3 +- src/test/librbd/io/test_mock_ObjectRequest.cc | 36 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/librbd/io/ObjectRequest.cc b/src/librbd/io/ObjectRequest.cc index bae5883903464..68ae5e27e4d6e 100644 --- a/src/librbd/io/ObjectRequest.cc +++ b/src/librbd/io/ObjectRequest.cc @@ -837,6 +837,7 @@ void ObjectListSnapsRequest::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::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 885eac36f16c6..f1dd79e604f7d 100644 --- 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 -- 2.39.5