From 151f473ee5242de1db4d5b88217b04c90234af79 Mon Sep 17 00:00:00 2001 From: Greg Farnum Date: Tue, 8 Dec 2020 23:33:11 +0000 Subject: [PATCH] osd: osdmap: Do not increase compatv when using stretch mode 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 --- src/osd/OSDMap.cc | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/osd/OSDMap.cc b/src/osd/OSDMap.cc index df33e819ff71f..6742352fb1278 100644 --- a/src/osd/OSDMap.cc +++ b/src/osd/OSDMap.cc @@ -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();