]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
osd: osdmap: Do not increase compatv when using stretch mode
authorGreg Farnum <gfarnum@redhat.com>
Tue, 8 Dec 2020 23:33:11 +0000 (23:33 +0000)
committerGreg Farnum <gfarnum@redhat.com>
Thu, 10 Dec 2020 14:35:15 +0000 (14:35 +0000)
This is analagous to my monmap fix in 2e3643647bfbe955b54c62c8aaf114744dedb86e,
but for the OSDMap. I was previously trying to be safe by changing compat_v
when using stretch details. Unfortunately, older clients are trying to
interpret this data (even though its purpose is to be a cluster-only data
region! See https://tracker.ceph.com/issues/48489), and crashing as a result.

Because enabling stretch mode is blocked by all OSDs understanding it, and
OSDs which do not understand it are not allowed to boot once it's on,
we can simply remove the compat_v change.

This does leave the miniscule risk that somebody will enable stretch mode
and run with it, then turn it off, an old OSD will *then* connect to the
cluster, and maybe some bad peering choices will happen -- but I don't
see a lot of other choices and this scenario is so unlikely I don't
think it's worth worrying about.

Signed-off-by: Greg Farnum <gfarnum@redhat.com>
src/osd/OSDMap.cc

index df33e819ff71f5a6dddb5f478558507ad3f9489c..6742352fb127874044e60f6dd0253d6421955f0c 100644 (file)
@@ -647,7 +647,6 @@ void OSDMap::Incremental::encode(ceph::buffer::list& bl, uint64_t features) cons
 
   {
     uint8_t target_v = 9; // if bumping this, be aware of stretch_mode target_v 10!
-    uint8_t new_compat_v = 0;
     if (!HAVE_FEATURE(features, SERVER_LUMINOUS)) {
       target_v = 2;
     } else if (!HAVE_FEATURE(features, SERVER_NAUTILUS)) {
@@ -656,7 +655,6 @@ void OSDMap::Incremental::encode(ceph::buffer::list& bl, uint64_t features) cons
     if (change_stretch_mode) {
       ceph_assert(target_v >= 9);
       target_v = std::max((uint8_t)10, target_v);
-      new_compat_v = std::max((uint8_t)10, std::max(new_compat_v, struct_compat));
     }
     ENCODE_START(target_v, 1, bl); // extended, osd-only data
     if (target_v < 7) {
@@ -707,7 +705,7 @@ void OSDMap::Incremental::encode(ceph::buffer::list& bl, uint64_t features) cons
       encode(new_stretch_mode_bucket, bl);
       encode(stretch_mode_enabled, bl);
     }
-    ENCODE_FINISH_NEW_COMPAT(bl, new_compat_v); // osd-only data
+    ENCODE_FINISH(bl); // osd-only data
   }
 
   crc_offset = bl.length();
@@ -3027,7 +3025,6 @@ void OSDMap::encode(ceph::buffer::list& bl, uint64_t features) const
     // NOTE: any new encoding dependencies must be reflected by
     // SIGNIFICANT_FEATURES
     uint8_t target_v = 9; // when bumping this, be aware of stretch_mode target_v 10!
-    uint8_t new_compat_v = 0;
     if (!HAVE_FEATURE(features, SERVER_LUMINOUS)) {
       target_v = 1;
     } else if (!HAVE_FEATURE(features, SERVER_MIMIC)) {
@@ -3038,7 +3035,6 @@ void OSDMap::encode(ceph::buffer::list& bl, uint64_t features) const
     if (stretch_mode_enabled) {
       ceph_assert(target_v >= 9);
       target_v = std::max((uint8_t)10, target_v);
-      new_compat_v = std::max((uint8_t)10, std::max(new_compat_v, struct_compat));
     }
     ENCODE_START(target_v, 1, bl); // extended, osd-only data
     if (target_v < 7) {
@@ -3095,7 +3091,7 @@ void OSDMap::encode(ceph::buffer::list& bl, uint64_t features) const
       encode(recovering_stretch_mode, bl);
       encode(stretch_mode_bucket, bl);
     }
-    ENCODE_FINISH_NEW_COMPAT(bl, new_compat_v); // osd-only data
+    ENCODE_FINISH(bl); // osd-only data
   }
 
   crc_offset = bl.length();