From: Loic Dachary <loic-201408@dachary.org>
Date: Tue, 16 Sep 2014 09:32:26 +0000 (+0200)
Subject: mon: pool create must not always create a ruleset
X-Git-Tag: v0.86~33^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F2506%2Fhead;p=ceph.git

mon: pool create must not always create a ruleset

The implicit creation of a ruleset when creating a pool is convenient
when nothing is specified. However, if the caller sets a ruleset name,
it should not implicitly create it but return ENOENT instead. Silently
creating a ruleset when there is a typo in the ruleset name is
confusing.

http://tracker.ceph.com/issues/9304 Fixes: #9304

Signed-off-by: Loic Dachary <loic-201408@dachary.org>
---

diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index b2fda0e0df76b..f7ce9194c9f3f 100644
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -5,6 +5,7 @@
  *
  * Copyright (C) 2004-2006 Sage Weil <sage@newdream.net>
  * Copyright (C) 2013,2014 Cloudwatt <libre.licensing@cloudwatt.com>
+ * Copyright (C) 2014 Red Hat <contact@redhat.com>
  *
  * Author: Loic Dachary <loic@dachary.org>
  *
@@ -5247,6 +5248,7 @@ done:
       goto reply;
     }
 
+    bool implicit_ruleset_creation = false;
     string ruleset_name;
     cmd_getval(g_ceph_context, cmdmap, "ruleset", ruleset_name);
     string erasure_code_profile;
@@ -5275,6 +5277,7 @@ done:
 	}
       }
       if (ruleset_name == "") {
+	implicit_ruleset_creation = true;
 	if (erasure_code_profile == "default") {
 	  ruleset_name = "erasure-code";
 	} else {
@@ -5288,6 +5291,17 @@ done:
       ruleset_name = erasure_code_profile;
     }
 
+    if (!implicit_ruleset_creation && ruleset_name != "") {
+      int ruleset;
+      err = get_crush_ruleset(ruleset_name, &ruleset, ss);
+      if (err == -EAGAIN) {
+	wait_for_finished_proposal(new C_RetryMessage(this, m));
+	return true;
+      }
+      if (err)
+	goto reply;
+    }
+
     int64_t expected_num_objects;
     cmd_getval(g_ceph_context, cmdmap, "expected_num_objects", expected_num_objects, int64_t(0));
     if (expected_num_objects < 0) {
diff --git a/src/test/mon/osd-pool-create.sh b/src/test/mon/osd-pool-create.sh
index 4ec4686ec0977..6bd3ec0e9d3b4 100755
--- a/src/test/mon/osd-pool-create.sh
+++ b/src/test/mon/osd-pool-create.sh
@@ -1,6 +1,7 @@
 #!/bin/bash
 #
 # Copyright (C) 2013,2014 Cloudwatt <libre.licensing@cloudwatt.com>
+# Copyright (C) 2014 Red Hat <contact@redhat.com>
 #
 # Author: Loic Dachary <loic@dachary.org>
 #
@@ -98,6 +99,12 @@ function TEST_erasure_crush_rule() {
     ./ceph osd erasure-code-profile set myprofile
     ./ceph osd pool create $poolname 12 12 erasure myprofile
     ./ceph osd crush rule ls | grep $poolname || return 1
+    #
+    # a non existent crush ruleset given in argument is an error
+    # http://tracker.ceph.com/issues/9304
+    #
+    poolname=pool_erasure3
+    ! ./ceph osd pool create $poolname 12 12 erasure myprofile INVALIDRULESET || return 1
 }
 
 function TEST_erasure_crush_rule_pending() {
@@ -126,7 +133,7 @@ function TEST_simple_crush_rule_pending() {
     test $result = true || return 1
     ./ceph osd pool create pool_simple 12 12 replicated $crush_ruleset || return 1
     CEPH_ARGS='' ./ceph --admin-daemon $dir/a/ceph-mon.a.asok log flush || return 1
-    grep "$crush_ruleset is pending, try again" $dir/a/log || return 1
+    grep "$crush_ruleset try again" $dir/a/log || return 1
 }
 
 function TEST_erasure_code_profile_default() {
@@ -240,10 +247,11 @@ function TEST_erasure_code_pool_lrc() {
 function TEST_replicated_pool() {
     local dir=$1
     run_mon $dir a --public-addr 127.0.0.1
-    ./ceph osd pool create replicated 12 12 replicated 2>&1 | \
+    ./ceph osd pool create replicated 12 12 replicated replicated_ruleset 2>&1 | \
         grep "pool 'replicated' created" || return 1
-    ./ceph osd pool create replicated 12 12 replicated 2>&1 | \
+    ./ceph osd pool create replicated 12 12 replicated replicated_ruleset 2>&1 | \
         grep 'already exists' || return 1
+    ! ./ceph osd pool create replicated0 12 12 replicated INVALIDRULESET
     # default is replicated
     ./ceph osd pool create replicated1 12 12 2>&1 | \
         grep "pool 'replicated1' created" || return 1