Skip to content

Remove NACKs from IPC code #4041

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
clumens wants to merge 20 commits into
base: main
Choose a base branch
from
Open

Remove NACKs from IPC code #4041

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
171d24c
Low: daemons: Don't return a NACK if execd gets an unknown message.
clumens Jan 26, 2026
2b4d3bb
Low: liblrmd: Explicitly handle receiving a NACK from execd.
clumens Jan 26, 2026
a479d9a
Refactor: daemons: Don't return a NACK on invalid execd messages.
clumens Jan 26, 2026
92107db
Refactor: libcib: Unindent the end of cib_native_signon.
clumens Jan 26, 2026
ca4b9c0
Low: libcib: Explicitly handle receiving a NACK from based.
clumens Jan 26, 2026
b2c2164
Refactor: libcib: Minor refactorings in cib_tls_signon.
clumens Jan 26, 2026
8e682e5
Low: libcib: Explicitly handle receiving a NACK from remote based.
clumens Jan 26, 2026
1649adc
Refactor: daemons: Don't return a NACK on invalid based messages.
clumens Jan 26, 2026
0c8d5e9
Refactor: libstonith: Unindent one level of stonith_api_signon.
clumens Jan 26, 2026
663aa7f
Refactor: libstonith: Additional unindenting in stonith_api_signon.
clumens Jan 26, 2026
580bab0
Low: libcib: Explicitly handle receiving a NACK from fenced.
clumens Jan 26, 2026
b29d7e3
Refactor: daemons: Don't return a NACK on invalid fenced messages.
clumens Jan 26, 2026
c887f23
Low: daemons: Don't NACK an invalid remote message.
clumens Jan 27, 2026
8b38617
Low: daemons: Delay creating an xmlNode in handle_register_request.
clumens Jan 27, 2026
714d247
Refactor: liblrmd: Fix log messages in remote_proxy_cb.
clumens Jan 27, 2026
13ae208
Refactor: liblrmd: Minor refactorings in remote_proxy_cb.
clumens Jan 28, 2026
cbadda5
Refactor: libcrmcommon: Remove the tag argument from pcmk__ipc_create…
clumens Jan 28, 2026
51e95cd
Refactor: libcrmcommon: Remove the tag argument from pcmk__ipc_create…
clumens Jan 28, 2026
18111fb
Refactor: libcrmcommon: Remove the tag argument from pcmk__ipc_send_a…
clumens Jan 28, 2026
314238d
Refactor: libcrmcommon: Remove the tag argument from pcmk__ipc_send_ack.
clumens Jan 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions daemons/attrd/attrd_ipc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 2 additions & 3 deletions daemons/attrd/attrd_sync.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
*
Expand Down Expand Up @@ -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",
Expand Down
8 changes: 4 additions & 4 deletions daemons/attrd/pacemaker-attrd.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
*
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 2 additions & 3 deletions daemons/based/based_ipc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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_NACK, NULL,
CRM_EX_PROTOCOL);
pcmk__ipc_send_ack(client, id, flags, NULL, CRM_EX_PROTOCOL);
return 0;
}

Expand Down Expand Up @@ -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;
}

Expand Down
8 changes: 3 additions & 5 deletions daemons/controld/controld_control.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
*
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion daemons/execd/execd_ipc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, NULL, CRM_EX_PROTOCOL);
} else {
pcmk__request_t request = {
.ipc_client = client,
Expand Down
2 changes: 1 addition & 1 deletion daemons/execd/execd_messages.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
NULL, CRM_EX_PROTOCOL);

pcmk__format_result(&request->result, CRM_EX_PROTOCOL, PCMK_EXEC_INVALID,
"Unknown request type '%s' (bug?)",
Expand Down
9 changes: 1 addition & 8 deletions daemons/execd/remoted_tls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions daemons/fenced/fenced_commands.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
*
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -3417,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
Expand Down
2 changes: 1 addition & 1 deletion daemons/fenced/fenced_ipc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, NULL, CRM_EX_PROTOCOL);
return 0;
}

Expand Down
3 changes: 1 addition & 2 deletions daemons/pacemakerd/pcmkd_ipc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 5 additions & 5 deletions daemons/pacemakerd/pcmkd_messages.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
*
Expand Down Expand Up @@ -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;
}

Expand All @@ -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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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?)",
Expand Down
9 changes: 3 additions & 6 deletions daemons/schedulerd/schedulerd_ipc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,22 +104,19 @@ 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;
}

sys_to = pcmk__xe_get(msg, PCMK__XA_CRM_SYS_TO);

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, ""));

Expand Down
8 changes: 4 additions & 4 deletions daemons/schedulerd/schedulerd_messages.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
*
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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?)",
Expand All @@ -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));
Expand Down
14 changes: 7 additions & 7 deletions include/crm/common/ipc_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,15 +216,15 @@ 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);
#define pcmk__ipc_create_ack(flags, tag, ver, st) \
pcmk__ipc_create_ack_as(__func__, __LINE__, (flags), (tag), (ver), (st))
const char *ver, crm_exit_t status);
#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,
uint32_t request, uint32_t flags, const char *tag,
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))
uint32_t request, uint32_t flags, const char *ver,
crm_exit_t status);
#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,
uint16_t index, struct iovec **result, ssize_t *bytes);
Expand Down
61 changes: 43 additions & 18 deletions lib/cib/cib_native.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
*
Expand Down Expand Up @@ -315,33 +315,58 @@ 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");
pcmk__log_xml_trace(reply, "reg-reply");

/* If we got an ACK in response, it came from dispatch_common and means
* one of two things:
*
* (1) We originally sent a PCMK__VALUE_CIB_NOTIFY and based is ACKing
Copy link
Copy Markdown
Contributor

@nrwahl2 nrwahl2 Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a PCMK__VALUE_CIB_NOTIFY response falls through, wouldn't that result in a "Reply to CIB registration message has unknown type" message?

Copy link
Copy Markdown
Contributor Author

@clumens clumens Apr 1, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this some more, I don't think the lib/cib/cib_native.c portions of this PR are actually necessary, or perhaps this code is in the wrong spot. In cib_native_signon, we are only sending a CIB_OP_REGISTER message and there's only one possible response to that. I don't think there's any point in handling an ACK here.

cib_native_perform_op_delegate looks like the more correct spot for this. Let me know if this makes sense to you too.

* that it's received and processed it. These ACKs will have a
* status of CRM_EX_OK. We'll let them fall through to be processed
* by the rest of the message handling below.
* (2) We originally sent an invalid message. At the moment, these ACKs
* can only ever have a status of CRM_EX_PROTOCOL. Handle them
* here.
*/
if (pcmk__xe_is(reply, PCMK__XE_ACK)) {
int status = 0;

rc = pcmk__xe_get_int(reply, PCMK_XA_STATUS, &status);

if ((rc == pcmk_rc_ok) && (status != CRM_EX_OK)) {
pcmk__err("Received error response from CIB manager: %s",
crm_exit_str(status));
return -EPROTO;
}
}

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;
msg_type = pcmk__xe_get(reply, PCMK__XA_CIB_OP);

} 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;
Expand Down
Loading