From df2e3bcb2ac790e179e97f6b8017b6fa6a8087bf Mon Sep 17 00:00:00 2001 From: Colin Patrick McCabe Date: Tue, 21 Jun 2011 13:00:04 -0700 Subject: [PATCH] initialize g_ceph_context in common_preinit 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 --- src/common/common_init.cc | 14 ++++++--- src/common/common_init.h | 38 +++++++++++++++++++++++ src/global/global_context.cc | 11 ++----- src/global/global_init.cc | 47 ++++++++++++++++++----------- src/global/global_init.h | 34 ++++++++++++++++++++- src/libceph.cc | 52 ++++++++++---------------------- src/librados.cc | 45 ++++++++------------------- src/rgw/librgw.cc | 36 ++++++++-------------- src/test/ceph_crypto.cc | 2 +- src/test/crypto.cc | 2 +- src/test/gather.cc | 5 ++- src/test/signals.cc | 2 +- src/test/test_libcommon_build.cc | 6 ++-- src/test/test_mutate.cc | 6 ++-- src/test/unit.h | 43 ++++++++++++++++++++++++++ 15 files changed, 209 insertions(+), 134 deletions(-) create mode 100644 src/test/unit.h diff --git a/src/common/common_init.cc b/src/common/common_init.cc index 0a01cee3b296f..8e70e018e96bd 100644 --- a/src/common/common_init.cc +++ b/src/common/common_init.cc @@ -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 +} diff --git a/src/common/common_init.h b/src/common/common_init.h index cac3ef01632d0..88ef485859a97 100644 --- a/src/common/common_init.h +++ b/src/common/common_init.h @@ -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 *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 diff --git a/src/global/global_context.cc b/src/global/global_context.cc index ec2545cbcf016..12527d7455c8d 100644 --- a/src/global/global_context.cc +++ b/src/global/global_context.cc @@ -18,11 +18,6 @@ /* * Global variables for use from process context. */ -CephContext *g_ceph_context = new CephContext(0); -md_config_t *g_conf(g_ceph_context->_conf); -DoutStreambuf ::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 ::traits_type> *_doss = NULL; diff --git a/src/global/global_init.cc b/src/global/global_init.cc index d73972ca1620a..22930e4ffd13e 100644 --- a/src/global/global_init.cc +++ b/src/global/global_init.cc @@ -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" @@ -29,6 +30,13 @@ #include #include +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 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; +} + diff --git a/src/global/global_init.h b/src/global/global_init.h index 6c08881e05fb6..68d07e20cb53c 100644 --- a/src/global/global_init.h +++ b/src/global/global_init.h @@ -25,10 +25,42 @@ 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 diff --git a/src/libceph.cc b/src/libceph.cc index a99f6b19a6781..65076cfe9cb47 100644 --- a/src/libceph.cc +++ b/src/libceph.cc @@ -28,10 +28,6 @@ #include #include -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) diff --git a/src/librados.cc b/src/librados.cc index 1cf58cd374d5e..e9e05bc421da4 100644 --- a/src/librados.cc +++ b/src/librados.cc @@ -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; } diff --git a/src/rgw/librgw.cc b/src/rgw/librgw.cc index 66d305dd479bd..16cb3c92d5fdc 100644 --- a/src/rgw/librgw.cc +++ b/src/rgw/librgw.cc @@ -26,32 +26,20 @@ #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); } diff --git a/src/test/ceph_crypto.cc b/src/test/ceph_crypto.cc index 4c3177a564c2e..403f6b5400b58 100644 --- a/src/test/ceph_crypto.cc +++ b/src/test/ceph_crypto.cc @@ -1,6 +1,6 @@ #include "common/ceph_crypto.h" -#include "gtest/gtest.h" +#include "test/unit.h" class CryptoEnvironment: public ::testing::Environment { public: diff --git a/src/test/crypto.cc b/src/test/crypto.cc index e1348ed234f2c..7c56260b31063 100644 --- a/src/test/crypto.cc +++ b/src/test/crypto.cc @@ -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: diff --git a/src/test/gather.cc b/src/test/gather.cc index f6b4e62935439..feb517628a898 100644 --- a/src/test/gather.cc +++ b/src/test/gather.cc @@ -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()); diff --git a/src/test/signals.cc b/src/test/signals.cc index 248325420a798..d0757973a2c44 100644 --- a/src/test/signals.cc +++ b/src/test/signals.cc @@ -2,7 +2,7 @@ #include "common/signal.h" #include "global/signal_handler.h" -#include "gtest/gtest.h" +#include "test/unit.h" #include #include diff --git a/src/test/test_libcommon_build.cc b/src/test/test_libcommon_build.cc index dc7c554e7836f..b86a996d165ca 100644 --- a/src/test/test_libcommon_build.cc +++ b/src/test/test_libcommon_build.cc @@ -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 diff --git a/src/test/test_mutate.cc b/src/test/test_mutate.cc index 8ebb44176387d..c6fb6acdc4d1c 100644 --- a/src/test/test_mutate.cc +++ b/src/test/test_mutate.cc @@ -16,11 +16,11 @@ * 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 #include diff --git a/src/test/unit.h b/src/test/unit.h new file mode 100644 index 0000000000000..4072d69870534 --- /dev/null +++ b/src/test/unit.h @@ -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 + +/* + * 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 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 -- 2.39.5