mon: handle bad snapshot removal reqs gracefully

Snapshot deletion requests on snap ids larger than the snap_seq of
the pool will leave the pool in a state with snap_seq being less
than max(removed_snaps).

This is bad because further deletion requests to a pool in this state
might crash the mon in some cases: the deletion also inserts the new
snap_seq into the removed_snaps set -- which might already exist
in this case and trigger an assert.

Such bad requests will be generated by rbd clients without a fix for
issue #21567.

The change in OSDMonitor prevents pools from getting into this state
and may prevent old broken clients from incorrectly deleting snaps.
The change in osd_types avoids a crash if a pool is already in this
state.

Fixes #18746

Signed-off-by: Paul Emmerich <paul.emmerich@croit.io>
(cherry picked from commit f42a6ba4c37cb10869718ec296352ae55254b44a)

 Conflicts:
	src/osd/osd_types.cc

diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index 788cd07a41ca65d5aabd68e6fcb6997bb1ae2ea6..c410d21a2859d39bae22449fc116f85e6321bbdd 100644 (file)
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -11217,6 +11217,10 @@ bool OSDMonitor::prepare_pool_op(MonOpRequestRef op)
 
   case POOL_OP_DELETE_UNMANAGED_SNAP:
     if (!pp.is_removed_snap(m->snapid)) {
+      if (m->snapid > pp.get_snap_seq()) {
+        _pool_op_reply(op, -ENOENT, osdmap.get_epoch());
+        return false;
+      }
       pp.remove_unmanaged_snap(m->snapid);
       changed = true;
     }

diff --git a/src/osd/osd_types.cc b/src/osd/osd_types.cc
index 393cd7097d23fdb9483949fd2571e506fc31f629..4403985165d708be12f1aab5281dbd67247b1ce2 100644 (file)
--- a/src/osd/osd_types.cc
+++ b/src/osd/osd_types.cc
@@ -1347,7 +1347,10 @@ void pg_pool_t::remove_unmanaged_snap(snapid_t s)
   assert(is_unmanaged_snaps_mode());
   removed_snaps.insert(s);
   snap_seq = snap_seq + 1;
-  removed_snaps.insert(get_snap_seq());
+  // try to add in the new seq, just to try to keep the interval_set contiguous
+  if (!removed_snaps.contains(get_snap_seq())) {
+    removed_snaps.insert(get_snap_seq());
+  }
 }
 
 SnapContext pg_pool_t::get_snap_context() const