From: Loic Dachary <loic-201408@dachary.org>
Date: Tue, 9 Sep 2014 19:38:05 +0000 (+0200)
Subject: erasure-code: refactor jerasure Liberation parameters checks
X-Git-Tag: v0.86~75^2~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=f917166157f3fce7ad0200f4f25e8466fcaedeea;p=ceph.git

erasure-code: refactor jerasure Liberation parameters checks

Create a virtual function to check for each parameter for Liberation
and its derivations Liber8tion and BlaumRoth. The code is moved around
but is unmodified.

A function reseting the parameters to sane defaults when an error
occurs is introduced. It is slightly different from the code that was
duplicated in Liberation and BlaumRoth. In both cases it now resets w.
It is a noop for Liber8tion because the user cannot modify this
parameter which is always set to the default.

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

diff --git a/src/erasure-code/jerasure/ErasureCodeJerasure.cc b/src/erasure-code/jerasure/ErasureCodeJerasure.cc
index 4bd6aeb21458..bc845833f319 100644
--- a/src/erasure-code/jerasure/ErasureCodeJerasure.cc
+++ b/src/erasure-code/jerasure/ErasureCodeJerasure.cc
@@ -372,36 +372,71 @@ unsigned ErasureCodeJerasureLiberation::get_alignment() const
   return alignment;
 }
 
-int ErasureCodeJerasureLiberation::parse(const map<std::string,std::string> &parameters,
-					 ostream *ss)
+bool ErasureCodeJerasureLiberation::check_k(ostream *ss) const
 {
-  int err = ErasureCodeJerasure::parse(parameters, ss);
-  err |= to_int("packetsize", parameters, &packetsize, DEFAULT_PACKETSIZE, ss);
-
-  bool error = false;
   if (k > w) {
     *ss << "k=" << k << " must be less than or equal to w=" << w << std::endl;
-    error = true;
+    return false;
+  } else {
+    return true;
   }
+}
+
+bool ErasureCodeJerasureLiberation::check_w(ostream *ss) const
+{
   if (w <= 2 || !is_prime(w)) {
     *ss <<  "w=" << w << " must be greater than two and be prime" << std::endl;
-    error = true;
+    return false;
+  } else {
+    return true;
   }
+}
+
+bool ErasureCodeJerasureLiberation::check_packetsize_set(ostream *ss) const
+{
   if (packetsize == 0) {
     *ss << "packetsize=" << packetsize << " must be set" << std::endl;
-    error = true;
+    return false;
+  } else {
+    return true;
   }
+}
+
+bool ErasureCodeJerasureLiberation::check_packetsize(ostream *ss) const
+{
   if ((packetsize%(sizeof(int))) != 0) {
     *ss << "packetsize=" << packetsize
 	<< " must be a multiple of sizeof(int) = " << sizeof(int) << std::endl;
-    error = true;
+    return false;
+  } else {
+    return true;
   }
+}
+
+void ErasureCodeJerasureLiberation::revert_to_default(ostream *ss)
+{
+  *ss << "reverting to k=" << DEFAULT_K << ", w="
+      << DEFAULT_W << ", packetsize=" << DEFAULT_PACKETSIZE << std::endl;
+  k = DEFAULT_K;
+  w = DEFAULT_W;
+  packetsize = DEFAULT_PACKETSIZE;
+}
+
+int ErasureCodeJerasureLiberation::parse(const map<std::string,std::string> &parameters,
+					 ostream *ss)
+{
+  int err = ErasureCodeJerasure::parse(parameters, ss);
+  err |= to_int("packetsize", parameters, &packetsize, DEFAULT_PACKETSIZE, ss);
+
+  bool error = false;
+  if (!check_k(ss))
+    error = true;
+  if (!check_w(ss))
+    error = true;
+  if (!check_packetsize_set(ss) || !check_packetsize(ss))
+    error = true;
   if (error) {
-    *ss << "reverting to k=" << DEFAULT_K << ", w="
-	<< DEFAULT_W << ", packetsize=" << DEFAULT_PACKETSIZE << std::endl;
-    k = DEFAULT_K;
-    w = DEFAULT_W;
-    packetsize = DEFAULT_PACKETSIZE;
+    revert_to_default(ss);
     err = -EINVAL;
   }
   return err;
@@ -434,19 +469,12 @@ int ErasureCodeJerasureLiber8tion::parse(const map<std::string,std::string> &par
   err |= to_int("packetsize", parameters, &packetsize, DEFAULT_PACKETSIZE, ss);
 
   bool error = false;
-  if (k > w) {
-    *ss << "k=" << k << " must be less than or equal to w=" << w << std::endl;
+  if (!check_k(ss))
     error = true;
-  }
-  if (packetsize == 0) {
-    *ss << "packetsize=" << packetsize << " must be set" << std::endl;
+  if (!check_packetsize_set(ss))
     error = true;
-  }
   if (error) {
-    *ss << "reverting to k=" << DEFAULT_K << ", packetsize="
-	<< DEFAULT_PACKETSIZE << std::endl;
-    k = DEFAULT_K;
-    packetsize = DEFAULT_PACKETSIZE;
+    revert_to_default(ss);
     err = -EINVAL;
   }
   return err;
diff --git a/src/erasure-code/jerasure/ErasureCodeJerasure.h b/src/erasure-code/jerasure/ErasureCodeJerasure.h
index c7c16b8dcffe..8034b99385f0 100644
--- a/src/erasure-code/jerasure/ErasureCodeJerasure.h
+++ b/src/erasure-code/jerasure/ErasureCodeJerasure.h
@@ -220,6 +220,11 @@ public:
                                char **coding,
                                int blocksize);
   virtual unsigned get_alignment() const;
+  virtual bool check_k(ostream *ss) const;
+  virtual bool check_w(ostream *ss) const;
+  virtual bool check_packetsize_set(ostream *ss) const;
+  virtual bool check_packetsize(ostream *ss) const;
+  virtual void revert_to_default(ostream *ss);
   virtual int parse(const map<std::string,std::string> &parameters,
 		    ostream *ss);
   virtual void prepare();