From: Alessio Podda Date: Fri, 24 Oct 2025 22:47:42 +0000 (+0200) Subject: Abstract updates into a vtable X-Git-Tag: v9.21.17~47^2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e36dc0ca761898ff951d101281bbf39bf2535ec5;p=thirdparty%2Fbind9.git Abstract updates into a vtable This commit adds a layer of indirection to the apply_diff logic used by IXFR and resigning by having the database updates go through a vtable. We do this in three steps: - We extend dns_rdatacallbacks_t vtable to allow subtraction and resigning. - We add a new set of api (begin|commit|abort)update to the dbmethods vtable, that model an incremental update that can be aborted. - We extract the core logic of diff_apply into a function that satisfies the new interface. - We make diff_apply use this new function, and log the results. The intent of this commit is to allow databases to expose a batch incremental update implementation, just like they expose a custom batch creation implementation through (begin|end)load. --- diff --git a/lib/dns/db.c b/lib/dns/db.c index 80f505d02bb..85758bcf2f5 100644 --- a/lib/dns/db.c +++ b/lib/dns/db.c @@ -300,6 +300,56 @@ dns_db_endload(dns_db_t *db, dns_rdatacallbacks_t *callbacks) { return ISC_R_NOTIMPLEMENTED; } +isc_result_t +dns_db_beginupdate(dns_db_t *db, dns_rdatacallbacks_t *callbacks) { + /* + * Begin updating 'db'. + */ + + REQUIRE(DNS_DB_VALID(db)); + REQUIRE(dns_db_iszone(db)); + REQUIRE(DNS_CALLBACK_VALID(callbacks)); + + if (db->methods->beginupdate != NULL) { + return (db->methods->beginupdate)(db, callbacks); + } + return ISC_R_NOTIMPLEMENTED; +} + +isc_result_t +dns_db_commitupdate(dns_db_t *db, dns_rdatacallbacks_t *callbacks) { + /* + * Commit the update to 'db'. + */ + + REQUIRE(DNS_DB_VALID(db)); + REQUIRE(dns_db_iszone(db)); + REQUIRE(DNS_CALLBACK_VALID(callbacks)); + + if (db->methods->commitupdate != NULL) { + return (db->methods->commitupdate)(db, callbacks); + } + + return ISC_R_NOTIMPLEMENTED; +} + +isc_result_t +dns_db_abortupdate(dns_db_t *db, dns_rdatacallbacks_t *callbacks) { + /* + * Abort the update to 'db'. + */ + + REQUIRE(DNS_DB_VALID(db)); + REQUIRE(dns_db_iszone(db)); + REQUIRE(DNS_CALLBACK_VALID(callbacks)); + + if (db->methods->abortupdate != NULL) { + return (db->methods->abortupdate)(db, callbacks); + } + + return ISC_R_NOTIMPLEMENTED; +} + isc_result_t dns_db_load(dns_db_t *db, const char *filename, dns_masterformat_t format, unsigned int options) { diff --git a/lib/dns/diff.c b/lib/dns/diff.c index a3364c433c9..f89d245a677 100644 --- a/lib/dns/diff.c +++ b/lib/dns/diff.c @@ -203,7 +203,6 @@ dns_diff_appendminimal(dns_diff_t *diff, dns_difftuple_t **tuplep) { } } - static void getownercase(dns_rdataset_t *rdataset, dns_name_t *name) { if (dns_rdataset_isassociated(rdataset)) { @@ -211,6 +210,80 @@ getownercase(dns_rdataset_t *rdataset, dns_name_t *name) { } } +static isc_result_t +update_rdataset(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, + dns_rdataset_t *rds, dns_diffop_t op) { + isc_result_t result; + unsigned int options; + dns_rdataset_t ardataset; + dns_dbnode_t *node = NULL; + bool is_resign; + + dns_rdataset_init(&ardataset); + + is_resign = rds->type == dns_rdatatype_rrsig && + (op == DNS_DIFFOP_DELRESIGN || op == DNS_DIFFOP_ADDRESIGN); + + if (rds->type != dns_rdatatype_nsec3 && + rds->covers != dns_rdatatype_nsec3) + { + CHECK(dns_db_findnode(db, name, true, &node)); + } else { + CHECK(dns_db_findnsec3node(db, name, true, &node)); + } + + switch (op) { + case DNS_DIFFOP_ADD: + case DNS_DIFFOP_ADDRESIGN: + options = DNS_DBADD_MERGE | DNS_DBADD_EXACT | + DNS_DBADD_EXACTTTL; + CHECK(dns_db_addrdataset(db, node, ver, 0, rds, options, + &ardataset)); + break; + case DNS_DIFFOP_DEL: + case DNS_DIFFOP_DELRESIGN: + options = DNS_DBSUB_EXACT | DNS_DBSUB_WANTOLD; + result = dns_db_subtractrdataset(db, node, ver, rds, options, + &ardataset); + switch (result) { + case ISC_R_SUCCESS: + case DNS_R_UNCHANGED: + case DNS_R_NXRRSET: + getownercase(&ardataset, name); + CHECK(result); + break; + default: + CHECK(result); + break; + } + break; + default: + UNREACHABLE(); + } + + if (is_resign) { + isc_stdtime_t resign; + resign = dns_rdataset_minresign(&ardataset); + dns_db_setsigningtime(db, &ardataset, resign); + } + +cleanup: + if (node != NULL) { + dns_db_detachnode(&node); + } + if (dns_rdataset_isassociated(&ardataset)) { + dns_rdataset_disassociate(&ardataset); + } + return result; +} + +static isc_result_t +update_callback(void *arg, const dns_name_t *name, dns_rdataset_t *rds, + dns_diffop_t op DNS__DB_FLARG) { + dns_updatectx_t *ctx = arg; + return update_rdataset(ctx->db, ctx->ver, (dns_name_t *)name, rds, op); +} + static const char * optotext(dns_diffop_t op) { switch (op) { @@ -228,23 +301,24 @@ optotext(dns_diffop_t op) { } static isc_result_t -diff_apply(const dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver, - bool warn) { +diff_apply(const dns_diff_t *diff, dns_rdatacallbacks_t *callbacks) { dns_difftuple_t *t; - dns_dbnode_t *node = NULL; isc_result_t result; char namebuf[DNS_NAME_FORMATSIZE]; char typebuf[DNS_RDATATYPE_FORMATSIZE]; char classbuf[DNS_RDATACLASS_FORMATSIZE]; + dns_updatectx_t *ctx; REQUIRE(DNS_DIFF_VALID(diff)); - REQUIRE(DNS_DB_VALID(db)); + REQUIRE(callbacks != NULL); + REQUIRE(callbacks->update != NULL); + + ctx = callbacks->add_private; t = ISC_LIST_HEAD(diff->tuples); while (t != NULL) { dns_name_t *name; - INSIST(node == NULL); name = &t->name; /* * Find the node. @@ -261,8 +335,6 @@ diff_apply(const dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver, dns_diffop_t op; dns_rdatalist_t rdl; dns_rdataset_t rds; - dns_rdataset_t ardataset; - unsigned int options; op = t->op; type = t->rdata.type; @@ -290,16 +362,6 @@ diff_apply(const dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver, rdl.rdclass = t->rdata.rdclass; rdl.ttl = t->ttl; - node = NULL; - if (type != dns_rdatatype_nsec3 && - covers != dns_rdatatype_nsec3) - { - CHECK(dns_db_findnode(db, name, true, &node)); - } else { - CHECK(dns_db_findnsec3node(db, name, true, - &node)); - } - while (t != NULL && dns_name_equal(&t->name, name) && t->op == op && t->rdata.type == type && rdata_covers(&t->rdata) == covers) @@ -309,7 +371,7 @@ diff_apply(const dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver, * dns_rdataset_setownercase. */ name = &t->name; - if (t->ttl != rdl.ttl && warn) { + if (t->ttl != rdl.ttl && ctx->warn) { dns_name_format(name, namebuf, sizeof(namebuf)); dns_rdatatype_format(t->rdata.type, @@ -338,7 +400,6 @@ diff_apply(const dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver, * Convert the rdatalist into a rdataset. */ dns_rdataset_init(&rds); - dns_rdataset_init(&ardataset); dns_rdatalist_tordataset(&rdl, &rds); dns_rdataset_setownercase(&rds, name); rds.trust = dns_trust_ultimate; @@ -346,42 +407,16 @@ diff_apply(const dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver, /* * Merge the rdataset into the database. */ - switch (op) { - case DNS_DIFFOP_ADD: - case DNS_DIFFOP_ADDRESIGN: - options = DNS_DBADD_MERGE | DNS_DBADD_EXACT | - DNS_DBADD_EXACTTTL; - result = dns_db_addrdataset(db, node, ver, 0, - &rds, options, - &ardataset); - break; - case DNS_DIFFOP_DEL: - case DNS_DIFFOP_DELRESIGN: - options = DNS_DBSUB_EXACT | DNS_DBSUB_WANTOLD; - result = dns_db_subtractrdataset(db, node, ver, - &rds, options, - &ardataset); - break; - default: - UNREACHABLE(); - } + result = callbacks->update(callbacks->add_private, name, + &rds, op DNS__DB_FILELINE); - if (result == ISC_R_SUCCESS) { - if (rds.type == dns_rdatatype_rrsig && - (op == DNS_DIFFOP_DELRESIGN || - op == DNS_DIFFOP_ADDRESIGN)) - { - isc_stdtime_t resign; - resign = dns_rdataset_minresign(&ardataset); - dns_db_setsigningtime(db, &ardataset, - resign); - } - if (op == DNS_DIFFOP_DEL || - op == DNS_DIFFOP_DELRESIGN) - { - getownercase(&ardataset, name); - } - } else if (result == DNS_R_UNCHANGED) { + switch (result) { + case ISC_R_SUCCESS: + /* + * OK. + */ + break; + case DNS_R_UNCHANGED: /* * This will not happen when executing a * dynamic update, because that code will @@ -390,13 +425,13 @@ diff_apply(const dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver, * from a server that is not as careful. * Issue a warning and continue. */ - if (warn) { - dns_name_format(dns_db_origin(db), + if (ctx->warn) { + dns_name_format(dns_db_origin(ctx->db), namebuf, sizeof(namebuf)); - dns_rdataclass_format(dns_db_class(db), - classbuf, - sizeof(classbuf)); + dns_rdataclass_format( + dns_db_class(ctx->db), classbuf, + sizeof(classbuf)); isc_log_write(DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_DIFF, ISC_LOG_WARNING, @@ -404,70 +439,70 @@ diff_apply(const dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver, "update with no effect", namebuf, classbuf); } - if (op == DNS_DIFFOP_DEL || - op == DNS_DIFFOP_DELRESIGN) - { - getownercase(&ardataset, name); - } - } else if (result == DNS_R_NXRRSET) { + result = ISC_R_SUCCESS; + break; + case DNS_R_NXRRSET: /* * OK. */ - if (op == DNS_DIFFOP_DEL || - op == DNS_DIFFOP_DELRESIGN) - { - getownercase(&ardataset, name); - } - if (dns_rdataset_isassociated(&ardataset)) { - dns_rdataset_disassociate(&ardataset); - } - } else { - if (result == DNS_R_NOTEXACT) { - dns_name_format(name, namebuf, - sizeof(namebuf)); - dns_rdatatype_format(type, typebuf, - sizeof(typebuf)); - dns_rdataclass_format(rdclass, classbuf, - sizeof(classbuf)); - isc_log_write( - DNS_LOGCATEGORY_GENERAL, - DNS_LOGMODULE_DIFF, - ISC_LOG_ERROR, - "dns_diff_apply: %s/%s/%s: %s " - "%s", - namebuf, typebuf, classbuf, - optotext(op), - isc_result_totext(result)); - } - if (dns_rdataset_isassociated(&ardataset)) { - dns_rdataset_disassociate(&ardataset); - } - CHECK(result); - } - dns_db_detachnode(&node); - if (dns_rdataset_isassociated(&ardataset)) { - dns_rdataset_disassociate(&ardataset); + result = ISC_R_SUCCESS; + break; + case DNS_R_NOTEXACT: + dns_name_format(name, namebuf, sizeof(namebuf)); + dns_rdatatype_format(type, typebuf, + sizeof(typebuf)); + dns_rdataclass_format(rdclass, classbuf, + sizeof(classbuf)); + isc_log_write(DNS_LOGCATEGORY_GENERAL, + DNS_LOGMODULE_DIFF, ISC_LOG_ERROR, + "dns_diff_apply: %s/%s/%s: %s " + "%s", + namebuf, typebuf, classbuf, + optotext(op), + isc_result_totext(result)); + break; + default: + break; } + + CHECK(result); } } return ISC_R_SUCCESS; cleanup: - if (node != NULL) { - dns_db_detachnode(&node); - } return result; } isc_result_t dns_diff_apply(const dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver) { - return diff_apply(diff, db, ver, true); + dns_updatectx_t ctx = { .db = db, .ver = ver, .warn = true }; + dns_rdatacallbacks_t callbacks; + dns_rdatacallbacks_init(&callbacks); + callbacks.update = update_callback; + callbacks.add_private = &ctx; + return diff_apply(diff, &callbacks); } isc_result_t dns_diff_applysilently(const dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver) { - return diff_apply(diff, db, ver, false); + dns_updatectx_t ctx = { .db = db, .ver = ver, .warn = false }; + dns_rdatacallbacks_t callbacks; + dns_rdatacallbacks_init(&callbacks); + callbacks.update = update_callback; + callbacks.add_private = &ctx; + return diff_apply(diff, &callbacks); +} + +isc_result_t +dns_diff_apply_with_callbacks(const dns_diff_t *diff, + dns_rdatacallbacks_t *callbacks) { + REQUIRE(DNS_DIFF_VALID(diff)); + REQUIRE(callbacks != NULL); + REQUIRE(callbacks->update != NULL); + + return diff_apply(diff, callbacks); } /* XXX this duplicates lots of code in diff_apply(). */ @@ -521,8 +556,9 @@ dns_diff_load(const dns_diff_t *diff, dns_rdatacallbacks_t *callbacks) { rds.trust = dns_trust_ultimate; INSIST(op == DNS_DIFFOP_ADD); - result = callbacks->add(callbacks->add_private, name, - &rds DNS__DB_FILELINE); + result = callbacks->update( + callbacks->add_private, name, &rds, + DNS_DIFFOP_ADD DNS__DB_FILELINE); if (result == DNS_R_UNCHANGED) { isc_log_write(DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_DIFF, diff --git a/lib/dns/include/dns/callbacks.h b/lib/dns/include/dns/callbacks.h index e9dce8c73a3..0ca5a92b392 100644 --- a/lib/dns/include/dns/callbacks.h +++ b/lib/dns/include/dns/callbacks.h @@ -34,18 +34,18 @@ struct dns_rdatacallbacks { unsigned int magic; /*% - * dns_load_master calls 'add' when it has an rdataset to add - * to the database. If defined, it calls 'setup' before and - * 'commit' after adding rdatasets. + * dns_load_master calls 'update' when it has an rdataset to update + * in the database. If defined, it calls 'setup' before and + * 'commit' after updating rdatasets. * * Some database implementations will commit each rdataset as - * soon as it's added, in which case 'setup' and 'commit' need + * soon as it's updated, in which case 'setup' and 'commit' need * not be defined. However, other implementations can be * optimized by grouping rdatasets into a transaction; the * setup and commit functions allow this transaction to be * opened and committed. */ - dns_addrdatasetfunc_t add; + dns_addrdatasetfunc_t update; dns_transactionfunc_t setup; dns_transactionfunc_t commit; diff --git a/lib/dns/include/dns/db.h b/lib/dns/include/dns/db.h index 8f0de40c5c0..975b0d91329 100644 --- a/lib/dns/include/dns/db.h +++ b/lib/dns/include/dns/db.h @@ -89,6 +89,12 @@ typedef struct dns_db_methods { isc_result_t (*beginload)(dns_db_t *db, dns_rdatacallbacks_t *callbacks); isc_result_t (*endload)(dns_db_t *db, dns_rdatacallbacks_t *callbacks); + isc_result_t (*beginupdate)(dns_db_t *db, + dns_rdatacallbacks_t *callbacks); + isc_result_t (*commitupdate)(dns_db_t *db, + dns_rdatacallbacks_t *callbacks); + isc_result_t (*abortupdate)(dns_db_t *db, + dns_rdatacallbacks_t *callbacks); void (*currentversion)(dns_db_t *db, dns_dbversion_t **versionp); isc_result_t (*newversion)(dns_db_t *db, dns_dbversion_t **versionp); void (*attachversion)(dns_db_t *db, dns_dbversion_t *source, @@ -530,6 +536,85 @@ dns_db_endload(dns_db_t *db, dns_rdatacallbacks_t *callbacks); * implementation used, syntax errors in the master file, etc. */ +isc_result_t +dns_db_beginupdate(dns_db_t *db, dns_rdatacallbacks_t *callbacks); +/*%< + * Begin updating 'db'. + * + * Requires: + * + * \li 'db' is a valid database. + * + * \li 'callbacks' is a pointer to an initialized dns_rdatacallbacks_t + * structure. + * + * Ensures: + * + * \li On success, callbacks->add will be a valid dns_addrdatasetfunc_t + * suitable for updating records in 'db' from IXFR operations. + * callbacks->add_private will be a valid DB update context + * which should be used as 'arg' when callbacks->add is called. + * + * Returns: + * + * \li #ISC_R_SUCCESS + * + * \li Other results are possible, depending upon the database + * implementation used. + */ + +isc_result_t +dns_db_commitupdate(dns_db_t *db, dns_rdatacallbacks_t *callbacks); +/*%< + * Commit the update to 'db'. Must be safe to double-call or call after + * dns_db_abortupdate. + * + * Requires: + * + * \li 'db' is a valid database that is being updated. + * + * \li 'callbacks' is a valid dns_rdatacallbacks_t structure. + * + * \li callbacks->add_private is not NULL and is a valid database update context. + * + * Ensures: + * + * \li 'callbacks' is returned to its state prior to calling dns_db_beginupdate() + * + * Returns: + * + * \li #ISC_R_SUCCESS + * + * \li Other results are possible, depending upon the database + * implementation used. + */ + +isc_result_t +dns_db_abortupdate(dns_db_t *db, dns_rdatacallbacks_t *callbacks); +/*%< + * Abort the update to 'db'. Must be safe to double-call or call after + * dns_db_commitupdate. + * + * Requires: + * + * \li 'db' is a valid database that is being updated. + * + * \li 'callbacks' is a valid dns_rdatacallbacks_t structure. + * + * \li callbacks->add_private is not NULL and is a valid database update context. + * + * Ensures: + * + * \li 'callbacks' is returned to its state prior to calling dns_db_beginupdate() + * + * Returns: + * + * \li #ISC_R_SUCCESS + * + * \li Other results are possible, depending upon the database + * implementation used. + */ + isc_result_t dns_db_load(dns_db_t *db, const char *filename, dns_masterformat_t format, unsigned int options); diff --git a/lib/dns/include/dns/diff.h b/lib/dns/include/dns/diff.h index e108f96f3e3..e2073f80bf7 100644 --- a/lib/dns/include/dns/diff.h +++ b/lib/dns/include/dns/diff.h @@ -59,13 +59,6 @@ * timeexpire. */ -typedef enum { - DNS_DIFFOP_ADD = 0, /*%< Add an RR. */ - DNS_DIFFOP_DEL = 1, /*%< Delete an RR. */ - DNS_DIFFOP_EXISTS = 2, /*%< Assert RR existence. */ - DNS_DIFFOP_ADDRESIGN = 4, /*%< ADD + RESIGN. */ - DNS_DIFFOP_DELRESIGN = 5 /*%< DEL + RESIGN. */ -} dns_diffop_t; typedef struct dns_difftuple dns_difftuple_t; typedef ISC_LIST(dns_difftuple_t) dns_difftuplelist_t; @@ -259,6 +252,24 @@ dns_diff_applysilently(const dns_diff_t *diff, dns_db_t *db, * */ +typedef struct { + dns_db_t *db; + dns_dbversion_t *ver; + bool warn; +} dns_updatectx_t; + +isc_result_t +dns_diff_apply_with_callbacks(const dns_diff_t *diff, dns_rdatacallbacks_t *callbacks); +/*%< + * Apply 'diff' to the database using the provided callbacks and context. + * The context contains the database, version, and warning flag. + * This allows for custom callback implementations. + * + * Requires: + *\li 'callbacks' points to a valid dns_rdatacallbacks_t structure + *\li 'callbacks->update' is not NULL + */ + isc_result_t dns_diff_load(const dns_diff_t *diff, dns_rdatacallbacks_t *callbacks); /*%< diff --git a/lib/dns/include/dns/types.h b/lib/dns/include/dns/types.h index 81dc8f95e23..ef1a5461543 100644 --- a/lib/dns/include/dns/types.h +++ b/lib/dns/include/dns/types.h @@ -183,6 +183,14 @@ typedef struct dst_gssapi_signverifyctx dst_gssapi_signverifyctx_t; typedef enum { dns_hash_sha1 = 1 } dns_hash_t; +typedef enum { + DNS_DIFFOP_ADD = 0, /*%< Add an RR. */ + DNS_DIFFOP_DEL = 1, /*%< Delete an RR. */ + DNS_DIFFOP_EXISTS = 2, /*%< Assert RR existence. */ + DNS_DIFFOP_ADDRESIGN = 4, /*%< ADD + RESIGN. */ + DNS_DIFFOP_DELRESIGN = 5 /*%< DEL + RESIGN. */ +} dns_diffop_t; + typedef enum { dns_fwdpolicy_none = 0, dns_fwdpolicy_first = 1, @@ -424,8 +432,8 @@ typedef void (*dns_loaddonefunc_t)(void *, isc_result_t); typedef void (*dns_rawdatafunc_t)(dns_zone_t *, dns_masterrawheader_t *); typedef isc_result_t (*dns_addrdatasetfunc_t)(void *arg, const dns_name_t *name, - dns_rdataset_t *rdataset - DNS__DB_FLARG); + dns_rdataset_t *rdataset, + dns_diffop_t op DNS__DB_FLARG); typedef void (*dns_transactionfunc_t)(void *arg); typedef isc_result_t (*dns_additionaldatafunc_t)( diff --git a/lib/dns/master.c b/lib/dns/master.c index ca12731a0bd..191b91fc281 100644 --- a/lib/dns/master.c +++ b/lib/dns/master.c @@ -498,7 +498,7 @@ loadctx_create(dns_masterformat_t format, isc_mem_t *mctx, unsigned int options, REQUIRE(lctxp != NULL && *lctxp == NULL); REQUIRE(callbacks != NULL); - REQUIRE(callbacks->add != NULL); + REQUIRE(callbacks->update != NULL); REQUIRE(callbacks->error != NULL); REQUIRE(callbacks->warn != NULL); REQUIRE(mctx != NULL); @@ -2857,8 +2857,8 @@ commit(dns_rdatacallbacks_t *callbacks, dns_loadctx_t *lctx, dataset.attributes.resign = true; dataset.resign = resign_fromlist(this, lctx); } - result = callbacks->add(callbacks->add_private, owner, - &dataset DNS__DB_FILELINE); + result = callbacks->update(callbacks->add_private, owner, + &dataset, DNS_DIFFOP_ADD DNS__DB_FILELINE); if (result != ISC_R_SUCCESS) { dns_name_format(owner, namebuf, sizeof(namebuf)); if (source != NULL) { diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c index b8a07be51de..f2e1d86cfc1 100644 --- a/lib/dns/qpzone.c +++ b/lib/dns/qpzone.c @@ -2064,7 +2064,7 @@ addwildcards(qpzonedb_t *qpdb, dns_qp_t *qp, const dns_name_t *name, static isc_result_t loading_addrdataset(void *arg, const dns_name_t *name, - dns_rdataset_t *rdataset DNS__DB_FLARG) { + dns_rdataset_t *rdataset, dns_diffop_t op ISC_ATTR_UNUSED DNS__DB_FLARG) { qpz_load_t *loadctx = arg; qpzonedb_t *qpdb = (qpzonedb_t *)loadctx->db; qpznode_t *node = NULL; @@ -2194,7 +2194,7 @@ beginload(dns_db_t *db, dns_rdatacallbacks_t *callbacks) { RWUNLOCK(&qpdb->lock, isc_rwlocktype_write); - callbacks->add = loading_addrdataset; + callbacks->update = loading_addrdataset; callbacks->setup = loading_setup; callbacks->commit = loading_commit; callbacks->add_private = loadctx; @@ -2229,7 +2229,7 @@ endload(dns_db_t *db, dns_rdatacallbacks_t *callbacks) { RWUNLOCK(&qpdb->lock, isc_rwlocktype_write); } - callbacks->add = NULL; + callbacks->update = NULL; callbacks->setup = NULL; callbacks->commit = NULL; callbacks->add_private = NULL; diff --git a/lib/dns/xfrin.c b/lib/dns/xfrin.c index f4ef175d40a..1464717f0df 100644 --- a/lib/dns/xfrin.c +++ b/lib/dns/xfrin.c @@ -516,6 +516,11 @@ ixfr_apply_one(dns_xfrin_t *xfr, ixfr_apply_data_t *data) { isc_result_t result = ISC_R_SUCCESS; uint64_t records; + dns_rdatacallbacks_t callbacks; + dns_rdatacallbacks_init(&callbacks); + dns_db_beginupdate(xfr->db, &callbacks); + + CHECK(ixfr_begin_transaction(&xfr->ixfr)); CHECK(dns_diff_apply(&data->diff, xfr->db, xfr->ver)); @@ -529,12 +534,27 @@ ixfr_apply_one(dns_xfrin_t *xfr, ixfr_apply_data_t *data) { CHECK(dns_journal_writediff(xfr->ixfr.journal, &data->diff)); } + /* + * At the moment, rdatacallbacks doesn't offer a way to inspect the + * result of a transaction before committing it. + * + * So we need to commit *before* calling dns_zone_verifydb, and rely + * on closeversion to actually do cleanup. + */ + dns_db_commitupdate(xfr->db, &callbacks); + CHECK(dns_zone_verifydb(xfr->zone, xfr->db, xfr->ver)); result = ixfr_end_transaction(&xfr->ixfr); return result; cleanup: + /* + * For the reason stated above, dns_db_abortupdate must *commit* the + * changes and rely on closeversion to clean them up. + */ + dns_db_abortupdate(xfr->db, &callbacks); + /* We need to end the transaction, but keep the previous error */ (void)ixfr_end_transaction(&xfr->ixfr);