On 08/16/2016 01:22 PM, Petr Cech wrote:
>
>
> 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.
Hello,
there is new patch number 3 which is a WIP.
I would like to ask you which version
of ldif printing do you prefer.
You can see [1] which shows the outputs.
Difference is that version A is step by step
reading of ldb messsage.
But, version B uses function
ldb_ldif_write(). This function needs
my_vprintf_fn() but the information isn't structured.
What do you prefer? :-)
[1] ldif.txt
Regards
--
Petr^4 Čech