On 10/22/2012 01:49 PM, Pavel Březina wrote:
On 10/19/2012 02:20 PM, Simo Sorce wrote:
On Fri, 2012-10-19 at 09:52 +0200, Pavel Březina wrote:
On 10/18/2012 08:43 PM, Stephen Gallagher wrote:
On 10/18/2012 09:50 AM, Jakub Hrozek wrote:
On Thu, Oct 18, 2012 at 11:28:10AM +0200, Pavel Březina wrote:
On 10/18/2012 11:15 AM, Pavel Březina wrote:
https://fedorahosted.org/sssd/ticket/1357
Neither systemd or our init script use pid file as a notification
that sssd is finished initializing. They will continue starting up
next service right after the original process is terminated.
Oops, I forgot to amend the patch with latest changes. The final
patch is attached.
I haven't really had time to read the patch too carefully yet, but my
first thought was to always try to use tevent signals if you need
signals at all.
Yes, I went through a lot of effort a little over a year ago to get the
monitor properly using tevent signals instead of managing its own
directly. Please do not add manual handlers.
Nack.
Hi guys,
I just want to make sure that we are on the same page here before I
start modifying it to tevent.
I am not mixing tevent signals with posix signals. There is no existing
tevent context available and there will never be (unless I create it).
I just need to catch SIGTERM in original process (that forks and become
monitor), so that monitor can signal the original process to quit. Or
wait for the monitor to exit (in case of error etc.) if the signal
doesn't come.
Using tevent for this case seems like using a sledgehammer to crack a
nut.
Pavel if this is before we create the tevent event context than it is ok
to directly handle signals, however put a comment there taht says so.
example: /* We use signals directly here because we don't have a tevent
context yet */
Simo.
Very well. New patch is attached.
Hi,
I'm sending new patch set.
[PATCH 1/2] exit original process after sssd is initialized
...unchanged.
[PATCH 2/2] create pid file immediately after fork again
We realized that sysv and systemd does not use pid file existence
as a notification of finished initialization. Therefore, we create
the pid file immediately after we fork to become daemon.
From 3544c9e7e83628c69481ed0e3c930cffb71d9299 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Thu, 18 Oct 2012 10:16:06 +0200
Subject: [PATCH 1/2] exit original process after sssd is initialized
https://fedorahosted.org/sssd/ticket/1357
Neither systemd or our init script use pid file as a notification
that sssd is finished initializing. They will continue starting up
next service right after the original process is terminated.
---
src/monitor/monitor.c | 19 +++++++++++++++++++
src/util/server.c | 51 ++++++++++++++++++++++++++++++++++++++++-----------
src/util/util.h | 1 +
3 files changed, 60 insertions(+), 11 deletions(-)
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index a5653999e7531e27556693f88d1d988cc6c24a59..a9949bbf964544c254656c4fbf5f2353b3f54ae7 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -157,6 +157,8 @@ struct mt_ctx {
const char *conf_path;
struct sss_sigchild_ctx *sigchld_ctx;
bool pid_file_created;
+ bool is_daemon;
+ pid_t parent_pid;
};
static int start_service(struct mt_svc *mt_svc);
@@ -449,6 +451,21 @@ static int mark_service_as_started(struct mt_svc *svc)
}
ctx->pid_file_created = true;
+
+ /* initialization is complete, terminate parent process
+ * if in daemon mode */
+ if (ctx->is_daemon && ctx->parent_pid > 0) {
+ DEBUG(SSSDBG_TRACE_FUNC, ("SSSD is initialized, "
+ "terminating parent process\n"));
+
+ errno = 0;
+ ret = kill(ctx->parent_pid, SIGTERM);
+ if (ret != 0) {
+ ret = errno;
+ DEBUG(SSSDBG_FATAL_FAILURE, ("Unable to terminate parent "
+ "process [%d]: %s\n", ret, strerror(ret)));
+ }
+ }
}
done:
@@ -2627,6 +2644,8 @@ int main(int argc, const char *argv[])
ret = server_setup(MONITOR_NAME, flags, monitor->conf_path, &main_ctx);
if (ret != EOK) return 2;
+ monitor->is_daemon = !opt_interactive;
+ monitor->parent_pid = main_ctx->parent_pid;
monitor->ev = main_ctx->event_ctx;
talloc_steal(main_ctx, monitor);
diff --git a/src/util/server.c b/src/util/server.c
index f3f1b201bc751a4017b1e974c3a734cca415a097..9285a9c5f493b1337938cbbe0508617fb7fd18f1 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -24,6 +24,7 @@
*/
#include <sys/types.h>
+#include <sys/wait.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
@@ -67,22 +68,48 @@ static void close_low_fds(void)
#endif
}
+static void deamon_parent_sigterm(int sig)
+{
+ _exit(0);
+}
+
/**
Become a daemon, discarding the controlling terminal.
**/
-void become_daemon(bool Fork)
+void become_daemon(bool Fork, pid_t *ppid)
{
- int ret;
+ pid_t pid;
+ int status;
+ int ret;
- if (Fork) {
- if (fork()) {
- _exit(0);
- }
- }
+ if (Fork) {
+ pid = fork();
+ if (pid != 0) {
+ *ppid = 0;
+
+ /* Terminate parent process on demand so we can hold systemd
+ * or initd from starting next service until sssd in initialized.
+ * We use signals directly here because we don't have a tevent
+ * context yet. */
+ CatchSignal(SIGTERM, deamon_parent_sigterm);
+
+ /* or exit when sssd monitor is terminated */
+ waitpid(pid, &status, 0);
+
+ ret = 0;
+ if (WIFEXITED(status)) {
+ ret = WEXITSTATUS(status);
+ }
+
+ _exit(ret);
+ }
+ }
+
+ *ppid = getppid();
/* detach from the terminal */
- setsid();
+ setsid();
/* chdir to / to be sure we're not on a remote filesystem */
errno = 0;
@@ -93,8 +120,8 @@ void become_daemon(bool Fork)
return;
}
- /* Close fd's 0,1,2. Needed if started by rsh */
- close_low_fds();
+ /* Close fd's 0,1,2. Needed if started by rsh */
+ close_low_fds();
}
int pidfile(const char *path, const char *name)
@@ -369,6 +396,7 @@ int server_setup(const char *name, int flags,
bool dm;
struct tevent_signal *tes;
struct logrotate_ctx *lctx;
+ pid_t parent_pid = 0;
debug_prg_name = strdup(name);
if (!debug_prg_name) {
@@ -385,7 +413,7 @@ int server_setup(const char *name, int flags,
if (flags & FLAGS_DAEMON) {
DEBUG(3,("Becoming a daemon.\n"));
- become_daemon(true);
+ become_daemon(true, &parent_pid);
}
if (flags & FLAGS_PID_FILE) {
@@ -430,6 +458,7 @@ int server_setup(const char *name, int flags,
return ENOMEM;
}
+ ctx->parent_pid = parent_pid;
ctx->event_ctx = event_ctx;
conf_db = talloc_asprintf(ctx, "%s/%s", DB_PATH, CONFDB_FILE);
diff --git a/src/util/util.h b/src/util/util.h
index a96f519ad959b8706f31c5cf7e4204c77e39b737..268cffccd633d54ef82805c0ebb77673c06a0360 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -358,6 +358,7 @@ void sss_log(int priority, const char *format, ...);
struct main_context {
struct tevent_context *event_ctx;
struct confdb_ctx *confdb_ctx;
+ pid_t parent_pid;
};
int die_if_parent_died(void);
--
1.7.11.7
From f1c64ccca21912fa106b714ae44ff8b9a4530cf0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Fri, 26 Oct 2012 12:46:48 +0200
Subject: [PATCH 2/2] create pid file immediately after fork again
Related to https://fedorahosted.org/sssd/ticket/1357
We realized that sysv and systemd does not use pid file existence
as a notification of finished initialization. Therefore, we create
the pid file immediately after we fork to become daemon.
---
src/monitor/monitor.c | 28 ++++------------------------
1 file changed, 4 insertions(+), 24 deletions(-)
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index a9949bbf964544c254656c4fbf5f2353b3f54ae7..4ebb816ccb1fbc0cbea20e82f63cc595e1e46624 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -156,7 +156,6 @@ struct mt_ctx {
struct netlink_ctx *nlctx;
const char *conf_path;
struct sss_sigchild_ctx *sigchld_ctx;
- bool pid_file_created;
bool is_daemon;
pid_t parent_pid;
};
@@ -439,19 +438,7 @@ static int mark_service_as_started(struct mt_svc *svc)
}
/* create the pid file if all services are alive */
- if (!ctx->pid_file_created && ctx->started_services == ctx->num_services) {
- DEBUG(SSSDBG_TRACE_FUNC, ("All services have successfully started, "
- "creating pid file\n"));
- ret = pidfile(PID_PATH, MONITOR_NAME);
- if (ret != EOK) {
- DEBUG(SSSDBG_FATAL_FAILURE,
- ("Error creating pidfile: %s/%s.pid! (%d [%s])\n",
- PID_PATH, MONITOR_NAME, ret, strerror(ret)));
- kill(getpid(), SIGTERM);
- }
-
- ctx->pid_file_created = true;
-
+ if (ctx->started_services == ctx->num_services) {
/* initialization is complete, terminate parent process
* if in daemon mode */
if (ctx->is_daemon && ctx->parent_pid > 0) {
@@ -1450,7 +1437,6 @@ static errno_t load_configuration(TALLOC_CTX *mem_ctx,
return ENOMEM;
}
- ctx->pid_file_created = false;
talloc_set_destructor((TALLOC_CTX *)ctx, monitor_ctx_destructor);
cdb_file = talloc_asprintf(ctx, "%s/%s", DB_PATH, CONFDB_FILE);
@@ -2565,6 +2551,9 @@ int main(int argc, const char *argv[])
return 6;
}
+ /* we want a pid file check */
+ flags |= FLAGS_PID_FILE;
+
/* Open before server_setup() does to have logging
* during configuration checking */
if (debug_to_file) {
@@ -2608,15 +2597,6 @@ int main(int argc, const char *argv[])
"netgroup nsswitch maps.");
}
- /* Check if the SSSD is already running */
- ret = check_file(SSSD_PIDFILE_PATH, 0, 0, 0600, CHECK_REG, NULL, false);
- if (ret == EOK) {
- DEBUG(SSSDBG_FATAL_FAILURE,
- ("pidfile exists at %s\n", SSSD_PIDFILE_PATH));
- ERROR("SSSD is already running\n");
- return 2;
- }
-
/* Parse config file, fail if cannot be done */
ret = load_configuration(tmp_ctx, config_file, &monitor);
if (ret != EOK) {
--
1.7.11.7
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
