On Wed, 10 Sep 2014, Loic Dachary wrote: > The only reason I can think of is that it requires write permission to a > directory that was previously not used. I think it will break a few unit > tests but nothing we can't fix.
Let's go for it then and clean up whatever breaks. We may want to change the default value for pid_file for some contexts, but I agree with Alexandre that that will be a better situation that the funny -f/-d side-effect we have now! sage > > On 10/09/2014 00:21, Sage Weil wrote: > > I'm inlined to just apply this original patch. Is there any reason *not* > > to create a pid file when -f is used? It's a change from previous > > behavior, but is there any possible harm that can come of it? > > > > sage > > > > > > On Wed, 30 Jul 2014, Alexandre Oliva wrote: > > > >> systemd wants to run daemons in foreground, but daemons wouldn't write > >> out the pid file with -f. Fixed. > >> > >> Signed-off-by: Alexandre Oliva <ol...@gnu.org> > >> --- > >> src/ceph_mon.cc | 3 +-- > >> src/common/config.cc | 2 -- > >> src/global/global_init.cc | 10 +++++++++- > >> 3 files changed, 10 insertions(+), 5 deletions(-) > >> > >> diff --git a/src/ceph_mon.cc b/src/ceph_mon.cc > >> index 4e84b4d..14dd6da 100644 > >> --- a/src/ceph_mon.cc > >> +++ b/src/ceph_mon.cc > >> @@ -406,8 +406,7 @@ int main(int argc, const char **argv) > >> // screwing us over > >> Preforker prefork; > >> if (!(flags & CINIT_FLAG_NO_DAEMON_ACTIONS)) { > >> - if (g_conf->daemonize) { > >> - global_init_prefork(g_ceph_context, 0); > >> + if (global_init_prefork(g_ceph_context, 0) >= 0) { > >> prefork.prefork(); > >> if (prefork.is_parent()) { > >> return prefork.parent_wait(); > >> diff --git a/src/common/config.cc b/src/common/config.cc > >> index 0ee7f58..4e3b6fe 100644 > >> --- a/src/common/config.cc > >> +++ b/src/common/config.cc > >> @@ -389,12 +389,10 @@ int md_config_t::parse_argv(std::vector<const > >> char*>& args) > >> } > >> else if (ceph_argparse_flag(args, i, "--foreground", "-f", > >> (char*)NULL)) { > >> set_val_or_die("daemonize", "false"); > >> - set_val_or_die("pid_file", ""); > >> } > >> else if (ceph_argparse_flag(args, i, "-d", (char*)NULL)) { > >> set_val_or_die("daemonize", "false"); > >> set_val_or_die("log_file", ""); > >> - set_val_or_die("pid_file", ""); > >> set_val_or_die("log_to_stderr", "true"); > >> set_val_or_die("err_to_stderr", "true"); > >> set_val_or_die("log_to_syslog", "false"); > >> diff --git a/src/global/global_init.cc b/src/global/global_init.cc > >> index 7b20343..f03677c 100644 > >> --- a/src/global/global_init.cc > >> +++ b/src/global/global_init.cc > >> @@ -166,8 +166,16 @@ int global_init_prefork(CephContext *cct, int flags) > >> if (g_code_env != CODE_ENVIRONMENT_DAEMON) > >> return -1; > >> const md_config_t *conf = cct->_conf; > >> - if (!conf->daemonize) > >> + if (!conf->daemonize) { > >> + if (atexit(pidfile_remove_void)) { > >> + derr << "global_init_daemonize: failed to set pidfile_remove > >> function " > >> + << "to run at exit." << dendl; > >> + } > >> + > >> + pidfile_write(g_conf); > >> + > >> return -1; > >> + } > >> > >> // stop log thread > >> g_ceph_context->_log->flush(); > >> > >> -- > >> Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/ > >> You must be the change you wish to see in the world. -- Gandhi > >> Be Free! -- http://FSFLA.org/ FSF Latin America board member > >> Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer > >> -- > >> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > >> the body of a message to majord...@vger.kernel.org > >> More majordomo info at http://vger.kernel.org/majordomo-info.html > >> > >> > > -- > > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > > the body of a message to majord...@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > -- > Lo?c Dachary, Artisan Logiciel Libre > > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html