From: Kefu Chai <kchai@redhat.com>
Date: Mon, 25 May 2015 12:14:32 +0000 (+0800)
Subject: mon: validate new crush for unknown names
X-Git-Tag: v0.94.3~51^2~5
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=cc1cc033930e8690a57674e842a003f6bbc7a242;p=ceph.git

mon: validate new crush for unknown names

* the "osd tree dump" command enumerates all buckets/osds found in either the
  crush map or the osd map. but the newly set crushmap is not validated for
  the dangling references, so we need to check to see if any item in new crush
  map is referencing unknown type/name when a new crush map is sent to
  monitor, reject it if any.

Fixes: #11680
Signed-off-by: Kefu Chai <kchai@redhat.com>
(cherry picked from commit a955f36a509e5412b1f72632a1a956d99e768e35)
---

diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index 58a70ef2b199..c8a2e6b17597 100644
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -4525,7 +4525,16 @@ bool OSDMonitor::prepare_command_impl(MMonCommand *m,
     // sanity check: test some inputs to make sure this map isn't totally broken
     dout(10) << " testing map" << dendl;
     stringstream ess;
+    // XXX: Use mon_lease as a timeout value for crushtool.
+    // If the crushtool consistently takes longer than 'mon_lease' seconds,
+    // then we would consistently trigger an election before the command
+    // finishes, having a flapping monitor unable to hold quorum.
     CrushTester tester(crush, ess);
+    if (!tester.check_name_maps()) {
+      err = -EINVAL;
+      ss << ess.str();
+      goto reply;
+    }
     int r = tester.test_with_crushtool(g_conf->crushtool,
 				       g_conf->mon_lease);
     if (r < 0) {
diff --git a/src/test/mon/osd-crush.sh b/src/test/mon/osd-crush.sh
index 4b1c601d41aa..c0857c4e99f4 100755
--- a/src/test/mon/osd-crush.sh
+++ b/src/test/mon/osd-crush.sh
@@ -188,6 +188,19 @@ function TEST_crush_rename_bucket() {
     ./ceph osd crush rename-bucket nonexistent something 2>&1 | grep "Error ENOENT" || return 1
 }
 
+function TEST_crush_reject_empty() {
+    local dir=$1
+    run_mon $dir a || return 1
+    # should have at least one OSD
+    run_osd $dir 0 || return 1
+
+    local empty_map=$dir/empty_map
+    :> $empty_map.txt
+    ./crushtool -c $empty_map.txt -o $empty_map.map || return 1
+    expect_failure $dir "Error EINVAL" \
+        ./ceph osd setcrushmap -i $empty_map.map || return 1
+}
+
 main osd-crush
 
 # Local Variables: