From: Jason Dillaman <dillaman@redhat.com>
Date: Fri, 17 Jul 2020 14:25:20 +0000 (-0400)
Subject: immutable-object-cache: fix error handling during start up
X-Git-Tag: v15.2.5~62^2~2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=7380b24760f032c8c0b96c2ecd59eca2f9ba1b8f;p=ceph.git

immutable-object-cache: fix error handling during start up

Previously the daemon would crash if it couldn't remove all the
files from the specified cache directory. It would also crash if
it could not access its specified domain socket file.

Fixes: https://tracker.ceph.com/issues/45169
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(cherry picked from commit f6dcb6c3dbd45574e1a7ca3f5e8f1f52a0d7ae1e)
---

diff --git a/src/tools/immutable_object_cache/CacheController.cc b/src/tools/immutable_object_cache/CacheController.cc
index 70146008d7f3..b8f53d3f8a0f 100644
--- a/src/tools/immutable_object_cache/CacheController.cc
+++ b/src/tools/immutable_object_cache/CacheController.cc
@@ -45,10 +45,13 @@ int CacheController::init() {
 int CacheController::shutdown() {
   ldout(m_cct, 20) << dendl;
 
-  int r = m_cache_server->stop();
-  if (r < 0) {
-    lderr(m_cct) << "stop error\n" << dendl;
-    return r;
+  int r;
+  if (m_cache_server != nullptr) {
+    r = m_cache_server->stop();
+    if (r < 0) {
+      lderr(m_cct) << "stop error\n" << dendl;
+      return r;
+    }
   }
 
   r = m_object_cache_store->shutdown();
@@ -64,10 +67,15 @@ void CacheController::handle_signal(int signum) {
   shutdown();
 }
 
-void CacheController::run() {
+int CacheController::run() {
   try {
     std::string controller_path =
       m_cct->_conf.get_val<std::string>("immutable_object_cache_sock");
+    if (controller_path.empty()) {
+      lderr(m_cct) << "'immutable_object_cache_sock' path not set" << dendl;
+      return -EINVAL;
+    }
+
     std::remove(controller_path.c_str());
 
     m_cache_server = new CacheServer(m_cct, controller_path,
@@ -76,10 +84,13 @@ void CacheController::run() {
 
     int ret = m_cache_server->run();
     if (ret != 0) {
-      throw std::runtime_error("io serivce run error");
+      return ret;
     }
+
+    return 0;
   } catch (std::exception& e) {
     lderr(m_cct) << "Exception: " << e.what() << dendl;
+    return -EFAULT;
   }
 }
 
diff --git a/src/tools/immutable_object_cache/CacheController.h b/src/tools/immutable_object_cache/CacheController.h
index bf5ae0714771..f70f6bb1c940 100644
--- a/src/tools/immutable_object_cache/CacheController.h
+++ b/src/tools/immutable_object_cache/CacheController.h
@@ -23,15 +23,15 @@ class CacheController {
 
   void handle_signal(int sinnum);
 
-  void run();
+  int run();
 
   void handle_request(CacheSession* session, ObjectCacheRequest* msg);
 
  private:
-  CacheServer *m_cache_server;
+  CacheServer *m_cache_server = nullptr;
   std::vector<const char*> m_args;
   CephContext *m_cct;
-  ObjectCacheStore *m_object_cache_store;
+  ObjectCacheStore *m_object_cache_store = nullptr;
 };
 
 }  // namespace immutable_obj_cache
diff --git a/src/tools/immutable_object_cache/CacheServer.cc b/src/tools/immutable_object_cache/CacheServer.cc
index 7e68d7e87515..08255e71c09b 100644
--- a/src/tools/immutable_object_cache/CacheServer.cc
+++ b/src/tools/immutable_object_cache/CacheServer.cc
@@ -53,20 +53,22 @@ int CacheServer::start_accept() {
   boost::system::error_code ec;
   m_acceptor.open(m_local_path.protocol(), ec);
   if (ec) {
-    ldout(cct, 1) << "m_acceptor open fails: " << ec.message() << dendl;
-    return -1;
+    lderr(cct) << "failed to open domain socket: " << ec.message() << dendl;
+    return -ec.value();
   }
 
   m_acceptor.bind(m_local_path, ec);
   if (ec) {
-    ldout(cct, 1) << "m_acceptor bind fails: " << ec.message() << dendl;
-    return -1;
+    lderr(cct) << "failed to bind to domain socket '"
+               << m_local_path << "': " << ec.message() << dendl;
+    return -ec.value();
   }
 
   m_acceptor.listen(boost::asio::socket_base::max_connections, ec);
   if (ec) {
-    ldout(cct, 1) << "m_acceptor listen fails: " << ec.message() << dendl;
-    return -1;
+    lderr(cct) << "failed to listen on domain socket: " << ec.message()
+               << dendl;
+    return -ec.value();
   }
 
   accept();
diff --git a/src/tools/immutable_object_cache/ObjectCacheStore.cc b/src/tools/immutable_object_cache/ObjectCacheStore.cc
index b3b793d7c2da..ece4127df850 100644
--- a/src/tools/immutable_object_cache/ObjectCacheStore.cc
+++ b/src/tools/immutable_object_cache/ObjectCacheStore.cc
@@ -60,17 +60,19 @@ int ObjectCacheStore::init(bool reset) {
 
   // TODO(dehao): fsck and reuse existing cache objects
   if (reset) {
-    std::error_code ec;
-    if (efs::exists(m_cache_root_dir)) {
-      // remove all sub folders
-      for (auto& p : efs::directory_iterator(m_cache_root_dir)) {
-        efs::remove_all(p.path());
-      }
-    } else {
-      if (!efs::create_directories(m_cache_root_dir, ec)) {
-        lderr(m_cct) << "fail to create cache store dir: " << ec << dendl;
-        return ec.value();
+    try {
+      if (efs::exists(m_cache_root_dir)) {
+        // remove all sub folders
+        for (auto& p : efs::directory_iterator(m_cache_root_dir)) {
+          efs::remove_all(p.path());
+        }
+      } else {
+        efs::create_directories(m_cache_root_dir);
       }
+    } catch (const efs::filesystem_error& e) {
+      lderr(m_cct) << "failed to initialize cache store directory: "
+                   << e.what() << dendl;
+      return -e.code().value();
     }
   }
   return 0;
diff --git a/src/tools/immutable_object_cache/main.cc b/src/tools/immutable_object_cache/main.cc
index bfe49df63238..29977f461f5b 100644
--- a/src/tools/immutable_object_cache/main.cc
+++ b/src/tools/immutable_object_cache/main.cc
@@ -67,7 +67,10 @@ int main(int argc, const char **argv) {
     goto cleanup;
   }
 
-  cachectl->run();
+  r = cachectl->run();
+  if (r < 0) {
+    goto cleanup;
+  }
 
  cleanup:
   unregister_async_signal_handler(SIGHUP, sighup_handler);