From 0cd36e0587a3738ef90ab0e892e72395ed7dce18 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Tue, 3 Dec 2013 21:39:03 -0800 Subject: [PATCH] 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 Reviewed-by: Greg Farnum --- qa/workunits/cephtool/test.sh | 6 +++++- src/mon/MonCommands.h | 2 +- src/mon/OSDMonitor.cc | 36 ++++++++++++++++++++--------------- 3 files changed, 27 insertions(+), 17 deletions(-) diff --git a/qa/workunits/cephtool/test.sh b/qa/workunits/cephtool/test.sh index 1a4d40228dd8b..94f37dd118d62 100755 --- 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 73805c82e4fe6..84bf54a427978 100644 --- 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 to ", "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 72029d844157d..377682aa8c08e 100644 --- a/src/mon/OSDMonitor.cc +++ b/src/mon/OSDMonitor.cc @@ -2826,19 +2826,26 @@ int OSDMonitor::prepare_command_pool_set(map &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 &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 &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 &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 &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"; -- 2.39.5