[Freeipa-devel] [PATCH 0192-0196] Write all changes to journal

[Petr Spacek]

On 13.12.2013 17:44, Petr Spacek wrote:
> On 23.10.2013 17:20, Petr Spacek wrote:
>> On 23.10.2013 17:12, Tomas Hozza wrote:
>>> On 10/10/2013 07:05 PM, Petr Spacek wrote:
>>>> Hello,
>>>>
>>>> this patch set adds journaling to bind-dyndb-ldap.
>>>>
>>>> Journaling requires proper SOA serial maintenance, so from now
>>>> SOA serial auto-incrementation is mandatory.
>>>>
>>>> Journal file is deleted on each start, so IXFR is limited to time frame
>>>> from last BIND start.
>>>>
>>>> https://fedorahosted.org/bind-dyndb-ldap/ticket/64
>>>>
>>>>
>>>> You can use my personal branch for testing:
>>>> https://github.com/spacekpe/bind-dyndb-ldap/tree/rbtdb.v7
>>>>
>>>> It contains all unpushed patches. Basically, next master should be identical
>>>> to this branch.
>>>>
>>>> Thank you for your time!
>>>>
>>>> -- Petr^2 Spacek
>>>
>>> ACK.
>>>
>>> IXFR works as should. Also tested that journal is cleared after BIND
>>> restart, so server responds with AXFR.
>>>
>>> Patches look good, I have only one small thing to patch 0193:
>>>
>>>> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
>>>> index
>>>> 0786979a1970f4b61ac5b92dd5554bf87032d1ff..89acbe610519bbe0610a07d5fa5d4ffceddc71cd
>>>>
>>>> 100644
>>>>
>>>> --- a/src/ldap_helper.c
>>>>
>>>> +++ b/src/ldap_helper.c
>>>>
>>>> ...
>>>>
>>>> @@ -1401,7 +1405,155 @@
>>>>
>>>> cleanup:
>>>> return result;
>>>> }
>>>> +/**
>>>> + * Process strictly minimal diff and detect if data were changed
>>>> + * and return latest SOA RR.
>>>> + *
>>>> + * @pre Input diff has to be minimal, i.e. it can't contain DEL & ADD
>>>> operation
>>>> + * for the same data under the same name and TTL.
>>>> + *
>>>> + * @pre If the tuple list contains SOA RR, then exactly one SOA RR
>>>> deletion
>>>> + * has to precede exactly one SOA RR addition.
>>>> + * (Each SOA RR deletion has to have matching addition.)
>>>> + *
>>>> + * @param[in] diff Input diff. List of tuples can be empty.
>>>> + * @param[out] soa_latest Pointer to last added SOA RR from tuple list.
>>>> + * Result can be NULL if there is no added SOA RR
>>>> + * in the tuple list.
>>>> + * @param[out] data_changed ISC_TRUE if any data other than SOA serial
>>>> were
>>>> + * changed. ISC_FALSE if nothing (except SOA
>>>> + * serial) was changed.
>>>> + *
>>>> + */
>>>> +static isc_result_t ATTR_NONNULLS
>>>> +diff_analyze_serial(dns_diff_t *diff, dns_difftuple_t **soa_latest,
>>>> + isc_boolean_t *data_changed) {
>>>> + dns_difftuple_t *t = NULL;
>>>> + dns_rdata_t *del_soa = NULL; /* last seen SOA with op == DEL */
>>>> + dns_difftuple_t *tmp_tuple = NULL; /* tuple used for SOA comparison */
>>>> + isc_result_t result = ISC_R_SUCCESS;
>>>> + int ret;
>>>> +
>>>> + REQUIRE(DNS_DIFF_VALID(diff));
>>>> + REQUIRE(soa_latest != NULL && *soa_latest == NULL);
>>>> + REQUIRE(data_changed != NULL);
>>>> +
>>>> + *data_changed = ISC_FALSE;
>>>> + for (t = HEAD(diff->tuples);
>>>> + t != NULL;
>>>> + t = NEXT(t, link)) {
>>>> + INSIST(tmp_tuple == NULL);
>>> after this ^^^ line tmp_tuple is NULL in the current for loop cycle.
>>>> + if (t->rdata.type != dns_rdatatype_soa)
>>>> + *data_changed = ISC_TRUE;
>>>> + else { /* SOA is always special case */
>>>> + if (t->op == DNS_DIFFOP_DEL ||
>>>> + t->op == DNS_DIFFOP_DELRESIGN) {
>>>> + /* delete operation has to precede add */
>>>> + INSIST(del_soa == NULL);
>>>> + INSIST(tmp_tuple == NULL);
>>> so this ^^^ check is duplicit
>>>> + del_soa = &t->rdata;
>>>> + } else if (t->op == DNS_DIFFOP_ADD ||
>>>> + t->op == DNS_DIFFOP_ADDRESIGN) {
>>>> + /* add operation has to follow a delete */
>>>> + INSIST(del_soa != NULL);
>>>> + INSIST(tmp_tuple == NULL);
>>> and also this ^^^ check is duplicit
>>>> + *soa_latest = t;
>>>> +
>>>> + /* detect if fields other than serial
>>>> + * were changed (compute only if necessary) */
>>>> + if (*data_changed == ISC_FALSE) {
>>>> + CHECK(dns_difftuple_copy(t, &tmp_tuple));
>>>> + dns_soa_setserial(dns_soa_getserial(del_soa),
>>>> + &tmp_tuple->rdata);
>>>> + ret = dns_rdata_compare(del_soa,
>>>> + &tmp_tuple->rdata);
>>>> + *data_changed = ISC_TF(ret != 0);
>>>> + }
>>>> + if (tmp_tuple != NULL)
>>>> + dns_difftuple_free(&tmp_tuple);
>>>> + /* re-start the SOA delete-add search cycle */
>>>> + del_soa = NULL;
>>>> + } else {
>>>> + INSIST("unexpected diff: op != ADD || DEL"
>>>> + == NULL);
>>>> + }
>>>> + }
>>>> + }
>>>> + /* SOA deletions & additions has to create self-contained couples */
>>>> + INSIST(del_soa == NULL && tmp_tuple == NULL);
>>>> +
>>>> +cleanup:
>>>> + if (tmp_tuple != NULL)
>>>> + dns_difftuple_free(&tmp_tuple);
>>>> + return result;
>>>> +}
>>>> +
>>>> ...
>>>
>>> Sorry for the formatting, obviously my email client is not perfect.
>>> Hope you can understand it...
>>
>> I will fix it before push.
>
> Patch 192 was unchanged.
>
> Patch 193 v2
> - redundant INSIST calls were removed
> - patch was rebased
>
> Patch 194 v2
> - copyright header was fixed
>
> Patch 196 v2
> - README was updated
> - missing header dns/journal.h was added
> - fixes in error handling: journal is correctly closed as necessary
> - patch was rebased

Pushed to master branch:
08e3b9deb31a060c85bba69539b68916181895d4
d881f7395c592f57589d049fb2733416f266dc3a
211954a5c6ce1e5cbf99520cb8b97a49000acfc3
29563e6b09459b9d4e2e65cc512a022d56a711ce
76bc77f0b0318590424e0c997528b742d888768e

-- 
Petr^2 Spacek