common: ping existing admin socket before unlink (dumpling)

src/common/admin_socket.cc

@@ -14,6 +14,7 @@
 
 #include "common/Thread.h"
 #include "common/admin_socket.h"
+#include "common/admin_socket_client.h"
 #include "common/config.h"
 #include "common/cmdparse.h"
 #include "common/dout.h"
@@ -183,15 +184,21 @@ std::string AdminSocket::bind_and_listen(const std::string &sock_path, int *fd)
 	   sizeof(struct sockaddr_un)) != 0) {
     int err = errno;
     if (err == EADDRINUSE) {
-      // The old UNIX domain socket must still be there.
-      // Let's unlink it and try again.
-      TEMP_FAILURE_RETRY(unlink(sock_path.c_str()));
-      if (bind(sock_fd, (struct sockaddr*)&address,
-	       sizeof(struct sockaddr_un)) == 0) {
-	err = 0;
-      }
-      else {
-	err = errno;
+      AdminSocketClient client(sock_path);
+      bool ok;
+      client.ping(&ok);
+      if (ok) {
+	ldout(m_cct, 20) << "socket " << sock_path << " is in use" << dendl;
+	err = EEXIST;
+      } else {
+	ldout(m_cct, 20) << "unlink stale file " << sock_path << dendl;
+	TEMP_FAILURE_RETRY(unlink(sock_path.c_str()));
+	if (bind(sock_fd, (struct sockaddr*)&address,
+		 sizeof(struct sockaddr_un)) == 0) {
+	  err = 0;
+	} else {
+	  err = errno;
+	}
       }
     }
     if (err != 0) {

src/common/admin_socket_client.cc

@@ -126,6 +126,14 @@ AdminSocketClient(const std::string &path)
 {
 }
 
+std::string AdminSocketClient::ping(bool *ok)
+{
+  std::string version;
+  std::string result = do_request("{\"prefix\":\"0\"}", &version);
+  *ok = result == "" && version.length() == 1;
+  return result;
+}
+
 std::string AdminSocketClient::do_request(std::string request, std::string *result)
 {
   int socket_fd = 0, res;

src/common/admin_socket_client.h

@@ -26,6 +26,7 @@ class AdminSocketClient
 public:
   AdminSocketClient(const std::string &path);
   std::string do_request(std::string request, std::string *result);
+  std::string ping(bool *ok);
 private:
   std::string m_path;
 };

src/test/admin_socket.cc

@@ -12,11 +12,15 @@
  *
  */
 
+#include <gtest/gtest.h>
+
 #include "common/Mutex.h"
+#include "common/Cond.h"
 #include "common/admin_socket.h"
 #include "common/admin_socket_client.h"
-#include "common/ceph_context.h"
-#include "test/unit.h"
+#include "common/ceph_argparse.h"
+#include "global/global_init.h"
+#include "global/global_context.h"
 
 #include <stdint.h>
 #include <string.h>
@@ -33,6 +37,9 @@ class AdminSocketTest
   bool init(const std::string &uri) {
     return m_asokc->init(uri);
   }
+  string bind_and_listen(const std::string &sock_path, int *fd) {
+    return m_asokc->bind_and_listen(sock_path, fd);
+  }
   bool shutdown() {
     m_asokc->shutdown();
     return true;
@@ -179,13 +186,125 @@ TEST(AdminSocket, RegisterCommandPrefixes) {
   ASSERT_EQ(true, asoct.shutdown());
 }
 
+class BlockingHook : public AdminSocketHook {
+public:
+  Mutex _lock;
+  Cond _cond;
+
+  BlockingHook() : _lock("BlockingHook::_lock") {}
+
+  bool call(std::string command, cmdmap_t& cmdmap, std::string format, bufferlist& result) {
+    Mutex::Locker l(_lock);
+    _cond.Wait(_lock);
+    return true;
+  }
+};
+
+TEST(AdminSocketClient, Ping) {
+  string path = get_rand_socket_path();
+  std::auto_ptr<AdminSocket>
+      asokc(new AdminSocket(g_ceph_context));
+  AdminSocketClient client(path);
+  // no socket
+  {
+    bool ok;
+    std::string result = client.ping(&ok);
+    EXPECT_NE(std::string::npos, result.find("No such file or directory"));
+    ASSERT_FALSE(ok);
+  }
+  // file exists but does not allow connections (no process, wrong type...)
+  ASSERT_TRUE(::creat(path.c_str(), 0777));
+  {
+    bool ok;
+    std::string result = client.ping(&ok);
+    EXPECT_NE(std::string::npos, result.find("Connection refused"));
+    ASSERT_FALSE(ok);
+  }
+  // a daemon is connected to the socket
+  {
+    AdminSocketTest asoct(asokc.get());
+    ASSERT_TRUE(asoct.init(path));
+    bool ok;
+    std::string result = client.ping(&ok);
+    EXPECT_EQ("", result);
+    ASSERT_TRUE(ok);
+    ASSERT_TRUE(asoct.shutdown());
+  }
+  // hardcoded five seconds timeout prevents infinite blockage
+  {
+    AdminSocketTest asoct(asokc.get());
+    BlockingHook *blocking = new BlockingHook();
+    ASSERT_EQ(0, asoct.m_asokc->register_command("0", "0", blocking, ""));
+    ASSERT_TRUE(asoct.init(path));
+    bool ok;
+    std::string result = client.ping(&ok);
+    EXPECT_NE(std::string::npos, result.find("Resource temporarily unavailable"));
+    ASSERT_FALSE(ok);
+    {
+      Mutex::Locker l(blocking->_lock);
+      blocking->_cond.Signal();
+    }
+    ASSERT_TRUE(asoct.shutdown());
+    delete blocking;
+  }
+}
+
+TEST(AdminSocket, bind_and_listen) {
+  string path = get_rand_socket_path();
+  std::auto_ptr<AdminSocket>
+      asokc(new AdminSocket(g_ceph_context));
+
+  AdminSocketTest asoct(asokc.get());
+  // successfull bind
+  {
+    int fd = 0;
+    string message;
+    message = asoct.bind_and_listen(path, &fd);
+    ASSERT_NE(0, fd);
+    ASSERT_EQ("", message);
+    ASSERT_EQ(0, ::close(fd));
+    ASSERT_EQ(0, ::unlink(path.c_str()));
+  }
+  // silently discard an existing file
+  {
+    int fd = 0;
+    string message;
+    ASSERT_TRUE(::creat(path.c_str(), 0777));
+    message = asoct.bind_and_listen(path, &fd);
+    ASSERT_NE(0, fd);
+    ASSERT_EQ("", message);
+    ASSERT_EQ(0, ::close(fd));
+    ASSERT_EQ(0, ::unlink(path.c_str()));
+  }
+  // do not take over a live socket
+  {
+    ASSERT_TRUE(asoct.init(path));
+    int fd = 0;
+    string message;
+    message = asoct.bind_and_listen(path, &fd);
+    EXPECT_NE(std::string::npos, message.find("File exists"));
+    ASSERT_TRUE(asoct.shutdown());
+  }
+}
+
+int main(int argc, char **argv) {
+  vector<const char*> args;
+  argv_to_vec(argc, (const char **)argv, args);
+
+  vector<const char*> def_args;
+  global_init(&def_args, args, CEPH_ENTITY_TYPE_CLIENT, CODE_ENVIRONMENT_UTILITY, 0);
+  common_init_finish(g_ceph_context);
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}
+
 /*
  * Local Variables:
  * compile-command: "cd .. ;
  *   make unittest_admin_socket &&
  *    valgrind \
  *    --max-stackframe=20000000 --tool=memcheck \
- *   ./unittest_admin_socket # --gtest_filter=AdminSocket.*
+ *   ./unittest_admin_socket --debug-asok 20 # --gtest_filter=AdminSocket*.*
  * "
  * End:
  */