From 19e9e9ea67584e68650f1ec776afb77e1493e3cb Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Mon, 4 May 2026 17:20:59 -0700 Subject: [PATCH 01/50] 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 1c9cbe00c73..c8039ee5e0e 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 4ef60985f98..795185a243e 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1235,7 +1235,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 e883bcd39f4e51833c5a458067216e11418c491a Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Mon, 4 May 2026 23:26:23 -0700 Subject: [PATCH 02/50] 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 c8039ee5e0e..5e319bb1ebc 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 74afade6e4da28657371b6883e341b7c91071ad8 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Mon, 4 May 2026 23:29:30 -0700 Subject: [PATCH 03/50] 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 5e319bb1ebc..cc1ad7a9f1e 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 a09a5fa9ca04bded8c7c382731ab05f3f9110b42 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Mon, 4 May 2026 23:50:43 -0700 Subject: [PATCH 04/50] 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 | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/lib/common/mainloop.c b/lib/common/mainloop.c index 1a0e8dd4a72..ddeb9b7fe30 100644 --- a/lib/common/mainloop.c +++ b/lib/common/mainloop.c @@ -797,40 +797,28 @@ mainloop_gio_callback(GIOChannel *gio, GIOCondition condition, gpointer data) } static void -mainloop_gio_destroy(gpointer c) +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) (gpointer 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 07ddc7ac194b5b1b4df076ce83c4b020c279742a Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Mon, 4 May 2026 23:57:19 -0700 Subject: [PATCH 05/50] Refactor: libcrmcommon: Simplify freeing of client->channel Signed-off-by: Reid Wahl --- lib/common/mainloop.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/lib/common/mainloop.c b/lib/common/mainloop.c index ddeb9b7fe30..7ed395a4a06 100644 --- a/lib/common/mainloop.c +++ b/lib/common/mainloop.c @@ -817,6 +817,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,16 +967,6 @@ mainloop_add_fd(const char *name, int priority, int fd, void *userdata, (G_IO_IN | G_IO_HUP | G_IO_NVAL | G_IO_ERR), 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); } else { From 5436c1172330b7202f09c577c5d4b5d07f47c5c0 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 02:21:24 -0700 Subject: [PATCH 06/50] 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 d666e8a38c2..385982fad4b 100644 --- a/lib/lrmd/lrmd_client.c +++ b/lib/lrmd/lrmd_client.c @@ -279,7 +279,9 @@ lrmd_dispatch_internal(gpointer data, gpointer 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(gpointer data, gpointer 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(gpointer data, gpointer 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(gpointer data, gpointer 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 dcbba4020b1a0bad90275754e76bf70886dc0a31 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 02:28:25 -0700 Subject: [PATCH 07/50] 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 385982fad4b..c6a80399428 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(gpointer 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(gpointer 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(gpointer 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(gpointer 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 051f8465b9df74d8c9630e7cf61f595e8549b6c8 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 02:52:22 -0700 Subject: [PATCH 08/50] API: libcrmcommon: did_rsc_op_fail() argument is now const Signed-off-by: Reid Wahl --- include/crm/common/actions.h | 4 ++-- lib/common/actions.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/crm/common/actions.h b/include/crm/common/actions.h index 7c457656865..dfa779f231f 100644 --- a/include/crm/common/actions.h +++ b/include/crm/common/actions.h @@ -1,5 +1,5 @@ /* - * Copyright 2004-2025 the Pacemaker project contributors + * Copyright 2004-2026 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -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 b4a4ee04c19..04bdd741713 100644 --- a/lib/common/actions.c +++ b/lib/common/actions.c @@ -508,9 +508,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; @@ -525,7 +525,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 7c8a953761c80328d1c1497b81b42426187938e4 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 09:54:53 -0700 Subject: [PATCH 09/50] 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 c30b0b4a9a4..21e7a1b7f5d 100644 --- a/include/crm/lrmd_events.h +++ b/include/crm/lrmd_events.h @@ -105,7 +105,7 @@ typedef struct lrmd_event_data_s { lrmd_event_data_t *lrmd_new_event(const char *rsc_id, const char *task, guint 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 c6a80399428..4ef7c96d825 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, guint 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 3167a7191eb052c15a4c517fe0d84bb20807de65 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 10:01:59 -0700 Subject: [PATCH 10/50] 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 017bdab4516..e8020759716 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 24d3c415d6d..bc10350ad46 100644 --- a/daemons/controld/controld_execd.c +++ b/daemons/controld/controld_execd.c @@ -152,7 +152,8 @@ history_free(gpointer 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 90a0db7c2f6515901009cda449bfbd46d2e9bdc1 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 02:01:48 -0700 Subject: [PATCH 11/50] 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 | 110 +++++++++++++----------- 5 files changed, 106 insertions(+), 106 deletions(-) diff --git a/daemons/controld/controld_execd.c b/daemons/controld/controld_execd.c index bc10350ad46..46a5a67783c 100644 --- a/daemons/controld/controld_execd.c +++ b/daemons/controld/controld_execd.c @@ -1726,11 +1726,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 68ad291646b..a9cc9ae2283 100644 --- a/daemons/controld/controld_execd_state.c +++ b/daemons/controld/controld_execd_state.c @@ -55,31 +55,28 @@ free_recurring_op(gpointer value) static gboolean fail_pending_op(gpointer key, gpointer value, gpointer 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 695bf55af4d..45fba1df747 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(gpointer 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 21e7a1b7f5d..ff718b09e25 100644 --- a/include/crm/lrmd_events.h +++ b/include/crm/lrmd_events.h @@ -45,11 +45,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; @@ -73,7 +73,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; @@ -97,10 +97,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 4ef7c96d825..8f11827df6f 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, guint 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(gpointer data, gpointer 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,69 @@ lrmd_dispatch_internal(gpointer data, gpointer 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_guint(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_guint(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 +486,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 +649,7 @@ lrmd_ipc_connection_destroy(gpointer 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 +672,12 @@ lrmd_ipc_connection_destroy(gpointer 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 +1103,7 @@ lrmd_tls_connection_destroy(gpointer 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 +1134,12 @@ lrmd_tls_connection_destroy(gpointer 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 @@ -2411,9 +2419,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 c0dac64c1ad8b40eca9eeb51a1425d2836e3ae08 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 10:36:30 -0700 Subject: [PATCH 12/50] 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 eae7cc10ea183b8f76c5093798397e7860556322 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 12:05:54 -0700 Subject: [PATCH 13/50] 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 9e0c5fcc6d1..53211131ca3 100644 --- a/include/crm/lrmd_internal.h +++ b/include/crm/lrmd_internal.h @@ -49,8 +49,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 8f11827df6f..22e361b35f6 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); } @@ -2422,23 +2423,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 0ef6cad81b9b3af67c026e8c5fdfeb6cff277bf3 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 12:42:45 -0700 Subject: [PATCH 14/50] 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 1da33b4f99807ece87164e69ac8d077706174c4a Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 13:01:04 -0700 Subject: [PATCH 15/50] 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 | 4 +--- lib/fencing/st_actions.c | 4 ++-- lib/services/services.c | 38 --------------------------------- 4 files changed, 5 insertions(+), 45 deletions(-) diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c index 1095b722e5f..8021d7f63b1 100644 --- a/daemons/execd/execd_commands.c +++ b/daemons/execd/execd_commands.c @@ -1015,8 +1015,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 c282813eb96..9483426a504 100644 --- a/include/crm/services_internal.h +++ b/include/crm/services_internal.h @@ -1,5 +1,5 @@ /* - * Copyright 2010-2025 the Pacemaker project contributors + * Copyright 2010-2026 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -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 c23d1d48a2a..8b5f4009e56 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 e752fdac75f..b9b7c27735c 100644 --- a/lib/services/services.c +++ b/lib/services/services.c @@ -1340,44 +1340,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 a3901abd7facab20080544c2d33c30ad4e011f94 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 13:15:09 -0700 Subject: [PATCH 16/50] 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 8c5da82a325..a0dc8957be8 100644 --- a/daemons/fenced/fenced_commands.c +++ b/daemons/fenced/fenced_commands.c @@ -1954,11 +1954,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; @@ -1980,7 +1980,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); } @@ -2034,7 +2034,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 ee20c6718c6c516b35a9dacf6efccf4c858e4d67 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 13:17:05 -0700 Subject: [PATCH 17/50] 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 a0dc8957be8..a4331364348 100644 --- a/daemons/fenced/fenced_commands.c +++ b/daemons/fenced/fenced_commands.c @@ -1958,23 +1958,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 c89a4a6cb807fec3a68a15a6fca432390c2489a6 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 13:24:07 -0700 Subject: [PATCH 18/50] Refactor: fencer: Drop some elses in execute_agent_action() Signed-off-by: Reid Wahl --- daemons/fenced/fenced_commands.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/daemons/fenced/fenced_commands.c b/daemons/fenced/fenced_commands.c index a4331364348..424d8647e2c 100644 --- a/daemons/fenced/fenced_commands.c +++ b/daemons/fenced/fenced_commands.c @@ -2028,17 +2028,18 @@ execute_agent_action(xmlNode *msg, pcmk__action_result_t *result) if (fencing_watchdog_timeout_ms <= 0) { 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; } @@ -2052,9 +2053,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 b86b981bf257ad34607f29e85e1558915736dfc6 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 13:40:32 -0700 Subject: [PATCH 19/50] 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 424d8647e2c..7edf88cf95a 100644 --- a/daemons/fenced/fenced_commands.c +++ b/daemons/fenced/fenced_commands.c @@ -1953,40 +1953,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) @@ -2031,11 +1997,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 3d778d6dfd4b4bfc95ba3d61f7d233718090514d Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 5 May 2026 13:48:42 -0700 Subject: [PATCH 20/50] 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 8021d7f63b1..0cb5ee1599b 100644 --- a/daemons/execd/execd_commands.c +++ b/daemons/execd/execd_commands.c @@ -1015,8 +1015,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 7edf88cf95a..74f339e3c2c 100644 --- a/daemons/fenced/fenced_commands.c +++ b/daemons/fenced/fenced_commands.c @@ -2007,7 +2007,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 58861d35c9e..6021e364e62 100644 --- a/daemons/fenced/fenced_history.c +++ b/daemons/fenced/fenced_history.c @@ -284,10 +284,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 8b5f4009e56..8626301cd5a 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 22e361b35f6..4790efbd291 100644 --- a/lib/lrmd/lrmd_client.c +++ b/lib/lrmd/lrmd_client.c @@ -2306,11 +2306,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 180c18083608f8773281d74fa033de7b15207478 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Fri, 8 May 2026 21:39:30 -0700 Subject: [PATCH 21/50] 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 | 18 ++---------------- lib/cib/cib_native.c | 14 ++------------ lib/cib/cib_remote.c | 14 ++------------ 4 files changed, 8 insertions(+), 51 deletions(-) diff --git a/lib/cib/cib_client.c b/lib/cib/cib_client.c index 83f55b6f15c..d0163312b3c 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 f409b3e61d4..4524d062021 100644 --- a/lib/cib/cib_file.c +++ b/lib/cib/cib_file.c @@ -758,23 +758,9 @@ 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; - } + filename = pcmk__str_copy(cib_location); + private = pcmk__assert_alloc(1, sizeof(file_opaque_t)); private->id = pcmk__generate_uuid(); private->filename = filename; diff --git a/lib/cib/cib_native.c b/lib/cib/cib_native.c index 57c8c87545e..e8bf64d8b4d 100644 --- a/lib/cib/cib_native.c +++ b/lib/cib/cib_native.c @@ -489,19 +489,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 c2d0fc63e74..dc0c1e2665e 100644 --- a/lib/cib/cib_remote.c +++ b/lib/cib/cib_remote.c @@ -717,19 +717,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 64d4e579e5e02c529d0749d6e502b440f2d4e24d Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 15:40:22 -0700 Subject: [PATCH 22/50] 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 6906c74059d..91c2c87983b 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); @@ -405,7 +405,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); guint n_envvars = 0; recipients++; From 82eaefc9dd231f4ac681595c39d6d26996eff95e Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 16:43:44 -0700 Subject: [PATCH 23/50] 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 91c2c87983b..6be53f3d7d9 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) @@ -346,6 +322,34 @@ unpack_alert(xmlNode *alert, pcmk__alert_t *entry, guint *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 @@ -405,12 +409,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); guint 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 5d53405393d8c390e24a26aab5316bdf0c1b9d4e Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 16:58:33 -0700 Subject: [PATCH 24/50] 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 795185a243e..42a44fa1960 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1100,11 +1100,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 @@ -1147,12 +1154,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 434491ddf0115cc6fe5d8649869ec05f17ab1798 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 17:01:48 -0700 Subject: [PATCH 25/50] 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 42a44fa1960..1f8f166f5f6 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -958,7 +958,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); @@ -982,7 +982,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 @@ -1090,7 +1090,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", @@ -1167,7 +1167,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 @@ -1242,7 +1242,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, @@ -1266,7 +1266,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; @@ -1276,7 +1276,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 @@ -1309,7 +1309,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 1c66c9f04333e6a633a91c275cc3d94184a68989 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 17:06:39 -0700 Subject: [PATCH 26/50] 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 | 8 ++--- lib/pengine/unpack.c | 31 +++++++--------- lib/pengine/utils.c | 50 +++++++++++--------------- 10 files changed, 163 insertions(+), 202 deletions(-) diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c index 1f8f166f5f6..068dd9b5f6e 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -628,7 +628,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; @@ -752,8 +752,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; @@ -873,9 +873,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; @@ -1158,7 +1157,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, @@ -1218,13 +1216,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++; @@ -1269,9 +1267,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); @@ -1383,9 +1380,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)) { @@ -1421,9 +1417,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; @@ -1588,9 +1583,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; @@ -1739,9 +1733,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 47c7ff18917..3d68fbc043f 100644 --- a/lib/pengine/clone.c +++ b/lib/pengine/clone.c @@ -445,10 +445,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) { @@ -510,10 +508,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; } @@ -556,10 +552,8 @@ pe__clone_xml(pcmk__output_t *out, va_list args) all = g_list_prepend(all, (gpointer) "*"); - 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; @@ -630,7 +624,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; @@ -653,9 +646,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)) { @@ -762,8 +755,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)) { @@ -785,8 +778,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)) { @@ -949,10 +942,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 bb6cb62c2d2..5cfaf238396 100644 --- a/lib/pengine/group.c +++ b/lib/pengine/group.c @@ -99,10 +99,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++; @@ -239,10 +237,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; @@ -285,17 +281,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, @@ -369,9 +363,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; @@ -406,10 +401,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 acf6b839f09..27a010d3476 100644 --- a/lib/pengine/native.c +++ b/lib/pengine/native.c @@ -66,10 +66,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, @@ -91,10 +91,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; @@ -137,7 +137,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 */ @@ -145,8 +145,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; } } @@ -163,9 +166,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); @@ -311,10 +315,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) { @@ -327,10 +329,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", @@ -804,10 +806,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, @@ -910,10 +912,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); } @@ -938,10 +940,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)) { @@ -957,10 +957,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); @@ -998,10 +996,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 @@ -1055,8 +1053,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; @@ -1069,8 +1067,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 d71c800cf04..3ebbe79d06a 100644 --- a/lib/pengine/pe_actions.c +++ b/lib/pengine/pe_actions.c @@ -1141,8 +1141,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, @@ -1171,7 +1171,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) { @@ -1189,8 +1188,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; @@ -1264,8 +1263,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; @@ -1387,8 +1386,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; @@ -1413,13 +1412,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; @@ -1456,8 +1454,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 4b200a5bb00..ccef06234bc 100644 --- a/lib/pengine/pe_notif.c +++ b/lib/pengine/pe_notif.c @@ -201,8 +201,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 7b43314937e..8fb85021b71 100644 --- a/lib/pengine/pe_output.c +++ b/lib/pengine/pe_output.c @@ -68,10 +68,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; @@ -379,9 +377,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; @@ -730,13 +730,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)) { @@ -750,8 +750,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"); @@ -1979,13 +1981,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, @@ -2364,8 +2366,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; @@ -2566,8 +2568,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)) { @@ -2601,8 +2603,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)); @@ -2705,8 +2707,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)) { @@ -3172,12 +3174,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 45a15f7ee07..6e123c203ca 100644 --- a/lib/pengine/remote.c +++ b/lib/pengine/remote.c @@ -1,5 +1,5 @@ /* - * Copyright 2013-2025 the Pacemaker project contributors + * Copyright 2013-2026 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -34,10 +34,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 1430a58a146..14c321a3b2b 100644 --- a/lib/pengine/unpack.c +++ b/lib/pengine/unpack.c @@ -831,7 +831,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); @@ -871,10 +870,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); @@ -1472,8 +1471,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; @@ -2595,10 +2594,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); } @@ -2627,14 +2625,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; guint interval_ms = 0; char *key = NULL; @@ -2774,7 +2771,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; @@ -2839,8 +2835,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); } @@ -5037,7 +5033,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; @@ -5070,8 +5065,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 7906fbe6305..9aa316d028b 100644 --- a/lib/pengine/utils.c +++ b/lib/pengine/utils.c @@ -153,10 +153,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, (gpointer) new_node->priv->id, new_node); } return result; @@ -217,8 +217,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)); @@ -306,10 +306,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); @@ -370,10 +368,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); } @@ -401,10 +399,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); } @@ -480,7 +476,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; @@ -499,9 +494,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)) { @@ -587,11 +581,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); } } @@ -612,11 +605,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); } } @@ -787,8 +779,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 551a0c93c23bd8f625e5a079df0b5e88f984bea2 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 17:25:29 -0700 Subject: [PATCH 27/50] 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 068dd9b5f6e..8df623de58c 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -285,7 +285,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; @@ -964,7 +964,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); @@ -1056,7 +1056,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); @@ -1199,7 +1199,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 @@ -1212,7 +1212,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 c1d394d0d7deeca73443c3866c82ee5cc4ed4635 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 17:35:05 -0700 Subject: [PATCH 28/50] 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 8df623de58c..6f586640452 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -943,6 +943,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), \ @@ -966,20 +1000,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 5f6bfcd2bf823071bd1cb0d5be494f0c4a41ddb3 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 18:05:56 -0700 Subject: [PATCH 29/50] 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 6f586640452..b1bd3de3444 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -977,6 +977,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), \ @@ -1000,43 +1057,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 e8aa6b9776f4e7c41fe24bb61d4a880f40d33053 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 18:27:43 -0700 Subject: [PATCH 30/50] 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 b1bd3de3444..f066da18d2f 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1034,6 +1034,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), \ @@ -1043,7 +1112,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; @@ -1061,57 +1129,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 0edb3ffdc4f80e84ca872fd536a0797073d45333 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 18:47:57 -0700 Subject: [PATCH 31/50] 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 f066da18d2f..2f452cafbf5 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1103,20 +1103,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); @@ -1131,34 +1181,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); @@ -1255,7 +1278,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 40e552bb9775abe6358e45e01552daa0faa9085d Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 19:02:50 -0700 Subject: [PATCH 32/50] 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 2f452cafbf5..41c39fabb4a 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1160,95 +1160,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 cd9c603b21bba0db662827de32d53874bb1b0954 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 19:10:28 -0700 Subject: [PATCH 33/50] 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 41c39fabb4a..abcf0f8e614 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1182,8 +1182,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) { @@ -1214,24 +1214,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 aecff95e3a90979161798519d64b95d80ce36fbd Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 19:20:35 -0700 Subject: [PATCH 34/50] 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 abcf0f8e614..638951d152c 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -352,33 +352,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); @@ -413,7 +417,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; @@ -423,31 +426,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"); } @@ -456,32 +463,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, ","); } @@ -495,15 +504,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, @@ -515,36 +524,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); @@ -563,16 +572,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? @@ -625,8 +634,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; @@ -637,12 +645,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); @@ -666,10 +677,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); @@ -790,22 +801,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; } @@ -1382,7 +1392,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 44b8f6df1c6746dc8a7f1cd657d9b7e10b91a5f9 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 21:54:44 -0700 Subject: [PATCH 35/50] 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 638951d152c..e68d49ff84b 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -319,38 +319,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) { @@ -649,7 +617,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; } @@ -1170,6 +1138,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 c38849e56a49240867bcefeca97276a2b50c6765 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 22:16:56 -0700 Subject: [PATCH 36/50] 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 e68d49ff84b..663aedb5d85 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1140,34 +1140,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; } /*! @@ -1200,15 +1207,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 6615a79c248181d93e31279cccfb0c6970cc505e Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 19:12:43 -0700 Subject: [PATCH 37/50] 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 663aedb5d85..5145c1a96cf 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1170,8 +1170,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 066a4434dd9be644b424b63fac7b0c2de7851428 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 22:19:12 -0700 Subject: [PATCH 38/50] 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 5145c1a96cf..5fa04b19910 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1240,7 +1240,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 876e458480fe0784fc209166cdc0ad1e04c83d5e Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 22:26:01 -0700 Subject: [PATCH 39/50] 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 5fa04b19910..7084b41bb2b 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1195,21 +1195,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; } @@ -1224,35 +1225,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 27f627d328b9478a6bd4f532092e5c8197df1964 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 22:50:20 -0700 Subject: [PATCH 40/50] 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 7084b41bb2b..c49589b2c66 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1256,6 +1256,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) { @@ -1283,101 +1389,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 d56430559de3cb5c76d56b83d137a1803cd1ba3c Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 22:54:00 -0700 Subject: [PATCH 41/50] 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 c49589b2c66..e088c32a83a 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -334,14 +334,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); @@ -408,14 +406,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); @@ -432,7 +429,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); } @@ -457,8 +454,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, ","); } @@ -621,7 +617,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); @@ -1168,7 +1164,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); } @@ -1223,11 +1219,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); @@ -1250,7 +1245,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 4eb60dea81133f7d01bf1b0d8bcf6ed8b5eaa6c3 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 23:00:07 -0700 Subject: [PATCH 42/50] 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 e088c32a83a..a76d95f6965 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -290,12 +290,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); @@ -396,13 +390,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 76af8bd235c2b1850d0e6a69e0969567a26b3e81 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 23:07:07 -0700 Subject: [PATCH 43/50] 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 a76d95f6965..002dc007970 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -275,29 +275,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 * @@ -1322,7 +1327,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 @@ -1387,7 +1392,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 435ef6ec0e3bbf22edefad3293190d600339e602 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 23:07:47 -0700 Subject: [PATCH 44/50] 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 002dc007970..f5db0242270 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -52,7 +52,6 @@ typedef struct { int promoted_max; int nreplicas; int nreplicas_per_host; - char *prefix; char *image; const char *ip_last; char *host_network; @@ -1361,7 +1360,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; @@ -1990,7 +1988,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 dfacfc10b200ead4617b1c366d6d785436d41013 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 23:16:20 -0700 Subject: [PATCH 45/50] 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 f5db0242270..44eb9689c5f 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1348,6 +1348,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) { @@ -1382,19 +1403,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 b7507990b1c14885cbf98a2613a39591751e74c8 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 23:26:12 -0700 Subject: [PATCH 46/50] 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 44eb9689c5f..6031ec79b61 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -1411,7 +1411,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; } @@ -1441,10 +1440,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 0e612fe6d9f8647083cc3ef4231672b05f66b6c5 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 13 May 2026 23:40:49 -0700 Subject: [PATCH 47/50] 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 6031ec79b61..49f92f96bd6 100644 --- a/lib/pengine/bundle.c +++ b/lib/pengine/bundle.c @@ -790,6 +790,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; } @@ -1413,31 +1438,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 a9b1f6676f3aa2c5a71f3dbc223abf86c362b587 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Thu, 14 May 2026 01:31:22 -0700 Subject: [PATCH 48/50] 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 8e655d1f6e3..991fb88c3c7 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 e38b48677b6e8c0ee0fde1dd334c400109515d56 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Thu, 14 May 2026 02:08:19 -0700 Subject: [PATCH 49/50] 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 670054bda656af374acad82b6d771c4907da8bf8 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Thu, 14 May 2026 02:22:46 -0700 Subject: [PATCH 50/50] 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; }