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: wip-pdonnell-testing-20200918.022351~582^2~2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=f6dcb6c3dbd45574e1a7ca3f5e8f1f52a0d7ae1e;p=ceph-ci.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>
---

diff --git a/src/tools/immutable_object_cache/CacheController.cc b/src/tools/immutable_object_cache/CacheController.cc
index 70146008d7f..b8f53d3f8a0 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 bf5ae071477..f70f6bb1c94 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 7e68d7e8751..08255e71c09 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 b3b793d7c2d..ece4127df85 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 bfe49df6323..29977f461f5 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);