URL: https://github.com/SSSD/sssd/pull/750
Author: alexey-tikhonov
Title: #750: Monitor: minor fixes
Action: opened
PR body:
"""
"""
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/750/head:pr750
git checkout pr750
From 4de41b4411ffb0450234ce045440ece77903f4e9 Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikh...@redhat.com>
Date: Tue, 12 Feb 2019 20:32:49 +0100
Subject: [PATCH 1/3] Monitor & utils: got rid of pid filename duplication
and simplified `pidfile()` function as well.
---
src/monitor/monitor.c | 11 ++++-------
src/util/server.c | 27 +++++++++++----------------
src/util/util.h | 5 +++--
3 files changed, 18 insertions(+), 25 deletions(-)
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 8d12f81339..0445c3c4a3 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -77,9 +77,6 @@
*/
#define MONITOR_MAX_RESTART_DELAY 4
-/* name of the monitor server instance */
-#define MONITOR_NAME "sssd"
-
/* Special value to leave the Kerberos Replay Cache set to use
* the libkrb5 defaults
*/
@@ -513,11 +510,11 @@ static int mark_service_as_started(struct mt_svc *svc)
DEBUG(SSSDBG_TRACE_FUNC,
"All services have successfully started, creating pid file\n");
- ret = pidfile(PID_PATH, MONITOR_NAME);
+ ret = pidfile(SSSD_PIDFILE);
if (ret != EOK) {
DEBUG(SSSDBG_FATAL_FAILURE,
- "Error creating pidfile: %s/%s.pid! (%d [%s])\n",
- PID_PATH, MONITOR_NAME, ret, strerror(ret));
+ "Error creating pidfile: %s! (%d [%s])\n",
+ SSSD_PIDFILE, ret, strerror(ret));
kill(getpid(), SIGTERM);
}
@@ -2571,7 +2568,7 @@ int main(int argc, const char *argv[])
ret = close(STDIN_FILENO);
if (ret != EOK) return 6;
- ret = server_setup(MONITOR_NAME, flags, 0, 0,
+ ret = server_setup(SSSD_MONITOR_NAME, flags, 0, 0,
monitor->conf_path, &main_ctx);
if (ret != EOK) return 2;
diff --git a/src/util/server.c b/src/util/server.c
index ddc2bed4ca..70f86e9bdc 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -139,11 +139,10 @@ void become_daemon(bool Fork)
close_low_fds();
}
-int pidfile(const char *path, const char *name)
+int pidfile(const char *file)
{
char pid_str[32];
pid_t pid;
- char *file;
int fd;
int ret, err;
ssize_t len;
@@ -151,11 +150,6 @@ int pidfile(const char *path, const char *name)
ssize_t written;
ssize_t pidlen = sizeof(pid_str) - 1;
- file = talloc_asprintf(NULL, "%s/%s.pid", path, name);
- if (!file) {
- return ENOMEM;
- }
-
fd = open(file, O_RDONLY, 0644);
err = errno;
if (fd != -1) {
@@ -166,7 +160,6 @@ int pidfile(const char *path, const char *name)
DEBUG(SSSDBG_CRIT_FAILURE,
"read failed [%d][%s].\n", ret, strerror(ret));
close(fd);
- talloc_free(file);
return EINVAL;
}
@@ -181,13 +174,11 @@ int pidfile(const char *path, const char *name)
/* succeeded in signaling the process -> another sssd process */
if (ret == 0) {
close(fd);
- talloc_free(file);
return EEXIST;
}
if (ret != 0 && errno != ESRCH) {
err = errno;
close(fd);
- talloc_free(file);
return err;
}
}
@@ -204,7 +195,6 @@ int pidfile(const char *path, const char *name)
}
} else {
if (err != ENOENT) {
- talloc_free(file);
return err;
}
}
@@ -212,10 +202,8 @@ int pidfile(const char *path, const char *name)
fd = open(file, O_CREAT | O_WRONLY | O_EXCL, 0644);
err = errno;
if (fd == -1) {
- talloc_free(file);
return err;
}
- talloc_free(file);
memset(pid_str, 0, sizeof(pid_str));
snprintf(pid_str, sizeof(pid_str) -1, "%u\n", (unsigned int) getpid());
@@ -465,6 +453,7 @@ int server_setup(const char *name, int flags,
char *locale;
int watchdog_interval;
pid_t my_pid;
+ char *pidfile_name;
my_pid = getpid();
ret = setpgid(my_pid, my_pid);
@@ -518,12 +507,18 @@ int server_setup(const char *name, int flags,
}
if (flags & FLAGS_PID_FILE) {
- ret = pidfile(get_pid_path(), name);
+ pidfile_name = talloc_asprintf(NULL, "%s/%s.pid", get_pid_path(), name);
+ if (!pidfile_name) {
+ return ENOMEM;
+ }
+ ret = pidfile(pidfile_name);
if (ret != EOK) {
- DEBUG(SSSDBG_FATAL_FAILURE, "Error creating pidfile: %s/%s.pid! "
- "(%d [%s])\n", get_pid_path(), name, ret, strerror(ret));
+ DEBUG(SSSDBG_FATAL_FAILURE, "Error creating pidfile: %s! "
+ "(%d [%s])\n", pidfile_name, ret, strerror(ret));
+ talloc_free(pidfile_name);
return ret;
}
+ talloc_free(pidfile_name);
}
/* Set up locale */
diff --git a/src/util/util.h b/src/util/util.h
index dbb1b00fee..312fc46d53 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -47,7 +47,8 @@
#include "util/debug.h"
/* name of the monitor server instance */
-#define SSSD_PIDFILE PID_PATH"/sssd.pid"
+#define SSSD_MONITOR_NAME "sssd"
+#define SSSD_PIDFILE PID_PATH"/"SSSD_MONITOR_NAME".pid"
#define MAX_PID_LENGTH 10
#define _(STRING) gettext (STRING)
@@ -174,7 +175,7 @@ struct main_context {
errno_t server_common_rotate_logs(struct confdb_ctx *confdb,
const char *conf_entry);
int die_if_parent_died(void);
-int pidfile(const char *path, const char *name);
+int pidfile(const char *file);
int server_setup(const char *name, int flags,
uid_t uid, gid_t gid,
const char *conf_entry,
From c5234a5f7be6c4bb74f3917fe20141b6c258a0a3 Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikh...@redhat.com>
Date: Tue, 12 Feb 2019 20:34:38 +0100
Subject: [PATCH 2/3] Monitor: got rid of redundant check
Check is inside `ctx->started_services == ctx->num_services` &&
(`socket_activated == true` => `num_services++`) && `started_services++`
=> condition `ctx->started_services == ctx->num_services` was already
met on previous step => `pid_file_created = true` => check is redundant
---
src/monitor/monitor.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 0445c3c4a3..84f60079db 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -502,12 +502,6 @@ 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) {
- if (svc->socket_activated) {
- /* There's no reason for trying to terminate the parent process
- * when the responder was socket-activated. */
- goto done;
- }
-
DEBUG(SSSDBG_TRACE_FUNC,
"All services have successfully started, creating pid file\n");
ret = pidfile(SSSD_PIDFILE);
From 6a20cfee60c39ae54e851bf766b78ec92ae25295 Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikh...@redhat.com>
Date: Tue, 12 Feb 2019 20:44:36 +0100
Subject: [PATCH 3/3] Monitor: fixed bug with services launch
Setting `services_started = true;` in case socket activated service
connects before all providers are up would prevent start of configured
services.
---
src/monitor/monitor.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 84f60079db..e9a2fa3a5a 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -255,7 +255,6 @@ errno_t socket_activated_service_not_found(struct mt_ctx *mt_ctx,
return EINVAL;
}
- mt_ctx->services_started = true;
mt_ctx->num_services++;
ret = get_service_config(mt_ctx, svc_name, &svc);
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
