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

[Tomas Hozza]

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...

Regards,
Tomas