os/FileStore: fix to handle readdir error correctly

Currently filestore code does not handle readdir error.
As man readdir(3) says, we need to check errno after readdir
returns NULL to determine if error happens or not.

This patch fixes the all readdir() calls to check errono and
handle it appropriately:
 - FileStore.cc ... abort if EIO error happens
 - BtrfsFileStoreBAckend.cc/LFNindex.cc
                ... return error to upper layer

Without this fixes, primary PG could fail to correctly perform
backfill operation and could lead data loss propagation described
in #50558.

Fixes: https://tracker.ceph.com/issues/50558
Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
(cherry picked from commit 5a6c6267a182f859471ee629b490777ee1e970dd)

diff --git a/src/os/filestore/BtrfsFileStoreBackend.cc b/src/os/filestore/BtrfsFileStoreBackend.cc
index df1d2452a1fc7d69d0e3155de10a32395c5c1a3f..27161c22ab752ba16067be8f1141da070623ce93 100644 (file)
--- a/src/os/filestore/BtrfsFileStoreBackend.cc
+++ b/src/os/filestore/BtrfsFileStoreBackend.cc
@@ -326,7 +326,17 @@ int BtrfsFileStoreBackend::list_checkpoints(list<string>& ls)
   list<string> snaps;
   char path[PATH_MAX];
   struct dirent *de;
-  while ((de = ::readdir(dir))) {
+  while (true) {
+    errno = 0;
+    de = ::readdir(dir);
+    if (de == nullptr) {
+      if (errno != 0) {
+        err = -errno;
+        dout(0) << "list_checkpoints: readdir '" << get_basedir_path() << "' failed: "
+                << cpp_strerror(err) << dendl;
+      }
+      break;
+    }
     snprintf(path, sizeof(path), "%s/%s", get_basedir_path().c_str(), de->d_name);
 
     struct stat st;

diff --git a/src/os/filestore/FileStore.cc b/src/os/filestore/FileStore.cc
index dc304659b1801da526bb3eccc4343397978da6cb..5b6c7e39b3d25044694539f414bf2a00ff50306c 100644 (file)
--- a/src/os/filestore/FileStore.cc
+++ b/src/os/filestore/FileStore.cc
@@ -4987,7 +4987,17 @@ int FileStore::list_collections(vector<coll_t>& ls, bool include_temp)
   }
 
   struct dirent *de = nullptr;
-  while ((de = ::readdir(dir))) {
+  while (true) {
+    errno = 0;
+    de = ::readdir(dir);
+    if (de == nullptr) {
+      if (errno != 0) {
+        r = -errno;
+        derr << "readdir failed " << fn << ": " << cpp_strerror(-r) << dendl;
+        if (r == -EIO && m_filestore_fail_eio) handle_eio();
+      }
+      break;
+    }
     if (de->d_type == DT_UNKNOWN) {
       // d_type not supported (non-ext[234], btrfs), must stat
       struct stat sb;

diff --git a/src/os/filestore/LFNIndex.cc b/src/os/filestore/LFNIndex.cc
index bbf65bdc66a3d752b69367e377855547052bd9a1..cc4fbad958de93f3202c551e8c22f3aee6b20e1f 100644 (file)
--- a/src/os/filestore/LFNIndex.cc
+++ b/src/os/filestore/LFNIndex.cc
@@ -429,7 +429,18 @@ int LFNIndex::list_objects(const vector<string> &to_list, int max_objs,
   int r = 0;
   int listed = 0;
   bool end = true;
-  while ((de = ::readdir(dir))) {
+  while (true) {
+    errno = 0;
+    de = ::readdir(dir);
+    if (de == nullptr) {
+      if (errno != 0) {
+        r = -errno;
+        dout(0) << "readdir failed " << to_list_path << ": "
+                << cpp_strerror(-r) << dendl;
+        goto cleanup;
+      }
+      break;
+    }
     end = false;
     if (max_objs > 0 && listed >= max_objs) {
       break;
@@ -477,7 +488,18 @@ int LFNIndex::list_subdirs(const vector<string> &to_list,
     return -errno;
 
   struct dirent *de = nullptr;
-  while ((de = ::readdir(dir))) {
+  int r = 0;
+  while (true) {
+    errno = 0;
+    de = ::readdir(dir);
+    if (de == nullptr) {
+      if (errno != 0) {
+        r = -errno;
+        dout(0) << "readdir failed " << to_list_path << ": "
+                << cpp_strerror(-r) << dendl;
+      }
+      break;
+    }
     string short_name(de->d_name);
     string demangled_name;
     if (lfn_is_subdir(short_name, &demangled_name)) {
@@ -486,7 +508,7 @@ int LFNIndex::list_subdirs(const vector<string> &to_list,
   }
 
   ::closedir(dir);
-  return 0;
+  return r;
 }
 
 int LFNIndex::create_path(const vector<string> &to_create)