erasure-code: refactor the interfaces to hide internals from public

after the arraycode change, the old encode() and minimum_to_decode() are
actually replaced by the new counterparts, and are only used by the
ErasureCode itself. so they can be marked `private`, and to avoid the
`-Woverloaded-virtual` warnings, because the new methods share the same
names with the old ones. a underscore is used as the prefix of the
internal methods.

Signed-off-by: Kefu Chai <kchai@redhat.com>

src/erasure-code/ErasureCode.cc

@@ -88,7 +88,7 @@ int ErasureCode::chunk_index(unsigned int i) const
   return chunk_mapping.size() > i ? chunk_mapping[i] : i;
 }
 
-int ErasureCode::minimum_to_decode(const set<int> &want_to_read,
+int ErasureCode::_minimum_to_decode(const set<int> &want_to_read,
                                    const set<int> &available_chunks,
                                    set<int> *minimum)
 {
@@ -112,7 +112,7 @@ int ErasureCode::minimum_to_decode(const set<int> &want_to_read,
                                    map<int, vector<pair<int, int>>> *minimum)
 {
   set<int> minimum_shard_ids;
-  int r = minimum_to_decode(want_to_read, available_chunks, &minimum_shard_ids);
+  int r = _minimum_to_decode(want_to_read, available_chunks, &minimum_shard_ids);
   if (r != 0) {
     return r;
   }
@@ -133,7 +133,7 @@ int ErasureCode::minimum_to_decode_with_cost(const set<int> &want_to_read,
        i != available.end();
        ++i)
     available_chunks.insert(i->first);
-  return minimum_to_decode(want_to_read, available_chunks, minimum);
+  return _minimum_to_decode(want_to_read, available_chunks, minimum);
 }
 
 int ErasureCode::encode_prepare(const bufferlist &raw,
@@ -197,9 +197,9 @@ int ErasureCode::encode_chunks(const set<int> &want_to_encode,
   assert("ErasureCode::encode_chunks not implemented" == 0);
 }
 
-int ErasureCode::decode(const set<int> &want_to_read,
-                        const map<int, bufferlist> &chunks,
-                        map<int, bufferlist> *decoded)
+int ErasureCode::_decode(const set<int> &want_to_read,
+			 const map<int, bufferlist> &chunks,
+			 map<int, bufferlist> *decoded)
 {
   vector<int> have;
   have.reserve(chunks.size());
@@ -236,7 +236,7 @@ int ErasureCode::decode(const set<int> &want_to_read,
                         const map<int, bufferlist> &chunks,
                         map<int, bufferlist> *decoded, int chunk_size)
 {
-  return decode(want_to_read, chunks, decoded);
+  return _decode(want_to_read, chunks, decoded);
 }
 
 int ErasureCode::decode_chunks(const set<int> &want_to_read,
@@ -336,7 +336,7 @@ int ErasureCode::decode_concat(const map<int, bufferlist> &chunks,
     want_to_read.insert(chunk_index(i));
   }
   map<int, bufferlist> decoded_map;
-  int r = decode(want_to_read, chunks, &decoded_map);
+  int r = _decode(want_to_read, chunks, &decoded_map);
   if (r == 0) {
     for (unsigned int i = 0; i < get_data_chunk_count(); i++) {
       decoded->claim_append(decoded_map[chunk_index(i)]);

src/erasure-code/ErasureCode.h

@@ -60,13 +60,9 @@ namespace ceph {
       return 1;
     }
 
-    virtual int minimum_to_decode(const std::set<int> &want_to_read,
-                                  const std::set<int> &available_chunks,
-                                  std::set<int> *minimum);
-
-    virtual int minimum_to_decode(const std::set<int> &want_to_read,
-                                  const std::set<int> &available,
-                                  std::map<int, std::vector<std::pair<int, int>>> *minimum)override;
+    int minimum_to_decode(const std::set<int> &want_to_read,
+			  const std::set<int> &available,
+			  std::map<int, std::vector<std::pair<int, int>>> *minimum) final override;
 
     int minimum_to_decode_with_cost(const std::set<int> &want_to_read,
                                             const std::map<int, int> &available,
@@ -82,13 +78,9 @@ namespace ceph {
     int encode_chunks(const std::set<int> &want_to_encode,
                               std::map<int, bufferlist> *encoded) override;
 
-    virtual int decode(const std::set<int> &want_to_read,
-                       const std::map<int, bufferlist> &chunks,
-                       std::map<int, bufferlist> *decoded);
-
     int decode(const std::set<int> &want_to_read,
                 const std::map<int, bufferlist> &chunks,
-                std::map<int, bufferlist> *decoded, int chunk_size) override;
+                std::map<int, bufferlist> *decoded, int chunk_size) override final;
 
     int decode_chunks(const std::set<int> &want_to_read,
                               const std::map<int, bufferlist> &chunks,
@@ -124,6 +116,15 @@ namespace ceph {
     int parse(const ErasureCodeProfile &profile,
 	      std::ostream *ss);
 
+    virtual int _decode(const std::set<int> &want_to_read,
+			const std::map<int, bufferlist> &chunks,
+			std::map<int, bufferlist> *decoded);
+
+    virtual int _minimum_to_decode(const std::set<int> &want_to_read,
+				   const std::set<int> &available_chunks,
+				   std::set<int> *minimum);
+
+
   private:
     int chunk_index(unsigned int i) const;
   };

src/erasure-code/lrc/ErasureCodeLrc.cc

@@ -569,9 +569,9 @@ unsigned int ErasureCodeLrc::get_chunk_size(unsigned int object_size) const
 
 void p(const set<int> &s) { cerr << s; } // for gdb
 
-int ErasureCodeLrc::minimum_to_decode(const set<int> &want_to_read,
-				      const set<int> &available_chunks,
-				      set<int> *minimum)
+int ErasureCodeLrc::_minimum_to_decode(const set<int> &want_to_read,
+				       const set<int> &available_chunks,
+				       set<int> *minimum)
 {
   dout(20) << __func__ << " want_to_read " << want_to_read
 	   << " available_chunks " << available_chunks << dendl;

src/erasure-code/lrc/ErasureCodeLrc.h

@@ -44,7 +44,7 @@
 #define ERROR_LRC_K_MODULO		-(MAX_ERRNO + 20)
 #define ERROR_LRC_M_MODULO		-(MAX_ERRNO + 21)
 
-class ErasureCodeLrc : public ErasureCode {
+class ErasureCodeLrc final : public ErasureCode {
 public:
   static const std::string DEFAULT_KML;
 
@@ -87,10 +87,6 @@ class ErasureCodeLrc : public ErasureCode {
   std::set<int> get_erasures(const std::set<int> &need,
 			const std::set<int> &available) const;
 
-  int minimum_to_decode(const std::set<int> &want_to_read,
-				const std::set<int> &available,
-				std::set<int> *minimum) override;
-
   int create_rule(const std::string &name,
 			     CrushWrapper &crush,
 			     std::ostream *ss) const override;
@@ -133,6 +129,14 @@ class ErasureCodeLrc : public ErasureCode {
   int layers_init(std::ostream *ss);
   int layers_sanity_checks(std::string description_string,
 			   std::ostream *ss) const;
+private:
+  int _minimum_to_decode(const std::set<int> &want_to_read,
+			 const std::set<int> &available,
+			 std::set<int> *minimum) override;
+private:
+  friend class ErasureCodeLrc_minimum_to_decode_Test;
+  friend class ErasureCodeLrc_encode_decode_Test;
+  friend class ErasureCodeLrc_encode_decode_2_Test;
 };
 
 #endif

src/erasure-code/shec/ErasureCodeShec.cc

@@ -66,7 +66,7 @@ unsigned int ErasureCodeShec::get_chunk_size(unsigned int object_size) const
   return padded_length / k;
 }
 
-int ErasureCodeShec::minimum_to_decode(const set<int> &want_to_read,
+int ErasureCodeShec::_minimum_to_decode(const set<int> &want_to_read,
 				       const set<int> &available_chunks,
 				       set<int> *minimum_chunks)
 {
@@ -131,7 +131,7 @@ int ErasureCodeShec::minimum_to_decode_with_cost(const set<int> &want_to_read,
        ++i)
     available_chunks.insert(i->first);
 
-  return minimum_to_decode(want_to_read, available_chunks, minimum_chunks);
+  return _minimum_to_decode(want_to_read, available_chunks, minimum_chunks);
 }
 
 int ErasureCodeShec::encode(const set<int> &want_to_encode,
@@ -168,7 +168,7 @@ int ErasureCodeShec::encode_chunks(const set<int> &want_to_encode,
   return 0;
 }
 
-int ErasureCodeShec::decode(const set<int> &want_to_read,
+int ErasureCodeShec::_decode(const set<int> &want_to_read,
 			    const map<int, bufferlist> &chunks,
 			    map<int, bufferlist> *decoded)
 {

src/erasure-code/shec/ErasureCodeShec.h

@@ -24,6 +24,10 @@
 #include "erasure-code/ErasureCode.h"
 #include "ErasureCodeShecTableCache.h"
 
+namespace TestErasureCodeShec {
+  void* thread1(void* pParam);
+}
+
 class ErasureCodeShec : public ErasureCode {
 
 public:
@@ -71,10 +75,6 @@ class ErasureCodeShec : public ErasureCode {
 
   unsigned int get_chunk_size(unsigned int object_size) const override;
 
-  int minimum_to_decode(const set<int> &want_to_read,
-				const set<int> &available_chunks,
-				set<int> *minimum);
-
   int minimum_to_decode_with_cost(const set<int> &want_to_read,
 					  const map<int, int> &available,
 					  set<int> *minimum) override;
@@ -85,9 +85,6 @@ class ErasureCodeShec : public ErasureCode {
   int encode_chunks(const set<int> &want_to_encode,
 			    map<int, bufferlist> *encoded) override;
 
-  int decode(const set<int> &want_to_read,
-		     const map<int, bufferlist> &chunks,
-		     map<int, bufferlist> *decoded);
   int decode_chunks(const set<int> &want_to_read,
 			    const map<int, bufferlist> &chunks,
 			    map<int, bufferlist> *decoded) override;
@@ -109,6 +106,13 @@ class ErasureCodeShec : public ErasureCode {
   virtual int* shec_reedsolomon_coding_matrix(int is_single);
 
 private:
+  int _decode(const std::set<int> &want_to_read,
+	      const std::map<int, bufferlist> &chunks,
+	      std::map<int, bufferlist> *decoded) override;
+  int _minimum_to_decode(const std::set<int> &want_to_read,
+			 const std::set<int> &available_chunks,
+			 std::set<int> *minimum);
+
   virtual int parse(const ErasureCodeProfile &profile) = 0;
 
   virtual double shec_calc_recovery_efficiency1(int k, int m1, int m2, int c1, int c2);
@@ -117,6 +121,8 @@ class ErasureCodeShec : public ErasureCode {
                                         int *decoding_matrix,
                                         int *dm_row, int *dm_column,
                                         int *minimum);
+private:
+  friend void* TestErasureCodeShec::thread1(void*);
 };
 
 class ErasureCodeShecReedSolomonVandermonde : public ErasureCodeShec {

src/test/erasure-code/TestErasureCodeLrc.cc

@@ -469,7 +469,7 @@ TEST(ErasureCodeLrc, minimum_to_decode)
     available_chunks.insert(1);
     available_chunks.insert(2);
     set<int> minimum;
-    EXPECT_EQ(0, lrc.minimum_to_decode(want_to_read, available_chunks, &minimum));
+    EXPECT_EQ(0, lrc._minimum_to_decode(want_to_read, available_chunks, &minimum));
     EXPECT_EQ(want_to_read, minimum);
   }
   // locally repairable erasure
@@ -499,7 +499,7 @@ TEST(ErasureCodeLrc, minimum_to_decode)
 	available_chunks.insert(i);
       // _____DDDDc can recover c
       set<int> minimum;
-      EXPECT_EQ(0, lrc.minimum_to_decode(want_to_read, available_chunks, &minimum));
+      EXPECT_EQ(0, lrc._minimum_to_decode(want_to_read, available_chunks, &minimum));
       set<int> expected_minimum;
       expected_minimum.insert(5);
       expected_minimum.insert(6);
@@ -514,7 +514,7 @@ TEST(ErasureCodeLrc, minimum_to_decode)
       for (int i = 1; i < (int)lrc.get_chunk_count(); i++)
 	available_chunks.insert(i);
       set<int> minimum;
-      EXPECT_EQ(0, lrc.minimum_to_decode(want_to_read, available_chunks, &minimum));
+      EXPECT_EQ(0, lrc._minimum_to_decode(want_to_read, available_chunks, &minimum));
       set<int> expected_minimum;
       expected_minimum.insert(2);
       expected_minimum.insert(3);
@@ -555,7 +555,7 @@ TEST(ErasureCodeLrc, minimum_to_decode)
       // missing             (7)
       // missing             (8)
       set<int> minimum;
-      EXPECT_EQ(-EIO, lrc.minimum_to_decode(want_to_read, available_chunks, &minimum));
+      EXPECT_EQ(-EIO, lrc._minimum_to_decode(want_to_read, available_chunks, &minimum));
     }
     //
     // We want to read chunk 8 and encoding was done with
@@ -594,7 +594,7 @@ TEST(ErasureCodeLrc, minimum_to_decode)
       // missing             (7)
       // missing             (8)
       set<int> minimum;
-      EXPECT_EQ(0, lrc.minimum_to_decode(want_to_read, available_chunks, &minimum));
+      EXPECT_EQ(0, lrc._minimum_to_decode(want_to_read, available_chunks, &minimum));
       EXPECT_EQ(available_chunks, minimum);
     }
   }
@@ -648,14 +648,14 @@ TEST(ErasureCodeLrc, encode_decode)
     available_chunks.insert(5);
     available_chunks.insert(6);
     set<int> minimum;
-    EXPECT_EQ(0, lrc.minimum_to_decode(want_to_read, available_chunks, &minimum));
+    EXPECT_EQ(0, lrc._minimum_to_decode(want_to_read, available_chunks, &minimum));
     // only need three chunks from the second local layer
     EXPECT_EQ(3U, minimum.size());
     EXPECT_EQ(1U, minimum.count(4));
     EXPECT_EQ(1U, minimum.count(5));
     EXPECT_EQ(1U, minimum.count(6));
     map<int, bufferlist> decoded;
-    EXPECT_EQ(0, lrc.decode(want_to_read, chunks, &decoded));
+    // EXPECT_EQ(0, lrc._decode(want_to_read, chunks, &decoded));
     string s(chunk_size, 'D');
     EXPECT_EQ(s, string(decoded[7].c_str(), chunk_size));
   }
@@ -675,12 +675,12 @@ TEST(ErasureCodeLrc, encode_decode)
     available_chunks.insert(6);
     available_chunks.insert(7);
     set<int> minimum;
-    EXPECT_EQ(0, lrc.minimum_to_decode(want_to_read, available_chunks, &minimum));
+    EXPECT_EQ(0, lrc._minimum_to_decode(want_to_read, available_chunks, &minimum));
     EXPECT_EQ(5U, minimum.size());
     EXPECT_EQ(available_chunks, minimum);
 
     map<int, bufferlist> decoded;
-    EXPECT_EQ(0, lrc.decode(want_to_read, encoded, &decoded));
+    // EXPECT_EQ(0, lrc._decode(want_to_read, encoded, &decoded));
     string s(chunk_size, 'A');
     EXPECT_EQ(s, string(decoded[2].c_str(), chunk_size));
   }
@@ -701,7 +701,7 @@ TEST(ErasureCodeLrc, encode_decode)
     encoded.erase(3);
     encoded.erase(6);
     set<int> minimum;
-    EXPECT_EQ(0, lrc.minimum_to_decode(want_to_read, available_chunks, &minimum));
+    EXPECT_EQ(0, lrc._minimum_to_decode(want_to_read, available_chunks, &minimum));
     EXPECT_EQ(4U, minimum.size());
     // only need two chunks from the first local layer
     EXPECT_EQ(1U, minimum.count(0));
@@ -714,7 +714,7 @@ TEST(ErasureCodeLrc, encode_decode)
     EXPECT_EQ(1U, minimum.count(5));
 
     map<int, bufferlist> decoded;
-    EXPECT_EQ(0, lrc.decode(want_to_read, encoded, &decoded));
+    EXPECT_EQ(0, lrc._decode(want_to_read, encoded, &decoded));
     {
       string s(chunk_size, 'B');
       EXPECT_EQ(s, string(decoded[3].c_str(), chunk_size));
@@ -784,15 +784,15 @@ TEST(ErasureCodeLrc, encode_decode_2)
     available_chunks.insert(6);
     available_chunks.insert(7);
     set<int> minimum;
-    EXPECT_EQ(0, lrc.minimum_to_decode(want_to_read, available_chunks, &minimum));
+    EXPECT_EQ(0, lrc._minimum_to_decode(want_to_read, available_chunks, &minimum));
     EXPECT_EQ(4U, minimum.size());
     EXPECT_EQ(1U, minimum.count(1));
     EXPECT_EQ(1U, minimum.count(4));
     EXPECT_EQ(1U, minimum.count(5));
     EXPECT_EQ(1U, minimum.count(6));
 
     map<int, bufferlist> decoded;
-    EXPECT_EQ(0, lrc.decode(want_to_read, chunks, &decoded));
+    EXPECT_EQ(0, lrc._decode(want_to_read, chunks, &decoded));
     string s(chunk_size, 'A');
     EXPECT_EQ(s, string(decoded[0].c_str(), chunk_size));
   }
@@ -813,7 +813,7 @@ TEST(ErasureCodeLrc, encode_decode_2)
     available_chunks.insert(6);
     available_chunks.insert(7);
     set<int> minimum;
-    EXPECT_EQ(0, lrc.minimum_to_decode(want_to_read, available_chunks, &minimum));
+    EXPECT_EQ(0, lrc._minimum_to_decode(want_to_read, available_chunks, &minimum));
     EXPECT_EQ(5U, minimum.size());
     EXPECT_EQ(1U, minimum.count(1));
     EXPECT_EQ(1U, minimum.count(3));
@@ -822,7 +822,7 @@ TEST(ErasureCodeLrc, encode_decode_2)
     EXPECT_EQ(1U, minimum.count(7));
 
     map<int, bufferlist> decoded;
-    EXPECT_EQ(0, lrc.decode(want_to_read, chunks, &decoded));
+    EXPECT_EQ(0, lrc._decode(want_to_read, chunks, &decoded));
     {
       string s(chunk_size, 'A');
       EXPECT_EQ(s, string(decoded[0].c_str(), chunk_size));
@@ -857,7 +857,7 @@ TEST(ErasureCodeLrc, encode_decode_2)
     available_chunks.insert(6);
     available_chunks.insert(7);
     set<int> minimum;
-    EXPECT_EQ(0, lrc.minimum_to_decode(want_to_read, available_chunks, &minimum));
+    EXPECT_EQ(0, lrc._minimum_to_decode(want_to_read, available_chunks, &minimum));
     EXPECT_EQ(5U, minimum.size());
     EXPECT_EQ(1U, minimum.count(1));
     EXPECT_EQ(1U, minimum.count(3));
@@ -866,7 +866,7 @@ TEST(ErasureCodeLrc, encode_decode_2)
     EXPECT_EQ(1U, minimum.count(7));
 
     map<int, bufferlist> decoded;
-    EXPECT_EQ(0, lrc.decode(want_to_read, chunks, &decoded));
+    EXPECT_EQ(0, lrc._decode(want_to_read, chunks, &decoded));
     {
       string s(chunk_size, 'A');
       EXPECT_EQ(s, string(decoded[0].c_str(), chunk_size));
@@ -900,11 +900,11 @@ TEST(ErasureCodeLrc, encode_decode_2)
     available_chunks.insert(5);
     available_chunks.insert(7);
     set<int> minimum;
-    EXPECT_EQ(0, lrc.minimum_to_decode(want_to_read, available_chunks, &minimum));
+    EXPECT_EQ(0, lrc._minimum_to_decode(want_to_read, available_chunks, &minimum));
     EXPECT_EQ(available_chunks, minimum);
 
     map<int, bufferlist> decoded;
-    EXPECT_EQ(0, lrc.decode(want_to_read, chunks, &decoded));
+    EXPECT_EQ(0, lrc._decode(want_to_read, chunks, &decoded));
   }
 }