On 11/08/2012 07:01 PM, Jan Cholasta wrote:
Hi,

On 8.11.2012 15:05, Ondrej Kos wrote:
https://fedorahosted.org/sssd/ticket/1589

patch is attached

O.


1) I think monitor and responders should use a different error message,
excluding "If greater version is expected, run SSSD ...". Telling users
to run SSSD when they in fact have it running might be confusing for
someone.

It might be even better to have different error messages for when the
version is greater than expected and when the version is lesser than
expected (not sure how hard it would be to implement that).

2) You should check for EMEDIUMTYPE in sss_cache.c and tools_util.c
right after sysdb_init_domain_and_sysdb is called IMO.

3) I would prefer if you did not suppress logging of debug messages when
ret == EMEDIUMTYPE.

Honza


1 - distinguished whether reporting from daemon or tool, added another errno return value - EUCLEAN meaning database version is higher then expected

2 - fixed

3 - fixed

new patch attached

O.

--
Ondrej Kos
Associate Software Engineer
Identity Management
Red Hat Czech

phone: +420-532-294-558
cell:  +420-736-417-909
ext:   82-62558
loc:   1013 Brno 1 office
irc:   okos @ #brno
From 653182042a6e02f2730abb89374ee5efaa4c1c10 Mon Sep 17 00:00:00 2001
From: Ondrej Kos <o...@redhat.com>
Date: Thu, 8 Nov 2012 14:34:36 +0100
Subject: [PATCH] Display more information on DB version crash

https://fedorahosted.org/sssd/ticket/1589

Added check for determining, whether database version is higher or
lower than expected. To distinguish it from other errors it uses
following retun values (further used for appropriate error message):
EMEDIUMTYPE for lower version than expected
EUCLEAN for higher version than expected

When SSSD or one of it's tools fails on DB version mismatch, new error
message is showed suggesting how to proceed.
---
 src/db/sysdb.c                          | 11 +++++++++--
 src/monitor/monitor.c                   |  1 +
 src/responder/common/responder_common.c |  3 +++
 src/tools/sss_cache.c                   |  3 +++
 src/tools/sss_seed.c                    |  3 +++
 src/tools/tools_util.c                  |  3 +++
 src/util/util.c                         | 24 ++++++++++++++++++++++++
 src/util/util.h                         |  2 ++
 8 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/src/db/sysdb.c b/src/db/sysdb.c
index 9685163b3f00cc2a45617072efa353589de111ce..0f3d0dbdbeabcfbaec6e07cf7e8026baee8cdc40 100644
--- a/src/db/sysdb.c
+++ b/src/db/sysdb.c
@@ -947,6 +947,13 @@ errno_t sysdb_add_to_domain(struct sss_domain_info *domain,
     return EOK;
 }
 
+errno_t
+sysdb_version_diff(const char *expected,
+                           const char *received)
+{
+    return (atof(expected) < atof(received)) ? EUCLEAN : EMEDIUMTYPE;
+}
+
 int sysdb_domain_init_internal(TALLOC_CTX *mem_ctx,
                                struct sss_domain_info *domain,
                                const char *db_path,
@@ -1037,7 +1044,7 @@ int sysdb_domain_init_internal(TALLOC_CTX *mem_ctx,
             if (!allow_upgrade) {
                 DEBUG(0, ("Wrong DB version (got %s expected %s)\n",
                           version, SYSDB_VERSION));
-                ret = EINVAL;
+                ret = sysdb_version_diff(SYSDB_VERSION, version);
                 goto done;
             }
 
@@ -1136,7 +1143,7 @@ int sysdb_domain_init_internal(TALLOC_CTX *mem_ctx,
 
         DEBUG(0,("Unknown DB version [%s], expected [%s] for domain %s!\n",
                  version?version:"not found", SYSDB_VERSION, domain->name));
-        ret = EINVAL;
+        ret = sysdb_version_diff(SYSDB_VERSION, version);
         goto done;
     }
 
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 956efb13ceb29a9361e60526e934883e0ecb5a16..b1ab0b9700cd71df6805ead4520ccfaa97c9464d 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -2113,6 +2113,7 @@ int monitor_process_init(struct mt_ctx *ctx,
     }
     ret = sysdb_init(tmp_ctx, ctx->cdb, NULL, true, &db_list);
     if (ret != EOK) {
+        if (ret == EMEDIUMTYPE || ret == EUCLEAN) sss_db_version_mismatch(ret, false);
         return ret;
     }
     talloc_zfree(tmp_ctx);
diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c
index d9f73fe2698cd7eacfb1a589c159e5cb037dab64..fa7cbc0aa98bd6cde91b758e26845c2f7d56b485 100644
--- a/src/responder/common/responder_common.c
+++ b/src/responder/common/responder_common.c
@@ -839,6 +839,9 @@ int sss_process_init(TALLOC_CTX *mem_ctx,
 
     ret = sysdb_init(rctx, cdb, NULL, false, &rctx->db_list);
     if (ret != EOK) {
+        if (ret == EMEDIUMTYPE || ret == EUCLEAN) {
+            sss_db_version_mismatch(ret, false);
+        }
         DEBUG(0, ("fatal error initializing resp_ctx\n"));
         return ret;
     }
diff --git a/src/tools/sss_cache.c b/src/tools/sss_cache.c
index 06b0099dc642812752949aa6cb8781ac1349ad90..bc97bf7b5886435a8481414ad569e5d61ad8c050 100644
--- a/src/tools/sss_cache.c
+++ b/src/tools/sss_cache.c
@@ -339,6 +339,9 @@ errno_t init_domains(struct cache_tool_ctx *ctx, const char *domain)
         ret = sysdb_init_domain_and_sysdb(ctx, ctx->confdb, domain, DB_PATH,
                                           &ctx->domains, &db_ctx);
         if (ret != EOK) {
+            if (ret == EMEDIUMTYPE || ret == EUCLEAN) {
+                sss_db_version_mismatch(ret, true);
+            }
             DEBUG(1, ("Could not initialize connection to the sysdb\n"));
             goto fail;
         }
diff --git a/src/tools/sss_seed.c b/src/tools/sss_seed.c
index cd1b263863e44411e7aac8772c3922178ad9dca4..ee79a49c37dcc39b4e9485b36bdf052ee103f246 100644
--- a/src/tools/sss_seed.c
+++ b/src/tools/sss_seed.c
@@ -631,6 +631,9 @@ static int seed_init_db(TALLOC_CTX *mem_ctx,
     ret = sysdb_init_domain_and_sysdb(tmp_ctx, confdb, domain_name,
                                       DB_PATH, &domain, &sysdb);
     if (ret != EOK) {
+        if (ret == EMEDIUMTYPE || ret == EUCLEAN) {
+            sss_db_version_mismatch(ret, true);
+        }
         DEBUG(SSSDBG_CRIT_FAILURE,
               ("Could not initialize connection to domain '%s' in sysdb.%s\n",
                domain_name, ret == ENOENT ? " Domain not found." : ""));
diff --git a/src/tools/tools_util.c b/src/tools/tools_util.c
index 99b79f171d82cde5f1c58988949422981e4d5ee3..81430c1a4dd8fe0448df3f884aee3e412996e614 100644
--- a/src/tools/tools_util.c
+++ b/src/tools/tools_util.c
@@ -57,6 +57,9 @@ static int setup_db(struct tools_ctx *ctx)
     ret = sysdb_init_domain_and_sysdb(ctx, ctx->confdb, "local", DB_PATH,
                                       &ctx->local, &ctx->sysdb);
     if (ret != EOK) {
+        if (ret == EMEDIUMTYPE || ret == EUCLEAN) {
+            sss_db_version_mismatch(ret, true);
+        }
         DEBUG(1, ("Could not initialize connection to the sysdb\n"));
         return ret;
     }
diff --git a/src/util/util.c b/src/util/util.c
index b812ef1b118803129caae25247efcbc1194b22f5..373b239fd1813bf010ca6e33f227daba0ac21e40 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -634,3 +634,27 @@ remove_ipv6_brackets(char *ipv6addr)
 
     return EOK;
 }
+
+/* Print out database version mismatch error message
+ * according to expected one and environment */
+void
+sss_db_version_mismatch(errno_t equ_type,
+                        bool in_tool)
+{
+    ERROR("DB version mismatch!\n");
+    switch (equ_type) {
+        case EUCLEAN:
+            ERROR("Lower version of database is expected.\n");
+            break;
+        case EMEDIUMTYPE:
+            ERROR("Higher version of database is expected. %s\n",
+                    in_tool ? "You could try running SSSD, which should "
+                    "handle the upgrade itself." : "");
+            break;
+        default:
+            break;
+    }
+    ERROR("Removing cached files in /var/run/sss/db/ should fix the issue, "
+            "but be aware, that deleting cache files also removes all of your "
+            "cached credentials.\n");
+}
diff --git a/src/util/util.h b/src/util/util.h
index b6ecfc2c952bd6a37f49194d961b3f762c9546e7..e16fc3b25b2afd0529fd1fc1633530a5022a4baa 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -360,6 +360,8 @@ void talloc_log_fn(const char *msg);
 
 void sss_log(int priority, const char *format, ...);
 
+void sss_db_version_mismatch(errno_t equ_type, bool in_tool);
+
 /* from server.c */
 struct main_context {
     struct tevent_context *event_ctx;
-- 
1.7.11.7

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to