From 3ee3cf30a573938a601f3fa4621c797584664d1b Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Fri, 11 Jul 2025 09:48:17 -0400 Subject: [PATCH 01/20] Refactor: daemons: Move IPC server skeleton code to execd_messages.c. This is the beginnings of using the pcmk__request_t interface in the server side of pacemaker-execd. For now, there's no substantial code changes - just moving the code into a new file. --- daemons/execd/Makefile.am | 5 +- daemons/execd/execd_messages.c | 164 ++++++++++++++++++++++++++++++++ daemons/execd/pacemaker-execd.c | 135 -------------------------- daemons/execd/pacemaker-execd.h | 4 +- 4 files changed, 170 insertions(+), 138 deletions(-) create mode 100644 daemons/execd/execd_messages.c diff --git a/daemons/execd/Makefile.am b/daemons/execd/Makefile.am index e177dbb1a8b..bcf96edec07 100644 --- a/daemons/execd/Makefile.am +++ b/daemons/execd/Makefile.am @@ -1,5 +1,5 @@ # -# Copyright 2012-2024 the Pacemaker project contributors +# Copyright 2012-2025 the Pacemaker project contributors # # The version control history for this file may have further details. # @@ -25,7 +25,8 @@ pacemaker_execd_LDADD += $(top_builddir)/lib/services/libcrmservice.la pacemaker_execd_LDADD += $(top_builddir)/lib/common/libcrmcommon.la pacemaker_execd_SOURCES = pacemaker-execd.c \ execd_commands.c \ - execd_alerts.c + execd_alerts.c \ + execd_messages.c sbin_PROGRAMS = pacemaker-remoted if BUILD_SYSTEMD diff --git a/daemons/execd/execd_messages.c b/daemons/execd/execd_messages.c new file mode 100644 index 00000000000..8f84664058c --- /dev/null +++ b/daemons/execd/execd_messages.c @@ -0,0 +1,164 @@ +/* + * Copyright 2012-2025 the Pacemaker project contributors + * + * The version control history for this file may have further details. + * + * This source code is licensed under the GNU Lesser General Public License + * version 2.1 or later (LGPLv2.1+) WITHOUT ANY WARRANTY. + */ + +#include + +#include // ENOMEM +#include // NULL, size_t +#include // int32_t, uint32_t +#include // gid_t, uid_t + +#include // g_byte_array_free, FALSE +#include // xmlNode +#include // qb_ipcs_connection_t, qb_ipcs_service_handlers + +#include // pcmk__xml_free +#include // crm_ipc_flags +#include // pcmk__client_s, pcmk__find_client +#include // pcmk_rc_e, pcmk_rc_str +#include // crm_xml_add, crm_element_value +#include // PCMK__XA_LRMD_* + +#include "pacemaker-execd.h" // client_disconnect_cleanup + +extern int lrmd_call_id; + +static int32_t +lrmd_ipc_accept(qb_ipcs_connection_t * c, uid_t uid, gid_t gid) +{ + crm_trace("Connection %p", c); + if (pcmk__new_client(c, uid, gid) == NULL) { + return -ENOMEM; + } + return 0; +} + +static void +lrmd_ipc_created(qb_ipcs_connection_t * c) +{ + pcmk__client_t *new_client = pcmk__find_client(c); + + crm_trace("Connection %p", c); + pcmk__assert(new_client != NULL); + /* Now that the connection is offically established, alert + * the other clients a new connection exists. */ + + notify_of_new_client(new_client); +} + +static int32_t +lrmd_ipc_dispatch(qb_ipcs_connection_t * c, void *data, size_t size) +{ + int rc = pcmk_rc_ok; + uint32_t id = 0; + uint32_t flags = 0; + pcmk__client_t *client = pcmk__find_client(c); + xmlNode *request = NULL; + + CRM_CHECK(client != NULL, crm_err("Invalid client"); + return FALSE); + CRM_CHECK(client->id != NULL, crm_err("Invalid client: %p", client); + return FALSE); + + rc = pcmk__ipc_msg_append(&client->buffer, data); + + if (rc == pcmk_rc_ipc_more) { + /* We haven't read the complete message yet, so just return. */ + return 0; + + } else if (rc == pcmk_rc_ok) { + /* We've read the complete message and there's already a header on + * the front. Pass it off for processing. + */ + request = pcmk__client_data2xml(client, &id, &flags); + g_byte_array_free(client->buffer, TRUE); + client->buffer = NULL; + + } else { + /* Some sort of error occurred reassembling the message. All we can + * do is clean up, log an error and return. + */ + crm_err("Error when reading IPC message: %s", pcmk_rc_str(rc)); + + if (client->buffer != NULL) { + g_byte_array_free(client->buffer, TRUE); + client->buffer = NULL; + } + + return 0; + } + + CRM_CHECK(flags & crm_ipc_client_response, crm_err("Invalid client request: %p", client); + return FALSE); + + if (!request) { + return 0; + } + + /* @TODO functionize some of this to reduce duplication with + * lrmd_remote_client_msg() + */ + + if (!client->name) { + const char *value = crm_element_value(request, + PCMK__XA_LRMD_CLIENTNAME); + + if (value == NULL) { + client->name = pcmk__itoa(pcmk__client_pid(c)); + } else { + client->name = pcmk__str_copy(value); + } + } + + lrmd_call_id++; + if (lrmd_call_id < 1) { + lrmd_call_id = 1; + } + + crm_xml_add(request, PCMK__XA_LRMD_CLIENTID, client->id); + crm_xml_add(request, PCMK__XA_LRMD_CLIENTNAME, client->name); + crm_xml_add_int(request, PCMK__XA_LRMD_CALLID, lrmd_call_id); + + process_lrmd_message(client, id, request); + pcmk__xml_free(request); + return 0; +} + +static int32_t +lrmd_ipc_closed(qb_ipcs_connection_t * c) +{ + pcmk__client_t *client = pcmk__find_client(c); + + if (client == NULL) { + return 0; + } + + crm_trace("Connection %p", c); + client_disconnect_cleanup(client->id); +#ifdef PCMK__COMPILE_REMOTE + ipc_proxy_remove_provider(client); +#endif + lrmd_client_destroy(client); + return 0; +} + +static void +lrmd_ipc_destroy(qb_ipcs_connection_t * c) +{ + lrmd_ipc_closed(c); + crm_trace("Connection %p", c); +} + +struct qb_ipcs_service_handlers lrmd_ipc_callbacks = { + .connection_accept = lrmd_ipc_accept, + .connection_created = lrmd_ipc_created, + .msg_process = lrmd_ipc_dispatch, + .connection_closed = lrmd_ipc_closed, + .connection_destroyed = lrmd_ipc_destroy +}; diff --git a/daemons/execd/pacemaker-execd.c b/daemons/execd/pacemaker-execd.c index 3229487e4e8..be6f871f64e 100644 --- a/daemons/execd/pacemaker-execd.c +++ b/daemons/execd/pacemaker-execd.c @@ -97,108 +97,6 @@ get_stonith_connection(void) return stonith_api; } -static int32_t -lrmd_ipc_accept(qb_ipcs_connection_t * c, uid_t uid, gid_t gid) -{ - crm_trace("Connection %p", c); - if (pcmk__new_client(c, uid, gid) == NULL) { - return -ENOMEM; - } - return 0; -} - -static void -lrmd_ipc_created(qb_ipcs_connection_t * c) -{ - pcmk__client_t *new_client = pcmk__find_client(c); - - crm_trace("Connection %p", c); - pcmk__assert(new_client != NULL); - /* Now that the connection is offically established, alert - * the other clients a new connection exists. */ - - notify_of_new_client(new_client); -} - -static int32_t -lrmd_ipc_dispatch(qb_ipcs_connection_t * c, void *data, size_t size) -{ - int rc = pcmk_rc_ok; - uint32_t id = 0; - uint32_t flags = 0; - pcmk__client_t *client = pcmk__find_client(c); - xmlNode *request = NULL; - - CRM_CHECK(client != NULL, crm_err("Invalid client"); - return FALSE); - CRM_CHECK(client->id != NULL, crm_err("Invalid client: %p", client); - return FALSE); - - rc = pcmk__ipc_msg_append(&client->buffer, data); - - if (rc == pcmk_rc_ipc_more) { - /* We haven't read the complete message yet, so just return. */ - return 0; - - } else if (rc == pcmk_rc_ok) { - /* We've read the complete message and there's already a header on - * the front. Pass it off for processing. - */ - request = pcmk__client_data2xml(client, &id, &flags); - g_byte_array_free(client->buffer, TRUE); - client->buffer = NULL; - - } else { - /* Some sort of error occurred reassembling the message. All we can - * do is clean up, log an error and return. - */ - crm_err("Error when reading IPC message: %s", pcmk_rc_str(rc)); - - if (client->buffer != NULL) { - g_byte_array_free(client->buffer, TRUE); - client->buffer = NULL; - } - - return 0; - } - - CRM_CHECK(flags & crm_ipc_client_response, crm_err("Invalid client request: %p", client); - return FALSE); - - if (!request) { - return 0; - } - - /* @TODO functionize some of this to reduce duplication with - * lrmd_remote_client_msg() - */ - - if (!client->name) { - const char *value = crm_element_value(request, - PCMK__XA_LRMD_CLIENTNAME); - - if (value == NULL) { - client->name = pcmk__itoa(pcmk__client_pid(c)); - } else { - client->name = pcmk__str_copy(value); - } - } - - lrmd_call_id++; - if (lrmd_call_id < 1) { - lrmd_call_id = 1; - } - - crm_xml_add(request, PCMK__XA_LRMD_CLIENTID, client->id); - crm_xml_add(request, PCMK__XA_LRMD_CLIENTNAME, client->name); - crm_xml_add_int(request, PCMK__XA_LRMD_CALLID, lrmd_call_id); - - process_lrmd_message(client, id, request); - - pcmk__xml_free(request); - return 0; -} - /*! * \internal * \brief Free a client connection, and exit if appropriate @@ -220,39 +118,6 @@ lrmd_client_destroy(pcmk__client_t *client) #endif } -static int32_t -lrmd_ipc_closed(qb_ipcs_connection_t * c) -{ - pcmk__client_t *client = pcmk__find_client(c); - - if (client == NULL) { - return 0; - } - - crm_trace("Connection %p", c); - client_disconnect_cleanup(client->id); -#ifdef PCMK__COMPILE_REMOTE - ipc_proxy_remove_provider(client); -#endif - lrmd_client_destroy(client); - return 0; -} - -static void -lrmd_ipc_destroy(qb_ipcs_connection_t * c) -{ - lrmd_ipc_closed(c); - crm_trace("Connection %p", c); -} - -static struct qb_ipcs_service_handlers lrmd_ipc_callbacks = { - .connection_accept = lrmd_ipc_accept, - .connection_created = lrmd_ipc_created, - .msg_process = lrmd_ipc_dispatch, - .connection_closed = lrmd_ipc_closed, - .connection_destroyed = lrmd_ipc_destroy -}; - // \return Standard Pacemaker return code int lrmd_server_send_reply(pcmk__client_t *client, uint32_t id, xmlNode *reply) diff --git a/daemons/execd/pacemaker-execd.h b/daemons/execd/pacemaker-execd.h index a66f5ef8889..12325a850e3 100644 --- a/daemons/execd/pacemaker-execd.h +++ b/daemons/execd/pacemaker-execd.h @@ -1,5 +1,5 @@ /* - * Copyright 2012-2024 the Pacemaker project contributors + * Copyright 2012-2025 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -20,6 +20,8 @@ extern GHashTable *rsc_list; extern time_t start_time; +extern struct qb_ipcs_service_handlers lrmd_ipc_callbacks; + typedef struct lrmd_rsc_s { char *rsc_id; char *class; From 836ae0df5a2d606e6047e6ec6366e601f147fe7a Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Tue, 15 Jul 2025 16:20:39 -0400 Subject: [PATCH 02/20] Refactor: daemons: Rename variables in execd_messages.c. This brings them more in line with usage in pe_ipc_dispatch (I'm hoping to condense all this code one day), and frees up the name "request" to be used by pcmk__request_t in the future. --- daemons/execd/execd_messages.c | 48 +++++++++++++++++----------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/daemons/execd/execd_messages.c b/daemons/execd/execd_messages.c index 8f84664058c..9fe453c663a 100644 --- a/daemons/execd/execd_messages.c +++ b/daemons/execd/execd_messages.c @@ -30,21 +30,21 @@ extern int lrmd_call_id; static int32_t -lrmd_ipc_accept(qb_ipcs_connection_t * c, uid_t uid, gid_t gid) +lrmd_ipc_accept(qb_ipcs_connection_t *qbc, uid_t uid, gid_t gid) { - crm_trace("Connection %p", c); - if (pcmk__new_client(c, uid, gid) == NULL) { + crm_trace("Connection %p", qbc); + if (pcmk__new_client(qbc, uid, gid) == NULL) { return -ENOMEM; } return 0; } static void -lrmd_ipc_created(qb_ipcs_connection_t * c) +lrmd_ipc_created(qb_ipcs_connection_t *qbc) { - pcmk__client_t *new_client = pcmk__find_client(c); + pcmk__client_t *new_client = pcmk__find_client(qbc); - crm_trace("Connection %p", c); + crm_trace("Connection %p", qbc); pcmk__assert(new_client != NULL); /* Now that the connection is offically established, alert * the other clients a new connection exists. */ @@ -53,13 +53,13 @@ lrmd_ipc_created(qb_ipcs_connection_t * c) } static int32_t -lrmd_ipc_dispatch(qb_ipcs_connection_t * c, void *data, size_t size) +lrmd_ipc_dispatch(qb_ipcs_connection_t *qbc, void *data, size_t size) { int rc = pcmk_rc_ok; uint32_t id = 0; uint32_t flags = 0; - pcmk__client_t *client = pcmk__find_client(c); - xmlNode *request = NULL; + pcmk__client_t *client = pcmk__find_client(qbc); + xmlNode *msg = NULL; CRM_CHECK(client != NULL, crm_err("Invalid client"); return FALSE); @@ -76,7 +76,7 @@ lrmd_ipc_dispatch(qb_ipcs_connection_t * c, void *data, size_t size) /* We've read the complete message and there's already a header on * the front. Pass it off for processing. */ - request = pcmk__client_data2xml(client, &id, &flags); + msg = pcmk__client_data2xml(client, &id, &flags); g_byte_array_free(client->buffer, TRUE); client->buffer = NULL; @@ -97,7 +97,7 @@ lrmd_ipc_dispatch(qb_ipcs_connection_t * c, void *data, size_t size) CRM_CHECK(flags & crm_ipc_client_response, crm_err("Invalid client request: %p", client); return FALSE); - if (!request) { + if (!msg) { return 0; } @@ -106,11 +106,11 @@ lrmd_ipc_dispatch(qb_ipcs_connection_t * c, void *data, size_t size) */ if (!client->name) { - const char *value = crm_element_value(request, + const char *value = crm_element_value(msg, PCMK__XA_LRMD_CLIENTNAME); if (value == NULL) { - client->name = pcmk__itoa(pcmk__client_pid(c)); + client->name = pcmk__itoa(pcmk__client_pid(qbc)); } else { client->name = pcmk__str_copy(value); } @@ -121,25 +121,25 @@ lrmd_ipc_dispatch(qb_ipcs_connection_t * c, void *data, size_t size) lrmd_call_id = 1; } - crm_xml_add(request, PCMK__XA_LRMD_CLIENTID, client->id); - crm_xml_add(request, PCMK__XA_LRMD_CLIENTNAME, client->name); - crm_xml_add_int(request, PCMK__XA_LRMD_CALLID, lrmd_call_id); + crm_xml_add(msg, PCMK__XA_LRMD_CLIENTID, client->id); + crm_xml_add(msg, PCMK__XA_LRMD_CLIENTNAME, client->name); + crm_xml_add_int(msg, PCMK__XA_LRMD_CALLID, lrmd_call_id); - process_lrmd_message(client, id, request); - pcmk__xml_free(request); + process_lrmd_message(client, id, msg); + pcmk__xml_free(msg); return 0; } static int32_t -lrmd_ipc_closed(qb_ipcs_connection_t * c) +lrmd_ipc_closed(qb_ipcs_connection_t *qbc) { - pcmk__client_t *client = pcmk__find_client(c); + pcmk__client_t *client = pcmk__find_client(qbc); if (client == NULL) { return 0; } - crm_trace("Connection %p", c); + crm_trace("Connection %p", qbc); client_disconnect_cleanup(client->id); #ifdef PCMK__COMPILE_REMOTE ipc_proxy_remove_provider(client); @@ -149,10 +149,10 @@ lrmd_ipc_closed(qb_ipcs_connection_t * c) } static void -lrmd_ipc_destroy(qb_ipcs_connection_t * c) +lrmd_ipc_destroy(qb_ipcs_connection_t *qbc) { - lrmd_ipc_closed(c); - crm_trace("Connection %p", c); + lrmd_ipc_closed(qbc); + crm_trace("Connection %p", qbc); } struct qb_ipcs_service_handlers lrmd_ipc_callbacks = { From a3bb87a9b701b5ea1b4895a067cf543f918be476 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Fri, 11 Jul 2025 10:38:45 -0400 Subject: [PATCH 03/20] Refactor: daemons: Add execd_process_message. At the moment, this function is fairly unfortunately named (there's already a process_lrmd_message). However, the new function is gradually going to be taking code from the old function until the old one is completely gone. So it's just a temporary confusion we'll need to live with for a bit. This new function holds the framework for handling commands with pcmk__request_t instead of the old system. It's a little tricky to convert these commands over one at a time and still keep everything working, which is why there's the big TODO comment. --- daemons/execd/execd_messages.c | 128 +++++++++++++++++++++++++++++++- daemons/execd/pacemaker-execd.c | 1 + daemons/execd/pacemaker-execd.h | 5 ++ daemons/execd/remoted_tls.c | 2 +- 4 files changed, 132 insertions(+), 4 deletions(-) diff --git a/daemons/execd/execd_messages.c b/daemons/execd/execd_messages.c index 9fe453c663a..8551e33ea10 100644 --- a/daemons/execd/execd_messages.c +++ b/daemons/execd/execd_messages.c @@ -10,25 +10,51 @@ #include #include // ENOMEM +#include // bool #include // NULL, size_t #include // int32_t, uint32_t +#include // free #include // gid_t, uid_t #include // g_byte_array_free, FALSE #include // xmlNode #include // qb_ipcs_connection_t, qb_ipcs_service_handlers +#include // QB_XS -#include // pcmk__xml_free +#include // CRM_SYSTEM_LRMD +#include // pcmk__process_request, pcmk__xml_free #include // crm_ipc_flags #include // pcmk__client_s, pcmk__find_client #include // pcmk_rc_e, pcmk_rc_str +#include // crm_strdup_printf #include // crm_xml_add, crm_element_value -#include // PCMK__XA_LRMD_* +#include // PCMK__XA_LRMD_*, pcmk__xe_is #include "pacemaker-execd.h" // client_disconnect_cleanup extern int lrmd_call_id; +static GHashTable *execd_handlers = NULL; + +static void +execd_register_handlers(void) +{ + pcmk__server_command_t handlers[] = { + { NULL, NULL }, + }; + + execd_handlers = pcmk__register_handlers(handlers); +} + +void +execd_unregister_handlers(void) +{ + if (execd_handlers != NULL) { + g_hash_table_destroy(execd_handlers); + execd_handlers = NULL; + } +} + static int32_t lrmd_ipc_accept(qb_ipcs_connection_t *qbc, uid_t uid, gid_t gid) { @@ -125,7 +151,7 @@ lrmd_ipc_dispatch(qb_ipcs_connection_t *qbc, void *data, size_t size) crm_xml_add(msg, PCMK__XA_LRMD_CLIENTNAME, client->name); crm_xml_add_int(msg, PCMK__XA_LRMD_CALLID, lrmd_call_id); - process_lrmd_message(client, id, msg); + execd_process_message(client, id, flags, msg); pcmk__xml_free(msg); return 0; } @@ -162,3 +188,99 @@ struct qb_ipcs_service_handlers lrmd_ipc_callbacks = { .connection_closed = lrmd_ipc_closed, .connection_destroyed = lrmd_ipc_destroy }; + +static bool +invalid_msg(xmlNode *msg) +{ + const char *to = crm_element_value(msg, PCMK__XA_T); + + /* IPC proxy messages do not get a t="" attribute set on them. */ + bool invalid = !pcmk__str_eq(to, CRM_SYSTEM_LRMD, pcmk__str_none) && + !pcmk__xe_is(msg, PCMK__XE_LRMD_IPC_PROXY); + + if (invalid) { + crm_info("Ignoring invalid IPC message: to '%s' not " CRM_SYSTEM_LRMD, + pcmk__s(to, "")); + crm_log_xml_info(msg, "[Invalid]"); + } + + return invalid; +} + +void +execd_process_message(pcmk__client_t *c, uint32_t id, uint32_t flags, xmlNode *msg) +{ + int rc = pcmk_rc_ok; + + if (execd_handlers == NULL) { + execd_register_handlers(); + } + + if (invalid_msg(msg)) { + pcmk__ipc_send_ack(c, id, flags, PCMK__XE_NACK, NULL, CRM_EX_PROTOCOL); + } else { + char *log_msg = NULL; + const char *reason = NULL; + xmlNode *reply = NULL; + + pcmk__request_t request = { + .ipc_client = c, + .ipc_id = id, + .ipc_flags = flags, + .peer = NULL, + .xml = msg, + .call_options = 0, + .result = PCMK__UNKNOWN_RESULT, + }; + + request.op = crm_element_value_copy(request.xml, PCMK__XA_LRMD_OP); + CRM_CHECK(request.op != NULL, return); + + crm_trace("Processing %s operation from %s", request.op, c->id); + + reply = pcmk__process_request(&request, execd_handlers); + + /* FIXME: THIS IS TEMPORARY + * + * If the above returns NULL (which could be because something bad happened, + * or because a message doesn't send a reply, but is most likely because not + * all messages have been implemented yet), try falling back to the older + * code. This means we don't have to implement everything before testing. + * Memory cleanup isn't important here. This will be removed. + */ + if (reply == NULL) { + crm_trace("Falling back to old function"); + process_lrmd_message(c, id, request.xml); + return; + } + + if (reply != NULL) { + rc = lrmd_server_send_reply(c, id, reply); + if (rc != pcmk_rc_ok) { + crm_warn("Reply to client %s failed: %s " QB_XS " rc=%d", + pcmk__client_name(c), pcmk_rc_str(rc), rc); + } + + pcmk__xml_free(reply); + } + + reason = request.result.exit_reason; + + log_msg = crm_strdup_printf("Processed %s request from %s %s: %s%s%s%s", + request.op, pcmk__request_origin_type(&request), + pcmk__request_origin(&request), + pcmk_exec_status_str(request.result.execution_status), + (reason == NULL)? "" : " (", + (reason == NULL)? "" : reason, + (reason == NULL)? "" : ")"); + + if (!pcmk__result_ok(&request.result)) { + crm_warn("%s", log_msg); + } else { + crm_debug("%s", log_msg); + } + + free(log_msg); + pcmk__reset_request(&request); + } +} diff --git a/daemons/execd/pacemaker-execd.c b/daemons/execd/pacemaker-execd.c index be6f871f64e..4979106a0de 100644 --- a/daemons/execd/pacemaker-execd.c +++ b/daemons/execd/pacemaker-execd.c @@ -189,6 +189,7 @@ exit_executor(void) #endif pcmk__client_cleanup(); + execd_unregister_handlers(); if (mainloop) { lrmd_drain_alerts(mainloop); diff --git a/daemons/execd/pacemaker-execd.h b/daemons/execd/pacemaker-execd.h index 12325a850e3..ca945361d42 100644 --- a/daemons/execd/pacemaker-execd.h +++ b/daemons/execd/pacemaker-execd.h @@ -102,8 +102,13 @@ void remoted_spawn_pidone(int argc, char **argv); void remoted_request_cib_schema_files(void); #endif +void execd_unregister_handlers(void); + int process_lrmd_alert_exec(pcmk__client_t *client, uint32_t id, xmlNode *request); void lrmd_drain_alerts(GMainLoop *mloop); +void execd_process_message(pcmk__client_t *c, uint32_t id, uint32_t flags, + xmlNode *msg); + #endif // PACEMAKER_EXECD__H diff --git a/daemons/execd/remoted_tls.c b/daemons/execd/remoted_tls.c index 13c867f3e63..9807fc0748f 100644 --- a/daemons/execd/remoted_tls.c +++ b/daemons/execd/remoted_tls.c @@ -145,7 +145,7 @@ lrmd_remote_client_msg(gpointer data) crm_xml_add(request, PCMK__XA_LRMD_CLIENTNAME, client->name); crm_xml_add_int(request, PCMK__XA_LRMD_CALLID, lrmd_call_id); - process_lrmd_message(client, id, request); + execd_process_message(client, id, client->flags, request); pcmk__xml_free(request); return 0; From 9f83b81061f94a1d3fd42c7e5ebb886dda7a2f57 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Fri, 11 Jul 2025 14:10:47 -0400 Subject: [PATCH 04/20] Refactor: daemons: Condense duplicated IPC code. Both places that call execd_process_message first add several additional attributes to the request XML, so it makes sense to move that code into execd_process_message instead. Note that there are two different approaches to setting client->name if it's missing. I don't see why the attribute would ever be missing, so I think it's safe to get rid of the lengthier approach that grabs the client PID as a last option. --- daemons/execd/execd_messages.c | 40 ++++++++++++--------------------- daemons/execd/pacemaker-execd.c | 1 - daemons/execd/remoted_tls.c | 14 ------------ 3 files changed, 14 insertions(+), 41 deletions(-) diff --git a/daemons/execd/execd_messages.c b/daemons/execd/execd_messages.c index 8551e33ea10..3e0b7160094 100644 --- a/daemons/execd/execd_messages.c +++ b/daemons/execd/execd_messages.c @@ -32,9 +32,8 @@ #include "pacemaker-execd.h" // client_disconnect_cleanup -extern int lrmd_call_id; - static GHashTable *execd_handlers = NULL; +static int lrmd_call_id = 0; static void execd_register_handlers(void) @@ -127,30 +126,6 @@ lrmd_ipc_dispatch(qb_ipcs_connection_t *qbc, void *data, size_t size) return 0; } - /* @TODO functionize some of this to reduce duplication with - * lrmd_remote_client_msg() - */ - - if (!client->name) { - const char *value = crm_element_value(msg, - PCMK__XA_LRMD_CLIENTNAME); - - if (value == NULL) { - client->name = pcmk__itoa(pcmk__client_pid(qbc)); - } else { - client->name = pcmk__str_copy(value); - } - } - - lrmd_call_id++; - if (lrmd_call_id < 1) { - lrmd_call_id = 1; - } - - crm_xml_add(msg, PCMK__XA_LRMD_CLIENTID, client->id); - crm_xml_add(msg, PCMK__XA_LRMD_CLIENTNAME, client->name); - crm_xml_add_int(msg, PCMK__XA_LRMD_CALLID, lrmd_call_id); - execd_process_message(client, id, flags, msg); pcmk__xml_free(msg); return 0; @@ -216,6 +191,19 @@ execd_process_message(pcmk__client_t *c, uint32_t id, uint32_t flags, xmlNode *m execd_register_handlers(); } + if (!c->name) { + c->name = crm_element_value_copy(msg, PCMK__XA_LRMD_CLIENTNAME); + } + + lrmd_call_id++; + if (lrmd_call_id < 1) { + lrmd_call_id = 1; + } + + crm_xml_add(msg, PCMK__XA_LRMD_CLIENTID, c->id); + crm_xml_add(msg, PCMK__XA_LRMD_CLIENTNAME, c->name); + crm_xml_add_int(msg, PCMK__XA_LRMD_CALLID, lrmd_call_id); + if (invalid_msg(msg)) { pcmk__ipc_send_ack(c, id, flags, PCMK__XE_NACK, NULL, CRM_EX_PROTOCOL); } else { diff --git a/daemons/execd/pacemaker-execd.c b/daemons/execd/pacemaker-execd.c index 4979106a0de..d5868bf66b7 100644 --- a/daemons/execd/pacemaker-execd.c +++ b/daemons/execd/pacemaker-execd.c @@ -40,7 +40,6 @@ static GMainLoop *mainloop = NULL; static qb_ipcs_service_t *ipcs = NULL; static stonith_t *stonith_api = NULL; -int lrmd_call_id = 0; time_t start_time; static struct { diff --git a/daemons/execd/remoted_tls.c b/daemons/execd/remoted_tls.c index 9807fc0748f..6d28f94f505 100644 --- a/daemons/execd/remoted_tls.c +++ b/daemons/execd/remoted_tls.c @@ -33,7 +33,6 @@ static pcmk__tls_t *tls = NULL; static int ssock = -1; -extern int lrmd_call_id; /*! * \internal @@ -131,19 +130,6 @@ lrmd_remote_client_msg(gpointer data) crm_element_value_int(request, PCMK__XA_LRMD_REMOTE_MSG_ID, &id); crm_trace("Processing remote client request %d", id); - if (!client->name) { - client->name = crm_element_value_copy(request, - PCMK__XA_LRMD_CLIENTNAME); - } - - lrmd_call_id++; - if (lrmd_call_id < 1) { - lrmd_call_id = 1; - } - - crm_xml_add(request, PCMK__XA_LRMD_CLIENTID, client->id); - crm_xml_add(request, PCMK__XA_LRMD_CLIENTNAME, client->name); - crm_xml_add_int(request, PCMK__XA_LRMD_CALLID, lrmd_call_id); execd_process_message(client, id, client->flags, request); pcmk__xml_free(request); From 78e899752bc35e6c00926e0bdedb8c59d11016fc Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 9 Jul 2025 10:40:58 -0400 Subject: [PATCH 05/20] Refactor: daemons: Use pcmk__request_t for CRM_OP_REGISTER. This commit is typical of how the commit for each command is going to work: * Expose a function in execd_commands.c to the rest of the exec daemon and give it a name that is consistent with "public" functions. * Have it return a standard pacemaker return code. * Remove the old handler block from process_lrmd_message (this is the older function, not the newer confusingly named function). * Add a new handler function and add that function to the handler table. --- daemons/execd/execd_commands.c | 21 ++++++++++----------- daemons/execd/execd_messages.c | 21 +++++++++++++++++++++ daemons/execd/pacemaker-execd.h | 3 +++ 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c index a59dee23560..cc8dfda8b61 100644 --- a/daemons/execd/execd_commands.c +++ b/daemons/execd/execd_commands.c @@ -1512,11 +1512,11 @@ free_rsc(gpointer data) free(rsc); } -static int -process_lrmd_signon(pcmk__client_t *client, xmlNode *request, int call_id, - xmlNode **reply) +int +execd_process_signon(pcmk__client_t *client, xmlNode *request, int call_id, + xmlNode **reply) { - int rc = pcmk_ok; + int rc = pcmk_rc_ok; time_t now = time(NULL); const char *protocol_version = crm_element_value(request, PCMK__XA_LRMD_PROTOCOL_VERSION); @@ -1525,7 +1525,7 @@ process_lrmd_signon(pcmk__client_t *client, xmlNode *request, int call_id, if (compare_version(protocol_version, LRMD_COMPATIBLE_PROTOCOL) < 0) { crm_err("Cluster API version must be greater than or equal to %s, not %s", LRMD_COMPATIBLE_PROTOCOL, protocol_version); - rc = -EPROTO; + rc = EPROTO; } if (pcmk__xe_attr_is_true(request, PCMK__XA_LRMD_IS_IPC_PROVIDER)) { @@ -1555,14 +1555,16 @@ process_lrmd_signon(pcmk__client_t *client, xmlNode *request, int call_id, remoted_request_cib_schema_files(); } } else { - rc = -EACCES; + rc = EACCES; } #else - rc = -EPROTONOSUPPORT; + rc = EPROTONOSUPPORT; #endif } - *reply = create_lrmd_reply(__func__, rc, call_id); + pcmk__assert(reply != NULL); + + *reply = create_lrmd_reply(__func__, pcmk_rc2legacy(rc), call_id); crm_xml_add(*reply, PCMK__XA_LRMD_OP, CRM_OP_REGISTER); crm_xml_add(*reply, PCMK__XA_LRMD_CLIENTID, client->id); crm_xml_add(*reply, PCMK__XA_LRMD_PROTOCOL_VERSION, LRMD_PROTOCOL_VERSION); @@ -1900,9 +1902,6 @@ process_lrmd_message(pcmk__client_t *client, uint32_t id, xmlNode *request) rc = -EPROTONOSUPPORT; #endif do_reply = 1; - } else if (pcmk__str_eq(op, CRM_OP_REGISTER, pcmk__str_none)) { - rc = process_lrmd_signon(client, request, call_id, &reply); - do_reply = 1; } else if (pcmk__str_eq(op, LRMD_OP_RSC_REG, pcmk__str_none)) { if (allowed) { rc = process_lrmd_rsc_register(client, id, request); diff --git a/daemons/execd/execd_messages.c b/daemons/execd/execd_messages.c index 3e0b7160094..e10010d7340 100644 --- a/daemons/execd/execd_messages.c +++ b/daemons/execd/execd_messages.c @@ -35,10 +35,31 @@ static GHashTable *execd_handlers = NULL; static int lrmd_call_id = 0; +static xmlNode * +handle_register_request(pcmk__request_t *request) +{ + int call_id = 0; + int rc = pcmk_rc_ok; + xmlNode *reply = NULL; + + crm_element_value_int(request->xml, PCMK__XA_LRMD_CALLID, &call_id); + rc = execd_process_signon(request->ipc_client, request->xml, call_id, &reply); + + if (rc != pcmk_rc_ok) { + pcmk__set_result(&request->result, pcmk_rc2exitc(rc), PCMK_EXEC_ERROR, + pcmk_rc_str(rc)); + return NULL; + } + + pcmk__set_result(&request->result, CRM_EX_OK, PCMK_EXEC_DONE, NULL); + return reply; +} + static void execd_register_handlers(void) { pcmk__server_command_t handlers[] = { + { CRM_OP_REGISTER, handle_register_request }, { NULL, NULL }, }; diff --git a/daemons/execd/pacemaker-execd.h b/daemons/execd/pacemaker-execd.h index ca945361d42..d3535388bd7 100644 --- a/daemons/execd/pacemaker-execd.h +++ b/daemons/execd/pacemaker-execd.h @@ -111,4 +111,7 @@ void lrmd_drain_alerts(GMainLoop *mloop); void execd_process_message(pcmk__client_t *c, uint32_t id, uint32_t flags, xmlNode *msg); +int execd_process_signon(pcmk__client_t *client, xmlNode *request, int call_id, + xmlNode **reply); + #endif // PACEMAKER_EXECD__H From 2e4be16b60a84419fa707551bdf0bbc4cc7a5fa1 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 9 Jul 2025 11:43:47 -0400 Subject: [PATCH 06/20] Refactor: daemons: Use pcmk__request_t for LRMD_OP_RSC_INFO. --- daemons/execd/execd_commands.c | 33 +++++++++++++---------------- daemons/execd/execd_messages.c | 37 +++++++++++++++++++++++++++++++++ daemons/execd/pacemaker-execd.h | 1 + 3 files changed, 52 insertions(+), 19 deletions(-) diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c index cc8dfda8b61..960c564069c 100644 --- a/daemons/execd/execd_commands.c +++ b/daemons/execd/execd_commands.c @@ -1598,35 +1598,37 @@ process_lrmd_rsc_register(pcmk__client_t *client, uint32_t id, xmlNode *request) return rc; } -static xmlNode * -process_lrmd_get_rsc_info(xmlNode *request, int call_id) +int +execd_process_get_rsc_info(xmlNode *request, int call_id, xmlNode **reply) { - int rc = pcmk_ok; + int rc = pcmk_rc_ok; xmlNode *rsc_xml = pcmk__xpath_find_one(request->doc, "//" PCMK__XE_LRMD_RSC, LOG_ERR); const char *rsc_id = crm_element_value(rsc_xml, PCMK__XA_LRMD_RSC_ID); - xmlNode *reply = NULL; lrmd_rsc_t *rsc = NULL; if (rsc_id == NULL) { - rc = -ENODEV; + rc = ENODEV; } else { rsc = g_hash_table_lookup(rsc_list, rsc_id); if (rsc == NULL) { crm_info("Agent information for '%s' not in cache", rsc_id); - rc = -ENODEV; + rc = ENODEV; } } - reply = create_lrmd_reply(__func__, rc, call_id); + CRM_LOG_ASSERT(reply != NULL); + + *reply = create_lrmd_reply(__func__, rc, call_id); if (rsc) { - crm_xml_add(reply, PCMK__XA_LRMD_RSC_ID, rsc->rsc_id); - crm_xml_add(reply, PCMK__XA_LRMD_CLASS, rsc->class); - crm_xml_add(reply, PCMK__XA_LRMD_PROVIDER, rsc->provider); - crm_xml_add(reply, PCMK__XA_LRMD_TYPE, rsc->type); + crm_xml_add(*reply, PCMK__XA_LRMD_RSC_ID, rsc->rsc_id); + crm_xml_add(*reply, PCMK__XA_LRMD_CLASS, rsc->class); + crm_xml_add(*reply, PCMK__XA_LRMD_PROVIDER, rsc->provider); + crm_xml_add(*reply, PCMK__XA_LRMD_TYPE, rsc->type); } - return reply; + + return rc; } static int @@ -1910,13 +1912,6 @@ process_lrmd_message(pcmk__client_t *client, uint32_t id, xmlNode *request) rc = -EACCES; } do_reply = 1; - } else if (pcmk__str_eq(op, LRMD_OP_RSC_INFO, pcmk__str_none)) { - if (allowed) { - reply = process_lrmd_get_rsc_info(request, call_id); - } else { - rc = -EACCES; - } - do_reply = 1; } else if (pcmk__str_eq(op, LRMD_OP_RSC_UNREG, pcmk__str_none)) { if (allowed) { rc = process_lrmd_rsc_unregister(client, id, request); diff --git a/daemons/execd/execd_messages.c b/daemons/execd/execd_messages.c index e10010d7340..182c99dfd3a 100644 --- a/daemons/execd/execd_messages.c +++ b/daemons/execd/execd_messages.c @@ -27,6 +27,7 @@ #include // pcmk__client_s, pcmk__find_client #include // pcmk_rc_e, pcmk_rc_str #include // crm_strdup_printf +#include // pcmk_is_set #include // crm_xml_add, crm_element_value #include // PCMK__XA_LRMD_*, pcmk__xe_is @@ -55,11 +56,47 @@ handle_register_request(pcmk__request_t *request) return reply; } +static xmlNode * +handle_rsc_info_request(pcmk__request_t *request) +{ + int call_id = 0; + int rc = pcmk_rc_ok; + bool allowed = pcmk_is_set(request->ipc_client->flags, + pcmk__client_privileged); + xmlNode *reply = NULL; + + if (!allowed) { + pcmk__set_result(&request->result, CRM_EX_INSUFFICIENT_PRIV, + PCMK_EXEC_ERROR, NULL); + crm_warn("Rejecting IPC request '%s' from unprivileged client %s", + request->op, pcmk__client_name(request->ipc_client)); + return NULL; + } + + crm_element_value_int(request->xml, PCMK__XA_LRMD_CALLID, &call_id); + + /* This returns ENODEV if the resource isn't in the cache which will be + * logged as an error. However, this isn't fatal to the client - it may + * querying to see if the resource exists before deciding to register it. + */ + rc = execd_process_get_rsc_info(request->xml, call_id, &reply); + + if (rc == pcmk_rc_ok) { + pcmk__set_result(&request->result, CRM_EX_OK, PCMK_EXEC_DONE, NULL); + } else { + pcmk__set_result(&request->result, pcmk_rc2exitc(rc), PCMK_EXEC_ERROR, + pcmk_rc_str(rc)); + } + + return reply; +} + static void execd_register_handlers(void) { pcmk__server_command_t handlers[] = { { CRM_OP_REGISTER, handle_register_request }, + { LRMD_OP_RSC_INFO, handle_rsc_info_request }, { NULL, NULL }, }; diff --git a/daemons/execd/pacemaker-execd.h b/daemons/execd/pacemaker-execd.h index d3535388bd7..15584166af2 100644 --- a/daemons/execd/pacemaker-execd.h +++ b/daemons/execd/pacemaker-execd.h @@ -111,6 +111,7 @@ void lrmd_drain_alerts(GMainLoop *mloop); void execd_process_message(pcmk__client_t *c, uint32_t id, uint32_t flags, xmlNode *msg); +int execd_process_get_rsc_info(xmlNode *request, int call_id, xmlNode **reply); int execd_process_signon(pcmk__client_t *client, xmlNode *request, int call_id, xmlNode **reply); From a21c98931e8bbc556a39d9437aa67f006cbc075c Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 9 Jul 2025 12:53:57 -0400 Subject: [PATCH 07/20] Refactor: daemons: Expose two more functions to all of execd. ...and rename them to make them consistent. These are needed to handle processing certain message types. --- daemons/execd/execd_commands.c | 18 +++++++++--------- daemons/execd/pacemaker-execd.h | 3 +++ 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c index 960c564069c..d51e4a13c02 100644 --- a/daemons/execd/execd_commands.c +++ b/daemons/execd/execd_commands.c @@ -547,8 +547,8 @@ schedule_lrmd_cmd(lrmd_rsc_t * rsc, lrmd_cmd_t * cmd) } } -static xmlNode * -create_lrmd_reply(const char *origin, int rc, int call_id) +xmlNode * +execd_create_reply(const char *origin, int rc, int call_id) { xmlNode *reply = pcmk__xe_create(NULL, PCMK__XE_LRMD_REPLY); @@ -696,8 +696,8 @@ send_cmd_complete_notify(lrmd_cmd_t * cmd) pcmk__xml_free(notify); } -static void -send_generic_notify(int rc, xmlNode * request) +void +execd_send_generic_notify(int rc, xmlNode *request) { if (pcmk__ipc_client_count() != 0) { int call_id = 0; @@ -1564,7 +1564,7 @@ execd_process_signon(pcmk__client_t *client, xmlNode *request, int call_id, pcmk__assert(reply != NULL); - *reply = create_lrmd_reply(__func__, pcmk_rc2legacy(rc), call_id); + *reply = execd_create_reply(__func__, pcmk_rc2legacy(rc), call_id); crm_xml_add(*reply, PCMK__XA_LRMD_OP, CRM_OP_REGISTER); crm_xml_add(*reply, PCMK__XA_LRMD_CLIENTID, client->id); crm_xml_add(*reply, PCMK__XA_LRMD_PROTOCOL_VERSION, LRMD_PROTOCOL_VERSION); @@ -1620,7 +1620,7 @@ execd_process_get_rsc_info(xmlNode *request, int call_id, xmlNode **reply) CRM_LOG_ASSERT(reply != NULL); - *reply = create_lrmd_reply(__func__, rc, call_id); + *reply = execd_create_reply(__func__, pcmk_rc2legacy(rc), call_id); if (rsc) { crm_xml_add(*reply, PCMK__XA_LRMD_RSC_ID, rsc->rsc_id); crm_xml_add(*reply, PCMK__XA_LRMD_CLASS, rsc->class); @@ -1856,7 +1856,7 @@ process_lrmd_get_recurring(xmlNode *request, int call_id) } } - reply = create_lrmd_reply(__func__, rc, call_id); + reply = execd_create_reply(__func__, rc, call_id); // If resource ID is not specified, check all resources if (rsc_id == NULL) { @@ -1988,7 +1988,7 @@ process_lrmd_message(pcmk__client_t *client, uint32_t id, xmlNode *request) int send_rc = pcmk_rc_ok; if (reply == NULL) { - reply = create_lrmd_reply(__func__, rc, call_id); + reply = execd_create_reply(__func__, rc, call_id); } send_rc = lrmd_server_send_reply(client, id, reply); pcmk__xml_free(reply); @@ -1999,6 +1999,6 @@ process_lrmd_message(pcmk__client_t *client, uint32_t id, xmlNode *request) } if (do_notify) { - send_generic_notify(rc, request); + execd_send_generic_notify(rc, request); } } diff --git a/daemons/execd/pacemaker-execd.h b/daemons/execd/pacemaker-execd.h index 15584166af2..2a18678131a 100644 --- a/daemons/execd/pacemaker-execd.h +++ b/daemons/execd/pacemaker-execd.h @@ -111,6 +111,9 @@ void lrmd_drain_alerts(GMainLoop *mloop); void execd_process_message(pcmk__client_t *c, uint32_t id, uint32_t flags, xmlNode *msg); +xmlNode *execd_create_reply(const char *origin, int rc, int call_id); +void execd_send_generic_notify(int rc, xmlNode *request); + int execd_process_get_rsc_info(xmlNode *request, int call_id, xmlNode **reply); int execd_process_signon(pcmk__client_t *client, xmlNode *request, int call_id, xmlNode **reply); From 397804a9233a1bbac0f2ac5bdf2069d7cebd1cac Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 9 Jul 2025 12:58:51 -0400 Subject: [PATCH 08/20] Refactor: daemons: Use pcmk__request_t for LRMD_OP_RSC_REG. Handling this message type means we also need to send out a generic notify message after sending the reply. For the moment, the rc value being passed to execd_send_generic_notify doesn't matter because execd_process_rsc_register doesn't return anything (previously, it only ever returned pcmk_ok). --- daemons/execd/execd_commands.c | 19 +++++----------- daemons/execd/execd_messages.c | 39 +++++++++++++++++++++++++++++++++ daemons/execd/pacemaker-execd.h | 1 + 3 files changed, 45 insertions(+), 14 deletions(-) diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c index d51e4a13c02..6950e8a0c5d 100644 --- a/daemons/execd/execd_commands.c +++ b/daemons/execd/execd_commands.c @@ -1577,25 +1577,24 @@ execd_process_signon(pcmk__client_t *client, xmlNode *request, int call_id, return rc; } -static int -process_lrmd_rsc_register(pcmk__client_t *client, uint32_t id, xmlNode *request) +void +execd_process_rsc_register(pcmk__client_t *client, uint32_t id, xmlNode *request) { - int rc = pcmk_ok; lrmd_rsc_t *rsc = build_rsc_from_xml(request); lrmd_rsc_t *dup = g_hash_table_lookup(rsc_list, rsc->rsc_id); if (dup && pcmk__str_eq(rsc->class, dup->class, pcmk__str_casei) && - pcmk__str_eq(rsc->provider, dup->provider, pcmk__str_casei) && pcmk__str_eq(rsc->type, dup->type, pcmk__str_casei)) { + pcmk__str_eq(rsc->provider, dup->provider, pcmk__str_casei) && + pcmk__str_eq(rsc->type, dup->type, pcmk__str_casei)) { crm_notice("Ignoring duplicate registration of '%s'", rsc->rsc_id); free_rsc(rsc); - return rc; + return; } g_hash_table_replace(rsc_list, rsc->rsc_id, rsc); crm_info("Cached agent information for '%s'", rsc->rsc_id); - return rc; } int @@ -1904,14 +1903,6 @@ process_lrmd_message(pcmk__client_t *client, uint32_t id, xmlNode *request) rc = -EPROTONOSUPPORT; #endif do_reply = 1; - } else if (pcmk__str_eq(op, LRMD_OP_RSC_REG, pcmk__str_none)) { - if (allowed) { - rc = process_lrmd_rsc_register(client, id, request); - do_notify = 1; - } else { - rc = -EACCES; - } - do_reply = 1; } else if (pcmk__str_eq(op, LRMD_OP_RSC_UNREG, pcmk__str_none)) { if (allowed) { rc = process_lrmd_rsc_unregister(client, id, request); diff --git a/daemons/execd/execd_messages.c b/daemons/execd/execd_messages.c index 182c99dfd3a..485fec6da05 100644 --- a/daemons/execd/execd_messages.c +++ b/daemons/execd/execd_messages.c @@ -91,12 +91,47 @@ handle_rsc_info_request(pcmk__request_t *request) return reply; } +static xmlNode * +handle_rsc_reg_request(pcmk__request_t *request) +{ + int call_id = 0; + bool allowed = pcmk_is_set(request->ipc_client->flags, + pcmk__client_privileged); + xmlNode *reply = NULL; + + if (!allowed) { + pcmk__set_result(&request->result, CRM_EX_INSUFFICIENT_PRIV, + PCMK_EXEC_ERROR, NULL); + crm_warn("Rejecting IPC request '%s' from unprivileged client %s", + request->op, pcmk__client_name(request->ipc_client)); + return NULL; + } + + crm_element_value_int(request->xml, PCMK__XA_LRMD_CALLID, &call_id); + + execd_process_rsc_register(request->ipc_client, request->ipc_id, request->xml); + + /* Create a generic reply since registering a resource doesn't create + * a more specific one. + */ + reply = execd_create_reply(__func__, pcmk_ok, call_id); + pcmk__set_result(&request->result, CRM_EX_OK, PCMK_EXEC_DONE, NULL); + return reply; +} + +static bool +requires_notify(const char *command) +{ + return pcmk__str_any_of(command, LRMD_OP_RSC_REG, NULL); +} + static void execd_register_handlers(void) { pcmk__server_command_t handlers[] = { { CRM_OP_REGISTER, handle_register_request }, { LRMD_OP_RSC_INFO, handle_rsc_info_request }, + { LRMD_OP_RSC_REG, handle_rsc_reg_request }, { NULL, NULL }, }; @@ -308,6 +343,10 @@ execd_process_message(pcmk__client_t *c, uint32_t id, uint32_t flags, xmlNode *m } pcmk__xml_free(reply); + + if (requires_notify(request.op)) { + execd_send_generic_notify(pcmk_ok, request.xml); + } } reason = request.result.exit_reason; diff --git a/daemons/execd/pacemaker-execd.h b/daemons/execd/pacemaker-execd.h index 2a18678131a..3cb341ee902 100644 --- a/daemons/execd/pacemaker-execd.h +++ b/daemons/execd/pacemaker-execd.h @@ -115,6 +115,7 @@ xmlNode *execd_create_reply(const char *origin, int rc, int call_id); void execd_send_generic_notify(int rc, xmlNode *request); int execd_process_get_rsc_info(xmlNode *request, int call_id, xmlNode **reply); +void execd_process_rsc_register(pcmk__client_t *client, uint32_t id, xmlNode *request); int execd_process_signon(pcmk__client_t *client, xmlNode *request, int call_id, xmlNode **reply); From 694061dec560051f737f67ffd99c6470c3c70857 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 9 Jul 2025 13:07:39 -0400 Subject: [PATCH 09/20] Refactor: daemons: Use pcmk__request_t for LRMD_OP_RSC_EXEC. Additionally, execd_process_rsc_exec should return a standard pacemaker return code. It doesn't do anything useful with the call_id value it extracts and returns, and the caller already knows the call_id (and doesn't do anything with the return value from execd_process_rsc_exec aside from check if it's an error or not). --- daemons/execd/execd_commands.c | 20 +++++----------- daemons/execd/execd_messages.c | 41 +++++++++++++++++++++++++++++++++ daemons/execd/pacemaker-execd.h | 1 + 3 files changed, 48 insertions(+), 14 deletions(-) diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c index 6950e8a0c5d..1eba8503318 100644 --- a/daemons/execd/execd_commands.c +++ b/daemons/execd/execd_commands.c @@ -1664,8 +1664,8 @@ process_lrmd_rsc_unregister(pcmk__client_t *client, uint32_t id, return rc; } -static int -process_lrmd_rsc_exec(pcmk__client_t *client, uint32_t id, xmlNode *request) +int +execd_process_rsc_exec(pcmk__client_t *client, xmlNode *request) { lrmd_rsc_t *rsc = NULL; lrmd_cmd_t *cmd = NULL; @@ -1673,25 +1673,24 @@ process_lrmd_rsc_exec(pcmk__client_t *client, uint32_t id, xmlNode *request) "//" PCMK__XE_LRMD_RSC, LOG_ERR); const char *rsc_id = crm_element_value(rsc_xml, PCMK__XA_LRMD_RSC_ID); - int call_id; if (!rsc_id) { - return -EINVAL; + return EINVAL; } + if (!(rsc = g_hash_table_lookup(rsc_list, rsc_id))) { crm_info("Resource '%s' not found (%d active resources)", rsc_id, g_hash_table_size(rsc_list)); - return -ENODEV; + return ENODEV; } cmd = create_lrmd_cmd(request, client); - call_id = cmd->call_id; /* Don't reference cmd after handing it off to be scheduled. * The cmd could get merged and freed. */ schedule_lrmd_cmd(rsc, cmd); - return call_id; + return pcmk_rc_ok; } static int @@ -1914,13 +1913,6 @@ process_lrmd_message(pcmk__client_t *client, uint32_t id, xmlNode *request) rc = -EACCES; } do_reply = 1; - } else if (pcmk__str_eq(op, LRMD_OP_RSC_EXEC, pcmk__str_none)) { - if (allowed) { - rc = process_lrmd_rsc_exec(client, id, request); - } else { - rc = -EACCES; - } - do_reply = 1; } else if (pcmk__str_eq(op, LRMD_OP_RSC_CANCEL, pcmk__str_none)) { if (allowed) { rc = process_lrmd_rsc_cancel(client, id, request); diff --git a/daemons/execd/execd_messages.c b/daemons/execd/execd_messages.c index 485fec6da05..e22cfcb0f53 100644 --- a/daemons/execd/execd_messages.c +++ b/daemons/execd/execd_messages.c @@ -56,6 +56,46 @@ handle_register_request(pcmk__request_t *request) return reply; } +static xmlNode * +handle_rsc_exec_request(pcmk__request_t *request) +{ + int call_id = 0; + int rc = pcmk_rc_ok; + bool allowed = pcmk_is_set(request->ipc_client->flags, + pcmk__client_privileged); + xmlNode *reply = NULL; + + if (!allowed) { + pcmk__set_result(&request->result, CRM_EX_INSUFFICIENT_PRIV, + PCMK_EXEC_ERROR, NULL); + crm_warn("Rejecting IPC request '%s' from unprivileged client %s", + request->op, pcmk__client_name(request->ipc_client)); + return NULL; + } + + crm_element_value_int(request->xml, PCMK__XA_LRMD_CALLID, &call_id); + + rc = execd_process_rsc_exec(request->ipc_client, request->xml); + + if (rc == pcmk_rc_ok) { + pcmk__set_result(&request->result, CRM_EX_OK, PCMK_EXEC_DONE, NULL); + + /* This looks redundant, but it's unfortunately necessary. The first + * argument is set as the PCMK__XA_LRMD_RC attribute in the response. + * On the other side of the connection, lrmd_send_command will read + * this and use it as its return value, which passes back up to the + * public API function lrmd_api_exec. + */ + reply = execd_create_reply(__func__, call_id, call_id); + } else { + pcmk__set_result(&request->result, pcmk_rc2exitc(rc), PCMK_EXEC_ERROR, + pcmk_rc_str(rc)); + reply = execd_create_reply(__func__, pcmk_rc2legacy(rc), call_id); + } + + return reply; +} + static xmlNode * handle_rsc_info_request(pcmk__request_t *request) { @@ -130,6 +170,7 @@ execd_register_handlers(void) { pcmk__server_command_t handlers[] = { { CRM_OP_REGISTER, handle_register_request }, + { LRMD_OP_RSC_EXEC, handle_rsc_exec_request }, { LRMD_OP_RSC_INFO, handle_rsc_info_request }, { LRMD_OP_RSC_REG, handle_rsc_reg_request }, { NULL, NULL }, diff --git a/daemons/execd/pacemaker-execd.h b/daemons/execd/pacemaker-execd.h index 3cb341ee902..878ac8231f7 100644 --- a/daemons/execd/pacemaker-execd.h +++ b/daemons/execd/pacemaker-execd.h @@ -115,6 +115,7 @@ xmlNode *execd_create_reply(const char *origin, int rc, int call_id); void execd_send_generic_notify(int rc, xmlNode *request); int execd_process_get_rsc_info(xmlNode *request, int call_id, xmlNode **reply); +int execd_process_rsc_exec(pcmk__client_t *client, xmlNode *request); void execd_process_rsc_register(pcmk__client_t *client, uint32_t id, xmlNode *request); int execd_process_signon(pcmk__client_t *client, xmlNode *request, int call_id, xmlNode **reply); From cd1ba028c3ecd938fd1ba1edbc2f67b3cb841167 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 9 Jul 2025 14:27:10 -0400 Subject: [PATCH 10/20] Refactor: daemons: Use pcmk__request_t for LRMD_OP_RSC_CANCEL. This additionally changes a couple functions to return a standard Pacemaker return code. --- daemons/execd/execd_commands.c | 23 ++++++++------------- daemons/execd/execd_messages.c | 36 +++++++++++++++++++++++++++++++++ daemons/execd/pacemaker-execd.h | 1 + 3 files changed, 45 insertions(+), 15 deletions(-) diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c index 1eba8503318..42148e91d17 100644 --- a/daemons/execd/execd_commands.c +++ b/daemons/execd/execd_commands.c @@ -1711,7 +1711,7 @@ cancel_op(const char *rsc_id, const char *action, guint interval_ms) * never existed. */ if (!rsc) { - return -ENODEV; + return ENODEV; } for (gIter = rsc->pending_ops; gIter != NULL; gIter = gIter->next) { @@ -1720,7 +1720,7 @@ cancel_op(const char *rsc_id, const char *action, guint interval_ms) if (action_matches(cmd, action, interval_ms)) { cmd->result.execution_status = PCMK_EXEC_CANCELLED; cmd_finalize(cmd, rsc); - return pcmk_ok; + return pcmk_rc_ok; } } @@ -1735,7 +1735,7 @@ cancel_op(const char *rsc_id, const char *action, guint interval_ms) if (rsc->active != cmd) { cmd_finalize(cmd, rsc); } - return pcmk_ok; + return pcmk_rc_ok; } } } else if (services_action_cancel(rsc_id, @@ -1745,10 +1745,10 @@ cancel_op(const char *rsc_id, const char *action, guint interval_ms) * this action was cancelled, which will destroy the cmd and remove * it from the recurring_op list. Do not do that in this function * if the service library says it cancelled it. */ - return pcmk_ok; + return pcmk_rc_ok; } - return -EOPNOTSUPP; + return EOPNOTSUPP; } static void @@ -1790,8 +1790,8 @@ cancel_all_recurring(lrmd_rsc_t * rsc, const char *client_id) g_list_free(cmd_list); } -static int -process_lrmd_rsc_cancel(pcmk__client_t *client, uint32_t id, xmlNode *request) +int +execd_process_rsc_cancel(pcmk__client_t *client, xmlNode *request) { xmlNode *rsc_xml = pcmk__xpath_find_one(request->doc, "//" PCMK__XE_LRMD_RSC, @@ -1803,7 +1803,7 @@ process_lrmd_rsc_cancel(pcmk__client_t *client, uint32_t id, xmlNode *request) crm_element_value_ms(rsc_xml, PCMK__XA_LRMD_RSC_INTERVAL, &interval_ms); if (!rsc_id || !action) { - return -EINVAL; + return EINVAL; } return cancel_op(rsc_id, action, interval_ms); @@ -1913,13 +1913,6 @@ process_lrmd_message(pcmk__client_t *client, uint32_t id, xmlNode *request) rc = -EACCES; } do_reply = 1; - } else if (pcmk__str_eq(op, LRMD_OP_RSC_CANCEL, pcmk__str_none)) { - if (allowed) { - rc = process_lrmd_rsc_cancel(client, id, request); - } else { - rc = -EACCES; - } - do_reply = 1; } else if (pcmk__str_eq(op, LRMD_OP_POKE, pcmk__str_none)) { do_notify = 1; do_reply = 1; diff --git a/daemons/execd/execd_messages.c b/daemons/execd/execd_messages.c index e22cfcb0f53..d5651d94a21 100644 --- a/daemons/execd/execd_messages.c +++ b/daemons/execd/execd_messages.c @@ -56,6 +56,41 @@ handle_register_request(pcmk__request_t *request) return reply; } +static xmlNode * +handle_rsc_cancel_request(pcmk__request_t *request) +{ + int call_id = 0; + int rc = pcmk_rc_ok; + bool allowed = pcmk_is_set(request->ipc_client->flags, + pcmk__client_privileged); + xmlNode *reply = NULL; + + if (!allowed) { + pcmk__set_result(&request->result, CRM_EX_INSUFFICIENT_PRIV, + PCMK_EXEC_ERROR, NULL); + crm_warn("Rejecting IPC request '%s' from unprivileged client %s", + request->op, pcmk__client_name(request->ipc_client)); + return NULL; + } + + crm_element_value_int(request->xml, PCMK__XA_LRMD_CALLID, &call_id); + + rc = execd_process_rsc_cancel(request->ipc_client, request->xml); + + if (rc == pcmk_rc_ok) { + pcmk__set_result(&request->result, CRM_EX_OK, PCMK_EXEC_DONE, NULL); + } else { + pcmk__set_result(&request->result, pcmk_rc2exitc(rc), PCMK_EXEC_ERROR, + pcmk_rc_str(rc)); + } + + /* Create a generic reply since canceling a resource doesn't create a + * more specific one. + */ + reply = execd_create_reply(__func__, pcmk_rc2legacy(rc), call_id); + return reply; +} + static xmlNode * handle_rsc_exec_request(pcmk__request_t *request) { @@ -170,6 +205,7 @@ execd_register_handlers(void) { pcmk__server_command_t handlers[] = { { CRM_OP_REGISTER, handle_register_request }, + { LRMD_OP_RSC_CANCEL, handle_rsc_cancel_request }, { LRMD_OP_RSC_EXEC, handle_rsc_exec_request }, { LRMD_OP_RSC_INFO, handle_rsc_info_request }, { LRMD_OP_RSC_REG, handle_rsc_reg_request }, diff --git a/daemons/execd/pacemaker-execd.h b/daemons/execd/pacemaker-execd.h index 878ac8231f7..4abe253d776 100644 --- a/daemons/execd/pacemaker-execd.h +++ b/daemons/execd/pacemaker-execd.h @@ -115,6 +115,7 @@ xmlNode *execd_create_reply(const char *origin, int rc, int call_id); void execd_send_generic_notify(int rc, xmlNode *request); int execd_process_get_rsc_info(xmlNode *request, int call_id, xmlNode **reply); +int execd_process_rsc_cancel(pcmk__client_t *client, xmlNode *request); int execd_process_rsc_exec(pcmk__client_t *client, xmlNode *request); void execd_process_rsc_register(pcmk__client_t *client, uint32_t id, xmlNode *request); int execd_process_signon(pcmk__client_t *client, xmlNode *request, int call_id, From 0f1ef52c8ffec6b759bb8b3caf2a95a843f01ec7 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 9 Jul 2025 14:35:32 -0400 Subject: [PATCH 11/20] Refactor: daemons: Use pcmk__request_t for LRMD_OP_ALERT_EXEC. --- daemons/execd/execd_alerts.c | 13 ++++++------ daemons/execd/execd_commands.c | 7 ------- daemons/execd/execd_messages.c | 36 +++++++++++++++++++++++++++++++++ daemons/execd/pacemaker-execd.h | 3 +-- 4 files changed, 44 insertions(+), 15 deletions(-) diff --git a/daemons/execd/execd_alerts.c b/daemons/execd/execd_alerts.c index bdf75f0477a..64b67c2712a 100644 --- a/daemons/execd/execd_alerts.c +++ b/daemons/execd/execd_alerts.c @@ -102,7 +102,7 @@ alert_complete(svc_action_t *action) } int -process_lrmd_alert_exec(pcmk__client_t *client, uint32_t id, xmlNode *request) +execd_process_alert_exec(pcmk__client_t *client, xmlNode *request) { static int alert_sequence_no = 0; @@ -114,17 +114,17 @@ process_lrmd_alert_exec(pcmk__client_t *client, uint32_t id, xmlNode *request) PCMK__XA_LRMD_ALERT_PATH); svc_action_t *action = NULL; int alert_timeout = 0; - int rc = pcmk_ok; + int rc = pcmk_rc_ok; GHashTable *params = NULL; struct alert_cb_s *cb_data = NULL; if ((alert_id == NULL) || (alert_path == NULL) || (client == NULL) || (client->id == NULL)) { /* hint static analyzer */ - rc = -EINVAL; + rc = EINVAL; goto err; } if (draining_alerts) { - return pcmk_ok; + return pcmk_rc_ok; } crm_element_value_int(alert_xml, PCMK__XA_LRMD_TIMEOUT, &alert_timeout); @@ -144,12 +144,13 @@ process_lrmd_alert_exec(pcmk__client_t *client, uint32_t id, xmlNode *request) action = services_alert_create(alert_id, alert_path, alert_timeout, params, alert_sequence_no, cb_data); if (action->rc != PCMK_OCF_UNKNOWN) { - rc = -E2BIG; + rc = E2BIG; goto err; } rc = services_action_user(action, CRM_DAEMON_USER); if (rc < 0) { + rc = pcmk_legacy2rc(rc); goto err; } @@ -157,7 +158,7 @@ process_lrmd_alert_exec(pcmk__client_t *client, uint32_t id, xmlNode *request) if (services_alert_async(action, alert_complete) == FALSE) { services_action_free(action); } - return pcmk_ok; + return pcmk_rc_ok; err: if (cb_data) { diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c index 42148e91d17..e03e203e3d7 100644 --- a/daemons/execd/execd_commands.c +++ b/daemons/execd/execd_commands.c @@ -1931,13 +1931,6 @@ process_lrmd_message(pcmk__client_t *client, uint32_t id, xmlNode *request) } else { rc = -EACCES; } - } else if (pcmk__str_eq(op, LRMD_OP_ALERT_EXEC, pcmk__str_none)) { - if (allowed) { - rc = process_lrmd_alert_exec(client, id, request); - } else { - rc = -EACCES; - } - do_reply = 1; } else if (pcmk__str_eq(op, LRMD_OP_GET_RECURRING, pcmk__str_none)) { if (allowed) { reply = process_lrmd_get_recurring(request, call_id); diff --git a/daemons/execd/execd_messages.c b/daemons/execd/execd_messages.c index d5651d94a21..09f9c1ac1d7 100644 --- a/daemons/execd/execd_messages.c +++ b/daemons/execd/execd_messages.c @@ -56,6 +56,41 @@ handle_register_request(pcmk__request_t *request) return reply; } +static xmlNode * +handle_alert_exec_request(pcmk__request_t *request) +{ + int call_id = 0; + int rc = pcmk_rc_ok; + bool allowed = pcmk_is_set(request->ipc_client->flags, + pcmk__client_privileged); + xmlNode *reply = NULL; + + if (!allowed) { + pcmk__set_result(&request->result, CRM_EX_INSUFFICIENT_PRIV, + PCMK_EXEC_ERROR, NULL); + crm_warn("Rejecting IPC request '%s' from unprivileged client %s", + request->op, pcmk__client_name(request->ipc_client)); + return NULL; + } + + crm_element_value_int(request->xml, PCMK__XA_LRMD_CALLID, &call_id); + + rc = execd_process_alert_exec(request->ipc_client, request->xml); + + if (rc == pcmk_rc_ok) { + pcmk__set_result(&request->result, CRM_EX_OK, PCMK_EXEC_DONE, NULL); + } else { + pcmk__set_result(&request->result, pcmk_rc2exitc(rc), PCMK_EXEC_ERROR, + pcmk_rc_str(rc)); + } + + /* Create a generic reply since executing an alert doesn't create a + * more specific one. + */ + reply = execd_create_reply(__func__, pcmk_rc2legacy(rc), call_id); + return reply; +} + static xmlNode * handle_rsc_cancel_request(pcmk__request_t *request) { @@ -205,6 +240,7 @@ execd_register_handlers(void) { pcmk__server_command_t handlers[] = { { CRM_OP_REGISTER, handle_register_request }, + { LRMD_OP_ALERT_EXEC, handle_alert_exec_request }, { LRMD_OP_RSC_CANCEL, handle_rsc_cancel_request }, { LRMD_OP_RSC_EXEC, handle_rsc_exec_request }, { LRMD_OP_RSC_INFO, handle_rsc_info_request }, diff --git a/daemons/execd/pacemaker-execd.h b/daemons/execd/pacemaker-execd.h index 4abe253d776..8215dd47182 100644 --- a/daemons/execd/pacemaker-execd.h +++ b/daemons/execd/pacemaker-execd.h @@ -104,8 +104,6 @@ void remoted_request_cib_schema_files(void); void execd_unregister_handlers(void); -int process_lrmd_alert_exec(pcmk__client_t *client, uint32_t id, - xmlNode *request); void lrmd_drain_alerts(GMainLoop *mloop); void execd_process_message(pcmk__client_t *c, uint32_t id, uint32_t flags, @@ -114,6 +112,7 @@ void execd_process_message(pcmk__client_t *c, uint32_t id, uint32_t flags, xmlNode *execd_create_reply(const char *origin, int rc, int call_id); void execd_send_generic_notify(int rc, xmlNode *request); +int execd_process_alert_exec(pcmk__client_t *client, xmlNode *request); int execd_process_get_rsc_info(xmlNode *request, int call_id, xmlNode **reply); int execd_process_rsc_cancel(pcmk__client_t *client, xmlNode *request); int execd_process_rsc_exec(pcmk__client_t *client, xmlNode *request); From 9dcef97b77c38eb92a1b07c0102634e7e70bee78 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 9 Jul 2025 14:39:45 -0400 Subject: [PATCH 12/20] Refactor: daemons: Use pcmk__request_t for LRMD_OP_POKE. --- daemons/execd/execd_commands.c | 3 --- daemons/execd/execd_messages.c | 16 +++++++++++++++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c index e03e203e3d7..0507ade9382 100644 --- a/daemons/execd/execd_commands.c +++ b/daemons/execd/execd_commands.c @@ -1913,9 +1913,6 @@ process_lrmd_message(pcmk__client_t *client, uint32_t id, xmlNode *request) rc = -EACCES; } do_reply = 1; - } else if (pcmk__str_eq(op, LRMD_OP_POKE, pcmk__str_none)) { - do_notify = 1; - do_reply = 1; } else if (pcmk__str_eq(op, LRMD_OP_CHECK, pcmk__str_none)) { if (allowed) { xmlNode *wrapper = pcmk__xe_first_child(request, diff --git a/daemons/execd/execd_messages.c b/daemons/execd/execd_messages.c index 09f9c1ac1d7..48c5689b120 100644 --- a/daemons/execd/execd_messages.c +++ b/daemons/execd/execd_messages.c @@ -91,6 +91,19 @@ handle_alert_exec_request(pcmk__request_t *request) return reply; } +static xmlNode * +handle_poke_request(pcmk__request_t *request) +{ + int call_id = 0; + xmlNode *reply = NULL; + + crm_element_value_int(request->xml, PCMK__XA_LRMD_CALLID, &call_id); + + /* Create a generic reply since this doesn't create a more specific one */ + reply = execd_create_reply(__func__, pcmk_ok, call_id); + return reply; +} + static xmlNode * handle_rsc_cancel_request(pcmk__request_t *request) { @@ -232,7 +245,7 @@ handle_rsc_reg_request(pcmk__request_t *request) static bool requires_notify(const char *command) { - return pcmk__str_any_of(command, LRMD_OP_RSC_REG, NULL); + return pcmk__str_any_of(command, LRMD_OP_POKE, LRMD_OP_RSC_REG, NULL); } static void @@ -241,6 +254,7 @@ execd_register_handlers(void) pcmk__server_command_t handlers[] = { { CRM_OP_REGISTER, handle_register_request }, { LRMD_OP_ALERT_EXEC, handle_alert_exec_request }, + { LRMD_OP_POKE, handle_poke_request }, { LRMD_OP_RSC_CANCEL, handle_rsc_cancel_request }, { LRMD_OP_RSC_EXEC, handle_rsc_exec_request }, { LRMD_OP_RSC_INFO, handle_rsc_info_request }, From f49fe3a21efe76a087cbb8e21641576d43119cf4 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 9 Jul 2025 14:47:18 -0400 Subject: [PATCH 13/20] Refactor: daemons: Use pcmk__request_t for LRMD_OP_GET_RECURRING. --- daemons/execd/execd_commands.c | 27 +++++++++++---------------- daemons/execd/execd_messages.c | 32 ++++++++++++++++++++++++++++++++ daemons/execd/pacemaker-execd.h | 1 + 3 files changed, 44 insertions(+), 16 deletions(-) diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c index 0507ade9382..332007c2e72 100644 --- a/daemons/execd/execd_commands.c +++ b/daemons/execd/execd_commands.c @@ -1826,13 +1826,12 @@ add_recurring_op_xml(xmlNode *reply, lrmd_rsc_t *rsc) } } -static xmlNode * -process_lrmd_get_recurring(xmlNode *request, int call_id) +int +execd_process_get_recurring(xmlNode *request, int call_id, xmlNode **reply) { - int rc = pcmk_ok; + int rc = pcmk_rc_ok; const char *rsc_id = NULL; lrmd_rsc_t *rsc = NULL; - xmlNode *reply = NULL; xmlNode *rsc_xml = NULL; // Resource ID is optional @@ -1850,11 +1849,13 @@ process_lrmd_get_recurring(xmlNode *request, int call_id) if (rsc == NULL) { crm_info("Resource '%s' not found (%d active resources)", rsc_id, g_hash_table_size(rsc_list)); - rc = -ENODEV; + rc = ENODEV; } } - reply = execd_create_reply(__func__, rc, call_id); + CRM_LOG_ASSERT(reply != NULL); + + *reply = execd_create_reply(__func__, pcmk_rc2legacy(rc), call_id); // If resource ID is not specified, check all resources if (rsc_id == NULL) { @@ -1864,12 +1865,13 @@ process_lrmd_get_recurring(xmlNode *request, int call_id) g_hash_table_iter_init(&iter, rsc_list); while (g_hash_table_iter_next(&iter, (gpointer *) &key, (gpointer *) &rsc)) { - add_recurring_op_xml(reply, rsc); + add_recurring_op_xml(*reply, rsc); } } else if (rsc) { - add_recurring_op_xml(reply, rsc); + add_recurring_op_xml(*reply, rsc); } - return reply; + + return rc; } void @@ -1928,13 +1930,6 @@ process_lrmd_message(pcmk__client_t *client, uint32_t id, xmlNode *request) } else { rc = -EACCES; } - } else if (pcmk__str_eq(op, LRMD_OP_GET_RECURRING, pcmk__str_none)) { - if (allowed) { - reply = process_lrmd_get_recurring(request, call_id); - } else { - rc = -EACCES; - } - do_reply = 1; } else { rc = -EOPNOTSUPP; do_reply = 1; diff --git a/daemons/execd/execd_messages.c b/daemons/execd/execd_messages.c index 48c5689b120..e30da07bc1e 100644 --- a/daemons/execd/execd_messages.c +++ b/daemons/execd/execd_messages.c @@ -91,6 +91,37 @@ handle_alert_exec_request(pcmk__request_t *request) return reply; } +static xmlNode * +handle_get_recurring_request(pcmk__request_t *request) +{ + int call_id = 0; + int rc = pcmk_rc_ok; + bool allowed = pcmk_is_set(request->ipc_client->flags, + pcmk__client_privileged); + xmlNode *reply = NULL; + + if (!allowed) { + pcmk__set_result(&request->result, CRM_EX_INSUFFICIENT_PRIV, + PCMK_EXEC_ERROR, NULL); + crm_warn("Rejecting IPC request '%s' from unprivileged client %s", + request->op, pcmk__client_name(request->ipc_client)); + return NULL; + } + + crm_element_value_int(request->xml, PCMK__XA_LRMD_CALLID, &call_id); + + rc = execd_process_get_recurring(request->xml, call_id, &reply); + + if (rc == pcmk_rc_ok) { + pcmk__set_result(&request->result, CRM_EX_OK, PCMK_EXEC_DONE, NULL); + } else { + pcmk__set_result(&request->result, pcmk_rc2exitc(rc), PCMK_EXEC_ERROR, + pcmk_rc_str(rc)); + } + + return reply; +} + static xmlNode * handle_poke_request(pcmk__request_t *request) { @@ -254,6 +285,7 @@ execd_register_handlers(void) pcmk__server_command_t handlers[] = { { CRM_OP_REGISTER, handle_register_request }, { LRMD_OP_ALERT_EXEC, handle_alert_exec_request }, + { LRMD_OP_GET_RECURRING, handle_get_recurring_request }, { LRMD_OP_POKE, handle_poke_request }, { LRMD_OP_RSC_CANCEL, handle_rsc_cancel_request }, { LRMD_OP_RSC_EXEC, handle_rsc_exec_request }, diff --git a/daemons/execd/pacemaker-execd.h b/daemons/execd/pacemaker-execd.h index 8215dd47182..b6d831e12fc 100644 --- a/daemons/execd/pacemaker-execd.h +++ b/daemons/execd/pacemaker-execd.h @@ -113,6 +113,7 @@ xmlNode *execd_create_reply(const char *origin, int rc, int call_id); void execd_send_generic_notify(int rc, xmlNode *request); int execd_process_alert_exec(pcmk__client_t *client, xmlNode *request); +int execd_process_get_recurring(xmlNode *request, int call_id, xmlNode **reply); int execd_process_get_rsc_info(xmlNode *request, int call_id, xmlNode **reply); int execd_process_rsc_cancel(pcmk__client_t *client, xmlNode *request); int execd_process_rsc_exec(pcmk__client_t *client, xmlNode *request); From 63f0603a8fb7cfccfa9b212dd952aa6fbb62ce90 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 9 Jul 2025 14:58:23 -0400 Subject: [PATCH 14/20] Refactor: daemons: Use pcmk__request_t for LRMD_OP_CHECK. --- daemons/execd/execd_commands.c | 15 ------------- daemons/execd/execd_messages.c | 39 ++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c index 332007c2e72..c31e057b4f6 100644 --- a/daemons/execd/execd_commands.c +++ b/daemons/execd/execd_commands.c @@ -1915,21 +1915,6 @@ process_lrmd_message(pcmk__client_t *client, uint32_t id, xmlNode *request) rc = -EACCES; } do_reply = 1; - } else if (pcmk__str_eq(op, LRMD_OP_CHECK, pcmk__str_none)) { - if (allowed) { - xmlNode *wrapper = pcmk__xe_first_child(request, - PCMK__XE_LRMD_CALLDATA, - NULL, NULL); - xmlNode *data = pcmk__xe_first_child(wrapper, NULL, NULL, NULL); - - const char *timeout = NULL; - - CRM_LOG_ASSERT(data != NULL); - timeout = crm_element_value(data, PCMK__XA_LRMD_WATCHDOG); - pcmk__valid_stonith_watchdog_timeout(timeout); - } else { - rc = -EACCES; - } } else { rc = -EOPNOTSUPP; do_reply = 1; diff --git a/daemons/execd/execd_messages.c b/daemons/execd/execd_messages.c index e30da07bc1e..a623fc90292 100644 --- a/daemons/execd/execd_messages.c +++ b/daemons/execd/execd_messages.c @@ -91,6 +91,44 @@ handle_alert_exec_request(pcmk__request_t *request) return reply; } +static xmlNode * +handle_check_request(pcmk__request_t *request) +{ + bool allowed = pcmk_is_set(request->ipc_client->flags, + pcmk__client_privileged); + xmlNode *wrapper = NULL; + xmlNode *data = NULL; + const char *timeout = NULL; + + if (!allowed) { + pcmk__set_result(&request->result, CRM_EX_INSUFFICIENT_PRIV, + PCMK_EXEC_ERROR, NULL); + crm_warn("Rejecting IPC request '%s' from unprivileged client %s", + request->op, pcmk__client_name(request->ipc_client)); + return NULL; + } + + wrapper = pcmk__xe_first_child(request->xml, + PCMK__XE_LRMD_CALLDATA, + NULL, NULL); + data = pcmk__xe_first_child(wrapper, NULL, NULL, NULL); + + if (data == NULL) { + pcmk__set_result(&request->result, CRM_EX_SOFTWARE, PCMK_EXEC_INVALID, + NULL); + return NULL; + } + + timeout = crm_element_value(data, PCMK__XA_LRMD_WATCHDOG); + /* FIXME: This just exits on certain conditions, which seems like a pretty + * extreme reaction for a daemon to take. + */ + pcmk__valid_stonith_watchdog_timeout(timeout); + + pcmk__set_result(&request->result, CRM_EX_OK, PCMK_EXEC_DONE, NULL); + return NULL; +} + static xmlNode * handle_get_recurring_request(pcmk__request_t *request) { @@ -285,6 +323,7 @@ execd_register_handlers(void) pcmk__server_command_t handlers[] = { { CRM_OP_REGISTER, handle_register_request }, { LRMD_OP_ALERT_EXEC, handle_alert_exec_request }, + { LRMD_OP_CHECK, handle_check_request }, { LRMD_OP_GET_RECURRING, handle_get_recurring_request }, { LRMD_OP_POKE, handle_poke_request }, { LRMD_OP_RSC_CANCEL, handle_rsc_cancel_request }, From 10d2a059f3cce1c949d22776667c396cec77863c Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 9 Jul 2025 15:10:08 -0400 Subject: [PATCH 15/20] Refactor: daemons: Use pcmk__request_t for LRMD_OP_RSC_UNREG. This requires a little extra work, in addition to the steps of exposing a function and changing its return type to be a standard Pacemaker return code. Sending a generic notify for unregistering a resource also requires inspecting the return value, which we luckily have in the reply XML so we can just grab it from there as needed. --- daemons/execd/execd_commands.c | 36 ++++++++---------------- daemons/execd/execd_messages.c | 50 +++++++++++++++++++++++++++++---- daemons/execd/pacemaker-execd.h | 1 + 3 files changed, 57 insertions(+), 30 deletions(-) diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c index c31e057b4f6..383aada7019 100644 --- a/daemons/execd/execd_commands.c +++ b/daemons/execd/execd_commands.c @@ -1630,11 +1630,10 @@ execd_process_get_rsc_info(xmlNode *request, int call_id, xmlNode **reply) return rc; } -static int -process_lrmd_rsc_unregister(pcmk__client_t *client, uint32_t id, - xmlNode *request) +int +execd_process_rsc_unregister(pcmk__client_t *client, xmlNode *request) { - int rc = pcmk_ok; + int rc = pcmk_rc_ok; lrmd_rsc_t *rsc = NULL; xmlNode *rsc_xml = pcmk__xpath_find_one(request->doc, "//" PCMK__XE_LRMD_RSC, @@ -1642,21 +1641,21 @@ process_lrmd_rsc_unregister(pcmk__client_t *client, uint32_t id, const char *rsc_id = crm_element_value(rsc_xml, PCMK__XA_LRMD_RSC_ID); if (!rsc_id) { - return -ENODEV; + return ENODEV; } rsc = g_hash_table_lookup(rsc_list, rsc_id); if (rsc == NULL) { crm_info("Ignoring unregistration of resource '%s', which is not registered", rsc_id); - return pcmk_ok; + return pcmk_rc_ok; } if (rsc->active) { /* let the caller know there are still active ops on this rsc to watch for */ crm_trace("Operation (%p) still in progress for unregistered resource %s", rsc->active, rsc_id); - rc = -EINPROGRESS; + rc = EINPROGRESS; } g_hash_table_remove(rsc_list, rsc_id); @@ -1884,17 +1883,17 @@ process_lrmd_message(pcmk__client_t *client, uint32_t id, xmlNode *request) int do_notify = 0; xmlNode *reply = NULL; - /* Certain IPC commands may be done only by privileged users (i.e. root or - * hacluster), because they would otherwise provide a means of bypassing - * ACLs. - */ - bool allowed = pcmk_is_set(client->flags, pcmk__client_privileged); - crm_trace("Processing %s operation from %s", op, client->id); crm_element_value_int(request, PCMK__XA_LRMD_CALLID, &call_id); if (pcmk__str_eq(op, CRM_OP_IPC_FWD, pcmk__str_none)) { #ifdef PCMK__COMPILE_REMOTE + /* Certain IPC commands may be done only by privileged users (i.e. root or + * hacluster), because they would otherwise provide a means of bypassing + * ACLs. + */ + bool allowed = pcmk_is_set(client->flags, pcmk__client_privileged); + if (allowed) { ipc_proxy_forward_client(client, request); } else { @@ -1904,17 +1903,6 @@ process_lrmd_message(pcmk__client_t *client, uint32_t id, xmlNode *request) rc = -EPROTONOSUPPORT; #endif do_reply = 1; - } else if (pcmk__str_eq(op, LRMD_OP_RSC_UNREG, pcmk__str_none)) { - if (allowed) { - rc = process_lrmd_rsc_unregister(client, id, request); - /* don't notify anyone about failed un-registers */ - if (rc == pcmk_ok || rc == -EINPROGRESS) { - do_notify = 1; - } - } else { - rc = -EACCES; - } - do_reply = 1; } else { rc = -EOPNOTSUPP; do_reply = 1; diff --git a/daemons/execd/execd_messages.c b/daemons/execd/execd_messages.c index a623fc90292..94ff7f461c5 100644 --- a/daemons/execd/execd_messages.c +++ b/daemons/execd/execd_messages.c @@ -311,10 +311,44 @@ handle_rsc_reg_request(pcmk__request_t *request) return reply; } +static xmlNode * +handle_rsc_unreg_request(pcmk__request_t *request) +{ + int call_id = 0; + int rc = pcmk_rc_ok; + bool allowed = pcmk_is_set(request->ipc_client->flags, + pcmk__client_privileged); + xmlNode *reply = NULL; + + if (!allowed) { + pcmk__set_result(&request->result, CRM_EX_INSUFFICIENT_PRIV, + PCMK_EXEC_ERROR, NULL); + crm_warn("Rejecting IPC request '%s' from unprivileged client %s", + request->op, pcmk__client_name(request->ipc_client)); + return NULL; + } + + crm_element_value_int(request->xml, PCMK__XA_LRMD_CALLID, &call_id); + + rc = execd_process_rsc_unregister(request->ipc_client, request->xml); + + /* Create a generic reply since unregistering a resource doesn't create + * a more specific one. + */ + reply = execd_create_reply(__func__, pcmk_rc2legacy(rc), call_id); + pcmk__set_result(&request->result, CRM_EX_OK, PCMK_EXEC_DONE, NULL); + return reply; +} + static bool -requires_notify(const char *command) +requires_notify(const char *command, int rc) { - return pcmk__str_any_of(command, LRMD_OP_POKE, LRMD_OP_RSC_REG, NULL); + if (pcmk__str_eq(command, LRMD_OP_RSC_UNREG, pcmk__str_none)) { + /* Don't notify about failed unregisters */ + return (rc == pcmk_ok) || (rc == -EINPROGRESS); + } else { + return pcmk__str_any_of(command, LRMD_OP_POKE, LRMD_OP_RSC_REG, NULL); + } } static void @@ -330,6 +364,7 @@ execd_register_handlers(void) { LRMD_OP_RSC_EXEC, handle_rsc_exec_request }, { LRMD_OP_RSC_INFO, handle_rsc_info_request }, { LRMD_OP_RSC_REG, handle_rsc_reg_request }, + { LRMD_OP_RSC_UNREG, handle_rsc_unreg_request }, { NULL, NULL }, }; @@ -534,17 +569,20 @@ execd_process_message(pcmk__client_t *c, uint32_t id, uint32_t flags, xmlNode *m } if (reply != NULL) { + int reply_rc = pcmk_ok; + rc = lrmd_server_send_reply(c, id, reply); if (rc != pcmk_rc_ok) { crm_warn("Reply to client %s failed: %s " QB_XS " rc=%d", pcmk__client_name(c), pcmk_rc_str(rc), rc); } - pcmk__xml_free(reply); - - if (requires_notify(request.op)) { - execd_send_generic_notify(pcmk_ok, request.xml); + crm_element_value_int(reply, PCMK__XA_LRMD_RC, &reply_rc); + if (requires_notify(request.op, reply_rc)) { + execd_send_generic_notify(reply_rc, request.xml); } + + pcmk__xml_free(reply); } reason = request.result.exit_reason; diff --git a/daemons/execd/pacemaker-execd.h b/daemons/execd/pacemaker-execd.h index b6d831e12fc..4fe029a7a34 100644 --- a/daemons/execd/pacemaker-execd.h +++ b/daemons/execd/pacemaker-execd.h @@ -118,6 +118,7 @@ int execd_process_get_rsc_info(xmlNode *request, int call_id, xmlNode **reply); int execd_process_rsc_cancel(pcmk__client_t *client, xmlNode *request); int execd_process_rsc_exec(pcmk__client_t *client, xmlNode *request); void execd_process_rsc_register(pcmk__client_t *client, uint32_t id, xmlNode *request); +int execd_process_rsc_unregister(pcmk__client_t *client, xmlNode *request); int execd_process_signon(pcmk__client_t *client, xmlNode *request, int call_id, xmlNode **reply); From c59059ba6ef3d429be97f5377316c11050a6234b Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 9 Jul 2025 15:22:06 -0400 Subject: [PATCH 16/20] Refactor: daemons: Use pcmk__request_t for LRMD_OP_IPC_FWD. --- daemons/execd/execd_commands.c | 27 ++++----------------------- daemons/execd/execd_messages.c | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c index 383aada7019..e51f19cbb07 100644 --- a/daemons/execd/execd_commands.c +++ b/daemons/execd/execd_commands.c @@ -1886,29 +1886,10 @@ process_lrmd_message(pcmk__client_t *client, uint32_t id, xmlNode *request) crm_trace("Processing %s operation from %s", op, client->id); crm_element_value_int(request, PCMK__XA_LRMD_CALLID, &call_id); - if (pcmk__str_eq(op, CRM_OP_IPC_FWD, pcmk__str_none)) { -#ifdef PCMK__COMPILE_REMOTE - /* Certain IPC commands may be done only by privileged users (i.e. root or - * hacluster), because they would otherwise provide a means of bypassing - * ACLs. - */ - bool allowed = pcmk_is_set(client->flags, pcmk__client_privileged); - - if (allowed) { - ipc_proxy_forward_client(client, request); - } else { - rc = -EACCES; - } -#else - rc = -EPROTONOSUPPORT; -#endif - do_reply = 1; - } else { - rc = -EOPNOTSUPP; - do_reply = 1; - crm_err("Unknown IPC request '%s' from client %s", - op, pcmk__client_name(client)); - } + rc = -EOPNOTSUPP; + do_reply = 1; + crm_err("Unknown IPC request '%s' from client %s", + op, pcmk__client_name(client)); if (rc == -EACCES) { crm_warn("Rejecting IPC request '%s' from unprivileged client %s", diff --git a/daemons/execd/execd_messages.c b/daemons/execd/execd_messages.c index 94ff7f461c5..93f9399d37d 100644 --- a/daemons/execd/execd_messages.c +++ b/daemons/execd/execd_messages.c @@ -36,6 +36,37 @@ static GHashTable *execd_handlers = NULL; static int lrmd_call_id = 0; +static xmlNode * +handle_ipc_fwd_request(pcmk__request_t *request) +{ + int call_id = 0; + int rc = pcmk_rc_ok; + xmlNode *reply = NULL; + +#ifdef PCMK__COMPILE_REMOTE + bool allowed = pcmk_is_set(request->ipc_client->flags, + pcmk__client_privileged); + + if (!allowed) { + pcmk__set_result(&request->result, CRM_EX_INSUFFICIENT_PRIV, + PCMK_EXEC_ERROR, NULL); + crm_warn("Rejecting IPC request '%s' from unprivileged client %s", + request->op, pcmk__client_name(request->ipc_client)); + return NULL; + } + + ipc_proxy_forward_client(request->ipc_client, request->xml); +#else + rc = EPROTONOSUPPORT; +#endif + + crm_element_value_int(request->xml, PCMK__XA_LRMD_CALLID, &call_id); + + /* Create a generic reply since forwarding doesn't create a more specific one */ + reply = execd_create_reply(__func__, pcmk_rc2legacy(rc), call_id); + return reply; +} + static xmlNode * handle_register_request(pcmk__request_t *request) { @@ -355,6 +386,7 @@ static void execd_register_handlers(void) { pcmk__server_command_t handlers[] = { + { CRM_OP_IPC_FWD, handle_ipc_fwd_request }, { CRM_OP_REGISTER, handle_register_request }, { LRMD_OP_ALERT_EXEC, handle_alert_exec_request }, { LRMD_OP_CHECK, handle_check_request }, From 7d56d4665c8ff7ed68dd571f87d9c55782dd6282 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Tue, 15 Jul 2025 09:02:52 -0400 Subject: [PATCH 17/20] Refactor: daemons: Add handle_unknown_request. Now that all messages are supported by the new code, we can add the unknown message handler to catch anything else. And that means everything is being handled by execd_process_message now, so process_lrmd_message can be removed. Fixes T126 --- daemons/execd/execd_commands.c | 45 --------------------------------- daemons/execd/execd_messages.c | 29 ++++++++++----------- daemons/execd/pacemaker-execd.h | 3 --- 3 files changed, 14 insertions(+), 63 deletions(-) diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c index e51f19cbb07..6e7ac7238ba 100644 --- a/daemons/execd/execd_commands.c +++ b/daemons/execd/execd_commands.c @@ -1872,48 +1872,3 @@ execd_process_get_recurring(xmlNode *request, int call_id, xmlNode **reply) return rc; } - -void -process_lrmd_message(pcmk__client_t *client, uint32_t id, xmlNode *request) -{ - int rc = pcmk_ok; - int call_id = 0; - const char *op = crm_element_value(request, PCMK__XA_LRMD_OP); - int do_reply = 0; - int do_notify = 0; - xmlNode *reply = NULL; - - crm_trace("Processing %s operation from %s", op, client->id); - crm_element_value_int(request, PCMK__XA_LRMD_CALLID, &call_id); - - rc = -EOPNOTSUPP; - do_reply = 1; - crm_err("Unknown IPC request '%s' from client %s", - op, pcmk__client_name(client)); - - if (rc == -EACCES) { - crm_warn("Rejecting IPC request '%s' from unprivileged client %s", - op, pcmk__client_name(client)); - } - - crm_debug("Processed %s operation from %s: rc=%d, reply=%d, notify=%d", - op, client->id, rc, do_reply, do_notify); - - if (do_reply) { - int send_rc = pcmk_rc_ok; - - if (reply == NULL) { - reply = execd_create_reply(__func__, rc, call_id); - } - send_rc = lrmd_server_send_reply(client, id, reply); - pcmk__xml_free(reply); - if (send_rc != pcmk_rc_ok) { - crm_warn("Reply to client %s failed: %s " QB_XS " rc=%d", - pcmk__client_name(client), pcmk_rc_str(send_rc), send_rc); - } - } - - if (do_notify) { - execd_send_generic_notify(rc, request); - } -} diff --git a/daemons/execd/execd_messages.c b/daemons/execd/execd_messages.c index 93f9399d37d..de914cebfdf 100644 --- a/daemons/execd/execd_messages.c +++ b/daemons/execd/execd_messages.c @@ -382,6 +382,19 @@ requires_notify(const char *command, int rc) } } + +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__format_result(&request->result, CRM_EX_PROTOCOL, PCMK_EXEC_INVALID, + "Unknown IPC request type '%s' (bug?)", + pcmk__s(request->op, "")); + return NULL; +} + static void execd_register_handlers(void) { @@ -397,7 +410,7 @@ execd_register_handlers(void) { LRMD_OP_RSC_INFO, handle_rsc_info_request }, { LRMD_OP_RSC_REG, handle_rsc_reg_request }, { LRMD_OP_RSC_UNREG, handle_rsc_unreg_request }, - { NULL, NULL }, + { NULL, handle_unknown_request }, }; execd_handlers = pcmk__register_handlers(handlers); @@ -586,20 +599,6 @@ execd_process_message(pcmk__client_t *c, uint32_t id, uint32_t flags, xmlNode *m reply = pcmk__process_request(&request, execd_handlers); - /* FIXME: THIS IS TEMPORARY - * - * If the above returns NULL (which could be because something bad happened, - * or because a message doesn't send a reply, but is most likely because not - * all messages have been implemented yet), try falling back to the older - * code. This means we don't have to implement everything before testing. - * Memory cleanup isn't important here. This will be removed. - */ - if (reply == NULL) { - crm_trace("Falling back to old function"); - process_lrmd_message(c, id, request.xml); - return; - } - if (reply != NULL) { int reply_rc = pcmk_ok; diff --git a/daemons/execd/pacemaker-execd.h b/daemons/execd/pacemaker-execd.h index 4fe029a7a34..6877ab0941e 100644 --- a/daemons/execd/pacemaker-execd.h +++ b/daemons/execd/pacemaker-execd.h @@ -64,9 +64,6 @@ int lrmd_server_send_notify(pcmk__client_t *client, xmlNode *msg); void notify_of_new_client(pcmk__client_t *new_client); -void process_lrmd_message(pcmk__client_t *client, uint32_t id, - xmlNode *request); - void free_rsc(gpointer data); void handle_shutdown_ack(void); From ba47d0b1a99fd19d3264951a5c070da13f5ebbe7 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 17 Jul 2025 11:03:07 -0400 Subject: [PATCH 18/20] Low: daemons: Correct result error string on unknown IPC messages. --- daemons/attrd/attrd_messages.c | 3 ++- daemons/fenced/fenced_commands.c | 3 ++- daemons/pacemakerd/pcmkd_messages.c | 2 +- daemons/schedulerd/schedulerd_messages.c | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/daemons/attrd/attrd_messages.c b/daemons/attrd/attrd_messages.c index 27cdadb6697..5cee2ffbd66 100644 --- a/daemons/attrd/attrd_messages.c +++ b/daemons/attrd/attrd_messages.c @@ -59,7 +59,8 @@ handle_unknown_request(pcmk__request_t *request) request->op, pcmk__request_origin_type(request), pcmk__request_origin(request)); pcmk__format_result(&request->result, CRM_EX_PROTOCOL, PCMK_EXEC_INVALID, - "Unknown request type '%s' (bug?)", request->op); + "Unknown request type '%s' (bug?)", + pcmk__s(request->op, "")); return NULL; } diff --git a/daemons/fenced/fenced_commands.c b/daemons/fenced/fenced_commands.c index 9934760ea5e..36e4c472d67 100644 --- a/daemons/fenced/fenced_commands.c +++ b/daemons/fenced/fenced_commands.c @@ -3485,7 +3485,8 @@ handle_unknown_request(pcmk__request_t *request) request->op, pcmk__request_origin_type(request), pcmk__request_origin(request)); pcmk__format_result(&request->result, CRM_EX_PROTOCOL, PCMK_EXEC_INVALID, - "Unknown IPC request type '%s' (bug?)", request->op); + "Unknown IPC request type '%s' (bug?)", + pcmk__s(request->op, "")); return fenced_construct_reply(request->xml, NULL, &request->result); } diff --git a/daemons/pacemakerd/pcmkd_messages.c b/daemons/pacemakerd/pcmkd_messages.c index 1f22720abb7..abcf134c745 100644 --- a/daemons/pacemakerd/pcmkd_messages.c +++ b/daemons/pacemakerd/pcmkd_messages.c @@ -149,7 +149,7 @@ handle_unknown_request(pcmk__request_t *request) pcmk__format_result(&request->result, CRM_EX_PROTOCOL, PCMK_EXEC_INVALID, "Unknown IPC request type '%s' (bug?)", - pcmk__client_name(request->ipc_client)); + pcmk__s(request->op, "")); return NULL; } diff --git a/daemons/schedulerd/schedulerd_messages.c b/daemons/schedulerd/schedulerd_messages.c index 1862b59f7fb..c5cc8ae4fca 100644 --- a/daemons/schedulerd/schedulerd_messages.c +++ b/daemons/schedulerd/schedulerd_messages.c @@ -182,7 +182,7 @@ handle_unknown_request(pcmk__request_t *request) pcmk__format_result(&request->result, CRM_EX_PROTOCOL, PCMK_EXEC_INVALID, "Unknown IPC request type '%s' (bug?)", - pcmk__client_name(request->ipc_client)); + pcmk__s(request->op, "")); return NULL; } From 5dc6e6c1399ddc76598a76a240886c0dc0f484c4 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 17 Jul 2025 11:31:21 -0400 Subject: [PATCH 19/20] Refactor: daemons: Add an execd_create_reply macro... and rename the existing function to be execd_create_reply_as. This gets rid of the need to pass __func__ as the first argument every time. --- daemons/execd/execd_commands.c | 8 ++++---- daemons/execd/execd_messages.c | 16 ++++++++-------- daemons/execd/pacemaker-execd.h | 5 ++++- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c index 6e7ac7238ba..eafd5f2d5d3 100644 --- a/daemons/execd/execd_commands.c +++ b/daemons/execd/execd_commands.c @@ -548,7 +548,7 @@ schedule_lrmd_cmd(lrmd_rsc_t * rsc, lrmd_cmd_t * cmd) } xmlNode * -execd_create_reply(const char *origin, int rc, int call_id) +execd_create_reply_as(const char *origin, int rc, int call_id) { xmlNode *reply = pcmk__xe_create(NULL, PCMK__XE_LRMD_REPLY); @@ -1564,7 +1564,7 @@ execd_process_signon(pcmk__client_t *client, xmlNode *request, int call_id, pcmk__assert(reply != NULL); - *reply = execd_create_reply(__func__, pcmk_rc2legacy(rc), call_id); + *reply = execd_create_reply(pcmk_rc2legacy(rc), call_id); crm_xml_add(*reply, PCMK__XA_LRMD_OP, CRM_OP_REGISTER); crm_xml_add(*reply, PCMK__XA_LRMD_CLIENTID, client->id); crm_xml_add(*reply, PCMK__XA_LRMD_PROTOCOL_VERSION, LRMD_PROTOCOL_VERSION); @@ -1619,7 +1619,7 @@ execd_process_get_rsc_info(xmlNode *request, int call_id, xmlNode **reply) CRM_LOG_ASSERT(reply != NULL); - *reply = execd_create_reply(__func__, pcmk_rc2legacy(rc), call_id); + *reply = execd_create_reply(pcmk_rc2legacy(rc), call_id); if (rsc) { crm_xml_add(*reply, PCMK__XA_LRMD_RSC_ID, rsc->rsc_id); crm_xml_add(*reply, PCMK__XA_LRMD_CLASS, rsc->class); @@ -1854,7 +1854,7 @@ execd_process_get_recurring(xmlNode *request, int call_id, xmlNode **reply) CRM_LOG_ASSERT(reply != NULL); - *reply = execd_create_reply(__func__, pcmk_rc2legacy(rc), call_id); + *reply = execd_create_reply(pcmk_rc2legacy(rc), call_id); // If resource ID is not specified, check all resources if (rsc_id == NULL) { diff --git a/daemons/execd/execd_messages.c b/daemons/execd/execd_messages.c index de914cebfdf..558123bbc47 100644 --- a/daemons/execd/execd_messages.c +++ b/daemons/execd/execd_messages.c @@ -63,7 +63,7 @@ handle_ipc_fwd_request(pcmk__request_t *request) crm_element_value_int(request->xml, PCMK__XA_LRMD_CALLID, &call_id); /* Create a generic reply since forwarding doesn't create a more specific one */ - reply = execd_create_reply(__func__, pcmk_rc2legacy(rc), call_id); + reply = execd_create_reply(pcmk_rc2legacy(rc), call_id); return reply; } @@ -118,7 +118,7 @@ handle_alert_exec_request(pcmk__request_t *request) /* Create a generic reply since executing an alert doesn't create a * more specific one. */ - reply = execd_create_reply(__func__, pcmk_rc2legacy(rc), call_id); + reply = execd_create_reply(pcmk_rc2legacy(rc), call_id); return reply; } @@ -200,7 +200,7 @@ handle_poke_request(pcmk__request_t *request) crm_element_value_int(request->xml, PCMK__XA_LRMD_CALLID, &call_id); /* Create a generic reply since this doesn't create a more specific one */ - reply = execd_create_reply(__func__, pcmk_ok, call_id); + reply = execd_create_reply(pcmk_ok, call_id); return reply; } @@ -235,7 +235,7 @@ handle_rsc_cancel_request(pcmk__request_t *request) /* Create a generic reply since canceling a resource doesn't create a * more specific one. */ - reply = execd_create_reply(__func__, pcmk_rc2legacy(rc), call_id); + reply = execd_create_reply(pcmk_rc2legacy(rc), call_id); return reply; } @@ -269,11 +269,11 @@ handle_rsc_exec_request(pcmk__request_t *request) * this and use it as its return value, which passes back up to the * public API function lrmd_api_exec. */ - reply = execd_create_reply(__func__, call_id, call_id); + reply = execd_create_reply(call_id, call_id); } else { pcmk__set_result(&request->result, pcmk_rc2exitc(rc), PCMK_EXEC_ERROR, pcmk_rc_str(rc)); - reply = execd_create_reply(__func__, pcmk_rc2legacy(rc), call_id); + reply = execd_create_reply(pcmk_rc2legacy(rc), call_id); } return reply; @@ -337,7 +337,7 @@ handle_rsc_reg_request(pcmk__request_t *request) /* Create a generic reply since registering a resource doesn't create * a more specific one. */ - reply = execd_create_reply(__func__, pcmk_ok, call_id); + reply = execd_create_reply(pcmk_ok, call_id); pcmk__set_result(&request->result, CRM_EX_OK, PCMK_EXEC_DONE, NULL); return reply; } @@ -366,7 +366,7 @@ handle_rsc_unreg_request(pcmk__request_t *request) /* Create a generic reply since unregistering a resource doesn't create * a more specific one. */ - reply = execd_create_reply(__func__, pcmk_rc2legacy(rc), call_id); + reply = execd_create_reply(pcmk_rc2legacy(rc), call_id); pcmk__set_result(&request->result, CRM_EX_OK, PCMK_EXEC_DONE, NULL); return reply; } diff --git a/daemons/execd/pacemaker-execd.h b/daemons/execd/pacemaker-execd.h index 6877ab0941e..d9d02d8807b 100644 --- a/daemons/execd/pacemaker-execd.h +++ b/daemons/execd/pacemaker-execd.h @@ -106,9 +106,12 @@ void lrmd_drain_alerts(GMainLoop *mloop); void execd_process_message(pcmk__client_t *c, uint32_t id, uint32_t flags, xmlNode *msg); -xmlNode *execd_create_reply(const char *origin, int rc, int call_id); +xmlNode *execd_create_reply_as(const char *origin, int rc, int call_id); void execd_send_generic_notify(int rc, xmlNode *request); +#define execd_create_reply(rc, call_id) \ + execd_create_reply_as(__func__, (rc), (call_id)) + int execd_process_alert_exec(pcmk__client_t *client, xmlNode *request); int execd_process_get_recurring(xmlNode *request, int call_id, xmlNode **reply); int execd_process_get_rsc_info(xmlNode *request, int call_id, xmlNode **reply); From 34d76196bc8d95a4e11a205a005fb9b4b44c57e0 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 21 Jul 2025 14:29:08 -0400 Subject: [PATCH 20/20] Low: daemons: Return CRM_EX_PROTOCOL when ACKing an unknown request. Only a couple daemons send an ACK on an unknown request. Those that do send CRM_EX_INVALID_PARAM, which doesn't quite seem like the right value. I guess I can see what it's going for, but CRM_EX_PROTOCOL seems like a better fit. --- daemons/pacemakerd/pcmkd_messages.c | 2 +- daemons/schedulerd/schedulerd_messages.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/daemons/pacemakerd/pcmkd_messages.c b/daemons/pacemakerd/pcmkd_messages.c index abcf134c745..21ebb7d8b31 100644 --- a/daemons/pacemakerd/pcmkd_messages.c +++ b/daemons/pacemakerd/pcmkd_messages.c @@ -145,7 +145,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_INVALID_PARAM); + PCMK__XE_ACK, NULL, CRM_EX_PROTOCOL); pcmk__format_result(&request->result, CRM_EX_PROTOCOL, PCMK_EXEC_INVALID, "Unknown IPC request type '%s' (bug?)", diff --git a/daemons/schedulerd/schedulerd_messages.c b/daemons/schedulerd/schedulerd_messages.c index c5cc8ae4fca..fd17e067fe1 100644 --- a/daemons/schedulerd/schedulerd_messages.c +++ b/daemons/schedulerd/schedulerd_messages.c @@ -178,7 +178,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_INVALID_PARAM); + PCMK__XE_ACK, NULL, CRM_EX_PROTOCOL); pcmk__format_result(&request->result, CRM_EX_PROTOCOL, PCMK_EXEC_INVALID, "Unknown IPC request type '%s' (bug?)",