On Fri, 2013-11-22 at 16:46 +0100, Jakub Hrozek wrote:
> On Wed, Nov 20, 2013 at 01:52:42PM +0100, Pavel Reichl wrote:
> > New patch hopefully addressing all mentioned issues is attached.
> >
> > PR
> >
>
> Hi Pavel,
>
> sorry for the late review.
>
> The patch works fine and mostly looks good, I'm just wondering about one
> change. I wonder if this block:
>
> > + if (ret == ENOENT) {
> > + /* sss specific error denoting missing configuration file */
> > + ret = ERR_MISSING_CONF;
> > + }
>
> Would be better placed in the confdb_init_db() function after the
> sss_ini_config_file_open() fails with ENOENT? Not 100% sure what would
> look better though.
> _______________________________________________
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Hi Jakub,
I think you are right because it makes more obvious which function is
supposed to fail with ENOENT. Improved patch is attached.
PR
np,
>From db51685823968d654864dc3882927b402da9e664 Mon Sep 17 00:00:00 2001
From: Pavel Reichl <pavel.rei...@redhat.com>
Date: Tue, 19 Nov 2013 11:24:31 +0000
Subject: [PATCH] monitor: Specific error message for missing sssd.conf
Specific error message is logged for missing sssd.conf file. New sssd specific
error value is introduced for this case.
Resolves:
https://fedorahosted.org/sssd/ticket/2156
---
src/confdb/confdb_setup.c | 9 +++++++--
src/monitor/monitor.c | 6 ++++++
src/util/util_errors.c | 1 +
src/util/util_errors.h | 1 +
4 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/src/confdb/confdb_setup.c b/src/confdb/confdb_setup.c
index b13553eaa560bb83ecf7a53b32ab116f38f7f480..2a34e4f7a3e5f9aa37760036520f5274e9289b70 100644
--- a/src/confdb/confdb_setup.c
+++ b/src/confdb/confdb_setup.c
@@ -155,8 +155,13 @@ int confdb_init_db(const char *config_file, struct confdb_ctx *cdb)
/* Open config file */
ret = sss_ini_config_file_open(init_data, config_file);
if (ret != EOK) {
- DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to open configuration file.\n"));
- ret = EIO;
+ DEBUG(SSSDBG_TRACE_FUNC,
+ ("sss_ini_config_file_open failed: %s [%d]\n", strerror(ret),
+ ret));
+ if (ret == ENOENT) {
+ /* sss specific error denoting missing configuration file */
+ ret = ERR_MISSING_CONF;
+ }
goto done;
}
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index f8a02b8b8be083860a4ae342a27b1297613af996..f887bf92e4aa2283e6ceadb44d7a63ed7df693fc 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -2785,6 +2785,12 @@ int main(int argc, const char *argv[])
ret = load_configuration(tmp_ctx, config_file, &monitor);
if (ret != EOK) {
switch (ret) {
+ case ERR_MISSING_CONF:
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ ("Configuration file: %s does not exist.\n", config_file));
+ sss_log(SSS_LOG_ALERT,
+ "Configuration file: %s does not exist.\n", config_file);
+ break;
case EPERM:
case EACCES:
DEBUG(SSSDBG_CRIT_FAILURE,
diff --git a/src/util/util_errors.c b/src/util/util_errors.c
index b3d10f97b0567ca21ca08a3f2d326ea401c28aff..114c8b04fd354b166d14e526a3bab6a6c0c05951 100644
--- a/src/util/util_errors.c
+++ b/src/util/util_errors.c
@@ -50,6 +50,7 @@ struct err_string error_to_str[] = {
{ "Dynamic DNS update not possible while offline" }, /* ERR_DYNDNS_OFFLINE */
{ "Entry not found" }, /* ERR_NOT_FOUND */
{ "Domain not found" }, /* ERR_DOMAIN_NOT_FOUND */
+ { "Missing configuration file" }, /* ERR_MISSING_CONF */
};
diff --git a/src/util/util_errors.h b/src/util/util_errors.h
index 685607c5bb1b4e7c37152c876518a2b1c69c18d6..bca45f392b0357c3f1c848768358cb1d47514715 100644
--- a/src/util/util_errors.h
+++ b/src/util/util_errors.h
@@ -72,6 +72,7 @@ enum sssd_errors {
ERR_DYNDNS_OFFLINE,
ERR_NOT_FOUND,
ERR_DOMAIN_NOT_FOUND,
+ ERR_MISSING_CONF,
ERR_LAST /* ALWAYS LAST */
};
--
1.8.3.1
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
