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

[thierry bordaz]

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.

    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').
      * 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 ?

thanks
thierry
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141009/afdd92de/attachment.htm>