Skip to content

Commit

Permalink
osd: osdmap: Do not increase compatv when using stretch mode
Browse files Browse the repository at this point in the history
This is analagous to my monmap fix in 2e36436,
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 <[email protected]>
  • Loading branch information
gregsfortytwo committed Dec 10, 2020
1 parent 208957e commit 151f473
Showing 1 changed file with 2 additions and 6 deletions.
8 changes: 2 additions & 6 deletions src/osd/OSDMap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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)) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit 151f473

Please sign in to comment.