On 08/16/2016 01:02 PM, Lukas Slebodnik wrote:
> 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(a)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(a)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
Thanks, Lukas.
Fixed patch set is attached. Some debug messages crossed 80 characters :-(
Regards
There were little misspelling, sorry.
Fixed patches are attached.
--
Petr^4 Čech