From 3f5fc965b2682289ef5cad7d7e053d7bd1b6b39a Mon Sep 17 00:00:00 2001 From: Loic Dachary Date: Sat, 15 Feb 2014 11:43:13 +0100 Subject: [PATCH] common: ping existing admin socket before unlink When a daemon initializes it tries to create an admin socket and unlinks any pre-existing file, regardless. If such a file is in use, it causes the existing daemon to loose its admin socket. The AdminSocketClient::ping is implemented to probe an existing socket, using the "0" message. The AdminSocket::bind_and_listen function is modified to call ping() on when it finds existing file. It unlinks the file only if the ping fails. http://tracker.ceph.com/issues/7188 fixes: #7188 Backport: emperor, dumpling Reviewed-by: Sage Weil Signed-off-by: Loic Dachary (cherry picked from commit 45600789f1ca399dddc5870254e5db883fb29b38) --- src/common/admin_socket.cc | 25 +++--- src/common/admin_socket_client.cc | 8 ++ src/common/admin_socket_client.h | 1 + src/test/admin_socket.cc | 125 +++++++++++++++++++++++++++++- 4 files changed, 147 insertions(+), 12 deletions(-) diff --git a/src/common/admin_socket.cc b/src/common/admin_socket.cc index dc208c30f643d..1190b6d7ff8bf 100644 --- a/src/common/admin_socket.cc +++ b/src/common/admin_socket.cc @@ -16,6 +16,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" @@ -184,15 +185,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) { diff --git a/src/common/admin_socket_client.cc b/src/common/admin_socket_client.cc index 335695f9b4ba8..8750560289594 100644 --- a/src/common/admin_socket_client.cc +++ b/src/common/admin_socket_client.cc @@ -127,6 +127,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; diff --git a/src/common/admin_socket_client.h b/src/common/admin_socket_client.h index 45654bbf5f7bf..b46ed24fff13d 100644 --- a/src/common/admin_socket_client.h +++ b/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; }; diff --git a/src/test/admin_socket.cc b/src/test/admin_socket.cc index 6484740674f6b..248f24ecc5c37 100644 --- a/src/test/admin_socket.cc +++ b/src/test/admin_socket.cc @@ -12,11 +12,15 @@ * */ +#include + #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 #include @@ -33,6 +37,9 @@ public: 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 + 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 + 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 args; + argv_to_vec(argc, (const char **)argv, args); + + vector 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: */ -- 2.39.5