crush: fix name map encoding

We screwed up and encoded using the name 'int' type instead of int32_t.
That means people have systems encoding this as both 32 and 64 bit,
depending on their architecture.  This could be worse: x86_64 still has a
32-bit int (at least in my environment).

In any case, mixing both word sizes in their clusters is broken as a
result, with the exception of the kernel code, which doesn't decode this
part of the map and will tolerate differently-sized servers.

Fix this by:

 * encoding using int32_t now
 * decoding either 32-bit or 64-bit values, by assuming that the strings
   will always be non-empty.  This appears to be the case.

However:

 * any cluster with 64-bit ints must upgrade all at once, or else the new
   code will start encoding 32-bit values and the old code will be
   confused.

Signed-off-by: Sage Weil <sage@inktank.com>
Reviewed-by: Greg Farnum <greg@inktank.com>

diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc
index 341304a7b7bc7fb25359c411fdd4fa966124e852..c6aff197465717363c986812f44382c5fc35e0dc 100644 (file)
--- a/src/crush/CrushWrapper.cc
+++ b/src/crush/CrushWrapper.cc
@@ -473,6 +473,25 @@ void CrushWrapper::encode(bufferlist& bl, bool lean) const
   ::encode(crush->choose_total_tries, bl);
 }
 
+static void decode_32_or_64_string_map(map<int32_t,string>& m, bufferlist::iterator& blp)
+{
+  m.clear();
+  __u32 n;
+  ::decode(n, blp);
+  while (n--) {
+    __s32 key;
+    ::decode(key, blp);
+
+    __u32 strlen;
+    ::decode(strlen, blp);
+    if (strlen == 0) {
+      // der, key was actually 64-bits!
+      ::decode(strlen, blp);
+    }
+    ::decode_nohead(strlen, m[key], blp);
+  }
+}
+
 void CrushWrapper::decode(bufferlist::iterator& blp)
 {
   create();
@@ -513,9 +532,12 @@ void CrushWrapper::decode(bufferlist::iterator& blp)
     }
 
     // name info
-    ::decode(type_map, blp);
-    ::decode(name_map, blp);
-    ::decode(rule_name_map, blp);
+    // NOTE: we had a bug where we were incoding int instead of int32, which means the
+    // 'key' field for these maps may be either 32 or 64 bits, depending.  tolerate
+    // both by assuming the string is always non-empty.
+    decode_32_or_64_string_map(type_map, blp);
+    decode_32_or_64_string_map(name_map, blp);
+    decode_32_or_64_string_map(rule_name_map, blp);
     build_rmaps();
 
     // tunables

diff --git a/src/crush/CrushWrapper.h b/src/crush/CrushWrapper.h
index 6acb199323c051bfca4fa6b56b16eaa553d6d4af..09c3d525eb0d0237e124c0fef5259324301954b4 100644 (file)
--- a/src/crush/CrushWrapper.h
+++ b/src/crush/CrushWrapper.h
@@ -53,9 +53,9 @@ class CrushWrapper {
   mutable Mutex mapper_lock;
 public:
   struct crush_map *crush;
-  std::map<int, string> type_map; /* bucket/device type names */
-  std::map<int, string> name_map; /* bucket/device names */
-  std::map<int, string> rule_name_map;
+  std::map<int32_t, string> type_map; /* bucket/device type names */
+  std::map<int32_t, string> name_map; /* bucket/device names */
+  std::map<int32_t, string> rule_name_map;
 
   /* reverse maps */
   bool have_rmaps;