From 01b25067596e2c951db432d84448b905e05726da Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Fri, 2 Jan 2026 21:23:59 -0800 Subject: [PATCH 01/47] Refactor: libcib: New cib__get_calldata() Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 10 ++-------- daemons/based/based_messages.c | 4 +--- include/crm/cib/internal.h | 1 + lib/cib/cib_native.c | 4 +--- lib/cib/cib_remote.c | 4 +--- lib/cib/cib_utils.c | 17 ++++++++--------- 6 files changed, 14 insertions(+), 26 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 664210f180c..57572d4e2e4 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -200,10 +200,7 @@ process_ping_reply(xmlNode *reply) uint64_t seq = 0; const char *host = pcmk__xe_get(reply, PCMK__XA_SRC); - xmlNode *wrapper = pcmk__xe_first_child(reply, PCMK__XE_CIB_CALLDATA, NULL, - NULL); - xmlNode *pong = pcmk__xe_first_child(wrapper, NULL, NULL, NULL); - + xmlNode *pong = cib__get_calldata(reply); const char *seq_s = pcmk__xe_get(pong, PCMK__XA_CIB_PING_ID); const char *digest = pcmk__xe_get(pong, PCMK_XA_DIGEST); @@ -245,10 +242,7 @@ process_ping_reply(xmlNode *reply) pcmk__trace("Processing ping reply %s from %s (%s)", seq_s, host, digest); if (!pcmk__str_eq(ping_digest, digest, pcmk__str_casei)) { - xmlNode *wrapper = pcmk__xe_first_child(pong, PCMK__XE_CIB_CALLDATA, - NULL, NULL); - xmlNode *remote_versions = pcmk__xe_first_child(wrapper, NULL, NULL, - NULL); + xmlNode *remote_versions = cib__get_calldata(pong); const char *admin_epoch_s = NULL; const char *epoch_s = NULL; diff --git a/daemons/based/based_messages.c b/daemons/based/based_messages.c index 7cd608aa0ef..1f0e2043b58 100644 --- a/daemons/based/based_messages.c +++ b/daemons/based/based_messages.c @@ -161,7 +161,6 @@ int based_process_schemas(xmlNode *req, xmlNode *input, xmlNode **cib, xmlNode **answer) { - xmlNode *wrapper = NULL; xmlNode *data = NULL; const char *after_ver = NULL; @@ -170,8 +169,7 @@ based_process_schemas(xmlNode *req, xmlNode *input, xmlNode **cib, *answer = pcmk__xe_create(NULL, PCMK__XA_SCHEMAS); - wrapper = pcmk__xe_first_child(req, PCMK__XE_CIB_CALLDATA, NULL, NULL); - data = pcmk__xe_first_child(wrapper, NULL, NULL, NULL); + data = cib__get_calldata(req); if (data == NULL) { pcmk__warn("No data specified in request"); return EPROTO; diff --git a/include/crm/cib/internal.h b/include/crm/cib/internal.h index 63b5eda8a6e..a3c505ec96a 100644 --- a/include/crm/cib/internal.h +++ b/include/crm/cib/internal.h @@ -187,6 +187,7 @@ cib__client_triggers_refresh(const char *name) } int cib__get_notify_patchset(const xmlNode *msg, const xmlNode **patchset); +xmlNode *cib__get_calldata(const xmlNode *request); int cib__perform_query(cib__op_fn_t fn, xmlNode *req, xmlNode **current_cib, xmlNode **output); diff --git a/lib/cib/cib_native.c b/lib/cib/cib_native.c index 57c8c87545e..8c19cdda340 100644 --- a/lib/cib/cib_native.c +++ b/lib/cib/cib_native.c @@ -126,9 +126,7 @@ cib_native_perform_op_delegate(cib_t *cib, const char *op, const char *host, rc = pcmk_ok; pcmk__xe_get_int(op_reply, PCMK__XA_CIB_CALLID, &reply_id); if (reply_id == cib->call_id) { - xmlNode *wrapper = pcmk__xe_first_child(op_reply, PCMK__XE_CIB_CALLDATA, - NULL, NULL); - xmlNode *tmp = pcmk__xe_first_child(wrapper, NULL, NULL, NULL); + xmlNode *tmp = cib__get_calldata(op_reply); pcmk__trace("Synchronous reply %d received", reply_id); if (pcmk__xe_get_int(op_reply, PCMK__XA_CIB_RC, &rc) != pcmk_rc_ok) { diff --git a/lib/cib/cib_remote.c b/lib/cib/cib_remote.c index c2d0fc63e74..a4e240f2dbb 100644 --- a/lib/cib/cib_remote.c +++ b/lib/cib/cib_remote.c @@ -201,9 +201,7 @@ cib_remote_perform_op(cib_t *cib, const char *op, const char *host, /* do nothing more */ } else if (!(call_options & cib_discard_reply)) { - xmlNode *wrapper = pcmk__xe_first_child(op_reply, PCMK__XE_CIB_CALLDATA, - NULL, NULL); - xmlNode *tmp = pcmk__xe_first_child(wrapper, NULL, NULL, NULL); + xmlNode *tmp = cib__get_calldata(op_reply); if (tmp == NULL) { pcmk__trace("No output in reply to \"%s\" command %d", op, diff --git a/lib/cib/cib_utils.c b/lib/cib/cib_utils.c index 835f46ff7b7..13f29bc385e 100644 --- a/lib/cib/cib_utils.c +++ b/lib/cib/cib_utils.c @@ -171,12 +171,14 @@ cib_acl_enabled(xmlNode *xml, const char *user) /*! * \internal - * \brief Get input data from a CIB request + * \brief Get call data from a CIB request * * \param[in] request CIB request XML + * + * \return Call data, or \c NULL if none is found */ -static xmlNode * -get_op_input(const xmlNode *request) +xmlNode * +cib__get_calldata(const xmlNode *request) { xmlNode *wrapper = pcmk__xe_first_child(request, PCMK__XE_CIB_CALLDATA, NULL, NULL); @@ -209,7 +211,7 @@ cib__perform_query(cib__op_fn_t fn, xmlNode *req, xmlNode **current_cib, user = pcmk__xe_get(req, PCMK__XA_CIB_USER); pcmk__xe_get_flags(req, PCMK__XA_CIB_CALLOPT, &call_options, cib_none); - input = get_op_input(req); + input = cib__get_calldata(req); cib = *current_cib; if (cib_acl_enabled(*current_cib, user) @@ -513,7 +515,7 @@ cib_perform_op(enum cib_variant variant, cib__op_fn_t fn, xmlNode *req, user = pcmk__xe_get(req, PCMK__XA_CIB_USER); pcmk__xe_get_flags(req, PCMK__XA_CIB_CALLOPT, &call_options, cib_none); - input = get_op_input(req); + input = cib__get_calldata(req); enable_acl = cib_acl_enabled(*cib, user); pcmk__trace("Processing %s for section '%s', user '%s'", op, @@ -776,12 +778,9 @@ cib_native_callback(cib_t * cib, xmlNode * msg, int call_id, int rc) cib_callback_client_t *blob = NULL; if (msg != NULL) { - xmlNode *wrapper = NULL; - pcmk__xe_get_int(msg, PCMK__XA_CIB_RC, &rc); pcmk__xe_get_int(msg, PCMK__XA_CIB_CALLID, &call_id); - wrapper = pcmk__xe_first_child(msg, PCMK__XE_CIB_CALLDATA, NULL, NULL); - output = pcmk__xe_first_child(wrapper, NULL, NULL, NULL); + output = cib__get_calldata(msg); } blob = cib__lookup_id(call_id); From 9ffea2bb0e35fd994a27f082297ae211722df4ff Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Fri, 2 Jan 2026 21:33:52 -0800 Subject: [PATCH 02/47] Refactor: libcib: New cib__set_calldata() Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 8 +------- daemons/based/based_messages.c | 13 ++++--------- include/crm/cib/internal.h | 1 + lib/cib/cib_utils.c | 32 +++++++++++++++++++++++++------- 4 files changed, 31 insertions(+), 23 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 57572d4e2e4..5dae2eb5cea 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -73,13 +73,7 @@ create_cib_reply(const char *op, const char *call_id, const char *client_id, pcmk__xe_set(reply, PCMK__XA_CIB_CLIENTID, client_id); pcmk__xe_set_int(reply, PCMK__XA_CIB_CALLOPT, call_options); pcmk__xe_set_int(reply, PCMK__XA_CIB_RC, rc); - - if (call_data != NULL) { - xmlNode *wrapper = pcmk__xe_create(reply, PCMK__XE_CIB_CALLDATA); - - pcmk__trace("Attaching reply output"); - pcmk__xml_copy(wrapper, call_data); - } + cib__set_calldata(reply, call_data); crm_log_xml_explicit(reply, "cib:reply"); return reply; diff --git a/daemons/based/based_messages.c b/daemons/based/based_messages.c index 1f0e2043b58..fd8fb3ad9d4 100644 --- a/daemons/based/based_messages.c +++ b/daemons/based/based_messages.c @@ -113,22 +113,19 @@ based_process_ping(xmlNode *req, xmlNode *input, xmlNode **cib, const char *seq = pcmk__xe_get(req, PCMK__XA_CIB_PING_ID); char *digest = pcmk__digest_xml(the_cib, true); - xmlNode *wrapper = NULL; - *answer = pcmk__xe_create(NULL, PCMK__XE_PING_RESPONSE); pcmk__xe_set(*answer, PCMK_XA_CRM_FEATURE_SET, CRM_FEATURE_SET); pcmk__xe_set(*answer, PCMK_XA_DIGEST, digest); pcmk__xe_set(*answer, PCMK__XA_CIB_PING_ID, seq); - wrapper = pcmk__xe_create(*answer, PCMK__XE_CIB_CALLDATA); - if (*cib != NULL) { // Use *cib so that ACL filtering is applied to the answer - xmlNode *shallow = pcmk__xe_create(wrapper, - (const char *) (*cib)->name); + xmlNode *shallow = pcmk__xe_create(NULL, (const char *) (*cib)->name); pcmk__xe_copy_attrs(shallow, *cib, pcmk__xaf_none); + cib__set_calldata(*answer, shallow); + pcmk__xml_free(shallow); } pcmk__info("Reporting our current digest to %s: %s for %s.%s.%s", @@ -379,7 +376,6 @@ sync_our_cib(xmlNode *request, bool all) const char *op = pcmk__xe_get(request, PCMK__XA_CIB_OP); pcmk__node_status_t *peer = NULL; xmlNode *replace_request = NULL; - xmlNode *wrapper = NULL; CRM_CHECK(the_cib != NULL, return EINVAL); CRM_CHECK(all || (host != NULL), return EINVAL); @@ -406,8 +402,7 @@ sync_our_cib(xmlNode *request, bool all) digest = pcmk__digest_xml(the_cib, true); pcmk__xe_set(replace_request, PCMK_XA_DIGEST, digest); - wrapper = pcmk__xe_create(replace_request, PCMK__XE_CIB_CALLDATA); - pcmk__xml_copy(wrapper, the_cib); + cib__set_calldata(replace_request, the_cib); if (!all) { peer = pcmk__get_node(0, host, NULL, pcmk__node_search_cluster_member); diff --git a/include/crm/cib/internal.h b/include/crm/cib/internal.h index a3c505ec96a..047712150b5 100644 --- a/include/crm/cib/internal.h +++ b/include/crm/cib/internal.h @@ -188,6 +188,7 @@ cib__client_triggers_refresh(const char *name) int cib__get_notify_patchset(const xmlNode *msg, const xmlNode **patchset); xmlNode *cib__get_calldata(const xmlNode *request); +void cib__set_calldata(xmlNode *request, xmlNode *data); int cib__perform_query(cib__op_fn_t fn, xmlNode *req, xmlNode **current_cib, xmlNode **output); diff --git a/lib/cib/cib_utils.c b/lib/cib/cib_utils.c index 13f29bc385e..81d38e713ce 100644 --- a/lib/cib/cib_utils.c +++ b/lib/cib/cib_utils.c @@ -175,7 +175,8 @@ cib_acl_enabled(xmlNode *xml, const char *user) * * \param[in] request CIB request XML * - * \return Call data, or \c NULL if none is found + * \return Call data added by \c cib__set_calldata(), or \c NULL if none is + * found */ xmlNode * cib__get_calldata(const xmlNode *request) @@ -186,6 +187,28 @@ cib__get_calldata(const xmlNode *request) return pcmk__xe_first_child(wrapper, NULL, NULL, NULL); } +/*! + * \internal + * \brief Add call data to a CIB request + * + * Add a copy of \p data to a new \c PCMK__XE_CIB_CALLDATA child of \p request. + * + * \param[in,out] request CIB request XML + * \param[in] data Call data to add a copy of (if \c NULL, do nothing) + */ +void +cib__set_calldata(xmlNode *request, xmlNode *data) +{ + xmlNode *wrapper = NULL; + + if (data == NULL) { + return; + } + + wrapper = pcmk__xe_create(request, PCMK__XE_CIB_CALLDATA); + pcmk__xml_copy(wrapper, data); +} + int cib__perform_query(cib__op_fn_t fn, xmlNode *req, xmlNode **current_cib, xmlNode **output) @@ -688,12 +711,7 @@ cib__create_op(cib_t *cib, const char *op, const char *host, pcmk__trace("Sending call options: %.8lx, %d", (long) call_options, call_options); pcmk__xe_set_int(*op_msg, PCMK__XA_CIB_CALLOPT, call_options); - - if (data != NULL) { - xmlNode *wrapper = pcmk__xe_create(*op_msg, PCMK__XE_CIB_CALLDATA); - - pcmk__xml_copy(wrapper, data); - } + cib__set_calldata(*op_msg, data); return pcmk_rc_ok; } From 151e4940bd578b9155d44434130ce92c59d71343 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Fri, 2 Jan 2026 22:09:22 -0800 Subject: [PATCH 03/47] Refactor: libcib: Call xml_apply_patchset() in cib_apply_patch_event() ...directly, instead of calling cib__process_apply_patch(). This will make an upcoming commit simpler. Signed-off-by: Reid Wahl --- lib/cib/cib_utils.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/cib/cib_utils.c b/lib/cib/cib_utils.c index 81d38e713ce..94f283c23b8 100644 --- a/lib/cib/cib_utils.c +++ b/lib/cib/cib_utils.c @@ -932,8 +932,7 @@ cib_apply_patch_event(xmlNode *event, xmlNode *input, xmlNode **output, *output = pcmk__xml_copy(NULL, input); } - rc = cib__process_apply_patch(event, diff, output, NULL); - rc = pcmk_rc2legacy(rc); + rc = xml_apply_patchset(*output, diff, true); if (rc == pcmk_ok) { return pcmk_ok; } From ba7d4442c242d1e981279141136517cde993d5b8 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Fri, 2 Jan 2026 22:13:48 -0800 Subject: [PATCH 04/47] Refactor: libcib: Drop input arg from cib__op_fn_t The request argument provides the input value for functions that need it, via cib__get_calldata(). Signed-off-by: Reid Wahl --- daemons/based/based_messages.c | 37 ++++++++++----------------- daemons/based/based_messages.h | 46 +++++++++------------------------- include/crm/cib/internal.h | 38 ++++++++-------------------- lib/cib/cib_file.c | 4 +-- lib/cib/cib_ops.c | 33 +++++++++++------------- lib/cib/cib_utils.c | 6 ++--- 6 files changed, 54 insertions(+), 110 deletions(-) diff --git a/daemons/based/based_messages.c b/daemons/based/based_messages.c index fd8fb3ad9d4..695b74d0d9e 100644 --- a/daemons/based/based_messages.c +++ b/daemons/based/based_messages.c @@ -38,7 +38,6 @@ xmlNode *the_cib = NULL; * \brief Process a \c PCMK__CIB_REQUEST_ABS_DELETE * * \param[in] req Ignored - * \param[in] input Ignored * \param[in] cib Ignored * \param[in] answer Ignored * @@ -47,8 +46,7 @@ xmlNode *the_cib = NULL; * \note This is unimplemented and simply returns an error. */ int -based_process_abs_delete(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer) +based_process_abs_delete(xmlNode *req, xmlNode **cib, xmlNode **answer) { /* @COMPAT Remove when PCMK__CIB_REQUEST_ABS_DELETE is removed. Note that * external clients with Pacemaker versions < 3.0.0 can send it. @@ -57,14 +55,14 @@ based_process_abs_delete(xmlNode *req, xmlNode *input, xmlNode **cib, } int -based_process_commit_transact(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer) +based_process_commit_transact(xmlNode *req, xmlNode **cib, xmlNode **answer) { /* On success, our caller will activate *cib locally, trigger a replace * notification if appropriate, and sync *cib to all nodes. On failure, our * caller will free *cib. */ int rc = pcmk_rc_ok; + xmlNode *input = cib__get_calldata(req); const char *client_id = pcmk__xe_get(req, PCMK__XA_CIB_CLIENTID); const char *origin = pcmk__xe_get(req, PCMK__XA_SRC); pcmk__client_t *client = pcmk__find_client_by_id(client_id); @@ -82,8 +80,7 @@ based_process_commit_transact(xmlNode *req, xmlNode *input, xmlNode **cib, } int -based_process_is_primary(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer) +based_process_is_primary(xmlNode *req, xmlNode **cib, xmlNode **answer) { // @COMPAT Pacemaker Remote clients <3.0.0 may send this return (based_is_primary? pcmk_rc_ok : EPERM); @@ -91,16 +88,14 @@ based_process_is_primary(xmlNode *req, xmlNode *input, xmlNode **cib, // @COMPAT: Remove when PCMK__CIB_REQUEST_NOOP is removed int -based_process_noop(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer) +based_process_noop(xmlNode *req, xmlNode **cib, xmlNode **answer) { *answer = NULL; return pcmk_rc_ok; } int -based_process_ping(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer) +based_process_ping(xmlNode *req, xmlNode **cib, xmlNode **answer) { /* existing_cib and *cib should be identical. In the absence of ACL * filtering, they should also match the_cib. However, they may be copies @@ -140,8 +135,7 @@ based_process_ping(xmlNode *req, xmlNode *input, xmlNode **cib, } int -based_process_primary(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer) +based_process_primary(xmlNode *req, xmlNode **cib, xmlNode **answer) { if (!based_is_primary) { pcmk__info("We are now in R/W mode"); @@ -155,8 +149,7 @@ based_process_primary(xmlNode *req, xmlNode *input, xmlNode **cib, } int -based_process_schemas(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer) +based_process_schemas(xmlNode *req, xmlNode **cib, xmlNode **answer) { xmlNode *data = NULL; @@ -198,8 +191,7 @@ based_process_schemas(xmlNode *req, xmlNode *input, xmlNode **cib, } int -based_process_secondary(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer) +based_process_secondary(xmlNode *req, xmlNode **cib, xmlNode **answer) { if (based_is_primary) { pcmk__info("We are now in R/O mode"); @@ -213,8 +205,7 @@ based_process_secondary(xmlNode *req, xmlNode *input, xmlNode **cib, } int -based_process_shutdown(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer) +based_process_shutdown(xmlNode *req, xmlNode **cib, xmlNode **answer) { const char *host = pcmk__xe_get(req, PCMK__XA_SRC); @@ -236,15 +227,13 @@ based_process_shutdown(xmlNode *req, xmlNode *input, xmlNode **cib, } int -based_process_sync(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer) +based_process_sync(xmlNode *req, xmlNode **cib, xmlNode **answer) { return sync_our_cib(req, true); } int -based_process_upgrade(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer) +based_process_upgrade(xmlNode *req, xmlNode **cib, xmlNode **answer) { int rc = pcmk_rc_ok; @@ -256,7 +245,7 @@ based_process_upgrade(xmlNode *req, xmlNode *input, xmlNode **cib, * re-broadcasts the request with PCMK__XA_CIB_SCHEMA_MAX, and each node * performs the upgrade (and notifies its local clients) here. */ - return cib__process_upgrade(req, input, cib, answer); + return cib__process_upgrade(req, cib, answer); } else { xmlNode *scratch = pcmk__xml_copy(NULL, *cib); diff --git a/daemons/based/based_messages.h b/daemons/based/based_messages.h index fddb943e086..78d0d7e5218 100644 --- a/daemons/based/based_messages.h +++ b/daemons/based/based_messages.h @@ -17,41 +17,19 @@ extern bool based_is_primary; extern xmlNode *the_cib; -int based_process_abs_delete(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer); - -int based_process_apply_patch(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer); - -int based_process_commit_transact(xmlNode *req, xmlNode *input, xmlNode **cib, +int based_process_abs_delete(xmlNode *req, xmlNode **cib, xmlNode **answer); +int based_process_apply_patch(xmlNode *req, xmlNode **cib, xmlNode **answer); +int based_process_commit_transact(xmlNode *req, xmlNode **cib, xmlNode **answer); - -int based_process_is_primary(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer); - -int based_process_noop(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer); - -int based_process_ping(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer); - -int based_process_primary(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer); - -int based_process_schemas(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer); - -int based_process_secondary(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer); - -int based_process_shutdown(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer); - -int based_process_sync(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer); - -int based_process_upgrade(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer); +int based_process_is_primary(xmlNode *req, xmlNode **cib, xmlNode **answer); +int based_process_noop(xmlNode *req, xmlNode **cib, xmlNode **answer); +int based_process_ping(xmlNode *req, xmlNode **cib, xmlNode **answer); +int based_process_primary(xmlNode *req, xmlNode **cib, xmlNode **answer); +int based_process_schemas(xmlNode *req, xmlNode **cib, xmlNode **answer); +int based_process_secondary(xmlNode *req, xmlNode **cib, xmlNode **answer); +int based_process_shutdown(xmlNode *req, xmlNode **cib, xmlNode **answer); +int based_process_sync(xmlNode *req, xmlNode **cib, xmlNode **answer); +int based_process_upgrade(xmlNode *req, xmlNode **cib, xmlNode **answer); int sync_our_cib(xmlNode *request, bool all); diff --git a/include/crm/cib/internal.h b/include/crm/cib/internal.h index 047712150b5..bd8496df193 100644 --- a/include/crm/cib/internal.h +++ b/include/crm/cib/internal.h @@ -103,8 +103,7 @@ enum cib__op_type { * replace *cib, but the replacement must become the root of the original * document. */ -typedef int (*cib__op_fn_t)(xmlNode *request, xmlNode *input, xmlNode **cib, - xmlNode **output); +typedef int (*cib__op_fn_t)(xmlNode *request, xmlNode **cib, xmlNode **output); typedef struct { const char *name; @@ -209,32 +208,15 @@ void cib_native_notify(gpointer data, gpointer user_data); int cib__get_operation(const char *op, const cib__operation_t **operation); -int cib__process_apply_patch(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer); - -int cib__process_bump(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer); - -int cib__process_create(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer); - -int cib__process_delete(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer); - -int cib__process_erase(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer); - -int cib__process_modify(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer); - -int cib__process_query(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer); - -int cib__process_replace(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer); - -int cib__process_upgrade(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer); +int cib__process_apply_patch(xmlNode *req, xmlNode **cib, xmlNode **answer); +int cib__process_bump(xmlNode *req, xmlNode **cib, xmlNode **answer); +int cib__process_create(xmlNode *req, xmlNode **cib, xmlNode **answer); +int cib__process_delete(xmlNode *req, xmlNode **cib, xmlNode **answer); +int cib__process_erase(xmlNode *req, xmlNode **cib, xmlNode **answer); +int cib__process_modify(xmlNode *req, xmlNode **cib, xmlNode **answer); +int cib__process_query(xmlNode *req, xmlNode **cib, xmlNode **answer); +int cib__process_replace(xmlNode *req, xmlNode **cib, xmlNode **answer); +int cib__process_upgrade(xmlNode *req, xmlNode **cib, xmlNode **answer); int cib_internal_op(cib_t * cib, const char *op, const char *host, const char *section, xmlNode * data, diff --git a/lib/cib/cib_file.c b/lib/cib/cib_file.c index f409b3e61d4..b60cdb55bbe 100644 --- a/lib/cib/cib_file.c +++ b/lib/cib/cib_file.c @@ -301,10 +301,10 @@ commit_transaction(cib_t *cib, xmlNode *transaction, xmlNode **result_cib) } static int -process_commit_transact(xmlNode *req, xmlNode *input, xmlNode **cib_xml, - xmlNode **answer) +process_commit_transact(xmlNode *req, xmlNode **cib_xml, xmlNode **answer) { int rc = pcmk_rc_ok; + xmlNode *input = cib__get_calldata(req); const char *client_id = pcmk__xe_get(req, PCMK__XA_CIB_CLIENTID); cib_t *cib = NULL; diff --git a/lib/cib/cib_ops.c b/lib/cib/cib_ops.c index d7a4b6d001c..92d03eeb8e7 100644 --- a/lib/cib/cib_ops.c +++ b/lib/cib/cib_ops.c @@ -164,9 +164,9 @@ cib__get_operation(const char *op, const cib__operation_t **operation) } int -cib__process_apply_patch(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer) +cib__process_apply_patch(xmlNode *req, xmlNode **cib, xmlNode **answer) { + const xmlNode *input = cib__get_calldata(req); int rc = xml_apply_patchset(*cib, input, true); return pcmk_legacy2rc(rc); @@ -190,7 +190,7 @@ update_counter(xmlNode *xml, const char *field, bool reset) } int -cib__process_bump(xmlNode *req, xmlNode *input, xmlNode **cib, xmlNode **answer) +cib__process_bump(xmlNode *req, xmlNode **cib, xmlNode **answer) { update_counter(*cib, PCMK_XA_EPOCH, false); return pcmk_rc_ok; @@ -290,11 +290,11 @@ process_create_xpath(const char *op, const char *xpath, xmlNode *input, } int -cib__process_create(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer) +cib__process_create(xmlNode *req, xmlNode **cib, xmlNode **answer) { const char *op = pcmk__xe_get(req, PCMK__XA_CIB_OP); const char *section = pcmk__xe_get(req, PCMK__XA_CIB_SECTION); + xmlNode *input = cib__get_calldata(req); xmlNode *failed = NULL; int rc = pcmk_rc_ok; xmlNode *update_section = NULL; @@ -311,7 +311,7 @@ cib__process_create(xmlNode *req, xmlNode *input, xmlNode **cib, if (pcmk__strcase_any_of(section, PCMK__XE_ALL, PCMK_XE_CIB, NULL) || pcmk__xe_is(input, PCMK_XE_CIB)) { - return cib__process_modify(req, input, cib, answer); + return cib__process_modify(req, cib, answer); } // @COMPAT Deprecated since 2.1.8 @@ -460,10 +460,10 @@ process_delete_section(const char *section, xmlNode *input, xmlNode *cib) } int -cib__process_delete(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer) +cib__process_delete(xmlNode *req, xmlNode **cib, xmlNode **answer) { const char *section = pcmk__xe_get(req, PCMK__XA_CIB_SECTION); + xmlNode *input = cib__get_calldata(req); uint32_t options = cib_none; pcmk__xe_get_flags(req, PCMK__XA_CIB_CALLOPT, &options, cib_none); @@ -478,8 +478,7 @@ cib__process_delete(xmlNode *req, xmlNode *input, xmlNode **cib, } int -cib__process_erase(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer) +cib__process_erase(xmlNode *req, xmlNode **cib, xmlNode **answer) { xmlNode *empty = createEmptyCib(0); xmlNode *empty_config = pcmk__xe_first_child(empty, PCMK_XE_CONFIGURATION, @@ -606,10 +605,10 @@ process_modify_section(int options, const char *section, xmlNode *input, } int -cib__process_modify(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer) +cib__process_modify(xmlNode *req, xmlNode **cib, xmlNode **answer) { const char *section = pcmk__xe_get(req, PCMK__XA_CIB_SECTION); + xmlNode *input = cib__get_calldata(req); uint32_t options = cib_none; pcmk__xe_get_flags(req, PCMK__XA_CIB_CALLOPT, &options, cib_none); @@ -746,8 +745,7 @@ process_query_section(int options, const char *section, xmlNode *cib, } int -cib__process_query(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer) +cib__process_query(xmlNode *req, xmlNode **cib, xmlNode **answer) { const char *section = pcmk__xe_get(req, PCMK__XA_CIB_SECTION); uint32_t options = cib_none; @@ -934,10 +932,10 @@ process_replace_section(const char *section, xmlNode *request, xmlNode *input, } int -cib__process_replace(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer) +cib__process_replace(xmlNode *req, xmlNode **cib, xmlNode **answer) { const char *section = pcmk__xe_get(req, PCMK__XA_CIB_SECTION); + xmlNode *input = cib__get_calldata(req); uint32_t options = cib_none; pcmk__xe_get_flags(req, PCMK__XA_CIB_CALLOPT, &options, cib_none); @@ -952,8 +950,7 @@ cib__process_replace(xmlNode *req, xmlNode *input, xmlNode **cib, } int -cib__process_upgrade(xmlNode *req, xmlNode *input, xmlNode **cib, - xmlNode **answer) +cib__process_upgrade(xmlNode *req, xmlNode **cib, xmlNode **answer) { int rc = pcmk_rc_ok; uint32_t options = cib_none; diff --git a/lib/cib/cib_utils.c b/lib/cib/cib_utils.c index 94f283c23b8..4405f97d790 100644 --- a/lib/cib/cib_utils.c +++ b/lib/cib/cib_utils.c @@ -218,7 +218,6 @@ cib__perform_query(cib__op_fn_t fn, xmlNode *req, xmlNode **current_cib, const char *section = NULL; const char *user = NULL; uint32_t call_options = cib_none; - xmlNode *input = NULL; xmlNode *cib = NULL; xmlNode *cib_filtered = NULL; @@ -234,7 +233,6 @@ cib__perform_query(cib__op_fn_t fn, xmlNode *req, xmlNode **current_cib, user = pcmk__xe_get(req, PCMK__XA_CIB_USER); pcmk__xe_get_flags(req, PCMK__XA_CIB_CALLOPT, &call_options, cib_none); - input = cib__get_calldata(req); cib = *current_cib; if (cib_acl_enabled(*current_cib, user) @@ -256,7 +254,7 @@ cib__perform_query(cib__op_fn_t fn, xmlNode *req, xmlNode **current_cib, saved_cib = cib; saved_doc = cib->doc; - rc = fn(req, input, &cib, output); + rc = fn(req, &cib, output); /* Sanity check: op should be read-only (but this does not check children, * attributes, or private data) @@ -575,7 +573,7 @@ cib_perform_op(enum cib_variant variant, cib__op_fn_t fn, xmlNode *req, saved_doc = (*cib)->doc; - rc = fn(req, input, cib, output); + rc = fn(req, cib, output); if (rc != pcmk_rc_ok) { goto done; } From 3c9a0d0fd348cfb75054611db529e71d591acb1d Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Fri, 2 Jan 2026 22:51:43 -0800 Subject: [PATCH 05/47] Refactor: libcib: Drop input argument from cib_perform_op helpers We were passing the input argument just for logging on error. But we're already logging the request, and the request always contains the input (if non-NULL) as the child of a PCMK__XE_CIB_CALLDATA child. Signed-off-by: Reid Wahl --- lib/cib/cib_utils.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/lib/cib/cib_utils.c b/lib/cib/cib_utils.c index 4405f97d790..b7c6673ade7 100644 --- a/lib/cib/cib_utils.c +++ b/lib/cib/cib_utils.c @@ -376,7 +376,6 @@ check_new_feature_set(const xmlNode *new_cib) * \param[in] old_cib \c PCMK_XE_CIB element before performing operation * \param[in] new_cib \c PCMK_XE_CIB element from result of operation * \param[in] request CIB request - * \param[in] input Input data for CIB request * * \return Standard Pacemaker return code * @@ -385,8 +384,7 @@ check_new_feature_set(const xmlNode *new_cib) */ static int check_cib_version_attr(const char *attr, const xmlNode *old_cib, - const xmlNode *new_cib, const xmlNode *request, - const xmlNode *input) + const xmlNode *new_cib, const xmlNode *request) { const char *op = pcmk__xe_get(request, PCMK__XA_CIB_OP); int old_version = 0; @@ -406,7 +404,6 @@ check_cib_version_attr(const char *attr, const xmlNode *old_cib, pcmk__err("%s went backwards in %s request: %d -> %d", attr, op, old_version, new_version); pcmk__log_xml_warn(request, "bad-request"); - pcmk__log_xml_warn(input, "bad-input"); return pcmk_rc_old_data; } @@ -423,7 +420,6 @@ check_cib_version_attr(const char *attr, const xmlNode *old_cib, * \param[in] old_cib \c PCMK_XE_CIB element before performing operation * \param[in] new_cib \c PCMK_XE_CIB element from result of operation * \param[in] request CIB request - * \param[in] input Input data for CIB request * * \return Standard Pacemaker return code * @@ -432,18 +428,17 @@ check_cib_version_attr(const char *attr, const xmlNode *old_cib, */ static int check_cib_versions(const xmlNode *old_cib, const xmlNode *new_cib, - const xmlNode *request, const xmlNode *input) + const xmlNode *request) { int rc = check_cib_version_attr(PCMK_XA_ADMIN_EPOCH, old_cib, new_cib, - request, input); + request); if (rc != pcmk_rc_undetermined) { return rc; } // @TODO Why aren't we checking PCMK_XA_NUM_UPDATES if epochs are equal? - rc = check_cib_version_attr(PCMK_XA_EPOCH, old_cib, new_cib, request, - input); + rc = check_cib_version_attr(PCMK_XA_EPOCH, old_cib, new_cib, request); if (rc == pcmk_rc_undetermined) { rc = pcmk_rc_ok; } @@ -512,7 +507,6 @@ cib_perform_op(enum cib_variant variant, cib__op_fn_t fn, xmlNode *req, const char *section = NULL; const char *user = NULL; uint32_t call_options = cib_none; - xmlNode *input = NULL; bool enable_acl = false; bool manage_version = true; @@ -536,7 +530,6 @@ cib_perform_op(enum cib_variant variant, cib__op_fn_t fn, xmlNode *req, user = pcmk__xe_get(req, PCMK__XA_CIB_USER); pcmk__xe_get_flags(req, PCMK__XA_CIB_CALLOPT, &call_options, cib_none); - input = cib__get_calldata(req); enable_acl = cib_acl_enabled(*cib, user); pcmk__trace("Processing %s for section '%s', user '%s'", op, @@ -610,7 +603,7 @@ cib_perform_op(enum cib_variant variant, cib__op_fn_t fn, xmlNode *req, } } - rc = check_cib_versions(old_versions, *cib, req, input); + rc = check_cib_versions(old_versions, *cib, req); pcmk__strip_xml_text(*cib); From 9f0bee2ea248655727781122be81bc18f81839fd Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 3 Jan 2026 01:14:58 -0800 Subject: [PATCH 06/47] Refactor: libcib: Reduce number of arguments in cib_ops.c I'm ambivalent about this, since it adds lines of code, but request contains everything needed by a lot of these helper functions. Signed-off-by: Reid Wahl --- lib/cib/cib_ops.c | 119 ++++++++++++++++++++++++++++------------------ 1 file changed, 72 insertions(+), 47 deletions(-) diff --git a/lib/cib/cib_ops.c b/lib/cib/cib_ops.c index 92d03eeb8e7..0da03c0263c 100644 --- a/lib/cib/cib_ops.c +++ b/lib/cib/cib_ops.c @@ -354,14 +354,19 @@ cib__process_create(xmlNode *req, xmlNode **cib, xmlNode **answer) } static int -process_delete_xpath(const char *op, int options, const char *xpath, - xmlNode *cib) +process_delete_xpath(const xmlNode *request, xmlNode *cib) { + const char *op = pcmk__xe_get(request, PCMK__XA_CIB_OP); + const char *xpath = pcmk__xe_get(request, PCMK__XA_CIB_SECTION); + uint32_t options = cib_none; + int num_results = 0; int rc = pcmk_rc_ok; xmlXPathObject *xpath_obj = pcmk__xpath_search(cib->doc, xpath); + pcmk__xe_get_flags(request, PCMK__XA_CIB_CALLOPT, &options, cib_none); + num_results = pcmk__xpath_num_results(xpath_obj); if (num_results == 0) { pcmk__debug("%s was already removed", xpath); @@ -434,8 +439,10 @@ delete_child(xmlNode *child, void *userdata) } static int -process_delete_section(const char *section, xmlNode *input, xmlNode *cib) +process_delete_section(const xmlNode *request, xmlNode *cib) { + const char *section = pcmk__xe_get(request, PCMK__XA_CIB_SECTION); + xmlNode *input = cib__get_calldata(request); xmlNode *obj_root = NULL; if ((section != NULL) && pcmk__xe_is(input, PCMK_XE_CIB)) { @@ -462,19 +469,15 @@ process_delete_section(const char *section, xmlNode *input, xmlNode *cib) int cib__process_delete(xmlNode *req, xmlNode **cib, xmlNode **answer) { - const char *section = pcmk__xe_get(req, PCMK__XA_CIB_SECTION); - xmlNode *input = cib__get_calldata(req); uint32_t options = cib_none; pcmk__xe_get_flags(req, PCMK__XA_CIB_CALLOPT, &options, cib_none); if (pcmk__is_set(options, cib_xpath)) { - const char *op = pcmk__xe_get(req, PCMK__XA_CIB_OP); - - return process_delete_xpath(op, options, section, *cib); + return process_delete_xpath(req, *cib); } - return process_delete_section(section, input, *cib); + return process_delete_section(req, *cib); } int @@ -506,14 +509,22 @@ cib__process_erase(xmlNode *req, xmlNode **cib, xmlNode **answer) } static int -process_modify_xpath(const char *op, int options, const char *xpath, - xmlNode *input, xmlNode *cib) +process_modify_xpath(const xmlNode *request, xmlNode *cib) { + const char *op = pcmk__xe_get(request, PCMK__XA_CIB_OP); + const char *xpath = pcmk__xe_get(request, PCMK__XA_CIB_SECTION); + xmlNode *input = cib__get_calldata(request); + uint32_t options = cib_none; + int num_results = 0; int rc = pcmk_rc_ok; xmlXPathObject *xpath_obj = NULL; - const bool score = pcmk__is_set(options, cib_score_update); - const uint32_t flags = (score? pcmk__xaf_score_update : pcmk__xaf_none); + uint32_t flags = pcmk__xaf_none; + + pcmk__xe_get_flags(request, PCMK__XA_CIB_CALLOPT, &options, cib_none); + if (pcmk__is_set(options, cib_score_update)) { + flags = pcmk__xaf_score_update; + } if (xpath == NULL) { xpath = pcmk__cib_abs_xpath_for(PCMK_XE_CIB); @@ -555,13 +566,20 @@ process_modify_xpath(const char *op, int options, const char *xpath, } static int -process_modify_section(int options, const char *section, xmlNode *input, - xmlNode *cib) +process_modify_section(const xmlNode *request, xmlNode *cib) { - const bool score = pcmk__is_set(options, cib_score_update); - const uint32_t flags = (score? pcmk__xaf_score_update : pcmk__xaf_none); + const char *section = pcmk__xe_get(request, PCMK__XA_CIB_SECTION); + xmlNode *input = cib__get_calldata(request); + uint32_t options = cib_none; + + uint32_t flags = pcmk__xaf_none; xmlNode *obj_root = NULL; + pcmk__xe_get_flags(request, PCMK__XA_CIB_CALLOPT, &options, cib_none); + if (pcmk__is_set(options, cib_score_update)) { + flags = pcmk__xaf_score_update; + } + if ((section != NULL) && pcmk__xe_is(input, PCMK_XE_CIB)) { input = pcmk_find_cib_element(input, section); } @@ -607,29 +625,30 @@ process_modify_section(int options, const char *section, xmlNode *input, int cib__process_modify(xmlNode *req, xmlNode **cib, xmlNode **answer) { - const char *section = pcmk__xe_get(req, PCMK__XA_CIB_SECTION); - xmlNode *input = cib__get_calldata(req); uint32_t options = cib_none; pcmk__xe_get_flags(req, PCMK__XA_CIB_CALLOPT, &options, cib_none); if (pcmk__is_set(options, cib_xpath)) { - const char *op = pcmk__xe_get(req, PCMK__XA_CIB_OP); - - return process_modify_xpath(op, options, section, input, *cib); + return process_modify_xpath(req, *cib); } - return process_modify_section(options, section, input, *cib); + return process_modify_section(req, *cib); } static int -process_query_xpath(const char *op, int options, const char *xpath, - xmlNode *cib, xmlNode **answer) +process_query_xpath(const xmlNode *request, xmlNode *cib, xmlNode **answer) { + const char *op = pcmk__xe_get(request, PCMK__XA_CIB_OP); + const char *xpath = pcmk__xe_get(request, PCMK__XA_CIB_SECTION); + uint32_t options = cib_none; + int num_results = 0; int rc = pcmk_rc_ok; xmlXPathObject *xpath_obj = pcmk__xpath_search(cib->doc, xpath); + pcmk__xe_get_flags(request, PCMK__XA_CIB_CALLOPT, &options, cib_none); + num_results = pcmk__xpath_num_results(xpath_obj); if (num_results == 0) { pcmk__debug("%s: %s does not exist", op, xpath); @@ -715,10 +734,13 @@ process_query_xpath(const char *op, int options, const char *xpath, } static int -process_query_section(int options, const char *section, xmlNode *cib, - xmlNode **answer) +process_query_section(const xmlNode *request, xmlNode *cib, xmlNode **answer) { + const char *section = pcmk__xe_get(request, PCMK__XA_CIB_SECTION); xmlNode *obj_root = NULL; + uint32_t options = cib_none; + + pcmk__xe_get_flags(request, PCMK__XA_CIB_CALLOPT, &options, cib_none); if (pcmk__str_eq(PCMK__XE_ALL, section, pcmk__str_casei)) { section = NULL; @@ -747,25 +769,23 @@ process_query_section(int options, const char *section, xmlNode *cib, int cib__process_query(xmlNode *req, xmlNode **cib, xmlNode **answer) { - const char *section = pcmk__xe_get(req, PCMK__XA_CIB_SECTION); uint32_t options = cib_none; pcmk__xe_get_flags(req, PCMK__XA_CIB_CALLOPT, &options, cib_none); if (pcmk__is_set(options, cib_xpath)) { - const char *op = pcmk__xe_get(req, PCMK__XA_CIB_OP); - - return process_query_xpath(op, options, section, *cib, answer); + return process_query_xpath(req, *cib, answer); } - return process_query_section(options, section, *cib, answer); + return process_query_section(req, *cib, answer); } static bool -replace_cib_digest_matches(xmlNode *request, xmlNode *input) +replace_cib_digest_matches(const xmlNode *request) { const char *peer = pcmk__xe_get(request, PCMK__XA_SRC); const char *expected = pcmk__xe_get(request, PCMK_XA_DIGEST); + const xmlNode *input = cib__get_calldata(request); char *calculated = NULL; bool matches = false; @@ -790,7 +810,7 @@ replace_cib_digest_matches(xmlNode *request, xmlNode *input) } static int -replace_cib(xmlNode *request, xmlNode *input, xmlNode **cib) +replace_cib(xmlNode *request, xmlNode **cib) { int updates = 0; int epoch = 0; @@ -802,12 +822,13 @@ replace_cib(xmlNode *request, xmlNode *input, xmlNode **cib) const char *reason = NULL; const char *peer = pcmk__xe_get(request, PCMK__XA_SRC); + xmlNode *input = cib__get_calldata(request); cib_version_details(*cib, &admin_epoch, &epoch, &updates); cib_version_details(input, &replace_admin_epoch, &replace_epoch, &replace_updates); - if (!replace_cib_digest_matches(request, input)) { + if (!replace_cib_digest_matches(request)) { pcmk__info("Replacement %d.%d.%d from %s not applied to %d.%d.%d: " "digest mismatch", replace_admin_epoch, replace_epoch, replace_updates, peer, admin_epoch, epoch, updates); @@ -847,13 +868,19 @@ replace_cib(xmlNode *request, xmlNode *input, xmlNode **cib) } static int -process_replace_xpath(const char *op, int options, const char *xpath, - xmlNode *request, xmlNode *input, xmlNode **cib) +process_replace_xpath(xmlNode *request, xmlNode **cib) { + const char *op = pcmk__xe_get(request, PCMK__XA_CIB_OP); + const char *xpath = pcmk__xe_get(request, PCMK__XA_CIB_SECTION); + xmlNode *input = cib__get_calldata(request); + uint32_t options = cib_none; + int num_results = 0; int rc = pcmk_rc_ok; xmlXPathObject *xpath_obj = pcmk__xpath_search((*cib)->doc, xpath); + pcmk__xe_get_flags(request, PCMK__XA_CIB_CALLOPT, &options, cib_none); + num_results = pcmk__xpath_num_results(xpath_obj); if (num_results == 0) { pcmk__debug("%s: %s does not exist", op, xpath); @@ -876,7 +903,7 @@ process_replace_xpath(const char *op, int options, const char *xpath, free(path); if (match == *cib) { - rc = replace_cib(request, input, cib); + rc = replace_cib(request, cib); break; } @@ -896,9 +923,11 @@ process_replace_xpath(const char *op, int options, const char *xpath, } static int -process_replace_section(const char *section, xmlNode *request, xmlNode *input, - xmlNode **cib) +process_replace_section(xmlNode *request, xmlNode **cib) { + const char *section = pcmk__xe_get(request, PCMK__XA_CIB_SECTION); + xmlNode *input = cib__get_calldata(request); + int rc = pcmk_rc_ok; xmlNode *obj_root = NULL; @@ -912,7 +941,7 @@ process_replace_section(const char *section, xmlNode *request, xmlNode *input, } if (pcmk__xe_is(input, PCMK_XE_CIB)) { - return replace_cib(request, input, cib); + return replace_cib(request, cib); } if (pcmk__str_eq(PCMK__XE_ALL, section, pcmk__str_casei) @@ -934,19 +963,15 @@ process_replace_section(const char *section, xmlNode *request, xmlNode *input, int cib__process_replace(xmlNode *req, xmlNode **cib, xmlNode **answer) { - const char *section = pcmk__xe_get(req, PCMK__XA_CIB_SECTION); - xmlNode *input = cib__get_calldata(req); uint32_t options = cib_none; pcmk__xe_get_flags(req, PCMK__XA_CIB_CALLOPT, &options, cib_none); if (pcmk__is_set(options, cib_xpath)) { - const char *op = pcmk__xe_get(req, PCMK__XA_CIB_OP); - - return process_replace_xpath(op, options, section, req, input, cib); + return process_replace_xpath(req, cib); } - return process_replace_section(section, req, input, cib); + return process_replace_section(req, cib); } int From 9b0d99810401d5661e9cbec5444676961e450439 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 3 Jan 2026 02:39:10 -0800 Subject: [PATCH 07/47] Log: based: Drop redundant log from cib_process_command() We already warn for the same thing in based_process_request() if applicable. Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 5dae2eb5cea..bf99cd5aad6 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -531,11 +531,7 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation, *reply = NULL; /* Start processing the request... */ - rc = pcmk__xe_get_flags(request, PCMK__XA_CIB_CALLOPT, &call_options, - cib_none); - if (rc != pcmk_rc_ok) { - pcmk__warn("Couldn't parse options from request: %s", pcmk_rc_str(rc)); - } + pcmk__xe_get_flags(request, PCMK__XA_CIB_CALLOPT, &call_options, cib_none); if (!privileged && pcmk__is_set(operation->flags, cib__op_attr_privileged)) { From 09659185da1321b312784bd8aea01730f210040e Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 3 Jan 2026 02:42:10 -0800 Subject: [PATCH 08/47] Refactor: based: Drop redundant cib_process_command() config_changed set This is already done a few lines prior. Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index bf99cd5aad6..ade42b38d8f 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -569,10 +569,6 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation, && !pcmk__any_flags_set(call_options, cib_dryrun|cib_transaction)) { if (result_cib != the_cib) { - if (pcmk__is_set(operation->flags, cib__op_attr_writes_through)) { - config_changed = true; - } - rc = based_activate_cib(result_cib, config_changed, op); } From 6e97cbbf95ddcf76d243dde4ba9e1da70ff857e2 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 3 Jan 2026 02:46:30 -0800 Subject: [PATCH 09/47] Refactor: based: Unindent block of cib_process_command() And drop a fairly useless trace message. cib_dryrun always has to be false there, by the way. Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index ade42b38d8f..559caa30d06 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -606,15 +606,15 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation, pcmk__xml_free(result_cib); } - if (!pcmk__any_flags_set(call_options, - cib_dryrun|cib_inhibit_notify|cib_transaction)) { - pcmk__trace("Sending notifications %d", - pcmk__is_set(call_options, cib_dryrun)); - based_diff_notify(op, pcmk_rc2legacy(rc), call_id, client_id, - client_name, originator, cib_diff); + if (pcmk__any_flags_set(call_options, + cib_dryrun|cib_inhibit_notify|cib_transaction)) { + goto done; } - done: + based_diff_notify(op, pcmk_rc2legacy(rc), call_id, client_id, client_name, + originator, cib_diff); + +done: if (!pcmk__is_set(call_options, cib_discard_reply)) { *reply = create_cib_reply(op, call_id, client_id, call_options, pcmk_rc2legacy(rc), output); From 889e0aa9483444de9469220423436a92de258cda Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 3 Jan 2026 02:54:11 -0800 Subject: [PATCH 10/47] Refactor: based: Unindent some of process_ping_reply() Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 98 ++++++++++++++++----------------- 1 file changed, 49 insertions(+), 49 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 559caa30d06..9df26c91d50 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -189,7 +189,7 @@ digest_timer_cb(gpointer data) } static void -process_ping_reply(xmlNode *reply) +process_ping_reply(xmlNode *reply) { uint64_t seq = 0; const char *host = pcmk__xe_get(reply, PCMK__XA_SRC); @@ -197,72 +197,72 @@ process_ping_reply(xmlNode *reply) xmlNode *pong = cib__get_calldata(reply); const char *seq_s = pcmk__xe_get(pong, PCMK__XA_CIB_PING_ID); const char *digest = pcmk__xe_get(pong, PCMK_XA_DIGEST); + long long seq_ll = 0; + int rc = pcmk_rc_ok; if (seq_s == NULL) { pcmk__debug("Ignoring ping reply with no " PCMK__XA_CIB_PING_ID); return; + } - } else { - long long seq_ll; - int rc = pcmk__scan_ll(seq_s, &seq_ll, 0LL); - - if (rc != pcmk_rc_ok) { - pcmk__debug("Ignoring ping reply with invalid " PCMK__XA_CIB_PING_ID - " '%s': %s", - seq_s, pcmk_rc_str(rc)); - return; - } - seq = (uint64_t) seq_ll; + rc = pcmk__scan_ll(seq_s, &seq_ll, 0); + if (rc != pcmk_rc_ok) { + pcmk__debug("Ignoring ping reply with invalid " PCMK__XA_CIB_PING_ID " " + "'%s': %s", seq_s, pcmk_rc_str(rc)); + return; } - if(digest == NULL) { + // @TODO Check for overflow? + seq = (uint64_t) seq_ll; + + if (digest == NULL) { pcmk__trace("Ignoring ping reply %s from %s with no digest", seq_s, host); + return; + } - } else if(seq != ping_seq) { + if (seq != ping_seq) { pcmk__trace("Ignoring out of sequence ping reply %s from %s", seq_s, host); + return; + } - } else if(ping_modified_since) { + if (ping_modified_since) { pcmk__trace("Ignoring ping reply %s from %s: cib updated since", seq_s, host); + return; + } - } else { - if(ping_digest == NULL) { - pcmk__trace("Calculating new digest"); - ping_digest = pcmk__digest_xml(the_cib, true); - } + if (ping_digest == NULL) { + pcmk__trace("Calculating new digest"); + ping_digest = pcmk__digest_xml(the_cib, true); + } + + pcmk__trace("Processing ping reply %s from %s (%s)", seq_s, host, digest); + if (!pcmk__str_eq(ping_digest, digest, pcmk__str_casei)) { + xmlNode *remote_versions = cib__get_calldata(pong); - pcmk__trace("Processing ping reply %s from %s (%s)", seq_s, host, - digest); - if (!pcmk__str_eq(ping_digest, digest, pcmk__str_casei)) { - xmlNode *remote_versions = cib__get_calldata(pong); - - const char *admin_epoch_s = NULL; - const char *epoch_s = NULL; - const char *num_updates_s = NULL; - - if (remote_versions != NULL) { - admin_epoch_s = pcmk__xe_get(remote_versions, - PCMK_XA_ADMIN_EPOCH); - epoch_s = pcmk__xe_get(remote_versions, - PCMK_XA_EPOCH); - num_updates_s = pcmk__xe_get(remote_versions, - PCMK_XA_NUM_UPDATES); - } - - pcmk__notice("Local CIB %s.%s.%s.%s differs from %s: %s.%s.%s.%s", - pcmk__xe_get(the_cib, PCMK_XA_ADMIN_EPOCH), - pcmk__xe_get(the_cib, PCMK_XA_EPOCH), - pcmk__xe_get(the_cib, PCMK_XA_NUM_UPDATES), - ping_digest, host, - pcmk__s(admin_epoch_s, "_"), - pcmk__s(epoch_s, "_"), - pcmk__s(num_updates_s, "_"), digest); - - pcmk__xml_free(remote_versions); - sync_our_cib(reply, false); + const char *admin_epoch_s = NULL; + const char *epoch_s = NULL; + const char *num_updates_s = NULL; + + if (remote_versions != NULL) { + admin_epoch_s = pcmk__xe_get(remote_versions, PCMK_XA_ADMIN_EPOCH); + epoch_s = pcmk__xe_get(remote_versions, PCMK_XA_EPOCH); + num_updates_s = pcmk__xe_get(remote_versions, PCMK_XA_NUM_UPDATES); } + + pcmk__notice("Local CIB %s.%s.%s.%s differs from %s: %s.%s.%s.%s", + pcmk__xe_get(the_cib, PCMK_XA_ADMIN_EPOCH), + pcmk__xe_get(the_cib, PCMK_XA_EPOCH), + pcmk__xe_get(the_cib, PCMK_XA_NUM_UPDATES), + ping_digest, host, + pcmk__s(admin_epoch_s, "_"), + pcmk__s(epoch_s, "_"), + pcmk__s(num_updates_s, "_"), digest); + + pcmk__xml_free(remote_versions); + sync_our_cib(reply, false); } } From b34f7258753e92017c9c3d95822718afca666df1 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 3 Jan 2026 09:13:12 -0800 Subject: [PATCH 11/47] Refactor: based: Unindent rest of process_ping_reply() Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 46 +++++++++++++++++---------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 9df26c91d50..fbb19d15aeb 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -200,6 +200,11 @@ process_ping_reply(xmlNode *reply) long long seq_ll = 0; int rc = pcmk_rc_ok; + xmlNode *remote_versions = cib__get_calldata(pong); + const char *admin_epoch_s = NULL; + const char *epoch_s = NULL; + const char *num_updates_s = NULL; + if (seq_s == NULL) { pcmk__debug("Ignoring ping reply with no " PCMK__XA_CIB_PING_ID); return; @@ -239,31 +244,28 @@ process_ping_reply(xmlNode *reply) } pcmk__trace("Processing ping reply %s from %s (%s)", seq_s, host, digest); - if (!pcmk__str_eq(ping_digest, digest, pcmk__str_casei)) { - xmlNode *remote_versions = cib__get_calldata(pong); - - const char *admin_epoch_s = NULL; - const char *epoch_s = NULL; - const char *num_updates_s = NULL; - if (remote_versions != NULL) { - admin_epoch_s = pcmk__xe_get(remote_versions, PCMK_XA_ADMIN_EPOCH); - epoch_s = pcmk__xe_get(remote_versions, PCMK_XA_EPOCH); - num_updates_s = pcmk__xe_get(remote_versions, PCMK_XA_NUM_UPDATES); - } + if (pcmk__str_eq(ping_digest, digest, pcmk__str_casei)) { + return; + } - pcmk__notice("Local CIB %s.%s.%s.%s differs from %s: %s.%s.%s.%s", - pcmk__xe_get(the_cib, PCMK_XA_ADMIN_EPOCH), - pcmk__xe_get(the_cib, PCMK_XA_EPOCH), - pcmk__xe_get(the_cib, PCMK_XA_NUM_UPDATES), - ping_digest, host, - pcmk__s(admin_epoch_s, "_"), - pcmk__s(epoch_s, "_"), - pcmk__s(num_updates_s, "_"), digest); - - pcmk__xml_free(remote_versions); - sync_our_cib(reply, false); + if (remote_versions != NULL) { + admin_epoch_s = pcmk__xe_get(remote_versions, PCMK_XA_ADMIN_EPOCH); + epoch_s = pcmk__xe_get(remote_versions, PCMK_XA_EPOCH); + num_updates_s = pcmk__xe_get(remote_versions, PCMK_XA_NUM_UPDATES); } + + pcmk__notice("Local CIB %s.%s.%s.%s differs from %s: %s.%s.%s.%s", + pcmk__xe_get(the_cib, PCMK_XA_ADMIN_EPOCH), + pcmk__xe_get(the_cib, PCMK_XA_EPOCH), + pcmk__xe_get(the_cib, PCMK_XA_NUM_UPDATES), + ping_digest, host, + pcmk__s(admin_epoch_s, "_"), + pcmk__s(epoch_s, "_"), + pcmk__s(num_updates_s, "_"), digest); + + pcmk__xml_free(remote_versions); + sync_our_cib(reply, false); } static void From 9a076de984e8c595bea6cce060e87e0cdbb9b8f5 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 3 Jan 2026 14:07:13 -0800 Subject: [PATCH 12/47] Refactor: based: Convert ping_seq to long long So that we can use pcmk__xe_get_ll(). This also avoids theoretical overflow risk, if a value is sent as uint64_t and then scanned as long long upon receiving it. That overflow should never happen in practice, as it would require sending a massive number of pings before restarting based. Also improve documentation a bit and add a note of non-understanding. Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 67 ++++++++++++++++----------------- 1 file changed, 33 insertions(+), 34 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index fbb19d15aeb..3570ec4c02e 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -10,7 +10,6 @@ #include #include // EACCES, ECONNREFUSED -#include // PRIu64 #include #include // NULL, size_t #include // uint32_t, uint64_t @@ -41,7 +40,7 @@ #define EXIT_ESCALATION_MS 10000 -static uint64_t ping_seq = 0; +static long long ping_seq = 0; static char *ping_digest = NULL; static bool ping_modified_since = false; @@ -148,42 +147,53 @@ do_local_notify(const xmlNode *xml, const char *client_id, bool sync_reply, * \internal * \brief Request CIB digests from all peer nodes * - * This is used as a callback that runs 5 seconds after we modify the CIB. It - * sends a ping request to all cluster nodes. They will respond by sending their - * current digests and version info, which we will validate in - * process_ping_reply(). This serves as a check of consistency across the - * cluster after a CIB update. + * This is used as a callback that runs 5 seconds after we modify the CIB on the + * DC. It sends a ping request to all cluster nodes. They will respond by + * sending their current digests and version info, which we will validate in + * process_ping_reply(). If their digest doesn't match, we'll sync our own CIB + * to them. This helps ensure consistency across the cluster after a CIB update. * * \param[in] data Ignored * * \return \c G_SOURCE_REMOVE (to destroy the timeout) + * + * \note It's not clear why we wait 5 seconds rather than sending the ping + * request immediately after a performing a modifying op. Perhaps it's to + * avoid overwhelming other nodes with ping requests when there are a lot + * of modifying requests in a short period. The timer restarts after + * every successful modifying op, so we send ping requests **at most** + * every 5 seconds. Or perhaps it's a remnant of legacy mode (pre-1.1.12). + * In any case, the other nodes shouldn't need time to process the + * modifying op before responding to the ping request. The ping request is + * sent after the op is sent, so it should also be received after the op + * is received. */ static gboolean digest_timer_cb(gpointer data) { - char *buf = NULL; xmlNode *ping = NULL; if (!based_is_primary) { + // Only the DC sends a ping return G_SOURCE_REMOVE; } - ping_seq++; + if (++ping_seq < 0) { + ping_seq = 0; + } + g_clear_pointer(&ping_digest, free); ping_modified_since = false; - buf = pcmk__assert_asprintf("%" PRIu64, ping_seq); - ping = pcmk__xe_create(NULL, PCMK__XE_PING); pcmk__xe_set(ping, PCMK__XA_T, PCMK__VALUE_CIB); pcmk__xe_set(ping, PCMK__XA_CIB_OP, CRM_OP_PING); - pcmk__xe_set(ping, PCMK__XA_CIB_PING_ID, buf); + pcmk__xe_set_ll(ping, PCMK__XA_CIB_PING_ID, ping_seq); pcmk__xe_set(ping, PCMK_XA_CRM_FEATURE_SET, CRM_FEATURE_SET); - pcmk__trace("Requesting peer digests (%s)", buf); + pcmk__trace("Requesting peer digests (%lld)", ping_seq); pcmk__cluster_send_message(NULL, pcmk_ipc_based, ping); - free(buf); pcmk__xml_free(ping); return G_SOURCE_REMOVE; } @@ -191,49 +201,38 @@ digest_timer_cb(gpointer data) static void process_ping_reply(xmlNode *reply) { - uint64_t seq = 0; - const char *host = pcmk__xe_get(reply, PCMK__XA_SRC); - xmlNode *pong = cib__get_calldata(reply); - const char *seq_s = pcmk__xe_get(pong, PCMK__XA_CIB_PING_ID); + long long seq = 0; + const char *host = pcmk__xe_get(reply, PCMK__XA_SRC); const char *digest = pcmk__xe_get(pong, PCMK_XA_DIGEST); - long long seq_ll = 0; - int rc = pcmk_rc_ok; xmlNode *remote_versions = cib__get_calldata(pong); const char *admin_epoch_s = NULL; const char *epoch_s = NULL; const char *num_updates_s = NULL; - if (seq_s == NULL) { - pcmk__debug("Ignoring ping reply with no " PCMK__XA_CIB_PING_ID); - return; - } + int rc = pcmk__xe_get_ll(pong, PCMK__XA_CIB_PING_ID, &seq); - rc = pcmk__scan_ll(seq_s, &seq_ll, 0); if (rc != pcmk_rc_ok) { - pcmk__debug("Ignoring ping reply with invalid " PCMK__XA_CIB_PING_ID " " - "'%s': %s", seq_s, pcmk_rc_str(rc)); + pcmk__debug("Ignoring ping reply with unset or invalid " + PCMK__XA_CIB_PING_ID ": %s", pcmk_rc_str(rc)); return; } - // @TODO Check for overflow? - seq = (uint64_t) seq_ll; - if (digest == NULL) { - pcmk__trace("Ignoring ping reply %s from %s with no digest", seq_s, + pcmk__trace("Ignoring ping reply %lld from %s with no digest", seq, host); return; } if (seq != ping_seq) { - pcmk__trace("Ignoring out of sequence ping reply %s from %s", seq_s, + pcmk__trace("Ignoring out-of-sequence ping reply %lld from %s", seq, host); return; } if (ping_modified_since) { - pcmk__trace("Ignoring ping reply %s from %s: cib updated since", seq_s, + pcmk__trace("Ignoring ping reply %lld from %s: CIB updated since", seq, host); return; } @@ -243,7 +242,7 @@ process_ping_reply(xmlNode *reply) ping_digest = pcmk__digest_xml(the_cib, true); } - pcmk__trace("Processing ping reply %s from %s (%s)", seq_s, host, digest); + pcmk__trace("Processing ping reply %lld from %s (%s)", seq, host, digest); if (pcmk__str_eq(ping_digest, digest, pcmk__str_casei)) { return; From 3829cffc80d08e7baba27633e6b5bf2a169c719a Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 3 Jan 2026 14:19:04 -0800 Subject: [PATCH 13/47] Low: based: Ignore ping reply if we're no longer DC It might be possible for a new DC to be elected after we send a ping request and before we process a modifying request (which would set ping_modified_since to true and cause the ping reply to be ignored). Add this check just in case. If we're no longer DC, then we don't want to sync our CIB to other nodes. Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 3570ec4c02e..65d72142570 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -219,6 +219,12 @@ process_ping_reply(xmlNode *reply) return; } + if (!based_is_primary) { + pcmk__trace("Ignoring ping reply %lld from %s because we are no longer " + "DC", seq, host); + return; + } + if (digest == NULL) { pcmk__trace("Ignoring ping reply %lld from %s with no digest", seq, host); From c390adac1a3577295f8b847c5f44f169b8a34dbb Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 3 Jan 2026 18:49:17 -0800 Subject: [PATCH 14/47] Refactor: based: Unindent outermost else block in based_process_upgrade Signed-off-by: Reid Wahl --- daemons/based/based_messages.c | 120 ++++++++++++++++----------------- 1 file changed, 60 insertions(+), 60 deletions(-) diff --git a/daemons/based/based_messages.c b/daemons/based/based_messages.c index 695b74d0d9e..849efb43436 100644 --- a/daemons/based/based_messages.c +++ b/daemons/based/based_messages.c @@ -237,6 +237,14 @@ based_process_upgrade(xmlNode *req, xmlNode **cib, xmlNode **answer) { int rc = pcmk_rc_ok; + xmlNode *scratch = NULL; + const char *host = pcmk__xe_get(req, PCMK__XA_SRC); + const char *client_id = pcmk__xe_get(req, PCMK__XA_CIB_CLIENTID); + const char *call_opts = pcmk__xe_get(req, PCMK__XA_CIB_CALLOPT); + const char *call_id = pcmk__xe_get(req, PCMK__XA_CIB_CALLID); + const char *original_schema = NULL; + const char *new_schema = NULL; + *answer = NULL; if (pcmk__xe_get(req, PCMK__XA_CIB_SCHEMA_MAX) != NULL) { @@ -246,80 +254,72 @@ based_process_upgrade(xmlNode *req, xmlNode **cib, xmlNode **answer) * performs the upgrade (and notifies its local clients) here. */ return cib__process_upgrade(req, cib, answer); + } - } else { - xmlNode *scratch = pcmk__xml_copy(NULL, *cib); - const char *host = pcmk__xe_get(req, PCMK__XA_SRC); - const char *original_schema = NULL; - const char *new_schema = NULL; - const char *client_id = pcmk__xe_get(req, PCMK__XA_CIB_CLIENTID); - const char *call_opts = pcmk__xe_get(req, PCMK__XA_CIB_CALLOPT); - const char *call_id = pcmk__xe_get(req, PCMK__XA_CIB_CALLID); - - original_schema = pcmk__xe_get(*cib, PCMK_XA_VALIDATE_WITH); - if (original_schema == NULL) { - pcmk__info("Rejecting upgrade request from %s: No " - PCMK_XA_VALIDATE_WITH, - host); - return pcmk_rc_cib_corrupt; - } + scratch = pcmk__xml_copy(NULL, *cib); + + original_schema = pcmk__xe_get(*cib, PCMK_XA_VALIDATE_WITH); + if (original_schema == NULL) { + pcmk__info("Rejecting upgrade request from %s: No " + PCMK_XA_VALIDATE_WITH, host); + return pcmk_rc_cib_corrupt; + } - rc = pcmk__update_schema(&scratch, NULL, true, true); - new_schema = pcmk__xe_get(scratch, PCMK_XA_VALIDATE_WITH); + rc = pcmk__update_schema(&scratch, NULL, true, true); + new_schema = pcmk__xe_get(scratch, PCMK_XA_VALIDATE_WITH); - if (pcmk__cmp_schemas_by_name(new_schema, original_schema) > 0) { - xmlNode *up = pcmk__xe_create(NULL, __func__); + if (pcmk__cmp_schemas_by_name(new_schema, original_schema) > 0) { + xmlNode *up = pcmk__xe_create(NULL, __func__); + + rc = pcmk_rc_ok; + pcmk__notice("Upgrade request from %s verified", host); + + pcmk__xe_set(up, PCMK__XA_T, PCMK__VALUE_CIB); + pcmk__xe_set(up, PCMK__XA_CIB_OP, PCMK__CIB_REQUEST_UPGRADE); + pcmk__xe_set(up, PCMK__XA_CIB_SCHEMA_MAX, new_schema); + pcmk__xe_set(up, PCMK__XA_CIB_DELEGATED_FROM, host); + pcmk__xe_set(up, PCMK__XA_CIB_CLIENTID, client_id); + pcmk__xe_set(up, PCMK__XA_CIB_CALLOPT, call_opts); + pcmk__xe_set(up, PCMK__XA_CIB_CALLID, call_id); + + pcmk__cluster_send_message(NULL, pcmk_ipc_based, up); + + pcmk__xml_free(up); + + } else if (rc == pcmk_rc_ok) { + rc = pcmk_rc_schema_unchanged; + } + + if (rc != pcmk_rc_ok) { + // Notify originating peer so it can notify its local clients + pcmk__node_status_t *origin = NULL; + + origin = pcmk__search_node_caches(0, host, NULL, + pcmk__node_search_cluster_member); - rc = pcmk_rc_ok; - pcmk__notice("Upgrade request from %s verified", host); + pcmk__info("Rejecting upgrade request from %s: %s " + QB_XS " rc=%d peer=%s", host, pcmk_rc_str(rc), rc, + ((origin != NULL)? origin->name : "lost")); + + if (origin != NULL) { + xmlNode *up = pcmk__xe_create(NULL, __func__); pcmk__xe_set(up, PCMK__XA_T, PCMK__VALUE_CIB); pcmk__xe_set(up, PCMK__XA_CIB_OP, PCMK__CIB_REQUEST_UPGRADE); - pcmk__xe_set(up, PCMK__XA_CIB_SCHEMA_MAX, new_schema); pcmk__xe_set(up, PCMK__XA_CIB_DELEGATED_FROM, host); + pcmk__xe_set(up, PCMK__XA_CIB_ISREPLYTO, host); pcmk__xe_set(up, PCMK__XA_CIB_CLIENTID, client_id); pcmk__xe_set(up, PCMK__XA_CIB_CALLOPT, call_opts); pcmk__xe_set(up, PCMK__XA_CIB_CALLID, call_id); - - pcmk__cluster_send_message(NULL, pcmk_ipc_based, up); - - pcmk__xml_free(up); - - } else if (rc == pcmk_rc_ok) { - rc = pcmk_rc_schema_unchanged; - } - - if (rc != pcmk_rc_ok) { - // Notify originating peer so it can notify its local clients - pcmk__node_status_t *origin = NULL; - - origin = pcmk__search_node_caches(0, host, NULL, - pcmk__node_search_cluster_member); - - pcmk__info("Rejecting upgrade request from %s: %s " - QB_XS " rc=%d peer=%s", host, pcmk_rc_str(rc), rc, - ((origin != NULL)? origin->name : "lost")); - - if (origin) { - xmlNode *up = pcmk__xe_create(NULL, __func__); - - pcmk__xe_set(up, PCMK__XA_T, PCMK__VALUE_CIB); - pcmk__xe_set(up, PCMK__XA_CIB_OP, PCMK__CIB_REQUEST_UPGRADE); - pcmk__xe_set(up, PCMK__XA_CIB_DELEGATED_FROM, host); - pcmk__xe_set(up, PCMK__XA_CIB_ISREPLYTO, host); - pcmk__xe_set(up, PCMK__XA_CIB_CLIENTID, client_id); - pcmk__xe_set(up, PCMK__XA_CIB_CALLOPT, call_opts); - pcmk__xe_set(up, PCMK__XA_CIB_CALLID, call_id); - pcmk__xe_set_int(up, PCMK__XA_CIB_UPGRADE_RC, - pcmk_rc2legacy(rc)); - if (!pcmk__cluster_send_message(origin, pcmk_ipc_based, up)) { - pcmk__warn("Could not send CIB upgrade result to %s", host); - } - pcmk__xml_free(up); + pcmk__xe_set_int(up, PCMK__XA_CIB_UPGRADE_RC, pcmk_rc2legacy(rc)); + if (!pcmk__cluster_send_message(origin, pcmk_ipc_based, up)) { + pcmk__warn("Could not send CIB upgrade result to %s", host); } + pcmk__xml_free(up); } - pcmk__xml_free(scratch); } + + pcmk__xml_free(scratch); return rc; } From 6c2a196609d10b7365fa027b9a526f43dee2879d Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 3 Jan 2026 18:54:11 -0800 Subject: [PATCH 15/47] Refactor: based: Unindent pcmk_rc_ok section of based_process_upgrade() Signed-off-by: Reid Wahl --- daemons/based/based_messages.c | 54 +++++++++++++++++----------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/daemons/based/based_messages.c b/daemons/based/based_messages.c index 849efb43436..e88721977e5 100644 --- a/daemons/based/based_messages.c +++ b/daemons/based/based_messages.c @@ -244,6 +244,7 @@ based_process_upgrade(xmlNode *req, xmlNode **cib, xmlNode **answer) const char *call_id = pcmk__xe_get(req, PCMK__XA_CIB_CALLID); const char *original_schema = NULL; const char *new_schema = NULL; + pcmk__node_status_t *origin = NULL; *answer = NULL; @@ -285,40 +286,39 @@ based_process_upgrade(xmlNode *req, xmlNode **cib, xmlNode **answer) pcmk__cluster_send_message(NULL, pcmk_ipc_based, up); pcmk__xml_free(up); + goto done; + } - } else if (rc == pcmk_rc_ok) { + if (rc == pcmk_rc_ok) { rc = pcmk_rc_schema_unchanged; } - if (rc != pcmk_rc_ok) { - // Notify originating peer so it can notify its local clients - pcmk__node_status_t *origin = NULL; - - origin = pcmk__search_node_caches(0, host, NULL, - pcmk__node_search_cluster_member); - - pcmk__info("Rejecting upgrade request from %s: %s " - QB_XS " rc=%d peer=%s", host, pcmk_rc_str(rc), rc, - ((origin != NULL)? origin->name : "lost")); - - if (origin != NULL) { - xmlNode *up = pcmk__xe_create(NULL, __func__); - - pcmk__xe_set(up, PCMK__XA_T, PCMK__VALUE_CIB); - pcmk__xe_set(up, PCMK__XA_CIB_OP, PCMK__CIB_REQUEST_UPGRADE); - pcmk__xe_set(up, PCMK__XA_CIB_DELEGATED_FROM, host); - pcmk__xe_set(up, PCMK__XA_CIB_ISREPLYTO, host); - pcmk__xe_set(up, PCMK__XA_CIB_CLIENTID, client_id); - pcmk__xe_set(up, PCMK__XA_CIB_CALLOPT, call_opts); - pcmk__xe_set(up, PCMK__XA_CIB_CALLID, call_id); - pcmk__xe_set_int(up, PCMK__XA_CIB_UPGRADE_RC, pcmk_rc2legacy(rc)); - if (!pcmk__cluster_send_message(origin, pcmk_ipc_based, up)) { - pcmk__warn("Could not send CIB upgrade result to %s", host); - } - pcmk__xml_free(up); + // Notify originating peer so it can notify its local clients + origin = pcmk__search_node_caches(0, host, NULL, + pcmk__node_search_cluster_member); + + pcmk__info("Rejecting upgrade request from %s: %s " QB_XS " rc=%d peer=%s", + host, pcmk_rc_str(rc), rc, + ((origin != NULL)? origin->name : "lost")); + + if (origin != NULL) { + xmlNode *up = pcmk__xe_create(NULL, __func__); + + pcmk__xe_set(up, PCMK__XA_T, PCMK__VALUE_CIB); + pcmk__xe_set(up, PCMK__XA_CIB_OP, PCMK__CIB_REQUEST_UPGRADE); + pcmk__xe_set(up, PCMK__XA_CIB_DELEGATED_FROM, host); + pcmk__xe_set(up, PCMK__XA_CIB_ISREPLYTO, host); + pcmk__xe_set(up, PCMK__XA_CIB_CLIENTID, client_id); + pcmk__xe_set(up, PCMK__XA_CIB_CALLOPT, call_opts); + pcmk__xe_set(up, PCMK__XA_CIB_CALLID, call_id); + pcmk__xe_set_int(up, PCMK__XA_CIB_UPGRADE_RC, pcmk_rc2legacy(rc)); + if (!pcmk__cluster_send_message(origin, pcmk_ipc_based, up)) { + pcmk__warn("Could not send CIB upgrade result to %s", host); } + pcmk__xml_free(up); } +done: pcmk__xml_free(scratch); return rc; } From 804fe228d5268f218331aabeef53abaa8230c0c1 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 3 Jan 2026 18:56:51 -0800 Subject: [PATCH 16/47] Refactor: based: Don't set answer to NULL in cib__op_fn_t functions These are called only by cib_perform_op()/cib__perform_query(), which assert that *answer is NULL. Signed-off-by: Reid Wahl --- daemons/based/based_messages.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/daemons/based/based_messages.c b/daemons/based/based_messages.c index e88721977e5..397ceeffaa7 100644 --- a/daemons/based/based_messages.c +++ b/daemons/based/based_messages.c @@ -90,7 +90,6 @@ based_process_is_primary(xmlNode *req, xmlNode **cib, xmlNode **answer) int based_process_noop(xmlNode *req, xmlNode **cib, xmlNode **answer) { - *answer = NULL; return pcmk_rc_ok; } @@ -209,8 +208,6 @@ based_process_shutdown(xmlNode *req, xmlNode **cib, xmlNode **answer) { const char *host = pcmk__xe_get(req, PCMK__XA_SRC); - *answer = NULL; - if (pcmk__xe_get(req, PCMK__XA_CIB_ISREPLYTO) == NULL) { pcmk__info("Peer %s is requesting to shut down", host); return pcmk_rc_ok; @@ -246,8 +243,6 @@ based_process_upgrade(xmlNode *req, xmlNode **cib, xmlNode **answer) const char *new_schema = NULL; pcmk__node_status_t *origin = NULL; - *answer = NULL; - if (pcmk__xe_get(req, PCMK__XA_CIB_SCHEMA_MAX) != NULL) { /* The originator of an upgrade request sends it to the DC, without * PCMK__XA_CIB_SCHEMA_MAX. If an upgrade is needed, the DC From 887f1713194b30b6e72e89c368609710c71c199e Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 3 Jan 2026 19:00:01 -0800 Subject: [PATCH 17/47] Refactor: libcib: Rename cib__perform_query() to cib__perform_op_ro() It's used for all non-modifying ops, not just queries. This also enables renaming cib_perform_op() to cib__perform_op_rw() next. Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 2 +- include/crm/cib/internal.h | 2 +- lib/cib/cib_file.c | 2 +- lib/cib/cib_utils.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 65d72142570..89951fd5221 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -548,7 +548,7 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation, } if (!pcmk__is_set(operation->flags, cib__op_attr_modifies)) { - rc = cib__perform_query(op_function, request, &the_cib, &output); + rc = cib__perform_op_ro(op_function, request, &the_cib, &output); goto done; } diff --git a/include/crm/cib/internal.h b/include/crm/cib/internal.h index bd8496df193..68ce3e0390c 100644 --- a/include/crm/cib/internal.h +++ b/include/crm/cib/internal.h @@ -189,7 +189,7 @@ int cib__get_notify_patchset(const xmlNode *msg, const xmlNode **patchset); xmlNode *cib__get_calldata(const xmlNode *request); void cib__set_calldata(xmlNode *request, xmlNode *data); -int cib__perform_query(cib__op_fn_t fn, xmlNode *req, xmlNode **current_cib, +int cib__perform_op_ro(cib__op_fn_t fn, xmlNode *req, xmlNode **current_cib, xmlNode **output); int cib_perform_op(enum cib_variant variant, cib__op_fn_t fn, xmlNode *req, diff --git a/lib/cib/cib_file.c b/lib/cib/cib_file.c index b60cdb55bbe..1a4cae69fec 100644 --- a/lib/cib/cib_file.c +++ b/lib/cib/cib_file.c @@ -164,7 +164,7 @@ process_request(cib_t *cib, xmlNode *request, xmlNode **output) read_only = !pcmk__is_set(operation->flags, cib__op_attr_modifies); if (read_only) { - rc = cib__perform_query(op_function, request, &private->cib_xml, + rc = cib__perform_op_ro(op_function, request, &private->cib_xml, output); } else { result_cib = private->cib_xml; diff --git a/lib/cib/cib_utils.c b/lib/cib/cib_utils.c index b7c6673ade7..adbd10c3732 100644 --- a/lib/cib/cib_utils.c +++ b/lib/cib/cib_utils.c @@ -210,7 +210,7 @@ cib__set_calldata(xmlNode *request, xmlNode *data) } int -cib__perform_query(cib__op_fn_t fn, xmlNode *req, xmlNode **current_cib, +cib__perform_op_ro(cib__op_fn_t fn, xmlNode *req, xmlNode **current_cib, xmlNode **output) { int rc = pcmk_rc_ok; From 6b02f5e889bd979f3771a11c3307fe982f40b5f6 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 3 Jan 2026 19:01:48 -0800 Subject: [PATCH 18/47] Refactor: libcib: Rename cib_perform_op() to cib__perform_op_rw() Avoid using the public API prefix, and make it clearer that this is not for all CIB ops, but rather the ones that may modify something. Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 6 +++--- daemons/based/based_transaction.c | 4 ++-- include/crm/cib/internal.h | 6 +++--- lib/cib/cib_file.c | 6 +++--- lib/cib/cib_utils.c | 6 +++--- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 89951fd5221..891712ec287 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -554,14 +554,14 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation, ping_modified_since = true; - /* result_cib must not be modified after cib_perform_op() returns. + /* result_cib must not be modified after cib__perform_op_rw() returns. * * It's not important whether the client variant is cib_native or * cib_remote. */ result_cib = the_cib; - rc = cib_perform_op(cib_undefined, op_function, request, &config_changed, - &result_cib, &cib_diff, &output); + rc = cib__perform_op_rw(cib_undefined, op_function, request, + &config_changed, &result_cib, &cib_diff, &output); /* Always write to disk for successful ops with the flag set. This also * negates the need to detect ordering changes. diff --git a/daemons/based/based_transaction.c b/daemons/based/based_transaction.c index 48bbb9ac43b..cc4754daae6 100644 --- a/daemons/based/based_transaction.c +++ b/daemons/based/based_transaction.c @@ -118,7 +118,7 @@ process_transaction_requests(xmlNode *transaction, const pcmk__client_t *client, * \note This function is expected to be called only by * \p based_process_commit_transact(). * \note \p result_cib is expected to be a copy of the current CIB as created by - * \p cib_perform_op(). + * \p cib__perform_op_rw(). * \note The caller is responsible for activating and syncing \p result_cib on * success, and for freeing it on failure. */ @@ -130,7 +130,7 @@ based_commit_transaction(xmlNode *transaction, const pcmk__client_t *client, int rc = pcmk_rc_ok; char *source = NULL; - // *result_cib should be a copy of the_cib (created by cib_perform_op()) + // *result_cib should be a copy of the_cib (created by cib__perform_op_rw()) pcmk__assert((result_cib != NULL) && (*result_cib != NULL) && (*result_cib != the_cib)); diff --git a/include/crm/cib/internal.h b/include/crm/cib/internal.h index 68ce3e0390c..2307511d33c 100644 --- a/include/crm/cib/internal.h +++ b/include/crm/cib/internal.h @@ -192,9 +192,9 @@ void cib__set_calldata(xmlNode *request, xmlNode *data); int cib__perform_op_ro(cib__op_fn_t fn, xmlNode *req, xmlNode **current_cib, xmlNode **output); -int cib_perform_op(enum cib_variant variant, cib__op_fn_t fn, xmlNode *req, - bool *config_changed, xmlNode **cib, xmlNode **diff, - xmlNode **output); +int cib__perform_op_rw(enum cib_variant variant, cib__op_fn_t fn, xmlNode *req, + bool *config_changed, xmlNode **cib, xmlNode **diff, + xmlNode **output); int cib__create_op(cib_t *cib, const char *op, const char *host, const char *section, xmlNode *data, int call_options, diff --git a/lib/cib/cib_file.c b/lib/cib/cib_file.c index 1a4cae69fec..7136a81fdfe 100644 --- a/lib/cib/cib_file.c +++ b/lib/cib/cib_file.c @@ -168,8 +168,8 @@ process_request(cib_t *cib, xmlNode *request, xmlNode **output) output); } else { result_cib = private->cib_xml; - rc = cib_perform_op(cib_file, op_function, request, &changed, - &result_cib, &cib_diff, output); + rc = cib__perform_op_rw(cib_file, op_function, request, &changed, + &result_cib, &cib_diff, output); } if (pcmk__is_set(call_options, cib_transaction)) { @@ -265,7 +265,7 @@ commit_transaction(cib_t *cib, xmlNode *transaction, xmlNode **result_cib) xmlNode *saved_cib = private->cib_xml; /* *result_cib should be a copy of private->cib_xml (created by - * cib_perform_op()) + * cib__perform_op_rw()) */ pcmk__assert((result_cib != NULL) && (*result_cib != NULL) && (*result_cib != private->cib_xml)); diff --git a/lib/cib/cib_utils.c b/lib/cib/cib_utils.c index adbd10c3732..10c852a9f4c 100644 --- a/lib/cib/cib_utils.c +++ b/lib/cib/cib_utils.c @@ -497,9 +497,9 @@ set_update_origin(xmlNode *new_cib, const xmlNode *request) } int -cib_perform_op(enum cib_variant variant, cib__op_fn_t fn, xmlNode *req, - bool *config_changed, xmlNode **cib, xmlNode **diff, - xmlNode **output) +cib__perform_op_rw(enum cib_variant variant, cib__op_fn_t fn, xmlNode *req, + bool *config_changed, xmlNode **cib, xmlNode **diff, + xmlNode **output) { int rc = pcmk_rc_ok; From 7548260697fe748bc45aa2671e7029476c47d724 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 3 Jan 2026 21:33:35 -0800 Subject: [PATCH 19/47] Refactor: based: Don't free remote_versions in process_ping_reply() This is part of request and will be freed when request is freed. Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 1 - 1 file changed, 1 deletion(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 891712ec287..8da19271f32 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -269,7 +269,6 @@ process_ping_reply(xmlNode *reply) pcmk__s(epoch_s, "_"), pcmk__s(num_updates_s, "_"), digest); - pcmk__xml_free(remote_versions); sync_our_cib(reply, false); } From ae92fa0fa0d223c02267f4f8d30036c6cc0c9341 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 3 Jan 2026 21:36:06 -0800 Subject: [PATCH 20/47] Refactor: based: Assume the_cib is not NULL We ensure that it's non-NULL in cib_init() before we start the mainloop. After that, no CIB operation should be able to set it to NULL. If it would do so, the operation should fail, and the current (non-NULL) CIB should be kept. (See also the assertions at the beginnings of cib__perform_op_ro() and cib__perform_op_rw() -- cib cannot be NULL if we're calling one of the cib__op_fn_t functions.) When processing requests within a transaction, we don't make a copy, so the_cib may temporarily become NULL. However, we don't return to the mainloop from processing a commit-transaction request until we're finished. At that point, either all requests were processed successfully (and the_cib != NULL) or something failed and we revert to the copy we made at the beginning of the commit-transaction. Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 21 ++++++--------------- daemons/based/based_messages.c | 15 ++++++--------- 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 8da19271f32..4d1830325f9 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -201,15 +201,13 @@ digest_timer_cb(gpointer data) static void process_ping_reply(xmlNode *reply) { + const char *host = pcmk__xe_get(reply, PCMK__XA_SRC); + xmlNode *pong = cib__get_calldata(reply); long long seq = 0; - const char *host = pcmk__xe_get(reply, PCMK__XA_SRC); const char *digest = pcmk__xe_get(pong, PCMK_XA_DIGEST); xmlNode *remote_versions = cib__get_calldata(pong); - const char *admin_epoch_s = NULL; - const char *epoch_s = NULL; - const char *num_updates_s = NULL; int rc = pcmk__xe_get_ll(pong, PCMK__XA_CIB_PING_ID, &seq); @@ -254,20 +252,13 @@ process_ping_reply(xmlNode *reply) return; } - if (remote_versions != NULL) { - admin_epoch_s = pcmk__xe_get(remote_versions, PCMK_XA_ADMIN_EPOCH); - epoch_s = pcmk__xe_get(remote_versions, PCMK_XA_EPOCH); - num_updates_s = pcmk__xe_get(remote_versions, PCMK_XA_NUM_UPDATES); - } - pcmk__notice("Local CIB %s.%s.%s.%s differs from %s: %s.%s.%s.%s", pcmk__xe_get(the_cib, PCMK_XA_ADMIN_EPOCH), pcmk__xe_get(the_cib, PCMK_XA_EPOCH), - pcmk__xe_get(the_cib, PCMK_XA_NUM_UPDATES), - ping_digest, host, - pcmk__s(admin_epoch_s, "_"), - pcmk__s(epoch_s, "_"), - pcmk__s(num_updates_s, "_"), digest); + pcmk__xe_get(the_cib, PCMK_XA_NUM_UPDATES), ping_digest, host, + pcmk__xe_get(remote_versions, PCMK_XA_ADMIN_EPOCH), + pcmk__xe_get(remote_versions, PCMK_XA_EPOCH), + pcmk__xe_get(remote_versions, PCMK_XA_NUM_UPDATES), digest); sync_our_cib(reply, false); } diff --git a/daemons/based/based_messages.c b/daemons/based/based_messages.c index 397ceeffaa7..4fdbd00774e 100644 --- a/daemons/based/based_messages.c +++ b/daemons/based/based_messages.c @@ -106,6 +106,7 @@ based_process_ping(xmlNode *req, xmlNode **cib, xmlNode **answer) const char *host = pcmk__xe_get(req, PCMK__XA_SRC); const char *seq = pcmk__xe_get(req, PCMK__XA_CIB_PING_ID); char *digest = pcmk__digest_xml(the_cib, true); + xmlNode *shallow = NULL; *answer = pcmk__xe_create(NULL, PCMK__XE_PING_RESPONSE); @@ -113,14 +114,11 @@ based_process_ping(xmlNode *req, xmlNode **cib, xmlNode **answer) pcmk__xe_set(*answer, PCMK_XA_DIGEST, digest); pcmk__xe_set(*answer, PCMK__XA_CIB_PING_ID, seq); - if (*cib != NULL) { - // Use *cib so that ACL filtering is applied to the answer - xmlNode *shallow = pcmk__xe_create(NULL, (const char *) (*cib)->name); - - pcmk__xe_copy_attrs(shallow, *cib, pcmk__xaf_none); - cib__set_calldata(*answer, shallow); - pcmk__xml_free(shallow); - } + // Use *cib so that ACL filtering is applied to the answer + shallow = pcmk__xe_create(NULL, (const char *) (*cib)->name); + pcmk__xe_copy_attrs(shallow, *cib, pcmk__xaf_none); + cib__set_calldata(*answer, shallow); + pcmk__xml_free(shallow); pcmk__info("Reporting our current digest to %s: %s for %s.%s.%s", host, digest, @@ -361,7 +359,6 @@ sync_our_cib(xmlNode *request, bool all) pcmk__node_status_t *peer = NULL; xmlNode *replace_request = NULL; - CRM_CHECK(the_cib != NULL, return EINVAL); CRM_CHECK(all || (host != NULL), return EINVAL); pcmk__debug("Syncing CIB to %s", (all? "all peers" : host)); From 72254f7e0d47f36d79849ce1e606c956d1d75aa1 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 4 Jan 2026 00:26:26 -0800 Subject: [PATCH 21/47] Doc: based, libcib: Clarify cib__op_attr_modifies This TODO will be addressed in the next commit. So the following note is pretty much irrelevant, but I'm leaving it here for completeness. based_process_shutdown() can currently change the state by calling based_terminate(), and it does not have this flag. That will be remedied in a later commit (though likely not in the same pull request as this commit). Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 7 +++++++ include/crm/cib/internal.h | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 4d1830325f9..a2d719a308f 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -542,6 +542,13 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation, goto done; } + /* @TODO The cib__op_attr_modifies flag means the request *may* modify + * *something*. A successful request with this flag set may not have + * modified anything (for example, a delete request when there is no match + * to delete), or it may have modified something other than the CIB (for + * example, the CIB manager's primary/secondary status). Thus we may be + * setting the ping_modified_since flag when the CIB has not been modified. + */ ping_modified_since = true; /* result_cib must not be modified after cib__perform_op_rw() returns. diff --git a/include/crm/cib/internal.h b/include/crm/cib/internal.h index 2307511d33c..1e0b0370357 100644 --- a/include/crm/cib/internal.h +++ b/include/crm/cib/internal.h @@ -48,7 +48,7 @@ enum cib__op_attr { //! No special attributes cib__op_attr_none = 0, - //! Modifies CIB + //! May modify state (of the CIB itself or of the CIB manager) cib__op_attr_modifies = (UINT32_C(1) << 1), //! Requires privileges From d54935dae1d29a5bb97e2a94ebd4ba4ce27ae9f5 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Mon, 12 Jan 2026 11:07:38 -0800 Subject: [PATCH 22/47] Refactor: based: Set ping_modified_since to true only if the CIB changed The CIB changed if cib_diff is not NULL and the current request is not a dry-run. If the current request is part of a transaction, then we certainly don't want to sync the current CIB to other nodes, so one might think we'd want to set ping_modified_since to true. However, we process an entire transaction at once without returning to the main loop, so we won't enter process_ping_reply() until after the commit-transaction request anyway. Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index a2d719a308f..5d23514e58c 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -542,15 +542,6 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation, goto done; } - /* @TODO The cib__op_attr_modifies flag means the request *may* modify - * *something*. A successful request with this flag set may not have - * modified anything (for example, a delete request when there is no match - * to delete), or it may have modified something other than the CIB (for - * example, the CIB manager's primary/secondary status). Thus we may be - * setting the ping_modified_since flag when the CIB has not been modified. - */ - ping_modified_since = true; - /* result_cib must not be modified after cib__perform_op_rw() returns. * * It's not important whether the client variant is cib_native or @@ -594,6 +585,10 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation, sync_our_cib(request, true); } + if (cib_diff != NULL) { + ping_modified_since = true; + } + mainloop_timer_start(digest_timer); } else if (rc == pcmk_rc_schema_validation) { From b187b1b529a0ba19e4780846a38476c0253e4f86 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 4 Jan 2026 00:34:55 -0800 Subject: [PATCH 23/47] Doc: based: Add Doxygen for process_ping_reply() Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 5d23514e58c..58603826786 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -198,6 +198,24 @@ digest_timer_cb(gpointer data) return G_SOURCE_REMOVE; } +/*! + * \internal + * \brief Process a reply to a \c CRM_OP_PING request + * + * See \c digest_timer_cb() for details on how the ping process works, and see + * \c based_process_ping() for the construction of the ping reply. + * + * We ignore the reply if we are no longer the DC, if the reply is malformed or + * received out of sequence, or if we may have modified the CIB since the last + * time we sent a ping request. + * + * Otherwise, we compare the CIB digest received in the reply against the digest + * of the local CIB. If the digests don't match, we sync our CIB to the node + * that sent the reply. This helps to ensure that all other nodes' views of the + * CIB eventually match the DC's view of the CIB. + * + * \param[in] reply Ping reply + */ static void process_ping_reply(xmlNode *reply) { @@ -242,7 +260,6 @@ process_ping_reply(xmlNode *reply) } if (ping_digest == NULL) { - pcmk__trace("Calculating new digest"); ping_digest = pcmk__digest_xml(the_cib, true); } From c7dc7b2d429012feb3172379a167e1e23577561b Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 4 Jan 2026 00:37:19 -0800 Subject: [PATCH 24/47] Refactor: based: sync_our_cib() takes const argument Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 2 +- daemons/based/based_messages.c | 4 ++-- daemons/based/based_messages.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 58603826786..a44e1b202bd 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -217,7 +217,7 @@ digest_timer_cb(gpointer data) * \param[in] reply Ping reply */ static void -process_ping_reply(xmlNode *reply) +process_ping_reply(const xmlNode *reply) { const char *host = pcmk__xe_get(reply, PCMK__XA_SRC); diff --git a/daemons/based/based_messages.c b/daemons/based/based_messages.c index 4fdbd00774e..cbf2ee4ac6c 100644 --- a/daemons/based/based_messages.c +++ b/daemons/based/based_messages.c @@ -317,7 +317,7 @@ based_process_upgrade(xmlNode *req, xmlNode **cib, xmlNode **answer) } static xmlNode * -cib_msg_copy(xmlNode *msg) +cib_msg_copy(const xmlNode *msg) { static const char *field_list[] = { PCMK__XA_T, @@ -350,7 +350,7 @@ cib_msg_copy(xmlNode *msg) } int -sync_our_cib(xmlNode *request, bool all) +sync_our_cib(const xmlNode *request, bool all) { int rc = pcmk_rc_ok; char *digest = NULL; diff --git a/daemons/based/based_messages.h b/daemons/based/based_messages.h index 78d0d7e5218..30b934a8600 100644 --- a/daemons/based/based_messages.h +++ b/daemons/based/based_messages.h @@ -31,6 +31,6 @@ int based_process_shutdown(xmlNode *req, xmlNode **cib, xmlNode **answer); int based_process_sync(xmlNode *req, xmlNode **cib, xmlNode **answer); int based_process_upgrade(xmlNode *req, xmlNode **cib, xmlNode **answer); -int sync_our_cib(xmlNode *request, bool all); +int sync_our_cib(const xmlNode *request, bool all); #endif // BASED_MESSAGES__H From d5ca362884abf79609272f1a6bdf0b9664794a7d Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 4 Jan 2026 00:39:10 -0800 Subject: [PATCH 25/47] Low: based: Free ping_digest on exit Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 1 + 1 file changed, 1 insertion(+) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index a44e1b202bd..78469b461ad 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -1029,6 +1029,7 @@ based_terminate(int exit_status) remote_tls_fd = 0; } + g_clear_pointer(&ping_digest, free); g_clear_pointer(&the_cib, pcmk__xml_free); // Exit immediately on error From 38cfbabfab8890489fe68fab8f890fb68cb9048a Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 4 Jan 2026 01:12:29 -0800 Subject: [PATCH 26/47] Refactor: based: Minor reorganization of cib_process_command() Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 38 +++++++++++++-------------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 78469b461ad..c5a6ba48e3c 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -517,33 +517,27 @@ static int cib_process_command(xmlNode *request, const cib__operation_t *operation, cib__op_fn_t op_function, xmlNode **reply, bool privileged) { + static mainloop_timer_t *digest_timer = NULL; + xmlNode *cib_diff = NULL; xmlNode *output = NULL; xmlNode *result_cib = NULL; - uint32_t call_options = cib_none; - const char *op = pcmk__xe_get(request, PCMK__XA_CIB_OP); const char *call_id = pcmk__xe_get(request, PCMK__XA_CIB_CALLID); const char *client_id = pcmk__xe_get(request, PCMK__XA_CIB_CLIENTID); const char *client_name = pcmk__xe_get(request, PCMK__XA_CIB_CLIENTNAME); const char *originator = pcmk__xe_get(request, PCMK__XA_SRC); - - int rc = pcmk_rc_ok; + uint32_t call_options = cib_none; bool config_changed = false; - - static mainloop_timer_t *digest_timer = NULL; - - pcmk__assert(cib_status == pcmk_rc_ok); + int rc = pcmk_rc_ok; if (digest_timer == NULL) { digest_timer = mainloop_timer_add("based_digest_timer", 5000, false, digest_timer_cb, NULL); } - *reply = NULL; - /* Start processing the request... */ pcmk__xe_get_flags(request, PCMK__XA_CIB_CALLOPT, &call_options, cib_none); @@ -568,20 +562,21 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation, rc = cib__perform_op_rw(cib_undefined, op_function, request, &config_changed, &result_cib, &cib_diff, &output); - /* Always write to disk for successful ops with the flag set. This also - * negates the need to detect ordering changes. - */ if ((rc == pcmk_rc_ok) - && pcmk__is_set(operation->flags, cib__op_attr_writes_through)) { + && !pcmk__any_flags_set(call_options, cib_dryrun|cib_transaction)) { - config_changed = true; - } + /* Always write to disk for successful ops with the writes-through flag + * set. This also avoids the need to detect ordering changes. + */ + const bool to_disk = config_changed + || pcmk__is_set(operation->flags, + cib__op_attr_writes_through); - if ((rc == pcmk_rc_ok) - && !pcmk__any_flags_set(call_options, cib_dryrun|cib_transaction)) { + const char *feature_set = pcmk__xe_get(the_cib, + PCMK_XA_CRM_FEATURE_SET); if (result_cib != the_cib) { - rc = based_activate_cib(result_cib, config_changed, op); + rc = based_activate_cib(result_cib, to_disk, op); } /* @COMPAT Nodes older than feature set 3.19.0 don't support @@ -595,9 +590,7 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation, */ if ((operation->type == cib__op_commit_transact) && pcmk__str_eq(originator, OUR_NODENAME, pcmk__str_casei) - && (pcmk__compare_versions(pcmk__xe_get(the_cib, - PCMK_XA_CRM_FEATURE_SET), - "3.19.0") < 0)) { + && (pcmk__compare_versions(feature_set, "3.19.0") < 0)) { sync_our_cib(request, true); } @@ -640,7 +633,6 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation, pcmk__xml_free(output); } - pcmk__trace("done"); pcmk__xml_free(cib_diff); return rc; } From 13421e3ace30106cb783759da4dfbacb188fc7f0 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Mon, 5 Jan 2026 23:47:31 -0800 Subject: [PATCH 27/47] Feature: libcrmcommon: Assert on memory error in mainloop_add_fd() This technically changes public-facing behavior. I don't see any reason why anything external should be using our mainloop functions, however, so it's only a matter of time until they're deprecated from public API. Signed-off-by: Reid Wahl --- lib/common/mainloop.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/common/mainloop.c b/lib/common/mainloop.c index 1a0e8dd4a72..bd1a54ee932 100644 --- a/lib/common/mainloop.c +++ b/lib/common/mainloop.c @@ -953,11 +953,8 @@ mainloop_add_fd(const char *name, int priority, int fd, void *userdata, mainloop_io_t *client = NULL; if (fd >= 0) { - client = calloc(1, sizeof(mainloop_io_t)); - if (client == NULL) { - return NULL; - } - client->name = strdup(name); + client = pcmk__assert_alloc(1, sizeof(mainloop_io_t)); + client->name = pcmk__str_copy(name); client->userdata = userdata; if (callbacks) { From 51c3876cd3b2656830f30747ee0ffb5b1b35986f Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Mon, 5 Jan 2026 23:57:55 -0800 Subject: [PATCH 28/47] Refactor: libcrmcommon: Unindent mainloop_add_fd() Signed-off-by: Reid Wahl --- lib/common/mainloop.c | 59 ++++++++++++++++++++++--------------------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/lib/common/mainloop.c b/lib/common/mainloop.c index bd1a54ee932..3ea23855e44 100644 --- a/lib/common/mainloop.c +++ b/lib/common/mainloop.c @@ -951,40 +951,41 @@ mainloop_add_fd(const char *name, int priority, int fd, void *userdata, struct mainloop_fd_callbacks * callbacks) { mainloop_io_t *client = NULL; + const GIOCondition condition = G_IO_IN|G_IO_HUP|G_IO_NVAL|G_IO_ERR; - if (fd >= 0) { - client = pcmk__assert_alloc(1, sizeof(mainloop_io_t)); - client->name = pcmk__str_copy(name); - client->userdata = userdata; - - if (callbacks) { - client->destroy_fn = callbacks->destroy; - client->dispatch_fn_io = callbacks->dispatch; - } + if (fd < 0) { + errno = EINVAL; + return NULL; + } - client->fd = fd; - client->channel = g_io_channel_unix_new(fd); - client->source = - g_io_add_watch_full(client->channel, priority, - (G_IO_IN | G_IO_HUP | G_IO_NVAL | G_IO_ERR), mainloop_gio_callback, - client, mainloop_gio_destroy); + client = pcmk__assert_alloc(1, sizeof(mainloop_io_t)); + client->name = pcmk__str_copy(name); + client->userdata = userdata; - /* Now that mainloop now holds a reference to channel, - * thanks to g_io_add_watch_full(), drop ours from g_io_channel_unix_new(). - * - * This means that channel will be free'd by: - * g_main_context_dispatch() or g_source_remove() - * -> g_source_destroy_internal() - * -> g_source_callback_unref() - * shortly after mainloop_gio_destroy() completes - */ - g_io_channel_unref(client->channel); - pcmk__trace("Added connection %d for %s[%p].%d", client->source, - client->name, client, fd); - } else { - errno = EINVAL; + if (callbacks != NULL) { + client->destroy_fn = callbacks->destroy; + client->dispatch_fn_io = callbacks->dispatch; } + client->fd = fd; + client->channel = g_io_channel_unix_new(fd); + client->source = g_io_add_watch_full(client->channel, priority, condition, + mainloop_gio_callback, client, + mainloop_gio_destroy); + + /* Now that mainloop now holds a reference to channel, thanks to + * g_io_add_watch_full(), drop ours from g_io_channel_unix_new(). + * + * This means that channel will be free'd by: + * g_main_context_dispatch() or g_source_remove() + * -> g_source_destroy_internal() + * -> g_source_callback_unref() + * shortly after mainloop_gio_destroy() completes + */ + g_io_channel_unref(client->channel); + + pcmk__trace("Added connection %d for %s[%p].%d", client->source, + client->name, client, fd); return client; } From e2c56c902961a7b5e397ad7f3ff2ce0684609b92 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Mon, 5 Jan 2026 23:39:09 -0800 Subject: [PATCH 29/47] Low: based: Free remote client if TLS session creation fails This wasn't a memory leak, because we would eventually free the client table. Now every based remote client is guaranteed to have a remote->source. Signed-off-by: Reid Wahl --- daemons/based/based_remote.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/daemons/based/based_remote.c b/daemons/based/based_remote.c index edfb6796501..0cc025c7eca 100644 --- a/daemons/based/based_remote.c +++ b/daemons/based/based_remote.c @@ -597,9 +597,13 @@ cib_remote_listen(gpointer data) /* create gnutls session for the server socket */ new_client->remote->tls_session = pcmk__new_tls_session(tls, csock); if (new_client->remote->tls_session == NULL) { + pcmk__err("Dropping remote connection from %s because we failed to " + "create a TLS session for it", ipstr); + pcmk__free_client(new_client); close(csock); return 0; } + } else { pcmk__set_client_flags(new_client, pcmk__client_tcp); new_client->remote->tcp_socket = csock; From 5ddbbc088d33e1dd7d67176a16527379088da613 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Mon, 5 Jan 2026 22:15:34 -0800 Subject: [PATCH 30/47] Refactor: based: Drop qb_ipcs_stats_get() call Other daemons don't check the number of active IPC client connections via libqb. It seems reasonable to follow the other daemons' lead and assume that the return value of pcmk__ipc_client_count() is all we need to care about. Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index c5a6ba48e3c..ad1e078fd44 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -20,7 +20,6 @@ #include // gboolean, gpointer, g_*, etc. #include // xmlNode -#include // QB_FALSE #include // qb_ipcs_connection_t #include // LOG_TRACE @@ -938,8 +937,6 @@ initiate_exit(void) void based_shutdown(int nsig) { - struct qb_ipcs_stats srv_stats; - if (!cib_shutdown_flag) { int disconnects = 0; qb_ipcs_connection_t *c = NULL; @@ -990,15 +987,13 @@ based_shutdown(int nsig) pcmk__info("Disconnected %d clients", disconnects); } - qb_ipcs_stats_get(ipcs_rw, &srv_stats, QB_FALSE); - if (pcmk__ipc_client_count() == 0) { - pcmk__info("All clients disconnected (%d)", srv_stats.active_connections); + pcmk__info("All clients disconnected"); initiate_exit(); } else { - pcmk__info("Waiting on %d clients to disconnect (%d)", - pcmk__ipc_client_count(), srv_stats.active_connections); + pcmk__info("Waiting on %d clients to disconnect", + pcmk__ipc_client_count()); } } From 50d70f1d4eeb00eaf3455f56e774dc0e0e27af08 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Thu, 8 Jan 2026 15:06:27 -0800 Subject: [PATCH 31/47] Doc: libcrmcommon: Add detail to mainloop_del_fd() Doxygen Somehow I got pretty confused last night about when or if the callback data was guaranteed to be destroyed. Note that the GSource itself is not guaranteed to be freed yet. There may be other references to it -- for example, if it has been dispatched but its callback hasn't run yet. This was the main reason for my confusion. However, since g_source_remove() marks the source as destroyed, nothing will happen if the source gets dispatched. Signed-off-by: Reid Wahl --- lib/common/mainloop.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/common/mainloop.c b/lib/common/mainloop.c index 3ea23855e44..3293e1b66de 100644 --- a/lib/common/mainloop.c +++ b/lib/common/mainloop.c @@ -998,7 +998,12 @@ mainloop_del_fd(mainloop_io_t *client) pcmk__trace("Removing client %s[%p]", client->name, client); - // mainloop_gio_destroy() gets called during source removal + /* g_source_remove() marks the source as destroyed, unsets the source + * callback (mainloop_gio_callback()), and destroys the callback data (the + * client) via the notify function (mainloop_gio_destroy()). We can rely on + * mainloop_gio_callback() not getting called again for this source, and on + * the client being destroyed. + */ g_source_remove(client->source); } From be3243be3fa35738f918faec8affa84d9aeb94ac Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 6 Jan 2026 01:04:20 -0800 Subject: [PATCH 32/47] Low: based: Drop remote clients during shutdown Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 11 +---------- daemons/based/based_remote.c | 35 +++++++++++++++++++++++++++++++++ daemons/based/based_remote.h | 1 + 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index ad1e078fd44..b123616f846 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -899,15 +899,6 @@ cib_force_exit(gpointer data) return FALSE; } -static void -disconnect_remote_client(gpointer key, gpointer value, gpointer user_data) -{ - pcmk__client_t *a_client = value; - - pcmk__err("Can't disconnect client %s: Not implemented", - pcmk__client_name(a_client)); -} - static void initiate_exit(void) { @@ -983,7 +974,7 @@ based_shutdown(int nsig) pcmk__debug("Disconnecting %d remote clients", pcmk__ipc_client_count()); - pcmk__foreach_ipc_client(disconnect_remote_client, NULL); + based_drop_remote_clients(); pcmk__info("Disconnected %d clients", disconnects); } diff --git a/daemons/based/based_remote.c b/daemons/based/based_remote.c index 0cc025c7eca..5d3ece67549 100644 --- a/daemons/based/based_remote.c +++ b/daemons/based/based_remote.c @@ -764,3 +764,38 @@ based_remote_init(void) remote_fd = init_remote_listener(port); } } + +/*! + * \internal + * \brief Disconnect and free a CIB manager client if it is a remote client + * + * If \p value is a remote client, drop it by removing its source from the + * mainloop. It will be freed by \c based_remote_client_destroy() via + * \c remote_client_fd_callbacks. + * + * \param[in] key Ignored + * \param[in,out] value CIB manager client (pcmk__client_t *) + * \param[in] user_data Ignored + */ +static void +drop_client_if_remote(gpointer key, gpointer value, gpointer user_data) +{ + pcmk__client_t *client = value; + + if (client->remote == NULL) { + return; + } + + pcmk__notice("Disconnecting remote client %s", pcmk__client_name(client)); + mainloop_del_fd(client->remote->source); +} + +/*! + * \internal + * \brief Disconnect and free all remote CIB manager clients + */ +void +based_drop_remote_clients(void) +{ + pcmk__foreach_ipc_client(drop_client_if_remote, NULL); +} diff --git a/daemons/based/based_remote.h b/daemons/based/based_remote.h index d2ab8f00802..1763dc04f55 100644 --- a/daemons/based/based_remote.h +++ b/daemons/based/based_remote.h @@ -14,5 +14,6 @@ extern int remote_fd; extern int remote_tls_fd; void based_remote_init(void); +void based_drop_remote_clients(void); #endif // BASED_REMOTE__H From ca4e05e34979120c49dd10a5e290364d7555097d Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 4 Jan 2026 01:22:39 -0800 Subject: [PATCH 33/47] Refactor: based: Use pcmk__drop_all_clients() in based_shutdown() To align better with attrd, fenced, and schedulerd. Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 47 +++++++-------------------------- 1 file changed, 9 insertions(+), 38 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index b123616f846..0c4992cfd8b 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -929,53 +929,24 @@ void based_shutdown(int nsig) { if (!cib_shutdown_flag) { - int disconnects = 0; - qb_ipcs_connection_t *c = NULL; - cib_shutdown_flag = true; - c = qb_ipcs_connection_first_get(ipcs_rw); - while (c != NULL) { - qb_ipcs_connection_t *last = c; - - c = qb_ipcs_connection_next_get(ipcs_rw, last); - - pcmk__debug("Disconnecting r/w client %p...", last); - qb_ipcs_disconnect(last); - qb_ipcs_connection_unref(last); - disconnects++; + if (ipcs_ro != NULL) { + pcmk__drop_all_clients(ipcs_ro); + g_clear_pointer(&ipcs_ro, qb_ipcs_destroy); } - c = qb_ipcs_connection_first_get(ipcs_ro); - while (c != NULL) { - qb_ipcs_connection_t *last = c; - - c = qb_ipcs_connection_next_get(ipcs_ro, last); - - pcmk__debug("Disconnecting r/o client %p...", last); - qb_ipcs_disconnect(last); - qb_ipcs_connection_unref(last); - disconnects++; + if (ipcs_rw != NULL) { + pcmk__drop_all_clients(ipcs_rw); + g_clear_pointer(&ipcs_rw, qb_ipcs_destroy); } - c = qb_ipcs_connection_first_get(ipcs_shm); - while (c != NULL) { - qb_ipcs_connection_t *last = c; - - c = qb_ipcs_connection_next_get(ipcs_shm, last); - - pcmk__debug("Disconnecting non-blocking r/w client %p...", last); - qb_ipcs_disconnect(last); - qb_ipcs_connection_unref(last); - disconnects++; + if (ipcs_shm != NULL) { + pcmk__drop_all_clients(ipcs_shm); + g_clear_pointer(&ipcs_shm, qb_ipcs_destroy); } - disconnects += pcmk__ipc_client_count(); - - pcmk__debug("Disconnecting %d remote clients", - pcmk__ipc_client_count()); based_drop_remote_clients(); - pcmk__info("Disconnected %d clients", disconnects); } if (pcmk__ipc_client_count() == 0) { From b2a8d47cca733c0376271ba136cef239973e1c88 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 6 Jan 2026 01:26:34 -0800 Subject: [PATCH 34/47] Refactor: based: Call based_shutdown() only once All IPC and remote clients have been disconnected and freed by the time we return from based_shutdown(). There's no need to wait for their destroy callbacks, or to have those callbacks call based_shutdown() again. Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 40 +++++++++++++++------------------ daemons/based/based_ipc.c | 9 -------- daemons/based/based_remote.c | 4 ---- 3 files changed, 18 insertions(+), 35 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 0c4992cfd8b..ea607eb8c99 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -928,35 +928,31 @@ initiate_exit(void) void based_shutdown(int nsig) { - if (!cib_shutdown_flag) { - cib_shutdown_flag = true; + if (cib_shutdown_flag) { + // Already shutting down + return; + } - if (ipcs_ro != NULL) { - pcmk__drop_all_clients(ipcs_ro); - g_clear_pointer(&ipcs_ro, qb_ipcs_destroy); - } + cib_shutdown_flag = true; - if (ipcs_rw != NULL) { - pcmk__drop_all_clients(ipcs_rw); - g_clear_pointer(&ipcs_rw, qb_ipcs_destroy); - } + if (ipcs_ro != NULL) { + pcmk__drop_all_clients(ipcs_ro); + g_clear_pointer(&ipcs_ro, qb_ipcs_destroy); + } - if (ipcs_shm != NULL) { - pcmk__drop_all_clients(ipcs_shm); - g_clear_pointer(&ipcs_shm, qb_ipcs_destroy); - } + if (ipcs_rw != NULL) { + pcmk__drop_all_clients(ipcs_rw); + g_clear_pointer(&ipcs_rw, qb_ipcs_destroy); + } - based_drop_remote_clients(); + if (ipcs_shm != NULL) { + pcmk__drop_all_clients(ipcs_shm); + g_clear_pointer(&ipcs_shm, qb_ipcs_destroy); } - if (pcmk__ipc_client_count() == 0) { - pcmk__info("All clients disconnected"); - initiate_exit(); + based_drop_remote_clients(); - } else { - pcmk__info("Waiting on %d clients to disconnect", - pcmk__ipc_client_count()); - } + initiate_exit(); } /*! diff --git a/daemons/based/based_ipc.c b/daemons/based/based_ipc.c index 7d59c025f9b..ff3f5fa316a 100644 --- a/daemons/based/based_ipc.c +++ b/daemons/based/based_ipc.c @@ -280,15 +280,6 @@ based_ipc_destroy(qb_ipcs_connection_t *c) { pcmk__trace("Destroying client connection %p", c); based_ipc_closed(c); - - /* Shut down if this was the last client to leave. - * - * @TODO Is it correct to do this for destroy but not for closed? Other - * daemons handle closed and destroyed connections in the same way. - */ - if (cib_shutdown_flag) { - based_shutdown(0); - } } struct qb_ipcs_service_handlers ipc_ro_callbacks = { diff --git a/daemons/based/based_remote.c b/daemons/based/based_remote.c index 5d3ece67549..3746d53d887 100644 --- a/daemons/based/based_remote.c +++ b/daemons/based/based_remote.c @@ -544,10 +544,6 @@ based_remote_client_destroy(gpointer user_data) pcmk__free_client(client); pcmk__trace("Freed the cib client"); - - if (cib_shutdown_flag) { - based_shutdown(0); - } } static int From c31b052005cc51dbbd221d8dffd40ee608c63b51 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 6 Jan 2026 01:34:43 -0800 Subject: [PATCH 35/47] Refactor: based: Inline initiate_exit() It no longer has a distinct role; this was no longer a logical split. Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 48 +++++++++++++++------------------ 1 file changed, 21 insertions(+), 27 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index ea607eb8c99..fd5e84a99ec 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -899,35 +899,12 @@ cib_force_exit(gpointer data) return FALSE; } -static void -initiate_exit(void) -{ - int active = 0; - xmlNode *leaving = NULL; - - active = pcmk__cluster_num_active_nodes(); - if (active < 2) { // This is the last active node - pcmk__info("Exiting without sending shutdown request (no active " - "peers)"); - based_terminate(CRM_EX_OK); - return; - } - - pcmk__info("Sending shutdown request to %d peers", active); - - leaving = pcmk__xe_create(NULL, PCMK__XE_EXIT_NOTIFICATION); - pcmk__xe_set(leaving, PCMK__XA_T, PCMK__VALUE_CIB); - pcmk__xe_set(leaving, PCMK__XA_CIB_OP, PCMK__CIB_REQUEST_SHUTDOWN); - - pcmk__cluster_send_message(NULL, pcmk_ipc_based, leaving); - pcmk__xml_free(leaving); - - pcmk__create_timer(EXIT_ESCALATION_MS, cib_force_exit, NULL); -} - void based_shutdown(int nsig) { + int active = 0; + xmlNode *notification = NULL; + if (cib_shutdown_flag) { // Already shutting down return; @@ -952,7 +929,24 @@ based_shutdown(int nsig) based_drop_remote_clients(); - initiate_exit(); + active = pcmk__cluster_num_active_nodes(); + if (active < 2) { + pcmk__info("Exiting without sending shutdown request (no active " + "peers)"); + based_terminate(CRM_EX_OK); + return; + } + + pcmk__info("Sending shutdown request to %d peers", active); + + notification = pcmk__xe_create(NULL, PCMK__XE_EXIT_NOTIFICATION); + pcmk__xe_set(notification, PCMK__XA_T, PCMK__VALUE_CIB); + pcmk__xe_set(notification, PCMK__XA_CIB_OP, PCMK__CIB_REQUEST_SHUTDOWN); + + pcmk__cluster_send_message(NULL, pcmk_ipc_based, notification); + pcmk__xml_free(notification); + + pcmk__create_timer(EXIT_ESCALATION_MS, cib_force_exit, NULL); } /*! From f470dbfe47b996abb88fb17e3280f376982fefb7 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 4 Jan 2026 12:58:16 -0800 Subject: [PATCH 36/47] Refactor: based: create_cib_reply/based_diff_notify take standard rc Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 16 ++++++++-------- daemons/based/based_notify.c | 4 ++-- daemons/based/based_notify.h | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index fd5e84a99ec..03cc29e7b4b 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -51,7 +51,7 @@ static bool ping_modified_since = false; * \param[in] call_id CIB call ID * \param[in] client_id CIB client ID * \param[in] call_options Group of enum cib_call_options flags - * \param[in] rc Request return code + * \param[in] rc Request return code (standard Pacemaker return code) * \param[in] call_data Request output data * * \return Reply XML (guaranteed not to be \c NULL) @@ -70,7 +70,7 @@ create_cib_reply(const char *op, const char *call_id, const char *client_id, pcmk__xe_set(reply, PCMK__XA_CIB_CALLID, call_id); pcmk__xe_set(reply, PCMK__XA_CIB_CLIENTID, client_id); pcmk__xe_set_int(reply, PCMK__XA_CIB_CALLOPT, call_options); - pcmk__xe_set_int(reply, PCMK__XA_CIB_RC, rc); + pcmk__xe_set_int(reply, PCMK__XA_CIB_RC, pcmk_rc2legacy(rc)); cib__set_calldata(reply, call_data); crm_log_xml_explicit(reply, "cib:reply"); @@ -619,13 +619,13 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation, goto done; } - based_diff_notify(op, pcmk_rc2legacy(rc), call_id, client_id, client_name, - originator, cib_diff); + based_diff_notify(op, rc, call_id, client_id, client_name, originator, + cib_diff); done: if (!pcmk__is_set(call_options, cib_discard_reply)) { - *reply = create_cib_reply(op, call_id, client_id, call_options, - pcmk_rc2legacy(rc), output); + *reply = create_cib_reply(op, call_id, client_id, call_options, rc, + output); } if (output != the_cib) { @@ -826,8 +826,8 @@ based_process_request(xmlNode *request, bool privileged, rc = cib_status; pcmk__err("Ignoring request because cluster configuration is invalid " "(please repair and restart): %s", pcmk_rc_str(rc)); - reply = create_cib_reply(op, call_id, client_id, call_options, - pcmk_rc2legacy(rc), the_cib); + reply = create_cib_reply(op, call_id, client_id, call_options, rc, + the_cib); } else if (process) { time_t start_time = time(NULL); diff --git a/daemons/based/based_notify.c b/daemons/based/based_notify.c index 2e671b9c35e..a71f03596d8 100644 --- a/daemons/based/based_notify.c +++ b/daemons/based/based_notify.c @@ -199,7 +199,7 @@ cib_notify_send(const xmlNode *xml) } void -based_diff_notify(const char *op, int result, const char *call_id, +based_diff_notify(const char *op, int rc, const char *call_id, const char *client_id, const char *client_name, const char *origin, xmlNode *diff) { @@ -219,7 +219,7 @@ based_diff_notify(const char *op, int result, const char *call_id, pcmk__xe_set(update_msg, PCMK__XA_CIB_CLIENTNAME, client_name); pcmk__xe_set(update_msg, PCMK__XA_CIB_CALLID, call_id); pcmk__xe_set(update_msg, PCMK__XA_SRC, origin); - pcmk__xe_set_int(update_msg, PCMK__XA_CIB_RC, result); + pcmk__xe_set_int(update_msg, PCMK__XA_CIB_RC, pcmk_rc2legacy(rc)); wrapper = pcmk__xe_create(update_msg, PCMK__XE_CIB_UPDATE_RESULT); pcmk__xml_copy(wrapper, diff); diff --git a/daemons/based/based_notify.h b/daemons/based/based_notify.h index 79886f79aa1..a4641513085 100644 --- a/daemons/based/based_notify.h +++ b/daemons/based/based_notify.h @@ -16,7 +16,7 @@ int based_update_notify_flags(const xmlNode *xml, pcmk__client_t *client); -void based_diff_notify(const char *op, int result, const char *call_id, +void based_diff_notify(const char *op, int rc, const char *call_id, const char *client_id, const char *client_name, const char *origin, xmlNode *diff); From 91b0122b84478d78a6255b7c91f50155615e2264 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 4 Jan 2026 13:06:53 -0800 Subject: [PATCH 37/47] Refactor: based: based_diff_notify() takes request argument Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 4 +--- daemons/based/based_notify.c | 27 +++++++++++++++++++-------- daemons/based/based_notify.h | 4 +--- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 03cc29e7b4b..fc1fd7e41cc 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -525,7 +525,6 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation, const char *op = pcmk__xe_get(request, PCMK__XA_CIB_OP); const char *call_id = pcmk__xe_get(request, PCMK__XA_CIB_CALLID); const char *client_id = pcmk__xe_get(request, PCMK__XA_CIB_CLIENTID); - const char *client_name = pcmk__xe_get(request, PCMK__XA_CIB_CLIENTNAME); const char *originator = pcmk__xe_get(request, PCMK__XA_SRC); uint32_t call_options = cib_none; @@ -619,8 +618,7 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation, goto done; } - based_diff_notify(op, rc, call_id, client_id, client_name, originator, - cib_diff); + based_diff_notify(request, rc, cib_diff); done: if (!pcmk__is_set(call_options, cib_discard_reply)) { diff --git a/daemons/based/based_notify.c b/daemons/based/based_notify.c index a71f03596d8..21458325b79 100644 --- a/daemons/based/based_notify.c +++ b/daemons/based/based_notify.c @@ -199,9 +199,7 @@ cib_notify_send(const xmlNode *xml) } void -based_diff_notify(const char *op, int rc, const char *call_id, - const char *client_id, const char *client_name, - const char *origin, xmlNode *diff) +based_diff_notify(const xmlNode *request, int rc, xmlNode *diff) { xmlNode *update_msg = NULL; xmlNode *wrapper = NULL; @@ -212,13 +210,26 @@ based_diff_notify(const char *op, int rc, const char *call_id, update_msg = pcmk__xe_create(NULL, PCMK__XE_NOTIFY); + /* We could simplify by copying all attributes from request. We would just + * have to ensure that there are never "private" attributes that we want to + * hide from external clients with notify callbacks. + */ pcmk__xe_set(update_msg, PCMK__XA_T, PCMK__VALUE_CIB_NOTIFY); pcmk__xe_set(update_msg, PCMK__XA_SUBT, PCMK__VALUE_CIB_DIFF_NOTIFY); - pcmk__xe_set(update_msg, PCMK__XA_CIB_OP, op); - pcmk__xe_set(update_msg, PCMK__XA_CIB_CLIENTID, client_id); - pcmk__xe_set(update_msg, PCMK__XA_CIB_CLIENTNAME, client_name); - pcmk__xe_set(update_msg, PCMK__XA_CIB_CALLID, call_id); - pcmk__xe_set(update_msg, PCMK__XA_SRC, origin); + + pcmk__xe_set(update_msg, PCMK__XA_CIB_OP, + pcmk__xe_get(request, PCMK__XA_CIB_OP)); + + pcmk__xe_set(update_msg, PCMK__XA_CIB_CLIENTID, + pcmk__xe_get(request, PCMK__XA_CIB_CLIENTID)); + + pcmk__xe_set(update_msg, PCMK__XA_CIB_CLIENTNAME, + pcmk__xe_get(request, PCMK__XA_CIB_CLIENTNAME)); + + pcmk__xe_set(update_msg, PCMK__XA_CIB_CALLID, + pcmk__xe_get(request, PCMK__XA_CIB_CALLID)); + + pcmk__xe_set(update_msg, PCMK__XA_SRC, pcmk__xe_get(request, PCMK__XA_SRC)); pcmk__xe_set_int(update_msg, PCMK__XA_CIB_RC, pcmk_rc2legacy(rc)); wrapper = pcmk__xe_create(update_msg, PCMK__XE_CIB_UPDATE_RESULT); diff --git a/daemons/based/based_notify.h b/daemons/based/based_notify.h index a4641513085..aec79e7b0b7 100644 --- a/daemons/based/based_notify.h +++ b/daemons/based/based_notify.h @@ -16,8 +16,6 @@ int based_update_notify_flags(const xmlNode *xml, pcmk__client_t *client); -void based_diff_notify(const char *op, int rc, const char *call_id, - const char *client_id, const char *client_name, - const char *origin, xmlNode *diff); +void based_diff_notify(const xmlNode *request, int rc, xmlNode *diff); #endif // BASED_NOTIFY__H From 3bf933154bcb8596131450dee578b71e226ffae2 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 4 Jan 2026 13:17:54 -0800 Subject: [PATCH 38/47] Refactor: based: create_cib_reply() takes request argument Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 42 +++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index fc1fd7e41cc..0da21eae6e9 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -47,12 +47,10 @@ static bool ping_modified_since = false; * \internal * \brief Create reply XML for a CIB request * - * \param[in] op CIB operation type - * \param[in] call_id CIB call ID - * \param[in] client_id CIB client ID - * \param[in] call_options Group of enum cib_call_options flags - * \param[in] rc Request return code (standard Pacemaker return code) - * \param[in] call_data Request output data + * \param[in] request CIB request + * \param[in] rc Request return code (standard Pacemaker return code) + * \param[in] call_data Request output data (may be entire live CIB or result + * CIB in case of error) * * \return Reply XML (guaranteed not to be \c NULL) * @@ -60,16 +58,28 @@ static bool ping_modified_since = false; * \p pcmk__xml_free(). */ static xmlNode * -create_cib_reply(const char *op, const char *call_id, const char *client_id, - uint32_t call_options, int rc, xmlNode *call_data) +create_cib_reply(const xmlNode *request, int rc, xmlNode *call_data) { xmlNode *reply = pcmk__xe_create(NULL, PCMK__XE_CIB_REPLY); pcmk__xe_set(reply, PCMK__XA_T, PCMK__VALUE_CIB); - pcmk__xe_set(reply, PCMK__XA_CIB_OP, op); - pcmk__xe_set(reply, PCMK__XA_CIB_CALLID, call_id); - pcmk__xe_set(reply, PCMK__XA_CIB_CLIENTID, client_id); - pcmk__xe_set_int(reply, PCMK__XA_CIB_CALLOPT, call_options); + + /* We could simplify by copying all attributes from request. We would just + * have to ensure that there are never "private" attributes that we want to + * hide from external clients with notify callbacks. + */ + pcmk__xe_set(reply, PCMK__XA_CIB_OP, + pcmk__xe_get(request, PCMK__XA_CIB_OP)); + + pcmk__xe_set(reply, PCMK__XA_CIB_CALLID, + pcmk__xe_get(request, PCMK__XA_CIB_CALLID)); + + pcmk__xe_set(reply, PCMK__XA_CIB_CLIENTID, + pcmk__xe_get(request, PCMK__XA_CIB_CLIENTID)); + + pcmk__xe_set(reply, PCMK__XA_CIB_CALLOPT, + pcmk__xe_get(request, PCMK__XA_CIB_CALLOPT)); + pcmk__xe_set_int(reply, PCMK__XA_CIB_RC, pcmk_rc2legacy(rc)); cib__set_calldata(reply, call_data); @@ -523,8 +533,6 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation, xmlNode *result_cib = NULL; const char *op = pcmk__xe_get(request, PCMK__XA_CIB_OP); - const char *call_id = pcmk__xe_get(request, PCMK__XA_CIB_CALLID); - const char *client_id = pcmk__xe_get(request, PCMK__XA_CIB_CLIENTID); const char *originator = pcmk__xe_get(request, PCMK__XA_SRC); uint32_t call_options = cib_none; @@ -622,8 +630,7 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation, done: if (!pcmk__is_set(call_options, cib_discard_reply)) { - *reply = create_cib_reply(op, call_id, client_id, call_options, rc, - output); + *reply = create_cib_reply(request, rc, output); } if (output != the_cib) { @@ -824,8 +831,7 @@ based_process_request(xmlNode *request, bool privileged, rc = cib_status; pcmk__err("Ignoring request because cluster configuration is invalid " "(please repair and restart): %s", pcmk_rc_str(rc)); - reply = create_cib_reply(op, call_id, client_id, call_options, rc, - the_cib); + reply = create_cib_reply(request, rc, the_cib); } else if (process) { time_t start_time = time(NULL); From 2df7d058ea0e4cede673b93399a9491289bfa254 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 4 Jan 2026 13:25:25 -0800 Subject: [PATCH 39/47] Doc: based: Drop irrelevant comment about legacy mode Legacy mode was removed by commit 7198fa62 (by making cib_legacy_mode() return FALSE). Then commit 061277ee made this comment's irrelevance explicit. Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 0da21eae6e9..47342756c08 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -813,10 +813,6 @@ based_process_request(xmlNode *request, bool privileged, } if (pcmk__is_set(call_options, cib_discard_reply)) { - /* If the request will modify the CIB, and we are in legacy mode, we - * need to build a reply so we can broadcast a diff, even if the - * requester doesn't want one. - */ needs_reply = false; local_notify = false; pcmk__trace("Client is not interested in the reply"); From 034dea0ecf8412a809ae3863b4a4d951450ea24f Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 4 Jan 2026 14:27:06 -0800 Subject: [PATCH 40/47] Refactor: based: Unindent do_local_notify() call Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 47342756c08..6fd86b1786b 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -853,12 +853,14 @@ based_process_request(xmlNode *request, bool privileged, send_peer_reply(reply, originator); } - if (local_notify && (client_id != NULL)) { - do_local_notify((process? reply : request), client_id, - pcmk__is_set(call_options, cib_sync_call), - (client == NULL)); + if (!local_notify || (client_id == NULL)) { + goto done; } + do_local_notify((process? reply : request), client_id, + pcmk__is_set(call_options, cib_sync_call), + (client == NULL)); + done: pcmk__xml_free(reply); return rc; From 79f22326dbb52474592fb734041f4cd426fe42fc Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 4 Jan 2026 14:33:21 -0800 Subject: [PATCH 41/47] Refactor: based: Guard create_cib_reply() call with !cib_discard_reply The other call is already guarded. Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 6fd86b1786b..874262eeda3 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -827,7 +827,10 @@ based_process_request(xmlNode *request, bool privileged, rc = cib_status; pcmk__err("Ignoring request because cluster configuration is invalid " "(please repair and restart): %s", pcmk_rc_str(rc)); - reply = create_cib_reply(request, rc, the_cib); + + if (!pcmk__is_set(call_options, cib_discard_reply)) { + reply = create_cib_reply(request, rc, the_cib); + } } else if (process) { time_t start_time = time(NULL); From be2a8018e3ea99d2adf4d3a61dc71a51c11f1d32 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 4 Jan 2026 14:37:25 -0800 Subject: [PATCH 42/47] Refactor: based: Move privilege check to based_process_request() Just an incremental change. This will simplify further in upcoming commits. Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 874262eeda3..8d23f8894ae 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -524,7 +524,7 @@ forward_request(xmlNode *request) static int cib_process_command(xmlNode *request, const cib__operation_t *operation, - cib__op_fn_t op_function, xmlNode **reply, bool privileged) + cib__op_fn_t op_function, xmlNode **reply) { static mainloop_timer_t *digest_timer = NULL; @@ -547,13 +547,6 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation, /* Start processing the request... */ pcmk__xe_get_flags(request, PCMK__XA_CIB_CALLOPT, &call_options, cib_none); - if (!privileged - && pcmk__is_set(operation->flags, cib__op_attr_privileged)) { - - rc = EACCES; - goto done; - } - if (!pcmk__is_set(operation->flags, cib__op_attr_modifies)) { rc = cib__perform_op_ro(op_function, request, &the_cib, &output); goto done; @@ -835,8 +828,18 @@ based_process_request(xmlNode *request, bool privileged, } else if (process) { time_t start_time = time(NULL); - rc = cib_process_command(request, operation, op_function, &reply, - privileged); + if (!privileged + && pcmk__is_set(operation->flags, cib__op_attr_privileged)) { + + rc = EACCES; + if (!pcmk__is_set(call_options, cib_discard_reply)) { + reply = create_cib_reply(request, rc, NULL); + } + + } else { + rc = cib_process_command(request, operation, op_function, &reply); + } + log_op_result(request, operation, rc, difftime(time(NULL), start_time)); if ((reply == NULL) && (needs_reply || local_notify)) { From f75af13033b94f555fd14335697eff43b841086d Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 4 Jan 2026 14:43:52 -0800 Subject: [PATCH 43/47] Refactor: based: Drop NULL-check of reply from cib_process_command() If needs_reply or local_notify is true, then cib_discard_reply is not set. (If cib_discard_reply is set, then we set needs_reply and local_notify to false earlier in based_process_request().) If cib_discard_reply is not set, then cib_process_command() always creates a non-NULL reply (in the done section). Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 8d23f8894ae..e6042f13bf9 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -841,12 +841,6 @@ based_process_request(xmlNode *request, bool privileged, } log_op_result(request, operation, rc, difftime(time(NULL), start_time)); - - if ((reply == NULL) && (needs_reply || local_notify)) { - pcmk__err("Unexpected NULL reply to message"); - pcmk__log_xml_err(request, "null reply"); - goto done; - } } if (pcmk__is_set(operation->flags, cib__op_attr_modifies)) { From 6ef967aef9a276d4526faf5babcf2747cc8dff45 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 4 Jan 2026 14:47:17 -0800 Subject: [PATCH 44/47] Refactor: based: Drop based_process_request last cib_discard_reply check It's redundant. If needs_reply is true, then cib_discard_reply is not set. (If cib_discard_reply is set, then we set needs_reply and local_notify to false earlier in based_process_request().) Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index e6042f13bf9..293d7b16764 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -848,8 +848,7 @@ based_process_request(xmlNode *request, bool privileged, pcmk__s(originator, "local"), client_name, call_id, (local_notify? " with local notification" : "")); - } else if (needs_reply && !stand_alone && (client == NULL) - && !pcmk__is_set(call_options, cib_discard_reply)) { + } else if (needs_reply && !stand_alone && (client == NULL)) { send_peer_reply(reply, originator); } From beb78cc45384ecb425160cde657f9bfa2404790d Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 4 Jan 2026 15:18:11 -0800 Subject: [PATCH 45/47] Refactor: libcib: Use done label in cib__perform_op_ro() Signed-off-by: Reid Wahl --- lib/cib/cib_utils.c | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/lib/cib/cib_utils.c b/lib/cib/cib_utils.c index 10c852a9f4c..d473ffb87e1 100644 --- a/lib/cib/cib_utils.c +++ b/lib/cib/cib_utils.c @@ -268,26 +268,34 @@ cib__perform_op_ro(cib__op_fn_t fn, xmlNode *req, xmlNode **current_cib, && (cib == xmlDocGetRootElement(cib->doc))); } - if (*output == NULL) { - // Do nothing + if (cib_filtered == *output) { + // Let the caller have this copy + return rc; + } - } else if (cib_filtered == *output) { - // Let them have this copy - cib_filtered = NULL; + if (*output == NULL) { + goto done; + } - } else if (*output == *current_cib) { - // They already know not to free it + if (*output == *current_cib) { + // Trust the caller to check this and not free *output + goto done; + } - } else if ((cib_filtered != NULL) - && ((*output)->doc == cib_filtered->doc)) { - // We're about to free the document of which *output is a part + if ((*output)->doc == (*current_cib)->doc) { + // Give the caller a copy that it can free *output = pcmk__xml_copy(NULL, *output); + goto done; + } - } else if ((*output)->doc == (*current_cib)->doc) { - // Give them a copy they can free - *output = pcmk__xml_copy(NULL, *output); + if ((cib_filtered == NULL) || ((*output)->doc != cib_filtered->doc)) { + goto done; } + // We're about to free the document of which *output is a part + *output = pcmk__xml_copy(NULL, *output); + +done: pcmk__xml_free(cib_filtered); return rc; } From 90b599f247deab2c930f5ae20b3ce7d3d2d0514d Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 4 Jan 2026 15:53:03 -0800 Subject: [PATCH 46/47] Low: libcib: Avoid memory leak in processing CIB file commit transaction process_transaction_requests() wasn't freeing output. Do that now. Also simplify the freeing of output. * process_request(): Make a copy of *output if it's not in the same doc as private->cib_xml. This lets us simplify file_perform_op_delegate(). * process_request(): No operation leaves result_cib set to the same value as *output unless it's also equal to private->cib_xml. (A query op can do this.) So drop a check in the done section. * file_perform_op_delegate(): Return only at the end of the function, within the done section. * file_perform_op_delegate(): Don't assign *output_data = NULL at the beginning. We'll do this in the done section if output is NULL. * file_perform_op_delegate(): Assume that output is not in the same document as private->cib_xml. We ensured this in process_request(). * file_perform_op_delegate(): Free output if and only if it has not been assigned to *output_data. And do the standard-to-legacy conversion at the end of file_perform_op_delegate(), in the done section. Signed-off-by: Reid Wahl --- lib/cib/cib_file.c | 59 +++++++++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/lib/cib/cib_file.c b/lib/cib/cib_file.c index 7136a81fdfe..898ec994a92 100644 --- a/lib/cib/cib_file.c +++ b/lib/cib/cib_file.c @@ -191,8 +191,16 @@ process_request(cib_t *cib, xmlNode *request, xmlNode **output) set_file_flags(private, file_flag_dirty); } + if (*output == NULL) { + goto done; + } + + if ((*output)->doc == private->cib_xml->doc) { + *output = pcmk__xml_copy(NULL, *output); + } + done: - if ((result_cib != private->cib_xml) && (result_cib != *output)) { + if (result_cib != private->cib_xml) { pcmk__xml_free(result_cib); } pcmk__xml_free(cib_diff); @@ -226,6 +234,8 @@ process_transaction_requests(cib_t *cib, xmlNode *transaction) int rc = process_request(cib, request, &output); + pcmk__xml_free(output); + if (rc != pcmk_rc_ok) { pcmk__err("Aborting transaction for CIB file client (%s) on file " "'%s' due to failed %s request: %s", @@ -403,38 +413,35 @@ file_perform_op_delegate(cib_t *cib, const char *op, const char *host, const cib__operation_t *operation = NULL; - pcmk__info("Handling %s operation for %s as %s", - pcmk__s(op, "invalid"), pcmk__s(section, "entire CIB"), + pcmk__info("Handling %s operation for %s as %s", pcmk__s(op, "invalid"), + pcmk__s(section, "entire CIB"), pcmk__s(user_name, "default user")); - if (output_data != NULL) { - *output_data = NULL; - } - if (cib->state == cib_disconnected) { - return -ENOTCONN; + rc = ENOTCONN; + goto done; } rc = cib__get_operation(op, &operation); - rc = pcmk_rc2legacy(rc); - if (rc != pcmk_ok) { + if (rc != pcmk_rc_ok) { // @COMPAT: At compatibility break, use rc directly - return -EPROTONOSUPPORT; + rc = EPROTONOSUPPORT; + goto done; } if (get_op_function(operation) == NULL) { // @COMPAT: At compatibility break, use EOPNOTSUPP pcmk__err("Operation %s is not supported by CIB file clients", op); - return -EPROTONOSUPPORT; + rc = EPROTONOSUPPORT; + goto done; } cib__set_call_options(call_options, "file operation", cib_no_mtime); rc = cib__create_op(cib, op, host, section, data, call_options, user_name, NULL, &request); - rc = pcmk_rc2legacy(rc); - if (rc != pcmk_ok) { - return rc; + if (rc != pcmk_rc_ok) { + goto done; } pcmk__xe_set(request, PCMK__XA_ACL_TARGET, user_name); @@ -442,30 +449,22 @@ file_perform_op_delegate(cib_t *cib, const char *op, const char *host, if (pcmk__is_set(call_options, cib_transaction)) { rc = cib__extend_transaction(cib, request); - rc = pcmk_rc2legacy(rc); goto done; } rc = process_request(cib, request, &output); - rc = pcmk_rc2legacy(rc); - - if ((output_data != NULL) && (output != NULL)) { - if (output->doc == private->cib_xml->doc) { - *output_data = pcmk__xml_copy(NULL, output); - } else { - *output_data = output; - } - } done: - if ((output != NULL) - && (output->doc != private->cib_xml->doc) - && ((output_data == NULL) || (output != *output_data))) { + pcmk__xml_free(request); + if (output_data != NULL) { + *output_data = output; + + } else { pcmk__xml_free(output); } - pcmk__xml_free(request); - return rc; + + return pcmk_rc2legacy(rc); } /*! From 1e7de54fd8bcf2cf915bacabfc8d1e01eab16381 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 2 May 2026 10:35:36 -0700 Subject: [PATCH 47/47] Refactor: libcib: Assert to satisfy Coverity Coverity is throwing a CHECKED_RETURN error here, because we check the return value of cib__get_operation() 4 out of 5 times. However, it was already checked in file_perform_op_delegate(). Signed-off-by: Reid Wahl --- lib/cib/cib_file.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cib/cib_file.c b/lib/cib/cib_file.c index 898ec994a92..773c0a5af48 100644 --- a/lib/cib/cib_file.c +++ b/lib/cib/cib_file.c @@ -151,8 +151,8 @@ process_request(cib_t *cib, xmlNode *request, xmlNode **output) file_opaque_t *private = cib->variant_opaque; - // We error checked these in callers - cib__get_operation(op, &operation); + // We error checked these in callers, but make Coverity happy + pcmk__assert(cib__get_operation(op, &operation) == pcmk_rc_ok); op_function = get_op_function(operation); rc = pcmk__xe_get_flags(request, PCMK__XA_CIB_CALLOPT, &call_options,