* Re: [PATCH] daemons: write pid file even when told not to daemonize
2014-07-31 2:08 [PATCH] daemons: write pid file even when told not to daemonize Alexandre Oliva
@ 2014-07-31 4:18 ` Loic Dachary
2014-07-31 4:30 ` Sage Weil
2014-09-09 22:21 ` Sage Weil
2014-09-12 10:22 ` Loic Dachary
2 siblings, 1 reply; 12+ messages in thread
From: Loic Dachary @ 2014-07-31 4:18 UTC (permalink / raw)
To: Alexandre Oliva, ceph-devel
[-- Attachment #1: Type: text/plain, Size: 2788 bytes --]
Hi Alexandre,
With this patch ceph-osd -f will try to create the default pid file : this is a non backward compatible change. Maybe there is a way for systemd to capture the pid of the process and store it instead of requiring the deamon to create the pid file ?
Cheers
On 31/07/2014 08:08, 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();
>
--
Loïc Dachary, Artisan Logiciel Libre
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] daemons: write pid file even when told not to daemonize
2014-07-31 4:18 ` Loic Dachary
@ 2014-07-31 4:30 ` Sage Weil
2014-08-28 7:35 ` Alexandre Oliva
0 siblings, 1 reply; 12+ messages in thread
From: Sage Weil @ 2014-07-31 4:30 UTC (permalink / raw)
To: Loic Dachary; +Cc: Alexandre Oliva, ceph-devel
On Thu, 31 Jul 2014, Loic Dachary wrote:
> Hi Alexandre,
>
> With this patch ceph-osd -f will try to create the default pid file :
> this is a non backward compatible change. Maybe there is a way for
> systemd to capture the pid of the process and store it instead of
> requiring the deamon to create the pid file ?
Do we need the pid file at all when we aren't using sysinit?
sage
>
> Cheers
>
> On 31/07/2014 08:08, 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();
> >
>
> --
> Lo?c Dachary, Artisan Logiciel Libre
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] daemons: write pid file even when told not to daemonize
2014-07-31 4:30 ` Sage Weil
@ 2014-08-28 7:35 ` Alexandre Oliva
2014-08-28 8:09 ` Loic Dachary
0 siblings, 1 reply; 12+ messages in thread
From: Alexandre Oliva @ 2014-08-28 7:35 UTC (permalink / raw)
To: Sage Weil; +Cc: Loic Dachary, ceph-devel
On Jul 31, 2014, Sage Weil <sweil@redhat.com> wrote:
> On Thu, 31 Jul 2014, Loic Dachary wrote:
>> Hi Alexandre,
>>
>> With this patch ceph-osd -f will try to create the default pid file :
>> this is a non backward compatible change. Maybe there is a way for
>> systemd to capture the pid of the process and store it instead of
>> requiring the deamon to create the pid file ?
> Do we need the pid file at all when we aren't using sysinit?
My own monitoring scripts use it, and ceph stop use it as well. I was
surprised we were not creating them in spite of an explicit command line
option to do so.
--
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] daemons: write pid file even when told not to daemonize
2014-08-28 7:35 ` Alexandre Oliva
@ 2014-08-28 8:09 ` Loic Dachary
2014-08-29 6:41 ` Alexandre Oliva
0 siblings, 1 reply; 12+ messages in thread
From: Loic Dachary @ 2014-08-28 8:09 UTC (permalink / raw)
To: Alexandre Oliva; +Cc: ceph-devel
[-- Attachment #1: Type: text/plain, Size: 902 bytes --]
Hi Alexandre,
Changing this behavior is not backward compatible but it is indeed more intuitive. Has it been a significant inconvenience so far ?
Cheers
On 28/08/2014 09:35, Alexandre Oliva wrote:
> On Jul 31, 2014, Sage Weil <sweil@redhat.com> wrote:
>
>> On Thu, 31 Jul 2014, Loic Dachary wrote:
>>> Hi Alexandre,
>>>
>>> With this patch ceph-osd -f will try to create the default pid file :
>>> this is a non backward compatible change. Maybe there is a way for
>>> systemd to capture the pid of the process and store it instead of
>>> requiring the deamon to create the pid file ?
>
>> Do we need the pid file at all when we aren't using sysinit?
>
> My own monitoring scripts use it, and ceph stop use it as well. I was
> surprised we were not creating them in spite of an explicit command line
> option to do so.
>
--
Loïc Dachary, Artisan Logiciel Libre
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] daemons: write pid file even when told not to daemonize
2014-08-28 8:09 ` Loic Dachary
@ 2014-08-29 6:41 ` Alexandre Oliva
2014-08-29 8:40 ` Loic Dachary
0 siblings, 1 reply; 12+ messages in thread
From: Alexandre Oliva @ 2014-08-29 6:41 UTC (permalink / raw)
To: Loic Dachary; +Cc: ceph-devel
On Aug 28, 2014, Loic Dachary <loic@dachary.org> wrote:
> Changing this behavior is not backward compatible but it is indeed more intuitive. Has it been a significant inconvenience so far ?
Before I wrote the patch, it was very inconvenient: in order to stop
ceph services, I had to dig up the PIDs from ps output and then kill the
processes manually, and I had to run ceph without my monitor that
automatically restarted services that died (checking that the pid file
was absent, or that the PID it named was not a running process).
After I applied the patch in my local build, I could just forget about
the earlier problems, and none surfaced because of the creation of the
PID file.
Is this what you were asking?
> On 28/08/2014 09:35, Alexandre Oliva wrote:
>> On Jul 31, 2014, Sage Weil <sweil@redhat.com> wrote:
>>
>>> On Thu, 31 Jul 2014, Loic Dachary wrote:
>>>> Hi Alexandre,
>>>>
>>>> With this patch ceph-osd -f will try to create the default pid file :
>>>> this is a non backward compatible change. Maybe there is a way for
>>>> systemd to capture the pid of the process and store it instead of
>>>> requiring the deamon to create the pid file ?
>>
>>> Do we need the pid file at all when we aren't using sysinit?
>>
>> My own monitoring scripts use it, and ceph stop use it as well. I was
>> surprised we were not creating them in spite of an explicit command line
>> option to do so.
--
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] daemons: write pid file even when told not to daemonize
2014-08-29 6:41 ` Alexandre Oliva
@ 2014-08-29 8:40 ` Loic Dachary
2014-09-04 19:48 ` Alexandre Oliva
0 siblings, 1 reply; 12+ messages in thread
From: Loic Dachary @ 2014-08-29 8:40 UTC (permalink / raw)
To: Alexandre Oliva; +Cc: ceph-devel
[-- Attachment #1: Type: text/plain, Size: 2090 bytes --]
On 29/08/2014 08:41, Alexandre Oliva wrote:
> On Aug 28, 2014, Loic Dachary <loic@dachary.org> wrote:
>
>> Changing this behavior is not backward compatible but it is indeed more intuitive. Has it been a significant inconvenience so far ?
>
> Before I wrote the patch, it was very inconvenient: in order to stop
> ceph services, I had to dig up the PIDs from ps output and then kill the
> processes manually, and I had to run ceph without my monitor that
> automatically restarted services that died (checking that the pid file
> was absent, or that the PID it named was not a running process).
>
> After I applied the patch in my local build, I could just forget about
> the earlier problems, and none surfaced because of the creation of the
> PID file.
>
> Is this what you were asking?
Hi Alexandre,
After a quick glance at the systemd man page http://www.freedesktop.org/software/systemd/man/systemd.service.html#Options it seems possible to do something like
https://github.com/dachary/ceph/compare/wip-systemd?expand=1
Alternatively, shouldn't systemctl kill ceph-osd@10.service be the canonical way of killing a process spawned by systemd ?
If both are impractical or frowned upon for some reason, it looks like your patch is the only way to go, indeed.
Cheers
>
>> On 28/08/2014 09:35, Alexandre Oliva wrote:
>>> On Jul 31, 2014, Sage Weil <sweil@redhat.com> wrote:
>>>
>>>> On Thu, 31 Jul 2014, Loic Dachary wrote:
>>>>> Hi Alexandre,
>>>>>
>>>>> With this patch ceph-osd -f will try to create the default pid file :
>>>>> this is a non backward compatible change. Maybe there is a way for
>>>>> systemd to capture the pid of the process and store it instead of
>>>>> requiring the deamon to create the pid file ?
>>>
>>>> Do we need the pid file at all when we aren't using sysinit?
>>>
>>> My own monitoring scripts use it, and ceph stop use it as well. I was
>>> surprised we were not creating them in spite of an explicit command line
>>> option to do so.
>
--
Loïc Dachary, Artisan Logiciel Libre
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] daemons: write pid file even when told not to daemonize
2014-08-29 8:40 ` Loic Dachary
@ 2014-09-04 19:48 ` Alexandre Oliva
0 siblings, 0 replies; 12+ messages in thread
From: Alexandre Oliva @ 2014-09-04 19:48 UTC (permalink / raw)
To: Loic Dachary; +Cc: ceph-devel
On Aug 29, 2014, Loic Dachary <loic@dachary.org> wrote:
> After a quick glance at the systemd man page http://www.freedesktop.org/software/systemd/man/systemd.service.html#Options it seems possible to do something like
> https://github.com/dachary/ceph/compare/wip-systemd?expand=1
Yeah, it sure is possible in systemd to start and track daemons that
don't have a “run in foreground” option, but it's not as efficient.
> Alternatively, shouldn't systemctl kill ceph-osd@10.service be the canonical way of killing a process spawned by systemd ?
If you know what number to use after @, I guess it would. I don't think
we're talking about the same version of ceph, when it comes to systemd
config files. Your patch doesn't modify any file present on my system
(still running 0.80.5)
> If both are impractical or frowned upon for some reason, it looks like
> your patch is the only way to go, indeed.
It's certainly not the only way to go, but silently dropping a command
line option sounds like a bug to me. Of course the patch I proposed
isn't the only way to go about that...
--
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] daemons: write pid file even when told not to daemonize
2014-07-31 2:08 [PATCH] daemons: write pid file even when told not to daemonize Alexandre Oliva
2014-07-31 4:18 ` Loic Dachary
@ 2014-09-09 22:21 ` Sage Weil
2014-09-09 22:48 ` Loic Dachary
2014-09-12 10:22 ` Loic Dachary
2 siblings, 1 reply; 12+ messages in thread
From: Sage Weil @ 2014-09-09 22:21 UTC (permalink / raw)
To: Alexandre Oliva; +Cc: ceph-devel
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
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] daemons: write pid file even when told not to daemonize
2014-09-09 22:21 ` Sage Weil
@ 2014-09-09 22:48 ` Loic Dachary
2014-09-09 22:54 ` Sage Weil
0 siblings, 1 reply; 12+ messages in thread
From: Loic Dachary @ 2014-09-09 22:48 UTC (permalink / raw)
To: Sage Weil, Alexandre Oliva; +Cc: ceph-devel
[-- 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 --]
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] daemons: write pid file even when told not to daemonize
2014-09-09 22:48 ` Loic Dachary
@ 2014-09-09 22:54 ` Sage Weil
0 siblings, 0 replies; 12+ messages in thread
From: Sage Weil @ 2014-09-09 22:54 UTC (permalink / raw)
To: Loic Dachary; +Cc: Alexandre Oliva, ceph-devel
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 <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
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] daemons: write pid file even when told not to daemonize
2014-07-31 2:08 [PATCH] daemons: write pid file even when told not to daemonize Alexandre Oliva
2014-07-31 4:18 ` Loic Dachary
2014-09-09 22:21 ` Sage Weil
@ 2014-09-12 10:22 ` Loic Dachary
2 siblings, 0 replies; 12+ messages in thread
From: Loic Dachary @ 2014-09-12 10:22 UTC (permalink / raw)
To: Alexandre Oliva, ceph-devel
[-- Attachment #1: Type: text/plain, Size: 2591 bytes --]
Hi Alexandre,
I applied the patch locally and running make check.
Cheers
On 31/07/2014 04:08, 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();
>
--
Loïc Dachary, Artisan Logiciel Libre
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread