From: Greg Farnum <gfarnum@redhat.com>
Date: Tue, 21 Jul 2020 17:55:57 +0000 (+0000)
Subject: osd: only encode stretch mode variables in the OSDMap when in use
X-Git-Tag: v16.1.0~1053^2~5
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=f42b8ce33c1eba5b3a9170ee1880ea583e684065;p=ceph.git

osd: only encode stretch mode variables in the OSDMap when in use

We were previously encoding them unconditionally, but that led to
OSDMap crc mismatches between old and new daemons, which was bad.

Instead, we only set target_v to encode stretch mode when it's in use,
and we assert safety upon doing so.

Signed-off-by: Greg Farnum <gfarnum@redhat.com>
---

diff --git a/src/osd/OSDMap.cc b/src/osd/OSDMap.cc
index 5aa14d2b0c80..73302d6fb142 100644
--- a/src/osd/OSDMap.cc
+++ b/src/osd/OSDMap.cc
@@ -646,13 +646,18 @@ void OSDMap::Incremental::encode(ceph::buffer::list& bl, uint64_t features) cons
   }
 
   {
-    uint8_t target_v = 10;
+    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)) {
       target_v = 6;
     }
+    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) {
       encode_addrvec_map_as_addr(new_hb_back_up, bl, features);
@@ -702,10 +707,6 @@ void OSDMap::Incremental::encode(ceph::buffer::list& bl, uint64_t features) cons
       encode(new_stretch_mode_bucket, bl);
       encode(stretch_mode_enabled, bl);
     }
-    if (change_stretch_mode) {
-      ceph_assert(target_v >= 10);
-      new_compat_v = 10;
-    }
     ENCODE_FINISH_NEW_COMPAT(bl, new_compat_v); // osd-only data
   }
 
@@ -3030,7 +3031,7 @@ 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 = 10;
+    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;
@@ -3039,6 +3040,11 @@ void OSDMap::encode(ceph::buffer::list& bl, uint64_t features) const
     } else if (!HAVE_FEATURE(features, SERVER_NAUTILUS)) {
       target_v = 6;
     }
+    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) {
       encode_addrvec_pvec_as_addr(osd_addrs->hb_back_addrs, bl, features);
@@ -3087,10 +3093,6 @@ void OSDMap::encode(ceph::buffer::list& bl, uint64_t features) const
     if (target_v >= 9) {
       encode(device_class_flags, bl);
     }
-    if (stretch_mode_enabled) {
-      ceph_assert(target_v >= 10);
-      new_compat_v = 10;
-    }
     if (target_v >= 10) {
       encode(stretch_mode_enabled, bl);
       encode(stretch_bucket_count, bl);