[Freeipa-devel] [PATCH 0064] Create ipa-otp-decrement 389DS plugin

[Martin Kosek]

On 10/14/2014 09:01 PM, Nathaniel McCallum wrote:
> On Thu, 2014-10-09 at 18:48 +0200, thierry bordaz wrote:
>> On 10/09/2014 05:51 PM, Nathaniel McCallum wrote:
>>
>>> On Thu, 2014-10-09 at 11:44 +0200, thierry bordaz wrote:
>>>> On 10/09/2014 12:15 AM, Nathaniel McCallum wrote:
>>>>
>>>>> On Wed, 2014-10-08 at 17:19 -0400, Simo Sorce wrote:
>>>>>> On Wed, 08 Oct 2014 15:53:39 -0400
>>>>>> Nathaniel McCallum <npmccallum at redhat.com> wrote:
>>>>>>
>>>>>>> As I understand my code, all servers will have csnD. Some servers will
>>>>>>> have valueB and others will have valueD, but valueB == valueD.
>>>>>>>
>>>>>>> We *never* discard a CSN. We only discard the counter/watermark mods
>>>>>>> in the replication operation.
>>>>>> What Thierry is saying is that the individual attributes in the entry
>>>>>> have associate the last CSN that modified them. Because you remove the
>>>>>> mods when ValueD == ValueB the counter attribute will not have the
>>>>>> associated CSN changed. But it doesn't really matter because the plugin
>>>>>> will always keep things consistent.
>>>>> Attached is a new version. It removes this optimization. If server X has
>>>>> valueB/csnB and receives valueD/csnD and valueB == valueD, the
>>>>> replication will be applied without any modification. However, if valueB
>>>>>> valueD and csnD > csnB, the counter mods will still be stripped.
>>>>> It also collapses the error check from betxnpre to bepre now that we
>>>>> have a fix for https://fedorahosted.org/389/ticket/47919 committed. The
>>>>> betxnpre functions are completely removed. Also, a dependency on 389
>>>>> 1.3.3.4 (not yet released) is added.
>>>>>
>>>>> Nathaniel
>>>> Hello Nathaniel,
>>>>
>>>>         For me the code is fine. Ack.
>>> New attached patch.
>>>
>>>>         I have two minor comments:
>>>>               * in preop_mod, when a direct update moves the counter
>>>>                 backward you send UNWILLING to perform with a message.
>>>>                 The message is allocated with slapi_ch_smprintf, you
>>>>                 may free it with slapi_ch_free_string (rather than
>>>>                 'free').
>>> Fixed.
>>>
>>>>               * About this message, for example when you have these
>>>>                 MODS (I admit they make no sens):
>>>>                 
>>>>                 changetype: modify
>>>>                 ipatokenHOTPcounter: MOD_DELETE
>>>>                 -
>>>>                 ipatokenHOTPcounter: MOD_INCREMENT
>>>>                 
>>>>                 The returned message will be "Will not decrement
>>>>                 ipatokenHOTPcounter", because 'simulate' will return
>>>>                 'COUNTER_UNSET+1'.
>>>>                 Is it the message you expected ?
>>> I changed the logic in simulate(). Please review it.
>>>
>>> Nathaniel
>>>
>> Hello Nathaniel,
>>
>>
>>         The patch is ok for me. Ack.
> 
> Since the ACK, the upstream 389 fix actually landed in 1.3.3.5. This
> patch changes nothing except the dependency version. I have tested it
> against the 1.3.3.5 build.
> 
> Nathaniel

Great! As soon as the new build land in Fedora 21 (and we add it to our Copr),
the patch can be pushed.

Martin