From: Sage Weil <sage.weil@dreamhost.com>
Date: Sun, 22 Apr 2012 21:35:39 +0000 (-0700)
Subject: log: do not set on_exit() callback for libraries
X-Git-Tag: v0.47~83^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=c4c59a094d2e355d5bbe90c6dc7e92473a35380d;p=ceph.git

log: do not set on_exit() callback for libraries

Set this up in either global_init() or common_init_finish(), both opportune
times that occur after config parsing has happened and the user has the
option to modify this behavior.  The exception would be libraries like
librados, which can't use rados_conf_* to enable this.  Arguably flush
functionality should be exposed through the librados API directly, instead
of futzing with on_exit().

Signed-off-by: Sage Weil <sage.weil@dreamhost.com>
---

diff --git a/src/common/common_init.cc b/src/common/common_init.cc
index 6562ae910eae..e4617ea9af75 100644
--- a/src/common/common_init.cc
+++ b/src/common/common_init.cc
@@ -58,6 +58,7 @@ CephContext *common_preinit(const CephInitParameters &iparams,
   case CODE_ENVIRONMENT_LIBRARY:
     conf->set_val_or_die("log_to_stderr", "false");
     conf->set_val_or_die("err_to_stderr", "false");
+    conf->set_val_or_die("log_flush_on_exit", "false");
     break;
 
   default:
@@ -103,6 +104,9 @@ void common_init_finish(CephContext *cct)
   ceph::crypto::init();
   cct->start_service_thread();
 
+  if (cct->_conf->log_flush_on_exit)
+    cct->_log->set_flush_on_exit();
+
   // Trigger callbacks on any config observers that were waiting for
   // it to become safe to start threads.
   cct->_conf->set_val("internal_safe_to_start_threads", "true");
diff --git a/src/common/config_opts.h b/src/common/config_opts.h
index 75d7c16d7b3d..7643cda9839c 100644
--- a/src/common/config_opts.h
+++ b/src/common/config_opts.h
@@ -22,9 +22,14 @@ OPTION(cluster_network, OPT_STR, "")
 OPTION(num_client, OPT_INT, 1)
 OPTION(monmap, OPT_STR, "")
 OPTION(mon_host, OPT_STR, "")
-OPTION(daemonize, OPT_BOOL, false)
 OPTION(lockdep, OPT_BOOL, false)
 OPTION(admin_socket, OPT_STR, "/var/run/ceph/$cluster.name.asok")
+
+OPTION(daemonize, OPT_BOOL, false)
+OPTION(pid_file, OPT_STR, "")
+OPTION(chdir, OPT_STR, "/")
+OPTION(max_open_files, OPT_LONGLONG, 0)
+
 OPTION(log_file, OPT_STR, "/var/log/ceph/$cluster.$name.log")
 OPTION(log_max_new, OPT_INT, 1000)
 OPTION(log_max_recent, OPT_INT, 1000000)
@@ -32,11 +37,10 @@ OPTION(log_to_stderr, OPT_BOOL, true)
 OPTION(err_to_stderr, OPT_BOOL, true)
 OPTION(log_to_syslog, OPT_BOOL, false)
 OPTION(err_to_syslog, OPT_BOOL, false)
+OPTION(log_flush_on_exit, OPT_BOOL, true)
+
 OPTION(clog_to_monitors, OPT_BOOL, true)
 OPTION(clog_to_syslog, OPT_BOOL, false)
-OPTION(pid_file, OPT_STR, "")
-OPTION(chdir, OPT_STR, "/")
-OPTION(max_open_files, OPT_LONGLONG, 0)
 
 DEFAULT_SUBSYS(0, 5)
 SUBSYS(lockdep, 0, 5)
diff --git a/src/global/global_init.cc b/src/global/global_init.cc
index 44db4c148101..90c1edc04511 100644
--- a/src/global/global_init.cc
+++ b/src/global/global_init.cc
@@ -107,6 +107,9 @@ void global_init(std::vector < const char * > *alt_def_args, std::vector < const
   block_signals(siglist, NULL);
   install_standard_sighandlers();
 
+  if (g_conf->log_flush_on_exit)
+    g_ceph_context->_log->set_flush_on_exit();
+
   if (g_lockdep) {
     dout(1) << "lockdep is enabled" << dendl;
     lockdep_register_ceph_context(cct);
diff --git a/src/log/Log.cc b/src/log/Log.cc
index 9cb90c154220..f029390f5a5e 100644
--- a/src/log/Log.cc
+++ b/src/log/Log.cc
@@ -30,7 +30,7 @@ static void log_on_exit(int r, void *p)
 }
 
 Log::Log(SubsystemMap *s)
-  : m_indirect_this(new (Log*)(this)),   // we will deliberately leak this
+  : m_indirect_this(NULL),
     m_subs(s),
     m_new(), m_recent(),
     m_fd(-1),
@@ -42,12 +42,6 @@ Log::Log(SubsystemMap *s)
 {
   int ret;
 
-  // Make sure we flush on shutdown.  We do this by deliberately
-  // leaking an indirect pointer to ourselves (on_exit() can't
-  // unregister a callback).  This is not racy only becuase we
-  // assume that exit() won't race with ~Log().
-  on_exit(log_on_exit, m_indirect_this);
-
   ret = pthread_spin_init(&m_lock, PTHREAD_PROCESS_SHARED);
   assert(ret == 0);
 
@@ -68,7 +62,9 @@ Log::Log(SubsystemMap *s)
 
 Log::~Log()
 {
-  *m_indirect_this = NULL;
+  if (m_indirect_this) {
+    *m_indirect_this = NULL;
+  }
 
   assert(!is_started());
   if (m_fd >= 0)
@@ -83,6 +79,18 @@ Log::~Log()
 
 ///
 
+void Log::set_flush_on_exit()
+{
+  // Make sure we flush on shutdown.  We do this by deliberately
+  // leaking an indirect pointer to ourselves (on_exit() can't
+  // unregister a callback).  This is not racy only becuase we
+  // assume that exit() won't race with ~Log().
+  if (m_indirect_this == NULL) {
+    m_indirect_this = new (Log*)(this);
+    on_exit(log_on_exit, m_indirect_this);
+  }
+}
+
 void Log::set_max_new(int n)
 {
   m_max_new = n;
diff --git a/src/log/Log.h b/src/log/Log.h
index 372cc789ba41..ab83a7c86e43 100644
--- a/src/log/Log.h
+++ b/src/log/Log.h
@@ -49,6 +49,8 @@ public:
   Log(SubsystemMap *s);
   virtual ~Log();
 
+  void set_flush_on_exit();
+
   void set_max_new(int n);
   void set_max_recent(int n);
   void set_log_file(std::string fn);