From 37a6c4057d367728ace74db6ce3f45e7a707a24a Mon Sep 17 00:00:00 2001
From: Ronen Friedman <rfriedma@redhat.com>
Date: Thu, 8 Oct 2020 15:35:35 +0300
Subject: [PATCH] osd: fix signatures of get_store_errors() and friends

The 'store' parameter was ignored by ScrubStore-related get_*_errors()
functions, and is now removed.

The functions are now marked 'const'. That required tagging the caching
member Store::backend as 'mutable'. While 'mutable' is an 'eyesore',
here is one of the rare cases where its use is justified. Following
https://isocpp.org/wiki/faq/const-correctness:
"When methods change the physical but not logical state, the method should
generally be marked as const since it really is an inspector-method."
(The text then continues and specifically prescribes 'mutable' for these situations.)

Signed-off-by: Ronen Friedman <rfriedma@redhat.com>
---
 src/osd/PrimaryLogPG.cc |  6 ++----
 src/osd/ScrubStore.cc   | 19 ++++++++-----------
 src/osd/ScrubStore.h    | 17 +++++++----------
 3 files changed, 17 insertions(+), 25 deletions(-)

diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc
index 47127382280..75cca704d99 100644
--- a/src/osd/PrimaryLogPG.cc
+++ b/src/osd/PrimaryLogPG.cc
@@ -1595,13 +1595,11 @@ int PrimaryLogPG::do_scrub_ls(const MOSDOp *m, OSDOp *osd_op)
   } else if (!scrubber.store) {
     r = -ENOENT;
   } else if (arg.get_snapsets) {
-    result.vals = scrubber.store->get_snap_errors(osd->store,
-						  get_pgid().pool(),
+    result.vals = scrubber.store->get_snap_errors(get_pgid().pool(),
 						  arg.start_after,
 						  arg.max_return);
   } else {
-    result.vals = scrubber.store->get_object_errors(osd->store,
-						    get_pgid().pool(),
+    result.vals = scrubber.store->get_object_errors(get_pgid().pool(),
 						    arg.start_after,
 						    arg.max_return);
   }
diff --git a/src/osd/ScrubStore.cc b/src/osd/ScrubStore.cc
index 75f834fd47f..a692a44353f 100644
--- a/src/osd/ScrubStore.cc
+++ b/src/osd/ScrubStore.cc
@@ -158,34 +158,31 @@ void Store::cleanup(ObjectStore::Transaction* t)
 }
 
 std::vector<bufferlist>
-Store::get_snap_errors(ObjectStore* store,
-		       int64_t pool,
+Store::get_snap_errors(int64_t pool,
 		       const librados::object_id_t& start,
-		       uint64_t max_return)
+		       uint64_t max_return) const
 {
   const string begin = (start.name.empty() ?
 			first_snap_key(pool) : to_snap_key(pool, start));
   const string end = last_snap_key(pool);
-  return get_errors(store, begin, end, max_return);     
+  return get_errors(begin, end, max_return);
 }
 
 std::vector<bufferlist>
-Store::get_object_errors(ObjectStore* store,
-			 int64_t pool,
+Store::get_object_errors(int64_t pool,
 			 const librados::object_id_t& start,
-			 uint64_t max_return)
+			 uint64_t max_return) const
 {
   const string begin = (start.name.empty() ?
 			first_object_key(pool) : to_object_key(pool, start));
   const string end = last_object_key(pool);
-  return get_errors(store, begin, end, max_return);
+  return get_errors(begin, end, max_return);
 }
 
 std::vector<bufferlist>
-Store::get_errors(ObjectStore* store,
-		  const string& begin,
+Store::get_errors(const string& begin,
 		  const string& end,
-		  uint64_t max_return)
+		  uint64_t max_return) const
 {
   vector<bufferlist> errors;
   auto next = std::make_pair(begin, bufferlist{});
diff --git a/src/osd/ScrubStore.h b/src/osd/ScrubStore.h
index 7722a66c35f..721aae09291 100644
--- a/src/osd/ScrubStore.h
+++ b/src/osd/ScrubStore.h
@@ -28,26 +28,23 @@ public:
   bool empty() const;
   void flush(ObjectStore::Transaction *);
   void cleanup(ObjectStore::Transaction *);
-  std::vector<ceph::buffer::list> get_snap_errors(ObjectStore* store,
-					  int64_t pool,
+  std::vector<ceph::buffer::list> get_snap_errors(int64_t pool,
 					  const librados::object_id_t& start,
-					  uint64_t max_return);
-  std::vector<ceph::buffer::list> get_object_errors(ObjectStore* store,
-					    int64_t pool,
+					  uint64_t max_return) const;
+  std::vector<ceph::buffer::list> get_object_errors(int64_t pool,
 					    const librados::object_id_t& start,
-					    uint64_t max_return);
+					    uint64_t max_return) const;
 private:
   Store(const coll_t& coll, const ghobject_t& oid, ObjectStore* store);
-  std::vector<ceph::buffer::list> get_errors(ObjectStore* store,
-				     const std::string& start, const std::string& end,
-				     uint64_t max_return);
+  std::vector<ceph::buffer::list> get_errors(const std::string& start, const std::string& end,
+				     uint64_t max_return) const;
 private:
   const coll_t coll;
   const ghobject_t hoid;
   // a temp object holding mappings from seq-id to inconsistencies found in
   // scrubbing
   OSDriver driver;
-  MapCacher::MapCacher<std::string, ceph::buffer::list> backend;
+  mutable MapCacher::MapCacher<std::string, ceph::buffer::list> backend;
   std::map<std::string, ceph::buffer::list> results;
 };
 }