]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
initialize g_ceph_context in common_preinit
authorColin Patrick McCabe <cmccabe@alumni.cmu.edu>
Tue, 21 Jun 2011 20:00:04 +0000 (13:00 -0700)
committerColin Patrick McCabe <cmccabe@alumni.cmu.edu>
Tue, 21 Jun 2011 23:23:37 +0000 (16:23 -0700)
Initialize g_ceph_context in common_preinit rather than in a global
constructor.

Add comments to all of the major initialization functions.

We still set globals in common_preinit, for the time being.

Signed-off-by: Colin McCabe <colin.mccabe@dreamhost.com>
15 files changed:
src/common/common_init.cc
src/common/common_init.h
src/global/global_context.cc
src/global/global_init.cc
src/global/global_init.h
src/libceph.cc
src/librados.cc
src/rgw/librgw.cc
src/test/ceph_crypto.cc
src/test/crypto.cc
src/test/gather.cc
src/test/signals.cc
src/test/test_libcommon_build.cc
src/test/test_mutate.cc
src/test/unit.h [new file with mode: 0644]

index 0a01cee3b296f2bc9dc9c86da20024e5a4cd7874..8e70e018e96bd0355703ecd068a78d050e7f8e21 100644 (file)
@@ -29,7 +29,7 @@
 #define _STR(x) #x
 #define STRINGIFY(x) _STR(x)
 
-CephContext *get_global_context(void);
+void global_init_set_globals(CephContext *cct);
 
 CephContext *common_preinit(const CephInitParameters &iparams,
                          enum code_environment_t code_env, int flags)
@@ -38,8 +38,10 @@ CephContext *common_preinit(const CephInitParameters &iparams,
   g_code_env = code_env;
 
   // Create a configuration object
-  // TODO: de-globalize
-  CephContext *cct = get_global_context(); //new CephContext();
+  CephContext *cct = new CephContext(iparams.module_type);
+
+  global_init_set_globals(cct); // TODO: fix #845
+
   md_config_t *conf = cct->_conf;
   // add config observers here
 
@@ -59,7 +61,6 @@ CephContext *common_preinit(const CephInitParameters &iparams,
       conf->set_val_or_die("daemonize", "false");
       break;
   }
-  cct->set_module_type(iparams.module_type);
   return cct;
 }
 
@@ -91,3 +92,8 @@ void common_init_finish(CephContext *cct)
   ceph::crypto::init();
   cct->start_service_thread();
 }
+
+void common_destroy_context(CephContext *cct)
+{
+  //delete cct;        // TODO: fix #845
+}
index cac3ef01632d0c5e3c886a79879a733dd413a453..88ef485859a970657d16cd4aceade676c3f2a2ab 100644 (file)
@@ -36,10 +36,48 @@ enum common_init_flags_t {
   CINIT_FLAG_NO_CLOSE_STDERR = 0x4,
 };
 
+/*
+ * NOTE: If you are writing a Ceph daemon, ignore this function and call
+ * global_init instead. It will call common_preinit for you.
+ *
+ * common_preinit creates the CephContext.
+ *
+ * After this function gives you a CephContext, you need to set up the
+ * Ceph configuration, which lives inside the CephContext as md_config_t.
+ * The initial settings are not very useful because they do not reflect what
+ * the user asked for.
+ *
+ * This is usually done by something like this:
+ * cct->_conf->parse_env();
+ * cct->_conf->apply_changes();
+ *
+ * Your library may also supply functions to read a configuration file.
+ */
 CephContext *common_preinit(const CephInitParameters &iparams,
                            enum code_environment_t code_env, int flags);
+
+/* Print out some parse errors. */
 void complain_about_parse_errors(CephContext *cct,
                                 std::deque<std::string> *parse_errors);
+
+/* This function is called after you have done your last
+ * fork. When you make this call, the system will initialize everything that
+ * cannot be initialized before a fork.
+ *
+ * This includes things like starting threads, initializing libraries that
+ * can't handle forking, and so forth.
+ *
+ * If you are writing a Ceph library, you can call this pretty much any time.
+ * We do not allow our library users to fork and continue using the Ceph
+ * libraries. The most obvious reason for this is that the threads started by
+ * the Ceph libraries would be destroyed by a fork().
+ */
 void common_init_finish(CephContext *cct);
 
+/* This function is called from library code to destroy a context created by
+ * the library.
+ * You should not call this function if you called global_init.
+ */
+void common_destroy_context(CephContext *cct);
+
 #endif
index ec2545cbcf016deff2cddcb1542f0c3d08ff35ea..12527d7455c8d2e4de3836fe74bcf46f5288285f 100644 (file)
 /*
  * Global variables for use from process context.
  */
-CephContext *g_ceph_context = new CephContext(0);
-md_config_t *g_conf(g_ceph_context->_conf);
-DoutStreambuf <char, std::basic_string<char>::traits_type> *_doss(g_ceph_context->_doss);
-
-CephContext *get_global_context(void)
-{
-  return g_ceph_context;
-}
+CephContext *g_ceph_context = NULL;
+md_config_t *g_conf = NULL;
+DoutStreambuf <char, std::basic_string<char>::traits_type> *_doss = NULL;
index d73972ca1620af33c5dae6b6c3c16141d6071499..22930e4ffd13e20b35aa7cee7c3abe82dbaeb214 100644 (file)
@@ -22,6 +22,7 @@
 #include "common/safe_io.h"
 #include "common/signal.h"
 #include "common/version.h"
+#include "global/global_init.h"
 #include "global/pidfile.h"
 #include "global/signal_handler.h"
 #include "include/color.h"
 #include <errno.h>
 #include <deque>
 
+void global_init_set_globals(CephContext *cct)
+{
+  g_ceph_context = cct;
+  g_conf = cct->_conf;
+  _doss = g_ceph_context->_doss;
+}
+
 static void output_ceph_version()
 {
   char buf[1024];
@@ -41,9 +49,13 @@ static void output_ceph_version()
 void global_init(std::vector < const char* >& args,
               uint32_t module_type, code_environment_t code_env, int flags)
 {
+  // You can only call global_init once.
+  assert(!g_ceph_context);
+
   CephInitParameters iparams =
     ceph_argparse_early_args(args, module_type, flags);
   CephContext *cct = common_preinit(iparams, code_env, flags);
+  global_init_set_globals(cct);
   md_config_t *conf = cct->_conf;
 
   std::deque<std::string> parse_errors;
@@ -105,23 +117,6 @@ static void pidfile_remove_void(void)
   pidfile_remove();
 }
 
-/* Map stderr to /dev/null. This isn't really re-entrant; we rely on the old unix
- * behavior that the file descriptor that gets assigned is the lowest
- * available one.
- */
-int global_init_shutdown_stderr(CephContext *cct)
-{
-  TEMP_FAILURE_RETRY(close(STDERR_FILENO));
-  if (open("/dev/null", O_RDONLY) < 0) {
-    int err = errno;
-    derr << "global_init_shutdown_stderr: open(/dev/null) failed: error "
-        << err << dendl;
-    return 1;
-  }
-  cct->_doss->handle_stderr_shutdown();
-  return 0;
-}
-
 void global_init_daemonize(CephContext *cct, int flags)
 {
   if (g_code_env != CODE_ENVIRONMENT_DAEMON)
@@ -200,3 +195,21 @@ void global_init_chdir(const CephContext *cct)
         << conf->chdir << "': " << cpp_strerror(err) << dendl;
   }
 }
+
+/* Map stderr to /dev/null. This isn't really re-entrant; we rely on the old unix
+ * behavior that the file descriptor that gets assigned is the lowest
+ * available one.
+ */
+int global_init_shutdown_stderr(CephContext *cct)
+{
+  TEMP_FAILURE_RETRY(close(STDERR_FILENO));
+  if (open("/dev/null", O_RDONLY) < 0) {
+    int err = errno;
+    derr << "global_init_shutdown_stderr: open(/dev/null) failed: error "
+        << err << dendl;
+    return 1;
+  }
+  cct->_doss->handle_stderr_shutdown();
+  return 0;
+}
+
index 6c08881e05fb6ee3ddbcb1fbb7b728b4e5f9c6f7..68d07e20cb53c4930ab2009a4d4d32c83a52f5c1 100644 (file)
 
 class CephContext;
 
+/*
+ * global_init is the first initialization function that
+ * daemons and utility programs need to call. It takes care of a lot of
+ * initialization, including setting up g_ceph_context.
+ */
 void global_init(std::vector < const char* >& args,
               uint32_t module_type, code_environment_t code_env, int flags);
-int global_init_shutdown_stderr(CephContext *cct);
+
+/*
+ * global_init_daemonize handles daemonizing a process. 
+ *
+ * If this is called, it *must* be called before common_init_finish
+ */
 void global_init_daemonize(CephContext *cct, int flags);
+
+/*
+ * global_init_chdir changes the process directory.
+ *
+ * If this is called, it *must* be called before common_init_finish
+ */
 void global_init_chdir(const CephContext *cct);
 
+/*
+ * Explicitly shut down stderr. Usually, you don't need to do
+ * this, because global_init_daemonize will do it for you. However, in some
+ * rare cases you need to call this explicitly.
+ *
+ * If this is called, it *must* be called before common_init_finish
+ */
+int global_init_shutdown_stderr(CephContext *cct);
+
+/*
+ * NOTE: you probably don't need to use this function. Use global_init instead!
+ *
+ * Explicitly set up the process globals: g_ceph_context, g_conf, and _doss.
+ */
+void global_init_set_globals(CephContext *cct);
+
 #endif
index a99f6b19a6781834fb5f0ace12732d3109bf4148..65076cfe9cb47122c20d0d1bcede7edf88bee22f 100644 (file)
 #include <string.h>
 #include <string>
 
-static Mutex libceph_init_mutex("libceph_init_mutex");
-static bool libceph_initialized = false; // FIXME! remove this
-static int nonce_seed = 0;
-
 class ceph_mount_info
 {
 public:
@@ -49,11 +45,10 @@ public:
   {
     try {
       shutdown();
-    // uncomment once conf is de-globalized
-//    if (conf) {
-//      free(conf);
-//      conf = NULL;
-//    }
+      if (cct) {
+       common_destroy_context(cct);
+       cct = NULL;
+      }
     }
     catch (const std::exception& e) {
       // we shouldn't get here, but if we do, we want to know about it.
@@ -215,10 +210,11 @@ extern "C" const char *ceph_version(int *pmajor, int *pminor, int *ppatch)
   return VERSION;
 }
 
-static int ceph_create_with_context_impl(struct ceph_mount_info **cmount, CephContext *cct)
+extern "C" int ceph_create_with_context(struct ceph_mount_info **cmount, CephContext *cct)
 {
-  // should hold libceph_init_mutex here
-  libceph_initialized = true;
+  // Function-static variables are thread-safe in gcc and in the forthcoming C++ standard
+  static int nonce_seed = 0;
+
   uint64_t nonce = (uint64_t)++nonce_seed * 1000000ull + (uint64_t)getpid();
   *cmount = new struct ceph_mount_info(nonce, cct);
   return 0;
@@ -226,32 +222,16 @@ static int ceph_create_with_context_impl(struct ceph_mount_info **cmount, CephCo
 
 extern "C" int ceph_create(struct ceph_mount_info **cmount, const char * const id)
 {
-  int ret;
-  libceph_init_mutex.Lock();
-  CephContext *cct = g_ceph_context;
-  if (!libceph_initialized) {
-    CephInitParameters iparams(CEPH_ENTITY_TYPE_CLIENT, CEPH_CONF_FILE_DEFAULT);
-    iparams.conf_file = "";
-    if (id) {
-      iparams.name.set(CEPH_ENTITY_TYPE_CLIENT, id);
-    }
-
-    cct = common_preinit(iparams, CODE_ENVIRONMENT_LIBRARY, 0);
-    cct->_conf->parse_env(); // environment variables coverride
-    cct->_conf->apply_changes();
+  CephInitParameters iparams(CEPH_ENTITY_TYPE_CLIENT, CEPH_CONF_FILE_DEFAULT);
+  iparams.conf_file = "";
+  if (id) {
+    iparams.name.set(CEPH_ENTITY_TYPE_CLIENT, id);
   }
-  ret = ceph_create_with_context_impl(cmount, cct);
-  libceph_init_mutex.Unlock();
-  return ret;
-}
 
-extern "C" int ceph_create_with_context(struct ceph_mount_info **cmount, CephContext *cct)
-{
-  int ret;
-  libceph_init_mutex.Lock();
-  ret = ceph_create_with_context_impl(cmount, cct);
-  libceph_init_mutex.Unlock();
-  return ret;
+  CephContext *cct = common_preinit(iparams, CODE_ENVIRONMENT_LIBRARY, 0);
+  cct->_conf->parse_env(); // environment variables coverride
+  cct->_conf->apply_changes();
+  return ceph_create_with_context(cmount, cct);
 }
 
 extern "C" void ceph_shutdown(struct ceph_mount_info *cmount)
index 1cf58cd374d5e78a20048aa595ab6592d9e679b2..e9e05bc421da41eac40b497fb285b729e4842580 100644 (file)
@@ -895,6 +895,8 @@ librados::RadosClient::
     messenger->destroy();
   if (objecter)
     delete objecter;
+  common_destroy_context(cct);
+  cct = NULL;
 }
 
 
@@ -3102,32 +3104,19 @@ librados::ObjectOperation::~ObjectOperation()
 
 
 ///////////////////////////// C API //////////////////////////////
-static Mutex rados_init_mutex("rados_init");
-static int rados_initialized = 0;
-
 extern "C" int rados_create(rados_t *pcluster, const char * const id)
 {
-  rados_init_mutex.Lock();
-  CephContext *cct_;
-  // FIXME: get rid of this mutex and the reference to the global
-  if (!rados_initialized) {
-    CephInitParameters iparams(CEPH_ENTITY_TYPE_CLIENT, CEPH_CONF_FILE_DEFAULT);
-    iparams.conf_file = "";
-    if (id) {
-      iparams.name.set(CEPH_ENTITY_TYPE_CLIENT, id);
-    }
+  CephInitParameters iparams(CEPH_ENTITY_TYPE_CLIENT, CEPH_CONF_FILE_DEFAULT);
+  iparams.conf_file = "";
+  if (id) {
+    iparams.name.set(CEPH_ENTITY_TYPE_CLIENT, id);
+  }
 
-    cct_ = common_preinit(iparams, CODE_ENVIRONMENT_LIBRARY, 0);
-    cct_->_conf->parse_env(); // environment variables override
-    cct_->_conf->apply_changes();
+  CephContext *cct = common_preinit(iparams, CODE_ENVIRONMENT_LIBRARY, 0);
+  cct->_conf->parse_env(); // environment variables override
+  cct->_conf->apply_changes();
 
-    ++rados_initialized;
-  }
-  else {
-    cct_ = g_ceph_context;
-  }
-  rados_init_mutex.Unlock();
-  librados::RadosClient *radosp = new librados::RadosClient(cct_);
+  librados::RadosClient *radosp = new librados::RadosClient(cct);
   *pcluster = (void *)radosp;
   return 0;
 }
@@ -3136,17 +3125,9 @@ extern "C" int rados_create(rados_t *pcluster, const char * const id)
  * already called global_init and want to use that particular configuration for
  * their cluster.
  */
-extern "C" int rados_create_with_context(rados_t *pcluster, CephContext *cct_)
+extern "C" int rados_create_with_context(rados_t *pcluster, CephContext *cct)
 {
-  rados_init_mutex.Lock();
-  if (!rados_initialized) {
-    ++rados_initialized;
-    /* This is a no-op now. g_ceph_context is still global in libcommon and we
-     * can't actually do anything useful with the provided conf pointer.
-     */
-  }
-  rados_init_mutex.Unlock();
-  librados::RadosClient *radosp = new librados::RadosClient(cct_);
+  librados::RadosClient *radosp = new librados::RadosClient(cct);
   *pcluster = (void *)radosp;
   return 0;
 }
index 66d305dd479bd5470c1fb37c0683782efe894d28..16cb3c92d5fdc903e22cbd785a0d3108c7b5d691 100644 (file)
 
 #define RGW_LOG(x) pdout(x, g_conf->rgw_log)
 
-static Mutex librgw_init_mutex("librgw_init");
-static int librgw_initialized = 0;
-
 int librgw_create(librgw_t *rgw, const char * const id)
 {
-  librgw_init_mutex.Lock();
-  CephContext *cct;
-  if (!librgw_initialized) {
-    CephInitParameters iparams(CEPH_ENTITY_TYPE_CLIENT, CEPH_CONF_FILE_DEFAULT);
-    iparams.conf_file = "";
-    if (id) {
-      iparams.name.set(CEPH_ENTITY_TYPE_CLIENT, id);
-    }
-    CephContext *cct = common_preinit(iparams, CODE_ENVIRONMENT_LIBRARY, 0);
-    cct->_conf->log_to_stderr = 1; // quiet by default
-    cct->_conf->parse_env(); // environment variables override
-    cct->_conf->apply_changes();
-
-    ++librgw_initialized;
-    common_init_finish(cct);
+  CephInitParameters iparams(CEPH_ENTITY_TYPE_CLIENT, CEPH_CONF_FILE_DEFAULT);
+  iparams.conf_file = "";
+  if (id) {
+    iparams.name.set(CEPH_ENTITY_TYPE_CLIENT, id);
   }
-  else {
-    cct = g_ceph_context;
-  }
-  librgw_init_mutex.Unlock();
-  *rgw = g_ceph_context;
+  CephContext *cct = common_preinit(iparams, CODE_ENVIRONMENT_LIBRARY, 0);
+  cct->_conf->log_to_stderr = 1; // quiet by default
+  cct->_conf->parse_env(); // environment variables override
+  cct->_conf->apply_changes();
+
+  common_init_finish(cct);
+  *rgw = cct;
   return 0;
 }
 
@@ -141,5 +129,5 @@ void librgw_free_bin(librgw_t rgw, char *bin)
 
 void librgw_shutdown(librgw_t rgw)
 {
-  // TODO: free configuration, etc.
+  common_init_finish(rgw);
 }
index 4c3177a564c2e9b7a333ebb604cdbc9cf3884018..403f6b5400b58cb8a2367e756770b7cfe3112a4a 100644 (file)
@@ -1,6 +1,6 @@
 #include "common/ceph_crypto.h"
 
-#include "gtest/gtest.h"
+#include "test/unit.h"
 
 class CryptoEnvironment: public ::testing::Environment {
 public:
index e1348ed234f2c0b46b19401fae65eafe03ca100d..7c56260b31063922d11e388bef15d582d98bedca 100644 (file)
@@ -5,7 +5,7 @@
 #include "auth/Crypto.h"
 #include "common/ceph_crypto.h"
 
-#include "gtest/gtest.h"
+#include "test/unit.h"
 
 class CryptoEnvironment: public ::testing::Environment {
 public:
index f6b4e629354393184958a2243b3e85000720ff78..feb517628a89830095c34716799d253c74f330bf 100644 (file)
@@ -11,9 +11,8 @@
  * Foundation.  See file COPYING.
  * 
  */
-#include "common/config.h"
 #include "include/Context.h"
-#include "gtest/gtest.h"
+#include "test/unit.h"
 
 class C_Checker : public Context {
 public:
@@ -25,7 +24,7 @@ public:
 };
 
 TEST(ContextGather, Constructor) {
-  C_Gather *gather = new C_Gather(g_ceph_context );
+  C_Gather *gather = new C_Gather(g_ceph_context);
   EXPECT_TRUE(gather->empty());
   EXPECT_EQ(0, gather->get_num());
   EXPECT_EQ(0, gather->get_num_remaining());
index 248325420a798e105e7f3759c040b0611674545e..d0757973a2c44c1a329f899c04bee488875198cf 100644 (file)
@@ -2,7 +2,7 @@
 #include "common/signal.h"
 #include "global/signal_handler.h"
 
-#include "gtest/gtest.h"
+#include "test/unit.h"
 
 #include <errno.h>
 #include <signal.h>
index dc7c554e7836fbd6b6a5f0e1866c1f40af947a20..b86a996d165ca9f3c80bd615c9637fc8d7cabd0f 100644 (file)
@@ -1,12 +1,12 @@
 #include "common/common_init.h"
 
-CephContext *get_global_context(void)
+void global_init_set_globals(CephContext *cct)
 {
-  return NULL;
+  // do nothing
 }
 
 /* This program exists to test that we can build libcommon without
- * referencing g_ceph_context->
+ * referencing g_ceph_context
  *
  * This program will go away as soon as we actually don't use g_ceph_context in
  * more programs. Obviously, at that point, those programs will provide an
index 8ebb44176387d628f7ec50627e8afdba65447f37..c6fb6acdc4d1c911b9e0ef44530bf1a2622fd237 100644 (file)
  * Test Ioctx::operate
  */
 
-#include "include/types.h"
-#include "include/rados/librados.hpp"
 #include "common/ceph_argparse.h"
-#include "global/global_init.h"
 #include "common/config.h"
+#include "global/global_init.h"
+#include "include/rados/librados.hpp"
+#include "include/types.h"
 
 #include <errno.h>
 #include <iostream>
diff --git a/src/test/unit.h b/src/test/unit.h
new file mode 100644 (file)
index 0000000..4072d69
--- /dev/null
@@ -0,0 +1,43 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+/*
+ * Ceph - scalable distributed file system
+ *
+ * Copyright (C) 2011 New Dream Network
+ *
+ * This is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License version 2.1, as published by the Free Software
+ * Foundation.  See file COPYING.
+ *
+ */
+
+#ifndef CEPH_UNIT_TEST_H
+#define CEPH_UNIT_TEST_H
+
+#include "include/types.h" // FIXME: ordering shouldn't be important, but right 
+                           // now, this include has to come before the others.
+
+#include "common/code_environment.h"
+#include "global/global_context.h"
+#include "global/global_init.h"
+#include "include/msgr.h" // for CEPH_ENTITY_TYPE_CLIENT
+#include "gtest/gtest.h"
+
+#include <vector>
+
+/*
+ * You only need to include this file if you are testing Ceph internal code. If
+ * you are testing library code, the library init() interfaces will handle
+ * initialization for you.
+ */
+int main(int argc, char **argv) {
+  std::vector<const char*> args;
+  global_init(args, CEPH_ENTITY_TYPE_CLIENT, CODE_ENVIRONMENT_UTILITY,
+             CINIT_FLAG_NO_DEFAULT_CONFIG_FILE);
+  common_init_finish(g_ceph_context);
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}
+
+#endif