From: Mykola Golub <mgolub@suse.com>
Date: Tue, 21 Sep 2021 15:17:17 +0000 (+0100)
Subject: osd: handle case when stat returns with error in get_hash_info
X-Git-Tag: v16.2.7~60^2~2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=8b8fa0e9a43110be60392924f995e7507aa5dbc9;p=ceph.git

osd: handle case when stat returns with error in get_hash_info

Previously in the case of the error we stored in the cache and
returned HashInfo(ec_impl->get_chunk_count()), which e.g. could
propagate to non-primary shards, introducing inconsistency.

The function's `checks` flag is replaced with `create` flag,
which seems to have more clear meaning here.

In be_deep_scrub the get_hash_info is still called with the
second argument false (i.e. with `create=false`, while previously
it was `checks=false`), which is done intentionally.

Fixes: https://tracker.ceph.com/issues/48959
Signed-off-by: Mykola Golub <mgolub@suse.com>
(cherry picked from commit e303dc1cf543fa467d6e0a16c122f77570865e21)

Conflicts:
	src/osd/ECBackend.cc
	src/osd/ECBackend.h
		(differen get_hash_info signature)
---

diff --git a/src/osd/ECBackend.cc b/src/osd/ECBackend.cc
index b11ad496dcf0..b13a99fbc9c5 100644
--- a/src/osd/ECBackend.cc
+++ b/src/osd/ECBackend.cc
@@ -1826,7 +1826,7 @@ void ECBackend::do_read_op(ReadOp &op)
 }
 
 ECUtil::HashInfoRef ECBackend::get_hash_info(
-  const hobject_t &hoid, bool checks, const map<string,bufferptr> *attrs)
+  const hobject_t &hoid, bool create, const map<string,bufferptr> *attrs)
 {
   dout(10) << __func__ << ": Getting attr on " << hoid << dendl;
   ECUtil::HashInfoRef ref = unstable_hashinfo_registry.lookup(hoid);
@@ -1838,7 +1838,6 @@ ECUtil::HashInfoRef ECBackend::get_hash_info(
       ghobject_t(hoid, ghobject_t::NO_GEN, get_parent()->whoami_shard().shard),
       &st);
     ECUtil::HashInfo hinfo(ec_impl->get_chunk_count());
-    // XXX: What does it mean if there is no object on disk?
     if (r >= 0) {
       dout(10) << __func__ << ": found on disk, size " << st.st_size << dendl;
       bufferlist bl;
@@ -1868,7 +1867,7 @@ ECUtil::HashInfoRef ECBackend::get_hash_info(
 	  dout(0) << __func__ << ": Can't decode hinfo for " << hoid << dendl;
 	  return ECUtil::HashInfoRef();
         }
-	if (checks && hinfo.get_total_chunk_size() != (uint64_t)st.st_size) {
+	if (hinfo.get_total_chunk_size() != (uint64_t)st.st_size) {
 	  dout(0) << __func__ << ": Mismatch of total_chunk_size "
 			       << hinfo.get_total_chunk_size() << dendl;
 	  return ECUtil::HashInfoRef();
@@ -1876,6 +1875,10 @@ ECUtil::HashInfoRef ECBackend::get_hash_info(
       } else if (st.st_size > 0) { // If empty object and no hinfo, create it
 	return ECUtil::HashInfoRef();
       }
+    } else if (r != -ENOENT || !create) {
+      derr << __func__ << ": stat " << hoid << " failed: " << cpp_strerror(r)
+           << dendl;
+      return ECUtil::HashInfoRef();
     }
     ref = unstable_hashinfo_registry.lookup_or_create(hoid, hinfo);
   }
@@ -1890,7 +1893,7 @@ void ECBackend::start_rmw(Op *op, PGTransactionUPtr &&t)
     sinfo,
     std::move(t),
     [&](const hobject_t &i) {
-      ECUtil::HashInfoRef ref = get_hash_info(i, false);
+      ECUtil::HashInfoRef ref = get_hash_info(i, true);
       if (!ref) {
 	derr << __func__ << ": get_hash_info(" << i << ")"
 	     << " returned a null pointer and there is no "
diff --git a/src/osd/ECBackend.h b/src/osd/ECBackend.h
index c39de1bfeeb8..45495376a291 100644
--- a/src/osd/ECBackend.h
+++ b/src/osd/ECBackend.h
@@ -629,7 +629,7 @@ public:
   const ECUtil::stripe_info_t sinfo;
   /// If modified, ensure that the ref is held until the update is applied
   SharedPtrRegistry<hobject_t, ECUtil::HashInfo> unstable_hashinfo_registry;
-  ECUtil::HashInfoRef get_hash_info(const hobject_t &hoid, bool checks = true,
+  ECUtil::HashInfoRef get_hash_info(const hobject_t &hoid, bool create = false,
 				    const std::map<std::string, ceph::buffer::ptr> *attr = NULL);
 
 public: