On 09/14/2016 04:00 PM, Lukas Slebodnik wrote:
On (06/09/16 13:15), Petr Cech wrote:
On 09/05/2016 02:31 PM, Fabiano Fidêncio wrote:
On Mon, Sep 5, 2016 at 11:59 AM, Fabiano Fidêncio <fiden...@example.com> wrote:
Petr,
I went through your patches and in general they look good to me.
However, I haven't done any tests yet with your patches (and I'll do
it after lunch).
I've done some tests and I've been able to see the ldif changes in the
domain log. So, I assume it's working.
For sure it's a good improvement! Would be worth to link some
documentation about ldiff as it may be confusing for someone who is
not used to it.
I'll wait for a new version of the patches and go through them again.
I really would like to have someone's else opinion on this series.
Please, below you can see a few comments. Feel completely free to
ignore the first one if you feel like doing it, it's just a minor :-)
For the other comments, I'd like to understand a few changes you have done.
Patch 0001: SYSDB: Adding message to inform which cache is used
About the following part of the patch:
+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;
+}
I personally don't like this kind of comparison done with flags. I'd
go for something like: if ((state_mask & SSS_SYSDB_BOTH_CACHE) != 0)
...
But this is a really minor and feel free to ignore it.
Patch 0002: SYSDB: Adding message about reason why cache changed
LGTM
Patch 0003: SYSDB: Adding wrappers for ldb_* operations
About the following parts of the patch:
On src/db/sysdb_ldb_wrapper.c
+#define ERR_FN_ENOMEM (-1 * ENOMEM)
+#define ERR_FN_ENOENT (-1 * ENOENT)
Why? I failed to understand why you're doing this here.
+ if (print_ctx == NULL) {
+ return -1;
+ return ERR_FN_ENOMEM;
+ }
I guess the return -1 is a leftover :-)
+ if (print_ctx->ldif == NULL) {
+ return -2;
+ return ERR_FN_ENOENT;
+ }
I guess the return -2 is also a leftover :-)
+ if (ret < 0) {
+ DEBUG(SSSDBG_MINOR_FAILURE, "ldb_ldif_write() failed with [%d][%s].\n",
+ -1 * ret, sss_strerror(-1 * ret));
+ goto done;
+ }
And here again this dance multiplying by -1 that I don't understand
the reason :-\
+done:
+ if (ldb_print_ctx != NULL && ldb_print_ctx->ldif != NULL) {
+ talloc_free(ldb_print_ctx->ldif);
+ }
+ talloc_free(ldb_print_ctx);
AFAIU talloc_free can gracefully handle NULL. Considering that's the
case I'd just check for (if ldb_print_ctx != NULL)
talloc_free(ldb_print_ctx->ldif);
Considering it doesn't, we may have some issues on trying to free
(ldb_print_ctx)
On src/db/sysdb_ldb_wrapper.h:
+int sss_ldb_rename(struct ldb_context *ldb,
+ struct ldb_dn * olddn,
+ struct ldb_dn *newdn);
Just a really minor codying style change here, remove the extra space
between * and olddn: struct ldb_dn * olddn, -> struct ldb_dn *olddn,
Patch0004: SYSDB: ldb_add --> sss_ldb_add in sysdb
Patch0005: SYSDB: ldb_delete --> sss_ldb_delete in sysdb
Patch0006: SYSDB: ldb_modify --> sss_ldb_modify in sysdb
Patch0007: SYSDB: ldb_rename --> sss_ldb_rename in sysdb
LGTM
Best Regards,
--
Fabiano Fidêncio
Hello,
there is new patch set attached.
I replaced all ldb_* to new wrapper in whole code.
Regards
--
Petr^4 Čech
From 529b0d3009f8310b8257d5a69639a0fafa30140c Mon Sep 17 00:00:00 2001
From: Petr Cech <pc...@example.com>
Date: Tue, 16 Aug 2016 09:32:18 +0200
Subject: [PATCH 1/7] 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
5d9c9fb24a149f8215b3027dcb4b0e1a183e4b43..847b663bdb2ec31de3eb3b4c33e2b942145a4c42
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) != 0) {
Let's assume that we will add new type of cache in future
(e.g. SSS_SYSDB_SECRET_CACHE)
If the value of "state_mask" was CACHE | TS_CACHE SECRET_CACHE
then this condition would be true but return incorrent string.
So, I did it more dynamic way now. See attached patch please.
+ storage = "cache, ts_cache";
+ } else if ((state_mask != SSS_SYSDB_TS_CACHE) != 0) {
+ storage = "ts_cache";
+ } else if ((state_mask &= SSS_SYSDB_CACHE) != 0) {
+ 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 abad5d0a7c730f8eb0699509ed21e559e6896f12 Mon Sep 17 00:00:00 2001
From: Petr Cech <pc...@example.com>
Date: Tue, 16 Aug 2016 09:33:46 +0200
Subject: [PATCH 2/7] SYSDB: Adding message about reason why cache changed
Resolves:
https://fedorahosted.org/sssd/ticket/3060
---
src/db/sysdb.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/src/db/sysdb.c b/src/db/sysdb.c
index
6f0b1b9e9b52bede68f03cb5674f65b91cc28c98..a76e8b47afc902d6c0c0ed5302b7f9231a11ade3
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,9 @@ 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 +1865,9 @@ 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: attr [%s] is replaced or
extended.\n",
^^
more then 80 columns
Addressed.
+ ldb_dn_get_linearized(entry_dn), mod_msg_el->name);
return true;
}
}
@@ -1869,6 +1876,9 @@ 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);
return true;
}
break;
@@ -1892,10 +1902,16 @@ bool sysdb_entry_attrs_diff(struct sysdb_ctx *sysdb,
const char *attrnames[attrs->num+1];
if (sysdb->ldb_ts == NULL) {
+ DEBUG(SSSDBG_TRACE_FUNC,
+ "Entry [%s] differs, reason: there is no ts_cache yet.\n",
+ ldb_dn_get_linearized(entry_dn));
return true;
}
if (is_ts_ldb_dn(entry_dn) == false) {
+ DEBUG(SSSDBG_TRACE_FUNC,
+ "Entry [%s] differs, reason: ts_cache doesn't trace this type of
entry.\n",
+ ldb_dn_get_linearized(entry_dn));
return true;
}
@@ -1930,7 +1946,7 @@ bool sysdb_entry_attrs_diff(struct sysdb_ctx *sysdb,
goto done;
}
- differs = sysdb_ldb_msg_difference(res->msgs[0], new_entry_msg);
+ differs = sysdb_ldb_msg_difference(entry_dn, res->msgs[0], new_entry_msg);
done:
talloc_free(tmp_ctx);
return differs;
--
2.7.4
We should also increase the debug_level for messages.
Addressed.
@see
[root@host ~]# grep "Entry \[" /var/log/sssd/sssd_example.com.log
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sysdb_ldb_msg_difference]
(0x0400): Entry [name=lsleb...@example.com,cn=users,cn=example.com,cn=sysdb]
differs, reason: attr [originalDN] is new.
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sysdb_set_entry_attr]
(0x0200): Entry [name=lsleb...@example.com,cn=users,cn=example.com,cn=sysdb]
has set [cache, ts_cache] attrs.
(Wed Sep 14 15:47:11 2016) [sssd[be[example.com]]] [sysdb_set_entry_attr]
(0x0200): Entry [name=lsleb...@example.com,cn=users,cn=example.com,cn=sysdb]
has set [cache, ts_cache] attrs.
(Wed Sep 14 15:47:11 2016) [sssd[be[example.com]]] [sysdb_ldb_msg_difference]
(0x0400): Entry [name=lsleb...@example.com,cn=groups,cn=example.com,cn=sysdb]
differs, reason: attr [lastUpdate] is new.
(Wed Sep 14 15:47:11 2016) [sssd[be[example.com]]] [sysdb_set_entry_attr]
(0x0200): Entry [name=lsleb...@example.com,cn=groups,cn=example.com,cn=sysdb]
has set [cache, ts_cache] attrs.
(Wed Sep 14 15:47:11 2016) [sssd[be[example.com]]] [sysdb_ldb_msg_difference]
(0x0400): Entry [name=de...@example.com,cn=groups,cn=example.com,cn=sysdb]
differs, reason: attr [lastUpdate] is new.
(Wed Sep 14 15:47:11 2016) [sssd[be[example.com]]] [sysdb_set_entry_attr]
(0x0200): Entry [name=de...@example.com,cn=groups,cn=example.com,cn=sysdb] has
set [cache, ts_cache] attrs.
(Wed Sep 14 15:47:11 2016) [sssd[be[example.com]]] [sysdb_ldb_msg_difference]
(0x0400): Entry
[name=idm-dev-...@example.com,cn=groups,cn=example.com,cn=sysdb] differs,
reason: attr [lastUpdate] is new.
(Wed Sep 14 15:47:11 2016) [sssd[be[example.com]]] [sysdb_set_entry_attr]
(0x0200): Entry
[name=idm-dev-...@example.com,cn=groups,cn=example.com,cn=sysdb] has set
[cache, ts_cache] attrs.
(Wed Sep 14 15:47:11 2016) [sssd[be[example.com]]] [sysdb_ldb_msg_difference]
(0x0400): Entry [name=lsleb...@example.com,cn=users,cn=example.com,cn=sysdb]
differs, reason: attr [initgrExpireTimestamp] is new.
(Wed Sep 14 15:47:11 2016) [sssd[be[example.com]]] [sysdb_set_entry_attr]
(0x0200): Entry [name=lsleb...@example.com,cn=users,cn=example.com,cn=sysdb]
has set [cache, ts_cache] attrs.
(Wed Sep 14 15:47:15 2016) [sssd[be[example.com]]] [sysdb_set_entry_attr]
(0x0200): Entry [name=lsleb...@example.com,cn=users,cn=example.com,cn=sysdb]
has set [cache, ts_cache] attrs.
(Wed Sep 14 15:47:15 2016) [sssd[be[example.com]]] [sysdb_set_entry_attr]
(0x0200): Entry [name=lsleb...@example.com,cn=users,cn=example.com,cn=sysdb]
has set [cache, ts_cache] attrs.
(Wed Sep 14 15:47:15 2016) [sssd[be[example.com]]] [sysdb_ldb_msg_difference]
(0x0400): Entry [name=lsleb...@example.com,cn=users,cn=example.com,cn=sysdb]
differs, reason: attr [ccacheFile] is new.
(Wed Sep 14 15:47:15 2016) [sssd[be[example.com]]] [sysdb_set_entry_attr]
(0x0200): Entry [name=lsleb...@example.com,cn=users,cn=example.com,cn=sysdb]
has set [cache, ts_cache] attrs.
(Wed Sep 14 15:47:15 2016) [sssd[be[example.com]]] [sysdb_ldb_msg_difference]
(0x0400): Entry [name=lsleb...@example.com,cn=users,cn=example.com,cn=sysdb]
differs, reason: attr [cachedPassword] is new.
(Wed Sep 14 15:47:15 2016) [sssd[be[example.com]]] [sysdb_set_entry_attr]
(0x0200): Entry [name=lsleb...@example.com,cn=users,cn=example.com,cn=sysdb]
has set [cache, ts_cache] attrs.
(Wed Sep 14 15:47:15 2016) [sssd[be[example.com]]] [sysdb_ldb_msg_difference]
(0x0400): Entry [name=lsleb...@example.com,cn=groups,cn=example.com,cn=sysdb]
differs, reason: attr [originalModifyTimestamp] is new.
(Wed Sep 14 15:47:15 2016) [sssd[be[example.com]]] [sysdb_set_entry_attr]
(0x0200): Entry [name=lsleb...@example.com,cn=groups,cn=example.com,cn=sysdb]
has set [cache, ts_cache] attrs.
(Wed Sep 14 15:47:23 2016) [sssd[be[example.com]]] [sysdb_set_entry_attr]
(0x0200): Entry [name=lsleb...@example.com,cn=users,cn=example.com,cn=sysdb]
has set [cache, ts_cache] attrs.
(Wed Sep 14 15:47:23 2016) [sssd[be[example.com]]] [sysdb_set_entry_attr]
(0x0200): Entry [name=lsleb...@example.com,cn=users,cn=example.com,cn=sysdb]
has set [cache, ts_cache] attrs.
(Wed Sep 14 15:47:23 2016) [sssd[be[example.com]]] [sysdb_set_entry_attr]
(0x0200): Entry [name=lsleb...@example.com,cn=users,cn=example.com,cn=sysdb]
has set [cache, ts_cache] attrs.
(Wed Sep 14 15:47:23 2016) [sssd[be[example.com]]] [sysdb_ldb_msg_difference]
(0x0400): Entry [name=lsleb...@example.com,cn=users,cn=example.com,cn=sysdb]
differs, reason: attr [cachedPassword] is replaced or extended.
(Wed Sep 14 15:47:23 2016) [sssd[be[example.com]]] [sysdb_set_entry_attr]
(0x0200): Entry [name=lsleb...@example.com,cn=users,cn=example.com,cn=sysdb]
has set [cache, ts_cache] attrs.
Other simillar debug messages use much higher debug level
e.g.
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sdap_attrs_add_ldap_attr]
(0x2000): shadowLastChange is not available for [lsleb...@example.com].
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sdap_attrs_add_ldap_attr]
(0x2000): shadowMin is not available for [lsleb...@example.com].
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sdap_attrs_add_ldap_attr]
(0x2000): shadowMax is not available for [lsleb...@example.com].
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sdap_attrs_add_ldap_attr]
(0x2000): shadowWarning is not available for [lsleb...@example.com].
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sdap_attrs_add_ldap_attr]
(0x2000): shadowInactive is not available for [lsleb...@example.com].
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sdap_attrs_add_ldap_attr]
(0x2000): shadowExpire is not available for [lsleb...@example.com].
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sdap_attrs_add_ldap_attr]
(0x2000): shadowFlag is not available for [lsleb...@example.com].
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sdap_attrs_add_ldap_attr]
(0x2000): krbLastPwdChange is not available for [lsleb...@example.com].
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sdap_attrs_add_ldap_attr]
(0x2000): krbPasswordExpiration is not available for [lsleb...@example.com].
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sdap_attrs_add_ldap_attr]
(0x2000): pwdAttribute is not available for [lsleb...@example.com].
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sdap_attrs_add_ldap_attr]
(0x2000): authorizedService is not available for [lsleb...@example.com].
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sdap_attrs_add_ldap_attr]
(0x2000): adAccountExpires is not available for [lsleb...@example.com].
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sdap_attrs_add_ldap_attr]
(0x2000): adUserAccountControl is not available for [lsleb...@example.com].
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sdap_attrs_add_ldap_attr]
(0x2000): nsAccountLock is not available for [lsleb...@example.com].
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sdap_attrs_add_ldap_attr]
(0x2000): authorizedHost is not available for [lsleb...@example.com].
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sdap_attrs_add_ldap_attr]
(0x2000): ndsLoginDisabled is not available for [lsleb...@example.com].
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sdap_attrs_add_ldap_attr]
(0x2000): ndsLoginExpirationTime is not available for [lsleb...@example.com].
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sdap_attrs_add_ldap_attr]
(0x2000): ndsLoginAllowedTimeMap is not available for [lsleb...@example.com].
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sdap_attrs_add_ldap_attr]
(0x2000): sshPublicKey is not available for [lsleb...@example.com].
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sdap_attrs_add_ldap_attr]
(0x2000): authType is not available for [lsleb...@example.com].
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sdap_attrs_add_ldap_attr]
(0x2000): userCertificate is not available for [lsleb...@example.com].
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sdap_attrs_add_ldap_attr]
(0x2000): Adding mail [lsleb...@example.com] to attributes of
[lsleb...@example.com].
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [sysdb_attrs_get_aliases]
(0x2000): Domain is case-insensitive; will add lowercased aliases
(Wed Sep 14 15:47:10 2016) [sssd[be[example.com]]] [is_email_from_domain]
(0x4000): Email [lsleb...@example.com] is from domain [example.com].
BTW new debug messages are 200 collumns long (in average).
and most of users use longer domains then example.com.
Is it intentional? Could it be shorter a little bit or it would
lower usability of debug messages.
> LS
Right. I changed order of information. Now the entry_dn is last one.
It could help. I mean important information comes first.
Regards
--
Petr^4 Čech
>From faf0049dda737e9d5bb539c26a1d0cdc78956f2e Mon Sep 17 00:00:00 2001
From: Petr Cech <pc...@redhat.com>
Date: Tue, 16 Aug 2016 09:32:18 +0200
Subject: [PATCH 1/3] SYSDB: Adding message to inform which cache is used
Resolves:
https://fedorahosted.org/sssd/ticket/3060
---
src/db/sysdb_ops.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 81 insertions(+)
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 5d9c9fb24a149f8215b3027dcb4b0e1a183e4b43..32666f5d67621ec1f7f8122a27a087eacf4ca08a 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -27,6 +27,11 @@
#include "util/cert.h"
#include <time.h>
+
+#define SSS_SYSDB_NO_CACHE 0x0
+#define SSS_SYSDB_CACHE 0x1
+#define SSS_SYSDB_TS_CACHE 0x2
+
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,14 +1181,66 @@ done:
return ret;
}
+static errno_t get_attr_storage(TALLOC_CTX *mem_ctx,
+ int state_mask,
+ char **_storage)
+{
+ TALLOC_CTX *tmp_ctx;
+ char **storage_list = NULL;
+ char *storage;
+ errno_t ret;
+
+ tmp_ctx = talloc_new(NULL);
+ if (!tmp_ctx) {
+ ret = ENOMEM;
+ goto done;
+ }
+
+ if ((state_mask |= SSS_SYSDB_CACHE) != 0) {
+ ret = add_string_to_list(tmp_ctx, "cache", &storage_list);
+ if (ret != EOK) {
+ ret = ENOMEM;
+ goto done;
+ }
+ }
+
+ if ((state_mask |= SSS_SYSDB_TS_CACHE) != 0) {
+ ret = add_string_to_list(tmp_ctx, "ts_cache", &storage_list);
+ if (ret != EOK) {
+ ret = ENOMEM;
+ goto done;
+ }
+ }
+
+ storage = talloc_strdup(tmp_ctx, "");
+ if (storage_list != NULL) {
+ for (int i = 0; storage_list[i] != NULL; i++) {
+ storage = talloc_strdup_append(storage, storage_list[i]);
+ if (storage_list[i + 1] != NULL) {
+ storage = talloc_strdup_append(storage, ", ");
+ }
+ }
+ }
+
+ *_storage = talloc_steal(mem_ctx, storage);
+ ret = EOK;
+
+done:
+ talloc_zfree(tmp_ctx);
+ return ret;
+}
+
int sysdb_set_entry_attr(struct sysdb_ctx *sysdb,
struct ldb_dn *entry_dn,
struct sysdb_attrs *attrs,
int mod_op)
{
+ TALLOC_CTX *tmp_ctx;
bool sysdb_write = true;
errno_t ret = EOK;
errno_t tret = EOK;
+ int state_mask = SSS_SYSDB_NO_CACHE;
+ char *used_storage = NULL;
sysdb_write = sysdb_entry_attrs_diff(sysdb, entry_dn, attrs, mod_op);
if (sysdb_write == true) {
@@ -1192,6 +1249,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 +1260,31 @@ 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) {
+
+ tmp_ctx = talloc_new(NULL);
+ if (tmp_ctx == NULL) {
+ goto done;
+ }
+
+ ret = get_attr_storage(tmp_ctx, state_mask, &used_storage);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_MINOR_FAILURE, "Function get_attr_storage failed.\n");
+ talloc_zfree(tmp_ctx);
+ goto done;
+ }
+ DEBUG(SSSDBG_TRACE_INTERNAL, "Attrs [%s] set for entry [%s].\n",
+ used_storage,
+ ldb_dn_get_linearized(entry_dn));
+ talloc_zfree(tmp_ctx);
+ }
+
+done:
return ret;
}
--
2.7.4
>From 6c17abefd53df76adcf7f32e1bbe0affb9cf96b6 Mon Sep 17 00:00:00 2001
From: Petr Cech <pc...@redhat.com>
Date: Tue, 16 Aug 2016 09:33:46 +0200
Subject: [PATCH 2/3] SYSDB: Adding message about reason why cache changed
Resolves:
https://fedorahosted.org/sssd/ticket/3060
---
src/db/sysdb.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/src/db/sysdb.c b/src/db/sysdb.c
index 6f0b1b9e9b52bede68f03cb5674f65b91cc28c98..b67769ed11fc0796d1987f09aa568c2db4a0ffab 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,9 @@ 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_INTERNAL,
+ "Added attr [%s] to entry [%s]\n",
+ mod_msg_el->name, ldb_dn_get_linearized(entry_dn));
return true;
}
break;
@@ -1855,12 +1859,15 @@ static bool sysdb_ldb_msg_difference(struct ldb_message *db_msg,
el_differs = ldb_msg_element_compare(db_msg_el, mod_msg_el);
if (el_differs) {
- /* We are replacing or extending element, there is a difference. If
- * some values already exist and ldb_add is not permissive,
+ /* We are replacing or extending element, there is a difference.
+ * If some values already exist and ldb_add is not permissive,
* ldb will throw an error, but that's not our job to check..
*/
if (is_ts_cache_attr(mod_msg_el->name) == false) {
/* We can ignore changes to timestamp attributes */
+ DEBUG(SSSDBG_TRACE_INTERNAL,
+ "Replaced/extended attr [%s] of entry [%s]\n",
+ mod_msg_el->name, ldb_dn_get_linearized(entry_dn) );
return true;
}
}
@@ -1869,6 +1876,9 @@ 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_INTERNAL,
+ "Deleted attr [%s] of entry [%s].\n",
+ mod_msg_el->name, ldb_dn_get_linearized(entry_dn));
return true;
}
break;
@@ -1892,10 +1902,16 @@ bool sysdb_entry_attrs_diff(struct sysdb_ctx *sysdb,
const char *attrnames[attrs->num+1];
if (sysdb->ldb_ts == NULL) {
+ DEBUG(SSSDBG_TRACE_FUNC,
+ "Entry [%s] differs, reason: there is no ts_cache yet.\n",
+ ldb_dn_get_linearized(entry_dn));
return true;
}
if (is_ts_ldb_dn(entry_dn) == false) {
+ DEBUG(SSSDBG_TRACE_FUNC,
+ "Entry [%s] differs, reason: ts_cache doesn't trace this type of entry.\n",
+ ldb_dn_get_linearized(entry_dn));
return true;
}
@@ -1930,7 +1946,7 @@ bool sysdb_entry_attrs_diff(struct sysdb_ctx *sysdb,
goto done;
}
- differs = sysdb_ldb_msg_difference(res->msgs[0], new_entry_msg);
+ differs = sysdb_ldb_msg_difference(entry_dn, res->msgs[0], new_entry_msg);
done:
talloc_free(tmp_ctx);
return differs;
--
2.7.4
>From 2db403fa78a44ee76036c96f4064ea51a116c6d1 Mon Sep 17 00:00:00 2001
From: Petr Cech <pc...@redhat.com>
Date: Tue, 16 Aug 2016 14:59:06 +0200
Subject: [PATCH 3/3] SYSDB: Adding wrappers for ldb_* operations
This patch adds 4 wrappers:
* sss_ldb_add()
* sss_ldb_delete()
* sss_ldb_modify()
* sss_ldb_rename()
Those wrappers write ldif message to log if
debug_level = SSSDBG_LDB.
Adding and modifying produce full ldif.
Deleting and renaming produce only short message.
If SSSDBG_LDB is not set, wrappers collapse to normal
ldb_* functions without additonial memory consumption.
Resolves:
https://fedorahosted.org/sssd/ticket/3060
---
Makefile.am | 2 +
src/db/sysdb_ldb_wrapper.c | 138 +++++++++++++++++++++++++++++++++++++++++++++
src/db/sysdb_ldb_wrapper.h | 35 ++++++++++++
src/util/debug.h | 1 +
4 files changed, 176 insertions(+)
create mode 100644 src/db/sysdb_ldb_wrapper.c
create mode 100644 src/db/sysdb_ldb_wrapper.h
diff --git a/Makefile.am b/Makefile.am
index f89af5a9d6d26c732574aa3651de8c175f538b28..f9f510e26abaee4cc46c6c65e40180baf3dd2d99 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -644,6 +644,7 @@ dist_noinst_HEADERS = \
src/db/sysdb_private.h \
src/db/sysdb_services.h \
src/db/sysdb_ssh.h \
+ src/db/sysdb_ldb_wrapper.h \
src/confdb/confdb.h \
src/confdb/confdb_private.h \
src/confdb/confdb_setup.h \
@@ -892,6 +893,7 @@ pkglib_LTLIBRARIES += libsss_util.la
libsss_util_la_SOURCES = \
src/confdb/confdb.c \
src/db/sysdb.c \
+ src/db/sysdb_ldb_wrapper.c \
src/db/sysdb_ops.c \
src/db/sysdb_search.c \
src/db/sysdb_selinux.c \
diff --git a/src/db/sysdb_ldb_wrapper.c b/src/db/sysdb_ldb_wrapper.c
new file mode 100644
index 0000000000000000000000000000000000000000..9aa32648987a241088d2701298e7404a76db2d52
--- /dev/null
+++ b/src/db/sysdb_ldb_wrapper.c
@@ -0,0 +1,138 @@
+/*
+ SSSD
+
+ System Database -- ldb wrappers
+
+ Copyright (C) Petr Cech <pc...@redhat.com> 2016
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <string.h>
+#include <ldb.h>
+#include "util/util.h"
+
+struct sss_ldif_vprint_ctx {
+ char *ldif;
+};
+
+static int ldif_vprintf_fn(void *private_data, const char *fmt, ...)
+{
+ struct sss_ldif_vprint_ctx *print_ctx;
+ va_list ap;
+ int lenght = 0;
+
+ /* Note that the function should return the number of
+ * bytes written, or a negative error code.
+ */
+
+ print_ctx = talloc_get_type(private_data, struct sss_ldif_vprint_ctx);
+
+ if (print_ctx == NULL) {
+ return (-1 * ENOMEM);
+ }
+
+ if (fmt != NULL) {
+ va_start(ap, fmt);
+
+ if (print_ctx->ldif != NULL) {
+ lenght = strlen(print_ctx->ldif);
+ }
+
+ print_ctx->ldif = talloc_vasprintf_append_buffer(print_ctx->ldif,
+ fmt, ap);
+ if (print_ctx->ldif == NULL) {
+ return (-1 * ENOENT);
+ }
+
+ lenght = strlen(print_ctx->ldif) - lenght;
+ va_end(ap);
+ }
+
+ return lenght;
+}
+
+static void sss_ldb_ldif2log(enum ldb_changetype changetype,
+ struct ldb_context *ldb,
+ const struct ldb_message *message)
+{
+ int ret;
+ struct ldb_ldif ldif;
+ struct sss_ldif_vprint_ctx *ldb_print_ctx;
+
+ ldb_print_ctx = talloc_zero(ldb, struct sss_ldif_vprint_ctx);
+ if (ldb_print_ctx == NULL) {
+ return;
+ }
+ ldb_print_ctx->ldif = NULL;
+
+ ldif.changetype = changetype;
+ ldif.msg = discard_const_p(struct ldb_message, message);
+
+ ret = ldb_ldif_write(ldb, ldif_vprintf_fn, ldb_print_ctx, &ldif);
+ if (ret < 0) {
+ ret = -1 * ret;
+ DEBUG(SSSDBG_MINOR_FAILURE, "ldb_ldif_write() failed with [%d][%s].\n",
+ ret, sss_strerror(ret));
+ goto done;
+ }
+
+ DEBUG(SSSDBG_LDB, "ldif\n[\n%s\n]\n", ldb_print_ctx->ldif);
+
+done:
+ talloc_free(ldb_print_ctx->ldif);
+ talloc_free(ldb_print_ctx);
+
+ return;
+}
+
+int sss_ldb_add(struct ldb_context *ldb, const struct ldb_message *message)
+{
+ if (DEBUG_IS_SET(SSSDBG_LDB) == true) {
+ sss_ldb_ldif2log(LDB_CHANGETYPE_ADD, ldb, message);
+ }
+
+ return ldb_add(ldb, message);
+}
+
+int sss_ldb_delete(struct ldb_context *ldb, struct ldb_dn *dn)
+{
+ if (DEBUG_IS_SET(SSSDBG_LDB) == true) {
+ DEBUG(SSSDBG_LDB, "Deleting [%s]\n", ldb_dn_get_rdn_name(dn));
+ }
+
+ return ldb_delete(ldb, dn);
+}
+
+int sss_ldb_modify(struct ldb_context *ldb, const struct ldb_message *message)
+{
+ if (DEBUG_IS_SET(SSSDBG_LDB) == true) {
+ sss_ldb_ldif2log(LDB_CHANGETYPE_MODIFY, ldb, message);
+ }
+
+ return ldb_modify(ldb, message);
+}
+
+int sss_ldb_rename(struct ldb_context *ldb,
+ struct ldb_dn *olddn,
+ struct ldb_dn *newdn)
+{
+ if (DEBUG_IS_SET(SSSDBG_LDB) == true) {
+ DEBUG(SSSDBG_LDB, "Renaming [%s] to [%s]\n",
+ ldb_dn_get_rdn_name(olddn),
+ ldb_dn_get_rdn_name(newdn));
+ }
+
+ return ldb_rename(ldb, olddn, newdn);
+}
diff --git a/src/db/sysdb_ldb_wrapper.h b/src/db/sysdb_ldb_wrapper.h
new file mode 100644
index 0000000000000000000000000000000000000000..07a42f0f110ded75c4ad5048e23dd13429a449c1
--- /dev/null
+++ b/src/db/sysdb_ldb_wrapper.h
@@ -0,0 +1,35 @@
+/*
+ SSSD
+
+ System Database -- ldb wrappers
+
+ Copyright (C) Petr Cech <pc...@redhat.com> 2016
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __SYSDB_LDB_WRAPPER_H__
+#define __SYSDB_LDB_WRAPPER_H__
+
+int sss_ldb_add(struct ldb_context *ldb, const struct ldb_message *message);
+
+int sss_ldb_delete(struct ldb_context *ldb, struct ldb_dn *dn);
+
+int sss_ldb_modify(struct ldb_context *ldb, const struct ldb_message *message);
+
+int sss_ldb_rename(struct ldb_context *ldb,
+ struct ldb_dn *olddn,
+ struct ldb_dn *newdn);
+
+#endif /* __SYSDB_LDB_WRAPPER_H__ */
diff --git a/src/util/debug.h b/src/util/debug.h
index 2a1bd4ffd30817d7128805996c21105fe40982a2..610a65b6b6b9a41c9d7062b118abe5f015e10d68 100644
--- a/src/util/debug.h
+++ b/src/util/debug.h
@@ -67,6 +67,7 @@ int get_fd_from_debug_file(void);
#define SSSDBG_TRACE_INTERNAL 0x2000 /* level 8 */
#define SSSDBG_TRACE_ALL 0x4000 /* level 9 */
#define SSSDBG_BE_FO 0x8000 /* level 9 */
+#define SSSDBG_LDB 0x10000 /* level 9 */
#define SSSDBG_IMPORTANT_INFO SSSDBG_OP_FAILURE
#define SSSDBG_INVALID -1
--
2.7.4
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
