Merge pull request ceph#5425 from ceph/wip-12479-next

librbd: test crash during client upgrade suite

Reviewed-by: Josh Durgin <[email protected]>

src/common/ceph_context.cc

@@ -20,6 +20,7 @@
 #include "common/perf_counters.h"
 #include "common/Thread.h"
 #include "common/ceph_context.h"
+#include "common/ceph_crypto.h"
 #include "common/config.h"
 #include "common/debug.h"
 #include "common/HeartbeatMap.h"
@@ -39,6 +40,41 @@
 
 using ceph::HeartbeatMap;
 
+namespace {
+
+class LockdepObs : public md_config_obs_t {
+public:
+  LockdepObs(CephContext *cct) : m_cct(cct), m_registered(false) {
+  }
+  virtual ~LockdepObs() {
+    if (m_registered) {
+      lockdep_unregister_ceph_context(m_cct);
+    }
+  }
+
+  const char** get_tracked_conf_keys() const {
+    static const char *KEYS[] = {"lockdep", NULL};
+    return KEYS;
+  }
+
+  void handle_conf_change(const md_config_t *conf,
+                          const std::set <std::string> &changed) {
+    if (conf->lockdep && !m_registered) {
+      lockdep_register_ceph_context(m_cct);
+      m_registered = true;
+    } else if (!conf->lockdep && m_registered) {
+      lockdep_unregister_ceph_context(m_cct);
+      m_registered = false;
+    }
+  }
+private:
+  CephContext *m_cct;
+  bool m_registered;
+};
+
+
+} // anonymous namespace
+
 class CephContextServiceThread : public Thread
 {
 public:
@@ -370,7 +406,8 @@ CephContext::CephContext(uint32_t module_type_)
     _perf_counters_conf_obs(NULL),
     _heartbeat_map(NULL),
     _crypto_none(NULL),
-    _crypto_aes(NULL)
+    _crypto_aes(NULL),
+    _lockdep_obs(NULL)
 {
   ceph_spin_init(&_service_thread_lock);
   ceph_spin_init(&_associated_objs_lock);
@@ -385,6 +422,9 @@ CephContext::CephContext(uint32_t module_type_)
   _cct_obs = new CephContextObs(this);
   _conf->add_observer(_cct_obs);
 
+  _lockdep_obs = new LockdepObs(this);
+  _conf->add_observer(_lockdep_obs);
+
   _perf_counters_collection = new PerfCountersCollection(this);
   _admin_socket = new AdminSocket(this);
   _heartbeat_map = new HeartbeatMap(this);
@@ -419,10 +459,6 @@ CephContext::~CephContext()
        it != _associated_objs.end(); ++it)
     delete it->second;
 
-  if (_conf->lockdep) {
-    lockdep_unregister_ceph_context(this);
-  }
-
   _admin_socket->unregister_command("perfcounters_dump");
   _admin_socket->unregister_command("perf dump");
   _admin_socket->unregister_command("1");
@@ -456,6 +492,10 @@ CephContext::~CephContext()
   delete _cct_obs;
   _cct_obs = NULL;
 
+  _conf->remove_observer(_lockdep_obs);
+  delete _lockdep_obs;
+  _lockdep_obs = NULL;
+
   _log->stop();
   delete _log;
   _log = NULL;
@@ -467,6 +507,7 @@ CephContext::~CephContext()
 
   delete _crypto_none;
   delete _crypto_aes;
+  ceph::crypto::shutdown();
 }
 
 void CephContext::start_service_thread()

src/common/ceph_context.h

@@ -171,6 +171,8 @@ class CephContext {
   ceph_spinlock_t _feature_lock;
   std::set<std::string> _experimental_features;
 
+  md_config_obs_t *_lockdep_obs;
+
   friend class CephContextObs;
 };
 

src/common/ceph_crypto.cc

@@ -12,6 +12,7 @@
  *
  */
 
+#include "include/int_types.h"
 #include "common/config.h"
 #include "common/ceph_context.h"
 #include "ceph_crypto.h"
@@ -40,37 +41,46 @@ ceph::crypto::HMACSHA1::~HMACSHA1()
 // for SECMOD_RestartModules()
 #include <secmod.h>
 
-// Initialization of NSS requires a mutex due to a race condition in
-// NSS_NoDB_Init.
 static pthread_mutex_t crypto_init_mutex = PTHREAD_MUTEX_INITIALIZER;
+static uint32_t crypto_refs = 0;
+static NSSInitContext *crypto_context = NULL;
 static pid_t crypto_init_pid = 0;
 
 void ceph::crypto::init(CephContext *cct)
 {
   pid_t pid = getpid();
-  SECStatus s;
   pthread_mutex_lock(&crypto_init_mutex);
   if (crypto_init_pid != pid) {
-    if (crypto_init_pid > 0)
+    if (crypto_init_pid > 0) {
       SECMOD_RestartModules(PR_FALSE);
+    }
     crypto_init_pid = pid;
   }
-  if (cct->_conf->nss_db_path.empty()) {
-    s = NSS_NoDB_Init(NULL);
-  } else {
-    s = NSS_Init(cct->_conf->nss_db_path.c_str());
+
+  if (++crypto_refs == 1) {
+    NSSInitParameters init_params;
+    memset(&init_params, 0, sizeof(init_params));
+    init_params.length = sizeof(init_params);
+
+    uint32_t flags = NSS_INIT_READONLY;
+    if (cct->_conf->nss_db_path.empty()) {
+      flags |= (NSS_INIT_NOCERTDB | NSS_INIT_NOMODDB);
+    }
+    crypto_context = NSS_InitContext(cct->_conf->nss_db_path.c_str(), "", "",
+                                     SECMOD_DB, &init_params, flags);
   }
   pthread_mutex_unlock(&crypto_init_mutex);
-  assert(s == SECSuccess);
+  assert(crypto_context != NULL);
 }
 
 void ceph::crypto::shutdown()
 {
-  SECStatus s;
   pthread_mutex_lock(&crypto_init_mutex);
-  s = NSS_Shutdown();
-  assert(s == SECSuccess);
-  crypto_init_pid = 0;
+  if (--crypto_refs == 0) {
+    NSS_ShutdownContext(crypto_context);
+    crypto_context = NULL;
+    crypto_init_pid = 0;
+  }
   pthread_mutex_unlock(&crypto_init_mutex);
 }
 

src/common/common_init.cc

@@ -119,9 +119,4 @@ void common_init_finish(CephContext *cct, int flags)
 
   if (!(flags & CINIT_FLAG_NO_DAEMON_ACTIONS))
     cct->start_service_thread();
-
-  if (cct->_conf->lockdep) {
-    g_lockdep = true;
-    lockdep_register_ceph_context(cct);
-  }
 }

src/common/lockdep.cc

@@ -67,6 +67,7 @@ void lockdep_register_ceph_context(CephContext *cct)
 {
   pthread_mutex_lock(&lockdep_mutex);
   if (g_lockdep_ceph_ctx == NULL) {
+    g_lockdep = true;
     g_lockdep_ceph_ctx = cct;
     lockdep_dout(0) << "lockdep start" << dendl;
 

src/global/global_init.cc

@@ -120,8 +120,6 @@ void global_init(std::vector < const char * > *alt_def_args,
 {
   global_pre_init(alt_def_args, args, module_type, code_env, flags);
 
-  g_lockdep = g_ceph_context->_conf->lockdep;
-
   // signal stuff
   int siglist[] = { SIGPIPE, 0 };
   block_signals(siglist, NULL);
@@ -142,9 +140,6 @@ void global_init(std::vector < const char * > *alt_def_args,
     }
   }
 
-  if (g_lockdep) {
-    lockdep_register_ceph_context(g_ceph_context);
-  }
   register_assert_context(g_ceph_context);
 
   // call all observers now.  this has the side-effect of configuring

src/rgw/rgw_main.cc

@@ -1260,8 +1260,6 @@ int main(int argc, const char **argv)
   dout(1) << "final shutdown" << dendl;
   g_ceph_context->put();
 
-  ceph::crypto::shutdown();
-
   signal_fd_finalize();
 
   return 0;

src/test/Makefile-client.am

@@ -337,8 +337,7 @@ ceph_test_librbd_api_SOURCES = \
 	test/librbd/test_main.cc
 ceph_test_librbd_api_CXXFLAGS = $(UNITTEST_CXXFLAGS)
 ceph_test_librbd_api_LDADD = \
-	$(LIBRBD) $(LIBRADOS) $(UNITTEST_LDADD) \
-	$(CEPH_GLOBAL) $(RADOS_TEST_LDADD)
+	$(LIBRBD) $(LIBRADOS) $(LIBCOMMON) $(UNITTEST_LDADD) $(RADOS_TEST_LDADD)
 bin_DEBUGPROGRAMS += ceph_test_librbd_api
 
 if WITH_LTTNG

src/test/librados_test_stub/LibradosTestStub.cc

@@ -113,6 +113,14 @@ extern "C" rados_config_t rados_cct(rados_t cluster)
   return reinterpret_cast<rados_config_t>(client->cct());
 }
 
+extern "C" int rados_conf_set(rados_t cluster, const char *option,
+                              const char *value) {
+  librados::TestRadosClient *impl =
+    reinterpret_cast<librados::TestRadosClient*>(cluster);
+  CephContext *cct = impl->cct();
+  return cct->_conf->set_val(option, value);
+}
+
 extern "C" int rados_conf_parse_env(rados_t cluster, const char *var) {
   librados::TestRadosClient *client =
     reinterpret_cast<librados::TestRadosClient*>(cluster);
@@ -205,6 +213,12 @@ extern "C" void rados_ioctx_destroy(rados_ioctx_t io) {
   ctx->put();
 }
 
+extern "C" rados_t rados_ioctx_get_cluster(rados_ioctx_t io) {
+  librados::TestIoCtxImpl *ctx =
+    reinterpret_cast<librados::TestIoCtxImpl*>(io);
+  return reinterpret_cast<rados_t>(ctx->get_rados_client());
+}
+
 extern "C" int rados_mon_command(rados_t cluster, const char **cmd,
                                  size_t cmdlen, const char *inbuf,
                                  size_t inbuflen, char **outbuf,
@@ -729,6 +743,31 @@ int Rados::blacklist_add(const std::string& client_address,
   return impl->blacklist_add(client_address, expire_seconds);
 }
 
+config_t Rados::cct() {
+  TestRadosClient *impl = reinterpret_cast<TestRadosClient*>(client);
+  return reinterpret_cast<config_t>(impl->cct());
+}
+
+int Rados::conf_set(const char *option, const char *value) {
+  return rados_conf_set(reinterpret_cast<rados_t>(client), option, value);
+}
+
+int Rados::conf_get(const char *option, std::string &val) {
+  TestRadosClient *impl = reinterpret_cast<TestRadosClient*>(client);
+  CephContext *cct = impl->cct();
+
+  char *str = NULL;
+  int ret = cct->_conf->get_val(option, &str, -1);
+  if (ret != 0) {
+    free(str);
+    return ret;
+  }
+
+  val = str;
+  free(str);
+  return 0;
+}
+
 int Rados::conf_parse_env(const char *env) const {
   return rados_conf_parse_env(reinterpret_cast<rados_t>(client), env);
 }

src/test/librbd/fsx.cc

@@ -42,7 +42,6 @@
 #include "include/krbd.h"
 #include "include/rados/librados.h"
 #include "include/rbd/librbd.h"
-#include "common/ceph_crypto.h"
 
 #define NUMPRINTCOLUMNS 32	/* # columns of data to print on each line */
 
@@ -2333,7 +2332,6 @@ main(int argc, char **argv)
 	krbd_destroy(krbd);
 	rados_shutdown(cluster);
 
-        ceph::crypto::shutdown();
 	free(original_buf);
 	free(good_buf);
 	free(temp_buf);