From 88d4852e79b94d03a6150617e03e2fd664788f68 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 14:43:51 -0700 Subject: [PATCH 01/75] Doc: build: Explain use of -std=c99 with cov-build Signed-off-by: Reid Wahl --- devel/Makefile.am | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/devel/Makefile.am b/devel/Makefile.am index bf80d371569..7dabdf66480 100644 --- a/devel/Makefile.am +++ b/devel/Makefile.am @@ -46,6 +46,14 @@ COVHTML = $(COVERITY_DIR)/output/errors # Coverity outputs are phony so they get rebuilt every invocation +# Coverity (as of version 2026.3) doesn't work correctly with C23. In +# particular, it doesn't recognize the predefined identifiers bool, true, and +# false, and it doesn't handle va_start() correctly. Here we tie compilation to +# C99 as a workaround. Case 03702771 was opened with Black Duck (Coverity +# vendor) to address this. That case is closed, and their engineering team is +# investigating further under issue STGCOV-8389. +# +# @COMPAT Improvements are expected in Coverity 2026.9. .PHONY: $(COVERITY_DIR) $(COVERITY_DIR): coverity-clean $(MAKE) $(AM_MAKEFLAGS) -C $(top_builddir) init core-clean From 5dae8d3cf68f0f502dbf66a17e6f24ebd273eadf Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Mon, 4 May 2026 17:20:59 -0700 Subject: [PATCH 02/75] Refactor: libcrmcommon, libpe_status: Drop an unused Coverity annotation annotations-warnings.txt in the coverity-TAG/output directory says the return overflow annotation is unused. Also flag the replica->child error as a false positive, rather than simply suppressing it. bundle_data->child->priv->children is a list of non-NULL items, and we've already dereferenced replica->child before we hit the line that Coverity complains about. Signed-off-by: Reid Wahl --- lib/common/ipc_client.c | 1 - lib/pengine/bundle.c | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/common/ipc_client.c b/lib/common/ipc_client.c index b3c3309351a..91f5d7968ec 100644 --- a/lib/common/ipc_client.c +++ b/lib/common/ipc_client.c @@ -1526,7 +1526,6 @@ crm_ipc_send(crm_ipc_t *client, const xmlNode *message, g_string_free(iov_buffer, TRUE); pcmk_free_ipc_event(iov); - // coverity[return_overflow] return rc; } diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index 7139b568b35..d6c3f005c2b 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1259,7 +1259,8 @@ pe__unpack_bundle(pcmk_resource_t *rsc) allocate_ip(bundle_data, replica, buffer); bundle_data->replicas = g_list_append(bundle_data->replicas, replica); - // coverity[null_field] replica->child can't be NULL here + + // coverity[null_field : FALSE] replica->child can't be NULL here bundle_data->attribute_target = g_hash_table_lookup(replica->child->priv->meta, PCMK_META_CONTAINER_ATTRIBUTE_TARGET); From 899ce242ca9dfe84d6114824f0088395bb51a348 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Mon, 4 May 2026 23:26:23 -0700 Subject: [PATCH 03/75] Refactor: libcrmcommon: Simplify crm_ipc_close() Signed-off-by: Reid Wahl --- lib/common/ipc_client.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/common/ipc_client.c b/lib/common/ipc_client.c index 91f5d7968ec..7cd8307b665 100644 --- a/lib/common/ipc_client.c +++ b/lib/common/ipc_client.c @@ -956,16 +956,13 @@ pcmk__connect_generic_ipc(crm_ipc_t *ipc) } void -crm_ipc_close(crm_ipc_t * client) +crm_ipc_close(crm_ipc_t *client) { - if (client) { - if (client->ipc) { - qb_ipcc_connection_t *ipc = client->ipc; - - client->ipc = NULL; - qb_ipcc_disconnect(ipc); - } + if (client == NULL) { + return; } + + g_clear_pointer(&client->ipc, qb_ipcc_disconnect); } void From 49bf3f6b040d13b87d73e7bc6f43ded1ed0b356c Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Mon, 4 May 2026 23:29:30 -0700 Subject: [PATCH 04/75] Refactor: libcrmcommon: Simplify crm_ipc_destroy() qb_ipcc_is_connected() returns QB_FALSE for a NULL argument. pcmk__ipc_free_client_buffer() does nothing when its argument's buffer field is NULL. Also remove a layer of nesting. Signed-off-by: Reid Wahl --- lib/common/ipc_client.c | 47 ++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/lib/common/ipc_client.c b/lib/common/ipc_client.c index 7cd8307b665..78c3800857a 100644 --- a/lib/common/ipc_client.c +++ b/lib/common/ipc_client.c @@ -966,33 +966,32 @@ crm_ipc_close(crm_ipc_t *client) } void -crm_ipc_destroy(crm_ipc_t * client) +crm_ipc_destroy(crm_ipc_t *client) { - if (client) { - if (client->ipc && qb_ipcc_is_connected(client->ipc)) { - pcmk__notice("Destroying active %s IPC connection", - client->server_name); - /* The next line is basically unsafe - * - * If this connection was attached to mainloop and mainloop is active, - * the 'disconnected' callback will end up back here and we'll end - * up free'ing the memory twice - something that can still happen - * even without this if we destroy a connection and it closes before - * we call exit - */ - /* crm_ipc_close(client); */ - } else { - pcmk__trace("Destroying inactive %s IPC connection", - client->server_name); - } - - if (client->buffer != NULL) { - pcmk__ipc_free_client_buffer(client); - } + if (client == NULL) { + return; + } - free(client->server_name); - free(client); + if (qb_ipcc_is_connected(client->ipc)) { + pcmk__notice("Destroying active %s IPC connection", + client->server_name); + /* The next line is basically unsafe + * + * If this connection was attached to mainloop and mainloop is active, + * the 'disconnected' callback will end up back here and we'll end + * up free'ing the memory twice - something that can still happen + * even without this if we destroy a connection and it closes before + * we call exit + */ + /* crm_ipc_close(client); */ + } else { + pcmk__trace("Destroying inactive %s IPC connection", + client->server_name); } + + pcmk__ipc_free_client_buffer(client); + free(client->server_name); + free(client); } /*! From e98b172c1a5b56c05ecbdaa07b62ab7d8b88162b Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Mon, 4 May 2026 23:50:43 -0700 Subject: [PATCH 05/75] Refactor: libcrmcommon: Simplify mainloop_gio_destroy() Commit 4facb964 introduced much of this complexity. The commit message says "Make mainloop_gio_destroy() tollerant of being called re-entrantly". However, there seems to be no way this could happen. Pacemaker is single-threaded, and the destroy_fn can't return control back to the main loop. Control returns to the main loop when mainloop_gio_destroy() returns. Signed-off-by: Reid Wahl --- lib/common/mainloop.c | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/lib/common/mainloop.c b/lib/common/mainloop.c index 530175cfe16..b0655fa3b6a 100644 --- a/lib/common/mainloop.c +++ b/lib/common/mainloop.c @@ -799,37 +799,25 @@ static void mainloop_gio_destroy(void *c) { mainloop_io_t *client = c; - char *c_name = strdup(client->name); - /* client->source is valid but about to be destroyed (ref_count == 0) in gmain.c - * client->channel will still have ref_count > 0... should be == 1 + /* client->source is valid but about to be destroyed (ref_count == 0) in + * gmain.c. client->channel will still have ref_count > 0 (should be == 1). */ - pcmk__trace("Destroying client %s[%p]", c_name, c); + pcmk__trace("Destroying client %s[%p]", client->name, client); - if (client->ipc) { - crm_ipc_close(client->ipc); - } - - if (client->destroy_fn) { - void (*destroy_fn)(void *userdata) = client->destroy_fn; + // @TODO Would it be safe to call this immediately before crm_ipc_destroy()? + crm_ipc_close(client->ipc); - client->destroy_fn = NULL; - destroy_fn(client->userdata); + if (client->destroy_fn != NULL) { + client->destroy_fn(client->userdata); } - if (client->ipc) { - crm_ipc_t *ipc = client->ipc; - - client->ipc = NULL; - crm_ipc_destroy(ipc); - } + crm_ipc_destroy(client->ipc); - pcmk__trace("Destroyed client %s[%p]", c_name, c); + pcmk__trace("Destroyed client %s[%p]", client->name, client); free(client->name); free(client); - - free(c_name); } /*! From edb7b33d324ead34c27a54c945c56e42647ae503 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Mon, 4 May 2026 23:57:19 -0700 Subject: [PATCH 06/75] Refactor: libcrmcommon: Simplify freeing of client->channel Signed-off-by: Reid Wahl --- lib/common/mainloop.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/lib/common/mainloop.c b/lib/common/mainloop.c index b0655fa3b6a..7515f9f057f 100644 --- a/lib/common/mainloop.c +++ b/lib/common/mainloop.c @@ -816,6 +816,13 @@ mainloop_gio_destroy(void *c) pcmk__trace("Destroyed client %s[%p]", client->name, client); + /* This is the destructor corresponding to mainloop_add_fd(). There, + * g_io_channel_unix_new() created client->channel and added a reference to + * it, so we drop a reference here. The channel will be freed when the + * reference count drops to zero. + */ + g_io_channel_unref(client->channel); + free(client->name); free(client); } @@ -960,17 +967,6 @@ mainloop_add_fd(const char *name, int priority, int fd, void *userdata, 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 75ea4b40cc261857bc420fa5f57ddc4921377704 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 02:21:24 -0700 Subject: [PATCH 07/75] Refactor: liblrmd: Make/rename some variables in lrmd_dispatch_internal Signed-off-by: Reid Wahl --- lib/lrmd/lrmd_client.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/lib/lrmd/lrmd_client.c b/lib/lrmd/lrmd_client.c index ecaac78903a..553d0f116ec 100644 --- a/lib/lrmd/lrmd_client.c +++ b/lib/lrmd/lrmd_client.c @@ -279,7 +279,9 @@ lrmd_dispatch_internal(void *data, void *user_data) xmlNode *msg = data; lrmd_t *lrmd = user_data; - const char *type; + const char *op = pcmk__xe_get(msg, PCMK__XA_LRMD_OP); + const char *rsc_id = pcmk__xe_get(msg, PCMK__XA_LRMD_RSC_ID); + const char *rsc_action = pcmk__xe_get(msg, PCMK__XA_LRMD_RSC_ACTION); const char *proxy_session = pcmk__xe_get(msg, PCMK__XA_LRMD_IPC_SESSION); lrmd_private_t *native = lrmd->lrmd_private; lrmd_event_data_t event = { 0, }; @@ -300,15 +302,16 @@ lrmd_dispatch_internal(void *data, void *user_data) } event.remote_nodename = native->remote_nodename; - type = pcmk__xe_get(msg, PCMK__XA_LRMD_OP); pcmk__xe_get_int(msg, PCMK__XA_LRMD_CALLID, &event.call_id); - event.rsc_id = pcmk__xe_get(msg, PCMK__XA_LRMD_RSC_ID); + event.rsc_id = rsc_id; - if (pcmk__str_eq(type, LRMD_OP_RSC_REG, pcmk__str_none)) { + if (pcmk__str_eq(op, LRMD_OP_RSC_REG, pcmk__str_none)) { event.type = lrmd_event_register; - } else if (pcmk__str_eq(type, LRMD_OP_RSC_UNREG, pcmk__str_none)) { + + } else if (pcmk__str_eq(op, LRMD_OP_RSC_UNREG, pcmk__str_none)) { event.type = lrmd_event_unregister; - } else if (pcmk__str_eq(type, LRMD_OP_RSC_EXEC, pcmk__str_none)) { + + } else if (pcmk__str_eq(op, LRMD_OP_RSC_EXEC, pcmk__str_none)) { int rc = 0; int exec_time = 0; int queue_time = 0; @@ -334,7 +337,7 @@ lrmd_dispatch_internal(void *data, void *user_data) pcmk__xe_get_int(msg, PCMK__XA_LRMD_QUEUE_TIME, &queue_time); event.queue_time = QB_MAX(0, queue_time); - event.op_type = pcmk__xe_get(msg, PCMK__XA_LRMD_RSC_ACTION); + event.op_type = rsc_action; event.user_data = pcmk__xe_get(msg, PCMK__XA_LRMD_RSC_USERDATA_STR); event.type = lrmd_event_exec_complete; @@ -344,15 +347,18 @@ lrmd_dispatch_internal(void *data, void *user_data) pcmk__xe_get(msg, PCMK__XA_LRMD_RSC_EXIT_REASON)); event.params = xml2list(msg); - } else if (pcmk__str_eq(type, LRMD_OP_NEW_CLIENT, pcmk__str_none)) { + + } else if (pcmk__str_eq(op, LRMD_OP_NEW_CLIENT, pcmk__str_none)) { event.type = lrmd_event_new_client; - } else if (pcmk__str_eq(type, LRMD_OP_POKE, pcmk__str_none)) { + + } else if (pcmk__str_eq(op, LRMD_OP_POKE, pcmk__str_none)) { event.type = lrmd_event_poke; + } else { return; } - pcmk__trace("op %s notify event received", type); + pcmk__trace("op %s notify event received", op); native->callback(&event); g_clear_pointer(&event.params, g_hash_table_destroy); From d0d7941f6735e53af4d4ecb8cc3af922cd91230e Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 02:28:25 -0700 Subject: [PATCH 08/75] Refactor: liblrmd: Unindent in a few more places Signed-off-by: Reid Wahl --- lib/lrmd/lrmd_client.c | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/lib/lrmd/lrmd_client.c b/lib/lrmd/lrmd_client.c index 553d0f116ec..36a6d4826c3 100644 --- a/lib/lrmd/lrmd_client.c +++ b/lib/lrmd/lrmd_client.c @@ -487,14 +487,16 @@ static void report_async_connection_result(lrmd_t *lrmd, int rc) { lrmd_private_t *native = lrmd->lrmd_private; + lrmd_event_data_t event = { 0, }; - if (native->callback) { - lrmd_event_data_t event = { 0, }; - event.type = lrmd_event_connect; - event.remote_nodename = native->remote_nodename; - event.connection_rc = rc; - native->callback(&event); + if (native->callback == NULL) { + return; } + + event.type = lrmd_event_connect; + event.remote_nodename = native->remote_nodename; + event.connection_rc = rc; + native->callback(&event); } static void @@ -645,6 +647,7 @@ lrmd_ipc_connection_destroy(void *userdata) { lrmd_t *lrmd = userdata; lrmd_private_t *native = lrmd->lrmd_private; + lrmd_event_data_t event = { 0, }; switch (native->type) { case pcmk__client_ipc: @@ -663,12 +666,13 @@ lrmd_ipc_connection_destroy(void *userdata) native->ipc = NULL; native->source = NULL; - if (native->callback) { - lrmd_event_data_t event = { 0, }; - event.type = lrmd_event_disconnect; - event.remote_nodename = native->remote_nodename; - native->callback(&event); + if (native->callback == NULL) { + return; } + + event.type = lrmd_event_disconnect; + event.remote_nodename = native->remote_nodename; + native->callback(&event); } /*! @@ -1094,6 +1098,7 @@ lrmd_tls_connection_destroy(void *userdata) { lrmd_t *lrmd = userdata; lrmd_private_t *native = lrmd->lrmd_private; + lrmd_event_data_t event = { 0, }; pcmk__info("TLS connection destroyed"); @@ -1120,12 +1125,13 @@ lrmd_tls_connection_destroy(void *userdata) native->source = 0; native->sock = -1; - if (native->callback) { - lrmd_event_data_t event = { 0, }; - event.remote_nodename = native->remote_nodename; - event.type = lrmd_event_disconnect; - native->callback(&event); + if (native->callback == NULL) { + return; } + + event.type = lrmd_event_connect; + event.remote_nodename = native->remote_nodename; + native->callback(&event); } static void From 7f7d54275573d18de5237cc4cead2dfdac1279a0 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 02:52:22 -0700 Subject: [PATCH 09/75] API: libcrmcommon: did_rsc_op_fail() argument is now const Signed-off-by: Reid Wahl --- include/crm/common/actions.h | 2 +- lib/common/actions.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/crm/common/actions.h b/include/crm/common/actions.h index 092b2ca41fa..b363f01b27f 100644 --- a/include/crm/common/actions.h +++ b/include/crm/common/actions.h @@ -77,7 +77,7 @@ gboolean decode_transition_magic(const char *magic, char **uuid, // @COMPAT Either these shouldn't be in libcrmcommon or lrmd_event_data_t should int rsc_op_expected_rc(const lrmd_event_data_t *event); -gboolean did_rsc_op_fail(lrmd_event_data_t *event, int target_rc); +gboolean did_rsc_op_fail(const lrmd_event_data_t *event, int target_rc); bool crm_op_needs_metadata(const char *rsc_class, const char *op); diff --git a/lib/common/actions.c b/lib/common/actions.c index 7a02ad29fed..14cdc8a02d5 100644 --- a/lib/common/actions.c +++ b/lib/common/actions.c @@ -510,9 +510,9 @@ rsc_op_expected_rc(const lrmd_event_data_t *op) } gboolean -did_rsc_op_fail(lrmd_event_data_t * op, int target_rc) +did_rsc_op_fail(const lrmd_event_data_t *event, int target_rc) { - switch (op->op_status) { + switch (event->op_status) { case PCMK_EXEC_CANCELLED: case PCMK_EXEC_PENDING: return FALSE; @@ -527,7 +527,7 @@ did_rsc_op_fail(lrmd_event_data_t * op, int target_rc) return TRUE; default: - if (target_rc != op->rc) { + if (target_rc != event->rc) { return TRUE; } } From 48cd8ad6a00251802e4c04ae8a11fbe25daa9fec Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 09:54:53 -0700 Subject: [PATCH 10/75] API: liblrmd: lrmd_copy_event() argument is now const Signed-off-by: Reid Wahl --- include/crm/lrmd_events.h | 2 +- lib/lrmd/lrmd_client.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/crm/lrmd_events.h b/include/crm/lrmd_events.h index 3a9e019f35a..24f98d8cf17 100644 --- a/include/crm/lrmd_events.h +++ b/include/crm/lrmd_events.h @@ -103,7 +103,7 @@ typedef struct lrmd_event_data_s { lrmd_event_data_t *lrmd_new_event(const char *rsc_id, const char *task, unsigned int interval_ms); -lrmd_event_data_t *lrmd_copy_event(lrmd_event_data_t *event); +lrmd_event_data_t *lrmd_copy_event(const lrmd_event_data_t *event); void lrmd_free_event(lrmd_event_data_t *event); #ifdef __cplusplus diff --git a/lib/lrmd/lrmd_client.c b/lib/lrmd/lrmd_client.c index 36a6d4826c3..06e301031f3 100644 --- a/lib/lrmd/lrmd_client.c +++ b/lib/lrmd/lrmd_client.c @@ -187,7 +187,7 @@ lrmd_new_event(const char *rsc_id, const char *task, unsigned int interval_ms) } lrmd_event_data_t * -lrmd_copy_event(lrmd_event_data_t * event) +lrmd_copy_event(const lrmd_event_data_t *event) { lrmd_event_data_t *copy = NULL; From 388387f8ca327b170a7137964a502be99869637f Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 10:01:59 -0700 Subject: [PATCH 11/75] Refactor: various: const lrmd_event_data_t in internal functions ...where appropriate. Signed-off-by: Reid Wahl --- daemons/controld/controld_alerts.c | 2 +- daemons/controld/controld_alerts.h | 4 ++-- daemons/controld/controld_cib.c | 6 +++--- daemons/controld/controld_execd.c | 5 +++-- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/daemons/controld/controld_alerts.c b/daemons/controld/controld_alerts.c index a3dd5b6d5f2..7833d68e930 100644 --- a/daemons/controld/controld_alerts.c +++ b/daemons/controld/controld_alerts.c @@ -71,7 +71,7 @@ crmd_alert_fencing_op(stonith_event_t * e) } void -crmd_alert_resource_op(const char *node, lrmd_event_data_t * op) +crmd_alert_resource_op(const char *node, const lrmd_event_data_t *op) { lrm_state_t *lrm_state; diff --git a/daemons/controld/controld_alerts.h b/daemons/controld/controld_alerts.h index 441c8fefdff..9e822771b13 100644 --- a/daemons/controld/controld_alerts.h +++ b/daemons/controld/controld_alerts.h @@ -1,5 +1,5 @@ /* - * Copyright 2015-2024 the Pacemaker project contributors + * Copyright 2015-2026 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -19,6 +19,6 @@ void crmd_unpack_alerts(xmlNode *alerts); void crmd_alert_node_event(pcmk__node_status_t *node); void crmd_alert_fencing_op(stonith_event_t *e); -void crmd_alert_resource_op(const char *node, lrmd_event_data_t *op); +void crmd_alert_resource_op(const char *node, const lrmd_event_data_t *op); #endif diff --git a/daemons/controld/controld_cib.c b/daemons/controld/controld_cib.c index f88fc0492d1..8b268db6ad2 100644 --- a/daemons/controld/controld_cib.c +++ b/daemons/controld/controld_cib.c @@ -518,7 +518,7 @@ build_parameter_list(const lrmd_event_data_t *op, } static void -append_restart_list(lrmd_event_data_t *op, struct ra_metadata_s *metadata, +append_restart_list(const lrmd_event_data_t *op, struct ra_metadata_s *metadata, xmlNode *update, const char *version) { GString *list = NULL; @@ -576,7 +576,7 @@ append_restart_list(lrmd_event_data_t *op, struct ra_metadata_s *metadata, } static void -append_secure_list(lrmd_event_data_t *op, struct ra_metadata_s *metadata, +append_secure_list(const lrmd_event_data_t *op, struct ra_metadata_s *metadata, xmlNode *update, const char *version) { GString *list = NULL; @@ -753,7 +753,7 @@ cib_rsc_callback(xmlNode * msg, int call_id, int rc, xmlNode * output, void *use * until it is active there again after the node comes back up. */ static bool -should_preserve_lock(lrmd_event_data_t *op) +should_preserve_lock(const lrmd_event_data_t *op) { if (!pcmk__is_set(controld_globals.flags, controld_shutdown_lock_enabled)) { return false; diff --git a/daemons/controld/controld_execd.c b/daemons/controld/controld_execd.c index 4c33460cac0..4df59c77479 100644 --- a/daemons/controld/controld_execd.c +++ b/daemons/controld/controld_execd.c @@ -152,7 +152,8 @@ history_free(void *data) } static void -update_history_cache(lrm_state_t * lrm_state, lrmd_rsc_info_t * rsc, lrmd_event_data_t * op) +update_history_cache(lrm_state_t *lrm_state, lrmd_rsc_info_t *rsc, + const lrmd_event_data_t *op) { int target_rc = 0; rsc_history_t *entry = NULL; @@ -269,7 +270,7 @@ send_task_ok_ack(const lrm_state_t *lrm_state, const ha_msg_input_t *input, } static inline const char * -op_node_name(lrmd_event_data_t *op) +op_node_name(const lrmd_event_data_t *op) { return pcmk__s(op->remote_nodename, controld_globals.cluster->priv->node_name); From 6e22cf8f1a224e8b335cba2257528c5ee2282260 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 02:01:48 -0700 Subject: [PATCH 12/75] API: liblrmd: lrmd_event_data_t char * members are now non-const lrmd_new_event() allocates memory for rsc_id and op_type. lrmd_copy_event() allocates memory for rsc_id, op_type, user_data, output, remote_nodename, and exit_reason. lrmd_dispatch_internal() allocates memory for output and exit_reason, but prior to this commit it used strings belonging to other objects for rsc_id, op_type, user_data, and remote_nodename. lrmd_free_event() frees all six fields. Signed-off-by: Reid Wahl --- daemons/controld/controld_execd.c | 6 +- daemons/controld/controld_execd_state.c | 27 +++--- daemons/controld/controld_remote_ra.c | 57 ++++++------- include/crm/lrmd_events.h | 12 +-- lib/lrmd/lrmd_client.c | 109 +++++++++++++----------- 5 files changed, 105 insertions(+), 106 deletions(-) diff --git a/daemons/controld/controld_execd.c b/daemons/controld/controld_execd.c index 4df59c77479..42c5a490e71 100644 --- a/daemons/controld/controld_execd.c +++ b/daemons/controld/controld_execd.c @@ -1729,11 +1729,13 @@ controld_ack_event_directly(const char *to_host, const char *to_sys, pcmk__node_status_t *peer = NULL; CRM_CHECK(op != NULL, return); + if (op->rsc_id == NULL) { - // op->rsc_id is a (const char *) but lrmd_free_event() frees it - pcmk__assert(rsc_id != NULL); op->rsc_id = pcmk__str_copy(rsc_id); } + + pcmk__assert(op->rsc_id != NULL); + if (to_sys == NULL) { to_sys = CRM_SYSTEM_TENGINE; } diff --git a/daemons/controld/controld_execd_state.c b/daemons/controld/controld_execd_state.c index 11ac96f77d9..77a5620fd1b 100644 --- a/daemons/controld/controld_execd_state.c +++ b/daemons/controld/controld_execd_state.c @@ -55,31 +55,28 @@ free_recurring_op(void *value) static gboolean fail_pending_op(void *key, void *value, void *user_data) { - lrmd_event_data_t event = { 0, }; lrm_state_t *lrm_state = user_data; active_op_t *op = value; + lrmd_event_data_t *event = lrmd_new_event(op->rsc_id, op->op_type, + op->interval_ms); pcmk__trace("Pre-emptively failing " PCMK__OP_FMT " on %s (call=%s, %s)", op->rsc_id, op->op_type, op->interval_ms, lrm_state->node_name, (const char *) key, op->user_data); - event.type = lrmd_event_exec_complete; - event.rsc_id = op->rsc_id; - event.op_type = op->op_type; - event.user_data = op->user_data; - event.timeout = 0; - event.interval_ms = op->interval_ms; - lrmd__set_result(&event, PCMK_OCF_UNKNOWN_ERROR, PCMK_EXEC_NOT_CONNECTED, + event->type = lrmd_event_exec_complete; + event->user_data = pcmk__str_copy(op->user_data); + lrmd__set_result(event, PCMK_OCF_UNKNOWN_ERROR, PCMK_EXEC_NOT_CONNECTED, "Action was pending when executor connection was dropped"); - event.t_run = op->start_time; - event.t_rcchange = op->start_time; + event->t_run = op->start_time; + event->t_rcchange = op->start_time; - event.call_id = op->call_id; - event.remote_nodename = lrm_state->node_name; - event.params = op->params; + event->call_id = op->call_id; + event->remote_nodename = pcmk__str_copy(lrm_state->node_name); + event->params = op->params; - process_lrm_event(lrm_state, &event, op, NULL); - lrmd__reset_result(&event); + process_lrm_event(lrm_state, event, op, NULL); + lrmd_free_event(event); return TRUE; } diff --git a/daemons/controld/controld_remote_ra.c b/daemons/controld/controld_remote_ra.c index 23a5512d3df..37c975bf2b9 100644 --- a/daemons/controld/controld_remote_ra.c +++ b/daemons/controld/controld_remote_ra.c @@ -399,26 +399,24 @@ check_remote_node_state(const remote_ra_cmd_t *cmd) static void report_remote_ra_result(remote_ra_cmd_t * cmd) { - lrmd_event_data_t op = { 0, }; + lrmd_event_data_t *event = lrmd_new_event(cmd->rsc_id, cmd->action, + cmd->interval_ms); check_remote_node_state(cmd); - op.type = lrmd_event_exec_complete; - op.rsc_id = cmd->rsc_id; - op.op_type = cmd->action; - op.user_data = cmd->userdata; - op.timeout = cmd->timeout; - op.interval_ms = cmd->interval_ms; - op.t_run = cmd->start_time; - op.t_rcchange = cmd->start_time; + event->type = lrmd_event_exec_complete; + event->user_data = pcmk__str_copy(cmd->userdata); + event->timeout = cmd->timeout; + event->t_run = cmd->start_time; + event->t_rcchange = cmd->start_time; - lrmd__set_result(&op, cmd->result.exit_status, cmd->result.execution_status, - cmd->result.exit_reason); + lrmd__set_result(event, cmd->result.exit_status, + cmd->result.execution_status, cmd->result.exit_reason); if (pcmk__is_set(cmd->status, cmd_reported_success) && !pcmk__result_ok(&(cmd->result))) { - op.t_rcchange = time(NULL); + event->t_rcchange = time(NULL); /* This edge case will likely never ever occur, but if it does the * result is that a failure will not be processed correctly. This is only * remotely possible because we are able to detect a connection resource's tcp @@ -428,27 +426,25 @@ report_remote_ra_result(remote_ra_cmd_t * cmd) * basically, we are not guaranteed that the first successful monitor op and * a subsequent failed monitor op will not occur in the same timestamp. We have to * make it look like the operations occurred at separate times though. */ - if (op.t_rcchange == op.t_run) { - op.t_rcchange++; + if (event->t_rcchange == event->t_run) { + event->t_rcchange++; } } if (cmd->params) { lrmd_key_value_t *tmp; - op.params = pcmk__strkey_table(free, free); + event->params = pcmk__strkey_table(free, free); for (tmp = cmd->params; tmp; tmp = tmp->next) { - pcmk__insert_dup(op.params, tmp->key, tmp->value); + pcmk__insert_dup(event->params, tmp->key, tmp->value); } } - op.call_id = cmd->call_id; - op.remote_nodename = cmd->owner; + event->call_id = cmd->call_id; + event->remote_nodename = pcmk__str_copy(cmd->owner); - lrm_op_callback(&op); - - g_clear_pointer(&op.params, g_hash_table_destroy); - lrmd__reset_result(&op); + lrm_op_callback(event); + lrmd_free_event(event); } /*! @@ -562,7 +558,7 @@ monitor_timeout_cb(void *data) static void synthesize_lrmd_success(lrm_state_t *lrm_state, const char *rsc_id, const char *op_type) { - lrmd_event_data_t op = { 0, }; + lrmd_event_data_t *event = NULL; if (lrm_state == NULL) { /* if lrm_state not given assume local */ @@ -570,14 +566,13 @@ synthesize_lrmd_success(lrm_state_t *lrm_state, const char *rsc_id, const char * } pcmk__assert(lrm_state != NULL); - op.type = lrmd_event_exec_complete; - op.rsc_id = rsc_id; - op.op_type = op_type; - op.t_run = time(NULL); - op.t_rcchange = op.t_run; - op.call_id = generate_callid(); - lrmd__set_result(&op, PCMK_OCF_OK, PCMK_EXEC_DONE, NULL); - process_lrm_event(lrm_state, &op, NULL, NULL); + event = lrmd_new_event(rsc_id, op_type, 0); + event->type = lrmd_event_exec_complete; + event->t_run = time(NULL); + event->t_rcchange = event->t_run; + event->call_id = generate_callid(); + lrmd__set_result(event, PCMK_OCF_OK, PCMK_EXEC_DONE, NULL); + process_lrm_event(lrm_state, event, NULL, NULL); } void diff --git a/include/crm/lrmd_events.h b/include/crm/lrmd_events.h index 24f98d8cf17..73fd3d6f4bc 100644 --- a/include/crm/lrmd_events.h +++ b/include/crm/lrmd_events.h @@ -43,11 +43,11 @@ typedef struct lrmd_event_data_s { enum lrmd_callback_event type; /*! The resource this event occurred on. */ - const char *rsc_id; + char *rsc_id; /*! The action performed, start, stop, monitor... */ - const char *op_type; + char *op_type; /*! The user data passed by caller of exec() API function */ - const char *user_data; + char *user_data; /*! The client api call id associated with this event */ int call_id; @@ -71,7 +71,7 @@ typedef struct lrmd_event_data_s { int op_status; /*! stdout from resource agent operation */ - const char *output; + char *output; /*! Timestamp of when op ran */ time_t t_run; @@ -95,10 +95,10 @@ typedef struct lrmd_event_data_s { /*! client node name associated with this connection * (used to match actions to the proper client when there are multiple) */ - const char *remote_nodename; + char *remote_nodename; /*! exit failure reason string from resource agent operation */ - const char *exit_reason; + char *exit_reason; } lrmd_event_data_t; lrmd_event_data_t *lrmd_new_event(const char *rsc_id, const char *task, diff --git a/lib/lrmd/lrmd_client.c b/lib/lrmd/lrmd_client.c index 06e301031f3..ee401a60eb7 100644 --- a/lib/lrmd/lrmd_client.c +++ b/lib/lrmd/lrmd_client.c @@ -179,7 +179,6 @@ lrmd_new_event(const char *rsc_id, const char *task, unsigned int interval_ms) { lrmd_event_data_t *event = pcmk__assert_alloc(1, sizeof(lrmd_event_data_t)); - // lrmd_event_data_t has (const char *) members that lrmd_free_event() frees event->rsc_id = pcmk__str_copy(rsc_id); event->op_type = pcmk__str_copy(task); event->interval_ms = interval_ms; @@ -195,7 +194,6 @@ lrmd_copy_event(const lrmd_event_data_t *event) copy->type = event->type; - // lrmd_event_data_t has (const char *) members that lrmd_free_event() frees copy->rsc_id = pcmk__str_copy(event->rsc_id); copy->op_type = pcmk__str_copy(event->op_type); copy->user_data = pcmk__str_copy(event->user_data); @@ -231,11 +229,11 @@ lrmd_free_event(lrmd_event_data_t *event) if (event == NULL) { return; } - // @TODO Why are these const char *? - free((void *) event->rsc_id); - free((void *) event->op_type); - free((void *) event->user_data); - free((void *) event->remote_nodename); + + free(event->rsc_id); + free(event->op_type); + free(event->user_data); + free(event->remote_nodename); lrmd__reset_result(event); g_clear_pointer(&event->params, g_hash_table_destroy); free(event); @@ -284,7 +282,7 @@ lrmd_dispatch_internal(void *data, void *user_data) const char *rsc_action = pcmk__xe_get(msg, PCMK__XA_LRMD_RSC_ACTION); const char *proxy_session = pcmk__xe_get(msg, PCMK__XA_LRMD_IPC_SESSION); lrmd_private_t *native = lrmd->lrmd_private; - lrmd_event_data_t event = { 0, }; + lrmd_event_data_t *event = NULL; if (proxy_session != NULL) { if (native->proxy_callback == NULL) { @@ -301,68 +299,68 @@ lrmd_dispatch_internal(void *data, void *user_data) return; } - event.remote_nodename = native->remote_nodename; - pcmk__xe_get_int(msg, PCMK__XA_LRMD_CALLID, &event.call_id); - event.rsc_id = rsc_id; + event = lrmd_new_event(rsc_id, rsc_action, 0); + event->remote_nodename = pcmk__str_copy(native->remote_nodename); + pcmk__xe_get_int(msg, PCMK__XA_LRMD_CALLID, &event->call_id); if (pcmk__str_eq(op, LRMD_OP_RSC_REG, pcmk__str_none)) { - event.type = lrmd_event_register; + event->type = lrmd_event_register; } else if (pcmk__str_eq(op, LRMD_OP_RSC_UNREG, pcmk__str_none)) { - event.type = lrmd_event_unregister; + event->type = lrmd_event_unregister; } else if (pcmk__str_eq(op, LRMD_OP_RSC_EXEC, pcmk__str_none)) { int rc = 0; int exec_time = 0; int queue_time = 0; - pcmk__xe_get_int(msg, PCMK__XA_LRMD_TIMEOUT, &event.timeout); - pcmk__xe_get_uint(msg, PCMK__XA_LRMD_RSC_INTERVAL, &event.interval_ms); + event->type = lrmd_event_exec_complete; + pcmk__xe_get_int(msg, PCMK__XA_LRMD_TIMEOUT, &event->timeout); + pcmk__xe_get_uint(msg, PCMK__XA_LRMD_RSC_INTERVAL, &event->interval_ms); pcmk__xe_get_int(msg, PCMK__XA_LRMD_RSC_START_DELAY, - &event.start_delay); + &event->start_delay); pcmk__xe_get_int(msg, PCMK__XA_LRMD_EXEC_RC, &rc); - event.rc = (enum ocf_exitcode) rc; + event->rc = (enum ocf_exitcode) rc; - pcmk__xe_get_int(msg, PCMK__XA_LRMD_EXEC_OP_STATUS, &event.op_status); - pcmk__xe_get_int(msg, PCMK__XA_LRMD_RSC_DELETED, &event.rsc_deleted); + pcmk__xe_get_int(msg, PCMK__XA_LRMD_EXEC_OP_STATUS, &event->op_status); + pcmk__xe_get_int(msg, PCMK__XA_LRMD_RSC_DELETED, &event->rsc_deleted); - pcmk__xe_get_time(msg, PCMK__XA_LRMD_RUN_TIME, &event.t_run); - pcmk__xe_get_time(msg, PCMK__XA_LRMD_RCCHANGE_TIME, &event.t_rcchange); + pcmk__xe_get_time(msg, PCMK__XA_LRMD_RUN_TIME, &event->t_run); + pcmk__xe_get_time(msg, PCMK__XA_LRMD_RCCHANGE_TIME, &event->t_rcchange); pcmk__xe_get_int(msg, PCMK__XA_LRMD_EXEC_TIME, &exec_time); CRM_LOG_ASSERT(exec_time >= 0); - event.exec_time = QB_MAX(0, exec_time); + event->exec_time = QB_MAX(0, exec_time); pcmk__xe_get_int(msg, PCMK__XA_LRMD_QUEUE_TIME, &queue_time); - event.queue_time = QB_MAX(0, queue_time); + event->queue_time = QB_MAX(0, queue_time); - event.op_type = rsc_action; - event.user_data = pcmk__xe_get(msg, PCMK__XA_LRMD_RSC_USERDATA_STR); - event.type = lrmd_event_exec_complete; + event->user_data = pcmk__xe_get_copy(msg, + PCMK__XA_LRMD_RSC_USERDATA_STR); - /* output and exit_reason may be freed by a callback */ - event.output = pcmk__xe_get_copy(msg, PCMK__XA_LRMD_RSC_OUTPUT); - lrmd__set_result(&event, event.rc, event.op_status, + event->output = pcmk__xe_get_copy(msg, PCMK__XA_LRMD_RSC_OUTPUT); + lrmd__set_result(event, event->rc, event->op_status, pcmk__xe_get(msg, PCMK__XA_LRMD_RSC_EXIT_REASON)); - event.params = xml2list(msg); + event->params = xml2list(msg); } else if (pcmk__str_eq(op, LRMD_OP_NEW_CLIENT, pcmk__str_none)) { - event.type = lrmd_event_new_client; + event->type = lrmd_event_new_client; } else if (pcmk__str_eq(op, LRMD_OP_POKE, pcmk__str_none)) { - event.type = lrmd_event_poke; + event->type = lrmd_event_poke; } else { - return; + goto done; } pcmk__trace("op %s notify event received", op); - native->callback(&event); - g_clear_pointer(&event.params, g_hash_table_destroy); - lrmd__reset_result(&event); + native->callback(event); + +done: + lrmd_free_event(event); } // \return Always 0, to indicate that IPC mainloop source should be kept @@ -487,16 +485,19 @@ static void report_async_connection_result(lrmd_t *lrmd, int rc) { lrmd_private_t *native = lrmd->lrmd_private; - lrmd_event_data_t event = { 0, }; + lrmd_event_data_t *event = NULL; if (native->callback == NULL) { return; } - event.type = lrmd_event_connect; - event.remote_nodename = native->remote_nodename; - event.connection_rc = rc; - native->callback(&event); + event = lrmd_new_event(NULL, NULL, 0); + event->type = lrmd_event_connect; + event->connection_rc = rc; + event->remote_nodename = pcmk__str_copy(native->remote_nodename); + + native->callback(event); + lrmd_free_event(event); } static void @@ -647,7 +648,7 @@ lrmd_ipc_connection_destroy(void *userdata) { lrmd_t *lrmd = userdata; lrmd_private_t *native = lrmd->lrmd_private; - lrmd_event_data_t event = { 0, }; + lrmd_event_data_t *event = NULL; switch (native->type) { case pcmk__client_ipc: @@ -670,9 +671,12 @@ lrmd_ipc_connection_destroy(void *userdata) return; } - event.type = lrmd_event_disconnect; - event.remote_nodename = native->remote_nodename; - native->callback(&event); + event = lrmd_new_event(NULL, NULL, 0); + event->type = lrmd_event_disconnect; + event->remote_nodename = pcmk__str_copy(native->remote_nodename); + + native->callback(event); + lrmd_free_event(event); } /*! @@ -1098,7 +1102,7 @@ lrmd_tls_connection_destroy(void *userdata) { lrmd_t *lrmd = userdata; lrmd_private_t *native = lrmd->lrmd_private; - lrmd_event_data_t event = { 0, }; + lrmd_event_data_t *event = NULL; pcmk__info("TLS connection destroyed"); @@ -1129,9 +1133,12 @@ lrmd_tls_connection_destroy(void *userdata) return; } - event.type = lrmd_event_connect; - event.remote_nodename = native->remote_nodename; - native->callback(&event); + event = lrmd_new_event(NULL, NULL, 0); + event->type = lrmd_event_disconnect; + event->remote_nodename = pcmk__str_copy(native->remote_nodename); + + native->callback(event); + lrmd_free_event(event); } static void @@ -2437,9 +2444,7 @@ lrmd__set_result(lrmd_event_data_t *event, enum ocf_exitcode rc, int op_status, event->rc = rc; event->op_status = op_status; - - // lrmd_event_data_t has (const char *) members that lrmd_free_event() frees - pcmk__str_update((char **) &event->exit_reason, exit_reason); + pcmk__str_update(&event->exit_reason, exit_reason); } /*! From 119bfbd840d3ab62b2db309b2dc7bd785d78e8b3 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 10:36:30 -0700 Subject: [PATCH 13/75] Low: cts-lab: Fix corosync-ignore pattern for CPG API connection failed This should have been done in commit e09bc86. Signed-off-by: Reid Wahl --- python/pacemaker/_cts/patterns.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/pacemaker/_cts/patterns.py b/python/pacemaker/_cts/patterns.py index 4b69cfe9a14..963f460f739 100644 --- a/python/pacemaker/_cts/patterns.py +++ b/python/pacemaker/_cts/patterns.py @@ -239,7 +239,7 @@ def __init__(self): self._components["corosync-ignore"] = components_common_ignore + [ r"Could not connect to Corosync CFG: Internal corosync error", - r"error:.*Connection to the CPG API failed: Library error", + r"error:.*Connection to the CPG API failed: Internal corosync error", r"\[[0-9]+\] exited with status [0-9]+ \(", r"\[[0-9]+\] terminated with signal 15", r"pacemaker-based.*error:.*Corosync connection lost", From f7b51bbf27eb50e9ea0d5fbc0947e4213fbe1de0 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 12:05:54 -0700 Subject: [PATCH 14/75] Refactor: liblrmd: Inline lrmd__reset_result() Signed-off-by: Reid Wahl --- include/crm/lrmd_internal.h | 2 -- lib/lrmd/lrmd_client.c | 20 ++------------------ 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/include/crm/lrmd_internal.h b/include/crm/lrmd_internal.h index d09fce57435..ceb0ed0f951 100644 --- a/include/crm/lrmd_internal.h +++ b/include/crm/lrmd_internal.h @@ -62,8 +62,6 @@ void lrmd__metadata_async(const lrmd_rsc_info_t *rsc, void lrmd__set_result(lrmd_event_data_t *event, enum ocf_exitcode rc, int op_status, const char *exit_reason); -void lrmd__reset_result(lrmd_event_data_t *event); - time_t lrmd__uptime(lrmd_t *lrmd); const char *lrmd__node_start_state(lrmd_t *lrmd); diff --git a/lib/lrmd/lrmd_client.c b/lib/lrmd/lrmd_client.c index ee401a60eb7..3c5b43253ba 100644 --- a/lib/lrmd/lrmd_client.c +++ b/lib/lrmd/lrmd_client.c @@ -233,8 +233,9 @@ lrmd_free_event(lrmd_event_data_t *event) free(event->rsc_id); free(event->op_type); free(event->user_data); + free(event->output); free(event->remote_nodename); - lrmd__reset_result(event); + free(event->exit_reason); g_clear_pointer(&event->params, g_hash_table_destroy); free(event); } @@ -2447,23 +2448,6 @@ lrmd__set_result(lrmd_event_data_t *event, enum ocf_exitcode rc, int op_status, pcmk__str_update(&event->exit_reason, exit_reason); } -/*! - * \internal - * \brief Clear an executor event's exit reason, output, and error output - * - * \param[in,out] event Executor event to reset - */ -void -lrmd__reset_result(lrmd_event_data_t *event) -{ - if (event == NULL) { - return; - } - - g_clear_pointer(&event->exit_reason, free); - g_clear_pointer(&event->output, free); -} - /*! * \internal * \brief Get the uptime of a remote resource connection From d987f3bd4a8c775d75d2267ea11ddabb2513794f Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 12:42:45 -0700 Subject: [PATCH 15/75] Refactor: libcrmcommon: Use pcmk__str_update() in pcmk__set_result() Signed-off-by: Reid Wahl --- lib/common/results.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/common/results.c b/lib/common/results.c index e80c1811053..23f31a07472 100644 --- a/lib/common/results.c +++ b/lib/common/results.c @@ -1209,11 +1209,7 @@ pcmk__set_result(pcmk__action_result_t *result, int exit_status, result->exit_status = exit_status; result->execution_status = exec_status; - - if (!pcmk__str_eq(result->exit_reason, exit_reason, pcmk__str_none)) { - free(result->exit_reason); - result->exit_reason = (exit_reason == NULL)? NULL : strdup(exit_reason); - } + pcmk__str_update(&result->exit_reason, exit_reason); } From 10bbf1cd8cb9aad36d2458dcea096c45f6cebf5a Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 13:01:04 -0700 Subject: [PATCH 16/75] Refactor: libcrmservice: Drop services__grab_{stdout,stderr}() I don't like stealing memory just to avoid allocating a string. It makes the code harder to reason about. And if we ever want to use the stdout_data and stderr_data of the affected svc_action_t in the future, we could run into bugs if we forget that the stdout/stderr was stolen. Signed-off-by: Reid Wahl --- daemons/execd/execd_commands.c | 4 ++-- include/crm/services_internal.h | 2 -- lib/fencing/st_actions.c | 4 ++-- lib/services/services.c | 38 --------------------------------- 4 files changed, 4 insertions(+), 44 deletions(-) diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c index ba1698ef6dd..7363b8bff1f 100644 --- a/daemons/execd/execd_commands.c +++ b/daemons/execd/execd_commands.c @@ -1018,8 +1018,8 @@ action_complete(svc_action_t * action) #endif finalize: - pcmk__set_result_output(&(cmd->result), services__grab_stdout(action), - services__grab_stderr(action)); + pcmk__set_result_output(&cmd->result, pcmk__str_copy(action->stdout_data), + pcmk__str_copy(action->stderr_data)); cmd_finalize(cmd, rsc); } diff --git a/include/crm/services_internal.h b/include/crm/services_internal.h index f7f7cc1bccf..b25a2b8c6a5 100644 --- a/include/crm/services_internal.h +++ b/include/crm/services_internal.h @@ -47,8 +47,6 @@ svc_action_t *services__create_resource_action(const char *name, enum svc_action_flags flags); const char *services__exit_reason(const svc_action_t *action); -char *services__grab_stdout(svc_action_t *action); -char *services__grab_stderr(svc_action_t *action); void services__set_result(svc_action_t *action, int agent_status, enum pcmk_exec_status exec_status, diff --git a/lib/fencing/st_actions.c b/lib/fencing/st_actions.c index a4273919951..da78ad843e0 100644 --- a/lib/fencing/st_actions.c +++ b/lib/fencing/st_actions.c @@ -67,8 +67,8 @@ set_result_from_svc_action(stonith_action_t *action, svc_action_t *svc_action) { services__copy_result(svc_action, &(action->result)); pcmk__set_result_output(&(action->result), - services__grab_stdout(svc_action), - services__grab_stderr(svc_action)); + pcmk__str_copy(svc_action->stdout_data), + pcmk__str_copy(svc_action->stderr_data)); } static void diff --git a/lib/services/services.c b/lib/services/services.c index 81bfd15f405..3f4a4182130 100644 --- a/lib/services/services.c +++ b/lib/services/services.c @@ -1344,44 +1344,6 @@ services__exit_reason(const svc_action_t *action) return action->opaque->exit_reason; } -/*! - * \internal - * \brief Steal stdout from an action - * - * \param[in,out] action Action whose stdout is desired - * - * \return Action's stdout (which may be NULL) - * \note Upon return, \p action will no longer track the output, so it is the - * caller's responsibility to free the return value. - */ -char * -services__grab_stdout(svc_action_t *action) -{ - char *output = action->stdout_data; - - action->stdout_data = NULL; - return output; -} - -/*! - * \internal - * \brief Steal stderr from an action - * - * \param[in,out] action Action whose stderr is desired - * - * \return Action's stderr (which may be NULL) - * \note Upon return, \p action will no longer track the output, so it is the - * caller's responsibility to free the return value. - */ -char * -services__grab_stderr(svc_action_t *action) -{ - char *output = action->stderr_data; - - action->stderr_data = NULL; - return output; -} - // Deprecated functions kept only for backward API compatibility // LCOV_EXCL_START From 75f93185ef9aa3bc072101fb7d131102281401e2 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 13:15:09 -0700 Subject: [PATCH 17/75] Refactor: fencer: Drop list_to_string() terminate_with_delim argument The sole call passes TRUE. Signed-off-by: Reid Wahl --- daemons/fenced/fenced_commands.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/daemons/fenced/fenced_commands.c b/daemons/fenced/fenced_commands.c index 6d92c57507b..d9bb2677d35 100644 --- a/daemons/fenced/fenced_commands.c +++ b/daemons/fenced/fenced_commands.c @@ -1959,11 +1959,11 @@ fenced_unregister_level(xmlNode *msg, pcmk__action_result_t *result) } static char * -list_to_string(GList *list, const char *delim, gboolean terminate_with_delim) +list_to_string(GList *list, const char *delim) { int max = g_list_length(list); size_t delim_len = delim?strlen(delim):0; - size_t alloc_size = 1 + (max?((max-1+(terminate_with_delim?1:0))*delim_len):0); + size_t alloc_size = 1 + (max?(max*delim_len):0); char *rv; char *pos = NULL; @@ -1985,7 +1985,7 @@ list_to_string(GList *list, const char *delim, gboolean terminate_with_delim) lead_delim = delim; } - if ((max != 0) && terminate_with_delim) { + if (max != 0) { sprintf(pos, "%s", delim); } @@ -2039,7 +2039,7 @@ execute_agent_action(xmlNode *msg, pcmk__action_result_t *result) pcmk__set_result(result, CRM_EX_OK, PCMK_EXEC_DONE, NULL); pcmk__set_result_output(result, list_to_string(stonith_watchdog_targets, - "\n", TRUE), + "\n"), NULL); return; From 18afe73ce8517621715a278826892cf2cb964d49 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 13:17:05 -0700 Subject: [PATCH 18/75] Refactor: fencer: Simplify list_to_string() a bit further Signed-off-by: Reid Wahl --- daemons/fenced/fenced_commands.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/daemons/fenced/fenced_commands.c b/daemons/fenced/fenced_commands.c index d9bb2677d35..12402e965be 100644 --- a/daemons/fenced/fenced_commands.c +++ b/daemons/fenced/fenced_commands.c @@ -1963,23 +1963,23 @@ list_to_string(GList *list, const char *delim) { int max = g_list_length(list); size_t delim_len = delim?strlen(delim):0; - size_t alloc_size = 1 + (max?(max*delim_len):0); + size_t alloc_size = 1; char *rv; char *pos = NULL; const char *lead_delim = ""; for (const GList *iter = list; iter != NULL; iter = iter->next) { - const char *value = (const char *) iter->data; + const char *value = iter->data; - alloc_size += strlen(value); + alloc_size += strlen(value) + delim_len; } rv = pcmk__assert_alloc(alloc_size, sizeof(char)); pos = rv; for (const GList *iter = list; iter != NULL; iter = iter->next) { - const char *value = (const char *) iter->data; + const char *value = iter->data; pos = &pos[sprintf(pos, "%s%s", lead_delim, value)]; lead_delim = delim; From 6e141b4d91b230713202b0cf3df97205fe3ca016 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 13:24:07 -0700 Subject: [PATCH 19/75] Refactor: fencer: Drop some elses in execute_agent_action() Signed-off-by: Reid Wahl --- daemons/fenced/fenced_commands.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/daemons/fenced/fenced_commands.c b/daemons/fenced/fenced_commands.c index 12402e965be..752e1fcbb4d 100644 --- a/daemons/fenced/fenced_commands.c +++ b/daemons/fenced/fenced_commands.c @@ -2034,16 +2034,18 @@ execute_agent_action(xmlNode *msg, pcmk__action_result_t *result) pcmk__set_result(result, CRM_EX_ERROR, PCMK_EXEC_NO_FENCE_DEVICE, "Watchdog fence device not configured"); return; + } - } else if (pcmk__str_eq(action, PCMK_ACTION_LIST, pcmk__str_none)) { + if (pcmk__str_eq(action, PCMK_ACTION_LIST, pcmk__str_none)) { pcmk__set_result(result, CRM_EX_OK, PCMK_EXEC_DONE, NULL); pcmk__set_result_output(result, list_to_string(stonith_watchdog_targets, "\n"), NULL); return; + } - } else if (pcmk__str_eq(action, PCMK_ACTION_MONITOR, pcmk__str_none)) { + if (pcmk__str_eq(action, PCMK_ACTION_MONITOR, pcmk__str_none)) { pcmk__set_result(result, CRM_EX_OK, PCMK_EXEC_DONE, NULL); return; } @@ -2057,9 +2059,11 @@ execute_agent_action(xmlNode *msg, pcmk__action_result_t *result) pcmk__format_result(result, CRM_EX_ERROR, PCMK_EXEC_NO_FENCE_DEVICE, "'%s' not found", id); return; + } + + if (!pcmk__is_set(device->flags, fenced_df_api_registered) + && pcmk__str_eq(action, PCMK_ACTION_MONITOR, pcmk__str_none)) { - } else if (!pcmk__is_set(device->flags, fenced_df_api_registered) - && (strcmp(action, PCMK_ACTION_MONITOR) == 0)) { // Monitors may run only on "started" (API-registered) devices pcmk__info("Ignoring API '%s' action request because device %s not " "active", From 6d093890bea13116291b7dfa61468e725e2fa06b Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 13:40:32 -0700 Subject: [PATCH 20/75] Refactor: fencer: Drop list_to_string() Now that it's been simplified, it's clear that this is equivalent to building up a GString with a newline after each target. Signed-off-by: Reid Wahl --- daemons/fenced/fenced_commands.c | 49 +++++++------------------------- 1 file changed, 11 insertions(+), 38 deletions(-) diff --git a/daemons/fenced/fenced_commands.c b/daemons/fenced/fenced_commands.c index 752e1fcbb4d..6359c63f01f 100644 --- a/daemons/fenced/fenced_commands.c +++ b/daemons/fenced/fenced_commands.c @@ -1958,40 +1958,6 @@ fenced_unregister_level(xmlNode *msg, pcmk__action_result_t *result) pcmk__set_result(result, CRM_EX_OK, PCMK_EXEC_DONE, NULL); } -static char * -list_to_string(GList *list, const char *delim) -{ - int max = g_list_length(list); - size_t delim_len = delim?strlen(delim):0; - size_t alloc_size = 1; - char *rv; - - char *pos = NULL; - const char *lead_delim = ""; - - for (const GList *iter = list; iter != NULL; iter = iter->next) { - const char *value = iter->data; - - alloc_size += strlen(value) + delim_len; - } - - rv = pcmk__assert_alloc(alloc_size, sizeof(char)); - pos = rv; - - for (const GList *iter = list; iter != NULL; iter = iter->next) { - const char *value = iter->data; - - pos = &pos[sprintf(pos, "%s%s", lead_delim, value)]; - lead_delim = delim; - } - - if (max != 0) { - sprintf(pos, "%s", delim); - } - - return rv; -} - /*! * \internal * \brief Execute a fence agent action directly (and asynchronously) @@ -2037,11 +2003,18 @@ execute_agent_action(xmlNode *msg, pcmk__action_result_t *result) } if (pcmk__str_eq(action, PCMK_ACTION_LIST, pcmk__str_none)) { + GString *buf = g_string_sized_new(64); + + for (const GList *iter = stonith_watchdog_targets; iter != NULL; + iter = iter->next) { + + g_string_append(buf, iter->data); + g_string_append_c(buf, '\n'); + } + pcmk__set_result(result, CRM_EX_OK, PCMK_EXEC_DONE, NULL); - pcmk__set_result_output(result, - list_to_string(stonith_watchdog_targets, - "\n"), - NULL); + pcmk__set_result_output(result, pcmk__str_copy(buf->str), NULL); + g_string_free(buf, TRUE); return; } From 0e034fed776b72343f7227de9dcc58bbba86376a Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 13:48:42 -0700 Subject: [PATCH 21/75] Refactor: libcrmcommon: Drop pcmk__set_result_output() I don't like stealing memory just to avoid allocating a string. It makes the code harder to reason about. Also, we could run into bugs if we forget that the stdout/stderr was stolen. Further, this confuses Coverity. It thinks pcmk__copy_result() is the allocator for pcmk__action_result_t and that pcmk__set_result_output() is the deallocator. Signed-off-by: Reid Wahl --- daemons/execd/execd_commands.c | 4 ++-- daemons/fenced/fenced_commands.c | 2 +- daemons/fenced/fenced_history.c | 5 ++--- include/crm/common/results_internal.h | 3 --- lib/common/results.c | 27 --------------------------- lib/fencing/st_actions.c | 11 ++++------- lib/lrmd/lrmd_client.c | 5 ++--- 7 files changed, 11 insertions(+), 46 deletions(-) diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c index 7363b8bff1f..0971f5740f1 100644 --- a/daemons/execd/execd_commands.c +++ b/daemons/execd/execd_commands.c @@ -1018,8 +1018,8 @@ action_complete(svc_action_t * action) #endif finalize: - pcmk__set_result_output(&cmd->result, pcmk__str_copy(action->stdout_data), - pcmk__str_copy(action->stderr_data)); + cmd->result.action_stdout = pcmk__str_copy(action->stdout_data); + cmd->result.action_stderr = pcmk__str_copy(action->stderr_data); cmd_finalize(cmd, rsc); } diff --git a/daemons/fenced/fenced_commands.c b/daemons/fenced/fenced_commands.c index 6359c63f01f..113ca66c18d 100644 --- a/daemons/fenced/fenced_commands.c +++ b/daemons/fenced/fenced_commands.c @@ -2013,7 +2013,7 @@ execute_agent_action(xmlNode *msg, pcmk__action_result_t *result) } pcmk__set_result(result, CRM_EX_OK, PCMK_EXEC_DONE, NULL); - pcmk__set_result_output(result, pcmk__str_copy(buf->str), NULL); + result->action_stdout = pcmk__str_copy(buf->str); g_string_free(buf, TRUE); return; } diff --git a/daemons/fenced/fenced_history.c b/daemons/fenced/fenced_history.c index 23864f7e2cf..955831d0c1f 100644 --- a/daemons/fenced/fenced_history.c +++ b/daemons/fenced/fenced_history.c @@ -282,10 +282,9 @@ stonith_xml_history_to_list(const xmlNode *history) } pcmk__set_result(&op->result, exit_status, execution_status, pcmk__xe_get(xml_op, PCMK_XA_EXIT_REASON)); - pcmk__set_result_output(&op->result, - pcmk__xe_get_copy(xml_op, PCMK__XA_ST_OUTPUT), - NULL); + op->result.action_stdout = pcmk__xe_get_copy(xml_op, + PCMK__XA_ST_OUTPUT); g_hash_table_replace(rv, id, op); CRM_LOG_ASSERT(g_hash_table_lookup(rv, id) != NULL); diff --git a/include/crm/common/results_internal.h b/include/crm/common/results_internal.h index 0bff95c33cd..da3383ce4e2 100644 --- a/include/crm/common/results_internal.h +++ b/include/crm/common/results_internal.h @@ -78,9 +78,6 @@ void pcmk__format_result(pcmk__action_result_t *result, int exit_status, enum pcmk_exec_status exec_status, const char *format, ...) G_GNUC_PRINTF(4, 5); -void pcmk__set_result_output(pcmk__action_result_t *result, - char *out, char *err); - void pcmk__reset_result(pcmk__action_result_t *result); void pcmk__copy_result(const pcmk__action_result_t *src, diff --git a/lib/common/results.c b/lib/common/results.c index 23f31a07472..9c0a57b275b 100644 --- a/lib/common/results.c +++ b/lib/common/results.c @@ -1251,33 +1251,6 @@ pcmk__format_result(pcmk__action_result_t *result, int exit_status, result->exit_reason = reason; } -/*! - * \internal - * \brief Set the output of an action - * - * \param[out] result Action result to set output for - * \param[in] out Action output to set (must be dynamically - * allocated) - * \param[in] err Action error output to set (must be dynamically - * allocated) - * - * \note \p result will take ownership of \p out and \p err, so the caller - * should not free them. - */ -void -pcmk__set_result_output(pcmk__action_result_t *result, char *out, char *err) -{ - if (result == NULL) { - return; - } - - free(result->action_stdout); - result->action_stdout = out; - - free(result->action_stderr); - result->action_stderr = err; -} - /*! * \internal * \brief Clear a result's exit reason, output, and error output diff --git a/lib/fencing/st_actions.c b/lib/fencing/st_actions.c index da78ad843e0..11ea045a8b1 100644 --- a/lib/fencing/st_actions.c +++ b/lib/fencing/st_actions.c @@ -65,10 +65,9 @@ static void log_action(stonith_action_t *action, pid_t pid); static void set_result_from_svc_action(stonith_action_t *action, svc_action_t *svc_action) { - services__copy_result(svc_action, &(action->result)); - pcmk__set_result_output(&(action->result), - pcmk__str_copy(svc_action->stdout_data), - pcmk__str_copy(svc_action->stderr_data)); + services__copy_result(svc_action, &action->result); + action->result.action_stdout = pcmk__str_copy(svc_action->stdout_data); + action->result.action_stderr = pcmk__str_copy(svc_action->stderr_data); } static void @@ -484,12 +483,10 @@ stonith__xe_get_result(const xmlNode *xml, pcmk__action_result_t *result) int exit_status = CRM_EX_OK; int execution_status = PCMK_EXEC_DONE; const char *exit_reason = NULL; - char *action_stdout = NULL; CRM_CHECK((xml != NULL) && (result != NULL), return); exit_reason = pcmk__xe_get(xml, PCMK_XA_EXIT_REASON); - action_stdout = pcmk__xe_get_copy(xml, PCMK__XA_ST_OUTPUT); // A result must include an exit status and execution status if ((pcmk__xe_get_int(xml, PCMK__XA_RC_CODE, &exit_status) != pcmk_rc_ok) @@ -515,7 +512,7 @@ stonith__xe_get_result(const xmlNode *xml, pcmk__action_result_t *result) } } pcmk__set_result(result, exit_status, execution_status, exit_reason); - pcmk__set_result_output(result, action_stdout, NULL); + result->action_stdout = pcmk__xe_get_copy(xml, PCMK__XA_ST_OUTPUT); } static void diff --git a/lib/lrmd/lrmd_client.c b/lib/lrmd/lrmd_client.c index 3c5b43253ba..7735636995e 100644 --- a/lib/lrmd/lrmd_client.c +++ b/lib/lrmd/lrmd_client.c @@ -2331,11 +2331,10 @@ metadata_complete(svc_action_t *action) pcmk__action_result_t result = PCMK__UNKNOWN_RESULT; services__copy_result(action, &result); - pcmk__set_result_output(&result, action->stdout_data, action->stderr_data); + result.action_stdout = pcmk__str_copy(action->stdout_data); + result.action_stderr = pcmk__str_copy(action->stderr_data); metadata_cb->callback(0, &result, metadata_cb->user_data); - result.action_stdout = NULL; // Prevent free, because action owns it - result.action_stderr = NULL; // Prevent free, because action owns it pcmk__reset_result(&result); free(metadata_cb); } From e3fe6c34128c01295946f9f647f114836d4374e5 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Fri, 8 May 2026 21:39:30 -0700 Subject: [PATCH 22/75] Feature: libcib: Assert on allocation error when creating new cib_t We were already doing this for file and remote cib_t objects (via pcmk__generate_uuid() and pcmk__str_copy(), respectively). We might as well also assert in other places in those variants, as well as asserting in native cib_t creation and in cib_new_variant(). This resolves some Coverity memory leak issues. Signed-off-by: Reid Wahl --- lib/cib/cib_client.c | 13 ++----------- lib/cib/cib_file.c | 20 ++------------------ lib/cib/cib_native.c | 14 ++------------ lib/cib/cib_remote.c | 14 ++------------ 4 files changed, 8 insertions(+), 53 deletions(-) diff --git a/lib/cib/cib_client.c b/lib/cib/cib_client.c index 685613c403b..f0681b09fb2 100644 --- a/lib/cib/cib_client.c +++ b/lib/cib/cib_client.c @@ -606,11 +606,7 @@ cib_new_variant(void) { cib_t *new_cib = NULL; - new_cib = calloc(1, sizeof(cib_t)); - - if (new_cib == NULL) { - return NULL; - } + new_cib = pcmk__assert_alloc(1, sizeof(cib_t)); remove_cib_op_callback(0, TRUE); /* remove all */ @@ -623,12 +619,7 @@ cib_new_variant(void) new_cib->notify_list = NULL; /* the rest will get filled in by the variant constructor */ - new_cib->cmds = calloc(1, sizeof(cib_api_operations_t)); - - if (new_cib->cmds == NULL) { - free(new_cib); - return NULL; - } + new_cib->cmds = pcmk__assert_alloc(1, sizeof(cib_api_operations_t)); new_cib->cmds->add_notify_callback = cib_client_add_notify_callback; new_cib->cmds->del_notify_callback = cib_client_del_notify_callback; diff --git a/lib/cib/cib_file.c b/lib/cib/cib_file.c index d50d679967b..9d02b784735 100644 --- a/lib/cib/cib_file.c +++ b/lib/cib/cib_file.c @@ -747,7 +747,6 @@ cib_file_new(const char *cib_location) { cib_t *cib = NULL; file_opaque_t *private = NULL; - char *filename = NULL; if (cib_location == NULL) { cib_location = getenv("CIB_file"); @@ -757,25 +756,10 @@ cib_file_new(const char *cib_location) } cib = cib_new_variant(); - if (cib == NULL) { - return NULL; - } - - filename = strdup(cib_location); - if (filename == NULL) { - free(cib); - return NULL; - } - - private = calloc(1, sizeof(file_opaque_t)); - if (private == NULL) { - free(cib); - free(filename); - return NULL; - } + private = pcmk__assert_alloc(1, sizeof(file_opaque_t)); private->id = pcmk__generate_uuid(); - private->filename = filename; + private->filename = pcmk__str_copy(cib_location); cib->variant = cib_file; cib->variant_opaque = private; diff --git a/lib/cib/cib_native.c b/lib/cib/cib_native.c index f5e59415d9a..d2d8a770144 100644 --- a/lib/cib/cib_native.c +++ b/lib/cib/cib_native.c @@ -485,19 +485,9 @@ cib_native_client_id(const cib_t *cib, const char **async_id, cib_t * cib_native_new(void) { - cib_native_opaque_t *native = NULL; cib_t *cib = cib_new_variant(); - - if (cib == NULL) { - return NULL; - } - - native = calloc(1, sizeof(cib_native_opaque_t)); - - if (native == NULL) { - free(cib); - return NULL; - } + cib_native_opaque_t *native = + pcmk__assert_alloc(1, sizeof(cib_native_opaque_t)); cib->variant = cib_native; cib->variant_opaque = native; diff --git a/lib/cib/cib_remote.c b/lib/cib/cib_remote.c index b9eb393afa6..a21a46cb68a 100644 --- a/lib/cib/cib_remote.c +++ b/lib/cib/cib_remote.c @@ -714,19 +714,9 @@ cib_t * cib_remote_new(const char *server, const char *user, const char *passwd, int port, gboolean encrypted) { - cib_remote_opaque_t *private = NULL; cib_t *cib = cib_new_variant(); - - if (cib == NULL) { - return NULL; - } - - private = calloc(1, sizeof(cib_remote_opaque_t)); - - if (private == NULL) { - free(cib); - return NULL; - } + cib_remote_opaque_t *private = + pcmk__assert_alloc(1, sizeof(cib_remote_opaque_t)); cib->variant = cib_remote; cib->variant_opaque = private; From d4f761022ff75d36b990a48e382950e460e2ee64 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 15:40:22 -0700 Subject: [PATCH 23/75] Refactor: libcrmcommon: Make pcmk__dup_alert() static Signed-off-by: Reid Wahl --- include/crm/common/alerts_internal.h | 1 - lib/common/alerts.c | 12 ++++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/include/crm/common/alerts_internal.h b/include/crm/common/alerts_internal.h index 1363ffe5d7e..2abe74ddcf9 100644 --- a/include/crm/common/alerts_internal.h +++ b/include/crm/common/alerts_internal.h @@ -80,7 +80,6 @@ enum pcmk__alert_keys_e { extern const char *pcmk__alert_keys[PCMK__ALERT_INTERNAL_KEY_MAX]; -pcmk__alert_t *pcmk__dup_alert(const pcmk__alert_t *entry); pcmk__alert_t *pcmk__alert_new(const char *id, const char *path); void pcmk__free_alert(pcmk__alert_t *entry); void pcmk__add_alert_key(GHashTable *table, enum pcmk__alert_keys_e name, diff --git a/lib/common/alerts.c b/lib/common/alerts.c index b67d345c9c6..daab1826a8a 100644 --- a/lib/common/alerts.c +++ b/lib/common/alerts.c @@ -79,14 +79,14 @@ pcmk__free_alert(pcmk__alert_t *entry) /*! * \internal - * \brief Duplicate an alert entry + * \brief Copy an alert entry * - * \param[in] entry Alert entry to duplicate + * \param[in] entry Alert entry to copy * - * \return Duplicate of alert entry + * \return Copy of alert entry */ -pcmk__alert_t * -pcmk__dup_alert(const pcmk__alert_t *entry) +static pcmk__alert_t * +copy_alert(const pcmk__alert_t *entry) { pcmk__alert_t *new_entry = pcmk__alert_new(entry->id, entry->path); @@ -406,7 +406,7 @@ pcmk__unpack_alerts(const xmlNode *alerts) recipient != NULL; recipient = pcmk__xe_next(recipient, PCMK_XE_RECIPIENT)) { - pcmk__alert_t *recipient_entry = pcmk__dup_alert(entry); + pcmk__alert_t *recipient_entry = copy_alert(entry); unsigned int n_envvars = 0; recipients++; From f98f6823282fd6b1b5ff1dd899ff547751c607dd Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 16:43:44 -0700 Subject: [PATCH 24/75] Low: libcrmcommon: Avoid memory leak when creating alert recipient entry Coverity found an issue in which we were copying the existing entry (which involves copying its recipient field) and then overwriting the copy's recipient field with a copy of the value from the PCMK_XE_RECIPIENT XML. Signed-off-by: Reid Wahl --- lib/common/alerts.c | 56 +++++++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/lib/common/alerts.c b/lib/common/alerts.c index daab1826a8a..e3babc1c3b8 100644 --- a/lib/common/alerts.c +++ b/lib/common/alerts.c @@ -77,30 +77,6 @@ pcmk__free_alert(pcmk__alert_t *entry) } } -/*! - * \internal - * \brief Copy an alert entry - * - * \param[in] entry Alert entry to copy - * - * \return Copy of alert entry - */ -static pcmk__alert_t * -copy_alert(const pcmk__alert_t *entry) -{ - pcmk__alert_t *new_entry = pcmk__alert_new(entry->id, entry->path); - - new_entry->timeout = entry->timeout; - new_entry->flags = entry->flags; - new_entry->envvars = pcmk__str_table_dup(entry->envvars); - new_entry->tstamp_format = pcmk__str_copy(entry->tstamp_format); - new_entry->recipient = pcmk__str_copy(entry->recipient); - if (entry->select_attribute_name) { - new_entry->select_attribute_name = g_strdupv(entry->select_attribute_name); - } - return new_entry; -} - void pcmk__add_alert_key(GHashTable *table, enum pcmk__alert_keys_e name, const char *value) @@ -347,6 +323,34 @@ unpack_alert(xmlNode *alert, pcmk__alert_t *entry, unsigned int *max_timeout) return rc; } +/*! + * \internal + * \brief Copy an alert entry + * + * This creates a deep copy of \p entry, except that the new object's + * \c recipient field is copied from \p recipient instead of from \p entry. + * \p recipient is a \c PCMK_XE_RECIPIENT element, and its \c PCMK_XA_VALUE + * attribute is copied as the new object's \c recipient field. + * + * \param[in] entry Alert entry to copy + * \param[in] recipient Recipient XML + * + * \return Copy of alert entry + */ +static pcmk__alert_t * +copy_alert(const pcmk__alert_t *entry, const xmlNode *recipient) +{ + pcmk__alert_t *new_entry = pcmk__alert_new(entry->id, entry->path); + + new_entry->tstamp_format = pcmk__str_copy(entry->tstamp_format); + new_entry->recipient = pcmk__xe_get_copy(recipient, PCMK_XA_VALUE); + new_entry->select_attribute_name = g_strdupv(entry->select_attribute_name); + new_entry->envvars = pcmk__str_table_dup(entry->envvars); + new_entry->timeout = entry->timeout; + new_entry->flags = entry->flags; + return new_entry; +} + /*! * \internal * \brief Unpack a CIB alerts section into a list of alert entries @@ -406,12 +410,10 @@ pcmk__unpack_alerts(const xmlNode *alerts) recipient != NULL; recipient = pcmk__xe_next(recipient, PCMK_XE_RECIPIENT)) { - pcmk__alert_t *recipient_entry = copy_alert(entry); + pcmk__alert_t *recipient_entry = copy_alert(entry, recipient); unsigned int n_envvars = 0; recipients++; - recipient_entry->recipient = pcmk__xe_get_copy(recipient, - PCMK_XA_VALUE); if (unpack_alert(recipient, recipient_entry, &max_timeout) != pcmk_rc_ok) { From 4415732d8b4e555c9acb759ec5207efb8263f89d Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 16:58:33 -0700 Subject: [PATCH 25/75] Refactor: libpe_status: Change a condition in pe__unpack_bundle() This is a slight decrease in redundancy. Usually I don't like more layers of nesting, but it's a small block here, and the whole block may be functionized later. Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index d6c3f005c2b..63ff2dded08 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1124,11 +1124,18 @@ pe__unpack_bundle(pcmk_resource_t *rsc) xml_obj = pcmk__xe_first_child(rsc->priv->xml, PCMK_XE_PRIMITIVE, NULL, NULL); - if (xml_obj && valid_network(bundle_data)) { + if (xml_obj != NULL) { const char *suffix = NULL; char *value = NULL; xmlNode *xml_set = NULL; + if (!valid_network(bundle_data)) { + pcmk__config_err("Cannot control %s inside %s without either " + PCMK_XA_IP_RANGE_START " or " PCMK_XA_CONTROL_PORT, + rsc->id, pcmk__xe_id(xml_obj)); + return false; + } + xml_resource = pcmk__xe_create(NULL, PCMK_XE_CLONE); /* @COMPAT We no longer use the tag, but we need to keep it as @@ -1171,12 +1178,6 @@ pe__unpack_bundle(pcmk_resource_t *rsc) //pcmk__xe_set(xml_obj, PCMK_XA_ID, bundle_data->prefix); pcmk__xml_copy(xml_resource, xml_obj); - - } else if(xml_obj) { - pcmk__config_err("Cannot control %s inside %s without either " - PCMK_XA_IP_RANGE_START " or " PCMK_XA_CONTROL_PORT, - rsc->id, pcmk__xe_id(xml_obj)); - return FALSE; } if(xml_resource) { From 92713b217f665f32843d1714869752f2a4f75206 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 17:01:48 -0700 Subject: [PATCH 26/75] Refactor: libpe_status: Use bool in pe__unpack_bundle() Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index 63ff2dded08..23e7991767a 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -982,7 +982,7 @@ pe__unpack_bundle(pcmk_resource_t *rsc) const xmlNode *xml_child = NULL; xmlNode *xml_resource = NULL; pe__bundle_variant_data_t *bundle_data = NULL; - bool need_log_mount = TRUE; + bool need_log_mount = true; pcmk__assert(rsc != NULL); pcmk__rsc_trace(rsc, "Processing resource %s...", rsc->id); @@ -1006,7 +1006,7 @@ pe__unpack_bundle(pcmk_resource_t *rsc) } if (xml_obj == NULL) { - return FALSE; + return false; } // Use 0 for default, minimum, and invalid PCMK_XA_PROMOTED_MAX @@ -1114,7 +1114,7 @@ pe__unpack_bundle(pcmk_resource_t *rsc) if (source && target) { mount_add(bundle_data, source, target, options, flags); if (strcmp(target, "/var/log") == 0) { - need_log_mount = FALSE; + need_log_mount = false; } } else { pcmk__config_err("Invalid mount directive %s", @@ -1191,7 +1191,7 @@ pe__unpack_bundle(pcmk_resource_t *rsc) pcmk__xml_free(xml_resource); if (rc != pcmk_rc_ok) { - return FALSE; + return false; } /* Currently, we always map the default authentication key location @@ -1266,7 +1266,7 @@ pe__unpack_bundle(pcmk_resource_t *rsc) g_hash_table_lookup(replica->child->priv->meta, PCMK_META_CONTAINER_ATTRIBUTE_TARGET); } - bundle_data->container_host_options = g_string_free(buffer, FALSE); + bundle_data->container_host_options = g_string_free(buffer, false); if (bundle_data->attribute_target) { pcmk__insert_dup(rsc->priv->meta, @@ -1290,7 +1290,7 @@ pe__unpack_bundle(pcmk_resource_t *rsc) bundle_data->replicas = g_list_append(bundle_data->replicas, replica); } - bundle_data->container_host_options = g_string_free(buffer, FALSE); + bundle_data->container_host_options = g_string_free(buffer, false); } for (GList *gIter = bundle_data->replicas; gIter != NULL; @@ -1300,7 +1300,7 @@ pe__unpack_bundle(pcmk_resource_t *rsc) if (create_replica_resources(rsc, bundle_data, replica) != pcmk_rc_ok) { pcmk__config_err("Failed unpacking resource %s", rsc->id); pcmk__free_resource(rsc); - return FALSE; + return false; } /* Utilization needs special handling for bundles. It makes no sense for @@ -1333,7 +1333,7 @@ pe__unpack_bundle(pcmk_resource_t *rsc) rsc->priv->children = g_list_append(rsc->priv->children, bundle_data->child); } - return TRUE; + return true; } static int From 20271e50c79161f0b2632c9d259a4c35fd5e854f Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 17:06:39 -0700 Subject: [PATCH 27/75] Refactor: libpe_status: Rename GLib iterators to iter and iter2 For consistency. Also drop some unnecessary casts along the way. Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 43 ++++++++++------------ lib/pengine/clone.c | 37 ++++++++----------- lib/pengine/group.c | 33 +++++++---------- lib/pengine/native.c | 78 ++++++++++++++++++++-------------------- lib/pengine/pe_actions.c | 26 +++++++------- lib/pengine/pe_notif.c | 4 +-- lib/pengine/pe_output.c | 55 ++++++++++++++-------------- lib/pengine/remote.c | 6 ++-- lib/pengine/unpack.c | 31 +++++++--------- lib/pengine/utils.c | 50 +++++++++++--------------- 10 files changed, 162 insertions(+), 201 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index 23e7991767a..b4b93d7d900 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -652,7 +652,7 @@ static int create_remote_resource(pcmk_resource_t *parent, pe__bundle_variant_data_t *data, pcmk__bundle_replica_t *replica) { - GHashTableIter gIter; + GHashTableIter iter; pcmk_node_t *node = NULL; pcmk_node_t *copy = NULL; xmlNode *xml_remote = NULL; @@ -776,8 +776,8 @@ create_remote_resource(pcmk_resource_t *parent, pe__bundle_variant_data_t *data, // Make Coverity happy pcmk__assert(replica->remote != NULL); - g_hash_table_iter_init(&gIter, replica->remote->priv->allowed_nodes); - while (g_hash_table_iter_next(&gIter, NULL, (void **)&node)) { + g_hash_table_iter_init(&iter, replica->remote->priv->allowed_nodes); + while (g_hash_table_iter_next(&iter, NULL, (void **) &node)) { if (pcmk__is_pacemaker_remote_node(node)) { // Remote resources can only run on 'normal' cluster node node->assign->score = -PCMK_SCORE_INFINITY; @@ -897,9 +897,8 @@ replica_for_remote(pcmk_resource_t *remote) } get_bundle_variant_data(bundle_data, top); - for (GList *gIter = bundle_data->replicas; gIter != NULL; - gIter = gIter->next) { - pcmk__bundle_replica_t *replica = gIter->data; + for (GList *iter = bundle_data->replicas; iter != NULL; iter = iter->next) { + pcmk__bundle_replica_t *replica = iter->data; if (replica->remote == remote) { return replica; @@ -1182,7 +1181,6 @@ pe__unpack_bundle(pcmk_resource_t *rsc) if(xml_resource) { int lpc = 0; - GList *childIter = NULL; pe__bundle_port_t *port = NULL; GString *buffer = NULL; int rc = pe__unpack_resource(xml_resource, &bundle_data->child, rsc, @@ -1242,13 +1240,13 @@ pe__unpack_bundle(pcmk_resource_t *rsc) bundle_data->ports = g_list_append(bundle_data->ports, port); buffer = g_string_sized_new(1024); - for (childIter = bundle_data->child->priv->children; - childIter != NULL; childIter = childIter->next) { + for (GList *iter = bundle_data->child->priv->children; + iter != NULL; iter = iter->next) { pcmk__bundle_replica_t *replica = NULL; replica = pcmk__assert_alloc(1, sizeof(pcmk__bundle_replica_t)); - replica->child = childIter->data; + replica->child = iter->data; pcmk__set_rsc_flags(replica->child, pcmk__rsc_exclusive_probes); replica->offset = lpc++; @@ -1293,9 +1291,8 @@ pe__unpack_bundle(pcmk_resource_t *rsc) bundle_data->container_host_options = g_string_free(buffer, false); } - for (GList *gIter = bundle_data->replicas; gIter != NULL; - gIter = gIter->next) { - pcmk__bundle_replica_t *replica = gIter->data; + for (GList *iter = bundle_data->replicas; iter != NULL; iter = iter->next) { + pcmk__bundle_replica_t *replica = iter->data; if (create_replica_resources(rsc, bundle_data, replica) != pcmk_rc_ok) { pcmk__config_err("Failed unpacking resource %s", rsc->id); @@ -1407,9 +1404,8 @@ pe__find_bundle_replica(const pcmk_resource_t *bundle, const pcmk_node_t *node) pcmk__assert((bundle != NULL) && (node != NULL)); get_bundle_variant_data(bundle_data, bundle); - for (GList *gIter = bundle_data->replicas; gIter != NULL; - gIter = gIter->next) { - pcmk__bundle_replica_t *replica = gIter->data; + for (GList *iter = bundle_data->replicas; iter != NULL; iter = iter->next) { + pcmk__bundle_replica_t *replica = iter->data; pcmk__assert((replica != NULL) && (replica->node != NULL)); if (pcmk__same_node(replica->node, node)) { @@ -1445,9 +1441,8 @@ pe__bundle_xml(pcmk__output_t *out, va_list args) print_everything = pcmk__str_in_list(rsc->id, only_rsc, pcmk__str_star_matches); - for (GList *gIter = bundle_data->replicas; gIter != NULL; - gIter = gIter->next) { - pcmk__bundle_replica_t *replica = gIter->data; + for (GList *iter = bundle_data->replicas; iter != NULL; iter = iter->next) { + pcmk__bundle_replica_t *replica = iter->data; pcmk_resource_t *ip = replica->ip; pcmk_resource_t *child = replica->child; pcmk_resource_t *container = replica->container; @@ -1612,9 +1607,8 @@ pe__bundle_html(pcmk__output_t *out, va_list args) print_everything = pcmk__str_in_list(rsc->id, only_rsc, pcmk__str_star_matches); - for (GList *gIter = bundle_data->replicas; gIter != NULL; - gIter = gIter->next) { - pcmk__bundle_replica_t *replica = gIter->data; + for (GList *iter = bundle_data->replicas; iter != NULL; iter = iter->next) { + pcmk__bundle_replica_t *replica = iter->data; pcmk_resource_t *ip = replica->ip; pcmk_resource_t *child = replica->child; pcmk_resource_t *container = replica->container; @@ -1763,9 +1757,8 @@ pe__bundle_text(pcmk__output_t *out, va_list args) print_everything = pcmk__str_in_list(rsc->id, only_rsc, pcmk__str_star_matches); - for (GList *gIter = bundle_data->replicas; gIter != NULL; - gIter = gIter->next) { - pcmk__bundle_replica_t *replica = gIter->data; + for (GList *iter = bundle_data->replicas; iter != NULL; iter = iter->next) { + pcmk__bundle_replica_t *replica = iter->data; pcmk_resource_t *ip = replica->ip; pcmk_resource_t *child = replica->child; pcmk_resource_t *container = replica->container; diff --git a/lib/pengine/clone.c b/lib/pengine/clone.c index b71a00e7aca..f1d7781583e 100644 --- a/lib/pengine/clone.c +++ b/lib/pengine/clone.c @@ -464,10 +464,8 @@ clone_unpack(pcmk_resource_t *rsc) bool clone_active(const pcmk_resource_t *rsc, bool all) { - for (GList *gIter = rsc->priv->children; - gIter != NULL; gIter = gIter->next) { - - pcmk_resource_t *child_rsc = (pcmk_resource_t *) gIter->data; + for (GList *iter = rsc->priv->children; iter != NULL; iter = iter->next) { + pcmk_resource_t *child_rsc = iter->data; bool child_active = child_rsc->priv->fns->active(child_rsc, all); if (all == FALSE && child_active) { @@ -529,10 +527,8 @@ is_set_recursive(const pcmk_resource_t *rsc, long long flag, bool any) return FALSE; } - for (GList *gIter = rsc->priv->children; - gIter != NULL; gIter = gIter->next) { - - if(is_set_recursive(gIter->data, flag, any)) { + for (GList *iter = rsc->priv->children; iter != NULL; iter = iter->next) { + if (is_set_recursive(iter->data, flag, any)) { if(any) { return TRUE; } @@ -575,10 +571,8 @@ pe__clone_xml(pcmk__output_t *out, va_list args) all = g_list_prepend(all, (void *) "*"); - for (GList *gIter = rsc->priv->children; - gIter != NULL; gIter = gIter->next) { - - pcmk_resource_t *child_rsc = (pcmk_resource_t *) gIter->data; + for (GList *iter = rsc->priv->children; iter != NULL; iter = iter->next) { + pcmk_resource_t *child_rsc = iter->data; if (pcmk__rsc_filtered_by_node(child_rsc, only_node)) { continue; @@ -649,7 +643,6 @@ pe__clone_default(pcmk__output_t *out, va_list args) GList *promoted_list = NULL; GList *started_list = NULL; - GList *gIter = NULL; const char *desc = NULL; @@ -672,9 +665,9 @@ pe__clone_default(pcmk__output_t *out, va_list args) && pcmk__str_in_list(rsc->id, only_rsc, pcmk__str_star_matches)); - for (gIter = rsc->priv->children; gIter != NULL; gIter = gIter->next) { + for (GList *iter = rsc->priv->children; iter != NULL; iter = iter->next) { gboolean print_full = FALSE; - pcmk_resource_t *child_rsc = (pcmk_resource_t *) gIter->data; + pcmk_resource_t *child_rsc = iter->data; bool partially_active = child_rsc->priv->fns->active(child_rsc, false); if (pcmk__rsc_filtered_by_node(child_rsc, only_node)) { @@ -781,8 +774,8 @@ pe__clone_default(pcmk__output_t *out, va_list args) /* Promoted */ promoted_list = g_list_sort(promoted_list, pe__cmp_node_name); - for (gIter = promoted_list; gIter; gIter = gIter->next) { - pcmk_node_t *host = gIter->data; + for (GList *iter = promoted_list; iter != NULL; iter = iter->next) { + pcmk_node_t *host = iter->data; if (!pcmk__str_in_list(host->priv->name, only_node, pcmk__str_star_matches|pcmk__str_casei)) { @@ -804,8 +797,8 @@ pe__clone_default(pcmk__output_t *out, va_list args) /* Started/Unpromoted */ started_list = g_list_sort(started_list, pe__cmp_node_name); - for (gIter = started_list; gIter; gIter = gIter->next) { - pcmk_node_t *host = gIter->data; + for (GList *iter = started_list; iter != NULL; iter = iter->next) { + pcmk_node_t *host = iter->data; if (!pcmk__str_in_list(host->priv->name, only_node, pcmk__str_star_matches|pcmk__str_casei)) { @@ -968,10 +961,8 @@ clone_resource_state(const pcmk_resource_t *rsc, bool current) { enum rsc_role_e clone_role = pcmk_role_unknown; - for (GList *gIter = rsc->priv->children; - gIter != NULL; gIter = gIter->next) { - - pcmk_resource_t *child_rsc = (pcmk_resource_t *) gIter->data; + for (GList *iter = rsc->priv->children; iter != NULL; iter = iter->next) { + pcmk_resource_t *child_rsc = iter->data; enum rsc_role_e a_role = child_rsc->priv->fns->state(child_rsc, current); diff --git a/lib/pengine/group.c b/lib/pengine/group.c index d864364ad7b..57ea37be97d 100644 --- a/lib/pengine/group.c +++ b/lib/pengine/group.c @@ -112,10 +112,8 @@ inactive_resources(pcmk_resource_t *rsc) { int retval = 0; - for (GList *gIter = rsc->priv->children; - gIter != NULL; gIter = gIter->next) { - - pcmk_resource_t *child_rsc = (pcmk_resource_t *) gIter->data; + for (GList *iter = rsc->priv->children; iter != NULL; iter = iter->next) { + pcmk_resource_t *child_rsc = iter->data; if (!child_rsc->priv->fns->active(child_rsc, true)) { retval++; @@ -252,10 +250,8 @@ group_active(const pcmk_resource_t *rsc, bool all) gboolean c_all = TRUE; gboolean c_any = FALSE; - for (GList *gIter = rsc->priv->children; - gIter != NULL; gIter = gIter->next) { - - pcmk_resource_t *child_rsc = (pcmk_resource_t *) gIter->data; + for (GList *iter = rsc->priv->children; iter != NULL; iter = iter->next) { + pcmk_resource_t *child_rsc = iter->data; if (child_rsc->priv->fns->active(child_rsc, all)) { c_any = TRUE; @@ -298,17 +294,15 @@ pe__group_xml(pcmk__output_t *out, va_list args) return rc; } - for (GList *gIter = rsc->priv->children; - gIter != NULL; gIter = gIter->next) { - - pcmk_resource_t *child_rsc = (pcmk_resource_t *) gIter->data; + for (GList *iter = rsc->priv->children; iter != NULL; iter = iter->next) { + pcmk_resource_t *child_rsc = iter->data; if (skip_child_rsc(rsc, child_rsc, parent_passes, only_rsc, show_opts)) { continue; } if (rc == pcmk_rc_no_output) { - char *count = pcmk__itoa(g_list_length(gIter)); + char *count = pcmk__itoa(g_list_length(iter)); const char *maintenance = pcmk__flag_text(rsc->flags, pcmk__rsc_maintenance); const char *managed = pcmk__flag_text(rsc->flags, @@ -382,9 +376,10 @@ pe__group_default(pcmk__output_t *out, va_list args) } } else { - for (GList *gIter = rsc->priv->children; - gIter != NULL; gIter = gIter->next) { - pcmk_resource_t *child_rsc = (pcmk_resource_t *) gIter->data; + for (GList *iter = rsc->priv->children; iter != NULL; + iter = iter->next) { + + pcmk_resource_t *child_rsc = iter->data; if (skip_child_rsc(rsc, child_rsc, parent_passes, only_rsc, show_opts)) { continue; @@ -419,10 +414,8 @@ group_resource_state(const pcmk_resource_t *rsc, bool current) { enum rsc_role_e group_role = pcmk_role_unknown; - for (GList *gIter = rsc->priv->children; - gIter != NULL; gIter = gIter->next) { - - pcmk_resource_t *child_rsc = (pcmk_resource_t *) gIter->data; + for (GList *iter = rsc->priv->children; iter != NULL; iter = iter->next) { + pcmk_resource_t *child_rsc = iter->data; enum rsc_role_e role = child_rsc->priv->fns->state(child_rsc, current); diff --git a/lib/pengine/native.c b/lib/pengine/native.c index e05d20d6e36..10208745821 100644 --- a/lib/pengine/native.c +++ b/lib/pengine/native.c @@ -84,10 +84,10 @@ native_priority_to_node(pcmk_resource_t *rsc, pcmk_node_t *node, const pcmk_resource_t *launcher = NULL; launcher = node->priv->remote->priv->launcher; - for (GList *gIter = launcher->priv->active_nodes; - gIter != NULL; gIter = gIter->next) { + for (GList *iter = launcher->priv->active_nodes; iter != NULL; + iter = iter->next) { - pcmk_node_t *a_node = gIter->data; + pcmk_node_t *a_node = iter->data; a_node->priv->priority += priority; pcmk__rsc_trace(rsc, @@ -109,10 +109,10 @@ native_add_running(pcmk_resource_t *rsc, pcmk_node_t *node, CRM_CHECK(node != NULL, return); - for (GList *gIter = rsc->priv->active_nodes; - gIter != NULL; gIter = gIter->next) { + for (GList *iter = rsc->priv->active_nodes; iter != NULL; + iter = iter->next) { - pcmk_node_t *a_node = (pcmk_node_t *) gIter->data; + pcmk_node_t *a_node = iter->data; if (pcmk__same_node(a_node, node)) { return; @@ -155,7 +155,7 @@ native_add_running(pcmk_resource_t *rsc, pcmk_node_t *node, switch (rsc->priv->multiply_active_policy) { case pcmk__multiply_active_stop: { - GHashTableIter gIter; + GHashTableIter iter; pcmk_node_t *local_node = NULL; /* make sure it doesn't come up again */ @@ -163,8 +163,11 @@ native_add_running(pcmk_resource_t *rsc, pcmk_node_t *node, g_hash_table_destroy); rsc->priv->allowed_nodes = pe__node_list2table(scheduler->nodes); - g_hash_table_iter_init(&gIter, rsc->priv->allowed_nodes); - while (g_hash_table_iter_next(&gIter, NULL, (void **)&local_node)) { + g_hash_table_iter_init(&iter, rsc->priv->allowed_nodes); + + while (g_hash_table_iter_next(&iter, NULL, + (void **) &local_node)) { + local_node->assign->score = -PCMK_SCORE_INFINITY; } } @@ -181,9 +184,10 @@ native_add_running(pcmk_resource_t *rsc, pcmk_node_t *node, && (parent->priv->multiply_active_policy == pcmk__multiply_active_block)) { - for (GList *gIter = parent->priv->children; - gIter != NULL; gIter = gIter->next) { - pcmk_resource_t *child = gIter->data; + for (GList *iter = parent->priv->children; iter != NULL; + iter = iter->next) { + + pcmk_resource_t *child = iter->data; pcmk__clear_rsc_flags(child, pcmk__rsc_managed); pcmk__set_rsc_flags(child, pcmk__rsc_blocked); @@ -329,10 +333,8 @@ native_find_rsc(pcmk_resource_t *rsc, const char *id, return rsc; } - for (GList *gIter = rsc->priv->children; - gIter != NULL; gIter = gIter->next) { - - pcmk_resource_t *child = (pcmk_resource_t *) gIter->data; + for (GList *iter = rsc->priv->children; iter != NULL; iter = iter->next) { + pcmk_resource_t *child = iter->data; result = rsc->priv->fns->find_rsc(child, id, on_node, flags); if (result) { @@ -345,10 +347,10 @@ native_find_rsc(pcmk_resource_t *rsc, const char *id, bool native_active(const pcmk_resource_t *rsc, bool all) { - for (GList *gIter = rsc->priv->active_nodes; - gIter != NULL; gIter = gIter->next) { + for (GList *iter = rsc->priv->active_nodes; iter != NULL; + iter = iter->next) { - pcmk_node_t *a_node = (pcmk_node_t *) gIter->data; + pcmk_node_t *a_node = iter->data; if (a_node->details->unclean) { pcmk__rsc_trace(rsc, "Resource %s: %s is unclean", @@ -822,10 +824,10 @@ pe__resource_xml(pcmk__output_t *out, va_list args) pcmk__assert(rc == pcmk_rc_ok); - for (GList *gIter = rsc->priv->active_nodes; - gIter != NULL; gIter = gIter->next) { + for (GList *iter = rsc->priv->active_nodes; iter != NULL; + iter = iter->next) { - pcmk_node_t *node = (pcmk_node_t *) gIter->data; + pcmk_node_t *node = iter->data; const char *cached = pcmk__btoa(node->details->online); rc = pe__name_and_nvpairs_xml(out, false, PCMK_XE_NODE, @@ -928,10 +930,10 @@ native_location(const pcmk_resource_t *rsc, GList **list, uint32_t target) if (rsc->priv->children != NULL) { - for (GList *gIter = rsc->priv->children; - gIter != NULL; gIter = gIter->next) { + for (GList *iter = rsc->priv->children; iter != NULL; + iter = iter->next) { - pcmk_resource_t *child = (pcmk_resource_t *) gIter->data; + pcmk_resource_t *child = iter->data; child->priv->fns->location(child, &result, target); } @@ -956,10 +958,8 @@ native_location(const pcmk_resource_t *rsc, GList **list, uint32_t target) } if (list) { - GList *gIter = result; - - for (; gIter != NULL; gIter = gIter->next) { - pcmk_node_t *node = (pcmk_node_t *) gIter->data; + for (GList *iter = result; iter != NULL; iter = iter->next) { + pcmk_node_t *node = iter->data; if ((*list == NULL) || (pe_find_node_id(*list, node->priv->id) == NULL)) { @@ -975,10 +975,8 @@ native_location(const pcmk_resource_t *rsc, GList **list, uint32_t target) static void get_rscs_brief(GList *rsc_list, GHashTable * rsc_table, GHashTable * active_table) { - GList *gIter = rsc_list; - - for (; gIter != NULL; gIter = gIter->next) { - pcmk_resource_t *rsc = (pcmk_resource_t *) gIter->data; + for (GList *iter = rsc_list; iter != NULL; iter = iter->next) { + pcmk_resource_t *rsc = iter->data; const char *class = pcmk__xe_get(rsc->priv->xml, PCMK_XA_CLASS); const char *kind = pcmk__xe_get(rsc->priv->xml, PCMK_XA_TYPE); @@ -1016,10 +1014,10 @@ get_rscs_brief(GList *rsc_list, GHashTable * rsc_table, GHashTable * active_tabl } if (active_table) { - for (GList *gIter2 = rsc->priv->active_nodes; - gIter2 != NULL; gIter2 = gIter2->next) { + for (GList *iter2 = rsc->priv->active_nodes; iter2 != NULL; + iter2 = iter2->next) { - pcmk_node_t *node = (pcmk_node_t *) gIter2->data; + pcmk_node_t *node = iter2->data; GHashTable *node_table = NULL; if (!node->details->unclean && !node->details->online @@ -1073,8 +1071,8 @@ pe__rscs_brief_output(pcmk__output_t *out, GList *rsc_list, uint32_t show_opts) sorted_rscs = g_hash_table_get_keys(rsc_table); sorted_rscs = g_list_sort(sorted_rscs, (GCompareFunc) strcmp); - for (GList *gIter = sorted_rscs; gIter; gIter = gIter->next) { - char *type = (char *) gIter->data; + for (GList *iter = sorted_rscs; iter != NULL; iter = iter->next) { + char *type = iter->data; int *rsc_counter = g_hash_table_lookup(rsc_table, type); GList *sorted_nodes = NULL; @@ -1087,8 +1085,8 @@ pe__rscs_brief_output(pcmk__output_t *out, GList *rsc_list, uint32_t show_opts) sorted_nodes = g_hash_table_get_keys(active_table); sorted_nodes = g_list_sort(sorted_nodes, (GCompareFunc) pcmk__numeric_strcasecmp); - for (GList *gIter2 = sorted_nodes; gIter2; gIter2 = gIter2->next) { - char *node_name = (char *) gIter2->data; + for (GList *iter2 = sorted_nodes; iter2 != NULL; iter2 = iter2->next) { + char *node_name = iter2->data; GHashTable *node_table = g_hash_table_lookup(active_table, node_name); int *active_counter = NULL; diff --git a/lib/pengine/pe_actions.c b/lib/pengine/pe_actions.c index 2e8d3237c62..00c72d02902 100644 --- a/lib/pengine/pe_actions.c +++ b/lib/pengine/pe_actions.c @@ -1158,8 +1158,8 @@ get_pseudo_op(const char *name, pcmk_scheduler_t *scheduler) static GList * find_unfencing_devices(GList *candidates, GList *matches) { - for (GList *gIter = candidates; gIter != NULL; gIter = gIter->next) { - pcmk_resource_t *candidate = gIter->data; + for (GList *iter = candidates; iter != NULL; iter = iter->next) { + pcmk_resource_t *candidate = iter->data; if (candidate->priv->children != NULL) { matches = find_unfencing_devices(candidate->priv->children, @@ -1188,7 +1188,6 @@ node_priority_fencing_delay(const pcmk_node_t *node, int online_count = 0; int top_priority = 0; int lowest_priority = 0; - GList *gIter = NULL; // PCMK_OPT_PRIORITY_FENCING_DELAY is disabled if (scheduler->priv->priority_fencing_ms == 0U) { @@ -1206,8 +1205,8 @@ node_priority_fencing_delay(const pcmk_node_t *node, return 0; } - for (gIter = scheduler->nodes; gIter != NULL; gIter = gIter->next) { - pcmk_node_t *n = gIter->data; + for (GList *iter = scheduler->nodes; iter != NULL; iter = iter->next) { + pcmk_node_t *n = iter->data; if (n->priv->variant != pcmk__node_variant_cluster) { continue; @@ -1281,8 +1280,8 @@ pe_fence_op(pcmk_node_t *node, const char *op, bool optional, GList *matches = find_unfencing_devices(scheduler->priv->resources, NULL); - for (GList *gIter = matches; gIter != NULL; gIter = gIter->next) { - pcmk_resource_t *match = gIter->data; + for (GList *iter = matches; iter != NULL; iter = iter->next) { + pcmk_resource_t *match = iter->data; const char *agent = g_hash_table_lookup(match->priv->meta, PCMK_XA_TYPE); pcmk__op_digest_t *data = NULL; @@ -1404,8 +1403,8 @@ find_first_action(const GList *input, const char *uuid, const char *task, { CRM_CHECK(uuid || task, return NULL); - for (const GList *gIter = input; gIter != NULL; gIter = gIter->next) { - pcmk_action_t *action = (pcmk_action_t *) gIter->data; + for (const GList *iter = input; iter != NULL; iter = iter->next) { + pcmk_action_t *action = iter->data; if (uuid != NULL && !pcmk__str_eq(uuid, action->uuid, pcmk__str_casei)) { continue; @@ -1430,13 +1429,12 @@ find_first_action(const GList *input, const char *uuid, const char *task, GList * find_actions(GList *input, const char *key, const pcmk_node_t *on_node) { - GList *gIter = input; GList *result = NULL; CRM_CHECK(key != NULL, return NULL); - for (; gIter != NULL; gIter = gIter->next) { - pcmk_action_t *action = (pcmk_action_t *) gIter->data; + for (GList *iter = input; iter != NULL; iter = iter->next) { + pcmk_action_t *action = iter->data; if (!pcmk__str_eq(key, action->uuid, pcmk__str_casei)) { continue; @@ -1473,8 +1471,8 @@ find_actions_exact(GList *input, const char *key, const pcmk_node_t *on_node) return NULL; } - for (GList *gIter = input; gIter != NULL; gIter = gIter->next) { - pcmk_action_t *action = (pcmk_action_t *) gIter->data; + for (GList *iter = input; iter != NULL; iter = iter->next) { + pcmk_action_t *action = iter->data; if ((action->node != NULL) && pcmk__str_eq(key, action->uuid, pcmk__str_casei) diff --git a/lib/pengine/pe_notif.c b/lib/pengine/pe_notif.c index a5f795407e6..5aac10a5d1d 100644 --- a/lib/pengine/pe_notif.c +++ b/lib/pengine/pe_notif.c @@ -211,8 +211,8 @@ notify_entries_to_strings(GList *list, GString **rsc_names, // Sort input list for user-friendliness (and ease of filtering duplicates) list = g_list_sort(list, compare_notify_entries); - for (GList *gIter = list; gIter != NULL; gIter = gIter->next) { - notify_entry_t *entry = (notify_entry_t *) gIter->data; + for (GList *iter = list; iter != NULL; iter = iter->next) { + notify_entry_t *entry = iter->data; // Entry must have a resource (with ID) CRM_LOG_ASSERT((entry != NULL) && (entry->rsc != NULL) diff --git a/lib/pengine/pe_output.c b/lib/pengine/pe_output.c index b351205fc75..815fd5649b9 100644 --- a/lib/pengine/pe_output.c +++ b/lib/pengine/pe_output.c @@ -89,10 +89,8 @@ add_extra_info(const pcmk_node_t *node, GList *rsc_list, pcmk_scheduler_t *scheduler, const char *attrname, int *expected_score) { - GList *gIter = NULL; - - for (gIter = rsc_list; gIter != NULL; gIter = gIter->next) { - pcmk_resource_t *rsc = (pcmk_resource_t *) gIter->data; + for (GList *iter = rsc_list; iter != NULL; iter = iter->next) { + pcmk_resource_t *rsc = iter->data; const char *type = g_hash_table_lookup(rsc->priv->meta, PCMK_XA_TYPE); const char *name = NULL; @@ -400,9 +398,11 @@ static bool is_mixed_version(pcmk_scheduler_t *scheduler) { const char *feature_set = NULL; - for (GList *gIter = scheduler->nodes; gIter != NULL; gIter = gIter->next) { - pcmk_node_t *node = gIter->data; + + for (GList *iter = scheduler->nodes; iter != NULL; iter = iter->next) { + pcmk_node_t *node = iter->data; const char *node_feature_set = get_node_feature_set(node); + if (node_feature_set != NULL) { if (feature_set == NULL) { feature_set = node_feature_set; @@ -751,13 +751,13 @@ ban_list(pcmk__output_t *out, va_list args) { uint32_t show_opts = va_arg(args, uint32_t); bool print_spacer = va_arg(args, int); - GList *gIter, *gIter2; int rc = pcmk_rc_no_output; /* Print each ban */ - for (gIter = scheduler->priv->location_constraints; - gIter != NULL; gIter = gIter->next) { - pcmk__location_t *location = gIter->data; + for (GList *iter = scheduler->priv->location_constraints; iter != NULL; + iter = iter->next) { + + pcmk__location_t *location = iter->data; const pcmk_resource_t *rsc = location->rsc; if (prefix != NULL && !g_str_has_prefix(location->id, prefix)) { @@ -771,8 +771,10 @@ ban_list(pcmk__output_t *out, va_list args) { continue; } - for (gIter2 = location->nodes; gIter2 != NULL; gIter2 = gIter2->next) { - pcmk_node_t *node = (pcmk_node_t *) gIter2->data; + for (GList *iter2 = location->nodes; iter2 != NULL; + iter2 = iter2->next) { + + pcmk_node_t *node = iter2->data; if (node->assign->score < 0) { PCMK__OUTPUT_LIST_HEADER(out, print_spacer, rc, "Negative Location Constraints"); @@ -2000,13 +2002,13 @@ node_text(pcmk__output_t *out, va_list args) { } } else { - GList *gIter2 = NULL; - out->begin_list(out, NULL, NULL, "%s", str->str); out->begin_list(out, NULL, NULL, "Resources"); - for (gIter2 = node->details->running_rsc; gIter2 != NULL; gIter2 = gIter2->next) { - pcmk_resource_t *rsc = (pcmk_resource_t *) gIter2->data; + for (GList *iter2 = node->details->running_rsc; iter2 != NULL; + iter2 = iter2->next) { + + pcmk_resource_t *rsc = iter2->data; show_opts |= pcmk_show_rsc_only; out->message(out, (const char *) rsc->priv->xml->name, @@ -2385,8 +2387,8 @@ node_attribute_list(pcmk__output_t *out, va_list args) { int rc = pcmk_rc_no_output; /* Display each node's attributes */ - for (GList *gIter = scheduler->nodes; gIter != NULL; gIter = gIter->next) { - pcmk_node_t *node = gIter->data; + for (GList *iter = scheduler->nodes; iter != NULL; iter = iter->next) { + pcmk_node_t *node = iter->data; GList *attr_list = NULL; GHashTableIter iter; @@ -2587,8 +2589,8 @@ node_list_html(pcmk__output_t *out, va_list args) { int rc = pcmk_rc_no_output; - for (GList *gIter = nodes; gIter != NULL; gIter = gIter->next) { - pcmk_node_t *node = (pcmk_node_t *) gIter->data; + for (GList *iter = nodes; iter != NULL; iter = iter->next) { + pcmk_node_t *node = iter->data; if (!pcmk__str_in_list(node->priv->name, only_node, pcmk__str_star_matches|pcmk__str_casei)) { @@ -2622,8 +2624,8 @@ node_list_text(pcmk__output_t *out, va_list args) { int rc = pcmk_rc_no_output; - for (GList *gIter = nodes; gIter != NULL; gIter = gIter->next) { - pcmk_node_t *node = (pcmk_node_t *) gIter->data; + for (GList *iter = nodes; iter != NULL; iter = iter->next) { + pcmk_node_t *node = iter->data; char *node_name = pe__node_display_name(node, pcmk__is_set(show_opts, pcmk_show_node_id)); @@ -2726,8 +2728,8 @@ node_list_xml(pcmk__output_t *out, va_list args) { * value of the list's PCMK_XA_NAME attribute. */ out->begin_list(out, NULL, NULL, PCMK_XE_NODES); - for (GList *gIter = nodes; gIter != NULL; gIter = gIter->next) { - pcmk_node_t *node = (pcmk_node_t *) gIter->data; + for (GList *iter = nodes; iter != NULL; iter = iter->next) { + pcmk_node_t *node = iter->data; if (!pcmk__str_in_list(node->priv->name, only_node, pcmk__str_star_matches|pcmk__str_casei)) { @@ -3193,12 +3195,11 @@ resource_operation_list(pcmk__output_t *out, va_list args) GList *op_list = va_arg(args, GList *); uint32_t show_opts = va_arg(args, uint32_t); - GList *gIter = NULL; int rc = pcmk_rc_no_output; /* Print each operation */ - for (gIter = op_list; gIter != NULL; gIter = gIter->next) { - xmlNode *xml_op = (xmlNode *) gIter->data; + for (GList *iter = op_list; iter != NULL; iter = iter->next) { + xmlNode *xml_op = iter->data; const char *task = pcmk__xe_get(xml_op, PCMK_XA_OPERATION); const char *interval_ms_s = pcmk__xe_get(xml_op, PCMK_META_INTERVAL); const char *op_rc = pcmk__xe_get(xml_op, PCMK__XA_RC_CODE); diff --git a/lib/pengine/remote.c b/lib/pengine/remote.c index 1e2a9ea608a..be3e0b6b2b9 100644 --- a/lib/pengine/remote.c +++ b/lib/pengine/remote.c @@ -43,10 +43,10 @@ pe__resource_contains_guest_node(const pcmk_scheduler_t *scheduler, if ((rsc != NULL) && (scheduler != NULL) && pcmk__is_set(scheduler->flags, pcmk__sched_have_remote_nodes)) { - for (GList *gIter = rsc->priv->launched; - gIter != NULL; gIter = gIter->next) { + for (GList *iter = rsc->priv->launched; iter != NULL; + iter = iter->next) { - pcmk_resource_t *launched = gIter->data; + pcmk_resource_t *launched = iter->data; if (pcmk__is_set(launched->flags, pcmk__rsc_is_remote_connection)) { return launched; diff --git a/lib/pengine/unpack.c b/lib/pengine/unpack.c index 4ff84c0e634..a6f5a3e2822 100644 --- a/lib/pengine/unpack.c +++ b/lib/pengine/unpack.c @@ -849,7 +849,6 @@ gboolean unpack_resources(const xmlNode *xml_resources, pcmk_scheduler_t *scheduler) { xmlNode *xml_obj = NULL; - GList *gIter = NULL; scheduler->priv->templates = pcmk__strkey_table(free, pcmk__free_idref); @@ -889,10 +888,10 @@ unpack_resources(const xmlNode *xml_resources, pcmk_scheduler_t *scheduler) pcmk__rsc_trace(new_rsc, "Added resource %s", new_rsc->id); } - for (gIter = scheduler->priv->resources; - gIter != NULL; gIter = gIter->next) { + for (GList *iter = scheduler->priv->resources; iter != NULL; + iter = iter->next) { - pcmk_resource_t *rsc = (pcmk_resource_t *) gIter->data; + pcmk_resource_t *rsc = iter->data; unpack_launcher(rsc, scheduler); link_rsc2remotenode(scheduler, rsc); @@ -1505,8 +1504,8 @@ unpack_status(xmlNode *status, pcmk_scheduler_t *scheduler) * we can stop connections for node shutdowns, and check the online status * of remote/guest nodes that didn't have any node history to unpack. */ - for (GList *gIter = scheduler->nodes; gIter != NULL; gIter = gIter->next) { - pcmk_node_t *this_node = gIter->data; + for (GList *iter = scheduler->nodes; iter != NULL; iter = iter->next) { + pcmk_node_t *this_node = iter->data; if (!pcmk__is_pacemaker_remote_node(this_node)) { continue; @@ -2628,10 +2627,9 @@ process_rsc_state(pcmk_resource_t *rsc, pcmk_node_t *node, } else { GList *possible_matches = pe__resource_actions(rsc, node, PCMK_ACTION_STOP, FALSE); - GList *gIter = possible_matches; - for (; gIter != NULL; gIter = gIter->next) { - pcmk_action_t *stop = (pcmk_action_t *) gIter->data; + for (GList *iter = possible_matches; iter != NULL; iter = iter->next) { + pcmk_action_t *stop = iter->data; pcmk__set_action_flags(stop, pcmk__action_optional); } @@ -2660,14 +2658,13 @@ process_recurring(pcmk_node_t *node, pcmk_resource_t *rsc, int counter = -1; const char *task = NULL; const char *status = NULL; - GList *gIter = sorted_op_list; pcmk__assert(rsc != NULL); pcmk__rsc_trace(rsc, "%s: Start index %d, stop index = %d", rsc->id, start_index, stop_index); - for (; gIter != NULL; gIter = gIter->next) { - xmlNode *rsc_op = (xmlNode *) gIter->data; + for (GList *iter = sorted_op_list; iter != NULL; iter = iter->next) { + xmlNode *rsc_op = iter->data; unsigned int interval_ms = 0; char *key = NULL; @@ -2807,7 +2804,6 @@ static pcmk_resource_t * unpack_lrm_resource(pcmk_node_t *node, const xmlNode *lrm_resource, pcmk_scheduler_t *scheduler) { - GList *gIter = NULL; int stop_index = -1; int start_index = -1; enum rsc_role_e req_role = pcmk_role_unknown; @@ -2872,8 +2868,8 @@ unpack_lrm_resource(pcmk_node_t *node, const xmlNode *lrm_resource, rsc->priv->orig_role = pcmk_role_unknown; sorted_op_list = g_list_sort(op_list, sort_op_by_callid); - for (gIter = sorted_op_list; gIter != NULL; gIter = gIter->next) { - xmlNode *rsc_op = (xmlNode *) gIter->data; + for (GList *iter = sorted_op_list; iter != NULL; iter = iter->next) { + xmlNode *rsc_op = iter->data; unpack_rsc_op(rsc, node, rsc_op, &last_failure, &on_fail); } @@ -5069,7 +5065,6 @@ extract_operations(const char *node, const char *rsc, xmlNode * rsc_entry, gbool xmlNode *rsc_op = NULL; - GList *gIter = NULL; GList *op_list = NULL; GList *sorted_op_list = NULL; @@ -5102,8 +5097,8 @@ extract_operations(const char *node, const char *rsc, xmlNode * rsc_entry, gbool calculate_active_ops(sorted_op_list, &start_index, &stop_index); - for (gIter = sorted_op_list; gIter != NULL; gIter = gIter->next) { - xmlNode *rsc_op = (xmlNode *) gIter->data; + for (GList *iter = sorted_op_list; iter != NULL; iter = iter->next) { + xmlNode *rsc_op = iter->data; counter++; diff --git a/lib/pengine/utils.c b/lib/pengine/utils.c index 37cdf3807dc..eb3558c261c 100644 --- a/lib/pengine/utils.c +++ b/lib/pengine/utils.c @@ -171,10 +171,10 @@ pe__node_list2table(const GList *list) GHashTable *result = NULL; result = pcmk__strkey_table(NULL, pcmk__free_node_copy); - for (const GList *gIter = list; gIter != NULL; gIter = gIter->next) { + for (const GList *iter = list; iter != NULL; iter = iter->next) { pcmk_node_t *new_node = NULL; - new_node = pe__copy_node((const pcmk_node_t *) gIter->data); + new_node = pe__copy_node((const pcmk_node_t *) iter->data); g_hash_table_insert(result, (void *) new_node->priv->id, new_node); } return result; @@ -235,8 +235,8 @@ pe__output_node_weights(const pcmk_resource_t *rsc, const char *comment, GList *list = g_list_sort(g_hash_table_get_values(nodes), pe__cmp_node_name); - for (const GList *gIter = list; gIter != NULL; gIter = gIter->next) { - const pcmk_node_t *node = (const pcmk_node_t *) gIter->data; + for (const GList *iter = list; iter != NULL; iter = iter->next) { + const pcmk_node_t *node = iter->data; out->message(out, "node-weight", rsc, comment, node->priv->name, pcmk_readable_score(node->assign->score)); @@ -324,10 +324,8 @@ pe__show_node_scores_as(const char *file, const char *function, int line, } // If this resource has children, repeat recursively for each - for (GList *gIter = rsc->priv->children; - gIter != NULL; gIter = gIter->next) { - - pcmk_resource_t *child = (pcmk_resource_t *) gIter->data; + for (GList *iter = rsc->priv->children; iter != NULL; iter = iter->next) { + pcmk_resource_t *child = iter->data; pe__show_node_scores_as(file, function, line, to_log, child, comment, child->priv->allowed_nodes, scheduler); @@ -388,10 +386,10 @@ resource_node_score(pcmk_resource_t *rsc, const pcmk_node_t *node, int score, return; } else { - for (GList *gIter = rsc->priv->children; - gIter != NULL; gIter = gIter->next) { + for (GList *iter = rsc->priv->children; iter != NULL; + iter = iter->next) { - pcmk_resource_t *child_rsc = (pcmk_resource_t *) gIter->data; + pcmk_resource_t *child_rsc = iter->data; resource_node_score(child_rsc, node, score, tag); } @@ -419,10 +417,8 @@ resource_location(pcmk_resource_t *rsc, const pcmk_node_t *node, int score, resource_node_score(rsc, node, score, tag); } else if (scheduler != NULL) { - GList *gIter = scheduler->nodes; - - for (; gIter != NULL; gIter = gIter->next) { - pcmk_node_t *node_iter = (pcmk_node_t *) gIter->data; + for (GList *iter = scheduler->nodes; iter != NULL; iter = iter->next) { + pcmk_node_t *node_iter = iter->data; resource_node_score(rsc, node_iter, score, tag); } @@ -498,7 +494,6 @@ get_target_role(const pcmk_resource_t *rsc, enum rsc_role_e *role) gboolean order_actions(pcmk_action_t *first, pcmk_action_t *then, uint32_t flags) { - GList *gIter = NULL; pcmk__related_action_t *wrapper = NULL; GList *list = NULL; @@ -517,9 +512,8 @@ order_actions(pcmk_action_t *first, pcmk_action_t *then, uint32_t flags) pcmk__assert(first != then); /* Filter dups, otherwise update_action_states() has too much work to do */ - gIter = first->actions_after; - for (; gIter != NULL; gIter = gIter->next) { - pcmk__related_action_t *after = gIter->data; + for (GList *iter = first->actions_after; iter != NULL; iter = iter->next) { + pcmk__related_action_t *after = iter->data; if ((after->action == then) && pcmk__any_flags_set(after->flags, flags)) { @@ -605,11 +599,10 @@ pe__clear_resource_flags_recursive(pcmk_resource_t *rsc, uint64_t flags) { pcmk__clear_rsc_flags(rsc, flags); - for (GList *gIter = rsc->priv->children; - gIter != NULL; gIter = gIter->next) { + for (GList *iter = rsc->priv->children; iter != NULL; iter = iter->next) { + pcmk_resource_t *child_rsc = iter->data; - pe__clear_resource_flags_recursive((pcmk_resource_t *) gIter->data, - flags); + pe__clear_resource_flags_recursive(child_rsc, flags); } } @@ -630,11 +623,10 @@ pe__set_resource_flags_recursive(pcmk_resource_t *rsc, uint64_t flags) { pcmk__set_rsc_flags(rsc, flags); - for (GList *gIter = rsc->priv->children; - gIter != NULL; gIter = gIter->next) { + for (GList *iter = rsc->priv->children; iter != NULL; iter = iter->next) { + pcmk_resource_t *child_rsc = iter->data; - pe__set_resource_flags_recursive((pcmk_resource_t *) gIter->data, - flags); + pe__set_resource_flags_recursive(child_rsc, flags); } } @@ -805,8 +797,8 @@ pe__filter_rsc_list(GList *rscs, GList *filter) { GList *retval = NULL; - for (GList *gIter = rscs; gIter; gIter = gIter->next) { - pcmk_resource_t *rsc = (pcmk_resource_t *) gIter->data; + for (GList *iter = rscs; iter != NULL; iter = iter->next) { + pcmk_resource_t *rsc = iter->data; /* I think the second condition is safe here for all callers of this * function. If not, it needs to move into pe__node_text. From 4a651b18b5dd197e40b867de8574c09349b3798f Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 17:25:29 -0700 Subject: [PATCH 28/75] Refactor: libpe_status: Drop strdup() within bundle.c We already use pcmk__assert_alloc() in several places, as well as GLib functions that allocate memory and assert on failure. Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index b4b93d7d900..f70e6652a84 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -310,7 +310,7 @@ allocate_ip(pe__bundle_variant_data_t *data, pcmk__bundle_replica_t *replica, replica->ipaddr = next_ip(data->ip_last); } else { - replica->ipaddr = strdup(data->ip_range_start); + replica->ipaddr = pcmk__str_copy(data->ip_range_start); } data->ip_last = replica->ipaddr; @@ -988,7 +988,7 @@ pe__unpack_bundle(pcmk_resource_t *rsc) bundle_data = pcmk__assert_alloc(1, sizeof(pe__bundle_variant_data_t)); rsc->priv->variant_opaque = bundle_data; - bundle_data->prefix = strdup(rsc->id); + bundle_data->prefix = pcmk__str_copy(rsc->id); xml_obj = pcmk__xe_first_child(rsc->priv->xml, PCMK_XE_DOCKER, NULL, NULL); @@ -1080,7 +1080,7 @@ pe__unpack_bundle(pcmk_resource_t *rsc) if(port->source != NULL && strlen(port->source) > 0) { if(port->target == NULL) { - port->target = strdup(port->source); + port->target = pcmk__str_copy(port->source); } bundle_data->ports = g_list_append(bundle_data->ports, port); @@ -1223,7 +1223,7 @@ pe__unpack_bundle(pcmk_resource_t *rsc) port = pcmk__assert_alloc(1, sizeof(pe__bundle_port_t)); if(bundle_data->control_port) { - port->source = strdup(bundle_data->control_port); + port->source = pcmk__str_copy(bundle_data->control_port); } else { /* If we wanted to respect PCMK_remote_port, we could use * crm_default_remote_port() here and elsewhere in this file instead @@ -1236,7 +1236,7 @@ pe__unpack_bundle(pcmk_resource_t *rsc) */ port->source = pcmk__itoa(DEFAULT_REMOTE_PORT); } - port->target = strdup(port->source); + port->target = pcmk__str_copy(port->source); bundle_data->ports = g_list_append(bundle_data->ports, port); buffer = g_string_sized_new(1024); From 92acc41a997ec86cd5a08193b40b2405cf45b623 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 17:35:05 -0700 Subject: [PATCH 29/75] Refactor: libpe_status: Functionize getting container XML for bundle Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 49 +++++++++++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 14 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index f70e6652a84..8da396c5925 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -967,6 +967,40 @@ pe__add_bundle_remote_name(pcmk_resource_t *rsc, xmlNode *xml, return node->priv->name; } +/*! + * \internal + * \brief Get container XML element from a bundle resource and set agent type + * + * On success, this sets the \c agent_type field of the resource's bundle + * private data. + * + * \param[in,out] rsc Bundle resource + * + * \return Child XML element for container within rsc->priv->xml + */ +static xmlNode * +get_container_xml(pcmk_resource_t *rsc) +{ + xmlNode *xml = NULL; + pe__bundle_variant_data_t *bundle_data = NULL; + + get_bundle_variant_data(bundle_data, rsc); + + xml = pcmk__xe_first_child(rsc->priv->xml, PCMK_XE_DOCKER, NULL, NULL); + if (xml != NULL) { + bundle_data->agent_type = PE__CONTAINER_AGENT_DOCKER; + return xml; + } + + xml = pcmk__xe_first_child(rsc->priv->xml, PCMK_XE_PODMAN, NULL, NULL); + if (xml != NULL) { + bundle_data->agent_type = PE__CONTAINER_AGENT_PODMAN; + return xml; + } + + return NULL; +} + #define pe__set_bundle_mount_flags(mount_xml, flags, flags_to_set) do { \ flags = pcmk__set_flags_as(__func__, __LINE__, LOG_TRACE, \ "Bundle mount", pcmk__xe_id(mount_xml), \ @@ -990,20 +1024,7 @@ pe__unpack_bundle(pcmk_resource_t *rsc) rsc->priv->variant_opaque = bundle_data; bundle_data->prefix = pcmk__str_copy(rsc->id); - xml_obj = pcmk__xe_first_child(rsc->priv->xml, PCMK_XE_DOCKER, NULL, - NULL); - if (xml_obj != NULL) { - bundle_data->agent_type = PE__CONTAINER_AGENT_DOCKER; - } - - if (xml_obj == NULL) { - xml_obj = pcmk__xe_first_child(rsc->priv->xml, PCMK_XE_PODMAN, NULL, - NULL); - if (xml_obj != NULL) { - bundle_data->agent_type = PE__CONTAINER_AGENT_PODMAN; - } - } - + xml_obj = get_container_xml(rsc); if (xml_obj == NULL) { return false; } From 4d5f8e24d68da8a8e4bcf58076f20262869280f8 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 18:05:56 -0700 Subject: [PATCH 30/75] Refactor: libpe_status: New unpack_bundle_container() Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 92 ++++++++++++++++++++++++++++---------------- 1 file changed, 58 insertions(+), 34 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index 8da396c5925..c83f303f286 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1001,6 +1001,63 @@ get_container_xml(pcmk_resource_t *rsc) return NULL; } +/*! + * \internal + * \brief Unpack a bundle resource's container child into bundle private data + * + * This does not create or unpack a container resource. + * + * \param[in,out] rsc Bundle resource + * + * \return Standard Pacemaker return code + */ +static int +unpack_bundle_container(pcmk_resource_t *rsc) +{ + xmlNode *xml = NULL; + pe__bundle_variant_data_t *bundle_data = NULL; + const char *value = NULL; + + xml = get_container_xml(rsc); + if (xml == NULL) { + return pcmk_rc_unpack_error; + } + + get_bundle_variant_data(bundle_data, rsc); + + // Use 0 for default, minimum, and invalid PCMK_XA_PROMOTED_MAX + value = pcmk__xe_get(xml, PCMK_XA_PROMOTED_MAX); + pcmk__scan_min_int(value, &bundle_data->promoted_max, 0); + + // Default nreplicas to PCMK_XA_PROMOTED_MAX if specified or 1 otherwise + value = pcmk__xe_get(xml, PCMK_XA_REPLICAS); + + if ((value == NULL) && (bundle_data->promoted_max > 0)) { + bundle_data->nreplicas = bundle_data->promoted_max; + + } else { + pcmk__scan_min_int(value, &bundle_data->nreplicas, 1); + } + + /* Communication between containers on the same host via the floating IPs + * works only if the container is started with: + * --userland-proxy=false --ip-masq=false + */ + value = pcmk__xe_get(xml, PCMK_XA_REPLICAS_PER_HOST); + pcmk__scan_min_int(value, &bundle_data->nreplicas_per_host, 1); + + if (bundle_data->nreplicas_per_host == 1) { + pcmk__clear_rsc_flags(rsc, pcmk__rsc_unique); + } + + bundle_data->container_command = pcmk__xe_get_copy(xml, + PCMK_XA_RUN_COMMAND); + bundle_data->launcher_options = pcmk__xe_get_copy(xml, PCMK_XA_OPTIONS); + bundle_data->image = pcmk__xe_get_copy(xml, PCMK_XA_IMAGE); + bundle_data->container_network = pcmk__xe_get_copy(xml, PCMK_XA_NETWORK); + return pcmk_rc_ok; +} + #define pe__set_bundle_mount_flags(mount_xml, flags, flags_to_set) do { \ flags = pcmk__set_flags_as(__func__, __LINE__, LOG_TRACE, \ "Bundle mount", pcmk__xe_id(mount_xml), \ @@ -1024,43 +1081,10 @@ pe__unpack_bundle(pcmk_resource_t *rsc) rsc->priv->variant_opaque = bundle_data; bundle_data->prefix = pcmk__str_copy(rsc->id); - xml_obj = get_container_xml(rsc); - if (xml_obj == NULL) { + if (unpack_bundle_container(rsc) != pcmk_rc_ok) { return false; } - // Use 0 for default, minimum, and invalid PCMK_XA_PROMOTED_MAX - value = pcmk__xe_get(xml_obj, PCMK_XA_PROMOTED_MAX); - pcmk__scan_min_int(value, &bundle_data->promoted_max, 0); - - /* Default replicas to PCMK_XA_PROMOTED_MAX if it was specified and 1 - * otherwise - */ - value = pcmk__xe_get(xml_obj, PCMK_XA_REPLICAS); - if ((value == NULL) && (bundle_data->promoted_max > 0)) { - bundle_data->nreplicas = bundle_data->promoted_max; - } else { - pcmk__scan_min_int(value, &bundle_data->nreplicas, 1); - } - - /* - * Communication between containers on the same host via the - * floating IPs only works if the container is started with: - * --userland-proxy=false --ip-masq=false - */ - value = pcmk__xe_get(xml_obj, PCMK_XA_REPLICAS_PER_HOST); - pcmk__scan_min_int(value, &bundle_data->nreplicas_per_host, 1); - if (bundle_data->nreplicas_per_host == 1) { - pcmk__clear_rsc_flags(rsc, pcmk__rsc_unique); - } - - bundle_data->container_command = pcmk__xe_get_copy(xml_obj, - PCMK_XA_RUN_COMMAND); - bundle_data->launcher_options = pcmk__xe_get_copy(xml_obj, PCMK_XA_OPTIONS); - bundle_data->image = pcmk__xe_get_copy(xml_obj, PCMK_XA_IMAGE); - bundle_data->container_network = pcmk__xe_get_copy(xml_obj, - PCMK_XA_NETWORK); - xml_obj = pcmk__xe_first_child(rsc->priv->xml, PCMK_XE_NETWORK, NULL, NULL); if(xml_obj) { From 052fbdf817712d30537f4a29aee79d22464026c3 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 18:27:43 -0700 Subject: [PATCH 31/75] Refactor: libpe_status: New unpack_bundle_network() Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 122 +++++++++++++++++++++++++------------------ 1 file changed, 70 insertions(+), 52 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index c83f303f286..cc9ab1bc4c6 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1058,6 +1058,75 @@ unpack_bundle_container(pcmk_resource_t *rsc) return pcmk_rc_ok; } +/*! + * \internal + * \brief Unpack a bundle resource's network child into bundle private data + * + * This does not create or unpack an IP resource. + * + * \param[in,out] rsc Bundle resource + */ +static void +unpack_bundle_network(pcmk_resource_t *rsc) +{ + xmlNode *xml = NULL; + pe__bundle_variant_data_t *bundle_data = NULL; + const char *value = NULL; + + xml = pcmk__xe_first_child(rsc->priv->xml, PCMK_XE_NETWORK, NULL, NULL); + if (xml == NULL) { + return; + } + + get_bundle_variant_data(bundle_data, rsc); + + bundle_data->ip_range_start = pcmk__xe_get_copy(xml, + PCMK_XA_IP_RANGE_START); + bundle_data->host_netmask = pcmk__xe_get_copy(xml, PCMK_XA_HOST_NETMASK); + bundle_data->host_network = pcmk__xe_get_copy(xml, PCMK_XA_HOST_INTERFACE); + bundle_data->control_port = pcmk__xe_get_copy(xml, PCMK_XA_CONTROL_PORT); + + value = pcmk__xe_get(xml, PCMK_XA_ADD_HOST); + if ((value == NULL) + || (pcmk__parse_bool(value, &bundle_data->add_host) != pcmk_rc_ok)) { + + // Default to true if unset or invaid + bundle_data->add_host = true; + } + + for (const xmlNode *mapping = pcmk__xe_first_child(xml, + PCMK_XE_PORT_MAPPING, + NULL, NULL); + mapping != NULL; + mapping = pcmk__xe_next(mapping, PCMK_XE_PORT_MAPPING)) { + + pe__bundle_port_t *port = pcmk__assert_alloc(1, + sizeof(pe__bundle_port_t)); + + port->source = pcmk__xe_get_copy(mapping, PCMK_XA_PORT); + + if (port->source == NULL) { + port->source = pcmk__xe_get_copy(mapping, PCMK_XA_RANGE); + + } else { + port->target = pcmk__xe_get_copy(mapping, PCMK_XA_INTERNAL_PORT); + } + + if (pcmk__str_empty(port->source)) { + pcmk__config_err("Invalid " PCMK_XA_PORT " directive %s", + pcmk__xe_id(mapping)); + port_free(port); + return; + } + + if (port->target == NULL) { + port->target = pcmk__str_copy(port->source); + } + + bundle_data->ports = g_list_append(bundle_data->ports, port); + } +} + #define pe__set_bundle_mount_flags(mount_xml, flags, flags_to_set) do { \ flags = pcmk__set_flags_as(__func__, __LINE__, LOG_TRACE, \ "Bundle mount", pcmk__xe_id(mount_xml), \ @@ -1067,7 +1136,6 @@ unpack_bundle_container(pcmk_resource_t *rsc) bool pe__unpack_bundle(pcmk_resource_t *rsc) { - const char *value = NULL; xmlNode *xml_obj = NULL; const xmlNode *xml_child = NULL; xmlNode *xml_resource = NULL; @@ -1085,57 +1153,7 @@ pe__unpack_bundle(pcmk_resource_t *rsc) return false; } - xml_obj = pcmk__xe_first_child(rsc->priv->xml, PCMK_XE_NETWORK, NULL, - NULL); - if(xml_obj) { - bundle_data->ip_range_start = pcmk__xe_get_copy(xml_obj, - PCMK_XA_IP_RANGE_START); - bundle_data->host_netmask = pcmk__xe_get_copy(xml_obj, - PCMK_XA_HOST_NETMASK); - bundle_data->host_network = pcmk__xe_get_copy(xml_obj, - PCMK_XA_HOST_INTERFACE); - bundle_data->control_port = pcmk__xe_get_copy(xml_obj, - PCMK_XA_CONTROL_PORT); - - value = pcmk__xe_get(xml_obj, PCMK_XA_ADD_HOST); - if ((value == NULL) - || (pcmk__parse_bool(value, - &bundle_data->add_host) != pcmk_rc_ok)) { - - // Default to true if unset or invaid - bundle_data->add_host = true; - } - - for (xml_child = pcmk__xe_first_child(xml_obj, PCMK_XE_PORT_MAPPING, - NULL, NULL); - xml_child != NULL; - xml_child = pcmk__xe_next(xml_child, PCMK_XE_PORT_MAPPING)) { - - pe__bundle_port_t *port = - pcmk__assert_alloc(1, sizeof(pe__bundle_port_t)); - - port->source = pcmk__xe_get_copy(xml_child, PCMK_XA_PORT); - - if(port->source == NULL) { - port->source = pcmk__xe_get_copy(xml_child, PCMK_XA_RANGE); - } else { - port->target = pcmk__xe_get_copy(xml_child, - PCMK_XA_INTERNAL_PORT); - } - - if(port->source != NULL && strlen(port->source) > 0) { - if(port->target == NULL) { - port->target = pcmk__str_copy(port->source); - } - bundle_data->ports = g_list_append(bundle_data->ports, port); - - } else { - pcmk__config_err("Invalid " PCMK_XA_PORT " directive %s", - pcmk__xe_id(xml_child)); - port_free(port); - } - } - } + unpack_bundle_network(rsc); xml_obj = pcmk__xe_first_child(rsc->priv->xml, PCMK_XE_STORAGE, NULL, NULL); From 3b4a239aa1c67de09502524637f6f44044665ca3 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 18:47:57 -0700 Subject: [PATCH 32/75] Refactor: libpe_status: New unpack_bundle_storage() Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 95 +++++++++++++++++++++++++++----------------- 1 file changed, 59 insertions(+), 36 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index cc9ab1bc4c6..17df307540e 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1127,20 +1127,70 @@ unpack_bundle_network(pcmk_resource_t *rsc) } } -#define pe__set_bundle_mount_flags(mount_xml, flags, flags_to_set) do { \ - flags = pcmk__set_flags_as(__func__, __LINE__, LOG_TRACE, \ - "Bundle mount", pcmk__xe_id(mount_xml), \ - flags, (flags_to_set), #flags_to_set); \ - } while (0) +/*! + * \internal + * \brief Unpack a bundle resource's storage child into bundle private data + * + * \param[in,out] rsc Bundle resource + * + * \return \c true if a mount with target \c "/var/log" was unpacked, or + * \c false otherwise + */ +static bool +unpack_bundle_storage(pcmk_resource_t *rsc) +{ + xmlNode *xml = NULL; + pe__bundle_variant_data_t *bundle_data = NULL; + bool have_log_mount = false; + + xml = pcmk__xe_first_child(rsc->priv->xml, PCMK_XE_STORAGE, NULL, NULL); + if (xml == NULL) { + return false; + } + + get_bundle_variant_data(bundle_data, rsc); + + for (const xmlNode *mapping = pcmk__xe_first_child(xml, + PCMK_XE_STORAGE_MAPPING, + NULL, NULL); + mapping != NULL; + mapping = pcmk__xe_next(mapping, PCMK_XE_STORAGE_MAPPING)) { + + const char *source = pcmk__xe_get(mapping, PCMK_XA_SOURCE_DIR); + const char *target = pcmk__xe_get(mapping, PCMK_XA_TARGET_DIR); + const char *options = pcmk__xe_get(mapping, PCMK_XA_OPTIONS); + uint32_t flags = pe__bundle_mount_none; + + if (source == NULL) { + source = pcmk__xe_get(mapping, PCMK_XA_SOURCE_DIR_ROOT); + flags = pcmk__set_flags_as(__func__, __LINE__, LOG_TRACE, + "Bundle mount", pcmk__xe_id(mapping), + flags, pe__bundle_mount_subdir, + "pe__bundle_mount_subdir"); + } + + if ((source == NULL) || (target == NULL)) { + pcmk__config_err("Invalid mount directive %s", + pcmk__xe_id(mapping)); + continue; + } + + mount_add(bundle_data, source, target, options, flags); + if (pcmk__str_eq(target, "/var/log", pcmk__str_none)) { + have_log_mount = true; + } + } + + return have_log_mount; +} bool pe__unpack_bundle(pcmk_resource_t *rsc) { xmlNode *xml_obj = NULL; - const xmlNode *xml_child = NULL; xmlNode *xml_resource = NULL; pe__bundle_variant_data_t *bundle_data = NULL; - bool need_log_mount = true; + bool have_log_mount = false; pcmk__assert(rsc != NULL); pcmk__rsc_trace(rsc, "Processing resource %s...", rsc->id); @@ -1155,34 +1205,7 @@ pe__unpack_bundle(pcmk_resource_t *rsc) unpack_bundle_network(rsc); - xml_obj = pcmk__xe_first_child(rsc->priv->xml, PCMK_XE_STORAGE, NULL, - NULL); - for (xml_child = pcmk__xe_first_child(xml_obj, PCMK_XE_STORAGE_MAPPING, - NULL, NULL); - xml_child != NULL; - xml_child = pcmk__xe_next(xml_child, PCMK_XE_STORAGE_MAPPING)) { - - const char *source = pcmk__xe_get(xml_child, PCMK_XA_SOURCE_DIR); - const char *target = pcmk__xe_get(xml_child, PCMK_XA_TARGET_DIR); - const char *options = pcmk__xe_get(xml_child, PCMK_XA_OPTIONS); - int flags = pe__bundle_mount_none; - - if (source == NULL) { - source = pcmk__xe_get(xml_child, PCMK_XA_SOURCE_DIR_ROOT); - pe__set_bundle_mount_flags(xml_child, flags, - pe__bundle_mount_subdir); - } - - if (source && target) { - mount_add(bundle_data, source, target, options, flags); - if (strcmp(target, "/var/log") == 0) { - need_log_mount = false; - } - } else { - pcmk__config_err("Invalid mount directive %s", - pcmk__xe_id(xml_child)); - } - } + have_log_mount = unpack_bundle_storage(rsc); xml_obj = pcmk__xe_first_child(rsc->priv->xml, PCMK_XE_PRIMITIVE, NULL, NULL); @@ -1279,7 +1302,7 @@ pe__unpack_bundle(pcmk_resource_t *rsc) mount_add(bundle_data, DEFAULT_REMOTE_KEY_LOCATION, DEFAULT_REMOTE_KEY_LOCATION, NULL, pe__bundle_mount_none); - if (need_log_mount) { + if (!have_log_mount) { mount_add(bundle_data, CRM_BUNDLE_DIR, "/var/log", NULL, pe__bundle_mount_subdir); } From dced077166d9b93222701cef916a1c6ed0a66744 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 19:02:50 -0700 Subject: [PATCH 33/75] Refactor: libpe_status: New unpack_bundle_primitive() Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 147 +++++++++++++++++++++++++------------------ 1 file changed, 86 insertions(+), 61 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index 17df307540e..409a432315c 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1184,95 +1184,120 @@ unpack_bundle_storage(pcmk_resource_t *rsc) return have_log_mount; } -bool -pe__unpack_bundle(pcmk_resource_t *rsc) +/*! + * \internal + * \brief Unpack a bundle resource's primitive child + * + * If a primitive child is found, this creates a \c PCMK_XE_CLONE element + * containing a copy of the primitive element and appropriate options. This does + * not unpack the created element. + * + * \param[in,out] rsc Bundle resource + * \param[out] clone_xml Where to store newly allocated \c PCMK_XE_CLONE + * + * \return Standard Pacemaker return code + * + * \note The caller is responsible for freeing \p *clone_xml using + * \c pcmk__xml_free(). + */ +static int +unpack_bundle_primitive(pcmk_resource_t *rsc, xmlNode **clone_xml) { - xmlNode *xml_obj = NULL; - xmlNode *xml_resource = NULL; + xmlNode *xml = NULL; pe__bundle_variant_data_t *bundle_data = NULL; - bool have_log_mount = false; + const char *suffix = NULL; + char *value = NULL; + xmlNode *xml_set = NULL; - pcmk__assert(rsc != NULL); - pcmk__rsc_trace(rsc, "Processing resource %s...", rsc->id); + xml = pcmk__xe_first_child(rsc->priv->xml, PCMK_XE_PRIMITIVE, NULL, NULL); + if (xml == NULL) { + return pcmk_rc_ok; + } - bundle_data = pcmk__assert_alloc(1, sizeof(pe__bundle_variant_data_t)); - rsc->priv->variant_opaque = bundle_data; - bundle_data->prefix = pcmk__str_copy(rsc->id); + get_bundle_variant_data(bundle_data, rsc); - if (unpack_bundle_container(rsc) != pcmk_rc_ok) { - return false; + if (!valid_network(bundle_data)) { + pcmk__config_err("Cannot control %s inside %s without either " + PCMK_XA_IP_RANGE_START " or " PCMK_XA_CONTROL_PORT, + rsc->id, pcmk__xe_id(xml)); + return pcmk_rc_unpack_error; } - unpack_bundle_network(rsc); + *clone_xml = pcmk__xe_create(NULL, PCMK_XE_CLONE); - have_log_mount = unpack_bundle_storage(rsc); + /* @COMPAT We no longer use the tag, but we need to keep it as part + * of the resource name, so that bundles don't restart in a rolling upgrade. + * (It also avoids needing to change regression tests.) + */ + suffix = (bundle_data->promoted_max > 0)? "master" : PCMK_XE_CLONE; + pcmk__xe_set_id(*clone_xml, "%s-%s", bundle_data->prefix, suffix); - xml_obj = pcmk__xe_first_child(rsc->priv->xml, PCMK_XE_PRIMITIVE, NULL, - NULL); - if (xml_obj != NULL) { - const char *suffix = NULL; - char *value = NULL; - xmlNode *xml_set = NULL; - - if (!valid_network(bundle_data)) { - pcmk__config_err("Cannot control %s inside %s without either " - PCMK_XA_IP_RANGE_START " or " PCMK_XA_CONTROL_PORT, - rsc->id, pcmk__xe_id(xml_obj)); - return false; - } + xml_set = pcmk__xe_create(*clone_xml, PCMK_XE_META_ATTRIBUTES); + pcmk__xe_set_id(xml_set, "%s-%s-meta", bundle_data->prefix, + (*clone_xml)->name); - xml_resource = pcmk__xe_create(NULL, PCMK_XE_CLONE); + crm_create_nvpair_xml(xml_set, NULL, PCMK_META_ORDERED, PCMK_VALUE_TRUE); - /* @COMPAT We no longer use the tag, but we need to keep it as - * part of the resource name, so that bundles don't restart in a rolling - * upgrade. (It also avoids needing to change regression tests.) - */ - suffix = (const char *) xml_resource->name; - if (bundle_data->promoted_max > 0) { - suffix = "master"; - } + value = pcmk__itoa(bundle_data->nreplicas); + crm_create_nvpair_xml(xml_set, NULL, PCMK_META_CLONE_MAX, value); + free(value); - pcmk__xe_set_id(xml_resource, "%s-%s", bundle_data->prefix, suffix); + value = pcmk__itoa(bundle_data->nreplicas_per_host); + crm_create_nvpair_xml(xml_set, NULL, PCMK_META_CLONE_NODE_MAX, value); + free(value); - xml_set = pcmk__xe_create(xml_resource, PCMK_XE_META_ATTRIBUTES); - pcmk__xe_set_id(xml_set, "%s-%s-meta", - bundle_data->prefix, xml_resource->name); + crm_create_nvpair_xml(xml_set, NULL, PCMK_META_GLOBALLY_UNIQUE, + pcmk__btoa(bundle_data->nreplicas_per_host > 1)); - crm_create_nvpair_xml(xml_set, NULL, - PCMK_META_ORDERED, PCMK_VALUE_TRUE); + if (bundle_data->promoted_max != 0) { + crm_create_nvpair_xml(xml_set, NULL, PCMK_META_PROMOTABLE, + PCMK_VALUE_TRUE); - value = pcmk__itoa(bundle_data->nreplicas); - crm_create_nvpair_xml(xml_set, NULL, PCMK_META_CLONE_MAX, value); + value = pcmk__itoa(bundle_data->promoted_max); + crm_create_nvpair_xml(xml_set, NULL, PCMK_META_PROMOTED_MAX, value); free(value); + } - value = pcmk__itoa(bundle_data->nreplicas_per_host); - crm_create_nvpair_xml(xml_set, NULL, PCMK_META_CLONE_NODE_MAX, value); - free(value); + //pcmk__xe_set(xml, PCMK_XA_ID, bundle_data->prefix); + pcmk__xml_copy(*clone_xml, xml); - crm_create_nvpair_xml(xml_set, NULL, PCMK_META_GLOBALLY_UNIQUE, - pcmk__btoa(bundle_data->nreplicas_per_host > 1)); + return pcmk_rc_ok; +} - if (bundle_data->promoted_max) { - crm_create_nvpair_xml(xml_set, NULL, - PCMK_META_PROMOTABLE, PCMK_VALUE_TRUE); +bool +pe__unpack_bundle(pcmk_resource_t *rsc) +{ + xmlNode *clone_xml = NULL; + pe__bundle_variant_data_t *bundle_data = NULL; + bool have_log_mount = false; - value = pcmk__itoa(bundle_data->promoted_max); - crm_create_nvpair_xml(xml_set, NULL, PCMK_META_PROMOTED_MAX, value); - free(value); - } + pcmk__assert(rsc != NULL); + pcmk__rsc_trace(rsc, "Processing resource %s...", rsc->id); + + bundle_data = pcmk__assert_alloc(1, sizeof(pe__bundle_variant_data_t)); + rsc->priv->variant_opaque = bundle_data; + bundle_data->prefix = pcmk__str_copy(rsc->id); + + if (unpack_bundle_container(rsc) != pcmk_rc_ok) { + return false; + } + + unpack_bundle_network(rsc); - //pcmk__xe_set(xml_obj, PCMK_XA_ID, bundle_data->prefix); - pcmk__xml_copy(xml_resource, xml_obj); + have_log_mount = unpack_bundle_storage(rsc); + + if (unpack_bundle_primitive(rsc, &clone_xml) != pcmk_rc_ok) { + return false; } - if(xml_resource) { + if (clone_xml != NULL) { int lpc = 0; pe__bundle_port_t *port = NULL; GString *buffer = NULL; - int rc = pe__unpack_resource(xml_resource, &bundle_data->child, rsc, + int rc = pe__unpack_resource(clone_xml, &bundle_data->child, rsc, rsc->priv->scheduler); - pcmk__xml_free(xml_resource); + pcmk__xml_free(clone_xml); if (rc != pcmk_rc_ok) { return false; From 0a9f5ececa3826bde0c2c8a54264b1233e8b26f8 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 19:10:28 -0700 Subject: [PATCH 34/75] Refactor: libpe_status: Use pcmk__xe_set_{int,bool} in unpack primitive Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index 409a432315c..77da272f549 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1206,8 +1206,8 @@ unpack_bundle_primitive(pcmk_resource_t *rsc, xmlNode **clone_xml) xmlNode *xml = NULL; pe__bundle_variant_data_t *bundle_data = NULL; const char *suffix = NULL; - char *value = NULL; xmlNode *xml_set = NULL; + xmlNode *nvpair = NULL; xml = pcmk__xe_first_child(rsc->priv->xml, PCMK_XE_PRIMITIVE, NULL, NULL); if (xml == NULL) { @@ -1238,24 +1238,25 @@ unpack_bundle_primitive(pcmk_resource_t *rsc, xmlNode **clone_xml) crm_create_nvpair_xml(xml_set, NULL, PCMK_META_ORDERED, PCMK_VALUE_TRUE); - value = pcmk__itoa(bundle_data->nreplicas); - crm_create_nvpair_xml(xml_set, NULL, PCMK_META_CLONE_MAX, value); - free(value); + nvpair = crm_create_nvpair_xml(xml_set, NULL, PCMK_META_CLONE_MAX, NULL); + pcmk__xe_set_int(nvpair, PCMK_XA_VALUE, bundle_data->nreplicas); - value = pcmk__itoa(bundle_data->nreplicas_per_host); - crm_create_nvpair_xml(xml_set, NULL, PCMK_META_CLONE_NODE_MAX, value); - free(value); + nvpair = crm_create_nvpair_xml(xml_set, NULL, PCMK_META_CLONE_NODE_MAX, + NULL); + pcmk__xe_set_int(nvpair, PCMK_XA_VALUE, bundle_data->nreplicas_per_host); - crm_create_nvpair_xml(xml_set, NULL, PCMK_META_GLOBALLY_UNIQUE, - pcmk__btoa(bundle_data->nreplicas_per_host > 1)); + nvpair = crm_create_nvpair_xml(xml_set, NULL, PCMK_META_GLOBALLY_UNIQUE, + NULL); + pcmk__xe_set_bool(nvpair, PCMK_XA_VALUE, + (bundle_data->nreplicas_per_host > 1)); if (bundle_data->promoted_max != 0) { crm_create_nvpair_xml(xml_set, NULL, PCMK_META_PROMOTABLE, PCMK_VALUE_TRUE); - value = pcmk__itoa(bundle_data->promoted_max); - crm_create_nvpair_xml(xml_set, NULL, PCMK_META_PROMOTED_MAX, value); - free(value); + nvpair = crm_create_nvpair_xml(xml_set, NULL, PCMK_META_PROMOTED_MAX, + NULL); + pcmk__xe_set_int(nvpair, PCMK_XA_VALUE, bundle_data->promoted_max); } //pcmk__xe_set(xml, PCMK_XA_ID, bundle_data->prefix); From 98850d1b001270d982b283ccf043def2ca88e72e Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 19:20:35 -0700 Subject: [PATCH 35/75] Refactor: libpe_status: Drop data arg from create_replica_resources() This is the bundle_data belonging to the parent argument (stored in parent->priv->variant_opaque). Also rename to bundle_data for consistency. Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 122 +++++++++++++++++++++++-------------------- 1 file changed, 66 insertions(+), 56 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index 77da272f549..3d9b15b4190 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -377,33 +377,37 @@ valid_network(pe__bundle_variant_data_t *data) } static int -create_ip_resource(pcmk_resource_t *parent, pe__bundle_variant_data_t *data, - pcmk__bundle_replica_t *replica) +create_ip_resource(pcmk_resource_t *parent, pcmk__bundle_replica_t *replica) { char *id = NULL; xmlNode *xml_ip = NULL; xmlNode *xml_obj = NULL; int rc = pcmk_rc_ok; + pe__bundle_variant_data_t *bundle_data = NULL; + + get_bundle_variant_data(bundle_data, parent); - if (data->ip_range_start == NULL) { + if (bundle_data->ip_range_start == NULL) { goto done; } - id = pcmk__assert_asprintf("%s-ip-%s", data->prefix, replica->ipaddr); + id = pcmk__assert_asprintf("%s-ip-%s", bundle_data->prefix, + replica->ipaddr); pcmk__xml_sanitize_id(id); xml_ip = create_resource(id, "heartbeat", "IPaddr2"); xml_obj = pcmk__xe_create(xml_ip, PCMK_XE_INSTANCE_ATTRIBUTES); - pcmk__xe_set_id(xml_obj, "%s-attributes-%d", data->prefix, replica->offset); + pcmk__xe_set_id(xml_obj, "%s-attributes-%d", bundle_data->prefix, + replica->offset); crm_create_nvpair_xml(xml_obj, NULL, "ip", replica->ipaddr); - if (data->host_network != NULL) { - crm_create_nvpair_xml(xml_obj, NULL, "nic", data->host_network); + if (bundle_data->host_network != NULL) { + crm_create_nvpair_xml(xml_obj, NULL, "nic", bundle_data->host_network); } crm_create_nvpair_xml(xml_obj, NULL, "cidr_netmask", - pcmk__s(data->host_netmask, "32")); + pcmk__s(bundle_data->host_netmask, "32")); xml_obj = pcmk__xe_create(xml_ip, PCMK_XE_OPERATIONS); crm_create_op_xml(xml_obj, id, PCMK_ACTION_MONITOR, "60s", NULL); @@ -438,7 +442,6 @@ container_agent_str(enum pe__container_agent t) static int create_container_resource(pcmk_resource_t *parent, - const pe__bundle_variant_data_t *data, pcmk__bundle_replica_t *replica) { char *id = NULL; @@ -448,31 +451,35 @@ create_container_resource(pcmk_resource_t *parent, GString *buffer = NULL; GString *dbuffer = NULL; int rc = pcmk_rc_ok; + const pe__bundle_variant_data_t *bundle_data = NULL; - if ((data->agent_type != PE__CONTAINER_AGENT_DOCKER) - && (data->agent_type != PE__CONTAINER_AGENT_PODMAN)) { + get_bundle_variant_data(bundle_data, parent); + + if ((bundle_data->agent_type != PE__CONTAINER_AGENT_DOCKER) + && (bundle_data->agent_type != PE__CONTAINER_AGENT_PODMAN)) { rc = pcmk_rc_unpack_error; goto done; } - agent_str = container_agent_str(data->agent_type); + agent_str = container_agent_str(bundle_data->agent_type); buffer = g_string_sized_new(4096); - id = pcmk__assert_asprintf("%s-%s-%d", data->prefix, agent_str, + id = pcmk__assert_asprintf("%s-%s-%d", bundle_data->prefix, agent_str, replica->offset); pcmk__xml_sanitize_id(id); xml_container = create_resource(id, "heartbeat", agent_str); xml_obj = pcmk__xe_create(xml_container, PCMK_XE_INSTANCE_ATTRIBUTES); - pcmk__xe_set_id(xml_obj, "%s-attributes-%d", data->prefix, replica->offset); + pcmk__xe_set_id(xml_obj, "%s-attributes-%d", bundle_data->prefix, + replica->offset); - crm_create_nvpair_xml(xml_obj, NULL, "image", data->image); + crm_create_nvpair_xml(xml_obj, NULL, "image", bundle_data->image); crm_create_nvpair_xml(xml_obj, NULL, "allow_pull", PCMK_VALUE_TRUE); crm_create_nvpair_xml(xml_obj, NULL, "force_kill", PCMK_VALUE_FALSE); crm_create_nvpair_xml(xml_obj, NULL, "reuse", PCMK_VALUE_FALSE); - if (data->agent_type == PE__CONTAINER_AGENT_DOCKER) { + if (bundle_data->agent_type == PE__CONTAINER_AGENT_DOCKER) { g_string_append(buffer, " --restart=no"); } @@ -481,32 +488,34 @@ create_container_resource(pcmk_resource_t *parent, * this makes applications happy who need their hostname to match the IP * they bind to. */ - if (data->ip_range_start != NULL) { - g_string_append_printf(buffer, " -h %s-%d", data->prefix, + if (bundle_data->ip_range_start != NULL) { + g_string_append_printf(buffer, " -h %s-%d", bundle_data->prefix, replica->offset); } g_string_append(buffer, " -e PCMK_stderr=1"); - if (data->container_network != NULL) { - pcmk__g_strcat(buffer, " --net=", data->container_network, NULL); + if (bundle_data->container_network != NULL) { + pcmk__g_strcat(buffer, " --net=", bundle_data->container_network, NULL); } - if (data->control_port != NULL) { + if (bundle_data->control_port != NULL) { pcmk__g_strcat(buffer, " -e PCMK_" PCMK__ENV_REMOTE_PORT "=", - data->control_port, NULL); + bundle_data->control_port, NULL); + } else { g_string_append_printf(buffer, " -e PCMK_" PCMK__ENV_REMOTE_PORT "=%d", DEFAULT_REMOTE_PORT); } - for (GList *iter = data->mounts; iter != NULL; iter = iter->next) { - pe__bundle_mount_t *mount = (pe__bundle_mount_t *) iter->data; + for (GList *iter = bundle_data->mounts; iter != NULL; iter = iter->next) { + pe__bundle_mount_t *mount = iter->data; char *source = NULL; if (pcmk__is_set(mount->flags, pe__bundle_mount_subdir)) { source = pcmk__assert_asprintf("%s/%s-%d", mount->source, - data->prefix, replica->offset); + bundle_data->prefix, + replica->offset); pcmk__add_separated_word(&dbuffer, 1024, source, ","); } @@ -520,15 +529,15 @@ create_container_resource(pcmk_resource_t *parent, free(source); } - for (GList *iter = data->ports; iter != NULL; iter = iter->next) { - pe__bundle_port_t *port = (pe__bundle_port_t *) iter->data; + for (GList *iter = bundle_data->ports; iter != NULL; iter = iter->next) { + pe__bundle_port_t *port = iter->data; if (replica->ipaddr != NULL) { pcmk__g_strcat(buffer, " -p ", replica->ipaddr, ":", port->source, ":", port->target, NULL); - } else if (!pcmk__str_eq(data->container_network, PCMK_VALUE_HOST, - pcmk__str_none)) { + } else if (!pcmk__str_eq(bundle_data->container_network, + PCMK_VALUE_HOST, pcmk__str_none)) { // No need to do port mapping if net == host pcmk__g_strcat(buffer, " -p ", port->source, ":", port->target, @@ -540,36 +549,36 @@ create_container_resource(pcmk_resource_t *parent, * it would cause restarts during rolling upgrades. * * In a previous version of the container resource creation logic, if - * data->launcher_options is not NULL, we append - * (" %s", data->launcher_options) even if data->launcher_options is an - * empty string. Likewise for data->container_host_options. Using + * bundle_data->launcher_options is not NULL, we append + * (" %s", bundle_data->launcher_options) even if + * bundle_data->launcher_options is an empty string. Likewise for + * bundle_data->container_host_options. Using * - * pcmk__add_word(buffer, 0, data->launcher_options) + * pcmk__add_word(buffer, 0, bundle_data->launcher_options) * * removes that extra trailing space, causing a resource definition change. */ - if (data->launcher_options != NULL) { - pcmk__g_strcat(buffer, " ", data->launcher_options, NULL); + if (bundle_data->launcher_options != NULL) { + pcmk__g_strcat(buffer, " ", bundle_data->launcher_options, NULL); } - if (data->container_host_options != NULL) { - pcmk__g_strcat(buffer, " ", data->container_host_options, NULL); + if (bundle_data->container_host_options != NULL) { + pcmk__g_strcat(buffer, " ", bundle_data->container_host_options, NULL); } - crm_create_nvpair_xml(xml_obj, NULL, "run_opts", - (const char *) buffer->str); + crm_create_nvpair_xml(xml_obj, NULL, "run_opts", buffer->str); g_string_free(buffer, TRUE); crm_create_nvpair_xml(xml_obj, NULL, "mount_points", - (dbuffer != NULL)? (const char *) dbuffer->str : ""); + (dbuffer != NULL)? dbuffer->str : ""); if (dbuffer != NULL) { g_string_free(dbuffer, TRUE); } if (replica->child != NULL) { - if (data->container_command != NULL) { + if (bundle_data->container_command != NULL) { crm_create_nvpair_xml(xml_obj, NULL, "run_cmd", - data->container_command); + bundle_data->container_command); } else { crm_create_nvpair_xml(xml_obj, NULL, "run_cmd", SBIN_DIR "/" PCMK__SERVER_REMOTED); @@ -588,16 +597,16 @@ create_container_resource(pcmk_resource_t *parent, * However, this would probably be better done via ACLs as with other * Pacemaker Remote nodes. */ - } else if ((child != NULL) && data->untrusted) { + } else if ((child != NULL) && bundle_data->untrusted) { crm_create_nvpair_xml(xml_obj, NULL, "run_cmd", CRM_DAEMON_DIR "/" PCMK__SERVER_EXECD); crm_create_nvpair_xml(xml_obj, NULL, "monitor_cmd", CRM_DAEMON_DIR "/pacemaker/cts-exec-helper -c poke"); #endif } else { - if (data->container_command != NULL) { + if (bundle_data->container_command != NULL) { crm_create_nvpair_xml(xml_obj, NULL, "run_cmd", - data->container_command); + bundle_data->container_command); } /* TODO: Allow users to specify their own? @@ -649,8 +658,7 @@ disallow_node(pcmk_resource_t *rsc, const char *uname) } static int -create_remote_resource(pcmk_resource_t *parent, pe__bundle_variant_data_t *data, - pcmk__bundle_replica_t *replica) +create_remote_resource(pcmk_resource_t *parent, pcmk__bundle_replica_t *replica) { GHashTableIter iter; pcmk_node_t *node = NULL; @@ -661,12 +669,15 @@ create_remote_resource(pcmk_resource_t *parent, pe__bundle_variant_data_t *data, const char *connect_name = NULL; pcmk_scheduler_t *scheduler = parent->priv->scheduler; int rc = pcmk_rc_ok; + pe__bundle_variant_data_t *bundle_data = NULL; + + get_bundle_variant_data(bundle_data, parent); - if ((replica->child == NULL) || !valid_network(data)) { + if ((replica->child == NULL) || !valid_network(bundle_data)) { goto done; } - id = pcmk__assert_asprintf("%s-%d", data->prefix, replica->offset); + id = pcmk__assert_asprintf("%s-%d", bundle_data->prefix, replica->offset); if (pe_find_resource(scheduler->priv->resources, id) != NULL) { free(id); @@ -690,10 +701,10 @@ create_remote_resource(pcmk_resource_t *parent, pe__bundle_variant_data_t *data, * that the bundle node is fenced by recovering the container, and that * remote should be ordered relative to the container. */ - if (data->control_port != NULL) { + if (bundle_data->control_port != NULL) { xml_remote = pe_create_remote_xml(NULL, id, replica->container->id, NULL, NULL, NULL, connect_name, - data->control_port); + bundle_data->control_port); } else { char *port_s = pcmk__itoa(DEFAULT_REMOTE_PORT); @@ -814,22 +825,21 @@ create_remote_resource(pcmk_resource_t *parent, pe__bundle_variant_data_t *data, static int create_replica_resources(pcmk_resource_t *parent, - pe__bundle_variant_data_t *data, pcmk__bundle_replica_t *replica) { int rc = pcmk_rc_ok; - rc = create_container_resource(parent, data, replica); + rc = create_container_resource(parent, replica); if (rc != pcmk_rc_ok) { return rc; } - rc = create_ip_resource(parent, data, replica); + rc = create_ip_resource(parent, replica); if (rc != pcmk_rc_ok) { return rc; } - rc = create_remote_resource(parent, data, replica); + rc = create_remote_resource(parent, replica); if (rc != pcmk_rc_ok) { return rc; } @@ -1406,7 +1416,7 @@ pe__unpack_bundle(pcmk_resource_t *rsc) for (GList *iter = bundle_data->replicas; iter != NULL; iter = iter->next) { pcmk__bundle_replica_t *replica = iter->data; - if (create_replica_resources(rsc, bundle_data, replica) != pcmk_rc_ok) { + if (create_replica_resources(rsc, replica) != pcmk_rc_ok) { pcmk__config_err("Failed unpacking resource %s", rsc->id); pcmk__free_resource(rsc); return false; From 903662bf0bf684381feb355560e9979643873dee Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 21:54:44 -0700 Subject: [PATCH 36/75] Refactor: libpe_status: Drop redundant valid_network() call If replica->child is not NULL, then clone_xml was not NULL in pe__unpack_bundle(). This implies that unpack_bundle_primitive() already called valid_network() and that it returned true. Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 66 ++++++++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index 3d9b15b4190..93333787b54 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -344,38 +344,6 @@ create_resource(const char *name, const char *provider, const char *kind) return rsc; } -/*! - * \internal - * \brief Check whether cluster can manage resource inside container - * - * \param[in,out] data Container variant data - * - * \return TRUE if networking configuration is acceptable, FALSE otherwise - * - * \note The resource is manageable if an IP range or control port has been - * specified. If a control port is used without an IP range, replicas per - * host must be 1. - */ -static bool -valid_network(pe__bundle_variant_data_t *data) -{ - if(data->ip_range_start) { - return TRUE; - } - if(data->control_port) { - if(data->nreplicas_per_host > 1) { - pcmk__config_err("Specifying the '" PCMK_XA_CONTROL_PORT "' for %s " - "requires '" PCMK_XA_REPLICAS_PER_HOST "=1'", - data->prefix); - data->nreplicas_per_host = 1; - // @TODO to be sure: - // pcmk__clear_rsc_flags(rsc, pcmk__rsc_unique); - } - return TRUE; - } - return FALSE; -} - static int create_ip_resource(pcmk_resource_t *parent, pcmk__bundle_replica_t *replica) { @@ -673,7 +641,7 @@ create_remote_resource(pcmk_resource_t *parent, pcmk__bundle_replica_t *replica) get_bundle_variant_data(bundle_data, parent); - if ((replica->child == NULL) || !valid_network(bundle_data)) { + if (replica->child == NULL) { goto done; } @@ -1194,6 +1162,38 @@ unpack_bundle_storage(pcmk_resource_t *rsc) return have_log_mount; } +/*! + * \internal + * \brief Check whether cluster can manage resource inside container + * + * \param[in,out] data Container variant data + * + * \return TRUE if networking configuration is acceptable, FALSE otherwise + * + * \note The resource is manageable if an IP range or control port has been + * specified. If a control port is used without an IP range, replicas per + * host must be 1. + */ +static bool +valid_network(pe__bundle_variant_data_t *data) +{ + if(data->ip_range_start) { + return TRUE; + } + if(data->control_port) { + if(data->nreplicas_per_host > 1) { + pcmk__config_err("Specifying the '" PCMK_XA_CONTROL_PORT "' for %s " + "requires '" PCMK_XA_REPLICAS_PER_HOST "=1'", + data->prefix); + data->nreplicas_per_host = 1; + // @TODO to be sure: + // pcmk__clear_rsc_flags(rsc, pcmk__rsc_unique); + } + return TRUE; + } + return FALSE; +} + /*! * \internal * \brief Unpack a bundle resource's primitive child From b7cbeb69cc8b19883dfd5dd2916524ef6b2e7ee3 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 22:16:56 -0700 Subject: [PATCH 37/75] Refactor: libpe_status: Clean up valid_network() Also rename it and have it take a pcmk_resource_t * argument. Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 55 +++++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 24 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index 93333787b54..b3e8c22fa6a 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1164,34 +1164,41 @@ unpack_bundle_storage(pcmk_resource_t *rsc) /*! * \internal - * \brief Check whether cluster can manage resource inside container + * \brief Validate a bundle's networking configuration for running a primitive * - * \param[in,out] data Container variant data + * The networking configuration is considered valid if an IP range or control + * port has been specified. If a control port is used without an IP range, + * replicas per host must be 1. * - * \return TRUE if networking configuration is acceptable, FALSE otherwise + * \param[in,out] rsc Bundle resource * - * \note The resource is manageable if an IP range or control port has been - * specified. If a control port is used without an IP range, replicas per - * host must be 1. + * \return \c true if the configuration is valid, or \c false otherwise */ static bool -valid_network(pe__bundle_variant_data_t *data) +valid_network_for_primitive(pcmk_resource_t *rsc) { - if(data->ip_range_start) { - return TRUE; - } - if(data->control_port) { - if(data->nreplicas_per_host > 1) { - pcmk__config_err("Specifying the '" PCMK_XA_CONTROL_PORT "' for %s " - "requires '" PCMK_XA_REPLICAS_PER_HOST "=1'", - data->prefix); - data->nreplicas_per_host = 1; - // @TODO to be sure: - // pcmk__clear_rsc_flags(rsc, pcmk__rsc_unique); - } - return TRUE; + pe__bundle_variant_data_t *bundle_data = NULL; + + get_bundle_variant_data(bundle_data, rsc); + + if (bundle_data->ip_range_start != NULL) { + return true; + } + + if (bundle_data->control_port == NULL) { + return false; + } + + if (bundle_data->nreplicas_per_host > 1) { + pcmk__config_warn("Using " PCMK_XA_REPLICAS_PER_HOST "=1 for %s " + "because specifying " PCMK_XA_CONTROL_PORT " " + "requires it", bundle_data->prefix); + bundle_data->nreplicas_per_host = 1; + // @TODO to be sure: + // pcmk__clear_rsc_flags(rsc, pcmk__rsc_unique); } - return FALSE; + + return true; } /*! @@ -1224,15 +1231,15 @@ unpack_bundle_primitive(pcmk_resource_t *rsc, xmlNode **clone_xml) return pcmk_rc_ok; } - get_bundle_variant_data(bundle_data, rsc); - - if (!valid_network(bundle_data)) { + if (!valid_network_for_primitive(rsc)) { pcmk__config_err("Cannot control %s inside %s without either " PCMK_XA_IP_RANGE_START " or " PCMK_XA_CONTROL_PORT, rsc->id, pcmk__xe_id(xml)); return pcmk_rc_unpack_error; } + get_bundle_variant_data(bundle_data, rsc); + *clone_xml = pcmk__xe_create(NULL, PCMK_XE_CLONE); /* @COMPAT We no longer use the tag, but we need to keep it as part From 5106391a9b91036116754780e42f04133f5ae15c Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 19:12:43 -0700 Subject: [PATCH 38/75] Low: libpe_status: Clear unique flag on setting nreplicas_per_host to 1 We're already doing this in unpack_bundle_container(), so it seems reasonable to do it in valid_network() if we have to reset nreplicas_per_host to 1 there. Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index b3e8c22fa6a..f47b600d404 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1194,8 +1194,7 @@ valid_network_for_primitive(pcmk_resource_t *rsc) "because specifying " PCMK_XA_CONTROL_PORT " " "requires it", bundle_data->prefix); bundle_data->nreplicas_per_host = 1; - // @TODO to be sure: - // pcmk__clear_rsc_flags(rsc, pcmk__rsc_unique); + pcmk__clear_rsc_flags(rsc, pcmk__rsc_unique); } return true; From 272bfe71f6b4b93cde0c9f81a42bbb6d4bb9a9f8 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 22:19:12 -0700 Subject: [PATCH 39/75] Refactor: libpe_status: Set globally-unique based on rsc unique flag The two conditions are equivalent now, and this is clearer. Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index f47b600d404..e3c757c4b1d 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1264,7 +1264,7 @@ unpack_bundle_primitive(pcmk_resource_t *rsc, xmlNode **clone_xml) nvpair = crm_create_nvpair_xml(xml_set, NULL, PCMK_META_GLOBALLY_UNIQUE, NULL); pcmk__xe_set_bool(nvpair, PCMK_XA_VALUE, - (bundle_data->nreplicas_per_host > 1)); + pcmk__is_set(rsc->flags, pcmk__rsc_unique)); if (bundle_data->promoted_max != 0) { crm_create_nvpair_xml(xml_set, NULL, PCMK_META_PROMOTABLE, From 6afd8db99c39e69614159580fd5d7f56559cb962 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 22:26:01 -0700 Subject: [PATCH 40/75] Refactor: libpe_status: Rename variables in unpack_bundle_primitive() For what I perceive to be clarity. Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index e3c757c4b1d..3c182828944 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1219,21 +1219,22 @@ valid_network_for_primitive(pcmk_resource_t *rsc) static int unpack_bundle_primitive(pcmk_resource_t *rsc, xmlNode **clone_xml) { - xmlNode *xml = NULL; + xmlNode *primitive_xml = NULL; pe__bundle_variant_data_t *bundle_data = NULL; const char *suffix = NULL; - xmlNode *xml_set = NULL; + xmlNode *meta = NULL; xmlNode *nvpair = NULL; - xml = pcmk__xe_first_child(rsc->priv->xml, PCMK_XE_PRIMITIVE, NULL, NULL); - if (xml == NULL) { + primitive_xml = pcmk__xe_first_child(rsc->priv->xml, PCMK_XE_PRIMITIVE, + NULL, NULL); + if (primitive_xml == NULL) { return pcmk_rc_ok; } if (!valid_network_for_primitive(rsc)) { pcmk__config_err("Cannot control %s inside %s without either " PCMK_XA_IP_RANGE_START " or " PCMK_XA_CONTROL_PORT, - rsc->id, pcmk__xe_id(xml)); + rsc->id, pcmk__xe_id(primitive_xml)); return pcmk_rc_unpack_error; } @@ -1248,35 +1249,33 @@ unpack_bundle_primitive(pcmk_resource_t *rsc, xmlNode **clone_xml) suffix = (bundle_data->promoted_max > 0)? "master" : PCMK_XE_CLONE; pcmk__xe_set_id(*clone_xml, "%s-%s", bundle_data->prefix, suffix); - xml_set = pcmk__xe_create(*clone_xml, PCMK_XE_META_ATTRIBUTES); - pcmk__xe_set_id(xml_set, "%s-%s-meta", bundle_data->prefix, + meta = pcmk__xe_create(*clone_xml, PCMK_XE_META_ATTRIBUTES); + pcmk__xe_set_id(meta, "%s-%s-meta", bundle_data->prefix, (*clone_xml)->name); - crm_create_nvpair_xml(xml_set, NULL, PCMK_META_ORDERED, PCMK_VALUE_TRUE); + crm_create_nvpair_xml(meta, NULL, PCMK_META_ORDERED, PCMK_VALUE_TRUE); - nvpair = crm_create_nvpair_xml(xml_set, NULL, PCMK_META_CLONE_MAX, NULL); + nvpair = crm_create_nvpair_xml(meta, NULL, PCMK_META_CLONE_MAX, NULL); pcmk__xe_set_int(nvpair, PCMK_XA_VALUE, bundle_data->nreplicas); - nvpair = crm_create_nvpair_xml(xml_set, NULL, PCMK_META_CLONE_NODE_MAX, - NULL); + nvpair = crm_create_nvpair_xml(meta, NULL, PCMK_META_CLONE_NODE_MAX, NULL); pcmk__xe_set_int(nvpair, PCMK_XA_VALUE, bundle_data->nreplicas_per_host); - nvpair = crm_create_nvpair_xml(xml_set, NULL, PCMK_META_GLOBALLY_UNIQUE, - NULL); + nvpair = crm_create_nvpair_xml(meta, NULL, PCMK_META_GLOBALLY_UNIQUE, NULL); pcmk__xe_set_bool(nvpair, PCMK_XA_VALUE, pcmk__is_set(rsc->flags, pcmk__rsc_unique)); if (bundle_data->promoted_max != 0) { - crm_create_nvpair_xml(xml_set, NULL, PCMK_META_PROMOTABLE, + crm_create_nvpair_xml(meta, NULL, PCMK_META_PROMOTABLE, PCMK_VALUE_TRUE); - nvpair = crm_create_nvpair_xml(xml_set, NULL, PCMK_META_PROMOTED_MAX, + nvpair = crm_create_nvpair_xml(meta, NULL, PCMK_META_PROMOTED_MAX, NULL); pcmk__xe_set_int(nvpair, PCMK_XA_VALUE, bundle_data->promoted_max); } - //pcmk__xe_set(xml, PCMK_XA_ID, bundle_data->prefix); - pcmk__xml_copy(*clone_xml, xml); + //pcmk__xe_set(primitive_xml, PCMK_XA_ID, bundle_data->prefix); + pcmk__xml_copy(*clone_xml, primitive_xml); return pcmk_rc_ok; } From 600067ba49e541f58d1b0ccb8156340fbc70afb2 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 22:50:20 -0700 Subject: [PATCH 41/75] Refactor: libpe_status: New create_child_resource() in bundle.c Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 196 +++++++++++++++++++++++-------------------- 1 file changed, 107 insertions(+), 89 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index 3c182828944..0484cb92e75 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1280,6 +1280,112 @@ unpack_bundle_primitive(pcmk_resource_t *rsc, xmlNode **clone_xml) return pcmk_rc_ok; } +static int +create_child_resource(pcmk_resource_t *rsc, xmlNode *clone_xml, + bool have_log_mount) +{ + int offset = 0; + pe__bundle_port_t *port = NULL; + GString *buffer = NULL; + int rc = pcmk_rc_ok; + pe__bundle_variant_data_t *bundle_data = NULL; + + get_bundle_variant_data(bundle_data, rsc); + + rc = pe__unpack_resource(clone_xml, &bundle_data->child, rsc, + rsc->priv->scheduler); + if (rc != pcmk_rc_ok) { + return rc; + } + + /* Currently, we always map the default authentication key location into the + * same location inside the container. + * + * Ideally, we would respect the host's PCMK_authkey_location, but: + * - it may be different on different nodes; + * - the actual connection will do extra checking to make sure the key file + * exists and is readable, that we can't do here on the DC + * - tools such as crm_resource and crm_simulate may not have the same + * environment variables as the cluster, causing operation digests to + * differ + * + * Always using the default location inside the container is fine, because + * we control the pacemaker_remote environment, and it avoids having to pass + * another environment variable to the container. + * + * @TODO A better solution may be to have only pacemaker_remote use the + * environment variable, and have the cluster nodes use a new cluster option + * for key location. This would introduce the limitation of the location + * being the same on all cluster nodes, but that's reasonable. + */ + mount_add(bundle_data, DEFAULT_REMOTE_KEY_LOCATION, + DEFAULT_REMOTE_KEY_LOCATION, NULL, pe__bundle_mount_none); + + if (!have_log_mount) { + mount_add(bundle_data, CRM_BUNDLE_DIR, "/var/log", NULL, + pe__bundle_mount_subdir); + } + + port = pcmk__assert_alloc(1, sizeof(pe__bundle_port_t)); + + if (bundle_data->control_port != NULL) { + port->source = pcmk__str_copy(bundle_data->control_port); + + } else { + /* If we wanted to respect PCMK_remote_port, we could use + * crm_default_remote_port() here and elsewhere in this file instead of + * DEFAULT_REMOTE_PORT. + * + * However, it gains nothing, since we control both the container + * environment and the connection resource parameters, and the user can + * use a different port if desired by setting PCMK_XA_CONTROL_PORT. + */ + port->source = pcmk__itoa(DEFAULT_REMOTE_PORT); + } + + port->target = pcmk__str_copy(port->source); + bundle_data->ports = g_list_append(bundle_data->ports, port); + buffer = g_string_sized_new(1024); + + for (GList *iter = bundle_data->child->priv->children; iter != NULL; + iter = iter->next) { + + pcmk__bundle_replica_t *replica = + pcmk__assert_alloc(1, sizeof(pcmk__bundle_replica_t)); + + replica->child = iter->data; + pcmk__set_rsc_flags(replica->child, pcmk__rsc_exclusive_probes); + replica->offset = offset++; + + /* Ensure the child's notify gets set based on the underlying + * primitive's value + */ + if (pcmk__is_set(replica->child->flags, pcmk__rsc_notify)) { + pcmk__set_rsc_flags(bundle_data->child, pcmk__rsc_notify); + } + + allocate_ip(bundle_data, replica, buffer); + bundle_data->replicas = g_list_append(bundle_data->replicas, replica); + + // coverity[null_field : FALSE] replica->child can't be NULL here + bundle_data->attribute_target = + g_hash_table_lookup(replica->child->priv->meta, + PCMK_META_CONTAINER_ATTRIBUTE_TARGET); + } + + bundle_data->container_host_options = g_string_free(buffer, false); + + if (bundle_data->attribute_target != NULL) { + pcmk__insert_dup(rsc->priv->meta, PCMK_META_CONTAINER_ATTRIBUTE_TARGET, + bundle_data->attribute_target); + pcmk__insert_dup(bundle_data->child->priv->meta, + PCMK_META_CONTAINER_ATTRIBUTE_TARGET, + bundle_data->attribute_target); + } + + return pcmk_rc_ok; +} + bool pe__unpack_bundle(pcmk_resource_t *rsc) { @@ -1307,101 +1413,13 @@ pe__unpack_bundle(pcmk_resource_t *rsc) } if (clone_xml != NULL) { - int lpc = 0; - pe__bundle_port_t *port = NULL; - GString *buffer = NULL; - int rc = pe__unpack_resource(clone_xml, &bundle_data->child, rsc, - rsc->priv->scheduler); + int rc = create_child_resource(rsc, clone_xml, have_log_mount); pcmk__xml_free(clone_xml); - if (rc != pcmk_rc_ok) { return false; } - /* Currently, we always map the default authentication key location - * into the same location inside the container. - * - * Ideally, we would respect the host's PCMK_authkey_location, but: - * - it may be different on different nodes; - * - the actual connection will do extra checking to make sure the key - * file exists and is readable, that we can't do here on the DC - * - tools such as crm_resource and crm_simulate may not have the same - * environment variables as the cluster, causing operation digests to - * differ - * - * Always using the default location inside the container is fine, - * because we control the pacemaker_remote environment, and it avoids - * having to pass another environment variable to the container. - * - * @TODO A better solution may be to have only pacemaker_remote use the - * environment variable, and have the cluster nodes use a new - * cluster option for key location. This would introduce the limitation - * of the location being the same on all cluster nodes, but that's - * reasonable. - */ - mount_add(bundle_data, DEFAULT_REMOTE_KEY_LOCATION, - DEFAULT_REMOTE_KEY_LOCATION, NULL, pe__bundle_mount_none); - - if (!have_log_mount) { - mount_add(bundle_data, CRM_BUNDLE_DIR, "/var/log", NULL, - pe__bundle_mount_subdir); - } - - port = pcmk__assert_alloc(1, sizeof(pe__bundle_port_t)); - if(bundle_data->control_port) { - port->source = pcmk__str_copy(bundle_data->control_port); - } else { - /* If we wanted to respect PCMK_remote_port, we could use - * crm_default_remote_port() here and elsewhere in this file instead - * of DEFAULT_REMOTE_PORT. - * - * However, it gains nothing, since we control both the container - * environment and the connection resource parameters, and the user - * can use a different port if desired by setting - * PCMK_XA_CONTROL_PORT. - */ - port->source = pcmk__itoa(DEFAULT_REMOTE_PORT); - } - port->target = pcmk__str_copy(port->source); - bundle_data->ports = g_list_append(bundle_data->ports, port); - - buffer = g_string_sized_new(1024); - for (GList *iter = bundle_data->child->priv->children; - iter != NULL; iter = iter->next) { - - pcmk__bundle_replica_t *replica = NULL; - - replica = pcmk__assert_alloc(1, sizeof(pcmk__bundle_replica_t)); - replica->child = iter->data; - pcmk__set_rsc_flags(replica->child, pcmk__rsc_exclusive_probes); - replica->offset = lpc++; - - // Ensure the child's notify gets set based on the underlying primitive's value - if (pcmk__is_set(replica->child->flags, pcmk__rsc_notify)) { - pcmk__set_rsc_flags(bundle_data->child, pcmk__rsc_notify); - } - - allocate_ip(bundle_data, replica, buffer); - bundle_data->replicas = g_list_append(bundle_data->replicas, - replica); - - // coverity[null_field : FALSE] replica->child can't be NULL here - bundle_data->attribute_target = - g_hash_table_lookup(replica->child->priv->meta, - PCMK_META_CONTAINER_ATTRIBUTE_TARGET); - } - bundle_data->container_host_options = g_string_free(buffer, false); - - if (bundle_data->attribute_target) { - pcmk__insert_dup(rsc->priv->meta, - PCMK_META_CONTAINER_ATTRIBUTE_TARGET, - bundle_data->attribute_target); - pcmk__insert_dup(bundle_data->child->priv->meta, - PCMK_META_CONTAINER_ATTRIBUTE_TARGET, - bundle_data->attribute_target); - } - } else { // Just a naked container, no pacemaker-remote GString *buffer = g_string_sized_new(1024); From 7508e193682351aea032b41344f14f767a1925f7 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 22:54:00 -0700 Subject: [PATCH 42/75] Refactor: libpe_status: Drop most uses of bundle_data->prefix It gets set to a copy of rsc->id at the beginning of pe__unpack_bundle, and then it's never changed. So we can use the bundle resource's ID instead. Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index 0484cb92e75..565b3772f9a 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -359,14 +359,12 @@ create_ip_resource(pcmk_resource_t *parent, pcmk__bundle_replica_t *replica) goto done; } - id = pcmk__assert_asprintf("%s-ip-%s", bundle_data->prefix, - replica->ipaddr); + id = pcmk__assert_asprintf("%s-ip-%s", parent->id, replica->ipaddr); pcmk__xml_sanitize_id(id); xml_ip = create_resource(id, "heartbeat", "IPaddr2"); xml_obj = pcmk__xe_create(xml_ip, PCMK_XE_INSTANCE_ATTRIBUTES); - pcmk__xe_set_id(xml_obj, "%s-attributes-%d", bundle_data->prefix, - replica->offset); + pcmk__xe_set_id(xml_obj, "%s-attributes-%d", parent->id, replica->offset); crm_create_nvpair_xml(xml_obj, NULL, "ip", replica->ipaddr); @@ -433,14 +431,13 @@ create_container_resource(pcmk_resource_t *parent, agent_str = container_agent_str(bundle_data->agent_type); buffer = g_string_sized_new(4096); - id = pcmk__assert_asprintf("%s-%s-%d", bundle_data->prefix, agent_str, + id = pcmk__assert_asprintf("%s-%s-%d", parent->id, agent_str, replica->offset); pcmk__xml_sanitize_id(id); xml_container = create_resource(id, "heartbeat", agent_str); xml_obj = pcmk__xe_create(xml_container, PCMK_XE_INSTANCE_ATTRIBUTES); - pcmk__xe_set_id(xml_obj, "%s-attributes-%d", bundle_data->prefix, - replica->offset); + pcmk__xe_set_id(xml_obj, "%s-attributes-%d", parent->id, replica->offset); crm_create_nvpair_xml(xml_obj, NULL, "image", bundle_data->image); crm_create_nvpair_xml(xml_obj, NULL, "allow_pull", PCMK_VALUE_TRUE); @@ -457,7 +454,7 @@ create_container_resource(pcmk_resource_t *parent, * they bind to. */ if (bundle_data->ip_range_start != NULL) { - g_string_append_printf(buffer, " -h %s-%d", bundle_data->prefix, + g_string_append_printf(buffer, " -h %s-%d", parent->id, replica->offset); } @@ -482,8 +479,7 @@ create_container_resource(pcmk_resource_t *parent, if (pcmk__is_set(mount->flags, pe__bundle_mount_subdir)) { source = pcmk__assert_asprintf("%s/%s-%d", mount->source, - bundle_data->prefix, - replica->offset); + parent->id, replica->offset); pcmk__add_separated_word(&dbuffer, 1024, source, ","); } @@ -645,7 +641,7 @@ create_remote_resource(pcmk_resource_t *parent, pcmk__bundle_replica_t *replica) goto done; } - id = pcmk__assert_asprintf("%s-%d", bundle_data->prefix, replica->offset); + id = pcmk__assert_asprintf("%s-%d", parent->id, replica->offset); if (pe_find_resource(scheduler->priv->resources, id) != NULL) { free(id); @@ -1192,7 +1188,7 @@ valid_network_for_primitive(pcmk_resource_t *rsc) if (bundle_data->nreplicas_per_host > 1) { pcmk__config_warn("Using " PCMK_XA_REPLICAS_PER_HOST "=1 for %s " "because specifying " PCMK_XA_CONTROL_PORT " " - "requires it", bundle_data->prefix); + "requires it", rsc->id); bundle_data->nreplicas_per_host = 1; pcmk__clear_rsc_flags(rsc, pcmk__rsc_unique); } @@ -1247,11 +1243,10 @@ unpack_bundle_primitive(pcmk_resource_t *rsc, xmlNode **clone_xml) * (It also avoids needing to change regression tests.) */ suffix = (bundle_data->promoted_max > 0)? "master" : PCMK_XE_CLONE; - pcmk__xe_set_id(*clone_xml, "%s-%s", bundle_data->prefix, suffix); + pcmk__xe_set_id(*clone_xml, "%s-%s", rsc->id, suffix); meta = pcmk__xe_create(*clone_xml, PCMK_XE_META_ATTRIBUTES); - pcmk__xe_set_id(meta, "%s-%s-meta", bundle_data->prefix, - (*clone_xml)->name); + pcmk__xe_set_id(meta, "%s-%s-meta", rsc->id, (*clone_xml)->name); crm_create_nvpair_xml(meta, NULL, PCMK_META_ORDERED, PCMK_VALUE_TRUE); @@ -1274,7 +1269,7 @@ unpack_bundle_primitive(pcmk_resource_t *rsc, xmlNode **clone_xml) pcmk__xe_set_int(nvpair, PCMK_XA_VALUE, bundle_data->promoted_max); } - //pcmk__xe_set(primitive_xml, PCMK_XA_ID, bundle_data->prefix); + //pcmk__xe_set(primitive_xml, PCMK_XA_ID, rsc->id); pcmk__xml_copy(*clone_xml, primitive_xml); return pcmk_rc_ok; From 1c3165abfe002d5f760a059bcd1d92c1ca140890 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 23:00:07 -0700 Subject: [PATCH 43/75] Refactor: libpe_status: Drop agent_type validation for bundles bundle_data->agent_type must be set to a valid value if get_container_xml() returned non-NULL. Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index 565b3772f9a..38bd6039ef5 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -315,12 +315,6 @@ allocate_ip(pe__bundle_variant_data_t *data, pcmk__bundle_replica_t *replica, data->ip_last = replica->ipaddr; - if ((data->agent_type != PE__CONTAINER_AGENT_DOCKER) - && (data->agent_type != PE__CONTAINER_AGENT_PODMAN)) { - - return; - } - if (data->add_host) { g_string_append_printf(buffer, " --add-host=%s-%d:%s", data->prefix, replica->offset, replica->ipaddr); @@ -421,13 +415,6 @@ create_container_resource(pcmk_resource_t *parent, get_bundle_variant_data(bundle_data, parent); - if ((bundle_data->agent_type != PE__CONTAINER_AGENT_DOCKER) - && (bundle_data->agent_type != PE__CONTAINER_AGENT_PODMAN)) { - - rc = pcmk_rc_unpack_error; - goto done; - } - agent_str = container_agent_str(bundle_data->agent_type); buffer = g_string_sized_new(4096); From 064a7cc6496d75ba15c81e95bee2cac6d17f91a5 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 23:07:07 -0700 Subject: [PATCH 44/75] Refactor: libpe_status: allocate_ip() takes a pcmk_resource_t argument This gets rid of the last use of bundle_data->prefix. Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index 38bd6039ef5..236389cd401 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -300,29 +300,34 @@ next_ip(const char *last_ip) } static void -allocate_ip(pe__bundle_variant_data_t *data, pcmk__bundle_replica_t *replica, +allocate_ip(pcmk_resource_t *parent, pcmk__bundle_replica_t *replica, GString *buffer) { - if(data->ip_range_start == NULL) { + pe__bundle_variant_data_t *bundle_data = NULL; + + get_bundle_variant_data(bundle_data, parent); + + if (bundle_data->ip_range_start == NULL) { return; + } - } else if(data->ip_last) { - replica->ipaddr = next_ip(data->ip_last); + if (bundle_data->ip_last != NULL) { + replica->ipaddr = next_ip(bundle_data->ip_last); } else { - replica->ipaddr = pcmk__str_copy(data->ip_range_start); + replica->ipaddr = pcmk__str_copy(bundle_data->ip_range_start); } - data->ip_last = replica->ipaddr; + bundle_data->ip_last = replica->ipaddr; - if (data->add_host) { - g_string_append_printf(buffer, " --add-host=%s-%d:%s", data->prefix, + if (bundle_data->add_host) { + g_string_append_printf(buffer, " --add-host=%s-%d:%s", parent->id, replica->offset, replica->ipaddr); return; } g_string_append_printf(buffer, " --hosts-entry=%s=%s-%d", replica->ipaddr, - data->prefix, replica->offset); + parent->id, replica->offset); } static xmlNode * @@ -1346,7 +1351,7 @@ create_child_resource(pcmk_resource_t *rsc, xmlNode *clone_xml, pcmk__set_rsc_flags(bundle_data->child, pcmk__rsc_notify); } - allocate_ip(bundle_data, replica, buffer); + allocate_ip(rsc, replica, buffer); bundle_data->replicas = g_list_append(bundle_data->replicas, replica); // coverity[null_field : FALSE] replica->child can't be NULL here @@ -1411,7 +1416,7 @@ pe__unpack_bundle(pcmk_resource_t *rsc) replica = pcmk__assert_alloc(1, sizeof(pcmk__bundle_replica_t)); replica->offset = lpc; - allocate_ip(bundle_data, replica, buffer); + allocate_ip(rsc, replica, buffer); bundle_data->replicas = g_list_append(bundle_data->replicas, replica); } From 6ef4908b1b7e30620d7266a951f4856fa4a2d06c Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 23:07:47 -0700 Subject: [PATCH 45/75] Refactor: libpe_status: Drop pe__bundle_variant_data_t:prefix Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index 236389cd401..d12a4597a49 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -73,7 +73,6 @@ typedef struct { int promoted_max; int nreplicas; int nreplicas_per_host; - char *prefix; char *image; const char *ip_last; char *host_network; @@ -1385,7 +1384,6 @@ pe__unpack_bundle(pcmk_resource_t *rsc) bundle_data = pcmk__assert_alloc(1, sizeof(pe__bundle_variant_data_t)); rsc->priv->variant_opaque = bundle_data; - bundle_data->prefix = pcmk__str_copy(rsc->id); if (unpack_bundle_container(rsc) != pcmk_rc_ok) { return false; @@ -2014,7 +2012,6 @@ pe__free_bundle(pcmk_resource_t *rsc) get_bundle_variant_data(bundle_data, rsc); pcmk__rsc_trace(rsc, "Freeing %s", rsc->id); - free(bundle_data->prefix); free(bundle_data->image); free(bundle_data->control_port); free(bundle_data->host_network); From faa3d97bebfed1f0958d542cb1af51486fa0e749 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 23:16:20 -0700 Subject: [PATCH 46/75] Refactor: libpe_status: New create_simple_replicas() Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index d12a4597a49..aab7960a75e 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1372,6 +1372,27 @@ create_child_resource(pcmk_resource_t *rsc, xmlNode *clone_xml, return pcmk_rc_ok; } +static void +create_simple_replicas(pcmk_resource_t *rsc) +{ + // Just a naked container, no pacemaker-remote + GString *buffer = g_string_sized_new(1024); + pe__bundle_variant_data_t *bundle_data = NULL; + + get_bundle_variant_data(bundle_data, rsc); + + for (int i = 0; i < bundle_data->nreplicas; i++) { + pcmk__bundle_replica_t *replica = + pcmk__assert_alloc(1, sizeof(pcmk__bundle_replica_t)); + + replica->offset = i; + allocate_ip(rsc, replica, buffer); + bundle_data->replicas = g_list_append(bundle_data->replicas, replica); + } + + bundle_data->container_host_options = g_string_free(buffer, false); +} + bool pe__unpack_bundle(pcmk_resource_t *rsc) { @@ -1406,19 +1427,7 @@ pe__unpack_bundle(pcmk_resource_t *rsc) } } else { - // Just a naked container, no pacemaker-remote - GString *buffer = g_string_sized_new(1024); - - for (int lpc = 0; lpc < bundle_data->nreplicas; lpc++) { - pcmk__bundle_replica_t *replica = NULL; - - replica = pcmk__assert_alloc(1, sizeof(pcmk__bundle_replica_t)); - replica->offset = lpc; - allocate_ip(rsc, replica, buffer); - bundle_data->replicas = g_list_append(bundle_data->replicas, - replica); - } - bundle_data->container_host_options = g_string_free(buffer, false); + create_simple_replicas(rsc); } for (GList *iter = bundle_data->replicas; iter != NULL; iter = iter->next) { From 7ee9eb441554b33596b0138ef446e00cc36ec056 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 23:26:12 -0700 Subject: [PATCH 47/75] Medium: libpe_status: Fix double-free in pe__unpack_bundle() This goes back at least as far as 5892a2bd. If pe__unpack_bundle() fails in create_replica_resources(), it calls pcmk__free_resource(rsc). Then the caller (pe__unpack_resource()) also frees rsc when it jumps to the "done" label. Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index aab7960a75e..0b85ca12978 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1435,7 +1435,6 @@ pe__unpack_bundle(pcmk_resource_t *rsc) if (create_replica_resources(rsc, replica) != pcmk_rc_ok) { pcmk__config_err("Failed unpacking resource %s", rsc->id); - pcmk__free_resource(rsc); return false; } @@ -1465,10 +1464,11 @@ pe__unpack_bundle(pcmk_resource_t *rsc) } } - if (bundle_data->child) { + if (bundle_data->child != NULL) { rsc->priv->children = g_list_append(rsc->priv->children, bundle_data->child); } + return true; } From a479d27e985918177acaffef0a69f1c1db46088b Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 23:40:49 -0700 Subject: [PATCH 48/75] Refactor: libpe_status: Move utilization to create_replica_resources() We're setting utilization for replica resources, so this makes sense to me. Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 50 ++++++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index 0b85ca12978..bae15afd0ff 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -814,6 +814,31 @@ create_replica_resources(pcmk_resource_t *parent, */ pcmk__set_rsc_flags(replica->remote, pcmk__rsc_remote_nesting_allowed); } + + /* Utilization needs special handling for bundles. It makes no sense for the + * inner primitive to have utilization, because it is tied one-to-one to the + * guest node created by the container resource -- and there's no way to set + * capacities for that guest node anyway. + * + * What the user really wants is to configure utilization for the container. + * However, the schema only allows utilization for primitives, and the + * container resource is implicit anyway, so the user can *only* configure + * utilization for the inner primitive. If they do, move the primitive's + * utilization values to the container. + * + * @TODO This means that bundles without an inner primitive can't have + * utilization. An alternative might be to allow utilization values in the + * top-level bundle XML in the schema, and copy those to each container. + */ + if (replica->child != NULL) { + GHashTable *empty = replica->container->priv->utilization; + + replica->container->priv->utilization = + replica->child->priv->utilization; + + replica->child->priv->utilization = empty; + } + return rc; } @@ -1437,31 +1462,6 @@ pe__unpack_bundle(pcmk_resource_t *rsc) pcmk__config_err("Failed unpacking resource %s", rsc->id); return false; } - - /* Utilization needs special handling for bundles. It makes no sense for - * the inner primitive to have utilization, because it is tied - * one-to-one to the guest node created by the container resource -- and - * there's no way to set capacities for that guest node anyway. - * - * What the user really wants is to configure utilization for the - * container. However, the schema only allows utilization for - * primitives, and the container resource is implicit anyway, so the - * user can *only* configure utilization for the inner primitive. If - * they do, move the primitive's utilization values to the container. - * - * @TODO This means that bundles without an inner primitive can't have - * utilization. An alternative might be to allow utilization values in - * the top-level bundle XML in the schema, and copy those to each - * container. - */ - if (replica->child != NULL) { - GHashTable *empty = replica->container->priv->utilization; - - replica->container->priv->utilization = - replica->child->priv->utilization; - - replica->child->priv->utilization = empty; - } } if (bundle_data->child != NULL) { From be26a31c4fe90b1a0e017d2b63d58ad18239f1b2 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Thu, 14 May 2026 01:31:22 -0700 Subject: [PATCH 49/75] Low: libcrmcommon, tools: Fix NULL dereference in crm_resource.c Found by Coverity with --enable-fnptr option. pcmk_controld_api_replies_expected() dereferences its argument. Signed-off-by: Reid Wahl --- lib/common/ipc_controld.c | 5 ++++- tools/crm_resource.c | 24 ++++++++++++++++-------- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/lib/common/ipc_controld.c b/lib/common/ipc_controld.c index 21bfdf0ea39..76a8e5f1397 100644 --- a/lib/common/ipc_controld.c +++ b/lib/common/ipc_controld.c @@ -623,8 +623,11 @@ pcmk_controld_api_refresh(pcmk_ipc_api_t *api, const char *target_node, unsigned int pcmk_controld_api_replies_expected(const pcmk_ipc_api_t *api) { - struct controld_api_private_s *private = api->api_data; + struct controld_api_private_s *private = NULL; + CRM_CHECK(api != NULL, return 0); + + private = api->api_data; return private->replies_expected; } diff --git a/tools/crm_resource.c b/tools/crm_resource.c index c8cd7d474c4..c5cba6a1b8e 100644 --- a/tools/crm_resource.c +++ b/tools/crm_resource.c @@ -268,16 +268,24 @@ static void start_mainloop(pcmk_ipc_api_t *capi) { // @TODO See if we can avoid setting exit_code as a global variable - unsigned int count = pcmk_controld_api_replies_expected(capi); + unsigned int count = 0; + + if (capi == NULL) { + // Should happen only during a dry run due to CIB_file being set + return; + } - if (count > 0) { - out->info(out, "Waiting for %u %s from the controller", - count, pcmk__plural_alt(count, "reply", "replies")); - exit_code = CRM_EX_DISCONNECT; // For unexpected disconnects - mainloop = g_main_loop_new(NULL, FALSE); - pcmk__create_timer(MESSAGE_TIMEOUT_S * 1000, resource_ipc_timeout, NULL); - g_main_loop_run(mainloop); + count = pcmk_controld_api_replies_expected(capi); + if (count == 0) { + return; } + + out->info(out, "Waiting for %u %s from the controller", count, + pcmk__plural_alt(count, "reply", "replies")); + exit_code = CRM_EX_DISCONNECT; // For unexpected disconnects + mainloop = g_main_loop_new(NULL, false); + pcmk__create_timer((MESSAGE_TIMEOUT_S * 1000), resource_ipc_timeout, NULL); + g_main_loop_run(mainloop); } static GList * From 94a5768e453277877c319ffe5f1d1a0729ebaae9 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Thu, 14 May 2026 02:08:19 -0700 Subject: [PATCH 50/75] Refactor: libcrmcommon: Check controld API reply type Instead of doing a string comparison. This is to address what appears to be a false positive from Coverity, but it also feels cleaner than the code line prior to this commit. Signed-off-by: Reid Wahl --- lib/common/ipc_controld.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/common/ipc_controld.c b/lib/common/ipc_controld.c index 76a8e5f1397..00306b9ae41 100644 --- a/lib/common/ipc_controld.c +++ b/lib/common/ipc_controld.c @@ -287,7 +287,7 @@ dispatch(pcmk_ipc_api_t *api, xmlNode *reply) pcmk__call_ipc_callback(api, pcmk_ipc_event_reply, status, &reply_data); // Free any reply data that was allocated - if (pcmk__str_eq(value, PCMK__CONTROLD_CMD_NODES, pcmk__str_casei)) { + if (reply_data.reply_type == pcmk_controld_reply_nodes) { g_list_free_full(reply_data.data.nodes, free); } From 49a1ff5210ba6b0081f0d9ec52ba6c407945ee93 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Thu, 14 May 2026 02:22:46 -0700 Subject: [PATCH 51/75] Low: libpacemaker: Fix list memory leaks in pcmk_agents.c Found by Coverity with --enable-fnptr. Signed-off-by: Reid Wahl --- lib/pacemaker/pcmk_agents.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/pacemaker/pcmk_agents.c b/lib/pacemaker/pcmk_agents.c index 2f56b81e998..755f97162e7 100644 --- a/lib/pacemaker/pcmk_agents.c +++ b/lib/pacemaker/pcmk_agents.c @@ -38,6 +38,7 @@ pcmk__list_alternatives(pcmk__output_t *out, const char *agent_spec) } lrmd_api_delete(lrmd_conn); + lrmd_list_freeall(list); return rc; } @@ -104,6 +105,7 @@ pcmk__list_agents(pcmk__output_t *out, char *agent_spec) } lrmd_api_delete(lrmd_conn); + lrmd_list_freeall(list); return rc; } @@ -156,6 +158,7 @@ pcmk__list_providers(pcmk__output_t *out, const char *agent_spec) } lrmd_api_delete(lrmd_conn); + lrmd_list_freeall(list); return rc; } @@ -203,6 +206,7 @@ pcmk__list_standards(pcmk__output_t *out) } lrmd_api_delete(lrmd_conn); + lrmd_list_freeall(list); return rc; } From 5feeb1f4e316923f57c30835363dc273ae2c7fe7 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 23 May 2026 09:43:38 -0700 Subject: [PATCH 52/75] Low: devel: Provide a model of g_clear_pointer() for Coverity This fixes a bunch of INCONSISTENT_UNION_ACCESS errors with GLib < 2.58. (We currently fix the GLib version at 2.42 in crm_internal.h.) It also makes one INCOMPLETE_DEALLOCATOR false positive go away from election.c. Signed-off-by: Reid Wahl --- devel/Makefile.am | 17 ++++++++++++++--- devel/cov_models.c | 21 +++++++++++++++++++++ devel/cov_nodefs.h | 6 ++++++ 3 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 devel/cov_models.c create mode 100644 devel/cov_nodefs.h diff --git a/devel/Makefile.am b/devel/Makefile.am index 7dabdf66480..05cbacf57f2 100644 --- a/devel/Makefile.am +++ b/devel/Makefile.am @@ -44,6 +44,13 @@ COVTAR = $(abs_top_builddir)/$(PACKAGE)-coverity-$(TAG).tgz COVEMACS = $(abs_top_builddir)/$(TAG).coverity COVHTML = $(COVERITY_DIR)/output/errors +# File with #nodef directives to suppress macro expansions +COV_NODEFS = $(abs_top_builddir)/devel/cov_nodefs.h + +# Custom function models +COV_MODEL_SRC = $(abs_top_builddir)/devel/cov_models.c +COV_MODEL_OUT = $(COVERITY_DIR)/cov_models.xmldb + # Coverity outputs are phony so they get rebuilt every invocation # Coverity (as of version 2026.3) doesn't work correctly with C23. In @@ -58,8 +65,12 @@ COVHTML = $(COVERITY_DIR)/output/errors $(COVERITY_DIR): coverity-clean $(MAKE) $(AM_MAKEFLAGS) -C $(top_builddir) init core-clean $(AM_V_GEN)cd $(top_builddir) \ - && cov-build --dir "$@" $(MAKE) $(AM_MAKEFLAGS) \ - CFLAGS="-std=c99 $(CFLAGS)" core + && cov-build --dir "$@" \ + --add-arg --preinclude_macros="$(COV_NODEFS)" \ + $(MAKE) $(AM_MAKEFLAGS) \ + CFLAGS="-std=c99 $(CFLAGS)" core \ + && cov-make-library --output-file "$(COV_MODEL_OUT)" \ + "$(COV_MODEL_SRC)" # Public coverity instance @@ -87,7 +98,7 @@ coverity-analyze: $(COVERITY_DIR) @echo "Analyzing (waiting for coverity license if necessary) ..." cd $(top_builddir) && cov-analyze --dir "$<" --wait-for-license \ --security --aggressiveness-level "$(COVLEVEL)" \ - --disable INCONSISTENT_UNION_ACCESS + --model-file "$(COV_MODEL_OUT)" .PHONY: $(COVEMACS) $(COVEMACS): coverity-analyze diff --git a/devel/cov_models.c b/devel/cov_models.c new file mode 100644 index 00000000000..a9f258c3d87 --- /dev/null +++ b/devel/cov_models.c @@ -0,0 +1,21 @@ +/* cov-make-library doesn't find the standard include paths unless we specify a + * compiler, so just define NULL here + */ +#ifndef NULL +#define NULL ((void *) 0) +#endif + +// See comment in cov_nodefs.h for an explanation +void +g_clear_pointer(void **ptr, void (*destroy_fn)(void *)) +{ + void *saved_ptr = NULL; + + if ((ptr == NULL) || (*ptr == NULL)) { + return; + } + + saved_ptr = *ptr; + *ptr = NULL; + destroy_fn(saved_ptr); +} diff --git a/devel/cov_nodefs.h b/devel/cov_nodefs.h new file mode 100644 index 00000000000..b73d3211315 --- /dev/null +++ b/devel/cov_nodefs.h @@ -0,0 +1,6 @@ +/* Coverity doesn't understand the g_clear_pointer() macro. We suppress the + * macro expansion and provide a custom function model for g_clear_pointer(). + * Coverity support case 03702994 is open to request that Coverity ships a + * model. + */ +#nodef g_clear_pointer From 086907c6babfdad959869e9d34d7d98f4eb06902 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 14:19:59 -0700 Subject: [PATCH 53/75] Low: libcrmcommon: Suppress INCOMPLETE_DEALLOCATOR false positive Coverity thinks pcmk__format_result() is the deallocator for pcmk__action_result_t, and that pcmk__copy_result() is the allocator. The correct deallocator is pcmk__reset_result(), but I have not found a way to convince Coverity of this. It seems not to understand that g_clear_pointer() frees memory. Signed-off-by: Reid Wahl --- lib/common/results.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/lib/common/results.c b/lib/common/results.c index 9c0a57b275b..8c5d825d95a 100644 --- a/lib/common/results.c +++ b/lib/common/results.c @@ -1283,6 +1283,20 @@ pcmk__copy_result(const pcmk__action_result_t *src, pcmk__action_result_t *dst) dst->exit_status = src->exit_status; dst->execution_status = src->execution_status; dst->exit_reason = pcmk__str_copy(src->exit_reason); + + /* Even with our custom function model and macro suppression for + * g_clear_pointer(), Coverity fails to identify pcmk__reset_result() as the + * deallocator of pcmk__action_result_t. Instead, it thinks + * pcmk__format_result() is the deallocator. The issue is definitely with + * Coverity misunderstanding g_clear_pointer(), because it identifies + * pcmk__reset_result() correctly if we call free() and set the fields to + * NULL directly. There is an issue filed with Black Duck (Coverity vendor): + * case 03702994. + */ + + // coverity[INCOMPLETE_DEALLOCATOR : FALSE] dst->action_stdout = pcmk__str_copy(src->action_stdout); + + // coverity[INCOMPLETE_DEALLOCATOR : FALSE] dst->action_stderr = pcmk__str_copy(src->action_stderr); } From 0d88572fed167b35a9f83dc391100940a0c5746c Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 23 May 2026 13:51:11 -0700 Subject: [PATCH 54/75] Refactor: executor: Drop dead_error_line Coverity suppression Signed-off-by: Reid Wahl --- daemons/execd/execd_messages.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/daemons/execd/execd_messages.c b/daemons/execd/execd_messages.c index 52384cf79be..95335f366a4 100644 --- a/daemons/execd/execd_messages.c +++ b/daemons/execd/execd_messages.c @@ -49,21 +49,21 @@ handle_ipc_fwd_request(pcmk__request_t *request) } rc = ipc_proxy_forward_client(request->ipc_client, request->xml); -#else - rc = EPROTONOSUPPORT; -#endif if (rc == pcmk_rc_ok) { - /* Coverity gets confused by the #ifdef above and thinks this block - * is unreachable due to rc always being EPROTONOSUPPORT. - */ - // coverity[dead_error_line] pcmk__set_result(&request->result, CRM_EX_OK, PCMK_EXEC_DONE, NULL); + } else { pcmk__set_result(&request->result, pcmk_rc2exitc(rc), PCMK_EXEC_ERROR, pcmk_rc_str(rc)); } +#else + rc = EPROTONOSUPPORT; + pcmk__set_result(&request->result, pcmk_rc2exitc(rc), PCMK_EXEC_ERROR, + pcmk_rc_str(rc)); +#endif + pcmk__xe_get_int(request->xml, PCMK__XA_LRMD_CALLID, &call_id); /* Create a generic reply since forwarding doesn't create a more specific one */ From 23cd11b579743b3363f4bfcb4aa7775bd78b1b1e Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 23 May 2026 14:23:22 -0700 Subject: [PATCH 55/75] Refactor: libcrmcommon: Drop unused Coverity +kill annotations No new errors appear when I remove these. At least as of Coverity 2026.3, Coverity models the abort() function. Signed-off-by: Reid Wahl --- include/crm/common/results.h | 1 - lib/common/results.c | 3 --- 2 files changed, 4 deletions(-) diff --git a/include/crm/common/results.h b/include/crm/common/results.h index 236e0859c50..1b28b69cf42 100644 --- a/include/crm/common/results.h +++ b/include/crm/common/results.h @@ -376,7 +376,6 @@ const char *crm_exit_str(crm_exit_t exit_code); _Noreturn crm_exit_t crm_exit(crm_exit_t rc); -/* coverity[+kill] */ void crm_abort(const char *file, const char *function, int line, const char *condition, gboolean do_core, gboolean do_fork); diff --git a/lib/common/results.c b/lib/common/results.c index 8c5d825d95a..80ba97022a3 100644 --- a/lib/common/results.c +++ b/lib/common/results.c @@ -134,7 +134,6 @@ log_assertion_as(const char *file, const char *function, int line, line, assert_condition); } -/* coverity[+kill] */ /*! * \internal * \brief Log a failed assertion and abort @@ -154,7 +153,6 @@ pcmk__abort_as(const char *file, const char *function, int line, abort(); } -/* coverity[+kill] */ /*! * \internal * \brief Handle a failed assertion @@ -212,7 +210,6 @@ fail_assert_as(const char *file, const char *function, int line, } } -/* coverity[+kill] */ void crm_abort(const char *file, const char *function, int line, const char *assert_condition, gboolean do_core, gboolean do_fork) From e645b0c2857f67982c42169713027e2c8a9167a2 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 23 May 2026 14:33:26 -0700 Subject: [PATCH 56/75] Refactor: libstonithd: Drop result_independent_of_operands suppression This doesn't introduce any new errors after refactoring. Signed-off-by: Reid Wahl --- lib/fencing/st_client.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/fencing/st_client.c b/lib/fencing/st_client.c index 20e73028a87..0116a0a5dc1 100644 --- a/lib/fencing/st_client.c +++ b/lib/fencing/st_client.c @@ -746,11 +746,13 @@ stonith_api_history(stonith_t * stonith, int call_options, const char *node, pcmk__xe_get_ll(op, PCMK__XA_ST_DATE_NSEC, &completed_nsec); - // Coverity complains here if long is the same size as long long - // coverity[result_independent_of_operands:FALSE] - if ((completed_nsec >= LONG_MIN) && (completed_nsec <= LONG_MAX)) { - kvp->completed_nsec = (long) completed_nsec; +#if (LLONG_MIN < LONG_MIN) + if ((completed_nsec < LONG_MIN) || (completed_nsec > LONG_MAX)) { + completed_nsec = 0; } +#endif // (LLONG_MIN < LONG_MIN) + + kvp->completed_nsec = completed_nsec; pcmk__xe_get_int(op, PCMK__XA_ST_STATE, &kvp->state); kvp->exit_reason = pcmk__xe_get_copy(op, PCMK_XA_EXIT_REASON); From b7a9ed24c2bd1eb818011ea0008a6c9a2bf6dd3e Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 23 May 2026 14:35:53 -0700 Subject: [PATCH 57/75] Refactor: libcrmservice: Drop check_after_deref Coverity suppression This one is currently unused, but it's an odd one; it seems to come and go with other changes. Signed-off-by: Reid Wahl --- lib/services/systemd.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/services/systemd.c b/lib/services/systemd.c index bf2d51db42e..01129198c14 100644 --- a/lib/services/systemd.c +++ b/lib/services/systemd.c @@ -1149,7 +1149,6 @@ systemd_create_override(const char *agent, int timeout) free(unit_name); - // coverity[check_after_deref : FALSE] if (filename != NULL) { g_string_free(filename, TRUE); } @@ -1186,7 +1185,6 @@ systemd_remove_override(const char *agent, int timeout) done: free(unit_name); - // coverity[check_after_deref : FALSE] if (filename != NULL) { g_string_free(filename, TRUE); } From b5f7f6b416328ddf30ab7f9bddf49d10dd4751cd Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 23 May 2026 14:38:56 -0700 Subject: [PATCH 58/75] Refactor: libcrmcommon: Drop null_field Coverity suppression This doesn't introduce any new errors. It's safer to assert. Signed-off-by: Reid Wahl --- lib/common/schemas.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/common/schemas.c b/lib/common/schemas.c index 7f2c4b6c8bd..351f901770b 100644 --- a/lib/common/schemas.c +++ b/lib/common/schemas.c @@ -1182,7 +1182,8 @@ pcmk__update_schema(xmlNode **xml, const char *max_schema_name, bool transform, break; // No further transformations possible } - // coverity[null_field] The index check ensures entry->next is not NULL + pcmk__assert(entry->next != NULL); + if (!transform || (current_schema->transforms == NULL) || validate_with_silent((*xml)->doc, entry->next->data)) { /* The next schema either doesn't require a transform or validates From fea2015ef031bf0a9bc113505cc1dee605e54676 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 23 May 2026 20:00:11 -0700 Subject: [PATCH 59/75] Refactor: libcrmcluster: Drop Coverity -alloc function annotation This doesn't introduce any new errors. Signed-off-by: Reid Wahl --- lib/cluster/membership.c | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/cluster/membership.c b/lib/cluster/membership.c index 14b33ec9fbe..1ae274e70c5 100644 --- a/lib/cluster/membership.c +++ b/lib/cluster/membership.c @@ -927,7 +927,6 @@ remove_conflicting_peer(pcmk__node_status_t *node) * * \return (Possibly newly created) cluster node cache entry */ -/* coverity[-alloc] Memory is referenced in one or both hashtables */ pcmk__node_status_t * pcmk__get_node(unsigned int id, const char *uname, const char *xml_id, uint32_t flags) From 9bbc3921d1736ee0a27262dd4c8650b5f9af027f Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 23 May 2026 20:16:52 -0700 Subject: [PATCH 60/75] Refactor: libcrmcommon: Drop echo argument from pcmk__output_t:prompt We always passed false. Signed-off-by: Reid Wahl --- include/crm/common/output_internal.h | 8 ++---- lib/cib/cib_remote.c | 4 +-- lib/common/output_log.c | 2 +- lib/common/output_none.c | 2 +- lib/common/output_text.c | 14 +++++----- tools/crm_mon_curses.c | 38 ++++++++-------------------- 6 files changed, 23 insertions(+), 45 deletions(-) diff --git a/include/crm/common/output_internal.h b/include/crm/common/output_internal.h index bdc0837bfb9..b32ddfe3213 100644 --- a/include/crm/common/output_internal.h +++ b/include/crm/common/output_internal.h @@ -524,12 +524,10 @@ struct pcmk__output_s { * to display. * * \param[in] prompt The prompt to display. This is required. - * \param[in] echo If true, echo the user's input to the screen. Set - * to false for password entry. * \param[out] dest Where to store the user's response. This is * required. */ - void (*prompt) (const char *prompt, bool echo, char **dest); + void (*prompt) (const char *prompt, char **dest); }; /*! @@ -730,12 +728,10 @@ pcmk__formatted_vprintf(pcmk__output_t *out, const char *format, va_list args) G * \brief Prompt the user for input. * * \param[in] prompt The prompt to display - * \param[in] echo If true, echo the user's input to the screen. Set - * to false for password entry. * \param[out] dest Where to store the user's response. */ void -pcmk__text_prompt(const char *prompt, bool echo, char **dest); +pcmk__text_prompt(const char *prompt, char **dest); uint8_t pcmk__output_get_log_level(const pcmk__output_t *out); diff --git a/lib/cib/cib_remote.c b/lib/cib/cib_remote.c index a21a46cb68a..49f697faf22 100644 --- a/lib/cib/cib_remote.c +++ b/lib/cib/cib_remote.c @@ -584,9 +584,9 @@ cib_remote_signon(cib_t *cib, const char *name, enum cib_conn_type type) /* If no pcmk__output_t is set, just assume that a text prompt * is good enough. */ - pcmk__text_prompt("Password", false, &(private->passwd)); + pcmk__text_prompt("Password", &private->passwd); } else { - private->out->prompt("Password", false, &(private->passwd)); + private->out->prompt("Password", &private->passwd); } } diff --git a/lib/common/output_log.c b/lib/common/output_log.c index 315f4ad1195..aebe0549e60 100644 --- a/lib/common/output_log.c +++ b/lib/common/output_log.c @@ -307,7 +307,7 @@ log_progress(pcmk__output_t *out, bool end) } static void -log_prompt(const char *prompt, bool echo, char **dest) +log_prompt(const char *prompt, char **dest) { /* This function intentionally left blank */ } diff --git a/lib/common/output_none.c b/lib/common/output_none.c index f368e60e4f3..2d13fb1bb72 100644 --- a/lib/common/output_none.c +++ b/lib/common/output_none.c @@ -122,7 +122,7 @@ none_progress(pcmk__output_t *out, bool end) } static void -none_prompt(const char *prompt, bool echo, char **dest) +none_prompt(const char *prompt, char **dest) { /* This function intentionally left blank */ } diff --git a/lib/common/output_text.c b/lib/common/output_text.c index b7cdb9e9d5b..c45fa519e66 100644 --- a/lib/common/output_text.c +++ b/lib/common/output_text.c @@ -473,7 +473,7 @@ pcmk__indented_printf(pcmk__output_t *out, const char *format, ...) } void -pcmk__text_prompt(const char *prompt, bool echo, char **dest) +pcmk__text_prompt(const char *prompt, char **dest) { int rc = 0; struct termios settings; @@ -481,13 +481,11 @@ pcmk__text_prompt(const char *prompt, bool echo, char **dest) pcmk__assert((prompt != NULL) && (dest != NULL)); - if (!echo) { - rc = tcgetattr(0, &settings); - if (rc == 0) { - orig_c_lflag = settings.c_lflag; - settings.c_lflag &= ~ECHO; - rc = tcsetattr(0, TCSANOW, &settings); - } + rc = tcgetattr(0, &settings); + if (rc == 0) { + orig_c_lflag = settings.c_lflag; + settings.c_lflag &= ~ECHO; + rc = tcsetattr(0, TCSANOW, &settings); } if (rc == 0) { diff --git a/tools/crm_mon_curses.c b/tools/crm_mon_curses.c index 39be9a8a8d4..64885d662b4 100644 --- a/tools/crm_mon_curses.c +++ b/tools/crm_mon_curses.c @@ -296,43 +296,27 @@ curses_progress(pcmk__output_t *out, bool end) { } static void -curses_prompt(const char *prompt, bool do_echo, char **dest) +curses_prompt(const char *prompt, char **dest) { - int rc = OK; + int rc = 0; pcmk__assert((prompt != NULL) && (dest != NULL)); - /* This is backwards from the text version of this function on purpose. We - * disable echo by default in curses_init, so we need to enable it here if - * asked for. - */ - if (do_echo) { - rc = echo(); - } + printw("%s: ", prompt); - if (rc == OK) { - printw("%s: ", prompt); + g_clear_pointer(dest, free); + *dest = pcmk__assert_alloc(1024, sizeof(char)); - if (*dest != NULL) { - free(*dest); - } - - *dest = pcmk__assert_alloc(1024, sizeof(char)); - /* On older systems, scanw is defined as taking a char * for its first argument, - * while newer systems rightly want a const char *. Accomodate both here due - * to building with -Werror. - */ - rc = scanw((NCURSES_CONST char *) "%1023s", *dest); - addch('\n'); - } + /* On older systems, scanw is defined as taking a char * for its first + * argument, while newer systems rightly want a const char *. Accommodate + * both here due to building with -Werror. + */ + rc = scanw((NCURSES_CONST char *) "%1023s", *dest); + addch('\n'); if (rc < 1) { g_clear_pointer(dest, free); } - - if (do_echo) { - noecho(); - } } pcmk__output_t * From aec4b8f5312be7a8d85284faf198a47e46fb888b Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 23 May 2026 21:48:33 -0700 Subject: [PATCH 61/75] Refactor: various: Don't free *dest in pcmk__output_t:prompt The sole caller already ensures that *dest is NULL. We assert like this in some other places where we expect an output argument to point to a NULL pointer. Signed-off-by: Reid Wahl --- lib/common/output_text.c | 4 +--- tools/crm_mon_curses.c | 3 +-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/common/output_text.c b/lib/common/output_text.c index c45fa519e66..02ba7a84c43 100644 --- a/lib/common/output_text.c +++ b/lib/common/output_text.c @@ -479,7 +479,7 @@ pcmk__text_prompt(const char *prompt, char **dest) struct termios settings; tcflag_t orig_c_lflag = 0; - pcmk__assert((prompt != NULL) && (dest != NULL)); + pcmk__assert((prompt != NULL) && (dest != NULL) && (*dest == NULL)); rc = tcgetattr(0, &settings); if (rc == 0) { @@ -491,8 +491,6 @@ pcmk__text_prompt(const char *prompt, char **dest) if (rc == 0) { fprintf(stderr, "%s: ", prompt); - g_clear_pointer(dest, free); - #if HAVE_SSCANF_M rc = scanf("%ms", dest); #else diff --git a/tools/crm_mon_curses.c b/tools/crm_mon_curses.c index 64885d662b4..d17ca15966e 100644 --- a/tools/crm_mon_curses.c +++ b/tools/crm_mon_curses.c @@ -300,11 +300,10 @@ curses_prompt(const char *prompt, char **dest) { int rc = 0; - pcmk__assert((prompt != NULL) && (dest != NULL)); + pcmk__assert((prompt != NULL) && (dest != NULL) && (*dest == NULL)); printw("%s: ", prompt); - g_clear_pointer(dest, free); *dest = pcmk__assert_alloc(1024, sizeof(char)); /* On older systems, scanw is defined as taking a char * for its first From ef4b4dc0b850ffa2779438b6a1f34557b1e46fad Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 23 May 2026 22:08:55 -0700 Subject: [PATCH 62/75] Refactor: libcrmcommon: Unindent pcmk__text_prompt() Signed-off-by: Reid Wahl --- lib/common/output_text.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/lib/common/output_text.c b/lib/common/output_text.c index 02ba7a84c43..fd01a2a5939 100644 --- a/lib/common/output_text.c +++ b/lib/common/output_text.c @@ -482,23 +482,27 @@ pcmk__text_prompt(const char *prompt, char **dest) pcmk__assert((prompt != NULL) && (dest != NULL) && (*dest == NULL)); rc = tcgetattr(0, &settings); - if (rc == 0) { - orig_c_lflag = settings.c_lflag; - settings.c_lflag &= ~ECHO; - rc = tcsetattr(0, TCSANOW, &settings); + if (rc != 0) { + return; } - if (rc == 0) { - fprintf(stderr, "%s: ", prompt); + orig_c_lflag = settings.c_lflag; + settings.c_lflag &= ~ECHO; + + rc = tcsetattr(0, TCSANOW, &settings); + if (rc != 0) { + return; + } + + fprintf(stderr, "%s: ", prompt); #if HAVE_SSCANF_M - rc = scanf("%ms", dest); + rc = scanf("%ms", dest); #else - *dest = pcmk__assert_alloc(1024, sizeof(char)); - rc = scanf("%1023s", *dest); + *dest = pcmk__assert_alloc(1024, sizeof(char)); + rc = scanf("%1023s", *dest); #endif - fprintf(stderr, "\n"); - } + fprintf(stderr, "\n"); if (rc < 1) { g_clear_pointer(dest, free); @@ -506,6 +510,6 @@ pcmk__text_prompt(const char *prompt, char **dest) if (orig_c_lflag != 0) { settings.c_lflag = orig_c_lflag; - /* rc = */ tcsetattr(0, TCSANOW, &settings); + tcsetattr(0, TCSANOW, &settings); } } From c8b32d46418ec408b2214c92701d680f18bfc388 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 24 May 2026 00:30:22 -0700 Subject: [PATCH 63/75] Refactor: libcrmcommon: Use getline() in pcmk__text_prompt() This avoids the HAVE_SSCANF_M check. It has the side effect of allowing whitespace characters within passwords. Also, we no longer strip leading or trailing whitespace. A password on my Linux system may have leading or trailing whitespace or whitespace in the middle. Signed-off-by: Reid Wahl --- lib/common/output_text.c | 19 +++++++++++-------- tools/crm_mon_curses.c | 4 ++++ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/lib/common/output_text.c b/lib/common/output_text.c index fd01a2a5939..eb3ab8f6cb4 100644 --- a/lib/common/output_text.c +++ b/lib/common/output_text.c @@ -478,6 +478,8 @@ pcmk__text_prompt(const char *prompt, char **dest) int rc = 0; struct termios settings; tcflag_t orig_c_lflag = 0; + size_t buf_size = 0; + ssize_t nread = 0; pcmk__assert((prompt != NULL) && (dest != NULL) && (*dest == NULL)); @@ -496,18 +498,19 @@ pcmk__text_prompt(const char *prompt, char **dest) fprintf(stderr, "%s: ", prompt); -#if HAVE_SSCANF_M - rc = scanf("%ms", dest); -#else - *dest = pcmk__assert_alloc(1024, sizeof(char)); - rc = scanf("%1023s", *dest); -#endif - fprintf(stderr, "\n"); + nread = getline(dest, &buf_size, stdin); - if (rc < 1) { + if (nread < 2) { + // Error or empty line ("\n") g_clear_pointer(dest, free); + + } else { + // Strip the trailing newline but no other whitespace + (*dest)[nread - 1] = '\0'; } + fprintf(stderr, "\n"); + if (orig_c_lflag != 0) { settings.c_lflag = orig_c_lflag; tcsetattr(0, TCSANOW, &settings); diff --git a/tools/crm_mon_curses.c b/tools/crm_mon_curses.c index d17ca15966e..61c15cf51b8 100644 --- a/tools/crm_mon_curses.c +++ b/tools/crm_mon_curses.c @@ -295,6 +295,10 @@ curses_progress(pcmk__output_t *out, bool end) { } } +/* @TODO Make the behavior here match that of pcmk__text_prompt(), which stores + * the entire line. scanw() here stores only the first 1023 non-whitespace + * characters. + */ static void curses_prompt(const char *prompt, char **dest) { From fede6cd98edd0fa4f0618be16c0f6c6e00549455 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 24 May 2026 12:57:56 -0700 Subject: [PATCH 64/75] Doc: tools: Add info about scanw() using const Signed-off-by: Reid Wahl --- tools/crm_mon_curses.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/crm_mon_curses.c b/tools/crm_mon_curses.c index 61c15cf51b8..59c4390e04a 100644 --- a/tools/crm_mon_curses.c +++ b/tools/crm_mon_curses.c @@ -313,6 +313,11 @@ curses_prompt(const char *prompt, char **dest) /* On older systems, scanw is defined as taking a char * for its first * argument, while newer systems rightly want a const char *. Accommodate * both here due to building with -Werror. + * + * @COMPAT The prototype was updated to use const in X/Open Curses Issue 7, + * and ncurses was updated to reflect it on 2018-04-07: + * * https://pubs.opengroup.org/onlinepubs/9699909599/toc.pdf + * * https://invisible-island.net/ncurses/NEWS.html#index-t20180407 */ rc = scanw((NCURSES_CONST char *) "%1023s", *dest); addch('\n'); From 570cfe8d1ca3347cfd0e4f45a83d74ccbca240d6 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 24 May 2026 17:57:10 -0700 Subject: [PATCH 65/75] Refactor: build: New PCMK__CURSES_H macro constant To replace all the HAVE_*CURSES constants and reduce some redundancy. Signed-off-by: Reid Wahl --- configure.ac | 36 +++++++++++++++++++++++------------- tools/crm_mon.h | 19 +++---------------- 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/configure.ac b/configure.ac index 4204b86fa15..1689cbaa72f 100644 --- a/configure.ac +++ b/configure.ac @@ -1301,7 +1301,22 @@ dnl (i.e. "ncurses" still wants the include to be simple, no-'n', "curses.h"). dnl dnl ncurses takes precedence. dnl -AC_CHECK_HEADERS([curses.h curses/curses.h ncurses.h ncurses/ncurses.h]) +found_curses_header=0 + +m4_foreach( + [header], + [[curses.h], [curses/curses.h], [ncurses.h], [ncurses/ncurses.h]], + [AS_IF( + [test "$found_curses_header" -eq 0], + [AC_CHECK_HEADER( + header, + [ + found_curses_header=1 + AC_DEFINE([PCMK__CURSES_H], [
], [Curses header file]) + ] + )] + )] +) save_LIBS="$LIBS" found_curses=0 @@ -1327,18 +1342,13 @@ AS_IF([test $found_curses -eq 1 && cc_supports_flag -Wcast-qual], [ AC_MSG_CHECKING([whether curses library is compatible]) AC_LINK_IFELSE( - [AC_LANG_PROGRAM([ -#if defined(HAVE_NCURSES_H) -# include -#elif defined(HAVE_NCURSES_NCURSES_H) -# include -#elif defined(HAVE_CURSES_H) -# include -#elif defined(HAVE_CURSES_CURSES_H) -# include -#endif - ], - [printw((const char *)"Test");] + [AC_LANG_PROGRAM( + [ + #ifdef PCMK__CURSES_H + #include PCMK__CURSES_H + #endif + ], + [printw((const char *) "Test");] )], [AC_MSG_RESULT([yes]) PCMK_FEATURES="$PCMK_FEATURES ncurses" diff --git a/tools/crm_mon.h b/tools/crm_mon.h index e224ed18186..0d6e82d1155 100644 --- a/tools/crm_mon.h +++ b/tools/crm_mon.h @@ -18,22 +18,9 @@ #include #include -/* - * The man pages for both curses and ncurses suggest inclusion of "curses.h". - * We believe the following to be acceptable and portable. - */ - -# if PCMK__ENABLE_CURSES -# if defined(HAVE_NCURSES_H) -# include -# elif defined(HAVE_NCURSES_NCURSES_H) -# include -# elif defined(HAVE_CURSES_H) -# include -# elif defined(HAVE_CURSES_CURSES_H) -# include -# endif -# endif +#if (PCMK__ENABLE_CURSES && defined(PCMK__CURSES_H)) +#include PCMK__CURSES_H +#endif // (PCMK__ENABLE_CURSES && defined(PCMK__CURSES_H)) typedef enum { mon_output_unset, From 5a11541c9f49256d192cef3067ee6f4d30383ec9 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 24 May 2026 18:27:02 -0700 Subject: [PATCH 66/75] Low: build: Check for NCURSES_CONST macro definition We use it in crm_mon_curses.c, but we've never checked for it at build time. Instead, we've always tried to use regular curses (non-ncurses) if ncurses is not available. NCURSES_CONST won't be defined there. Signed-off-by: Reid Wahl --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 1689cbaa72f..98f2a5128df 100644 --- a/configure.ac +++ b/configure.ac @@ -1348,7 +1348,7 @@ AS_IF([test $found_curses -eq 1 && cc_supports_flag -Wcast-qual], [ #include PCMK__CURSES_H #endif ], - [printw((const char *) "Test");] + [printw((NCURSES_CONST char *) "Test");] )], [AC_MSG_RESULT([yes]) PCMK_FEATURES="$PCMK_FEATURES ncurses" From b87f22254aac5ceab63f2331e6f6a817b165fd73 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 24 May 2026 18:59:57 -0700 Subject: [PATCH 67/75] Low: build: Ignore curses library unless it's ncurses INSTALL.md already documents the need for ncurses if a user wants to use crm_mon in interactive mode. Also, we require NCURSES_CONST to be defined in that case; this has been true since bd203b2. So we might as well require ncurses explicitly and drop support for "regular" curses. This shouldn't change behavior. Signed-off-by: Reid Wahl --- configure.ac | 62 ++++++++++++++++++++++------------------------- tools/Makefile.am | 2 +- tools/crm_mon.h | 6 ++--- 3 files changed, 33 insertions(+), 37 deletions(-) diff --git a/configure.ac b/configure.ac index 98f2a5128df..f9d43c3e4e0 100644 --- a/configure.ac +++ b/configure.ac @@ -1292,46 +1292,41 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include ]], [[sighandler_t *f;]] dnl ======================================================================== dnl ncurses dnl ======================================================================== -dnl -dnl A few OSes (e.g. Linux) deliver a default "ncurses" alongside "curses". -dnl Many non-Linux deliver "curses"; sites may add "ncurses". -dnl -dnl However, the source-code recommendation for both is to #include "curses.h" -dnl (i.e. "ncurses" still wants the include to be simple, no-'n', "curses.h"). -dnl -dnl ncurses takes precedence. -dnl -found_curses_header=0 + +found_ncurses_header=0 m4_foreach( - [header], - [[curses.h], [curses/curses.h], [ncurses.h], [ncurses/ncurses.h]], + [header], [[ncurses.h], [ncurses/ncurses.h]], [AS_IF( - [test "$found_curses_header" -eq 0], + [test "$found_ncurses_header" -eq 0], [AC_CHECK_HEADER( header, [ - found_curses_header=1 - AC_DEFINE([PCMK__CURSES_H], [
], [Curses header file]) + found_ncurses_header=1 + AC_DEFINE([PCMK__NCURSES_H], [
], [Ncurses header file]) ] )] )] ) save_LIBS="$LIBS" -found_curses=0 -CURSES_LIBS="" +found_ncurses=0 +NCURSES_LIBS="" LIBS="" -AC_SEARCH_LIBS([printw], [ncurses curses], - [test "$ac_cv_search_printw" = "none required" || CURSES_LIBS="$LIBS" - found_curses=1], - [found_curses=0]) +AC_SEARCH_LIBS( + [printw], [ncurses], + [ + test "$ac_cv_search_printw" = "none required" || NCURSES_LIBS="$LIBS" + found_ncurses=1 + ], + [found_ncurses=0] +) LIBS="$save_LIBS" dnl Check for printw() prototype compatibility -AS_IF([test $found_curses -eq 1 && cc_supports_flag -Wcast-qual], [ +AS_IF([test $found_ncurses -eq 1 && cc_supports_flag -Wcast-qual], [ ac_save_LIBS="$LIBS" - LIBS="$CURSES_LIBS" + LIBS="$NCURSES_LIBS" # avoid broken test because of hardened build environment in Fedora 23+ # - https://fedoraproject.org/wiki/Changes/Harden_All_Packages @@ -1340,12 +1335,12 @@ AS_IF([test $found_curses -eq 1 && cc_supports_flag -Wcast-qual], [ [cc_temp_flags "-Wcast-qual $WERROR -fPIC"], [cc_temp_flags "-Wcast-qual $WERROR"]) - AC_MSG_CHECKING([whether curses library is compatible]) + AC_MSG_CHECKING([whether ncurses library is compatible]) AC_LINK_IFELSE( [AC_LANG_PROGRAM( [ - #ifdef PCMK__CURSES_H - #include PCMK__CURSES_H + #ifdef PCMK__NCURSES_H + #include PCMK__NCURSES_H #endif ], [printw((NCURSES_CONST char *) "Test");] @@ -1354,12 +1349,12 @@ AS_IF([test $found_curses -eq 1 && cc_supports_flag -Wcast-qual], [ PCMK_FEATURES="$PCMK_FEATURES ncurses" ], [ - found_curses=0 - CURSES_LIBS="" + found_ncurses=0 + NCURSES_LIBS="" AC_MSG_RESULT([no]) - AC_MSG_WARN(m4_normalize([Disabling curses because the printw() - function of your (n)curses library is old. - If you wish to enable curses, update to a + AC_MSG_WARN(m4_normalize([Disabling ncurses because the printw() + function of your ncurses library is old. + If you wish to enable ncurses, update to a newer version (ncurses 5.4 or later is recommended, available from https://invisible-island.net/ncurses/) @@ -1371,8 +1366,9 @@ AS_IF([test $found_curses -eq 1 && cc_supports_flag -Wcast-qual], [ cc_restore_flags ]) -AC_DEFINE_UNQUOTED([PCMK__ENABLE_CURSES], [$found_curses], [have ncurses library]) -AC_SUBST(CURSES_LIBS) +AC_DEFINE_UNQUOTED([PCMK__ENABLE_CURSES], [$found_ncurses], + [have ncurses library]) +AC_SUBST(NCURSES_LIBS) dnl ======================================================================== dnl Profiling and GProf diff --git a/tools/Makefile.am b/tools/Makefile.am index 0c19f541e41..1d02df3ebee 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -99,7 +99,7 @@ crm_mon_LDADD += $(top_builddir)/lib/pengine/libpe_status.la crm_mon_LDADD += $(top_builddir)/lib/fencing/libstonithd.la crm_mon_LDADD += $(top_builddir)/lib/cib/libcib.la crm_mon_LDADD += $(top_builddir)/lib/common/libcrmcommon.la -crm_mon_LDADD += $(CURSES_LIBS) +crm_mon_LDADD += $(NCURSES_LIBS) crm_verify_SOURCES = crm_verify.c crm_verify_LDADD = $(top_builddir)/lib/pacemaker/libpacemaker.la diff --git a/tools/crm_mon.h b/tools/crm_mon.h index 0d6e82d1155..0d508b7f703 100644 --- a/tools/crm_mon.h +++ b/tools/crm_mon.h @@ -18,9 +18,9 @@ #include #include -#if (PCMK__ENABLE_CURSES && defined(PCMK__CURSES_H)) -#include PCMK__CURSES_H -#endif // (PCMK__ENABLE_CURSES && defined(PCMK__CURSES_H)) +#if (PCMK__ENABLE_CURSES && defined(PCMK__NCURSES_H)) +#include PCMK__NCURSES_H +#endif // (PCMK__ENABLE_CURSES && defined(PCMK__NCURSES_H)) typedef enum { mon_output_unset, From a7a1ffb534eebe8df7ba8ed0c0aaa896f0c3a2a8 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 24 May 2026 23:26:33 -0700 Subject: [PATCH 68/75] Refactor: libcrmcommon: Unindent decode_transition_magic() Signed-off-by: Reid Wahl --- lib/common/actions.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/lib/common/actions.c b/lib/common/actions.c index 14cdc8a02d5..3d23c9da16d 100644 --- a/lib/common/actions.c +++ b/lib/common/actions.c @@ -393,11 +393,11 @@ decode_transition_magic(const char *magic, char **uuid, int *transition_id, int { int res = 0; char *key = NULL; - gboolean result = TRUE; + bool result = false; int local_op_status = -1; int local_op_rc = -1; - CRM_CHECK(magic != NULL, return FALSE); + CRM_CHECK(magic != NULL, goto done); #ifdef HAVE_SSCANF_M res = sscanf(magic, "%d:%d;%ms", &local_op_status, &local_op_rc, &key); @@ -409,22 +409,28 @@ decode_transition_magic(const char *magic, char **uuid, int *transition_id, int if (res == EOF) { pcmk__err("Could not decode transition information '%s': %s", magic, pcmk_rc_str(errno)); - result = FALSE; - } else if (res < 3) { + goto done; + } + + if (res < 3) { pcmk__warn("Transition information '%s' incomplete (%d of 3 expected " "items)", magic, res); - result = FALSE; - } else { - if (op_status) { - *op_status = local_op_status; - } - if (op_rc) { - *op_rc = local_op_rc; - } - result = decode_transition_key(key, uuid, transition_id, action_id, - target_rc); + goto done; + } + + if (op_status != NULL) { + *op_status = local_op_status; + } + + if (op_rc != NULL) { + *op_rc = local_op_rc; } + + result = decode_transition_key(key, uuid, transition_id, action_id, + target_rc); + +done: free(key); return result; } From 2774de5556e4bcd486d8efb68b88328d69f29a51 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Mon, 25 May 2026 02:26:14 -0700 Subject: [PATCH 69/75] Feature: libcrmcommon: decode_transition_magic() validates more strictly Technically this changes the behavior of a public API function. * We now reject transition magic strings that begin with whitespace. * We now reject transition magic strings whose op_status or op_rc field begins with a plus sign. These fields must now consist of an optional minus sign followed by digits. * We now reject transition magic strings whose op_status or op_rc field overflows the range of an int. * We now preserve any whitespace at the end of a transition magic string. These are all side effects of replacing the sscanf() calls with a regex match and explicit integer parsing. Signed-off-by: Reid Wahl --- lib/common/actions.c | 60 ++++++++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 24 deletions(-) diff --git a/lib/common/actions.c b/lib/common/actions.c index 3d23c9da16d..1db616bfc11 100644 --- a/lib/common/actions.c +++ b/lib/common/actions.c @@ -388,50 +388,62 @@ pcmk__notify_key(const char *rsc_id, const char *notify_type, * for freeing the memory for *uuid using free(). */ gboolean -decode_transition_magic(const char *magic, char **uuid, int *transition_id, int *action_id, - int *op_status, int *op_rc, int *target_rc) +decode_transition_magic(const char *magic, char **uuid, int *transition_id, + int *action_id, int *op_status, int *op_rc, + int *target_rc) { - int res = 0; - char *key = NULL; + GRegex *regex = NULL; + GMatchInfo *match_info = NULL; + char **matches = NULL; + + long long status_ll = 0; + long long rc_ll = 0; bool result = false; - int local_op_status = -1; - int local_op_rc = -1; CRM_CHECK(magic != NULL, goto done); -#ifdef HAVE_SSCANF_M - res = sscanf(magic, "%d:%d;%ms", &local_op_status, &local_op_rc, &key); -#else - // magic must have >=4 other characters - key = pcmk__assert_alloc(strlen(magic) - 3, sizeof(char)); - res = sscanf(magic, "%d:%d;%s", &local_op_status, &local_op_rc, key); -#endif - if (res == EOF) { - pcmk__err("Could not decode transition information '%s': %s", magic, - pcmk_rc_str(errno)); + regex = g_regex_new("^(-?\\d+):(-?\\d+);(.*)$", 0, 0, NULL); + pcmk__assert(regex != NULL); + + if (!g_regex_match(regex, magic, 0, &match_info)) { + pcmk__err("Could not decode transition information '%s': does not " + "match pattern 'STATUS:RC;KEY'", magic); + goto done; + } + + matches = g_match_info_fetch_all(match_info); + + if ((pcmk__scan_ll(matches[1], &status_ll, 0) != pcmk_rc_ok) + || (status_ll < INT_MIN) || (status_ll > INT_MAX)) { + + pcmk__err("Could not decode transition information '%s': op status " + "'%s' is not a valid int", magic, matches[1]); goto done; } - if (res < 3) { - pcmk__warn("Transition information '%s' incomplete (%d of 3 expected " - "items)", - magic, res); + if ((pcmk__scan_ll(matches[2], &rc_ll, 0) != pcmk_rc_ok) + || (rc_ll < INT_MIN) || (rc_ll > INT_MAX)) { + + pcmk__err("Could not decode transition information '%s': op rc '%s' is " + "not a valid int", magic, matches[2]); goto done; } if (op_status != NULL) { - *op_status = local_op_status; + *op_status = status_ll; } if (op_rc != NULL) { - *op_rc = local_op_rc; + *op_rc = rc_ll; } - result = decode_transition_key(key, uuid, transition_id, action_id, + result = decode_transition_key(matches[3], uuid, transition_id, action_id, target_rc); done: - free(key); + g_regex_unref(regex); + g_match_info_unref(match_info); + g_strfreev(matches); return result; } From 82dd75bd3a4ca01dbd27df486fd3edf38e5fa8ee Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Mon, 25 May 2026 02:31:23 -0700 Subject: [PATCH 70/75] Refactor: build: Drop HAVE_SSCANF_M Nothing uses it anymore. Signed-off-by: Reid Wahl --- configure.ac | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/configure.ac b/configure.ac index f9d43c3e4e0..a05bb0bad08 100644 --- a/configure.ac +++ b/configure.ac @@ -1221,19 +1221,6 @@ AC_RUN_IFELSE([AC_LANG_PROGRAM([[ [AC_MSG_ERROR([strerror() is not C99-compliant])], [AC_MSG_ERROR([strerror() is not C99-compliant])]) -AC_RUN_IFELSE([AC_LANG_PROGRAM([[#include ]], [[ - const char *s = "some-command-line-arg"; - char *name = NULL; - int n = sscanf(s, "%ms", &name); - return n != 1; - ]])], - [have_sscanf_m="yes"], - [have_sscanf_m="no"], - [have_sscanf_m="no"]) -AS_IF([test x"$have_sscanf_m" = x"yes"], - [AC_DEFINE([HAVE_SSCANF_M], [1], - [Define to 1 if sscanf %m modifier is available])]) - AC_RUN_IFELSE([AC_LANG_PROGRAM([[#include ]], [[ char s[200]; time_t t; From e2b37d6a58b65c017c70dfd0b00db62bdeca6b1c Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Mon, 25 May 2026 03:41:44 -0700 Subject: [PATCH 71/75] Refactor: libstonithd: Drop null_field Coverity suppression Coverity was flagging this as an error because it didn't know whether pcmk__trace() might set entry->notify to NULL. Coverity doesn't have a model for qb_log_from_external_source(), which pcmk__trace() calls. We could provide a model, but instead we just drop the "%p", which doesn't seem helpful. Signed-off-by: Reid Wahl --- lib/fencing/st_client.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/fencing/st_client.c b/lib/fencing/st_client.c index 0116a0a5dc1..1ed617e7fe3 100644 --- a/lib/fencing/st_client.c +++ b/lib/fencing/st_client.c @@ -1560,14 +1560,14 @@ stonith_send_notification(void *data, void *user_data) return; } else if (!pcmk__str_eq(entry->event, event, pcmk__str_none)) { - pcmk__trace("Skipping callback - event mismatch %p/%s vs. %s", entry, entry->event, event); + pcmk__trace("Skipping callback - event mismatch %s vs. %s", + entry->event, event); return; } st_event = xml_to_event(blob->xml); - pcmk__trace("Invoking callback for %p/%s event...", entry, event); - // coverity[null_field] + pcmk__trace("Invoking callback for %s event...", event); entry->notify(blob->stonith, st_event); pcmk__trace("Callback invoked..."); From 6d5706805060eb7133f86e2e4f4b9b00ff5f47ce Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Mon, 25 May 2026 13:47:07 -0700 Subject: [PATCH 72/75] Refactor: libpe_status: Drop NULL_FIELD coverity suppression Coverity thinks that passing replica to g_list_append() can set replica->child to NULL. Opened support case 03704783 with Black Duck (Coverity vendor), for them to investigate this further. Signed-off-by: Reid Wahl --- lib/pengine/bundle.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index bae15afd0ff..bcc50008154 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1376,12 +1376,11 @@ create_child_resource(pcmk_resource_t *rsc, xmlNode *clone_xml, } allocate_ip(rsc, replica, buffer); - bundle_data->replicas = g_list_append(bundle_data->replicas, replica); - // coverity[null_field : FALSE] replica->child can't be NULL here bundle_data->attribute_target = g_hash_table_lookup(replica->child->priv->meta, PCMK_META_CONTAINER_ATTRIBUTE_TARGET); + bundle_data->replicas = g_list_append(bundle_data->replicas, replica); } bundle_data->container_host_options = g_string_free(buffer, false); From 01cd71c535e5169c78356f18d344b0861ae6e567 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 27 May 2026 16:29:12 -0700 Subject: [PATCH 73/75] Refactor: libpe_status: Clarify is_set_recursive() somewhat * Use bool instead of gboolean. * Use const GList *. * Improve spacing. * Reduce nesting of conditions and make them more explicit. * Rename to pcmk__is_set_recursive() to emphasize that this is an internal library function. * Use uint64_t instead of long long for flag. Signed-off-by: Reid Wahl --- include/crm/pengine/internal.h | 5 +-- lib/pacemaker/pcmk_sched_bundle.c | 2 +- lib/pacemaker/pcmk_sched_clone.c | 2 +- lib/pacemaker/pcmk_sched_instances.c | 2 +- lib/pengine/clone.c | 53 ++++++++++++++++------------ 5 files changed, 36 insertions(+), 28 deletions(-) diff --git a/include/crm/pengine/internal.h b/include/crm/pengine/internal.h index d779232c7ab..d4d83dfb13d 100644 --- a/include/crm/pengine/internal.h +++ b/include/crm/pengine/internal.h @@ -11,7 +11,7 @@ #define PCMK__CRM_PENGINE_INTERNAL__H #include // bool, false -#include // uint32_t +#include // uint32_t, uint64_t #include // strncmp #include // xmlNode @@ -259,7 +259,8 @@ pe_base_name_eq(const pcmk_resource_t *rsc, const char *id) int pe__target_rc_from_xml(const xmlNode *xml_op); int pe__cmp_node_name(const void *a, const void *b); -bool is_set_recursive(const pcmk_resource_t *rsc, long long flag, bool any); +bool pcmk__is_set_recursive(const pcmk_resource_t *rsc, uint64_t flag, + bool any); pcmk__op_digest_t *pe__calculate_digests(pcmk_resource_t *rsc, const char *task, unsigned int *interval_ms, diff --git a/lib/pacemaker/pcmk_sched_bundle.c b/lib/pacemaker/pcmk_sched_bundle.c index 14963711805..8c1d389b4dd 100644 --- a/lib/pacemaker/pcmk_sched_bundle.c +++ b/lib/pacemaker/pcmk_sched_bundle.c @@ -499,7 +499,7 @@ replica_apply_coloc_score(const pcmk__bundle_replica_t *replica, chosen = container->priv->fns->location(container, NULL, pcmk__rsc_node_assigned); if ((chosen == NULL) - || is_set_recursive(container, pcmk__rsc_blocked, true)) { + || pcmk__is_set_recursive(container, pcmk__rsc_blocked, true)) { return true; } diff --git a/lib/pacemaker/pcmk_sched_clone.c b/lib/pacemaker/pcmk_sched_clone.c index 80872e548fc..49435231951 100644 --- a/lib/pacemaker/pcmk_sched_clone.c +++ b/lib/pacemaker/pcmk_sched_clone.c @@ -343,7 +343,7 @@ pcmk__clone_apply_coloc_score(pcmk_resource_t *dependent, chosen = instance->priv->fns->location(instance, NULL, pcmk__rsc_node_assigned); if ((chosen != NULL) - && !is_set_recursive(instance, pcmk__rsc_blocked, TRUE)) { + && !pcmk__is_set_recursive(instance, pcmk__rsc_blocked, true)) { pcmk__rsc_trace(primary, "Allowing %s: %s %d", colocation->id, pcmk__node_name(chosen), chosen->assign->score); diff --git a/lib/pacemaker/pcmk_sched_instances.c b/lib/pacemaker/pcmk_sched_instances.c index 463358f8a9b..d540ba89470 100644 --- a/lib/pacemaker/pcmk_sched_instances.c +++ b/lib/pacemaker/pcmk_sched_instances.c @@ -1114,7 +1114,7 @@ pcmk__instance_matches(const pcmk_resource_t *instance, const pcmk_node_t *node, return false; } - if (!is_set_recursive(instance, pcmk__rsc_blocked, true)) { + if (!pcmk__is_set_recursive(instance, pcmk__rsc_blocked, true)) { uint32_t target = pcmk__rsc_node_assigned; if (current) { diff --git a/lib/pengine/clone.c b/lib/pengine/clone.c index f1d7781583e..a9e28b740fe 100644 --- a/lib/pengine/clone.c +++ b/lib/pengine/clone.c @@ -12,7 +12,7 @@ #include // va_arg, va_list #include // bool, true, false #include // NULL -#include // uint32_t +#include // uint32_t, uint64_t #include // free #include // strcmp, strchr @@ -515,33 +515,38 @@ configured_role(pcmk_resource_t *rsc) } bool -is_set_recursive(const pcmk_resource_t *rsc, long long flag, bool any) +pcmk__is_set_recursive(const pcmk_resource_t *rsc, uint64_t flag, bool any) { - bool all = !any; + const bool all = !any; + bool is_set = pcmk__is_set(rsc->flags, flag); - if (pcmk__is_set(rsc->flags, flag)) { - if(any) { - return TRUE; - } - } else if(all) { - return FALSE; + if (any && is_set) { + return true; } - for (GList *iter = rsc->priv->children; iter != NULL; iter = iter->next) { - if (is_set_recursive(iter->data, flag, any)) { - if(any) { - return TRUE; - } + if (all && !is_set) { + return false; + } - } else if(all) { - return FALSE; + for (const GList *iter = rsc->priv->children; iter != NULL; + iter = iter->next) { + + is_set = pcmk__is_set_recursive(iter->data, flag, any); + + if (any && is_set) { + return true; } - } - if(all) { - return TRUE; + if (all && !is_set) { + return false; + } } - return FALSE; + + /* If all is true, then flag was set for rsc and all of its descendants. + * Otherwise, any is true, and flag was not set for rsc or any of its + * descendants. + */ + return all; } PCMK__OUTPUT_ARGS("clone", "uint32_t", "pcmk_resource_t *", "GList *", @@ -710,9 +715,11 @@ pe__clone_default(pcmk__output_t *out, va_list args) pcmk__insert_dup(stopped, child_rsc->id, "Stopped"); } - } else if (is_set_recursive(child_rsc, pcmk__rsc_removed, TRUE) - || !is_set_recursive(child_rsc, pcmk__rsc_managed, FALSE) - || is_set_recursive(child_rsc, pcmk__rsc_failed, TRUE)) { + } else if (pcmk__is_set_recursive(child_rsc, pcmk__rsc_removed, true) + || !pcmk__is_set_recursive(child_rsc, pcmk__rsc_managed, + false) + || pcmk__is_set_recursive(child_rsc, pcmk__rsc_failed, + true)) { // Print individual instance when active removed/unmanaged/failed print_full = TRUE; From fd62b7d33661c9e086cfdb0e253c97bd612ea3c6 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 27 May 2026 17:48:06 -0700 Subject: [PATCH 74/75] Refactor: libpe_status: New rsc_managed_recursive() Signed-off-by: Reid Wahl --- lib/pengine/clone.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/lib/pengine/clone.c b/lib/pengine/clone.c index a9e28b740fe..7e87cd75526 100644 --- a/lib/pengine/clone.c +++ b/lib/pengine/clone.c @@ -514,6 +514,35 @@ configured_role(pcmk_resource_t *rsc) return role; } +/*! + * \internal + * \brief Check whether a resource and all of its descendants are managed + * + * \param[in] rsc Resource + * + * \return \c true if \p rsc and all of its descendants are managed, or \c false + * otherwise + */ +static bool +rsc_managed_recursive(const pcmk_resource_t *rsc) +{ + if (!pcmk__is_set(rsc->flags, pcmk__rsc_managed)) { + return false; + } + + for (const GList *iter = rsc->priv->children; iter != NULL; + iter = iter->next) { + + const pcmk_resource_t *child_rsc = iter->data; + + if (!rsc_managed_recursive(child_rsc)) { + return false; + } + } + + return true; +} + bool pcmk__is_set_recursive(const pcmk_resource_t *rsc, uint64_t flag, bool any) { @@ -716,8 +745,7 @@ pe__clone_default(pcmk__output_t *out, va_list args) } } else if (pcmk__is_set_recursive(child_rsc, pcmk__rsc_removed, true) - || !pcmk__is_set_recursive(child_rsc, pcmk__rsc_managed, - false) + || !rsc_managed_recursive(child_rsc) || pcmk__is_set_recursive(child_rsc, pcmk__rsc_failed, true)) { From 1b55802117706c0d306e9599b3d44a2c1195cad4 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 27 May 2026 17:51:34 -0700 Subject: [PATCH 75/75] Refactor: libpe_status: Drop pcmk__is_set_recursive() any argument We always pass true now. Signed-off-by: Reid Wahl --- include/crm/pengine/internal.h | 3 +-- lib/pacemaker/pcmk_sched_bundle.c | 2 +- lib/pacemaker/pcmk_sched_clone.c | 2 +- lib/pacemaker/pcmk_sched_instances.c | 2 +- lib/pengine/clone.c | 30 +++++++--------------------- 5 files changed, 11 insertions(+), 28 deletions(-) diff --git a/include/crm/pengine/internal.h b/include/crm/pengine/internal.h index d4d83dfb13d..1834db892ba 100644 --- a/include/crm/pengine/internal.h +++ b/include/crm/pengine/internal.h @@ -259,8 +259,7 @@ pe_base_name_eq(const pcmk_resource_t *rsc, const char *id) int pe__target_rc_from_xml(const xmlNode *xml_op); int pe__cmp_node_name(const void *a, const void *b); -bool pcmk__is_set_recursive(const pcmk_resource_t *rsc, uint64_t flag, - bool any); +bool pcmk__is_set_recursive(const pcmk_resource_t *rsc, uint64_t flag); pcmk__op_digest_t *pe__calculate_digests(pcmk_resource_t *rsc, const char *task, unsigned int *interval_ms, diff --git a/lib/pacemaker/pcmk_sched_bundle.c b/lib/pacemaker/pcmk_sched_bundle.c index 8c1d389b4dd..523705d982f 100644 --- a/lib/pacemaker/pcmk_sched_bundle.c +++ b/lib/pacemaker/pcmk_sched_bundle.c @@ -499,7 +499,7 @@ replica_apply_coloc_score(const pcmk__bundle_replica_t *replica, chosen = container->priv->fns->location(container, NULL, pcmk__rsc_node_assigned); if ((chosen == NULL) - || pcmk__is_set_recursive(container, pcmk__rsc_blocked, true)) { + || pcmk__is_set_recursive(container, pcmk__rsc_blocked)) { return true; } diff --git a/lib/pacemaker/pcmk_sched_clone.c b/lib/pacemaker/pcmk_sched_clone.c index 49435231951..0115f0df374 100644 --- a/lib/pacemaker/pcmk_sched_clone.c +++ b/lib/pacemaker/pcmk_sched_clone.c @@ -343,7 +343,7 @@ pcmk__clone_apply_coloc_score(pcmk_resource_t *dependent, chosen = instance->priv->fns->location(instance, NULL, pcmk__rsc_node_assigned); if ((chosen != NULL) - && !pcmk__is_set_recursive(instance, pcmk__rsc_blocked, true)) { + && !pcmk__is_set_recursive(instance, pcmk__rsc_blocked)) { pcmk__rsc_trace(primary, "Allowing %s: %s %d", colocation->id, pcmk__node_name(chosen), chosen->assign->score); diff --git a/lib/pacemaker/pcmk_sched_instances.c b/lib/pacemaker/pcmk_sched_instances.c index d540ba89470..f5dc3b93c85 100644 --- a/lib/pacemaker/pcmk_sched_instances.c +++ b/lib/pacemaker/pcmk_sched_instances.c @@ -1114,7 +1114,7 @@ pcmk__instance_matches(const pcmk_resource_t *instance, const pcmk_node_t *node, return false; } - if (!pcmk__is_set_recursive(instance, pcmk__rsc_blocked, true)) { + if (!pcmk__is_set_recursive(instance, pcmk__rsc_blocked)) { uint32_t target = pcmk__rsc_node_assigned; if (current) { diff --git a/lib/pengine/clone.c b/lib/pengine/clone.c index 7e87cd75526..35d1ded230a 100644 --- a/lib/pengine/clone.c +++ b/lib/pengine/clone.c @@ -544,38 +544,23 @@ rsc_managed_recursive(const pcmk_resource_t *rsc) } bool -pcmk__is_set_recursive(const pcmk_resource_t *rsc, uint64_t flag, bool any) +pcmk__is_set_recursive(const pcmk_resource_t *rsc, uint64_t flag) { - const bool all = !any; - bool is_set = pcmk__is_set(rsc->flags, flag); - - if (any && is_set) { + if (pcmk__is_set(rsc->flags, flag)) { return true; } - if (all && !is_set) { - return false; - } - for (const GList *iter = rsc->priv->children; iter != NULL; iter = iter->next) { - is_set = pcmk__is_set_recursive(iter->data, flag, any); + const pcmk_resource_t *child_rsc = iter->data; - if (any && is_set) { + if (pcmk__is_set_recursive(child_rsc, flag)) { return true; } - - if (all && !is_set) { - return false; - } } - /* If all is true, then flag was set for rsc and all of its descendants. - * Otherwise, any is true, and flag was not set for rsc or any of its - * descendants. - */ - return all; + return false; } PCMK__OUTPUT_ARGS("clone", "uint32_t", "pcmk_resource_t *", "GList *", @@ -744,10 +729,9 @@ pe__clone_default(pcmk__output_t *out, va_list args) pcmk__insert_dup(stopped, child_rsc->id, "Stopped"); } - } else if (pcmk__is_set_recursive(child_rsc, pcmk__rsc_removed, true) + } else if (pcmk__is_set_recursive(child_rsc, pcmk__rsc_removed) || !rsc_managed_recursive(child_rsc) - || pcmk__is_set_recursive(child_rsc, pcmk__rsc_failed, - true)) { + || pcmk__is_set_recursive(child_rsc, pcmk__rsc_failed)) { // Print individual instance when active removed/unmanaged/failed print_full = TRUE;