mon/OSDMonitor: take 'osd pool set ...' value as a string again

We ran into problems before when we made this a string because a mixed
cluster of mons might forward a client request with the wrong schema.
To make this work, we make the new code understand both the new and
old schema, and also backport a change to emperor and dumpling to
handle the new schema.

For the previous attempt to do this, see:
 337195f04653eed8e8f153a5b074f3bd48408998
 2fe0d0d97af95c22db80800f5b9da51f672d9407

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

diff --git a/qa/workunits/cephtool/test.sh b/qa/workunits/cephtool/test.sh
index 1a4d40228dd8ba600860a69a27a3f20b06deafca..94f37dd118d62fab34c74b0595236a21f8a4e334 100755 (executable)
--- a/qa/workunits/cephtool/test.sh
+++ b/qa/workunits/cephtool/test.sh
@@ -328,8 +328,12 @@ ceph osd pool set data size 3
 ceph osd pool get data size | grep 'size: 3'
 ceph osd pool set data size 2
 
-ceph osd pool set data hashpspool 1
+ceph osd pool set data hashpspool true
+ceph osd pool set data hashpspool false
 ceph osd pool set data hashpspool 0
+ceph osd pool set data hashpspool 1
+expect_false ceph osd pool set data hashpspool asdf
+expect_false ceph osd pool set data hashpspool 2
 
 ceph osd pool get rbd crush_ruleset | grep 'crush_ruleset: 2'
 

diff --git a/src/mon/MonCommands.h b/src/mon/MonCommands.h
index 73805c82e4fe6e1c35b895e58ab2f4b84b7c41f5..84bf54a42797821019f30e4532f52785446035cf 100644 (file)
--- a/src/mon/MonCommands.h
+++ b/src/mon/MonCommands.h
@@ -513,7 +513,7 @@ COMMAND("osd pool get " \
 COMMAND("osd pool set " \
        "name=pool,type=CephPoolname " \
        "name=var,type=CephChoices,strings=size|min_size|crash_replay_interval|pg_num|pgp_num|crush_ruleset|hashpspool " \
-       "name=val,type=CephInt", \
+       "name=val,type=CephString", \
        "set pool parameter <var> to <val>", "osd", "rw", "cli,rest")
 // 'val' is a CephString because it can include a unit.  Perhaps
 // there should be a Python type for validation/conversion of strings

diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index 72029d844157df1efb12b3d21a8b1996b7eaf725..377682aa8c08ecd964f496e4f86e9df122776a24 100644 (file)
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -2826,19 +2826,26 @@ int OSDMonitor::prepare_command_pool_set(map<string,cmd_vartype> &cmdmap,
   if (pending_inc.new_pools.count(pool))
     p = pending_inc.new_pools[pool];
 
-  // accept val as a json string or int, and parse out int
-  // values from the string as needed
-  string raw_val = cmd_vartype_stringify(cmdmap["val"]);
+  // accept val as a json string in the normal case (current
+  // generation monitor).  parse out int or float values from the
+  // string as needed.  however, if it is not a string, try to pull
+  // out an int, in case an older monitor with an older json schema is
+  // forwarding a request.
   string val;
-  cmd_getval(g_ceph_context, cmdmap, "val", val);
   string interr;
   int64_t n = 0;
-  if (!cmd_getval(g_ceph_context, cmdmap, "val", n))
+  if (!cmd_getval(g_ceph_context, cmdmap, "val", val)) {
+    // wasn't a string; maybe an older mon forwarded json with an int?
+    if (!cmd_getval(g_ceph_context, cmdmap, "val", n))
+      return -EINVAL;  // no value!
+  } else {
+    // we got a string.  see if it contains an int.
     n = strict_strtoll(val.c_str(), 10, &interr);
+  }
 
   if (var == "size") {
     if (interr.length()) {
-      ss << "error parsing integer value '" << raw_val << "': " << interr;
+      ss << "error parsing integer value '" << val << "': " << interr;
       return -EINVAL;
     }
     if (n == 0 || n > 10) {
@@ -2851,21 +2858,21 @@ int OSDMonitor::prepare_command_pool_set(map<string,cmd_vartype> &cmdmap,
     ss << "set pool " << pool << " size to " << n;
   } else if (var == "min_size") {
     if (interr.length()) {
-      ss << "error parsing integer value '" << raw_val << "': " << interr;
+      ss << "error parsing integer value '" << val << "': " << interr;
       return -EINVAL;
     }
     p.min_size = n;
     ss << "set pool " << pool << " min_size to " << n;
   } else if (var == "crash_replay_interval") {
     if (interr.length()) {
-      ss << "error parsing integer value '" << raw_val << "': " << interr;
+      ss << "error parsing integer value '" << val << "': " << interr;
       return -EINVAL;
     }
     p.crash_replay_interval = n;
     ss << "set pool " << pool << " to crash_replay_interval to " << n;
   } else if (var == "pg_num") {
     if (interr.length()) {
-      ss << "error parsing integer value '" << raw_val << "': " << interr;
+      ss << "error parsing integer value '" << val << "': " << interr;
       return -EINVAL;
     }
     if (n <= (int)p.get_pg_num()) {
@@ -2898,7 +2905,7 @@ int OSDMonitor::prepare_command_pool_set(map<string,cmd_vartype> &cmdmap,
     }
   } else if (var == "pgp_num") {
     if (interr.length()) {
-      ss << "error parsing integer value '" << raw_val << "': " << interr;
+      ss << "error parsing integer value '" << val << "': " << interr;
       return -EINVAL;
     }
     if (n <= 0) {
@@ -2919,7 +2926,7 @@ int OSDMonitor::prepare_command_pool_set(map<string,cmd_vartype> &cmdmap,
     }
   } else if (var == "crush_ruleset") {
     if (interr.length()) {
-      ss << "error parsing integer value '" << raw_val << "': " << interr;
+      ss << "error parsing integer value '" << val << "': " << interr;
       return -EINVAL;
     }
     if (osdmap.crush->rule_exists(n)) {
@@ -2931,15 +2938,14 @@ int OSDMonitor::prepare_command_pool_set(map<string,cmd_vartype> &cmdmap,
     }
   } else if (var == "hashpspool") {
     // make sure we only compare against 'n' if we didn't receive a string
-    int flag_val = (!val.empty() ? -1 : n);
-    if (val == "true" || flag_val == 1) {
+    if (val == "true" || (interr.empty() && n == 1)) {
       p.flags |= pg_pool_t::FLAG_HASHPSPOOL;
       ss << "set";
-    } else if (val == "false" || flag_val == 0) {
+    } else if (val == "false" || (interr.empty() && n == 0)) {
       p.flags ^= pg_pool_t::FLAG_HASHPSPOOL;
       ss << "unset";
     } else {
-      ss << "expecting value 0 or 1";
+      ss << "expecting value 'true', 'false', '0', or '1'";
       return -EINVAL;
     }
     ss << " pool " << pool << " flag hashpspool";