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

[thierry bordaz]

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.

    Thank you
    thierry

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