From: Colin Patrick McCabe <cmccabe@alumni.cmu.edu>
Date: Wed, 8 Dec 2010 19:50:15 +0000 (-0800)
Subject: logging: DoutStreambuf: handle daemonizing better
X-Git-Tag: v0.25~463^2~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=fdc7414ecd688ede516d533c4d9d0750163c3e1d;p=ceph.git

logging: DoutStreambuf: handle daemonizing better

Signed-off-by: Colin McCabe <colinm@hq.newdream.net>
---

diff --git a/src/common/DoutStreambuf.cc b/src/common/DoutStreambuf.cc
index 31c9c3fddff9..9a7c5b23643c 100644
--- a/src/common/DoutStreambuf.cc
+++ b/src/common/DoutStreambuf.cc
@@ -31,10 +31,15 @@
 // TODO: get rid of this lock using thread-local storage
 extern Mutex _dout_lock;
 
-/* True if we should output high-priority messages to stderr */
-static bool use_stderr = true;
-
 //////////////////////// Helper functions //////////////////////////
+// Try a 0-byte write to a file descriptor to see if it open.
+static bool fd_is_open(int fd)
+{
+  char buf;
+  ssize_t res = TEMP_FAILURE_RETRY(write(fd, &buf, 0));
+  return (res == 0);
+}
+
 static bool empty(const char *str)
 {
   if (!str)
@@ -260,15 +265,17 @@ DoutStreambuf<charT, traits>::overflow(DoutStreambuf<charT, traits>::int_type c)
 }
 
 template <typename charT, typename traits>
-void DoutStreambuf<charT, traits>::set_use_stderr(bool val)
+void DoutStreambuf<charT, traits>::handle_stderr_closed()
 {
-  _dout_lock.Lock();
-  use_stderr = val;
-  if (val)
-    flags |= DOUTSB_FLAG_STDERR;
-  else
-    flags &= ~DOUTSB_FLAG_STDERR;
-  _dout_lock.Unlock();
+  assert(_dout_lock.is_locked());
+  flags &= ~DOUTSB_FLAG_STDERR;
+}
+
+template <typename charT, typename traits>
+void DoutStreambuf<charT, traits>::handle_stdout_closed()
+{
+  assert(_dout_lock.is_locked());
+  flags &= ~DOUTSB_FLAG_STDOUT;
 }
 
 template <typename charT, typename traits>
@@ -281,9 +288,11 @@ void DoutStreambuf<charT, traits>::read_global_config()
     flags |= DOUTSB_FLAG_SYSLOG;
   }
   if (g_conf.log_to_stdout) {
-    flags |= DOUTSB_FLAG_STDOUT;
+    if (fd_is_open(STDOUT_FILENO)) {
+      flags |= DOUTSB_FLAG_STDOUT;
+    }
   }
-  if (use_stderr) {
+  if (fd_is_open(STDERR_FILENO)) {
     flags |= DOUTSB_FLAG_STDERR;
   }
   if (g_conf.log_to_file) {
@@ -315,7 +324,7 @@ set_prio(int prio)
 template <typename charT, typename traits>
 int DoutStreambuf<charT, traits>::handle_pid_change()
 {
-  Mutex::Locker l(_dout_lock);
+  assert(_dout_lock.is_locked());
   if (!(flags & DOUTSB_FLAG_OFILE))
     return 0;
 
@@ -392,7 +401,6 @@ std::string DoutStreambuf<charT, traits>::config_to_str() const
   ostringstream oss;
   oss << "g_conf.log_to_syslog = " << g_conf.log_to_syslog << "\n";
   oss << "g_conf.log_to_stdout = " << g_conf.log_to_stdout << "\n";
-  oss << "use_stderr = " << use_stderr << "\n";
   oss << "g_conf.log_to_file = " << g_conf.log_to_file << "\n";
   oss << "g_conf.log_file = '" << cpp_str(g_conf.log_file) << "'\n";
   oss << "g_conf.log_dir = '" << cpp_str(g_conf.log_dir) << "'\n";
@@ -529,7 +537,7 @@ bool DoutStreambuf<charT, traits>::_read_ofile_config()
 
   assert(ofd == -1);
   ofd = open(opath.c_str(),
-	    O_CREAT | O_WRONLY | O_CLOEXEC | O_APPEND, S_IWUSR | S_IRUSR);
+	    O_CREAT | O_WRONLY | O_APPEND, S_IWUSR | S_IRUSR);
   if (ofd < 0) {
     int err = errno;
     ostringstream oss;
diff --git a/src/common/DoutStreambuf.h b/src/common/DoutStreambuf.h
index afef48f572d5..76968198b24f 100644
--- a/src/common/DoutStreambuf.h
+++ b/src/common/DoutStreambuf.h
@@ -44,11 +44,13 @@ public:
 
   DoutStreambuf();
 
-  // Set or clear the use_stderr bit
-  // If this bit is cleared, we don't bother outputting high priority messages
-  // on stderr any more. We should clear the bit after daemonizing, since
-  // stderr -> /dev/null at that point.
-  void set_use_stderr(bool val);
+  // Call when you close stderr.  Not strictly necessary, since we would get an
+  // error the next time we tried to write to stdedrr. But nicer than waiting
+  // for the error to happen.
+  void handle_stderr_closed();
+
+  // Call when you close stdout.
+  void handle_stdout_closed();
 
   // Set the flags based on the global configuration
   void read_global_config();
diff --git a/src/common/debug.cc b/src/common/debug.cc
index 79ec003ed8f5..10cd6ceed9ec 100644
--- a/src/common/debug.cc
+++ b/src/common/debug.cc
@@ -35,9 +35,11 @@ void _dout_open_log()
   _dout_need_open = false;
 }
 
-int dout_handle_pid_change()  // after calling daemon()
+int dout_handle_daemonize()
 {
   Mutex::Locker l(_dout_lock);
+  _doss->handle_stdout_closed();
+  _doss->handle_stderr_closed();
   return _doss->handle_pid_change();
 }
 
@@ -47,11 +49,6 @@ int dout_create_rank_symlink(int n)
   return _doss->create_rank_symlink(n);
 }
 
-void dout_disable_stderr()
-{
-  _doss->set_use_stderr(false);
-}
-
 void hex2str(const char *s, int len, char *buf, int dest_len)
 {
   int pos = 0;
diff --git a/src/common/debug.h b/src/common/debug.h
index 9dcc0a054533..af6fe9957e1b 100644
--- a/src/common/debug.h
+++ b/src/common/debug.h
@@ -31,11 +31,7 @@ extern Mutex _dout_lock;
 
 extern void _dout_open_log();
 
-// Call when the pid changes. For example, after calling daemon().
-extern int dout_handle_pid_change();
-
-// Skip output to stderr.
-extern void dout_disable_stderr();
+extern int dout_handle_daemonize();
 
 extern int dout_create_rank_symlink(int n);
 
diff --git a/src/msg/SimpleMessenger.cc b/src/msg/SimpleMessenger.cc
index 0d0c4e7a5ee1..cdc1d28399b4 100644
--- a/src/msg/SimpleMessenger.cc
+++ b/src/msg/SimpleMessenger.cc
@@ -2377,7 +2377,6 @@ int SimpleMessenger::start(bool nodaemon)
 	      << dendl;
     }
     dout(1) << "messenger.start daemonizing" << dendl;
-    dout_disable_stderr();
 
     if (1) {
       daemon(1, 0);
@@ -2398,8 +2397,7 @@ int SimpleMessenger::start(bool nodaemon)
       ::mkdir(g_conf.chdir, 0700);
       ::chdir(g_conf.chdir);
     }
-
-    dout_handle_pid_change();
+    dout_handle_daemonize();
   }
 
   // go!