From 3596ca077097a4e0ff8e8d05a410c2044332391e Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Tue, 24 Feb 2026 12:46:35 +0100 Subject: [PATCH] librbd/mirror: detect trashed snapshots in UnlinkPeerRequest If two instances of UnlinkPeerRequest race with each other (e.g. due to rbd-mirror daemon unlinking from a previous mirror snapshot and the user taking another mirror snapshot at same time), the snapshot that UnlinkPeerRequest was created for may be in the process of being removed (which may mean trashed by SnapshotRemoveRequest::trash_snap()) or fully removed by the time unlink_peer() grabs the image lock. Because trashed snapshots weren't handled explicitly, UnlinkPeerRequest could spuriously fail with EINVAL ("not mirror snapshot" case) instead of the expected ENOENT ("missing snapshot" case). This in turn could lead to spurious ImageReplayer failures with it stopping prematurely. Fixes: https://tracker.ceph.com/issues/68279 Signed-off-by: Ilya Dryomov --- .../mirror/snapshot/UnlinkPeerRequest.cc | 9 ++++--- .../snapshot/test_mock_UnlinkPeerRequest.cc | 25 +++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/librbd/mirror/snapshot/UnlinkPeerRequest.cc b/src/librbd/mirror/snapshot/UnlinkPeerRequest.cc index e89a4387541..2063cd0ba31 100644 --- a/src/librbd/mirror/snapshot/UnlinkPeerRequest.cc +++ b/src/librbd/mirror/snapshot/UnlinkPeerRequest.cc @@ -79,10 +79,13 @@ void UnlinkPeerRequest::unlink_peer() { } } - if (r == -ENOENT) { - ldout(cct, 15) << "missing snapshot: snap_id=" << m_snap_id << dendl; + if (r == -ENOENT || + std::holds_alternative( + snap_namespace)) { + ldout(cct, 15) << "missing or trashed snapshot: snap_id=" << m_snap_id + << dendl; m_image_ctx->image_lock.unlock_shared(); - finish(r); + finish(-ENOENT); return; } diff --git a/src/test/librbd/mirror/snapshot/test_mock_UnlinkPeerRequest.cc b/src/test/librbd/mirror/snapshot/test_mock_UnlinkPeerRequest.cc index 96e1cb9ee8d..4626580c463 100644 --- a/src/test/librbd/mirror/snapshot/test_mock_UnlinkPeerRequest.cc +++ b/src/test/librbd/mirror/snapshot/test_mock_UnlinkPeerRequest.cc @@ -291,6 +291,31 @@ TEST_F(TestMockMirrorSnapshotUnlinkPeerRequest, SnapshotDNE) { ASSERT_EQ(-ENOENT, ctx.wait()); } +TEST_F(TestMockMirrorSnapshotUnlinkPeerRequest, TrashedSnapshot) { + REQUIRE_FORMAT_V2(); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockTestImageCtx mock_image_ctx(*ictx); + cls::rbd::TrashSnapshotNamespace ns{ + cls::rbd::SNAPSHOT_NAMESPACE_TYPE_MIRROR, "mirror_snap"}; + auto snap_id = snap_create(mock_image_ctx, ns, "trash_snap"); + + expect_get_snap_info(mock_image_ctx, snap_id); + + InSequence seq; + + expect_is_refresh_required(mock_image_ctx, true); + expect_refresh_image(mock_image_ctx, 0); + + C_SaferCond ctx; + auto req = new MockUnlinkPeerRequest(&mock_image_ctx, snap_id, "peer_uuid", + true, &ctx); + req->send(); + ASSERT_EQ(-ENOENT, ctx.wait()); +} + TEST_F(TestMockMirrorSnapshotUnlinkPeerRequest, PeerDNE) { REQUIRE_FORMAT_V2(); -- 2.47.3