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 2ff2000d709166cb7f91fca2374741d92c11773f..a62919c4c5f99628fff247e82160720dc2e41033 100644 (file)
--- a/src/os/filestore/BtrfsFileStoreBackend.cc
+++ b/src/os/filestore/BtrfsFileStoreBackend.cc
@@ -322,7 +322,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 d387947e5d080874697d336cd54c1769c7db3c96..228758ed2ba8fee1bfb5d17295e77005e1d8dfda 100644 (file)
--- a/src/os/filestore/FileStore.cc
+++ b/src/os/filestore/FileStore.cc
@@ -4973,7 +4973,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 2451ae8c705209f4a3e35ca754b296a695aeb50b..2aacdceffdc40e72147274f0495c660a768f2f3f 100644 (file)
--- a/src/os/filestore/LFNIndex.cc
+++ b/src/os/filestore/LFNIndex.cc
@@ -420,7 +420,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;
@@ -468,7 +479,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)) {
@@ -477,7 +499,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)