On 11/14/2012 11:53 AM, Jan Cholasta wrote:
On 13.11.2012 15:41, Ondrej Kos wrote:
On 11/12/2012 01:14 PM, Jan Cholasta wrote:
On 9.11.2012 13:24, Ondrej Kos wrote:
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.
+errno_t
+sysdb_version_diff(const char *expected,
+ const char *received)
+{
+ return (atof(expected) < atof(received)) ? EUCLEAN : EMEDIUMTYPE;
+}
+
I don't think you can use atof here, as it is locale-dependent. For
example, when the current locale is cs_CZ, then atof("0.13") == 0.0,
because in Czech the decimal mark is ",", so atof will stop parsing when
it hits the ".".
I think it would be nice (though not necessary) to add a generic
function for comparing version strings to util and use it for the
comparison.
You use this:
+ if (ret == EMEDIUMTYPE || ret == EUCLEAN) {
+ sss_db_version_mismatch(ret, true);
+ }
everywhere in the patch, I think you should make a macro out of it.
Please add src/util/util.c to po/POTFILES.in, so that gettext can pick
up the error messages you have added for translation.
Honza
New patch attached (without adding src/util/util.c to po/POTFILES.in
since there's new bug filed for it)
O.
I'm afraid this is not a very good solution:
+double
+sss_db_version_str_to_num(char *str)
+{
+ int i;
+ char *c;
+ bool sep_occured = false;
+
+ for (i = 0, c = str; c[i]; i++) {
+ if (c[i] == ',' || c[i] == '.') {
+ if (sep_occured) {
+ return NAN;
+ }
+ c[i] = '.';
+ sep_occured = true;
+ } else if (!isdigit(c[i])) {
+ return NAN;
+ }
+ }
+ return atof(str);
+}
This fixes it for Czech, but what about languages that do not use "."
and neither "," as a decimal mark? Also I don't think using floats for
this is a particularly good idea. I think we can use something like this
instead:
errno_t
sss_db_version_check(const char *expected,
const char *received)
{
int ret;
unsigned int exp_major, exp_minor, recv_major, recv_minor;
ret = sscanf(expected, "%u.%u", &exp_major, &exp_minor);
if (ret != 2) {
return EINVAL;
}
ret = sscanf(receibed, "%u.%u", &recv_major, &recv_minor);
if (ret != 2) {
return EINVAL;
}
if (exp_major < recv_major) {
return EUCLEAN;
} else if (exp_major > recv_major) {
return EMEDIUMTYPE;
}
if (exp_minor < recv_minor) {
return EUCLEAN;
} else if (exp_minor > recv_minor) {
return EMEDIUMTYPE;
}
return EOK;
}
Honza
I misunderstood the situation with decimal mark. Here's new patch.
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 818842d03ddff1e6b79102a5932e12b23fec2dfe 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 | 4 +--
src/monitor/monitor.c | 1 +
src/responder/common/responder_common.c | 1 +
src/tools/sss_cache.c | 1 +
src/tools/sss_seed.c | 1 +
src/tools/tools_util.c | 1 +
src/util/util.c | 55 +++++++++++++++++++++++++++++++++
src/util/util.h | 11 +++++++
8 files changed, 73 insertions(+), 2 deletions(-)
diff --git a/src/db/sysdb.c b/src/db/sysdb.c
index 9685163b3f00cc2a45617072efa353589de111ce..ee359a01447f974b54e6d748ae5b191dba9441fa 100644
--- a/src/db/sysdb.c
+++ b/src/db/sysdb.c
@@ -1037,7 +1037,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 = sss_db_version_check(SYSDB_VERSION, version);
goto done;
}
@@ -1136,7 +1136,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 = sss_db_version_check(SYSDB_VERSION, version);
goto done;
}
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 956efb13ceb29a9361e60526e934883e0ecb5a16..29d4222d2eb6403c0018fa2c0180cc1dacbbf14a 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) {
+ SSS_DB_MIS_CHECK(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..eb528d2d854ffb0159ef5a8f9f628ca4be0614b5 100644
--- a/src/responder/common/responder_common.c
+++ b/src/responder/common/responder_common.c
@@ -839,6 +839,7 @@ int sss_process_init(TALLOC_CTX *mem_ctx,
ret = sysdb_init(rctx, cdb, NULL, false, &rctx->db_list);
if (ret != EOK) {
+ SSS_DB_MIS_CHECK(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..bd5edbc89a06cd5dbab814fdbc7cceb541b52382 100644
--- a/src/tools/sss_cache.c
+++ b/src/tools/sss_cache.c
@@ -339,6 +339,7 @@ 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) {
+ SSS_DB_MIS_CHECK(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..ae80f43f75ce1e5a8ab5bf3c30d54fef15ccdf91 100644
--- a/src/tools/sss_seed.c
+++ b/src/tools/sss_seed.c
@@ -631,6 +631,7 @@ 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) {
+ SSS_DB_MIS_CHECK(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..649b8eb9f96963d575662cb8b62dadb607a8b3b1 100644
--- a/src/tools/tools_util.c
+++ b/src/tools/tools_util.c
@@ -57,6 +57,7 @@ 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) {
+ SSS_DB_MIS_CHECK(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..11c02b9f8474a87d2ee5e0909252aa956982d4eb 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -634,3 +634,58 @@ remove_ipv6_brackets(char *ipv6addr)
return EOK;
}
+
+errno_t
+sss_db_version_check(const char *expected,
+ const char *received)
+{
+ int ret;
+ unsigned int exp_major, exp_minor, recv_major, recv_minor;
+
+ ret = sscanf(expected, "%u.%u", &exp_major, &exp_minor);
+ if (ret != SSS_DB_CHECK_PTS) {
+ return EINVAL;
+ }
+ ret = sscanf(received, "%u.%u", &recv_major, &recv_minor);
+ if (ret != SSS_DB_CHECK_PTS) {
+ return EINVAL;
+ }
+
+ if (recv_major > exp_major) {
+ return EUCLEAN;
+ } else if (recv_major < exp_major) {
+ return EMEDIUMTYPE;
+ }
+
+ if (recv_minor > exp_minor) {
+ return EUCLEAN;
+ } else if (recv_minor < exp_minor) {
+ return EMEDIUMTYPE;
+ }
+
+ 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..c825e7dd162edd8ca2af283d8f3fd127542e64d2 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -360,6 +360,17 @@ void talloc_log_fn(const char *msg);
void sss_log(int priority, const char *format, ...);
+errno_t sss_db_version_check(const char *expected, const char *received);
+
+void sss_db_version_mismatch(errno_t equ_type, bool in_tool);
+
+#define SSS_DB_CHECK_PTS 2
+
+#define SSS_DB_MIS_CHECK(ret, in_tool) \
+ if (ret == EMEDIUMTYPE || ret == EUCLEAN) { \
+ sss_db_version_mismatch(ret, 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
