From 171d24c656b2a4d36a7babd2ba58e4106f14a858 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 26 Jan 2026 10:15:37 -0500 Subject: [PATCH 01/20] Low: daemons: Don't return a NACK if execd gets an unknown message. No other daemon behaves this way on unknown message, and this isn't old code. I just added it in 7d56d4665c8 without mentioning any reasoning behind it. I can't find any clients that are expecting a NACK in this case so we should be good to just change this. Ref T991 --- daemons/execd/execd_messages.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/daemons/execd/execd_messages.c b/daemons/execd/execd_messages.c index 59be6f18e47..a397b3501b6 100644 --- a/daemons/execd/execd_messages.c +++ b/daemons/execd/execd_messages.c @@ -395,7 +395,7 @@ static xmlNode * handle_unknown_request(pcmk__request_t *request) { pcmk__ipc_send_ack(request->ipc_client, request->ipc_id, request->ipc_flags, - PCMK__XE_NACK, NULL, CRM_EX_PROTOCOL); + PCMK__XE_ACK, NULL, CRM_EX_PROTOCOL); pcmk__format_result(&request->result, CRM_EX_PROTOCOL, PCMK_EXEC_INVALID, "Unknown request type '%s' (bug?)", From d675185ccb5a8574f42aa6caaa16883ca1fd336f Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 26 Jan 2026 10:37:15 -0500 Subject: [PATCH 02/20] Low: liblrmd: Explicitly handle receiving a NACK from execd. It's possible for execd to reply with an ACK message with an error code. This happens if execd receives an invalid message - this could be a programming error, or it could potentially be a problem in transmission. The client code was only incidentally handling these messages. Before, it would fall through to the "Invalid registration message" case and be handled that way. However, because it's part of the protocol for the server to respond with a NACK on errors, it seems more correct to me for us to explicitly handle this message first thing. Note that this doesn't change any behavior. Before the client would get a message it didn't expect and the IPC connection would fail. Now the client gets an error message and the IPC connection will fail. I just like the more defined behavior. Ref T991 --- lib/lrmd/lrmd_client.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/lib/lrmd/lrmd_client.c b/lib/lrmd/lrmd_client.c index 906bede4cdf..a2b1b409c26 100644 --- a/lib/lrmd/lrmd_client.c +++ b/lib/lrmd/lrmd_client.c @@ -1021,6 +1021,22 @@ process_lrmd_handshake_reply(xmlNode *reply, lrmd_private_t *native) const char *tmp_ticket = pcmk__xe_get(reply, PCMK__XA_LRMD_CLIENTID); const char *start_state = pcmk__xe_get(reply, PCMK__XA_NODE_START_STATE); + /* If we received a NACK in response, the executor thinks we originally + * sent an invalid message. + */ + if (pcmk__xe_is(reply, PCMK__XE_NACK)) { + int status = 0; + + rc = pcmk__xe_get_int(reply, PCMK_XA_STATUS, &status); + + if ((rc == pcmk_rc_ok) && (status != 0)) { + pcmk__err("Received error response from executor: %s", + crm_exit_str(status)); + pcmk__log_xml_err(reply, "Bad reply"); + return EPROTO; + } + } + pcmk__xe_get_int(reply, PCMK__XA_LRMD_RC, &rc); rc = pcmk_legacy2rc(rc); From 8e6931f434782f8f1314e707182995450aeaee0a Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 26 Jan 2026 13:32:12 -0500 Subject: [PATCH 03/20] Refactor: daemons: Don't return a NACK on invalid execd messages. It's confusing to have both ACKs and NACKs in the IPC protocol when both of those can also include an error code. Most daemons do not send a NACK anyway, and most (all?) clients aren't set up to handle it. In the interest of simplifying the IPC code a bit, I'm going to remove NACKs so clients can always expect an ACK and read the error code to know what to do. Ref T991 --- daemons/execd/execd_ipc.c | 2 +- lib/lrmd/lrmd_client.c | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/daemons/execd/execd_ipc.c b/daemons/execd/execd_ipc.c index fb9a0d8f6ac..281189cdb40 100644 --- a/daemons/execd/execd_ipc.c +++ b/daemons/execd/execd_ipc.c @@ -170,7 +170,7 @@ execd_ipc_dispatch(qb_ipcs_connection_t *c, void *data, size_t size) if ((msg == NULL) || execd_invalid_msg(msg)) { pcmk__debug("Unrecognizable IPC data from PID %d", pcmk__client_pid(c)); - pcmk__ipc_send_ack(client, id, flags, PCMK__XE_NACK, NULL, CRM_EX_PROTOCOL); + pcmk__ipc_send_ack(client, id, flags, PCMK__XE_ACK, NULL, CRM_EX_PROTOCOL); } else { pcmk__request_t request = { .ipc_client = client, diff --git a/lib/lrmd/lrmd_client.c b/lib/lrmd/lrmd_client.c index a2b1b409c26..e3fcac50961 100644 --- a/lib/lrmd/lrmd_client.c +++ b/lib/lrmd/lrmd_client.c @@ -1021,10 +1021,15 @@ process_lrmd_handshake_reply(xmlNode *reply, lrmd_private_t *native) const char *tmp_ticket = pcmk__xe_get(reply, PCMK__XA_LRMD_CLIENTID); const char *start_state = pcmk__xe_get(reply, PCMK__XA_NODE_START_STATE); - /* If we received a NACK in response, the executor thinks we originally - * sent an invalid message. + /* If we received an ACK with an error status in response, the executor + * thinks we originally sent an invalid message. + * + * NOTE: At the moment, all ACK messages sent in the handshake process + * will have an error status. However, this may change in the future so + * we'll let those fall through to the rest of the message handling below + * so we get some log messages should we change that in the future. */ - if (pcmk__xe_is(reply, PCMK__XE_NACK)) { + if (pcmk__xe_is(reply, PCMK__XE_ACK)) { int status = 0; rc = pcmk__xe_get_int(reply, PCMK_XA_STATUS, &status); From 3384d1c4490b9dcf1a37290e47551a9d49468385 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 26 Jan 2026 15:11:27 -0500 Subject: [PATCH 04/20] Refactor: libcib: Unindent the end of cib_native_signon. There's no doubt more that could be done along these lines, but for the moment this will have to do. I need to add some additional error handling into this function so it makes sense to unindent to make some room. --- lib/cib/cib_native.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/lib/cib/cib_native.c b/lib/cib/cib_native.c index 2c54b4a7bab..0d3fbc71719 100644 --- a/lib/cib/cib_native.c +++ b/lib/cib/cib_native.c @@ -1,6 +1,6 @@ /* * Copyright 2004 International Business Machines - * Later changes copyright 2004-2025 the Pacemaker project contributors + * Later changes copyright 2004-2026 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -315,33 +315,35 @@ cib_native_signon(cib_t *cib, const char *name, enum cib_conn_type type) if (rc == pcmk_ok) { xmlNode *reply = NULL; + const char *msg_type = NULL; if (crm_ipc_send(native->ipc, hello, crm_ipc_client_response, -1, - &reply) > 0) { - const char *msg_type = pcmk__xe_get(reply, PCMK__XA_CIB_OP); + &reply) <= 0) { + rc = -ECOMM; + goto done; + } - pcmk__log_xml_trace(reply, "reg-reply"); + msg_type = pcmk__xe_get(reply, PCMK__XA_CIB_OP); - if (!pcmk__str_eq(msg_type, CRM_OP_REGISTER, pcmk__str_casei)) { - pcmk__info("Reply to CIB registration message has unknown type " - "'%s'", - msg_type); - rc = -EPROTO; + pcmk__log_xml_trace(reply, "reg-reply"); - } else { - native->token = pcmk__xe_get_copy(reply, PCMK__XA_CIB_CLIENTID); - if (native->token == NULL) { - rc = -EPROTO; - } - } - pcmk__xml_free(reply); + if (!pcmk__str_eq(msg_type, CRM_OP_REGISTER, pcmk__str_casei)) { + pcmk__info("Reply to CIB registration message has unknown type " + "'%s'", + msg_type); + rc = -EPROTO; } else { - rc = -ECOMM; + native->token = pcmk__xe_get_copy(reply, PCMK__XA_CIB_CLIENTID); + if (native->token == NULL) { + rc = -EPROTO; + } } - pcmk__xml_free(hello); } +done: + pcmk__xml_free(hello); + if (rc == pcmk_ok) { pcmk__info("Successfully connected to CIB manager for %s", name); return pcmk_ok; From 89801e0e09ff9e2855e890f2d8ade28c4056a676 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 26 Jan 2026 15:22:50 -0500 Subject: [PATCH 05/20] Low: libcib: Explicitly handle receiving a NACK from based. It's possible for based to reply with an ACK message with an error code. This happens if based receives an invalid message - this could be a programming error, or it could potentially be a problem in transmission. The client code was only incidentally handling these messages. Before, it would fall through to the "Reply to the CIB registration message has unknown type" case and be handled that way. However, because it's part of the protocol for the server to respond with a NACK on errors, it seems more correct to me for us to explicitly handle this message first thing. Note that this doesn't change any behavior. Before the client would get a message it didn't expect and the IPC connection would fail. Now the client gets an error message and the IPC connection will fail. I just like the more defined behavior. Ref T991 --- lib/cib/cib_native.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/lib/cib/cib_native.c b/lib/cib/cib_native.c index 0d3fbc71719..2b50a4dbdf1 100644 --- a/lib/cib/cib_native.c +++ b/lib/cib/cib_native.c @@ -323,10 +323,25 @@ cib_native_signon(cib_t *cib, const char *name, enum cib_conn_type type) goto done; } - msg_type = pcmk__xe_get(reply, PCMK__XA_CIB_OP); - pcmk__log_xml_trace(reply, "reg-reply"); + /* If we received a NACK in response, based thinks we originally + * sent an invalid message. + */ + if (pcmk__xe_is(reply, PCMK__XE_NACK)) { + int status = 0; + + rc = pcmk__xe_get_int(reply, PCMK_XA_STATUS, &status); + + if ((rc == pcmk_rc_ok) && (status != 0)) { + pcmk__err("Received error response from CIB manager: %s", + crm_exit_str(status)); + return -EPROTO; + } + } + + msg_type = pcmk__xe_get(reply, PCMK__XA_CIB_OP); + if (!pcmk__str_eq(msg_type, CRM_OP_REGISTER, pcmk__str_casei)) { pcmk__info("Reply to CIB registration message has unknown type " "'%s'", From d9c7587e190d95f28a9129577df6e7e2e13bfed1 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 26 Jan 2026 15:28:32 -0500 Subject: [PATCH 06/20] Refactor: libcib: Minor refactorings in cib_tls_signon. * Change the tag that pcmk__log_xml_trace logs with to be the same as in cib_native_signon. * Use g_clear_pointer when cleaning up the answer variable. * Unindent the code at the end of the function. I'm going to be adding additional error handling here too, so it's nice to have some extra room to do so. --- lib/cib/cib_remote.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/lib/cib/cib_remote.c b/lib/cib/cib_remote.c index d25e8008231..66be83c8b91 100644 --- a/lib/cib/cib_remote.c +++ b/lib/cib/cib_remote.c @@ -1,5 +1,5 @@ /* - * Copyright 2008-2025 the Pacemaker project contributors + * Copyright 2008-2026 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -357,6 +357,8 @@ cib_tls_signon(cib_t *cib, pcmk__remote_t *connection, gboolean event_channel) xmlNode *answer = NULL; xmlNode *login = NULL; + const char *msg_type = NULL; + const char *tmp_ticket = NULL; static struct mainloop_fd_callbacks cib_fd_callbacks = { 0, }; @@ -429,28 +431,30 @@ cib_tls_signon(cib_t *cib, pcmk__remote_t *connection, gboolean event_channel) answer = pcmk__remote_message_xml(connection); - pcmk__log_xml_trace(answer, "Reply"); if (answer == NULL) { rc = -EPROTO; + goto done; + } - } else { - /* grab the token */ - const char *msg_type = pcmk__xe_get(answer, PCMK__XA_CIB_OP); - const char *tmp_ticket = pcmk__xe_get(answer, PCMK__XA_CIB_CLIENTID); + pcmk__log_xml_trace(answer, "reg-reply"); - if (!pcmk__str_eq(msg_type, CRM_OP_REGISTER, pcmk__str_casei)) { - pcmk__err("Invalid registration message: %s", msg_type); - rc = -EPROTO; + /* grab the token */ + msg_type = pcmk__xe_get(answer, PCMK__XA_CIB_OP); + tmp_ticket = pcmk__xe_get(answer, PCMK__XA_CIB_CLIENTID); - } else if (tmp_ticket == NULL) { - rc = -EPROTO; + if (!pcmk__str_eq(msg_type, CRM_OP_REGISTER, pcmk__str_casei)) { + pcmk__err("Invalid registration message: %s", msg_type); + rc = -EPROTO; - } else { - connection->token = strdup(tmp_ticket); - } + } else if (tmp_ticket == NULL) { + rc = -EPROTO; + + } else { + connection->token = strdup(tmp_ticket); } - pcmk__xml_free(answer); - answer = NULL; + +done: + g_clear_pointer(&answer, pcmk__xml_free); if (rc != 0) { cib_tls_close(cib); From 7a9f5f763b1ead79117dd858f7ee26ed6adc41c6 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 26 Jan 2026 15:32:17 -0500 Subject: [PATCH 07/20] Low: libcib: Explicitly handle receiving a NACK from remote based. This is just like previous patches, but applies to signing on to a remote CIB instead. Ref T991 --- lib/cib/cib_remote.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/lib/cib/cib_remote.c b/lib/cib/cib_remote.c index 66be83c8b91..e5cb235a9ec 100644 --- a/lib/cib/cib_remote.c +++ b/lib/cib/cib_remote.c @@ -438,6 +438,21 @@ cib_tls_signon(cib_t *cib, pcmk__remote_t *connection, gboolean event_channel) pcmk__log_xml_trace(answer, "reg-reply"); + /* If we received a NACK in response, based thinks we originally + * sent an invalid message. + */ + if (pcmk__xe_is(answer, PCMK__XE_NACK)) { + int status = 0; + + rc = pcmk__xe_get_int(answer, PCMK_XA_STATUS, &status); + + if ((rc == pcmk_rc_ok) && (status != 0)) { + pcmk__err("Received error response from CIB manager: %s", + crm_exit_str(status)); + return -EPROTO; + } + } + /* grab the token */ msg_type = pcmk__xe_get(answer, PCMK__XA_CIB_OP); tmp_ticket = pcmk__xe_get(answer, PCMK__XA_CIB_CLIENTID); From 16f7a9ce31a365d3825ee95c7324a840856d2d9e Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 26 Jan 2026 15:51:28 -0500 Subject: [PATCH 08/20] Refactor: daemons: Don't return a NACK on invalid based messages. It's confusing to have both ACKs and NACKs in the IPC protocol when both of those can also include an error code. Most daemons do not send a NACK anyway, and most (all?) clients aren't set up to handle it. In the interest of simplifying the IPC code a bit, I'm going to remove NACKs so clients can always expect an ACK and read the error code to know what to do. Ref T991 --- daemons/based/based_ipc.c | 2 +- lib/cib/cib_native.c | 11 ++++++++--- lib/cib/cib_remote.c | 11 ++++++++--- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/daemons/based/based_ipc.c b/daemons/based/based_ipc.c index 55606f12fd4..6d572decfa2 100644 --- a/daemons/based/based_ipc.c +++ b/daemons/based/based_ipc.c @@ -122,7 +122,7 @@ dispatch_common(qb_ipcs_connection_t *c, void *data, bool privileged) if (msg == NULL) { pcmk__debug("Unrecognizable IPC data from PID %d", pcmk__client_pid(c)); - pcmk__ipc_send_ack(client, id, flags, PCMK__XE_NACK, NULL, + pcmk__ipc_send_ack(client, id, flags, PCMK__XE_ACK, NULL, CRM_EX_PROTOCOL); return 0; } diff --git a/lib/cib/cib_native.c b/lib/cib/cib_native.c index 2b50a4dbdf1..0c8afd17d6e 100644 --- a/lib/cib/cib_native.c +++ b/lib/cib/cib_native.c @@ -325,10 +325,15 @@ cib_native_signon(cib_t *cib, const char *name, enum cib_conn_type type) pcmk__log_xml_trace(reply, "reg-reply"); - /* If we received a NACK in response, based thinks we originally - * sent an invalid message. + /* If we received an ACK with an error status in response, based + * thinks we originally sent an invalid message. + * + * NOTE: At the moment, all ACK messages sent in the signon process + * will have an error status. However, this may change in the future so + * we'll let those fall through to the rest of the message handling below + * so we get some log messages should we change that in the future. */ - if (pcmk__xe_is(reply, PCMK__XE_NACK)) { + if (pcmk__xe_is(reply, PCMK__XE_ACK)) { int status = 0; rc = pcmk__xe_get_int(reply, PCMK_XA_STATUS, &status); diff --git a/lib/cib/cib_remote.c b/lib/cib/cib_remote.c index e5cb235a9ec..fa7c13761d1 100644 --- a/lib/cib/cib_remote.c +++ b/lib/cib/cib_remote.c @@ -438,10 +438,15 @@ cib_tls_signon(cib_t *cib, pcmk__remote_t *connection, gboolean event_channel) pcmk__log_xml_trace(answer, "reg-reply"); - /* If we received a NACK in response, based thinks we originally - * sent an invalid message. + /* If we received an ACK with an error status in response, based + * thinks we originally sent an invalid message. + * + * NOTE: At the moment, all ACK messages sent in the signon process + * will have an error status. However, this may change in the future so + * we'll let those fall through to the rest of the message handling below + * so we get some log messages should we change that in the future. */ - if (pcmk__xe_is(answer, PCMK__XE_NACK)) { + if (pcmk__xe_is(answer, PCMK__XE_ACK)) { int status = 0; rc = pcmk__xe_get_int(answer, PCMK_XA_STATUS, &status); From f70225fce5fb29c771007034374f4304c0031a03 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 26 Jan 2026 16:21:02 -0500 Subject: [PATCH 09/20] Refactor: libstonith: Unindent one level of stonith_api_signon. I'm going to be adding some additional error handling in this function, so it'll be nice to have some room to do so. This only unindents the first level of the end of this function. There's more to do, but it's easier to follow if it's broken up into two patches. --- lib/fencing/st_client.c | 82 +++++++++++++++++++++-------------------- 1 file changed, 43 insertions(+), 39 deletions(-) diff --git a/lib/fencing/st_client.c b/lib/fencing/st_client.c index 971aadf8d64..07df4c1c444 100644 --- a/lib/fencing/st_client.c +++ b/lib/fencing/st_client.c @@ -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. * @@ -1128,6 +1128,9 @@ stonith_api_signon(stonith_t * stonith, const char *name, int *stonith_fd) stonith_private_t *native = NULL; const char *display_name = name? name : "client"; + xmlNode *reply = NULL; + xmlNode *hello = NULL; + struct ipc_client_callbacks st_callbacks = { .dispatch = stonith_dispatch_internal, .destroy = stonith_connection_destroy @@ -1170,53 +1173,54 @@ stonith_api_signon(stonith_t * stonith, const char *name, int *stonith_fd) if (native->ipc == NULL) { rc = -ENOTCONN; - } else { - xmlNode *reply = NULL; - xmlNode *hello = pcmk__xe_create(NULL, PCMK__XE_STONITH_COMMAND); + goto done; + } - pcmk__xe_set(hello, PCMK__XA_T, PCMK__VALUE_STONITH_NG); - pcmk__xe_set(hello, PCMK__XA_ST_OP, CRM_OP_REGISTER); - pcmk__xe_set(hello, PCMK__XA_ST_CLIENTNAME, name); - rc = crm_ipc_send(native->ipc, hello, crm_ipc_client_response, -1, &reply); + hello = pcmk__xe_create(NULL, PCMK__XE_STONITH_COMMAND); - if (rc < 0) { - pcmk__debug("Couldn't register with the fencer: %s " QB_XS " rc=%d", - pcmk_strerror(rc), rc); - rc = -ECOMM; + pcmk__xe_set(hello, PCMK__XA_T, PCMK__VALUE_STONITH_NG); + pcmk__xe_set(hello, PCMK__XA_ST_OP, CRM_OP_REGISTER); + pcmk__xe_set(hello, PCMK__XA_ST_CLIENTNAME, name); + rc = crm_ipc_send(native->ipc, hello, crm_ipc_client_response, -1, &reply); + + if (rc < 0) { + pcmk__debug("Couldn't register with the fencer: %s " QB_XS " rc=%d", + pcmk_strerror(rc), rc); + rc = -ECOMM; + + } else if (reply == NULL) { + pcmk__debug("Couldn't register with the fencer: no reply"); + rc = -EPROTO; - } else if (reply == NULL) { - pcmk__debug("Couldn't register with the fencer: no reply"); + } else { + const char *msg_type = pcmk__xe_get(reply, PCMK__XA_ST_OP); + + native->token = pcmk__xe_get_copy(reply, PCMK__XA_ST_CLIENTID); + if (!pcmk__str_eq(msg_type, CRM_OP_REGISTER, pcmk__str_none)) { + pcmk__debug("Couldn't register with the fencer: invalid reply " + "type '%s'", + pcmk__s(msg_type, "(missing)")); + pcmk__log_xml_debug(reply, "Invalid fencer reply"); rc = -EPROTO; - } else { - const char *msg_type = pcmk__xe_get(reply, PCMK__XA_ST_OP); - - native->token = pcmk__xe_get_copy(reply, PCMK__XA_ST_CLIENTID); - if (!pcmk__str_eq(msg_type, CRM_OP_REGISTER, pcmk__str_none)) { - pcmk__debug("Couldn't register with the fencer: invalid reply " - "type '%s'", - pcmk__s(msg_type, "(missing)")); - pcmk__log_xml_debug(reply, "Invalid fencer reply"); - rc = -EPROTO; - - } else if (native->token == NULL) { - pcmk__debug("Couldn't register with the fencer: no token in " - "reply"); - pcmk__log_xml_debug(reply, "Invalid fencer reply"); - rc = -EPROTO; + } else if (native->token == NULL) { + pcmk__debug("Couldn't register with the fencer: no token in " + "reply"); + pcmk__log_xml_debug(reply, "Invalid fencer reply"); + rc = -EPROTO; - } else { - pcmk__debug("Connection to fencer by %s succeeded " - "(registration token: %s)", - display_name, native->token); - rc = pcmk_ok; - } + } else { + pcmk__debug("Connection to fencer by %s succeeded " + "(registration token: %s)", + display_name, native->token); + rc = pcmk_ok; } - - pcmk__xml_free(reply); - pcmk__xml_free(hello); } +done: + pcmk__xml_free(reply); + pcmk__xml_free(hello); + if (rc != pcmk_ok) { pcmk__debug("Connection attempt to fencer by %s failed: %s " QB_XS " rc=%d", From 8e47ecda6dfa840a056a4c81af82e409fc5f1e15 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 26 Jan 2026 16:24:00 -0500 Subject: [PATCH 10/20] Refactor: libstonith: Additional unindenting in stonith_api_signon. --- lib/fencing/st_client.c | 49 ++++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/lib/fencing/st_client.c b/lib/fencing/st_client.c index 07df4c1c444..d4d91f0bb66 100644 --- a/lib/fencing/st_client.c +++ b/lib/fencing/st_client.c @@ -1130,6 +1130,7 @@ stonith_api_signon(stonith_t * stonith, const char *name, int *stonith_fd) xmlNode *reply = NULL; xmlNode *hello = NULL; + const char *msg_type = NULL; struct ipc_client_callbacks st_callbacks = { .dispatch = stonith_dispatch_internal, @@ -1187,34 +1188,36 @@ stonith_api_signon(stonith_t * stonith, const char *name, int *stonith_fd) pcmk__debug("Couldn't register with the fencer: %s " QB_XS " rc=%d", pcmk_strerror(rc), rc); rc = -ECOMM; + goto done; + } - } else if (reply == NULL) { + if (reply == NULL) { pcmk__debug("Couldn't register with the fencer: no reply"); rc = -EPROTO; + goto done; + } - } else { - const char *msg_type = pcmk__xe_get(reply, PCMK__XA_ST_OP); - - native->token = pcmk__xe_get_copy(reply, PCMK__XA_ST_CLIENTID); - if (!pcmk__str_eq(msg_type, CRM_OP_REGISTER, pcmk__str_none)) { - pcmk__debug("Couldn't register with the fencer: invalid reply " - "type '%s'", - pcmk__s(msg_type, "(missing)")); - pcmk__log_xml_debug(reply, "Invalid fencer reply"); - rc = -EPROTO; - - } else if (native->token == NULL) { - pcmk__debug("Couldn't register with the fencer: no token in " - "reply"); - pcmk__log_xml_debug(reply, "Invalid fencer reply"); - rc = -EPROTO; + msg_type = pcmk__xe_get(reply, PCMK__XA_ST_OP); - } else { - pcmk__debug("Connection to fencer by %s succeeded " - "(registration token: %s)", - display_name, native->token); - rc = pcmk_ok; - } + native->token = pcmk__xe_get_copy(reply, PCMK__XA_ST_CLIENTID); + if (!pcmk__str_eq(msg_type, CRM_OP_REGISTER, pcmk__str_none)) { + pcmk__debug("Couldn't register with the fencer: invalid reply " + "type '%s'", + pcmk__s(msg_type, "(missing)")); + pcmk__log_xml_debug(reply, "Invalid fencer reply"); + rc = -EPROTO; + + } else if (native->token == NULL) { + pcmk__debug("Couldn't register with the fencer: no token in " + "reply"); + pcmk__log_xml_debug(reply, "Invalid fencer reply"); + rc = -EPROTO; + + } else { + pcmk__debug("Connection to fencer by %s succeeded " + "(registration token: %s)", + display_name, native->token); + rc = pcmk_ok; } done: From bcef05c80ee379da5b9b089bf909d0909332bf5a Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 26 Jan 2026 16:26:37 -0500 Subject: [PATCH 11/20] Low: libcib: Explicitly handle receiving a NACK from fenced. It's possible for fenced to reply with an ACK message with an error code. This happens if fenced receives an invalid message - this could be a programming error, or it could potentially be a problem in transmission. The client code was only incidentally handling these messages. Before, it would fall through to the "invalid reply type" case and be handled that way. However, because it's part of the protocol for the server to respond with a NACK on errors, it seems more correct to me for us to explicitly handle this message first thing. Note that this doesn't change any behavior. Before the client would get a message it didn't expect and the IPC connection would fail. Now the client gets an error message and the IPC connection will fail. I just like the more defined behavior. Ref T991 --- lib/fencing/st_client.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/lib/fencing/st_client.c b/lib/fencing/st_client.c index d4d91f0bb66..f4d3d790bdd 100644 --- a/lib/fencing/st_client.c +++ b/lib/fencing/st_client.c @@ -1197,6 +1197,21 @@ stonith_api_signon(stonith_t * stonith, const char *name, int *stonith_fd) goto done; } + /* If we received a NACK in response, fenced thinks we originally + * sent an invalid message. + */ + if (pcmk__xe_is(reply, PCMK__XE_NACK)) { + int status = 0; + + rc = pcmk__xe_get_int(reply, PCMK_XA_STATUS, &status); + + if ((rc == pcmk_rc_ok) && (status != 0)) { + pcmk__err("Received error response from CIB manager: %s", + crm_exit_str(status)); + return -EPROTO; + } + } + msg_type = pcmk__xe_get(reply, PCMK__XA_ST_OP); native->token = pcmk__xe_get_copy(reply, PCMK__XA_ST_CLIENTID); From 668515305da4572e6c02ff964811372e890076d4 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 26 Jan 2026 16:36:54 -0500 Subject: [PATCH 12/20] Refactor: daemons: Don't return a NACK on invalid fenced messages. It's confusing to have both ACKs and NACKs in the IPC protocol when both of those can also include an error code. Most daemons do not send a NACK anyway, and most (all?) clients aren't set up to handle it. In the interest of simplifying the IPC code a bit, I'm going to remove NACKs so clients can always expect an ACK and read the error code to know what to do. Ref T991 --- daemons/fenced/fenced_ipc.c | 2 +- lib/fencing/st_client.c | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/daemons/fenced/fenced_ipc.c b/daemons/fenced/fenced_ipc.c index d2e9bd6fcb1..ffb7b418a6c 100644 --- a/daemons/fenced/fenced_ipc.c +++ b/daemons/fenced/fenced_ipc.c @@ -137,7 +137,7 @@ fenced_ipc_dispatch(qb_ipcs_connection_t *c, void *data, size_t size) if (msg == NULL) { pcmk__debug("Unrecognizable IPC data from PID %d", pcmk__client_pid(c)); - pcmk__ipc_send_ack(client, id, flags, PCMK__XE_NACK, NULL, CRM_EX_PROTOCOL); + pcmk__ipc_send_ack(client, id, flags, PCMK__XE_ACK, NULL, CRM_EX_PROTOCOL); return 0; } diff --git a/lib/fencing/st_client.c b/lib/fencing/st_client.c index f4d3d790bdd..9cc5fe6e20e 100644 --- a/lib/fencing/st_client.c +++ b/lib/fencing/st_client.c @@ -1197,10 +1197,15 @@ stonith_api_signon(stonith_t * stonith, const char *name, int *stonith_fd) goto done; } - /* If we received a NACK in response, fenced thinks we originally - * sent an invalid message. + /* If we received an ACK with an error status in response, fenced + * thinks we originally sent an invalid message. + * + * NOTE: At the moment, all ACK messages sent in the signon process + * will have an error status. However, this may change in the future so + * we'll let those fall through to the rest of the message handling below + * so we get some log messages should we change that in the future. */ - if (pcmk__xe_is(reply, PCMK__XE_NACK)) { + if (pcmk__xe_is(reply, PCMK__XE_ACK)) { int status = 0; rc = pcmk__xe_get_int(reply, PCMK_XA_STATUS, &status); From b31e0d2bfa09e1c55ce73b473d2b8c920f67b8f2 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Tue, 27 Jan 2026 11:36:32 -0500 Subject: [PATCH 13/20] Low: daemons: Don't NACK an invalid remote message. I've been testing the handling of invalid messages by making sure execd_invalid_msg always returns true. This will trigger for the very first message a daemon receives, which makes it pretty easy to experiment with. In this case, the first message is register: (pcmk__remote_message_xml@remote.c:358) trace: [remote msg] In real life, we could be getting an invalid message by something being garbled or a misbehaving client. Tracing through the code that got us to the above log message, you'll see that the remote message was almost certainly received via the TLS channel (it could also be a TCP socket, but I haven't seen anything use that). However, ACKs/NACKs are an IPC mechanism, which happens on the local system only and uses completely different communications channels. There is a way to proxy IPC so that it's retransmitted to another system, but we only proxy IPC if the received message has lrmd_op="ipc_fwd". In this case, the local system is the end point of the invalid message. We would also be originating the ACK/NACK, which would mean we'd need to be able to construct some sort of fake proxied message and put it in the TLS or TCP channel. I guess it's possible we could do this, but it seems like a lot of work when the client isn't even expecting it (see handle_remote_msg). This was just recently added in 7acc4f30d72 and has not been in any release, so there's no mixed version upgrade concerns here. Ref T991 --- daemons/execd/remoted_tls.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/daemons/execd/remoted_tls.c b/daemons/execd/remoted_tls.c index 1654c06ae32..3f48879c6d6 100644 --- a/daemons/execd/remoted_tls.c +++ b/daemons/execd/remoted_tls.c @@ -127,16 +127,9 @@ lrmd_remote_client_msg(gpointer data) msg = pcmk__remote_message_xml(client->remote); - if (msg == NULL) { + if ((msg == NULL) || execd_invalid_msg(msg)) { pcmk__debug("Unrecognizable IPC data from PID %d", client->pid); - } else if (execd_invalid_msg(msg)) { - int id = 0; - pcmk__debug("Unrecognizable IPC data from PID %d", client->pid); - - pcmk__xe_get_int(msg, PCMK__XA_LRMD_REMOTE_MSG_ID, &id); - pcmk__ipc_send_ack(client, id, crm_ipc_client_response, PCMK__XE_NACK, - NULL, CRM_EX_PROTOCOL); } else { pcmk__request_t request = { .ipc_client = client, From 1fc55a3915f3b3b1eaeb53d99ffc190e0091b722 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Tue, 27 Jan 2026 13:06:38 -0500 Subject: [PATCH 14/20] Low: daemons: Delay creating an xmlNode in handle_register_request. That way, we don't have to free it if we're just going to return right away. --- daemons/fenced/fenced_commands.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/daemons/fenced/fenced_commands.c b/daemons/fenced/fenced_commands.c index f0d4d4dbad1..b8ff70852ec 100644 --- a/daemons/fenced/fenced_commands.c +++ b/daemons/fenced/fenced_commands.c @@ -1,5 +1,5 @@ /* - * Copyright 2009-2025 the Pacemaker project contributors + * Copyright 2009-2026 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -3297,12 +3297,13 @@ handle_unknown_request(pcmk__request_t *request) static xmlNode * handle_register_request(pcmk__request_t *request) { - xmlNode *reply = pcmk__xe_create(NULL, "reply"); + xmlNode *reply = NULL; if (request->peer != NULL) { return handle_unknown_request(request); } + reply = pcmk__xe_create(NULL, "reply"); pcmk__xe_set(reply, PCMK__XA_ST_OP, CRM_OP_REGISTER); pcmk__xe_set(reply, PCMK__XA_ST_CLIENTID, request->ipc_client->id); pcmk__set_result(&request->result, CRM_EX_OK, PCMK_EXEC_DONE, NULL); From 3a887df2d8d6f03fc2ca0e2326dbb86dae65e3e8 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Tue, 27 Jan 2026 14:20:05 -0500 Subject: [PATCH 15/20] Refactor: liblrmd: Fix log messages in remote_proxy_cb. Due to the if/else block, in all these log messages, op will be "request". This means all these log messages say something along the lines of "Relayed request request 1 from...". --- lib/lrmd/proxy_common.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/lib/lrmd/proxy_common.c b/lib/lrmd/proxy_common.c index 7c81dbd3757..6e4343cab71 100644 --- a/lib/lrmd/proxy_common.c +++ b/lib/lrmd/proxy_common.c @@ -292,11 +292,10 @@ remote_proxy_cb(lrmd_t *lrmd, const char *node_name, xmlNode *msg) if(rc < 0) { xmlNode *op_reply = pcmk__xe_create(NULL, PCMK__XE_NACK); - pcmk__err("Could not relay %s request %d from %s to %s for %s: " + pcmk__err("Could not relay request %d from %s to %s for %s: " "%s (%d)", - op, msg_id, proxy->node_name, - crm_ipc_name(proxy->ipc), name, pcmk_strerror(rc), - rc); + msg_id, proxy->node_name, crm_ipc_name(proxy->ipc), + name, pcmk_strerror(rc), rc); /* Send a n'ack so the caller doesn't block */ pcmk__xe_set(op_reply, PCMK_XA_FUNCTION, __func__); @@ -306,9 +305,8 @@ remote_proxy_cb(lrmd_t *lrmd, const char *node_name, xmlNode *msg) pcmk__xml_free(op_reply); } else { - pcmk__trace("Relayed %s request %d from %s to %s for %s", op, - msg_id, proxy->node_name, crm_ipc_name(proxy->ipc), - name); + pcmk__trace("Relayed request %d from %s to %s for %s", msg_id, + proxy->node_name, crm_ipc_name(proxy->ipc), name); proxy->last_request_id = msg_id; } @@ -317,20 +315,18 @@ remote_proxy_cb(lrmd_t *lrmd, const char *node_name, xmlNode *msg) xmlNode *op_reply = NULL; // @COMPAT pacemaker_remoted <= 1.1.10 - pcmk__trace("Relaying %s request %d from %s to %s for %s", op, - msg_id, proxy->node_name, crm_ipc_name(proxy->ipc), - name); + pcmk__trace("Relaying request %d from %s to %s for %s", msg_id, + proxy->node_name, crm_ipc_name(proxy->ipc), name); rc = crm_ipc_send(proxy->ipc, request, flags, 10000, &op_reply); if(rc < 0) { - pcmk__err("Could not relay %s request %d from %s to %s for %s: " + pcmk__err("Could not relay request %d from %s to %s for %s: " "%s (%d)", - op, msg_id, proxy->node_name, crm_ipc_name(proxy->ipc), + msg_id, proxy->node_name, crm_ipc_name(proxy->ipc), name, pcmk_strerror(rc), rc); } else { - pcmk__trace("Relayed %s request %d from %s to %s for %s", op, - msg_id, proxy->node_name, crm_ipc_name(proxy->ipc), - name); + pcmk__trace("Relayed request %d from %s to %s for %s", msg_id, + proxy->node_name, crm_ipc_name(proxy->ipc), name); } if(op_reply) { From f80cf3425bc299055882607432410fb7193618db Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 28 Jan 2026 10:48:51 -0500 Subject: [PATCH 16/20] Refactor: liblrmd: Minor refactorings in remote_proxy_cb. * Make the comment explaining why we're sending a NACK a little more verbose so we can understand it better in the future. * Return after sending the NACK, allowing the code under it to be unindented. There's tons more of this that could be done, but I'm not going to worry about that right now. Ref T991 --- lib/lrmd/proxy_common.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/lrmd/proxy_common.c b/lib/lrmd/proxy_common.c index 6e4343cab71..ada1d5e4830 100644 --- a/lib/lrmd/proxy_common.c +++ b/lib/lrmd/proxy_common.c @@ -289,7 +289,7 @@ remote_proxy_cb(lrmd_t *lrmd, const char *node_name, xmlNode *msg) rc = crm_ipc_send(proxy->ipc, request, flags, 5000, NULL); - if(rc < 0) { + if (rc < 0) { xmlNode *op_reply = pcmk__xe_create(NULL, PCMK__XE_NACK); pcmk__err("Could not relay request %d from %s to %s for %s: " @@ -297,19 +297,25 @@ remote_proxy_cb(lrmd_t *lrmd, const char *node_name, xmlNode *msg) msg_id, proxy->node_name, crm_ipc_name(proxy->ipc), name, pcmk_strerror(rc), rc); - /* Send a n'ack so the caller doesn't block */ + /* Send a NACK to the caller (for instance, a program like + * cibadmin or crm_mon running on the remote node) so it doesn't + * block waiting for a reply. Nothing actually checks that it + * receives a PCMK__XE_NACK, but it's got to receive something + * and since this message isn't being used anywhere else, it's + * a good one to use. + */ pcmk__xe_set(op_reply, PCMK_XA_FUNCTION, __func__); pcmk__xe_set_int(op_reply, PCMK__XA_LINE, __LINE__); pcmk__xe_set_int(op_reply, PCMK_XA_RC, rc); remote_proxy_relay_response(proxy, op_reply, msg_id); pcmk__xml_free(op_reply); - - } else { - pcmk__trace("Relayed request %d from %s to %s for %s", msg_id, - proxy->node_name, crm_ipc_name(proxy->ipc), name); - proxy->last_request_id = msg_id; + return; } + pcmk__trace("Relayed request %d from %s to %s for %s", msg_id, + proxy->node_name, crm_ipc_name(proxy->ipc), name); + proxy->last_request_id = msg_id; + } else { int rc = pcmk_ok; xmlNode *op_reply = NULL; From ebd782740f0a797a2cf479118570f3164a5d14cd Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 28 Jan 2026 11:17:57 -0500 Subject: [PATCH 17/20] Refactor: libcrmcommon: Remove the tag argument from pcmk__ipc_create_ack_as. All ACKs are now created with PCMK__XE_ACK so this argument serves no purpose. Ref T991 --- include/crm/common/ipc_internal.h | 4 ++-- lib/common/ipc_server.c | 21 +++++++++++---------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/include/crm/common/ipc_internal.h b/include/crm/common/ipc_internal.h index e29ceb08e37..a312fe8e454 100644 --- a/include/crm/common/ipc_internal.h +++ b/include/crm/common/ipc_internal.h @@ -216,9 +216,9 @@ void pcmk__drop_all_clients(qb_ipcs_service_t *s); void pcmk__set_client_queue_max(pcmk__client_t *client, const char *qmax); xmlNode *pcmk__ipc_create_ack_as(const char *function, int line, uint32_t flags, - const char *tag, const char *ver, crm_exit_t status); + const char *ver, crm_exit_t status); #define pcmk__ipc_create_ack(flags, tag, ver, st) \ - pcmk__ipc_create_ack_as(__func__, __LINE__, (flags), (tag), (ver), (st)) + pcmk__ipc_create_ack_as(__func__, __LINE__, (flags), (ver), (st)) int pcmk__ipc_send_ack_as(const char *function, int line, pcmk__client_t *c, uint32_t request, uint32_t flags, const char *tag, diff --git a/lib/common/ipc_server.c b/lib/common/ipc_server.c index 5e7aa6e6ed3..15910201ef8 100644 --- a/lib/common/ipc_server.c +++ b/lib/common/ipc_server.c @@ -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. * @@ -957,7 +957,6 @@ pcmk__ipc_send_xml(pcmk__client_t *c, uint32_t request, const xmlNode *message, * \param[in] function Calling function * \param[in] line Source file line within calling function * \param[in] flags IPC flags to use when sending - * \param[in] tag Element name to use for acknowledgement * \param[in] ver IPC protocol version (can be NULL) * \param[in] status Exit status code to add to ack * @@ -968,17 +967,19 @@ pcmk__ipc_send_xml(pcmk__client_t *c, uint32_t request, const xmlNode *message, */ xmlNode * pcmk__ipc_create_ack_as(const char *function, int line, uint32_t flags, - const char *tag, const char *ver, crm_exit_t status) + const char *ver, crm_exit_t status) { xmlNode *ack = NULL; - if (pcmk__is_set(flags, crm_ipc_client_response)) { - ack = pcmk__xe_create(NULL, tag); - pcmk__xe_set(ack, PCMK_XA_FUNCTION, function); - pcmk__xe_set_int(ack, PCMK__XA_LINE, line); - pcmk__xe_set_int(ack, PCMK_XA_STATUS, (int) status); - pcmk__xe_set(ack, PCMK__XA_IPC_PROTO_VERSION, ver); + if (!pcmk__is_set(flags, crm_ipc_client_response)) { + return ack; } + + ack = pcmk__xe_create(NULL, PCMK__XE_ACK); + pcmk__xe_set(ack, PCMK_XA_FUNCTION, function); + pcmk__xe_set_int(ack, PCMK__XA_LINE, line); + pcmk__xe_set_int(ack, PCMK_XA_STATUS, (int) status); + pcmk__xe_set(ack, PCMK__XA_IPC_PROTO_VERSION, ver); return ack; } @@ -1003,7 +1004,7 @@ pcmk__ipc_send_ack_as(const char *function, int line, pcmk__client_t *c, const char *ver, crm_exit_t status) { int rc = pcmk_rc_ok; - xmlNode *ack = pcmk__ipc_create_ack_as(function, line, flags, tag, ver, status); + xmlNode *ack = pcmk__ipc_create_ack_as(function, line, flags, ver, status); if (ack != NULL) { pcmk__trace("Ack'ing IPC message from client %s as <%s status=%d>", From 5110f40882a2b899a4d0364b042326538a5116f7 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 28 Jan 2026 11:19:25 -0500 Subject: [PATCH 18/20] Refactor: libcrmcommon: Remove the tag argument from pcmk__ipc_create_ack. All ACKs are now created with PCMK__XE_ACK so this argument serves no purpose. Ref T991 --- daemons/fenced/fenced_commands.c | 3 +-- include/crm/common/ipc_internal.h | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/daemons/fenced/fenced_commands.c b/daemons/fenced/fenced_commands.c index b8ff70852ec..ff26a9e4ccc 100644 --- a/daemons/fenced/fenced_commands.c +++ b/daemons/fenced/fenced_commands.c @@ -3418,8 +3418,7 @@ handle_notify_request(pcmk__request_t *request) pcmk__set_result(&request->result, CRM_EX_OK, PCMK_EXEC_DONE, NULL); pcmk__set_request_flags(request, pcmk__request_reuse_options); - return pcmk__ipc_create_ack(request->ipc_flags, PCMK__XE_ACK, NULL, - CRM_EX_OK); + return pcmk__ipc_create_ack(request->ipc_flags, NULL, CRM_EX_OK); } // STONITH_OP_RELAY diff --git a/include/crm/common/ipc_internal.h b/include/crm/common/ipc_internal.h index a312fe8e454..5686707e4fa 100644 --- a/include/crm/common/ipc_internal.h +++ b/include/crm/common/ipc_internal.h @@ -217,7 +217,7 @@ void pcmk__set_client_queue_max(pcmk__client_t *client, const char *qmax); xmlNode *pcmk__ipc_create_ack_as(const char *function, int line, uint32_t flags, const char *ver, crm_exit_t status); -#define pcmk__ipc_create_ack(flags, tag, ver, st) \ +#define pcmk__ipc_create_ack(flags, ver, st) \ pcmk__ipc_create_ack_as(__func__, __LINE__, (flags), (ver), (st)) int pcmk__ipc_send_ack_as(const char *function, int line, pcmk__client_t *c, From 1d030bd963125c96c8f367ff81f4943a354c58fb Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 28 Jan 2026 11:22:40 -0500 Subject: [PATCH 19/20] Refactor: libcrmcommon: Remove the tag argument from pcmk__ipc_send_ack_as. All ACKs are now created with PCMK__XE_ACK so this argument serves no purpose. Ref T991 --- include/crm/common/ipc_internal.h | 6 +++--- lib/common/ipc_server.c | 22 ++++++++++++---------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/include/crm/common/ipc_internal.h b/include/crm/common/ipc_internal.h index 5686707e4fa..288454fe09f 100644 --- a/include/crm/common/ipc_internal.h +++ b/include/crm/common/ipc_internal.h @@ -221,10 +221,10 @@ xmlNode *pcmk__ipc_create_ack_as(const char *function, int line, uint32_t flags, pcmk__ipc_create_ack_as(__func__, __LINE__, (flags), (ver), (st)) int pcmk__ipc_send_ack_as(const char *function, int line, pcmk__client_t *c, - uint32_t request, uint32_t flags, const char *tag, - const char *ver, crm_exit_t status); + uint32_t request, uint32_t flags, const char *ver, + crm_exit_t status); #define pcmk__ipc_send_ack(c, req, flags, tag, ver, st) \ - pcmk__ipc_send_ack_as(__func__, __LINE__, (c), (req), (flags), (tag), (ver), (st)) + pcmk__ipc_send_ack_as(__func__, __LINE__, (c), (req), (flags), (ver), (st)) int pcmk__ipc_prepare_iov(uint32_t request, const GString *message, uint16_t index, struct iovec **result, ssize_t *bytes); diff --git a/lib/common/ipc_server.c b/lib/common/ipc_server.c index 15910201ef8..f1f8c072c18 100644 --- a/lib/common/ipc_server.c +++ b/lib/common/ipc_server.c @@ -992,7 +992,6 @@ pcmk__ipc_create_ack_as(const char *function, int line, uint32_t flags, * \param[in] c Client to send ack to * \param[in] request Request ID being replied to * \param[in] flags IPC flags to use when sending - * \param[in] tag Element name to use for acknowledgement * \param[in] ver IPC protocol version (can be NULL) * \param[in] status Status code to send with acknowledgement * @@ -1000,20 +999,23 @@ pcmk__ipc_create_ack_as(const char *function, int line, uint32_t flags, */ int pcmk__ipc_send_ack_as(const char *function, int line, pcmk__client_t *c, - uint32_t request, uint32_t flags, const char *tag, - const char *ver, crm_exit_t status) + uint32_t request, uint32_t flags, const char *ver, + crm_exit_t status) { int rc = pcmk_rc_ok; xmlNode *ack = pcmk__ipc_create_ack_as(function, line, flags, ver, status); - if (ack != NULL) { - pcmk__trace("Ack'ing IPC message from client %s as <%s status=%d>", - pcmk__client_name(c), tag, status); - pcmk__log_xml_trace(ack, "sent-ack"); - c->request_id = 0; - rc = pcmk__ipc_send_xml(c, request, ack, flags); - pcmk__xml_free(ack); + if (ack == NULL) { + return rc; } + + pcmk__trace("Ack'ing IPC message from client %s as <" PCMK__XE_ACK + " status=%d>", + pcmk__client_name(c), status); + pcmk__log_xml_trace(ack, "sent-ack"); + c->request_id = 0; + rc = pcmk__ipc_send_xml(c, request, ack, flags); + pcmk__xml_free(ack); return rc; } From e1411c8e412ff37067e8677171d1b98f5562d3a9 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 28 Jan 2026 11:29:28 -0500 Subject: [PATCH 20/20] Refactor: libcrmcommon: Remove the tag argument from pcmk__ipc_send_ack. All ACKs are now created with PCMK__XE_ACK so this argument serves no purpose. Fixes T991 --- daemons/attrd/attrd_ipc.c | 3 +-- daemons/attrd/attrd_sync.c | 5 ++--- daemons/attrd/pacemaker-attrd.h | 8 ++++---- daemons/based/based_ipc.c | 5 ++--- daemons/controld/controld_control.c | 8 +++----- daemons/execd/execd_ipc.c | 2 +- daemons/execd/execd_messages.c | 2 +- daemons/fenced/fenced_ipc.c | 2 +- daemons/pacemakerd/pcmkd_ipc.c | 3 +-- daemons/pacemakerd/pcmkd_messages.c | 10 +++++----- daemons/schedulerd/schedulerd_ipc.c | 9 +++------ daemons/schedulerd/schedulerd_messages.c | 8 ++++---- include/crm/common/ipc_internal.h | 2 +- 13 files changed, 29 insertions(+), 38 deletions(-) diff --git a/daemons/attrd/attrd_ipc.c b/daemons/attrd/attrd_ipc.c index af8b4afdf5a..cc7c36f85f5 100644 --- a/daemons/attrd/attrd_ipc.c +++ b/daemons/attrd/attrd_ipc.c @@ -596,8 +596,7 @@ attrd_ipc_dispatch(qb_ipcs_connection_t * c, void *data, size_t size) if (xml == NULL) { pcmk__debug("Unrecognizable IPC data from PID %d", pcmk__client_pid(c)); - pcmk__ipc_send_ack(client, id, flags, PCMK__XE_ACK, NULL, - CRM_EX_PROTOCOL); + pcmk__ipc_send_ack(client, id, flags, NULL, CRM_EX_PROTOCOL); return 0; } else { diff --git a/daemons/attrd/attrd_sync.c b/daemons/attrd/attrd_sync.c index 8477338d00d..fb2efcbd4c4 100644 --- a/daemons/attrd/attrd_sync.c +++ b/daemons/attrd/attrd_sync.c @@ -1,5 +1,5 @@ /* - * Copyright 2022-2025 the Pacemaker project contributors + * Copyright 2022-2026 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -382,8 +382,7 @@ confirmation_timeout_cb(gpointer data) client->id); pcmk__ipc_send_ack(client, action->ipc_id, action->flags|crm_ipc_client_response, - PCMK__XE_ACK, ATTRD_PROTOCOL_VERSION, - CRM_EX_TIMEOUT); + ATTRD_PROTOCOL_VERSION, CRM_EX_TIMEOUT); g_hash_table_iter_remove(&iter); pcmk__trace("%u requests now in expected confirmations table", diff --git a/daemons/attrd/pacemaker-attrd.h b/daemons/attrd/pacemaker-attrd.h index 83a3204bdec..41926990523 100644 --- a/daemons/attrd/pacemaker-attrd.h +++ b/daemons/attrd/pacemaker-attrd.h @@ -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. * @@ -53,9 +53,9 @@ #define ATTRD_SUPPORTS_MULTI_MESSAGE(x) ((x) >= 4) #define ATTRD_SUPPORTS_CONFIRMATION(x) ((x) >= 5) -#define attrd_send_ack(client, id, flags) \ - pcmk__ipc_send_ack((client), (id), (flags), PCMK__XE_ACK, \ - ATTRD_PROTOCOL_VERSION, CRM_EX_INDETERMINATE) +#define attrd_send_ack(client, id, flags) \ + pcmk__ipc_send_ack((client), (id), (flags), ATTRD_PROTOCOL_VERSION, \ + CRM_EX_INDETERMINATE) void attrd_init_mainloop(void); void attrd_run_mainloop(void); diff --git a/daemons/based/based_ipc.c b/daemons/based/based_ipc.c index 6d572decfa2..327e14cad7e 100644 --- a/daemons/based/based_ipc.c +++ b/daemons/based/based_ipc.c @@ -122,8 +122,7 @@ dispatch_common(qb_ipcs_connection_t *c, void *data, bool privileged) if (msg == NULL) { pcmk__debug("Unrecognizable IPC data from PID %d", pcmk__client_pid(c)); - pcmk__ipc_send_ack(client, id, flags, PCMK__XE_ACK, NULL, - CRM_EX_PROTOCOL); + pcmk__ipc_send_ack(client, id, flags, NULL, CRM_EX_PROTOCOL); return 0; } @@ -200,7 +199,7 @@ dispatch_common(qb_ipcs_connection_t *c, void *data, bool privileged) status = CRM_EX_INVALID_PARAM; } - pcmk__ipc_send_ack(client, id, flags, PCMK__XE_ACK, NULL, status); + pcmk__ipc_send_ack(client, id, flags, NULL, status); return 0; } diff --git a/daemons/controld/controld_control.c b/daemons/controld/controld_control.c index a371ce707fc..95223bf9ed3 100644 --- a/daemons/controld/controld_control.c +++ b/daemons/controld/controld_control.c @@ -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. * @@ -404,12 +404,10 @@ dispatch_controller_ipc(qb_ipcs_connection_t * c, void *data, size_t size) } if (msg == NULL) { - pcmk__ipc_send_ack(client, id, flags, PCMK__XE_ACK, NULL, - CRM_EX_PROTOCOL); + pcmk__ipc_send_ack(client, id, flags, NULL, CRM_EX_PROTOCOL); return 0; } - pcmk__ipc_send_ack(client, id, flags, PCMK__XE_ACK, NULL, - CRM_EX_INDETERMINATE); + pcmk__ipc_send_ack(client, id, flags, NULL, CRM_EX_INDETERMINATE); pcmk__assert(client->user != NULL); pcmk__update_acl_user(msg, PCMK__XA_CRM_USER, client->user); diff --git a/daemons/execd/execd_ipc.c b/daemons/execd/execd_ipc.c index 281189cdb40..3bee3752ba1 100644 --- a/daemons/execd/execd_ipc.c +++ b/daemons/execd/execd_ipc.c @@ -170,7 +170,7 @@ execd_ipc_dispatch(qb_ipcs_connection_t *c, void *data, size_t size) if ((msg == NULL) || execd_invalid_msg(msg)) { pcmk__debug("Unrecognizable IPC data from PID %d", pcmk__client_pid(c)); - pcmk__ipc_send_ack(client, id, flags, PCMK__XE_ACK, NULL, CRM_EX_PROTOCOL); + pcmk__ipc_send_ack(client, id, flags, NULL, CRM_EX_PROTOCOL); } else { pcmk__request_t request = { .ipc_client = client, diff --git a/daemons/execd/execd_messages.c b/daemons/execd/execd_messages.c index a397b3501b6..43e3dc61e0a 100644 --- a/daemons/execd/execd_messages.c +++ b/daemons/execd/execd_messages.c @@ -395,7 +395,7 @@ static xmlNode * handle_unknown_request(pcmk__request_t *request) { pcmk__ipc_send_ack(request->ipc_client, request->ipc_id, request->ipc_flags, - PCMK__XE_ACK, NULL, CRM_EX_PROTOCOL); + NULL, CRM_EX_PROTOCOL); pcmk__format_result(&request->result, CRM_EX_PROTOCOL, PCMK_EXEC_INVALID, "Unknown request type '%s' (bug?)", diff --git a/daemons/fenced/fenced_ipc.c b/daemons/fenced/fenced_ipc.c index ffb7b418a6c..6f49e8863ac 100644 --- a/daemons/fenced/fenced_ipc.c +++ b/daemons/fenced/fenced_ipc.c @@ -137,7 +137,7 @@ fenced_ipc_dispatch(qb_ipcs_connection_t *c, void *data, size_t size) if (msg == NULL) { pcmk__debug("Unrecognizable IPC data from PID %d", pcmk__client_pid(c)); - pcmk__ipc_send_ack(client, id, flags, PCMK__XE_ACK, NULL, CRM_EX_PROTOCOL); + pcmk__ipc_send_ack(client, id, flags, NULL, CRM_EX_PROTOCOL); return 0; } diff --git a/daemons/pacemakerd/pcmkd_ipc.c b/daemons/pacemakerd/pcmkd_ipc.c index 60752f1b712..eef86112760 100644 --- a/daemons/pacemakerd/pcmkd_ipc.c +++ b/daemons/pacemakerd/pcmkd_ipc.c @@ -149,8 +149,7 @@ pacemakerd_ipc_dispatch(qb_ipcs_connection_t *c, void *data, size_t size) if (msg == NULL) { pcmk__debug("Unrecognizable IPC data from PID %d", pcmk__client_pid(c)); - pcmk__ipc_send_ack(client, id, flags, PCMK__XE_ACK, NULL, - CRM_EX_PROTOCOL); + pcmk__ipc_send_ack(client, id, flags, NULL, CRM_EX_PROTOCOL); return 0; } else { diff --git a/daemons/pacemakerd/pcmkd_messages.c b/daemons/pacemakerd/pcmkd_messages.c index 07fc6327d6c..3372fec2df0 100644 --- a/daemons/pacemakerd/pcmkd_messages.c +++ b/daemons/pacemakerd/pcmkd_messages.c @@ -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. * @@ -34,7 +34,7 @@ handle_node_cache_request(pcmk__request_t *request) pcmk__client_name(request->ipc_client)); pcmk__ipc_send_ack(request->ipc_client, request->ipc_id, request->ipc_flags, - PCMK__XE_ACK, NULL, CRM_EX_OK); + NULL, CRM_EX_OK); return NULL; } @@ -55,7 +55,7 @@ handle_ping_request(pcmk__request_t *request) pcmk__s(pcmk__xe_get(msg, PCMK_XA_ORIGIN), "")); pcmk__ipc_send_ack(request->ipc_client, request->ipc_id, request->ipc_flags, - PCMK__XE_ACK, NULL, CRM_EX_INDETERMINATE); + NULL, CRM_EX_INDETERMINATE); ping = pcmk__xe_create(NULL, PCMK__XE_PING_RESPONSE); value = pcmk__xe_get(msg, PCMK__XA_CRM_SYS_TO); @@ -112,7 +112,7 @@ handle_shutdown_request(pcmk__request_t *request) pcmk__client_privileged); pcmk__ipc_send_ack(request->ipc_client, request->ipc_id, request->ipc_flags, - PCMK__XE_ACK, NULL, CRM_EX_INDETERMINATE); + NULL, CRM_EX_INDETERMINATE); shutdown = pcmk__xe_create(NULL, PCMK__XE_SHUTDOWN); @@ -150,7 +150,7 @@ static xmlNode * handle_unknown_request(pcmk__request_t *request) { pcmk__ipc_send_ack(request->ipc_client, request->ipc_id, request->ipc_flags, - PCMK__XE_ACK, NULL, CRM_EX_PROTOCOL); + NULL, CRM_EX_PROTOCOL); pcmk__format_result(&request->result, CRM_EX_PROTOCOL, PCMK_EXEC_INVALID, "Unknown request type '%s' (bug?)", diff --git a/daemons/schedulerd/schedulerd_ipc.c b/daemons/schedulerd/schedulerd_ipc.c index 95be262bedd..ec827c6598c 100644 --- a/daemons/schedulerd/schedulerd_ipc.c +++ b/daemons/schedulerd/schedulerd_ipc.c @@ -104,8 +104,7 @@ schedulerd_ipc_dispatch(qb_ipcs_connection_t *c, void *data, size_t size) if (msg == NULL) { pcmk__debug("Unrecognizable IPC data from PID %d", pcmk__client_pid(c)); - pcmk__ipc_send_ack(client, id, flags, PCMK__XE_ACK, NULL, - CRM_EX_PROTOCOL); + pcmk__ipc_send_ack(client, id, flags, NULL, CRM_EX_PROTOCOL); return 0; } @@ -113,13 +112,11 @@ schedulerd_ipc_dispatch(qb_ipcs_connection_t *c, void *data, size_t size) if (pcmk__str_eq(pcmk__xe_get(msg, PCMK__XA_SUBT), PCMK__VALUE_RESPONSE, pcmk__str_none)) { - pcmk__ipc_send_ack(client, id, flags, PCMK__XE_ACK, NULL, - CRM_EX_INDETERMINATE); + pcmk__ipc_send_ack(client, id, flags, NULL, CRM_EX_INDETERMINATE); pcmk__info("Ignoring IPC reply from %s", pcmk__client_name(client)); } else if (!pcmk__str_eq(sys_to, CRM_SYSTEM_PENGINE, pcmk__str_none)) { - pcmk__ipc_send_ack(client, id, flags, PCMK__XE_ACK, NULL, - CRM_EX_INDETERMINATE); + pcmk__ipc_send_ack(client, id, flags, NULL, CRM_EX_INDETERMINATE); pcmk__info("Ignoring invalid IPC message: to '%s' not " CRM_SYSTEM_PENGINE, pcmk__s(sys_to, "")); diff --git a/daemons/schedulerd/schedulerd_messages.c b/daemons/schedulerd/schedulerd_messages.c index b8c969cf40b..d54bd1de15e 100644 --- a/daemons/schedulerd/schedulerd_messages.c +++ b/daemons/schedulerd/schedulerd_messages.c @@ -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. * @@ -79,7 +79,7 @@ handle_pecalc_request(pcmk__request_t *request) pcmk_scheduler_t *scheduler = init_scheduler(); pcmk__ipc_send_ack(request->ipc_client, request->ipc_id, request->ipc_flags, - PCMK__XE_ACK, NULL, CRM_EX_INDETERMINATE); + NULL, CRM_EX_INDETERMINATE); digest = pcmk__digest_xml(xml_data, false); converted = pcmk__xml_copy(NULL, xml_data); @@ -187,7 +187,7 @@ static xmlNode * handle_unknown_request(pcmk__request_t *request) { pcmk__ipc_send_ack(request->ipc_client, request->ipc_id, request->ipc_flags, - PCMK__XE_ACK, NULL, CRM_EX_PROTOCOL); + NULL, CRM_EX_PROTOCOL); pcmk__format_result(&request->result, CRM_EX_PROTOCOL, PCMK_EXEC_INVALID, "Unknown request type '%s' (bug?)", @@ -199,7 +199,7 @@ static xmlNode * handle_hello_request(pcmk__request_t *request) { pcmk__ipc_send_ack(request->ipc_client, request->ipc_id, request->ipc_flags, - PCMK__XE_ACK, NULL, CRM_EX_INDETERMINATE); + NULL, CRM_EX_INDETERMINATE); pcmk__trace("Received IPC hello from %s", pcmk__client_name(request->ipc_client)); diff --git a/include/crm/common/ipc_internal.h b/include/crm/common/ipc_internal.h index 288454fe09f..76a7b7f40b4 100644 --- a/include/crm/common/ipc_internal.h +++ b/include/crm/common/ipc_internal.h @@ -223,7 +223,7 @@ xmlNode *pcmk__ipc_create_ack_as(const char *function, int line, uint32_t flags, int pcmk__ipc_send_ack_as(const char *function, int line, pcmk__client_t *c, uint32_t request, uint32_t flags, const char *ver, crm_exit_t status); -#define pcmk__ipc_send_ack(c, req, flags, tag, ver, st) \ +#define pcmk__ipc_send_ack(c, req, flags, ver, st) \ pcmk__ipc_send_ack_as(__func__, __LINE__, (c), (req), (flags), (ver), (st)) int pcmk__ipc_prepare_iov(uint32_t request, const GString *message,