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 https://tracker.ceph.com/issues/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: https://tracker.ceph.com/issues/18746
Signed-off-by: Paul Emmerich <paul.emmerich@croit.io>
(cherry picked from commit f42a6ba4c37cb10869718ec296352ae55254b44a)

Conflicts:
	src/osd/osd_types.cc: trivial resolution

diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index fec7b0e93b958b256b61cc88e336881183da69fa..3c08a2f2b82d64e9602c4905fc1fb2f2bea205b0 100644 (file)
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -8143,6 +8143,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 1c07d31c581699bc90aa53bb1687b2a8ba54a8e0..3f212f3e9211af817443028f406dd8c662de0c79 100644 (file)
--- a/src/osd/osd_types.cc
+++ b/src/osd/osd_types.cc
@@ -1317,7 +1317,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