mon: normalize erasure-code profile for storage and comparison

An erasure-code profile set by the user may be modified by the erasure
code plugin, most of the time to add default values for domain failure,
techniques etc.

Whenever the monitor gets an erasure code profile from the user or the
configuration file, it uses it to initialize the corresponding erasure
code plugin and:

 * validate the profile and get an error if it is invalid

 * get a normalized profile

The normalized profile is stored in the OSDMap. Should the user try
to set the same erasure code profile with the different values that lead
to the same normalized form, the modification will be accepted as a
noop, as if the same values were given. For instance:

   ceph osd erasure-code-profile set myprofile k=2

   ceph osd erasure-code-profile set myprofile k=2 m=1

are the same because the default value for m is 1 for the default
jerasure plugin.

If the normalized form is not used for storage and comparison, the
second call with m=1 will fail and require --force to override
myprofile, although the user can read from the documentation that it
matches the default value.

Signed-off-by: Loic Dachary <ldachary@redhat.com>

diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index 0fb63cfb6893a21b01bf805dbc556e52dbe0d311..6fbc0943fe83a7c0fed49e7a47c7d6b15d09e9d1 100644 (file)
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -3940,6 +3940,17 @@ int OSDMonitor::crush_rename_bucket(const string& srcname,
   return 0;
 }
 
+int OSDMonitor::normalize_profile(ErasureCodeProfile &profile, ostream *ss)
+{
+  ErasureCodeInterfaceRef erasure_code;
+  ErasureCodePluginRegistry &instance = ErasureCodePluginRegistry::instance();
+  ErasureCodeProfile::const_iterator plugin = profile.find("plugin");
+  int err = instance.factory(plugin->second, profile, &erasure_code, ss);
+  if (err)
+    return err;
+  return erasure_code->init(profile, ss);
+}
+
 int OSDMonitor::crush_ruleset_create_erasure(const string &name,
                                             const string &profile,
                                             int *ruleset,
@@ -5451,22 +5462,6 @@ bool OSDMonitor::prepare_command_impl(MMonCommand *m,
     }
     string plugin = profile_map["plugin"];
 
-    if (osdmap.has_erasure_code_profile(name)) {
-      if (osdmap.get_erasure_code_profile(name) == profile_map) {
-       err = 0;
-       goto reply;
-      }
-      if (!force) {
-       err = -EPERM;
-       ss << "will not override erasure code profile " << name
-          << " because the existing profile "
-          << osdmap.get_erasure_code_profile(name)
-          << " is different from the proposed profile "
-          << profile_map;
-       goto reply;
-      }
-    }
-
     if (pending_inc.has_erasure_code_profile(name)) {
       dout(20) << "erasure code profile " << name << " try again" << dendl;
       goto wait;
@@ -5483,7 +5478,34 @@ bool OSDMonitor::prepare_command_impl(MMonCommand *m,
          goto reply;
        }
       }
-      dout(20) << "erasure code profile " << name << " set" << dendl;
+      err = normalize_profile(profile_map, &ss);
+      if (err)
+       goto reply;
+
+      if (osdmap.has_erasure_code_profile(name)) {
+       ErasureCodeProfile existing_profile_map =
+         osdmap.get_erasure_code_profile(name);
+       err = normalize_profile(existing_profile_map, &ss);
+       if (err)
+         goto reply;
+
+       if (existing_profile_map == profile_map) {
+         err = 0;
+         goto reply;
+       }
+       if (!force) {
+         err = -EPERM;
+         ss << "will not override erasure code profile " << name
+            << " because the existing profile "
+            << existing_profile_map
+            << " is different from the proposed profile "
+            << profile_map;
+         goto reply;
+       }
+      }
+
+      dout(20) << "erasure code profile set " << name << "="
+              << profile_map << dendl;
       pending_inc.set_erasure_code_profile(name, profile_map);
     }
 
@@ -5517,7 +5539,11 @@ bool OSDMonitor::prepare_command_impl(MMonCommand *m,
                                                      &ss);
        if (err)
          goto reply;
-       dout(20) << "erasure code profile " << profile << " set" << dendl;
+       err = normalize_profile(profile_map, &ss);
+       if (err)
+         goto reply;
+       dout(20) << "erasure code profile set " << profile << "="
+                << profile_map << dendl;
        pending_inc.set_erasure_code_profile(profile, profile_map);
        goto wait;
       }

diff --git a/src/mon/OSDMonitor.h b/src/mon/OSDMonitor.h
index bd533c9eb8ecf68968063aec2d14d67b9b613654..bb8d3e1427cdbe195c74eaac02a4a2dd99f8f8b1 100644 (file)
--- a/src/mon/OSDMonitor.h
+++ b/src/mon/OSDMonitor.h
@@ -277,6 +277,7 @@ private:
   int crush_rename_bucket(const string& srcname,
                          const string& dstname,
                          ostream *ss);
+  int normalize_profile(ErasureCodeProfile &profile, ostream *ss);
   int crush_ruleset_create_erasure(const string &name,
                                   const string &profile,
                                   int *ruleset,

diff --git a/src/test/mon/osd-crush.sh b/src/test/mon/osd-crush.sh
index 4857ca2b2595690ce8de68a73289f72f8f9b0e94..b0fd840753bfb35501e0b9a24f1a2213062d0abb 100755 (executable)
--- a/src/test/mon/osd-crush.sh
+++ b/src/test/mon/osd-crush.sh
@@ -113,7 +113,7 @@ function TEST_crush_rule_create_erasure() {
     ! ./ceph osd erasure-code-profile ls | grep default || return 1
     ./ceph osd crush rule create-erasure $ruleset || return 1
     CEPH_ARGS='' ./ceph --admin-daemon $dir/ceph-mon.a.asok log flush || return 1
-    grep 'profile default set' $dir/mon.a.log || return 1
+    grep 'profile set default' $dir/mon.a.log || return 1
     ./ceph osd erasure-code-profile ls | grep default || return 1
     ./ceph osd crush rule rm $ruleset || return 1
     ! ./ceph osd crush rule ls | grep $ruleset || return 1