Re: [PATCH] daemons: write pid file even when told not to daemonize

[Loic Dachary <loic@dachary.org>]

[-- Attachment #1: Type: text/plain, Size: 3734 bytes --]

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.

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 <oliva@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 majordomo@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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
Loïc Dachary, Artisan Logiciel Libre


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]