On (16/08/16 12:52), Petr Cech wrote:
On 08/16/2016 10:15 AM, Jakub Hrozek wrote:
On Tue, Aug 16, 2016 at 09:50:19AM +0200, Petr Cech wrote:
Hello list,
I am solving ticket [1] now. There are three points mentioned. A have prepared patches for the first two. I would like to ask anybody it is right or if I miss something.
The third point is about full LDIFF in special debug level. What does it mean 'special debug level'? Is it new option, for example?
[1] https://fedorahosted.org/sssd/ticket/3060
Regards
Please no magic constants in SSSD code :)
Hello Jakub,
there is fixed version without magic :-)
-- Petr^4 'magician' Čech
From 2ca78a82c579c5244aebd9a58b56a9886f6bc4b5 Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Tue, 16 Aug 2016 09:32:18 +0200 Subject: [PATCH 1/2] SYSDB: Adding message to inform which cache is used
Resolves: https://fedorahosted.org/sssd/ticket/3060
src/db/sysdb_ops.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c index 44fb5b70e6d33fffbca5824f831a3229254ecb57..a81840b2515d09f91d1dfa783bcf08f0fad112b4 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -27,6 +27,12 @@ #include "util/cert.h" #include <time.h>
+#define SSS_SYSDB_NO_CACHE 0x0 +#define SSS_SYSDB_CACHE 0x1 +#define SSS_SYSDB_TS_CACHE 0x2 +#define SSS_SYSDB_BOTH_CACHE (SSS_SYSDB_CACHE | SSS_SYSDB_TS_CACHE)
static uint32_t get_attr_as_uint32(struct ldb_message *msg, const char *attr) { const struct ldb_val *v = ldb_msg_find_ldb_val(msg, attr); @@ -1176,6 +1182,21 @@ done: return ret; }
+static const char *get_attr_storage(int state_mask) +{
- const char *storage = "";
- if (state_mask == SSS_SYSDB_BOTH_CACHE ) {
storage = "cache, ts_cache";
- } else if (state_mask == SSS_SYSDB_TS_CACHE) {
storage = "ts_cache";
- } else if (state_mask == SSS_SYSDB_CACHE) {
storage = "cache";
- }
- return storage;
+}
int sysdb_set_entry_attr(struct sysdb_ctx *sysdb, struct ldb_dn *entry_dn, struct sysdb_attrs *attrs, @@ -1184,6 +1205,7 @@ int sysdb_set_entry_attr(struct sysdb_ctx *sysdb, bool sysdb_write = true; errno_t ret = EOK; errno_t tret = EOK;
int state_mask = SSS_SYSDB_NO_CACHE;
sysdb_write = sysdb_entry_attrs_diff(sysdb, entry_dn, attrs, mod_op); if (sysdb_write == true) {
@@ -1192,6 +1214,8 @@ int sysdb_set_entry_attr(struct sysdb_ctx *sysdb, DEBUG(SSSDBG_MINOR_FAILURE, "Cannot set attrs for %s, %d [%s]\n", ldb_dn_get_linearized(entry_dn), ret, sss_strerror(ret));
} else {
}state_mask |= SSS_SYSDB_CACHE; }
@@ -1201,9 +1225,17 @@ int sysdb_set_entry_attr(struct sysdb_ctx *sysdb, DEBUG(SSSDBG_MINOR_FAILURE, "Cannot set ts attrs for %s\n", ldb_dn_get_linearized(entry_dn)); /* Not fatal */
} else {
state_mask |= SSS_SYSDB_TS_CACHE; }
}
if (state_mask != SSS_SYSDB_NO_CACHE) {
DEBUG(SSSDBG_FUNC_DATA, "Entry [%s] has set [%s] attrs.\n",
ldb_dn_get_linearized(entry_dn),
get_attr_storage(state_mask));
}
return ret;
}
-- 2.7.4
From 6e7143b26fb5696a9b684c0da96353a7d5d07700 Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Tue, 16 Aug 2016 09:33:46 +0200 Subject: [PATCH 2/2] SYSDB: Adding message about reason why cache changed
Resolves: https://fedorahosted.org/sssd/ticket/3060
src/db/sysdb.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/src/db/sysdb.c b/src/db/sysdb.c index 6f0b1b9e9b52bede68f03cb5674f65b91cc28c98..9d1abc2b3dd0ce5db626544673795eebfbc28bcd 100644 --- a/src/db/sysdb.c +++ b/src/db/sysdb.c @@ -1821,7 +1821,8 @@ bool sysdb_msg_attrs_modts_differs(struct ldb_message *old_entry, return true; }
-static bool sysdb_ldb_msg_difference(struct ldb_message *db_msg, +static bool sysdb_ldb_msg_difference(struct ldb_dn *entry_dn,
struct ldb_message *db_msg, struct ldb_message *mod_msg)
{ struct ldb_message_element *mod_msg_el; @@ -1848,6 +1849,10 @@ static bool sysdb_ldb_msg_difference(struct ldb_message *db_msg, */ if (mod_msg_el->num_values > 0) { /* We can ignore additions of timestamp attributes */
DEBUG(SSSDBG_TRACE_FUNC, "Entry [%s] differs, reason: " \
"attr [%s] is new.\n",
ldb_dn_get_linearized(entry_dn),
mod_msg_el->name); return true; } break;
@@ -1861,6 +1866,11 @@ static bool sysdb_ldb_msg_difference(struct ldb_message *db_msg, */ if (is_ts_cache_attr(mod_msg_el->name) == false) { /* We can ignore changes to timestamp attributes */
DEBUG(SSSDBG_TRACE_FUNC, "Entry [%s] differs, reason: " \
^^ this is not required It will be concatenated even without it.
"attr [%s] is replaced " \
"or extended.\n",
ldb_dn_get_linearized(entry_dn),
mod_msg_el->name);
return true; } }
@@ -1869,6 +1879,10 @@ static bool sysdb_ldb_msg_difference(struct ldb_message *db_msg, db_msg_el = ldb_msg_find_element(db_msg, mod_msg_el->name); if (db_msg_el != NULL) { /* We are deleting a valid element, there is a difference */
DEBUG(SSSDBG_TRACE_FUNC, "Entry [%s] differs, reason: " \
"attr [%s] is deleted.\n",
ldb_dn_get_linearized(entry_dn),
mod_msg_el->name);
a) it's not alligned properly (1 extra space :-) b) IMH0; it's it's easier to read the debug messages in nested code if it start on next line. e.g. DEBUG(SSSDBG_TRACE_FUNC, "Entry [%s] differs, reason: attr [%s] is deleted.\n", ldb_dn_get_linearized(entry_dn), mod_msg_el->name);
LS