From f87e1116e2ea40a9e1fc9e5609ea6df010b29e38 Mon Sep 17 00:00:00 2001 From: Matan Breizman Date: Thu, 29 Feb 2024 14:46:31 +0000 Subject: [PATCH 1/2] mon/OSDMonitor: fix rmsnap command ``` There are 2 ways to remove pool snaps, rados tool or mon command (ceph osd pool rmsnap). It seems that the monitor command is not reporting the actual removal via new_removed_snaps which is later proceed in OSDMap::apply_incremental. This will result in a clone object leakage since the snap id won't be marked as purged (and won't be trimmed). ``` Fixes: https://tracker.ceph.com/issues/64646 Signed-off-by: Matan Breizman (cherry picked from commit f19ad233025957964559410e6697aa8a2a024bc1) --- src/mon/OSDMonitor.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc index 297d4b6c43501..740c037603cf1 100644 --- a/src/mon/OSDMonitor.cc +++ b/src/mon/OSDMonitor.cc @@ -12957,6 +12957,7 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op, if (sn) { pp->remove_snap(sn); pp->set_snap_epoch(pending_inc.epoch); + pending_inc.new_removed_snaps[pool].insert(sn); ss << "removed pool " << poolstr << " snap " << snapname; } else { ss << "already removed pool " << poolstr << " snap " << snapname; From cabc30eb9dad0025506eafe2b6622d1538985825 Mon Sep 17 00:00:00 2001 From: Matan Breizman Date: Sun, 3 Mar 2024 13:43:10 +0000 Subject: [PATCH 2/2] mon/OSDMonitor: unify remove_pool_snap callers No changes in behavior. Signed-off-by: Matan Breizman (cherry picked from commit 70c1d79d1d0f52cd65541f59cedb00e39cba3f76) --- src/mon/OSDMonitor.cc | 21 +++++++++++++-------- src/mon/OSDMonitor.h | 4 ++++ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc index 740c037603cf1..612a41c7d1bf7 100644 --- a/src/mon/OSDMonitor.cc +++ b/src/mon/OSDMonitor.cc @@ -4591,6 +4591,17 @@ void OSDMonitor::send_incremental(epoch_t first, } } +bool OSDMonitor::remove_pool_snap(std::string_view snapname, + pg_pool_t &pp, int64_t pool) { + snapid_t snapid = pp.snap_exists(snapname); + if (snapid) { + pp.remove_snap(snapid); + pending_inc.new_removed_snaps[pool].insert(snapid); + return true; + } + return false; +}; + int OSDMonitor::get_version(version_t ver, bufferlist& bl) { return get_version(ver, mon.get_quorum_con_features(), bl); @@ -12953,11 +12964,8 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op, pp = &pending_inc.new_pools[pool]; *pp = *p; } - snapid_t sn = pp->snap_exists(snapname.c_str()); - if (sn) { - pp->remove_snap(sn); + if (remove_pool_snap(snapname, *pp, pool)) { pp->set_snap_epoch(pending_inc.epoch); - pending_inc.new_removed_snaps[pool].insert(sn); ss << "removed pool " << poolstr << " snap " << snapname; } else { ss << "already removed pool " << poolstr << " snap " << snapname; @@ -14173,10 +14181,7 @@ bool OSDMonitor::prepare_pool_op(MonOpRequestRef op) case POOL_OP_DELETE_SNAP: { - snapid_t s = pp.snap_exists(m->name.c_str()); - if (s) { - pp.remove_snap(s); - pending_inc.new_removed_snaps[m->pool].insert(s); + if (remove_pool_snap(m->name, pp, m->pool)) { changed = true; } } diff --git a/src/mon/OSDMonitor.h b/src/mon/OSDMonitor.h index 992fb9deadc9e..6c065b5de1563 100644 --- a/src/mon/OSDMonitor.h +++ b/src/mon/OSDMonitor.h @@ -405,6 +405,10 @@ class OSDMonitor : public PaxosService, MOSDMap *build_incremental(epoch_t first, epoch_t last, uint64_t features); void send_full(MonOpRequestRef op); void send_incremental(MonOpRequestRef op, epoch_t first); + + bool remove_pool_snap(std::string_view snapname, + pg_pool_t &pp, int64_t pool); + public: /** * Make sure the existing (up) OSDs support the given features