From bb20b4c31ef2496c3648bb4fc490edd1466b873b Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 15 Jul 2026 11:19:45 -0400 Subject: [PATCH 01/60] Refactor: daemons: Rearrange the big cleanup block at the end of attrd. * Protect everything in the block against being given a NULL, which allows getting rid of the initialized variable. * Rearrange things so similar functions are next to each other. --- daemons/attrd/attrd_cib.c | 5 ++++- daemons/attrd/pacemaker-attrd.c | 29 ++++++++++------------------- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/daemons/attrd/attrd_cib.c b/daemons/attrd/attrd_cib.c index e33d525fd41..4c684cb435c 100644 --- a/daemons/attrd/attrd_cib.c +++ b/daemons/attrd/attrd_cib.c @@ -154,7 +154,10 @@ attrd_cib_connect(int max_retry) void attrd_cib_disconnect(void) { - CRM_CHECK(the_cib != NULL, return); + if (the_cib == NULL) { + return; + } + the_cib->cmds->del_notify_callback(the_cib, PCMK__VALUE_CIB_DIFF_NOTIFY, attrd_cib_updated_cb); cib__clean_up_connection(&the_cib); diff --git a/daemons/attrd/pacemaker-attrd.c b/daemons/attrd/pacemaker-attrd.c index 1682ba8ba56..d2223605745 100644 --- a/daemons/attrd/pacemaker-attrd.c +++ b/daemons/attrd/pacemaker-attrd.c @@ -108,7 +108,6 @@ main(int argc, char **argv) int rc = pcmk_rc_ok; GError *error = NULL; - bool initialized = false; GOptionGroup *output_group = NULL; pcmk__common_args_t *args = pcmk__new_common_args(SUMMARY); @@ -155,8 +154,6 @@ main(int argc, char **argv) goto done; } - initialized = true; - attributes = pcmk__strkey_table(NULL, attrd_free_attribute); /* Connect to the CIB before connecting to the cluster or listening for IPC. @@ -202,28 +199,22 @@ main(int argc, char **argv) attrd_run_mainloop(); done: - if (initialized) { - pcmk__info("Shutting down attribute manager"); - - attrd_ipc_cleanup(); - attrd_lrmd_disconnect(); + pcmk__info("Shutting down attribute manager"); - if (!attrd_stand_alone()) { - attrd_cib_disconnect(); - } - - attrd_free_removed_peers(); - attrd_free_waitlist(); - attrd_cluster_disconnect(); - attrd_unregister_handlers(); - g_hash_table_destroy(attributes); - } + attrd_ipc_cleanup(); + attrd_lrmd_disconnect(); + attrd_unregister_handlers(); + attrd_cib_disconnect(); + attrd_cluster_disconnect(); + attrd_free_removed_peers(); + attrd_free_waitlist(); attrd_cleanup_xml_ids(); + g_clear_pointer(&attributes, g_hash_table_destroy); + g_strfreev(processed_args); pcmk__free_arg_context(context); - g_strfreev(log_files); pcmk__output_and_clear_error(&error, out); From 12d7c5d4176fbb489e0fa0ae4701c9e9165b8cf1 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 15 Jul 2026 14:04:56 -0400 Subject: [PATCH 02/60] Refactor: daemons: Add an atexit handler for certain cleanup in attrd. We traditionally haven't used atexit in pacemaker, but I don't think there's a good reason for that and it can definitely be handy. Here, I'm using it to clean up memory allocated in command line processing which I think is a great use and should be duplicated throughout. There's a couple rules I want to set down for its use: * For now, since we're not sure if there's any real downsides to this, it should only be used for cleaning up command line processing. This includes anything allocated as the arg_data member of a GOptionEntry struct. It's possible for this stuff to escape getting cleaned up - for example, if `tool --help` is called because glib argument processing calls exit() after printing the help (see T1019). * It's not possible to use formatted output in the atexit handler because that will have been freed already. It's also not possible to do anything with a mainloop, logging, or XML because crm_exit -> pcmk_common_cleanup will have been called. * Because there's no explicit call to the atexit handler (it's registered, and then just happens eventually), we need to keep it simple to prevent confusion. Additionally, if multiple atexit handlers are registered, they are called in the reverse order from registration which could add to the confusion. One additional reason I want to do this in the daemons in particular is that it's possible for there to be multiple exit paths in the daemon code (for instance, attrd_cib_destroy_cb -> attrd_shutdown calls crm_exit) and without the atexit handler, this memory will never be freed. --- daemons/attrd/pacemaker-attrd.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/daemons/attrd/pacemaker-attrd.c b/daemons/attrd/pacemaker-attrd.c index d2223605745..610052e5032 100644 --- a/daemons/attrd/pacemaker-attrd.c +++ b/daemons/attrd/pacemaker-attrd.c @@ -31,7 +31,10 @@ #define SUMMARY "daemon for managing Pacemaker node attributes" static gboolean stand_alone = false; -gchar **log_files = NULL; + +static gchar **log_files = NULL; +static gchar **processed_args = NULL; +static GOptionContext *context = NULL; static GOptionEntry entries[] = { { "stand-alone", 's', G_OPTION_FLAG_NONE, G_OPTION_ARG_NONE, &stand_alone, @@ -102,17 +105,28 @@ build_arg_context(pcmk__common_args_t *args, GOptionGroup **group) { return context; } +static void +attrd_cleanup_cmdline(void) +{ + g_clear_pointer(&processed_args, g_strfreev); + g_clear_pointer(&context, g_option_context_free); + g_clear_pointer(&log_files, g_strfreev); +} + int main(int argc, char **argv) { int rc = pcmk_rc_ok; GError *error = NULL; - GOptionGroup *output_group = NULL; - pcmk__common_args_t *args = pcmk__new_common_args(SUMMARY); - gchar **processed_args = pcmk__cmdline_preproc(argv, NULL); - GOptionContext *context = build_arg_context(args, &output_group); + pcmk__common_args_t *args = NULL; + + atexit(attrd_cleanup_cmdline); + + args = pcmk__new_common_args(SUMMARY); + processed_args = pcmk__cmdline_preproc(argv, NULL); + context = build_arg_context(args, &output_group); attrd_init_mainloop(); crm_log_preinit(NULL, argc, argv); @@ -213,10 +227,6 @@ main(int argc, char **argv) g_clear_pointer(&attributes, g_hash_table_destroy); - g_strfreev(processed_args); - pcmk__free_arg_context(context); - g_strfreev(log_files); - pcmk__output_and_clear_error(&error, out); if (out != NULL) { From 87ed6eb39cfe24ffd78e2a02199691d30a690445 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 22 Jul 2026 14:35:41 -0400 Subject: [PATCH 03/60] Refactor: daemons: Move cleanup out of attrd_shutdown. We can't clean anything up until after the main loop has exited, since there might still be pending sources to be dispatched. --- daemons/attrd/attrd_utils.c | 5 ----- daemons/attrd/pacemaker-attrd.c | 2 ++ 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/daemons/attrd/attrd_utils.c b/daemons/attrd/attrd_utils.c index 3025f197e76..d34b09a5f7f 100644 --- a/daemons/attrd/attrd_utils.c +++ b/daemons/attrd/attrd_utils.c @@ -65,11 +65,6 @@ attrd_shutdown(int nsig) mainloop_destroy_signal(SIGUSR2); mainloop_destroy_signal(SIGTRAP); - attrd_free_waitlist(); - attrd_free_confirmations(); - - g_clear_pointer(&peer_protocol_vers, g_hash_table_destroy); - if ((mloop == NULL) || !g_main_loop_is_running(mloop)) { /* If there's no main loop active, just exit. This should be possible * only if we get SIGTERM in brief windows at start-up and shutdown. diff --git a/daemons/attrd/pacemaker-attrd.c b/daemons/attrd/pacemaker-attrd.c index 610052e5032..7429dea0fba 100644 --- a/daemons/attrd/pacemaker-attrd.c +++ b/daemons/attrd/pacemaker-attrd.c @@ -223,9 +223,11 @@ main(int argc, char **argv) attrd_free_removed_peers(); attrd_free_waitlist(); + attrd_free_confirmations(); attrd_cleanup_xml_ids(); g_clear_pointer(&attributes, g_hash_table_destroy); + g_clear_pointer(&peer_protocol_vers, g_hash_table_destroy); pcmk__output_and_clear_error(&error, out); From f2f4120d4a3f6c6bc2d6e6309638d05217101ed9 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 22 Jul 2026 14:37:00 -0400 Subject: [PATCH 04/60] Refactor: daemons: Move attrd cleanup into its own function. For the moment, this is just for organizational purposes. Once I introduce the pcmk__daemon_t object, this may become a function on that object instead. I haven't decided yet. --- daemons/attrd/pacemaker-attrd.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/daemons/attrd/pacemaker-attrd.c b/daemons/attrd/pacemaker-attrd.c index 7429dea0fba..9947e918938 100644 --- a/daemons/attrd/pacemaker-attrd.c +++ b/daemons/attrd/pacemaker-attrd.c @@ -113,6 +113,24 @@ attrd_cleanup_cmdline(void) g_clear_pointer(&log_files, g_strfreev); } +static void +attrd_cleanup(void) +{ + attrd_ipc_cleanup(); + attrd_lrmd_disconnect(); + attrd_unregister_handlers(); + attrd_cib_disconnect(); + attrd_cluster_disconnect(); + + attrd_free_removed_peers(); + attrd_free_waitlist(); + attrd_free_confirmations(); + attrd_cleanup_xml_ids(); + + g_clear_pointer(&attributes, g_hash_table_destroy); + g_clear_pointer(&peer_protocol_vers, g_hash_table_destroy); +} + int main(int argc, char **argv) { @@ -215,19 +233,7 @@ main(int argc, char **argv) done: pcmk__info("Shutting down attribute manager"); - attrd_ipc_cleanup(); - attrd_lrmd_disconnect(); - attrd_unregister_handlers(); - attrd_cib_disconnect(); - attrd_cluster_disconnect(); - - attrd_free_removed_peers(); - attrd_free_waitlist(); - attrd_free_confirmations(); - attrd_cleanup_xml_ids(); - - g_clear_pointer(&attributes, g_hash_table_destroy); - g_clear_pointer(&peer_protocol_vers, g_hash_table_destroy); + attrd_cleanup(); pcmk__output_and_clear_error(&error, out); From adb91c118ab436930a608881cb4eb3463d6022d7 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 22 Jul 2026 14:45:43 -0400 Subject: [PATCH 05/60] Refactor: daemons: Split up attrd_shutdown and attrd_quit_main_loop. attrd_quit_main_loop just signals that the main loop should be shut down after its current iteration, and then sets the exit code. It can be called from anywhere that was previously calling attrd_shutdown + crm_exit. In particular, attrd_cib_destroy_cb and attrd_cpg_destroy can use this new function instead. Despite how it may initially seem, these functions are main loop sources, so calling attrd_quit_main_loop in them will cause us to return back to the done label in main() once all previously dispatched sources are finished. attrd_shutdown is essentially just a wrapper at this point to provide a function with the correct type to pass to mainloop_add_signal. This means that - aside from pcmk__serve_attrd_ipc - there should be no other places in attrd that directly call crm_exit. All exit paths flow through terminating the main loop, to the done label in main(), and through the cleanup functions there. There are, of course, plenty of places in the pacemaker libraries that call crm_exit, but that's a much bigger project. --- daemons/attrd/attrd_cib.c | 11 ++++---- daemons/attrd/attrd_corosync.c | 9 +++---- daemons/attrd/attrd_utils.c | 48 +++++++++++++++++++++------------ daemons/attrd/pacemaker-attrd.c | 2 -- daemons/attrd/pacemaker-attrd.h | 1 + 5 files changed, 41 insertions(+), 30 deletions(-) diff --git a/daemons/attrd/attrd_cib.c b/daemons/attrd/attrd_cib.c index 4c684cb435c..d983b6e8607 100644 --- a/daemons/attrd/attrd_cib.c +++ b/daemons/attrd/attrd_cib.c @@ -35,13 +35,12 @@ attrd_cib_destroy_cb(void *user_data) if (attrd_shutting_down()) { pcmk__info("Disconnected from the CIB manager"); - - } else { - // @TODO This should trigger a reconnect, not a shutdown - pcmk__crit("Lost connection to the CIB manager, shutting down"); - attrd_exit_status = CRM_EX_DISCONNECT; - attrd_shutdown(0); + return; } + + // @TODO This should trigger a reconnect, not a shutdown + pcmk__crit("Lost connection to the CIB manager, shutting down"); + attrd_quit_main_loop(CRM_EX_DISCONNECT); } static void diff --git a/daemons/attrd/attrd_corosync.c b/daemons/attrd/attrd_corosync.c index 20519e0911d..78f147a3358 100644 --- a/daemons/attrd/attrd_corosync.c +++ b/daemons/attrd/attrd_corosync.c @@ -181,12 +181,11 @@ attrd_cpg_destroy(void *unused) { if (attrd_shutting_down()) { pcmk__info("Disconnected from Corosync process group"); - - } else { - pcmk__crit("Lost connection to Corosync process group, shutting down"); - attrd_exit_status = CRM_EX_DISCONNECT; - attrd_shutdown(0); + return; } + + pcmk__crit("Lost connection to Corosync process group, shutting down"); + attrd_quit_main_loop(CRM_EX_DISCONNECT); } #endif // SUPPORT_COROSYNC diff --git a/daemons/attrd/attrd_utils.c b/daemons/attrd/attrd_utils.c index d34b09a5f7f..67cc9f6ee09 100644 --- a/daemons/attrd/attrd_utils.c +++ b/daemons/attrd/attrd_utils.c @@ -45,18 +45,20 @@ attrd_shutting_down(void) return shutting_down; } -/*! - * \internal - * \brief Exit (using mainloop or not, as appropriate) - * - * \param[in] nsig Ignored - */ void -attrd_shutdown(int nsig) +attrd_quit_main_loop(crm_exit_t ec) { - // Tell various functions not to do anthing + if (attrd_shutting_down()) { + return; + } + + pcmk__info("Shutting down attribute manager"); + + // Tell various functions not to do anything shutting_down = true; + attrd_exit_status = ec; + // Don't respond to signals while shutting down mainloop_destroy_signal(SIGTERM); mainloop_destroy_signal(SIGCHLD); @@ -65,15 +67,27 @@ attrd_shutdown(int nsig) mainloop_destroy_signal(SIGUSR2); mainloop_destroy_signal(SIGTRAP); - if ((mloop == NULL) || !g_main_loop_is_running(mloop)) { - /* If there's no main loop active, just exit. This should be possible - * only if we get SIGTERM in brief windows at start-up and shutdown. - */ - crm_exit(CRM_EX_OK); - } else { - g_main_loop_quit(mloop); - g_main_loop_unref(mloop); - } + /* There's no way to get to this function without the main loop running, + * but check just in case someone adds one in the future + */ + CRM_CHECK((mloop != NULL) && g_main_loop_is_running(mloop), return); + + g_main_loop_quit(mloop); + g_main_loop_unref(mloop); +} + +/*! + * \internal + * \brief Quit the main loop and set the exit code to \c CRM_EX_OK + * + * \param[in] nsig Ignored + * + * \note This is a main loop signal handler function. + */ +void +attrd_shutdown(int nsig) +{ + attrd_quit_main_loop(CRM_EX_OK); } /*! diff --git a/daemons/attrd/pacemaker-attrd.c b/daemons/attrd/pacemaker-attrd.c index 9947e918938..f0cec4e0625 100644 --- a/daemons/attrd/pacemaker-attrd.c +++ b/daemons/attrd/pacemaker-attrd.c @@ -231,8 +231,6 @@ main(int argc, char **argv) attrd_run_mainloop(); done: - pcmk__info("Shutting down attribute manager"); - attrd_cleanup(); pcmk__output_and_clear_error(&error, out); diff --git a/daemons/attrd/pacemaker-attrd.h b/daemons/attrd/pacemaker-attrd.h index 8d9b50ad408..5d70042d211 100644 --- a/daemons/attrd/pacemaker-attrd.h +++ b/daemons/attrd/pacemaker-attrd.h @@ -62,6 +62,7 @@ void attrd_run_mainloop(void); void attrd_free_waitlist(void); void attrd_shutdown(int nsig); +void attrd_quit_main_loop(crm_exit_t ec); bool attrd_shutting_down(void); bool attrd_stand_alone(void); void attrd_ipc_init(void); From d4f526f866119b6f3b624547f8e02b5cadf84110 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 22 Jul 2026 14:51:26 -0400 Subject: [PATCH 06/60] Refactor: daemons: Make attrd_shutdown static. It's not used outside of the main file anymore. --- daemons/attrd/attrd_utils.c | 14 -------------- daemons/attrd/pacemaker-attrd.c | 14 ++++++++++++++ daemons/attrd/pacemaker-attrd.h | 1 - 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/daemons/attrd/attrd_utils.c b/daemons/attrd/attrd_utils.c index 67cc9f6ee09..338303c063a 100644 --- a/daemons/attrd/attrd_utils.c +++ b/daemons/attrd/attrd_utils.c @@ -76,20 +76,6 @@ attrd_quit_main_loop(crm_exit_t ec) g_main_loop_unref(mloop); } -/*! - * \internal - * \brief Quit the main loop and set the exit code to \c CRM_EX_OK - * - * \param[in] nsig Ignored - * - * \note This is a main loop signal handler function. - */ -void -attrd_shutdown(int nsig) -{ - attrd_quit_main_loop(CRM_EX_OK); -} - /*! * \internal * \brief Create a main loop for attrd diff --git a/daemons/attrd/pacemaker-attrd.c b/daemons/attrd/pacemaker-attrd.c index f0cec4e0625..6090b52f5b3 100644 --- a/daemons/attrd/pacemaker-attrd.c +++ b/daemons/attrd/pacemaker-attrd.c @@ -131,6 +131,20 @@ attrd_cleanup(void) g_clear_pointer(&peer_protocol_vers, g_hash_table_destroy); } +/*! + * \internal + * \brief Quit the main loop and set the exit code to \c CRM_EX_OK + * + * \param[in] nsig Ignored + * + * \note This is a main loop signal handler function. + */ +static void +attrd_shutdown(int nsig) +{ + attrd_quit_main_loop(CRM_EX_OK); +} + int main(int argc, char **argv) { diff --git a/daemons/attrd/pacemaker-attrd.h b/daemons/attrd/pacemaker-attrd.h index 5d70042d211..f3ed6ce7d43 100644 --- a/daemons/attrd/pacemaker-attrd.h +++ b/daemons/attrd/pacemaker-attrd.h @@ -61,7 +61,6 @@ void attrd_init_mainloop(void); void attrd_run_mainloop(void); void attrd_free_waitlist(void); -void attrd_shutdown(int nsig); void attrd_quit_main_loop(crm_exit_t ec); bool attrd_shutting_down(void); bool attrd_stand_alone(void); From dbf71079429cc4d6df86f799677aaf540ab86821 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 22 Jul 2026 13:25:38 -0400 Subject: [PATCH 07/60] Refactor: libcrmcommon,daemons: Introduce the pcmk__daemon_t object. This will eventually become an object that controls all the mainloop, IPC, and CPG aspects of a daemon. For the moment, all it does is handle the mainloop of the attribute daemon and not much of that either. The shutdown side is going to take some more work. Not all daemons are ready to be converted to use this object. At the moment, it's limited to attrd, execd, fenced, pacemakerd, and schedulerd. I think I'm going to take the approach of converting each daemon to use the mainloop support before adding anything else to the object. One thing to note about this is that I'm using pcmk_ipc_server as the identifier for the server type. We don't have any other server type enum that I can find, and all servers connect to IPC so this is probably good enough. This is all internal API, so it's easy to change this type later. --- daemons/attrd/attrd_utils.c | 29 +------------ daemons/attrd/pacemaker-attrd.c | 22 +++++++--- daemons/attrd/pacemaker-attrd.h | 4 +- include/crm/common/daemon_internal.h | 47 +++++++++++++++++++++ include/crm/common/internal.h | 1 + lib/common/Makefile.am | 1 + lib/common/daemon.c | 63 ++++++++++++++++++++++++++++ 7 files changed, 131 insertions(+), 36 deletions(-) create mode 100644 include/crm/common/daemon_internal.h create mode 100644 lib/common/daemon.c diff --git a/daemons/attrd/attrd_utils.c b/daemons/attrd/attrd_utils.c index 338303c063a..88979e66e50 100644 --- a/daemons/attrd/attrd_utils.c +++ b/daemons/attrd/attrd_utils.c @@ -25,7 +25,6 @@ cib_t *the_cib = NULL; static bool shutting_down = false; -static GMainLoop *mloop = NULL; /* A hash table storing information on the protocol version of each peer attrd. * The key is the peer's uname, and the value is the protocol version number. @@ -67,33 +66,7 @@ attrd_quit_main_loop(crm_exit_t ec) mainloop_destroy_signal(SIGUSR2); mainloop_destroy_signal(SIGTRAP); - /* There's no way to get to this function without the main loop running, - * but check just in case someone adds one in the future - */ - CRM_CHECK((mloop != NULL) && g_main_loop_is_running(mloop), return); - - g_main_loop_quit(mloop); - g_main_loop_unref(mloop); -} - -/*! - * \internal - * \brief Create a main loop for attrd - */ -void -attrd_init_mainloop(void) -{ - mloop = g_main_loop_new(NULL, FALSE); -} - -/*! - * \internal - * \brief Run attrd main loop - */ -void -attrd_run_mainloop(void) -{ - g_main_loop_run(mloop); + pcmk__daemon_quit(&attrd); } /* strlen("value") */ diff --git a/daemons/attrd/pacemaker-attrd.c b/daemons/attrd/pacemaker-attrd.c index 6090b52f5b3..da2fa10a0cd 100644 --- a/daemons/attrd/pacemaker-attrd.c +++ b/daemons/attrd/pacemaker-attrd.c @@ -30,6 +30,10 @@ #define SUMMARY "daemon for managing Pacemaker node attributes" +pcmk__daemon_t attrd = { + .type = pcmk_ipc_attrd, +}; + static gboolean stand_alone = false; static gchar **log_files = NULL; @@ -160,9 +164,7 @@ main(int argc, char **argv) processed_args = pcmk__cmdline_preproc(argv, NULL); context = build_arg_context(args, &output_group); - attrd_init_mainloop(); crm_log_preinit(NULL, argc, argv); - mainloop_add_signal(SIGTERM, attrd_shutdown); pcmk__register_formats(output_group, formats); if (!g_option_context_parse_strv(context, &processed_args, &error)) { @@ -240,9 +242,19 @@ main(int argc, char **argv) attrd_send_protocol(NULL); attrd_ipc_init(); - pcmk__notice("Pacemaker node attribute manager successfully started and " - "accepting connections"); - attrd_run_mainloop(); + + rc = pcmk__daemon_init(&attrd); + if (rc != pcmk_rc_ok) { + attrd_exit_status = CRM_EX_ERROR; + g_set_error(&error, PCMK__EXITC_ERROR, attrd_exit_status, + "Error initializing daemon object: %s", + pcmk_rc_str(rc)); + goto done; + } + + mainloop_add_signal(SIGTERM, attrd_shutdown); + + pcmk__daemon_run(&attrd); done: attrd_cleanup(); diff --git a/daemons/attrd/pacemaker-attrd.h b/daemons/attrd/pacemaker-attrd.h index f3ed6ce7d43..d4b10f78285 100644 --- a/daemons/attrd/pacemaker-attrd.h +++ b/daemons/attrd/pacemaker-attrd.h @@ -57,9 +57,6 @@ pcmk__ipc_send_ack((client), (id), (flags), ATTRD_PROTOCOL_VERSION, \ CRM_EX_INDETERMINATE) -void attrd_init_mainloop(void); -void attrd_run_mainloop(void); - void attrd_free_waitlist(void); void attrd_quit_main_loop(crm_exit_t ec); bool attrd_shutting_down(void); @@ -188,6 +185,7 @@ typedef struct { extern pcmk_cluster_t *attrd_cluster; extern GHashTable *attributes; extern GHashTable *peer_protocol_vers; +extern pcmk__daemon_t attrd; #define CIB_OP_TIMEOUT_S 120 diff --git a/include/crm/common/daemon_internal.h b/include/crm/common/daemon_internal.h new file mode 100644 index 00000000000..7029eb110aa --- /dev/null +++ b/include/crm/common/daemon_internal.h @@ -0,0 +1,47 @@ +/* + * Copyright 2026 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. + */ + +#ifndef PCMK__INCLUDED_CRM_COMMON_INTERNAL_H +#error "Include instead of directly" +#endif + +#ifndef PCMK__CRM_COMMON_DAEMON_INTERNAL__H +#define PCMK__CRM_COMMON_DAEMON_INTERNAL__H + +#include // GMainLoop + +#include // pcmk_ipc_server + +#ifdef __cplusplus +extern "C" { +#endif + +/*! + * \internal + * \brief This structure describes and manages a single pacemaker daemon + */ +typedef struct { + //! Daemon type, indexed by the IPC enum + enum pcmk_ipc_server type; + + //! Main loop + GMainLoop *mainloop; +} pcmk__daemon_t; + +// Mainloop management functions + +int pcmk__daemon_init(pcmk__daemon_t *srv); +void pcmk__daemon_quit(pcmk__daemon_t *srv); +void pcmk__daemon_run(pcmk__daemon_t *srv); + +#ifdef __cplusplus +} +#endif + +#endif // PCMK__CRM_COMMON_DAEMON_INTERNAL__H diff --git a/include/crm/common/internal.h b/include/crm/common/internal.h index 88bdac5d242..b3b560f88ec 100644 --- a/include/crm/common/internal.h +++ b/include/crm/common/internal.h @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include diff --git a/lib/common/Makefile.am b/lib/common/Makefile.am index 2292aef0d1b..290a57db638 100644 --- a/lib/common/Makefile.am +++ b/lib/common/Makefile.am @@ -56,6 +56,7 @@ if BUILD_CIBSECRETS libcrmcommon_la_SOURCES += cib_secrets.c endif libcrmcommon_la_SOURCES += cmdline.c +libcrmcommon_la_SOURCES += daemon.c libcrmcommon_la_SOURCES += digest.c libcrmcommon_la_SOURCES += health.c libcrmcommon_la_SOURCES += io.c diff --git a/lib/common/daemon.c b/lib/common/daemon.c new file mode 100644 index 00000000000..5216d49ce4d --- /dev/null +++ b/lib/common/daemon.c @@ -0,0 +1,63 @@ +/* + * Copyright 2026 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 // false +#include // NULL + +#include // g_clear_pointer, g_main_loop_* + +#include // CRM_CHECK +#include // CRM_EX_*, crm_exit, pcmk_rc_* + +/*! + * \internal + * \brief Initialize a previously allocated daemon object + * + * \param[in,out] srv The daemon object + * + * \return Standard Pacemaker return code + */ +int +pcmk__daemon_init(pcmk__daemon_t *srv) +{ + srv->mainloop = g_main_loop_new(NULL, false); + return pcmk_rc_ok; +} + +/*! + * \internal + * \brief Quit the daemon's main loop + * + * \param[in,out] srv The daemon object + */ +void +pcmk__daemon_quit(pcmk__daemon_t *srv) +{ + CRM_CHECK((srv->mainloop != NULL) && g_main_loop_is_running(srv->mainloop), + return); + + g_main_loop_quit(srv->mainloop); +} + +/*! + * \internal + * \brief Run a daemon + * + * \param[in,out] srv The daemon object + */ +void +pcmk__daemon_run(pcmk__daemon_t *srv) +{ + pcmk__notice("Pacemaker %s successfully started and accepting connections", + pcmk__server_log_name(srv->type)); + g_main_loop_run(srv->mainloop); + g_clear_pointer(&srv->mainloop, g_main_loop_unref); +} From a5c50733d53d8a8b9c845d8027a82ea2df926133 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 22 Jul 2026 13:33:29 -0400 Subject: [PATCH 08/60] Refactor: daemons: Move shutting_down into pcmk__daemon_t. --- daemons/attrd/attrd_cib.c | 6 +++--- daemons/attrd/attrd_corosync.c | 4 ++-- daemons/attrd/attrd_elections.c | 4 ++-- daemons/attrd/attrd_ipc.c | 2 +- daemons/attrd/attrd_utils.c | 19 ++----------------- daemons/attrd/pacemaker-attrd.c | 7 +++++++ daemons/attrd/pacemaker-attrd.h | 1 - include/crm/common/daemon_internal.h | 5 +++++ 8 files changed, 22 insertions(+), 26 deletions(-) diff --git a/daemons/attrd/attrd_cib.c b/daemons/attrd/attrd_cib.c index d983b6e8607..c97325ccbe6 100644 --- a/daemons/attrd/attrd_cib.c +++ b/daemons/attrd/attrd_cib.c @@ -33,7 +33,7 @@ attrd_cib_destroy_cb(void *user_data) cib->cmds->signoff(cib); - if (attrd_shutting_down()) { + if (attrd.shutting_down) { pcmk__info("Disconnected from the CIB manager"); return; } @@ -55,7 +55,7 @@ attrd_cib_updated_cb(const char *event, xmlNode *msg) } if (pcmk__cib_element_in_patchset(patchset, PCMK_XE_ALERTS)) { - if (attrd_shutting_down()) { + if (attrd.shutting_down) { pcmk__debug("Ignoring alerts change in CIB during shutdown"); } else { mainloop_set_trigger(attrd_config_read); @@ -80,7 +80,7 @@ attrd_cib_updated_cb(const char *event, xmlNode *msg) if (status_changed || pcmk__cib_element_in_patchset(patchset, PCMK_XE_NODES)) { - if (attrd_shutting_down()) { + if (attrd.shutting_down) { pcmk__debug("Ignoring node change in CIB during shutdown"); return; } diff --git a/daemons/attrd/attrd_corosync.c b/daemons/attrd/attrd_corosync.c index 78f147a3358..8c1f42b5a04 100644 --- a/daemons/attrd/attrd_corosync.c +++ b/daemons/attrd/attrd_corosync.c @@ -82,7 +82,7 @@ attrd_peer_message(pcmk__node_status_t *peer, xmlNode *xml) return; } - if (attrd_shutting_down()) { + if (attrd.shutting_down) { /* If we're shutting down, we want to continue responding to election * ops as long as we're a cluster member (because our vote may be * needed). Ignore all other messages. @@ -179,7 +179,7 @@ attrd_cpg_dispatch(cpg_handle_t handle, const struct cpg_name *group_name, static void attrd_cpg_destroy(void *unused) { - if (attrd_shutting_down()) { + if (attrd.shutting_down) { pcmk__info("Disconnected from Corosync process group"); return; } diff --git a/daemons/attrd/attrd_elections.c b/daemons/attrd/attrd_elections.c index c48c808fda7..8a0c7d4915b 100644 --- a/daemons/attrd/attrd_elections.c +++ b/daemons/attrd/attrd_elections.c @@ -46,7 +46,7 @@ attrd_start_election_if_needed(void) { if ((peer_writer == NULL) && (election_state(attrd_cluster) != election_in_progress) - && !attrd_shutting_down()) { + && !attrd.shutting_down) { pcmk__info("Starting an election to determine the writer"); election_vote(attrd_cluster); @@ -68,7 +68,7 @@ attrd_handle_election_op(const pcmk__node_status_t *peer, xmlNode *xml) pcmk__xe_set(xml, PCMK__XA_SRC, peer->name); // Don't become writer if we're shutting down - rc = election_count_vote(attrd_cluster, xml, !attrd_shutting_down()); + rc = election_count_vote(attrd_cluster, xml, !attrd.shutting_down); switch(rc) { case election_start: diff --git a/daemons/attrd/attrd_ipc.c b/daemons/attrd/attrd_ipc.c index 3b0113ccac1..4ed8bdbccb0 100644 --- a/daemons/attrd/attrd_ipc.c +++ b/daemons/attrd/attrd_ipc.c @@ -492,7 +492,7 @@ static int32_t attrd_ipc_accept(qb_ipcs_connection_t *c, uid_t uid, gid_t gid) { pcmk__trace("New client connection %p", c); - if (attrd_shutting_down()) { + if (attrd.shutting_down) { pcmk__info("Ignoring new connection from pid %d during shutdown", pcmk__client_pid(c)); return -ECONNREFUSED; diff --git a/daemons/attrd/attrd_utils.c b/daemons/attrd/attrd_utils.c index 88979e66e50..d931db33b01 100644 --- a/daemons/attrd/attrd_utils.c +++ b/daemons/attrd/attrd_utils.c @@ -24,37 +24,22 @@ cib_t *the_cib = NULL; -static bool shutting_down = false; - /* A hash table storing information on the protocol version of each peer attrd. * The key is the peer's uname, and the value is the protocol version number. */ GHashTable *peer_protocol_vers = NULL; -/*! - * \internal - * \brief Check whether local attribute manager is shutting down - * - * \return \c true if local attribute manager has begun shutdown sequence, - * otherwise \c false - */ -bool -attrd_shutting_down(void) -{ - return shutting_down; -} - void attrd_quit_main_loop(crm_exit_t ec) { - if (attrd_shutting_down()) { + if (attrd.shutting_down) { return; } pcmk__info("Shutting down attribute manager"); // Tell various functions not to do anything - shutting_down = true; + attrd.shutting_down = true; attrd_exit_status = ec; diff --git a/daemons/attrd/pacemaker-attrd.c b/daemons/attrd/pacemaker-attrd.c index da2fa10a0cd..93e4f803f59 100644 --- a/daemons/attrd/pacemaker-attrd.c +++ b/daemons/attrd/pacemaker-attrd.c @@ -257,6 +257,13 @@ main(int argc, char **argv) pcmk__daemon_run(&attrd); done: + /* If we got here through any of the "goto done" calls instead of by the + * main loop quitting on SIGTERM, shutting_down will still be false. Set + * it here so attrd_cleanup -> attrd_cib_disconnect -> attrd_cib_destroy_cb + * doesn't call pcmk__daemon_quit with no main loop. + */ + attrd.shutting_down = true; + attrd_cleanup(); pcmk__output_and_clear_error(&error, out); diff --git a/daemons/attrd/pacemaker-attrd.h b/daemons/attrd/pacemaker-attrd.h index d4b10f78285..b0de4374a19 100644 --- a/daemons/attrd/pacemaker-attrd.h +++ b/daemons/attrd/pacemaker-attrd.h @@ -59,7 +59,6 @@ void attrd_free_waitlist(void); void attrd_quit_main_loop(crm_exit_t ec); -bool attrd_shutting_down(void); bool attrd_stand_alone(void); void attrd_ipc_init(void); void attrd_ipc_cleanup(void); diff --git a/include/crm/common/daemon_internal.h b/include/crm/common/daemon_internal.h index 7029eb110aa..47efc093ce7 100644 --- a/include/crm/common/daemon_internal.h +++ b/include/crm/common/daemon_internal.h @@ -14,6 +14,8 @@ #ifndef PCMK__CRM_COMMON_DAEMON_INTERNAL__H #define PCMK__CRM_COMMON_DAEMON_INTERNAL__H +#include // bool + #include // GMainLoop #include // pcmk_ipc_server @@ -30,6 +32,9 @@ typedef struct { //! Daemon type, indexed by the IPC enum enum pcmk_ipc_server type; + //! Is the daemon currently shutting down? + bool shutting_down; + //! Main loop GMainLoop *mainloop; } pcmk__daemon_t; From 72d1fada0c3da65830406125999a69e5ec967fa5 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 22 Jul 2026 13:37:57 -0400 Subject: [PATCH 09/60] Refactor: daemons: Add the standalone mode flag to pcmk__daemon_t. This is useful for starting up a daemon where it doesn't connect to the CIB or any other daemons, which is handy for running unit tests (for instance, cts-attrd). Not all daemons support this, but several do and it's arguable that all should eventually. --- daemons/attrd/attrd_cib.c | 2 +- daemons/attrd/pacemaker-attrd.c | 24 +++++------------------- daemons/attrd/pacemaker-attrd.h | 1 - include/crm/common/daemon_internal.h | 4 ++++ 4 files changed, 10 insertions(+), 21 deletions(-) diff --git a/daemons/attrd/attrd_cib.c b/daemons/attrd/attrd_cib.c index c97325ccbe6..8115076a3dc 100644 --- a/daemons/attrd/attrd_cib.c +++ b/daemons/attrd/attrd_cib.c @@ -520,7 +520,7 @@ write_attribute(attribute_t *a, bool ignore_delay) } // Private attributes (or any in standalone mode) are not written to the CIB - if (attrd_stand_alone() || pcmk__is_set(a->flags, attrd_attr_is_private)) { + if (attrd.stand_alone || pcmk__is_set(a->flags, attrd_attr_is_private)) { should_write = false; } diff --git a/daemons/attrd/pacemaker-attrd.c b/daemons/attrd/pacemaker-attrd.c index 93e4f803f59..4669e23019a 100644 --- a/daemons/attrd/pacemaker-attrd.c +++ b/daemons/attrd/pacemaker-attrd.c @@ -34,14 +34,13 @@ pcmk__daemon_t attrd = { .type = pcmk_ipc_attrd, }; -static gboolean stand_alone = false; - static gchar **log_files = NULL; static gchar **processed_args = NULL; static GOptionContext *context = NULL; static GOptionEntry entries[] = { - { "stand-alone", 's', G_OPTION_FLAG_NONE, G_OPTION_ARG_NONE, &stand_alone, + { "stand-alone", 's', G_OPTION_FLAG_NONE, G_OPTION_ARG_NONE, + &attrd.stand_alone, "(Advanced use only) Run in stand-alone mode", NULL }, { "logfile", 'l', G_OPTION_FLAG_NONE, G_OPTION_ARG_FILENAME_ARRAY, @@ -63,19 +62,6 @@ lrmd_t *the_lrmd = NULL; crm_trigger_t *attrd_config_read = NULL; crm_exit_t attrd_exit_status = CRM_EX_OK; -/*! - * \internal - * \brief Check whether local attribute manager is running in stand-alone mode - * - * \return \c true if local attribute manager is in stand-alone mode, or - * \c false otherwise - */ -bool -attrd_stand_alone(void) -{ - return stand_alone; -} - static bool ipc_already_running(void) { @@ -191,7 +177,7 @@ main(int argc, char **argv) crm_log_init(PCMK__VALUE_ATTRD, LOG_INFO, TRUE, FALSE, argc, argv, FALSE); pcmk__notice("Starting Pacemaker node attribute manager%s", - (attrd_stand_alone() ? " in standalone mode" : "")); + (attrd.stand_alone ? " in standalone mode" : "")); if (ipc_already_running()) { attrd_exit_status = CRM_EX_OK; @@ -208,7 +194,7 @@ main(int argc, char **argv) * This allows us to assume the CIB is connected whenever we process a * cluster or IPC message (which also avoids start-up race conditions). */ - if (!attrd_stand_alone()) { + if (!attrd.stand_alone) { if (attrd_cib_connect(30) != pcmk_ok) { attrd_exit_status = CRM_EX_FATAL; g_set_error(&error, PCMK__EXITC_ERROR, attrd_exit_status, @@ -230,7 +216,7 @@ main(int argc, char **argv) // Initialization that requires the cluster to be connected attrd_election_init(); - if (!attrd_stand_alone()) { + if (!attrd.stand_alone) { attrd_cib_init(); } diff --git a/daemons/attrd/pacemaker-attrd.h b/daemons/attrd/pacemaker-attrd.h index b0de4374a19..78f9ba7f1f4 100644 --- a/daemons/attrd/pacemaker-attrd.h +++ b/daemons/attrd/pacemaker-attrd.h @@ -59,7 +59,6 @@ void attrd_free_waitlist(void); void attrd_quit_main_loop(crm_exit_t ec); -bool attrd_stand_alone(void); void attrd_ipc_init(void); void attrd_ipc_cleanup(void); diff --git a/include/crm/common/daemon_internal.h b/include/crm/common/daemon_internal.h index 47efc093ce7..8f9599fcd29 100644 --- a/include/crm/common/daemon_internal.h +++ b/include/crm/common/daemon_internal.h @@ -35,6 +35,10 @@ typedef struct { //! Is the daemon currently shutting down? bool shutting_down; + // NOTE: This is set by glib command line processing, hence gboolean + //! Is the daemon running in stand alone mode? + gboolean stand_alone; + //! Main loop GMainLoop *mainloop; } pcmk__daemon_t; From 11454691964135febbce88494a81f6523be72978 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 22 Jul 2026 13:42:57 -0400 Subject: [PATCH 10/60] Refactor: daemons: Move the exit status into pcmk__daemon_t. At the moment this isn't used anywhere outside of the daemon itself, but that will change. --- daemons/attrd/attrd_utils.c | 2 +- daemons/attrd/pacemaker-attrd.c | 28 ++++++++++++++-------------- daemons/attrd/pacemaker-attrd.h | 1 - include/crm/common/daemon_internal.h | 4 ++++ 4 files changed, 19 insertions(+), 16 deletions(-) diff --git a/daemons/attrd/attrd_utils.c b/daemons/attrd/attrd_utils.c index d931db33b01..049e0f69020 100644 --- a/daemons/attrd/attrd_utils.c +++ b/daemons/attrd/attrd_utils.c @@ -41,7 +41,7 @@ attrd_quit_main_loop(crm_exit_t ec) // Tell various functions not to do anything attrd.shutting_down = true; - attrd_exit_status = ec; + attrd.ec = ec; // Don't respond to signals while shutting down mainloop_destroy_signal(SIGTERM); diff --git a/daemons/attrd/pacemaker-attrd.c b/daemons/attrd/pacemaker-attrd.c index 4669e23019a..d5fb2c26a65 100644 --- a/daemons/attrd/pacemaker-attrd.c +++ b/daemons/attrd/pacemaker-attrd.c @@ -32,6 +32,7 @@ pcmk__daemon_t attrd = { .type = pcmk_ipc_attrd, + .ec = CRM_EX_OK, }; static gchar **log_files = NULL; @@ -60,7 +61,6 @@ static pcmk__supported_format_t formats[] = { lrmd_t *the_lrmd = NULL; crm_trigger_t *attrd_config_read = NULL; -crm_exit_t attrd_exit_status = CRM_EX_OK; static bool ipc_already_running(void) @@ -154,14 +154,14 @@ main(int argc, char **argv) pcmk__register_formats(output_group, formats); if (!g_option_context_parse_strv(context, &processed_args, &error)) { - attrd_exit_status = CRM_EX_USAGE; + attrd.ec = CRM_EX_USAGE; goto done; } rc = pcmk__output_new(&out, args->output_ty, args->output_dest, argv); if ((rc != pcmk_rc_ok) || (out == NULL)) { - attrd_exit_status = CRM_EX_ERROR; - g_set_error(&error, PCMK__EXITC_ERROR, attrd_exit_status, + attrd.ec = CRM_EX_ERROR; + g_set_error(&error, PCMK__EXITC_ERROR, attrd.ec, "Error creating output format %s: %s", args->output_ty, pcmk_rc_str(rc)); goto done; @@ -180,8 +180,8 @@ main(int argc, char **argv) (attrd.stand_alone ? " in standalone mode" : "")); if (ipc_already_running()) { - attrd_exit_status = CRM_EX_OK; - g_set_error(&error, PCMK__EXITC_ERROR, attrd_exit_status, + attrd.ec = CRM_EX_OK; + g_set_error(&error, PCMK__EXITC_ERROR, attrd.ec, "Aborting start-up because an attribute manager " "instance is already active"); pcmk__crit("%s", error->message); @@ -196,8 +196,8 @@ main(int argc, char **argv) */ if (!attrd.stand_alone) { if (attrd_cib_connect(30) != pcmk_ok) { - attrd_exit_status = CRM_EX_FATAL; - g_set_error(&error, PCMK__EXITC_ERROR, attrd_exit_status, + attrd.ec = CRM_EX_FATAL; + g_set_error(&error, PCMK__EXITC_ERROR, attrd.ec, "Could not connect to the CIB"); goto done; } @@ -205,8 +205,8 @@ main(int argc, char **argv) } if (attrd_cluster_connect() != pcmk_rc_ok) { - attrd_exit_status = CRM_EX_FATAL; - g_set_error(&error, PCMK__EXITC_ERROR, attrd_exit_status, + attrd.ec = CRM_EX_FATAL; + g_set_error(&error, PCMK__EXITC_ERROR, attrd.ec, "Could not connect to the cluster"); goto done; } @@ -231,8 +231,8 @@ main(int argc, char **argv) rc = pcmk__daemon_init(&attrd); if (rc != pcmk_rc_ok) { - attrd_exit_status = CRM_EX_ERROR; - g_set_error(&error, PCMK__EXITC_ERROR, attrd_exit_status, + attrd.ec = CRM_EX_ERROR; + g_set_error(&error, PCMK__EXITC_ERROR, attrd.ec, "Error initializing daemon object: %s", pcmk_rc_str(rc)); goto done; @@ -255,9 +255,9 @@ main(int argc, char **argv) pcmk__output_and_clear_error(&error, out); if (out != NULL) { - out->finish(out, attrd_exit_status, true, NULL); + out->finish(out, attrd.ec, true, NULL); pcmk__output_free(out); } pcmk__unregister_formats(); - crm_exit(attrd_exit_status); + crm_exit(attrd.ec); } diff --git a/daemons/attrd/pacemaker-attrd.h b/daemons/attrd/pacemaker-attrd.h index 78f9ba7f1f4..275ccb0d7be 100644 --- a/daemons/attrd/pacemaker-attrd.h +++ b/daemons/attrd/pacemaker-attrd.h @@ -88,7 +88,6 @@ int attrd_failure_regex(regex_t *regex, const char *rsc, const char *op, unsigned int interval_ms); extern cib_t *the_cib; -extern crm_exit_t attrd_exit_status; /* Alerts */ diff --git a/include/crm/common/daemon_internal.h b/include/crm/common/daemon_internal.h index 8f9599fcd29..0dc717b66d7 100644 --- a/include/crm/common/daemon_internal.h +++ b/include/crm/common/daemon_internal.h @@ -19,6 +19,7 @@ #include // GMainLoop #include // pcmk_ipc_server +#include // crm_exit_t #ifdef __cplusplus extern "C" { @@ -39,6 +40,9 @@ typedef struct { //! Is the daemon running in stand alone mode? gboolean stand_alone; + //! What is the exit code of the daemon? + crm_exit_t ec; + //! Main loop GMainLoop *mainloop; } pcmk__daemon_t; From 845a3385e8631d08026bd415bdaa9df4ec83d162 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 22 Jul 2026 15:17:53 -0400 Subject: [PATCH 11/60] Refactor: libcrmcommon,daemons: Make pcmk__daemon_quit more useful. Basically, move everything that's in attrd_quit_main_loop into pcmk__daemon_quit and get rid of the former function. This makes it more useful in other daemons in the future. I wanted to move the mainloop_add_signal(SIGTERM, ...) function into the more generic daemon code as well, but that's not really possible. In order to do that, we need a function with the right type to pass as the callback. At the moment, attrd_shutdown fills that role. Anything I can think of to do just involves rewriting that same function somewhere else. I might as well leave it alone. --- daemons/attrd/attrd_cib.c | 2 +- daemons/attrd/attrd_corosync.c | 2 +- daemons/attrd/attrd_utils.c | 25 ------------------------- daemons/attrd/pacemaker-attrd.c | 2 +- daemons/attrd/pacemaker-attrd.h | 1 - include/crm/common/daemon_internal.h | 2 +- lib/common/daemon.c | 25 +++++++++++++++++++++++-- 7 files changed, 27 insertions(+), 32 deletions(-) diff --git a/daemons/attrd/attrd_cib.c b/daemons/attrd/attrd_cib.c index 8115076a3dc..36ba1c03e29 100644 --- a/daemons/attrd/attrd_cib.c +++ b/daemons/attrd/attrd_cib.c @@ -40,7 +40,7 @@ attrd_cib_destroy_cb(void *user_data) // @TODO This should trigger a reconnect, not a shutdown pcmk__crit("Lost connection to the CIB manager, shutting down"); - attrd_quit_main_loop(CRM_EX_DISCONNECT); + pcmk__daemon_quit(&attrd, CRM_EX_DISCONNECT); } static void diff --git a/daemons/attrd/attrd_corosync.c b/daemons/attrd/attrd_corosync.c index 8c1f42b5a04..b8b1e323613 100644 --- a/daemons/attrd/attrd_corosync.c +++ b/daemons/attrd/attrd_corosync.c @@ -185,7 +185,7 @@ attrd_cpg_destroy(void *unused) } pcmk__crit("Lost connection to Corosync process group, shutting down"); - attrd_quit_main_loop(CRM_EX_DISCONNECT); + pcmk__daemon_quit(&attrd, CRM_EX_DISCONNECT); } #endif // SUPPORT_COROSYNC diff --git a/daemons/attrd/attrd_utils.c b/daemons/attrd/attrd_utils.c index 049e0f69020..de214d62db2 100644 --- a/daemons/attrd/attrd_utils.c +++ b/daemons/attrd/attrd_utils.c @@ -29,31 +29,6 @@ cib_t *the_cib = NULL; */ GHashTable *peer_protocol_vers = NULL; -void -attrd_quit_main_loop(crm_exit_t ec) -{ - if (attrd.shutting_down) { - return; - } - - pcmk__info("Shutting down attribute manager"); - - // Tell various functions not to do anything - attrd.shutting_down = true; - - attrd.ec = ec; - - // Don't respond to signals while shutting down - mainloop_destroy_signal(SIGTERM); - mainloop_destroy_signal(SIGCHLD); - mainloop_destroy_signal(SIGPIPE); - mainloop_destroy_signal(SIGUSR1); - mainloop_destroy_signal(SIGUSR2); - mainloop_destroy_signal(SIGTRAP); - - pcmk__daemon_quit(&attrd); -} - /* strlen("value") */ #define plus_plus_len (5) diff --git a/daemons/attrd/pacemaker-attrd.c b/daemons/attrd/pacemaker-attrd.c index d5fb2c26a65..04c43949132 100644 --- a/daemons/attrd/pacemaker-attrd.c +++ b/daemons/attrd/pacemaker-attrd.c @@ -132,7 +132,7 @@ attrd_cleanup(void) static void attrd_shutdown(int nsig) { - attrd_quit_main_loop(CRM_EX_OK); + pcmk__daemon_quit(&attrd, CRM_EX_OK); } int diff --git a/daemons/attrd/pacemaker-attrd.h b/daemons/attrd/pacemaker-attrd.h index 275ccb0d7be..19a9bdd843d 100644 --- a/daemons/attrd/pacemaker-attrd.h +++ b/daemons/attrd/pacemaker-attrd.h @@ -58,7 +58,6 @@ CRM_EX_INDETERMINATE) void attrd_free_waitlist(void); -void attrd_quit_main_loop(crm_exit_t ec); void attrd_ipc_init(void); void attrd_ipc_cleanup(void); diff --git a/include/crm/common/daemon_internal.h b/include/crm/common/daemon_internal.h index 0dc717b66d7..55e9dc1450e 100644 --- a/include/crm/common/daemon_internal.h +++ b/include/crm/common/daemon_internal.h @@ -50,7 +50,7 @@ typedef struct { // Mainloop management functions int pcmk__daemon_init(pcmk__daemon_t *srv); -void pcmk__daemon_quit(pcmk__daemon_t *srv); +void pcmk__daemon_quit(pcmk__daemon_t *srv, crm_exit_t ec); void pcmk__daemon_run(pcmk__daemon_t *srv); #ifdef __cplusplus diff --git a/lib/common/daemon.c b/lib/common/daemon.c index 5216d49ce4d..639379b81a7 100644 --- a/lib/common/daemon.c +++ b/lib/common/daemon.c @@ -9,6 +9,7 @@ #include +#include // SIG* #include // false #include // NULL @@ -36,11 +37,31 @@ pcmk__daemon_init(pcmk__daemon_t *srv) * \internal * \brief Quit the daemon's main loop * - * \param[in,out] srv The daemon object + * \param[in,out] srv The daemon object + * \param[in] ec The exit code to assign to the daemon */ void -pcmk__daemon_quit(pcmk__daemon_t *srv) +pcmk__daemon_quit(pcmk__daemon_t *srv, crm_exit_t ec) { + if (srv->shutting_down) { + return; + } + + pcmk__info("Shutting down %s", pcmk__server_log_name(srv->type)); + + // Tell various functions not to do anything + srv->shutting_down = true; + + srv->ec = ec; + + // Don't respond to signals while shutting down + mainloop_destroy_signal(SIGTERM); + mainloop_destroy_signal(SIGCHLD); + mainloop_destroy_signal(SIGPIPE); + mainloop_destroy_signal(SIGUSR1); + mainloop_destroy_signal(SIGUSR2); + mainloop_destroy_signal(SIGTRAP); + CRM_CHECK((srv->mainloop != NULL) && g_main_loop_is_running(srv->mainloop), return); From b010a0575349f5aa8ddc019a005af553c3137e57 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 22 Jul 2026 15:44:12 -0400 Subject: [PATCH 12/60] Refactor: daemons,libcrmcommon: Add pcmk__daemon_ipc_running. This is intended to be a function that all daemons can use to determine if a previous instance is running before continuing to start up. At the moment, however, options to use it are fairly limited: * based, controld, execd, and fenced have not been converted to use the pcmk_ipc_api_t client interface that this function requires. For that matter, I don't even see where execd checks for a previous instance using the old interface either. * pacemakerd does some very different and daemon-specific stuff if it finds a previous instance, so it's not really suitable for conversion. * schedulerd doesn't check for a previous instance, which seems like an oversight to me. However, maybe there's a good reason for it? --- daemons/attrd/pacemaker-attrd.c | 36 ++++++------------------ include/crm/common/daemon_internal.h | 29 +++++++++++++++++-- lib/common/daemon.c | 42 +++++++++++++++++++++++++++- 3 files changed, 77 insertions(+), 30 deletions(-) diff --git a/daemons/attrd/pacemaker-attrd.c b/daemons/attrd/pacemaker-attrd.c index 04c43949132..39d72b678fd 100644 --- a/daemons/attrd/pacemaker-attrd.c +++ b/daemons/attrd/pacemaker-attrd.c @@ -30,9 +30,14 @@ #define SUMMARY "daemon for managing Pacemaker node attributes" +static pcmk__daemon_ipc_fns_t ipc_fns = { + .already_running = pcmk__daemon_ipc_running, +}; + pcmk__daemon_t attrd = { .type = pcmk_ipc_attrd, .ec = CRM_EX_OK, + .ipc_fns = &ipc_fns, }; static gchar **log_files = NULL; @@ -62,30 +67,6 @@ static pcmk__supported_format_t formats[] = { lrmd_t *the_lrmd = NULL; crm_trigger_t *attrd_config_read = NULL; -static bool -ipc_already_running(void) -{ - pcmk_ipc_api_t *old_instance = NULL; - int rc = pcmk_rc_ok; - - rc = pcmk_new_ipc_api(&old_instance, pcmk_ipc_attrd); - if (rc != pcmk_rc_ok) { - return false; - } - - rc = pcmk__connect_ipc(old_instance, pcmk_ipc_dispatch_sync, 2); - if (rc != pcmk_rc_ok) { - pcmk__debug("No existing %s instance found: %s", - pcmk_ipc_name(old_instance, true), pcmk_rc_str(rc)); - pcmk_free_ipc_api(old_instance); - return false; - } - - pcmk_disconnect_ipc(old_instance); - pcmk_free_ipc_api(old_instance); - return true; -} - static GOptionContext * build_arg_context(pcmk__common_args_t *args, GOptionGroup **group) { GOptionContext *context = NULL; @@ -176,10 +157,8 @@ main(int argc, char **argv) pcmk__add_logfiles(log_files, out); crm_log_init(PCMK__VALUE_ATTRD, LOG_INFO, TRUE, FALSE, argc, argv, FALSE); - pcmk__notice("Starting Pacemaker node attribute manager%s", - (attrd.stand_alone ? " in standalone mode" : "")); - if (ipc_already_running()) { + if (attrd.ipc_fns->already_running(&attrd)) { attrd.ec = CRM_EX_OK; g_set_error(&error, PCMK__EXITC_ERROR, attrd.ec, "Aborting start-up because an attribute manager " @@ -188,6 +167,9 @@ main(int argc, char **argv) goto done; } + pcmk__notice("Starting Pacemaker node attribute manager%s", + (attrd.stand_alone ? " in standalone mode" : "")); + attributes = pcmk__strkey_table(NULL, attrd_free_attribute); /* Connect to the CIB before connecting to the cluster or listening for IPC. diff --git a/include/crm/common/daemon_internal.h b/include/crm/common/daemon_internal.h index 55e9dc1450e..cea94d9e588 100644 --- a/include/crm/common/daemon_internal.h +++ b/include/crm/common/daemon_internal.h @@ -25,11 +25,30 @@ extern "C" { #endif +typedef struct pcmk__daemon_s pcmk__daemon_t; + /*! * \internal - * \brief This structure describes and manages a single pacemaker daemon + * \brief Daemon-specific IPC operations */ typedef struct { + /*! + * \internal + * \brief Determine if an instance of an IPC server is already running + * + * \param[in,out] d The daemon object + * + * \return \c true if an instance of the daemon is already running, and + * \c false if not + */ + bool (*already_running)(pcmk__daemon_t *); +} pcmk__daemon_ipc_fns_t; + +/*! + * \internal + * \brief This structure describes and manages a single pacemaker daemon + */ +struct pcmk__daemon_s { //! Daemon type, indexed by the IPC enum enum pcmk_ipc_server type; @@ -45,7 +64,13 @@ typedef struct { //! Main loop GMainLoop *mainloop; -} pcmk__daemon_t; + + pcmk__daemon_ipc_fns_t *ipc_fns; +}; + +// IPC functions + +bool pcmk__daemon_ipc_running(pcmk__daemon_t *srv); // Mainloop management functions diff --git a/lib/common/daemon.c b/lib/common/daemon.c index 639379b81a7..9c53cd32164 100644 --- a/lib/common/daemon.c +++ b/lib/common/daemon.c @@ -10,11 +10,12 @@ #include #include // SIG* -#include // false +#include // bool, false, true #include // NULL #include // g_clear_pointer, g_main_loop_* +#include // pcmk_ipc_api_t, pcmk_*_ipc_api #include // CRM_CHECK #include // CRM_EX_*, crm_exit, pcmk_rc_* @@ -33,6 +34,45 @@ pcmk__daemon_init(pcmk__daemon_t *srv) return pcmk_rc_ok; } +/*! + * \internal + * \brief Determine if an instance of an IPC server is already running + * + * \param[in,out] srv The daemon object + * + * \return \c true if an instance of \p srv is already running, and \c false if not + * + * \note This function can be used to determine if a daemon is up and running + * since all daemons use IPC. + * + * \note This function only works for those daemons that have been converted + * to use \c pcmk_ipc_api_t as the client interface. Older daemons will + * have to use their own daemon specific method to figure this out. + */ +bool +pcmk__daemon_ipc_running(pcmk__daemon_t *srv) +{ + pcmk_ipc_api_t *old_instance = NULL; + int rc = pcmk_rc_ok; + + rc = pcmk_new_ipc_api(&old_instance, srv->type); + if (rc != pcmk_rc_ok) { + return false; + } + + rc = pcmk__connect_ipc(old_instance, pcmk_ipc_dispatch_sync, 2); + if (rc != pcmk_rc_ok) { + pcmk__debug("No existing %s instance found: %s", + pcmk_ipc_name(old_instance, true), pcmk_rc_str(rc)); + pcmk_free_ipc_api(old_instance); + return false; + } + + pcmk_disconnect_ipc(old_instance); + pcmk_free_ipc_api(old_instance); + return true; +} + /*! * \internal * \brief Quit the daemon's main loop From 1c6a408fdb4cc73ab51fce460b57af51d6a77c68 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Tue, 21 Jul 2026 16:19:38 -0400 Subject: [PATCH 13/60] Refactor: libcrmcommon,daemons: Don't exit in pcmk__serve_attrd_ipc. This is all part of the plan to reduce the number of places crm_exit is called in the daemons so all shutdown goes through the same place. It turns out there's an awful lot of places a daemon can shut down. Here's one more. Instead, attrd_ipc_init -> pcmk__serve_attrd_ipc should both return an error code on failure and then the caller can decide what to do. I've left the error printing where it is so it doesn't have to be duplicated. And while I'm nearby, don't call crm_exit from ipc_proxy_init. Setting the exit code in ipc_proxy_init seems a little pointless at the moment, but it will be useful much later on. --- daemons/attrd/attrd_ipc.c | 3 ++- daemons/attrd/pacemaker-attrd.c | 5 ++++- daemons/attrd/pacemaker-attrd.h | 2 +- daemons/execd/pacemaker-execd.c | 7 +++++-- daemons/execd/pacemaker-execd.h | 3 ++- daemons/execd/remoted_proxy.c | 12 +++++++++--- lib/common/ipc_server.c | 3 --- 7 files changed, 23 insertions(+), 12 deletions(-) diff --git a/daemons/attrd/attrd_ipc.c b/daemons/attrd/attrd_ipc.c index 4ed8bdbccb0..1544f157099 100644 --- a/daemons/attrd/attrd_ipc.c +++ b/daemons/attrd/attrd_ipc.c @@ -649,8 +649,9 @@ attrd_ipc_cleanup(void) * \internal * \brief Set up attrd IPC communication */ -void +bool attrd_ipc_init(void) { pcmk__serve_attrd_ipc(&ipcs, &ipc_callbacks); + return ipcs != NULL; } diff --git a/daemons/attrd/pacemaker-attrd.c b/daemons/attrd/pacemaker-attrd.c index 39d72b678fd..319da7ddd69 100644 --- a/daemons/attrd/pacemaker-attrd.c +++ b/daemons/attrd/pacemaker-attrd.c @@ -209,7 +209,10 @@ main(int argc, char **argv) */ attrd_send_protocol(NULL); - attrd_ipc_init(); + if (!attrd_ipc_init()) { + attrd.ec = CRM_EX_FATAL; + goto done; + } rc = pcmk__daemon_init(&attrd); if (rc != pcmk_rc_ok) { diff --git a/daemons/attrd/pacemaker-attrd.h b/daemons/attrd/pacemaker-attrd.h index 19a9bdd843d..8f111312028 100644 --- a/daemons/attrd/pacemaker-attrd.h +++ b/daemons/attrd/pacemaker-attrd.h @@ -58,7 +58,7 @@ CRM_EX_INDETERMINATE) void attrd_free_waitlist(void); -void attrd_ipc_init(void); +bool attrd_ipc_init(void); void attrd_ipc_cleanup(void); int attrd_cib_connect(int max_retry); diff --git a/daemons/execd/pacemaker-execd.c b/daemons/execd/pacemaker-execd.c index 26b780e8e9a..72d46fea92d 100644 --- a/daemons/execd/pacemaker-execd.c +++ b/daemons/execd/pacemaker-execd.c @@ -43,6 +43,7 @@ static GMainLoop *mainloop = NULL; static stonith_t *fencer_api = NULL; time_t start_time; +crm_exit_t exit_code = CRM_EX_OK; static struct { gchar **log_files; @@ -319,7 +320,6 @@ int main(int argc, char **argv) { int rc = pcmk_rc_ok; - crm_exit_t exit_code = CRM_EX_OK; const char *option = NULL; @@ -430,7 +430,10 @@ main(int argc, char **argv) exit_code = CRM_EX_FATAL; goto done; } - ipc_proxy_init(); + + if (!ipc_proxy_init()) { + goto done; + } #endif mainloop_add_signal(SIGTERM, lrmd_shutdown); diff --git a/daemons/execd/pacemaker-execd.h b/daemons/execd/pacemaker-execd.h index ffc98c3b7e5..8f6dc042b3d 100644 --- a/daemons/execd/pacemaker-execd.h +++ b/daemons/execd/pacemaker-execd.h @@ -23,6 +23,7 @@ extern GHashTable *rsc_list; extern time_t start_time; +extern crm_exit_t exit_code; typedef struct { char *rsc_id; @@ -86,7 +87,7 @@ stonith_t *execd_get_fencer_connection(void); void execd_fencer_connection_failed(void); #ifdef PCMK__COMPILE_REMOTE -void ipc_proxy_init(void); +bool ipc_proxy_init(void); void ipc_proxy_cleanup(void); void ipc_proxy_add_provider(pcmk__client_t *client); void ipc_proxy_remove_provider(pcmk__client_t *client); diff --git a/daemons/execd/remoted_proxy.c b/daemons/execd/remoted_proxy.c index bc3c9a98838..0447d147bba 100644 --- a/daemons/execd/remoted_proxy.c +++ b/daemons/execd/remoted_proxy.c @@ -514,23 +514,29 @@ ipc_proxy_remove_provider(pcmk__client_t *ipc_proxy) g_list_free(remove_these); } -void +bool ipc_proxy_init(void) { ipc_clients = pcmk__strkey_table(NULL, NULL); pcmk__serve_based_ipc(&cib_ro, &cib_rw, &cib_proxy_callbacks_ro, &cib_proxy_callbacks_rw); + pcmk__serve_attrd_ipc(&attrd_ipcs, &attrd_proxy_callbacks); + if (attrd_ipcs == NULL) { + exit_code = CRM_EX_FATAL; + return false; + } pcmk__serve_controld_ipc(&controld_ipcs, &crmd_proxy_callbacks); if (controld_ipcs == NULL) { - // Error already logged - crm_exit(CRM_EX_FATAL); + exit_code = CRM_EX_FATAL; + return false; } pcmk__serve_fenced_ipc(&fencer_ipcs, &fencer_proxy_callbacks); pcmk__serve_pacemakerd_ipc(&pacemakerd_ipcs, &pacemakerd_proxy_callbacks); + return true; } void diff --git a/lib/common/ipc_server.c b/lib/common/ipc_server.c index ec92392918f..35a39dd3fe0 100644 --- a/lib/common/ipc_server.c +++ b/lib/common/ipc_server.c @@ -1125,8 +1125,6 @@ pcmk__serve_controld_ipc(qb_ipcs_service_t **ipcs, * * \param[out] ipcs Where to store newly created IPC server * \param[in] cb IPC callbacks - * - * \note This function exits fatally on error. */ void pcmk__serve_attrd_ipc(qb_ipcs_service_t **ipcs, @@ -1142,7 +1140,6 @@ pcmk__serve_attrd_ipc(qb_ipcs_service_t **ipcs, pcmk__server_log_name(pcmk_ipc_attrd)); pcmk__crit("Verify pacemaker and pacemaker_remote are not both " "enabled"); - crm_exit(CRM_EX_FATAL); } } From 252a1745374c4fcd0b276748251c9949cf2bd667 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Fri, 17 Jul 2026 09:44:15 -0400 Subject: [PATCH 14/60] Refactor: daemons: Add an atexit handler for certain cleanup in execd. See a previous commit to attrd for an explanation of this commit. We're just doing the same thing in execd that is now being done in attrd. --- daemons/execd/pacemaker-execd.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/daemons/execd/pacemaker-execd.c b/daemons/execd/pacemaker-execd.c index 72d46fea92d..34d68aadfa2 100644 --- a/daemons/execd/pacemaker-execd.c +++ b/daemons/execd/pacemaker-execd.c @@ -45,6 +45,9 @@ static stonith_t *fencer_api = NULL; time_t start_time; crm_exit_t exit_code = CRM_EX_OK; +static gchar **processed_args = NULL; +static GOptionContext *context = NULL; + static struct { gchar **log_files; #ifdef PCMK__COMPILE_REMOTE @@ -316,6 +319,17 @@ build_arg_context(pcmk__common_args_t *args, GOptionGroup **group) return context; } +static void +execd_cleanup_cmdline(void) +{ + g_clear_pointer(&processed_args, g_strfreev); + g_clear_pointer(&context, g_option_context_free); + g_clear_pointer(&options.log_files, g_strfreev); +#ifdef PCMK__COMPILE_REMOTE + g_clear_pointer(&options.port, g_free); +#endif +} + int main(int argc, char **argv) { @@ -329,8 +343,8 @@ main(int argc, char **argv) GOptionGroup *output_group = NULL; pcmk__common_args_t *args = NULL; - gchar **processed_args = NULL; - GOptionContext *context = NULL; + + atexit(execd_cleanup_cmdline); #ifdef PCMK__COMPILE_REMOTE // If necessary, create PID 1 now before any file descriptors are opened @@ -447,14 +461,6 @@ main(int argc, char **argv) exit_executor(); done: - g_strfreev(options.log_files); -#ifdef PCMK__COMPILE_REMOTE - g_free(options.port); -#endif // PCMK__COMPILE_REMOTE - - g_strfreev(processed_args); - pcmk__free_arg_context(context); - pcmk__output_and_clear_error(&error, out); if (out != NULL) { From 87a520199200f6433cb59044e9b0dd280187913b Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Fri, 17 Jul 2026 10:12:33 -0400 Subject: [PATCH 15/60] Refactor: daemons: Unindent drain_check and lrmd_drain_alerts. The real point of this commit is to make sure that lrmd_drain_alerts can be called even if mloop is NULL but while I'm here, I might as well fully unindent both of these functions. --- daemons/execd/execd_alerts.c | 42 +++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/daemons/execd/execd_alerts.c b/daemons/execd/execd_alerts.c index 5d1f9e3e7fa..e835e0c0621 100644 --- a/daemons/execd/execd_alerts.c +++ b/daemons/execd/execd_alerts.c @@ -168,28 +168,40 @@ execd_process_alert_exec(pcmk__client_t *client, xmlNode *request) static bool drain_check(unsigned int remaining_timeout_ms) { - if (inflight_alerts != NULL) { - unsigned int count = g_hash_table_size(inflight_alerts); + unsigned int count = 0; - if (count > 0) { - pcmk__trace("%d alerts pending (%.3fs timeout remaining)", - count, (remaining_timeout_ms / 1000.0)); - return TRUE; - } + if (inflight_alerts == NULL) { + return false; } - return FALSE; + + count = g_hash_table_size(inflight_alerts); + if (count > 0) { + pcmk__trace("%d alerts pending (%.3fs timeout remaining)", + count, (remaining_timeout_ms / 1000.0)); + return true; + } + + return false; } void lrmd_drain_alerts(GMainLoop *mloop) { - if (inflight_alerts != NULL) { - unsigned int timer_ms = max_inflight_timeout() + 5000; + unsigned int timer_ms = 0; + + if (mloop == NULL) { + return; + } - pcmk__trace("Draining in-flight alerts (timeout %.3fs)", - (timer_ms / 1000.0)); - draining_alerts = TRUE; - pcmk_drain_main_loop(mloop, timer_ms, drain_check); - g_clear_pointer(&inflight_alerts, g_hash_table_destroy); + if (inflight_alerts == NULL) { + return; } + + timer_ms = max_inflight_timeout() + 5000; + + pcmk__trace("Draining in-flight alerts (timeout %.3fs)", + (timer_ms / 1000.0)); + draining_alerts = TRUE; + pcmk_drain_main_loop(mloop, timer_ms, drain_check); + g_clear_pointer(&inflight_alerts, g_hash_table_destroy); } From ebcc67191df6d781ede678b5be3c204e224e44b1 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Fri, 17 Jul 2026 10:20:20 -0400 Subject: [PATCH 16/60] Refactor: daemons: Unindent lrmd_shutdown. This is just to make the shutdown paths a little bit easier to follow so it's easier to simplify them later. --- daemons/execd/pacemaker-execd.c | 61 ++++++++++++++++----------------- 1 file changed, 29 insertions(+), 32 deletions(-) diff --git a/daemons/execd/pacemaker-execd.c b/daemons/execd/pacemaker-execd.c index 34d68aadfa2..cc27eddaa9d 100644 --- a/daemons/execd/pacemaker-execd.c +++ b/daemons/execd/pacemaker-execd.c @@ -193,10 +193,7 @@ exit_executor(void) ipc_proxy_cleanup(); #endif - if (mainloop) { - lrmd_drain_alerts(mainloop); - } - + lrmd_drain_alerts(mainloop); execd_unregister_handlers(); g_hash_table_destroy(rsc_list); @@ -216,40 +213,40 @@ lrmd_shutdown(int nsig) #ifdef PCMK__COMPILE_REMOTE pcmk__client_t *ipc_proxy = ipc_proxy_get_provider(); + if (ipc_proxy == NULL) { + exit_executor(); + } + /* If there are active proxied IPC providers, then we may be running * resources, so notify the cluster that we wish to shut down. */ - if (ipc_proxy) { - if (shutting_down) { - pcmk__notice("Waiting for cluster to stop resources before " - "exiting"); - return; - } - - pcmk__info("Sending shutdown request to cluster"); - if (ipc_proxy_shutdown_req(ipc_proxy) < 0) { - pcmk__crit("Shutdown request failed, exiting immediately"); + if (shutting_down) { + pcmk__notice("Waiting for cluster to stop resources before exiting"); + return; + } - } else { - /* We requested a shutdown. Now, we need to wait for an - * acknowledgement from the proxy host, then wait for all proxy - * hosts to disconnect (which ensures that all resources have been - * stopped). - */ - shutting_down = TRUE; - - /* Stop accepting new proxy connections */ - execd_stop_tls_server(); - - /* Currently, we let the OS kill us if the clients don't disconnect - * in a reasonable time. We could instead set a long timer here - * (shorter than what the OS is likely to use) and exit immediately - * if it pops. - */ - return; - } + pcmk__info("Sending shutdown request to cluster"); + if (ipc_proxy_shutdown_req(ipc_proxy) < 0) { + pcmk__crit("Shutdown request failed, exiting immediately"); + exit_executor(); } + + /* We requested a shutdown. Now, we need to wait for an acknowledgement + * from the proxy host, then wait for all proxy hosts to disconnect (which + * ensures that all resources have been stopped). + */ + shutting_down = TRUE; + + /* Stop accepting new proxy connections */ + execd_stop_tls_server(); + + /* Currently, we let the OS kill us if the clients don't disconnect in a + * reasonable time. We could instead set a long timer here (shorter than + * what the OS is likely to use) and exit immediately if it pops. + */ + return; #endif + exit_executor(); } From cdba0e9928aff038fbebb9f98c483ad2e05f41e4 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 23 Jul 2026 11:29:46 -0400 Subject: [PATCH 17/60] Refactor: daemons: Rename exit_executor to execd_cleanup. For the moment, it does still exit as well. However, a future commit will change it to be more like attrd (where we have a cleanup function and a shutdown function), and renaming it in a separate step makes things a little clearer later. --- daemons/execd/pacemaker-execd.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/daemons/execd/pacemaker-execd.c b/daemons/execd/pacemaker-execd.c index cc27eddaa9d..8d376cbf3d7 100644 --- a/daemons/execd/pacemaker-execd.c +++ b/daemons/execd/pacemaker-execd.c @@ -60,7 +60,7 @@ static struct { static gboolean shutting_down = FALSE; #endif -static void exit_executor(void); +static void execd_cleanup(void); static void fencer_connection_destroy_cb(stonith_t *st, stonith_event_t *e) @@ -120,7 +120,7 @@ lrmd_client_destroy(pcmk__client_t *client) * if there are no more proxied IPC providers */ if (shutting_down && (ipc_proxy_get_provider() == NULL)) { - exit_executor(); + execd_cleanup(); } #endif } @@ -174,12 +174,8 @@ lrmd_server_send_notify(pcmk__client_t *client, xmlNode *msg) return ENOTCONN; } -/*! - * \internal - * \brief Clean up and exit immediately - */ static void -exit_executor(void) +execd_cleanup(void) { const unsigned int nclients = pcmk__ipc_client_count(); @@ -195,7 +191,7 @@ exit_executor(void) lrmd_drain_alerts(mainloop); execd_unregister_handlers(); - g_hash_table_destroy(rsc_list); + g_clear_pointer(&rsc_list, g_hash_table_destroy); // @TODO End mainloop instead so all cleanup is done crm_exit(CRM_EX_OK); @@ -214,7 +210,7 @@ lrmd_shutdown(int nsig) pcmk__client_t *ipc_proxy = ipc_proxy_get_provider(); if (ipc_proxy == NULL) { - exit_executor(); + execd_cleanup(); } /* If there are active proxied IPC providers, then we may be running @@ -228,7 +224,7 @@ lrmd_shutdown(int nsig) pcmk__info("Sending shutdown request to cluster"); if (ipc_proxy_shutdown_req(ipc_proxy) < 0) { pcmk__crit("Shutdown request failed, exiting immediately"); - exit_executor(); + execd_cleanup(); } /* We requested a shutdown. Now, we need to wait for an acknowledgement @@ -247,7 +243,7 @@ lrmd_shutdown(int nsig) return; #endif - exit_executor(); + execd_cleanup(); } /*! @@ -278,7 +274,7 @@ handle_shutdown_nack(void) if (shutting_down) { pcmk__info("Exiting immediately after IPC proxy provider indicated no " "resources will be stopped"); - exit_executor(); + execd_cleanup(); return; } #endif @@ -455,7 +451,7 @@ main(int argc, char **argv) g_main_loop_run(mainloop); /* should never get here */ - exit_executor(); + execd_cleanup(); done: pcmk__output_and_clear_error(&error, out); From e748c7ac8bc2cd7be00da1792656a542e9fb0c3b Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 23 Jul 2026 12:57:46 -0400 Subject: [PATCH 18/60] Refactor: daemons: Add execd_quit_main_loop. Shutting down in execd is complicated by the fact that it's really two programs in one - pacemaker-execd, and pacemaker_remoted. Thus, it seems easiest to switch the shutdown process over to being focused on quitting the main loop one piece at a time, instead of in a single patch like we could with attrd. This patch handles the pacemaker-execd case, switching it to using lrmd_shutdown to catch the SIGTERM signal, which calls execd_quit_main_loop to shut down the main loop, which then falls through to execd_cleanup to free stuff and then continue through the rest of the done label. --- daemons/execd/pacemaker-execd.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/daemons/execd/pacemaker-execd.c b/daemons/execd/pacemaker-execd.c index 8d376cbf3d7..6dc91a7b10c 100644 --- a/daemons/execd/pacemaker-execd.c +++ b/daemons/execd/pacemaker-execd.c @@ -61,6 +61,7 @@ static gboolean shutting_down = FALSE; #endif static void execd_cleanup(void); +static void execd_quit_main_loop(crm_exit_t ec); static void fencer_connection_destroy_cb(stonith_t *st, stonith_event_t *e) @@ -193,8 +194,10 @@ execd_cleanup(void) execd_unregister_handlers(); g_clear_pointer(&rsc_list, g_hash_table_destroy); +#ifdef PCMK__COMPILE_REMOTE // @TODO End mainloop instead so all cleanup is done crm_exit(CRM_EX_OK); +#endif } /*! @@ -243,7 +246,7 @@ lrmd_shutdown(int nsig) return; #endif - execd_cleanup(); + execd_quit_main_loop(CRM_EX_OK); } /*! @@ -323,6 +326,17 @@ execd_cleanup_cmdline(void) #endif } +static void +execd_quit_main_loop(crm_exit_t ec) +{ + /* There's no way to get to this function without the main loop running, + * but check just in case someone adds one in the future + */ + CRM_CHECK((mainloop != NULL) && g_main_loop_is_running(mainloop), return); + + g_main_loop_quit(mainloop); +} + int main(int argc, char **argv) { @@ -449,11 +463,11 @@ main(int argc, char **argv) "accepting connections"); pcmk__notice("OCF resource agent search path is %s", PCMK__OCF_RA_PATH); g_main_loop_run(mainloop); + g_clear_pointer(&mainloop, g_main_loop_unref); - /* should never get here */ +done: execd_cleanup(); -done: pcmk__output_and_clear_error(&error, out); if (out != NULL) { From a4130eebd8ae6b9d601eb02895b7619d25dd1ae9 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 23 Jul 2026 13:47:46 -0400 Subject: [PATCH 19/60] Refactor: daemons: Use execd_quit_main_loop for most remote cases. The only case not covered here is handle_shutdown_nack, which has a complicated enough explanation that I'm going to put it into its own patch. To keep things working, for the moment, I'm just calling crm_exit there. For the rest of the cases, no longer call crm_exit at the end of execd_cleanup and instead call execd_quit_main_loop which ensures we take the same paths as in the non-remote case. lrmd_client_destroy is slightly more complicated, but it's called from functions that are mainloop sources so it should be fine. --- daemons/execd/pacemaker-execd.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/daemons/execd/pacemaker-execd.c b/daemons/execd/pacemaker-execd.c index 6dc91a7b10c..6ddf7ea3c9e 100644 --- a/daemons/execd/pacemaker-execd.c +++ b/daemons/execd/pacemaker-execd.c @@ -117,11 +117,17 @@ lrmd_client_destroy(pcmk__client_t *client) pcmk__free_client(client); #ifdef PCMK__COMPILE_REMOTE - /* If we were waiting to shut down, we can now safely do so - * if there are no more proxied IPC providers + /* If we were waiting to shut down, we can now safely do so if there are + * no more proxied IPC providers + * + * This kills the main loop and cleans everything up. Control flow will + * eventually make it back up to lrmd_remote_client_destroy or + * execd_ipc_closed, which were both called directly from the main loop. + * After that, we'll return to the done label in main() and finish + * shutting down. */ if (shutting_down && (ipc_proxy_get_provider() == NULL)) { - execd_cleanup(); + execd_quit_main_loop(CRM_EX_OK); } #endif } @@ -193,11 +199,6 @@ execd_cleanup(void) lrmd_drain_alerts(mainloop); execd_unregister_handlers(); g_clear_pointer(&rsc_list, g_hash_table_destroy); - -#ifdef PCMK__COMPILE_REMOTE - // @TODO End mainloop instead so all cleanup is done - crm_exit(CRM_EX_OK); -#endif } /*! @@ -213,7 +214,7 @@ lrmd_shutdown(int nsig) pcmk__client_t *ipc_proxy = ipc_proxy_get_provider(); if (ipc_proxy == NULL) { - execd_cleanup(); + goto done; } /* If there are active proxied IPC providers, then we may be running @@ -227,7 +228,7 @@ lrmd_shutdown(int nsig) pcmk__info("Sending shutdown request to cluster"); if (ipc_proxy_shutdown_req(ipc_proxy) < 0) { pcmk__crit("Shutdown request failed, exiting immediately"); - execd_cleanup(); + goto done; } /* We requested a shutdown. Now, we need to wait for an acknowledgement @@ -244,6 +245,8 @@ lrmd_shutdown(int nsig) * what the OS is likely to use) and exit immediately if it pops. */ return; + +done: #endif execd_quit_main_loop(CRM_EX_OK); @@ -278,7 +281,7 @@ handle_shutdown_nack(void) pcmk__info("Exiting immediately after IPC proxy provider indicated no " "resources will be stopped"); execd_cleanup(); - return; + crm_exit(CRM_EX_OK); } #endif pcmk__debug("Ignoring unexpected shutdown rejection from IPC proxy " From eba829969fd824d7f0c81ed94b190c416b9c559f Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 23 Jul 2026 14:04:59 -0400 Subject: [PATCH 20/60] Refactor: daemons: Shut down properly from inside handle_shutdown_nack. The idea here is that all shutdown paths proceed back to the done label in main() so we can do the same cleanups in every case. This is complicated in handle_shutdown_nack since we're so far down in the call stack and there's a bunch of callbacks involved. Here's exactly what's happening: * handle_shutdown_nack returns ESHUTDOWN to its caller... * ipc_proxy_forward_client. All we need to do here is pass the return code on up the call stack to... * handle_ipc_fwd_request. We see the special ESHUTDOWN return code and return a NULL reply. handle_ipc_fwd_request is a part of the execd_handlers struct and is called as a function pointer so next... * We return to pcmk__process_request and then to execd_handle_request. We don't do anything here because we have a NULL reply. Next we return to... * We return to either lrmd_remote_client_msg or execd_ipc_dispatch. Both of these functions end up being called from the main loop (the latter is much harder to follow because it passes through libqb, but the key in understanding this is knowing that we add GIO sources to the main loop for the libqb callbacks). Thus, once we finally return from these functions, we'll be back in the main loop. That will lead us back to after g_main_loop_run is called, which puts us at the done label main(), where we call execd_cleanup just like everywhere else. --- daemons/execd/execd_messages.c | 10 +++++++++- daemons/execd/pacemaker-execd.c | 28 +++++++++++++++++++++++++--- daemons/execd/pacemaker-execd.h | 2 +- daemons/execd/remoted_proxy.c | 3 +-- 4 files changed, 36 insertions(+), 7 deletions(-) diff --git a/daemons/execd/execd_messages.c b/daemons/execd/execd_messages.c index 95335f366a4..a88daa0a11f 100644 --- a/daemons/execd/execd_messages.c +++ b/daemons/execd/execd_messages.c @@ -50,9 +50,17 @@ handle_ipc_fwd_request(pcmk__request_t *request) rc = ipc_proxy_forward_client(request->ipc_client, request->xml); - if (rc == pcmk_rc_ok) { + if ((rc == pcmk_rc_ok) || (rc == ESHUTDOWN)) { pcmk__set_result(&request->result, CRM_EX_OK, PCMK_EXEC_DONE, NULL); + if (rc == ESHUTDOWN) { + /* We're shutting down so return NULL for the reply, but + * execd_handle_request will still want to process a result which + * is why we set one above. + */ + return NULL; + } + } else { pcmk__set_result(&request->result, pcmk_rc2exitc(rc), PCMK_EXEC_ERROR, pcmk_rc_str(rc)); diff --git a/daemons/execd/pacemaker-execd.c b/daemons/execd/pacemaker-execd.c index 6ddf7ea3c9e..89bd6c02221 100644 --- a/daemons/execd/pacemaker-execd.c +++ b/daemons/execd/pacemaker-execd.c @@ -269,23 +269,45 @@ handle_shutdown_ack(void) "provider"); } +#ifdef PCMK__COMPILE_REMOTE +static bool +execd_quit_on_nack(pcmk__daemon_t *srv) +{ + lrmd_drain_alerts(execd.mainloop); + return true; +} +#endif + /*! * \internal * \brief Handle rejection of shutdown request + * + * \return Standard Pacemaker return code */ -void +int handle_shutdown_nack(void) { #ifdef PCMK__COMPILE_REMOTE if (shutting_down) { pcmk__info("Exiting immediately after IPC proxy provider indicated no " "resources will be stopped"); - execd_cleanup(); - crm_exit(CRM_EX_OK); + + /* Avoid calling the original quit function because that can potentially + * just lead us right back to this point. However, we still want to do + * everything in pcmk__daemon_quit (most importantly, kill the main loop) + * as well as drain alerts. + */ + execd.shutting_down = false; + execd.fns->quit = execd_quit_on_nack; + pcmk__daemon_quit(&execd, CRM_EX_OK); + + return ESHUTDOWN; } #endif + pcmk__debug("Ignoring unexpected shutdown rejection from IPC proxy " "provider"); + return pcmk_rc_ok; } static GOptionEntry entries[] = { diff --git a/daemons/execd/pacemaker-execd.h b/daemons/execd/pacemaker-execd.h index 8f6dc042b3d..7a5b362ab00 100644 --- a/daemons/execd/pacemaker-execd.h +++ b/daemons/execd/pacemaker-execd.h @@ -71,7 +71,7 @@ void execd_free_rsc(void *data); void handle_shutdown_ack(void); -void handle_shutdown_nack(void); +int handle_shutdown_nack(void); void lrmd_client_destroy(pcmk__client_t *client); diff --git a/daemons/execd/remoted_proxy.c b/daemons/execd/remoted_proxy.c index 0447d147bba..7f3bd78a878 100644 --- a/daemons/execd/remoted_proxy.c +++ b/daemons/execd/remoted_proxy.c @@ -176,8 +176,7 @@ ipc_proxy_forward_client(pcmk__client_t *ipc_proxy, xmlNode *xml) } if (pcmk__str_eq(msg_type, LRMD_IPC_OP_SHUTDOWN_NACK, pcmk__str_casei)) { - handle_shutdown_nack(); - return rc; + return handle_shutdown_nack(); } ipc_client = pcmk__find_client_by_id(session); From b26fe148d7ef8aeddc9997ec0aaad2dee5021a83 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 23 Jul 2026 14:28:39 -0400 Subject: [PATCH 21/60] Refactor: daemons: Move lrmd_shutdown into execd_quit_main_loop. And then add execd_shutdown as the signal handler, paralleling what's going on in attrd. --- daemons/execd/pacemaker-execd.c | 106 ++++++++++++++++---------------- 1 file changed, 54 insertions(+), 52 deletions(-) diff --git a/daemons/execd/pacemaker-execd.c b/daemons/execd/pacemaker-execd.c index 89bd6c02221..b80c958e9b8 100644 --- a/daemons/execd/pacemaker-execd.c +++ b/daemons/execd/pacemaker-execd.c @@ -201,57 +201,6 @@ execd_cleanup(void) g_clear_pointer(&rsc_list, g_hash_table_destroy); } -/*! - * \internal - * \brief Request cluster shutdown if appropriate, otherwise exit immediately - * - * \param[in] nsig Signal that caused invocation (ignored) - */ -static void -lrmd_shutdown(int nsig) -{ -#ifdef PCMK__COMPILE_REMOTE - pcmk__client_t *ipc_proxy = ipc_proxy_get_provider(); - - if (ipc_proxy == NULL) { - goto done; - } - - /* If there are active proxied IPC providers, then we may be running - * resources, so notify the cluster that we wish to shut down. - */ - if (shutting_down) { - pcmk__notice("Waiting for cluster to stop resources before exiting"); - return; - } - - pcmk__info("Sending shutdown request to cluster"); - if (ipc_proxy_shutdown_req(ipc_proxy) < 0) { - pcmk__crit("Shutdown request failed, exiting immediately"); - goto done; - } - - /* We requested a shutdown. Now, we need to wait for an acknowledgement - * from the proxy host, then wait for all proxy hosts to disconnect (which - * ensures that all resources have been stopped). - */ - shutting_down = TRUE; - - /* Stop accepting new proxy connections */ - execd_stop_tls_server(); - - /* Currently, we let the OS kill us if the clients don't disconnect in a - * reasonable time. We could instead set a long timer here (shorter than - * what the OS is likely to use) and exit immediately if it pops. - */ - return; - -done: -#endif - - execd_quit_main_loop(CRM_EX_OK); -} - /*! * \internal * \brief Log a shutdown acknowledgment @@ -354,6 +303,45 @@ execd_cleanup_cmdline(void) static void execd_quit_main_loop(crm_exit_t ec) { +#ifdef PCMK__COMPILE_REMOTE + pcmk__client_t *ipc_proxy = ipc_proxy_get_provider(); + + if (ipc_proxy == NULL) { + goto done; + } + + /* If there are active proxied IPC providers, then we may be running + * resources, so notify the cluster that we wish to shut down. + */ + if (shutting_down) { + pcmk__notice("Waiting for cluster to stop resources before exiting"); + return; + } + + pcmk__info("Sending shutdown request to cluster"); + if (ipc_proxy_shutdown_req(ipc_proxy) < 0) { + pcmk__crit("Shutdown request failed, exiting immediately"); + goto done; + } + + /* We requested a shutdown. Now, we need to wait for an acknowledgement + * from the proxy host, then wait for all proxy hosts to disconnect (which + * ensures that all resources have been stopped). + */ + shutting_down = TRUE; + + /* Stop accepting new proxy connections */ + execd_stop_tls_server(); + + /* Currently, we let the OS kill us if the clients don't disconnect in a + * reasonable time. We could instead set a long timer here (shorter than + * what the OS is likely to use) and exit immediately if it pops. + */ + return; + +done: +#endif + /* There's no way to get to this function without the main loop running, * but check just in case someone adds one in the future */ @@ -362,6 +350,20 @@ execd_quit_main_loop(crm_exit_t ec) g_main_loop_quit(mainloop); } +/*! + * \internal + * \brief Quit the main loop and set the exit code to \c CRM_EX_OK + * + * \param[in] nsig Ignored + * + * \note This is a main loop signal handler function. + */ +static void +execd_shutdown(int nsig) +{ + execd_quit_main_loop(CRM_EX_OK); +} + int main(int argc, char **argv) { @@ -482,7 +484,7 @@ main(int argc, char **argv) } #endif - mainloop_add_signal(SIGTERM, lrmd_shutdown); + mainloop_add_signal(SIGTERM, execd_shutdown); mainloop = g_main_loop_new(NULL, FALSE); pcmk__notice("Pacemaker " EXECD_TYPE " executor successfully started and " "accepting connections"); From 1ae6b3ac0c0fc6e9644e9acc0ee6c49ca9cd96ab Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 23 Jul 2026 14:59:23 -0400 Subject: [PATCH 22/60] Refactor: daemons: Add pcmk__daemon_t to execd for exit status. At the moment, we're not even calling pcmk__daemon_init on it, which should be fine because we're not using it for anything else. This is just to make it easier to follow the patches that convert execd. --- daemons/execd/pacemaker-execd.c | 18 +++++++++++------- daemons/execd/pacemaker-execd.h | 2 +- daemons/execd/remoted_proxy.c | 4 ++-- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/daemons/execd/pacemaker-execd.c b/daemons/execd/pacemaker-execd.c index b80c958e9b8..832c913719c 100644 --- a/daemons/execd/pacemaker-execd.c +++ b/daemons/execd/pacemaker-execd.c @@ -40,10 +40,14 @@ # define SUMMARY "resource agent executor daemon for Pacemaker cluster nodes" #endif +pcmk__daemon_t execd = { + .type = pcmk_ipc_execd, + .ec = CRM_EX_OK, +}; + static GMainLoop *mainloop = NULL; static stonith_t *fencer_api = NULL; time_t start_time; -crm_exit_t exit_code = CRM_EX_OK; static gchar **processed_args = NULL; static GOptionContext *context = NULL; @@ -397,14 +401,14 @@ main(int argc, char **argv) pcmk__register_formats(output_group, formats); if (!g_option_context_parse_strv(context, &processed_args, &error)) { - exit_code = CRM_EX_USAGE; + execd.ec = CRM_EX_USAGE; goto done; } rc = pcmk__output_new(&out, args->output_ty, args->output_dest, argv); if (rc != pcmk_rc_ok) { - exit_code = CRM_EX_ERROR; - g_set_error(&error, PCMK__EXITC_ERROR, exit_code, + execd.ec = CRM_EX_ERROR; + g_set_error(&error, PCMK__EXITC_ERROR, execd.ec, "Error creating output format %s: %s", args->output_ty, pcmk_rc_str(rc)); goto done; @@ -475,7 +479,7 @@ main(int argc, char **argv) if (lrmd_init_remote_tls_server() < 0) { pcmk__err("Failed to create TLS listener: shutting down and staying " "down"); - exit_code = CRM_EX_FATAL; + execd.ec = CRM_EX_FATAL; goto done; } @@ -498,9 +502,9 @@ main(int argc, char **argv) pcmk__output_and_clear_error(&error, out); if (out != NULL) { - out->finish(out, exit_code, true, NULL); + out->finish(out, execd.ec, true, NULL); pcmk__output_free(out); } pcmk__unregister_formats(); - crm_exit(exit_code); + crm_exit(execd.ec); } diff --git a/daemons/execd/pacemaker-execd.h b/daemons/execd/pacemaker-execd.h index 7a5b362ab00..79e04f24c5d 100644 --- a/daemons/execd/pacemaker-execd.h +++ b/daemons/execd/pacemaker-execd.h @@ -22,8 +22,8 @@ #include // stonith_t extern GHashTable *rsc_list; +extern pcmk__daemon_t execd; extern time_t start_time; -extern crm_exit_t exit_code; typedef struct { char *rsc_id; diff --git a/daemons/execd/remoted_proxy.c b/daemons/execd/remoted_proxy.c index 7f3bd78a878..4f46ea1a441 100644 --- a/daemons/execd/remoted_proxy.c +++ b/daemons/execd/remoted_proxy.c @@ -523,13 +523,13 @@ ipc_proxy_init(void) pcmk__serve_attrd_ipc(&attrd_ipcs, &attrd_proxy_callbacks); if (attrd_ipcs == NULL) { - exit_code = CRM_EX_FATAL; + execd.ec = CRM_EX_FATAL; return false; } pcmk__serve_controld_ipc(&controld_ipcs, &crmd_proxy_callbacks); if (controld_ipcs == NULL) { - exit_code = CRM_EX_FATAL; + execd.ec = CRM_EX_FATAL; return false; } From da0d010de6e813784c3d5d75c7a9337b436106a9 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 23 Jul 2026 15:02:51 -0400 Subject: [PATCH 23/60] Refactor: daemons: Use pcmk__daemon_t for shutting_down in execd. --- daemons/execd/pacemaker-execd.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/daemons/execd/pacemaker-execd.c b/daemons/execd/pacemaker-execd.c index 832c913719c..e35afc2414c 100644 --- a/daemons/execd/pacemaker-execd.c +++ b/daemons/execd/pacemaker-execd.c @@ -59,11 +59,6 @@ static struct { #endif // PCMK__COMPILE_REMOTE } options; -#ifdef PCMK__COMPILE_REMOTE -/* whether shutdown request has been sent */ -static gboolean shutting_down = FALSE; -#endif - static void execd_cleanup(void); static void execd_quit_main_loop(crm_exit_t ec); @@ -130,7 +125,7 @@ lrmd_client_destroy(pcmk__client_t *client) * After that, we'll return to the done label in main() and finish * shutting down. */ - if (shutting_down && (ipc_proxy_get_provider() == NULL)) { + if (execd.shutting_down && (ipc_proxy_get_provider() == NULL)) { execd_quit_main_loop(CRM_EX_OK); } #endif @@ -213,7 +208,7 @@ void handle_shutdown_ack(void) { #ifdef PCMK__COMPILE_REMOTE - if (shutting_down) { + if (execd.shutting_down) { pcmk__info("IPC proxy provider acknowledged shutdown request"); return; } @@ -241,7 +236,7 @@ int handle_shutdown_nack(void) { #ifdef PCMK__COMPILE_REMOTE - if (shutting_down) { + if (execd.shutting_down) { pcmk__info("Exiting immediately after IPC proxy provider indicated no " "resources will be stopped"); @@ -317,7 +312,7 @@ execd_quit_main_loop(crm_exit_t ec) /* If there are active proxied IPC providers, then we may be running * resources, so notify the cluster that we wish to shut down. */ - if (shutting_down) { + if (execd.shutting_down) { pcmk__notice("Waiting for cluster to stop resources before exiting"); return; } @@ -332,7 +327,7 @@ execd_quit_main_loop(crm_exit_t ec) * from the proxy host, then wait for all proxy hosts to disconnect (which * ensures that all resources have been stopped). */ - shutting_down = TRUE; + execd.shutting_down = true; /* Stop accepting new proxy connections */ execd_stop_tls_server(); From 6db51736c5a49c72bb37eb4fbcd89320f73d7b56 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 23 Jul 2026 15:29:43 -0400 Subject: [PATCH 24/60] Refactor: libcrmcommon: Add a quit function to pcmk__daemon_t. At the moment this is entirely optional - a daemon can choose to implement it, or not. If implemented, the idea is that it can perform certain shutdown tasks (nothing that cleans up, though) and decides whether or not to continue with shutdown. This is needed for execd. --- include/crm/common/daemon_internal.h | 23 +++++++++++++++++++++++ lib/common/daemon.c | 6 ++++++ 2 files changed, 29 insertions(+) diff --git a/include/crm/common/daemon_internal.h b/include/crm/common/daemon_internal.h index cea94d9e588..9517a6f358b 100644 --- a/include/crm/common/daemon_internal.h +++ b/include/crm/common/daemon_internal.h @@ -27,6 +27,27 @@ extern "C" { typedef struct pcmk__daemon_s pcmk__daemon_t; +/*! + * \internal + * \brief Daemon-specific general operations + */ +typedef struct { + /*! + * \internal + * \brief Perform daemon-specific quitting tasks + * + * This function should not perform any cleanup or memory freeing tasks. + * It is meant to terminate anything that needs to happen before the + * main loop quits, as well as to determine whether or not that happens + * at all. + * + * \param[in,out] srv The daemon object + * + * \return \c true if quitting should continue, and \c false if not + */ + bool (*quit)(pcmk__daemon_t *); +} pcmk__daemon_fns_t; + /*! * \internal * \brief Daemon-specific IPC operations @@ -65,6 +86,8 @@ struct pcmk__daemon_s { //! Main loop GMainLoop *mainloop; + pcmk__daemon_fns_t *fns; + pcmk__daemon_ipc_fns_t *ipc_fns; }; diff --git a/lib/common/daemon.c b/lib/common/daemon.c index 9c53cd32164..96968246270 100644 --- a/lib/common/daemon.c +++ b/lib/common/daemon.c @@ -87,6 +87,12 @@ pcmk__daemon_quit(pcmk__daemon_t *srv, crm_exit_t ec) return; } + if ((srv->fns != NULL) && (srv->fns->quit != NULL)) { + if (!srv->fns->quit(srv)) { + return; + } + } + pcmk__info("Shutting down %s", pcmk__server_log_name(srv->type)); // Tell various functions not to do anything From ff4dd0733ee7bf06128fe53ce17dc43b3ada2414 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 23 Jul 2026 15:37:50 -0400 Subject: [PATCH 25/60] Refactor: daemons: Finish converting execd to using pcmk__daemon_t. Because shutting down execd is a little more complicated due to it being two programs in one, we need to use the new quit() function on a pcmk__daemon_t object to help decide whether or not to continue with shutdown. --- daemons/execd/pacemaker-execd.c | 51 ++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/daemons/execd/pacemaker-execd.c b/daemons/execd/pacemaker-execd.c index e35afc2414c..163e28ed8af 100644 --- a/daemons/execd/pacemaker-execd.c +++ b/daemons/execd/pacemaker-execd.c @@ -40,12 +40,18 @@ # define SUMMARY "resource agent executor daemon for Pacemaker cluster nodes" #endif +static bool execd_quit(pcmk__daemon_t *d); + +static pcmk__daemon_fns_t fns = { + .quit = execd_quit, +}; + pcmk__daemon_t execd = { .type = pcmk_ipc_execd, .ec = CRM_EX_OK, + .fns = &fns, }; -static GMainLoop *mainloop = NULL; static stonith_t *fencer_api = NULL; time_t start_time; @@ -59,9 +65,6 @@ static struct { #endif // PCMK__COMPILE_REMOTE } options; -static void execd_cleanup(void); -static void execd_quit_main_loop(crm_exit_t ec); - static void fencer_connection_destroy_cb(stonith_t *st, stonith_event_t *e) { @@ -126,7 +129,9 @@ lrmd_client_destroy(pcmk__client_t *client) * shutting down. */ if (execd.shutting_down && (ipc_proxy_get_provider() == NULL)) { - execd_quit_main_loop(CRM_EX_OK); + // Unset shutting_down so pcmk__daemon_quit does something + execd.shutting_down = false; + pcmk__daemon_quit(&execd, CRM_EX_OK); } #endif } @@ -195,7 +200,6 @@ execd_cleanup(void) ipc_proxy_cleanup(); #endif - lrmd_drain_alerts(mainloop); execd_unregister_handlers(); g_clear_pointer(&rsc_list, g_hash_table_destroy); } @@ -299,8 +303,8 @@ execd_cleanup_cmdline(void) #endif } -static void -execd_quit_main_loop(crm_exit_t ec) +static bool +execd_quit(pcmk__daemon_t *d) { #ifdef PCMK__COMPILE_REMOTE pcmk__client_t *ipc_proxy = ipc_proxy_get_provider(); @@ -314,7 +318,7 @@ execd_quit_main_loop(crm_exit_t ec) */ if (execd.shutting_down) { pcmk__notice("Waiting for cluster to stop resources before exiting"); - return; + return false; } pcmk__info("Sending shutdown request to cluster"); @@ -336,17 +340,13 @@ execd_quit_main_loop(crm_exit_t ec) * reasonable time. We could instead set a long timer here (shorter than * what the OS is likely to use) and exit immediately if it pops. */ - return; + return false; done: #endif - /* There's no way to get to this function without the main loop running, - * but check just in case someone adds one in the future - */ - CRM_CHECK((mainloop != NULL) && g_main_loop_is_running(mainloop), return); - - g_main_loop_quit(mainloop); + lrmd_drain_alerts(execd.mainloop); + return true; } /*! @@ -360,7 +360,7 @@ execd_quit_main_loop(crm_exit_t ec) static void execd_shutdown(int nsig) { - execd_quit_main_loop(CRM_EX_OK); + pcmk__daemon_quit(&execd, CRM_EX_OK); } int @@ -483,13 +483,18 @@ main(int argc, char **argv) } #endif + rc = pcmk__daemon_init(&execd); + if (rc != pcmk_rc_ok) { + execd.ec = CRM_EX_ERROR; + g_set_error(&error, PCMK__EXITC_ERROR, execd.ec, + "Error initializing daemon object: %s", + pcmk_rc_str(rc)); + goto done; + } + mainloop_add_signal(SIGTERM, execd_shutdown); - mainloop = g_main_loop_new(NULL, FALSE); - pcmk__notice("Pacemaker " EXECD_TYPE " executor successfully started and " - "accepting connections"); - pcmk__notice("OCF resource agent search path is %s", PCMK__OCF_RA_PATH); - g_main_loop_run(mainloop); - g_clear_pointer(&mainloop, g_main_loop_unref); + + pcmk__daemon_run(&execd); done: execd_cleanup(); From 181a7bc573064b9dae8c58e6e72d051fe2129cac Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 23 Jul 2026 15:43:43 -0400 Subject: [PATCH 26/60] Refactor: libcrmcommon,daemons: Don't exit in pcmk__serve_execd_ipc. --- daemons/execd/execd_ipc.c | 3 ++- daemons/execd/pacemaker-execd.c | 5 ++++- daemons/execd/pacemaker-execd.h | 2 +- lib/common/ipc_server.c | 3 --- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/daemons/execd/execd_ipc.c b/daemons/execd/execd_ipc.c index c8821f51c9e..47f87a99ec4 100644 --- a/daemons/execd/execd_ipc.c +++ b/daemons/execd/execd_ipc.c @@ -218,8 +218,9 @@ execd_ipc_cleanup(void) * \internal * \brief Set up executor IPC communication */ -void +bool execd_ipc_init(void) { pcmk__serve_execd_ipc(&ipcs, &ipc_callbacks); + return ipcs != NULL; } diff --git a/daemons/execd/pacemaker-execd.c b/daemons/execd/pacemaker-execd.c index 163e28ed8af..467f645ad06 100644 --- a/daemons/execd/pacemaker-execd.c +++ b/daemons/execd/pacemaker-execd.c @@ -468,7 +468,10 @@ main(int argc, char **argv) rsc_list = pcmk__strkey_table(NULL, execd_free_rsc); - execd_ipc_init(); + if (!execd_ipc_init()) { + execd.ec = CRM_EX_FATAL; + goto done; + } #ifdef PCMK__COMPILE_REMOTE if (lrmd_init_remote_tls_server() < 0) { diff --git a/daemons/execd/pacemaker-execd.h b/daemons/execd/pacemaker-execd.h index 79e04f24c5d..83dee5130f1 100644 --- a/daemons/execd/pacemaker-execd.h +++ b/daemons/execd/pacemaker-execd.h @@ -105,7 +105,7 @@ void lrmd_drain_alerts(GMainLoop *mloop); bool execd_invalid_msg(xmlNode *msg); void execd_handle_request(pcmk__request_t *request); -void execd_ipc_init(void); +bool execd_ipc_init(void); void execd_ipc_cleanup(void); xmlNode *execd_create_reply_as(const char *origin, int rc, int call_id); diff --git a/lib/common/ipc_server.c b/lib/common/ipc_server.c index 35a39dd3fe0..cf996555ba2 100644 --- a/lib/common/ipc_server.c +++ b/lib/common/ipc_server.c @@ -1149,8 +1149,6 @@ pcmk__serve_attrd_ipc(qb_ipcs_service_t **ipcs, * * \param[out] ipcs Where to store newly created IPC server * \param[in] cb IPC callbacks - * - * \note This function exits fatally on error. */ void pcmk__serve_execd_ipc(qb_ipcs_service_t **ipcs, @@ -1162,7 +1160,6 @@ pcmk__serve_execd_ipc(qb_ipcs_service_t **ipcs, if (*ipcs == NULL) { pcmk__crit("Failed to create %s IPC server; shutting down", pcmk__server_log_name(pcmk_ipc_execd)); - crm_exit(CRM_EX_FATAL); } } From da27e4cb371fa1de4b1c2f6ce7d0fea861c8a764 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 20 Jul 2026 15:03:57 -0400 Subject: [PATCH 27/60] Refactor: libcrmcommon,daemons: Move start_time into the daemon object. Not all daemons keep track of this, but a couple do and it seems like it could be a useful thing for others in the future. --- daemons/execd/execd_commands.c | 2 +- daemons/execd/pacemaker-execd.c | 3 --- daemons/execd/pacemaker-execd.h | 2 -- include/crm/common/daemon_internal.h | 4 ++++ lib/common/daemon.c | 3 +++ 5 files changed, 8 insertions(+), 6 deletions(-) diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c index 89d77d6ec08..488cba0fe6e 100644 --- a/daemons/execd/execd_commands.c +++ b/daemons/execd/execd_commands.c @@ -1580,7 +1580,7 @@ execd_process_signon(pcmk__client_t *client, xmlNode *request, int call_id, pcmk__xe_set(*reply, PCMK__XA_LRMD_OP, CRM_OP_REGISTER); pcmk__xe_set(*reply, PCMK__XA_LRMD_CLIENTID, client->id); pcmk__xe_set(*reply, PCMK__XA_LRMD_PROTOCOL_VERSION, LRMD_PROTOCOL_VERSION); - pcmk__xe_set_time(*reply, PCMK__XA_UPTIME, now - start_time); + pcmk__xe_set_time(*reply, PCMK__XA_UPTIME, now - execd.start_time); if (start_state) { pcmk__xe_set(*reply, PCMK__XA_NODE_START_STATE, start_state); diff --git a/daemons/execd/pacemaker-execd.c b/daemons/execd/pacemaker-execd.c index 467f645ad06..4320e56dc36 100644 --- a/daemons/execd/pacemaker-execd.c +++ b/daemons/execd/pacemaker-execd.c @@ -53,7 +53,6 @@ pcmk__daemon_t execd = { }; static stonith_t *fencer_api = NULL; -time_t start_time; static gchar **processed_args = NULL; static GOptionContext *context = NULL; @@ -445,8 +444,6 @@ main(int argc, char **argv) } #endif // PCMK__COMPILE_REMOTE - start_time = time(NULL); - pcmk__notice("Starting Pacemaker " EXECD_TYPE " executor"); /* The presence of this variable allegedly controls whether child diff --git a/daemons/execd/pacemaker-execd.h b/daemons/execd/pacemaker-execd.h index 83dee5130f1..2403ad1e8dd 100644 --- a/daemons/execd/pacemaker-execd.h +++ b/daemons/execd/pacemaker-execd.h @@ -12,7 +12,6 @@ #include // bool #include // uint32_t -#include // time_t #include // GList, GHashTable, GMainLoop #include // xmlNode @@ -23,7 +22,6 @@ extern GHashTable *rsc_list; extern pcmk__daemon_t execd; -extern time_t start_time; typedef struct { char *rsc_id; diff --git a/include/crm/common/daemon_internal.h b/include/crm/common/daemon_internal.h index 9517a6f358b..08117af92ec 100644 --- a/include/crm/common/daemon_internal.h +++ b/include/crm/common/daemon_internal.h @@ -15,6 +15,7 @@ #define PCMK__CRM_COMMON_DAEMON_INTERNAL__H #include // bool +#include // time_t #include // GMainLoop @@ -80,6 +81,9 @@ struct pcmk__daemon_s { //! Is the daemon running in stand alone mode? gboolean stand_alone; + //! When did the daemon start running? + time_t start_time; + //! What is the exit code of the daemon? crm_exit_t ec; diff --git a/lib/common/daemon.c b/lib/common/daemon.c index 96968246270..bc725f75e93 100644 --- a/lib/common/daemon.c +++ b/lib/common/daemon.c @@ -12,6 +12,7 @@ #include // SIG* #include // bool, false, true #include // NULL +#include // time #include // g_clear_pointer, g_main_loop_* @@ -30,6 +31,8 @@ int pcmk__daemon_init(pcmk__daemon_t *srv) { + srv->start_time = time(NULL); + srv->mainloop = g_main_loop_new(NULL, false); return pcmk_rc_ok; } From f6cf94bff8671c1b2207546fdda74fa08f482ee8 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 20 Jul 2026 15:15:31 -0400 Subject: [PATCH 28/60] Refactor: daemons: Add an atexit handler for certain cleanup in fenced. See a previous commit to attrd for an explanation of this commit. We're just doing the same thing in fenced that is now being done in attrd. --- daemons/fenced/pacemaker-fenced.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/daemons/fenced/pacemaker-fenced.c b/daemons/fenced/pacemaker-fenced.c index 531d0738c10..a79bb83d098 100644 --- a/daemons/fenced/pacemaker-fenced.c +++ b/daemons/fenced/pacemaker-fenced.c @@ -47,6 +47,8 @@ static GMainLoop *mainloop = NULL; gboolean stonith_shutdown_flag = FALSE; static pcmk__output_t *out = NULL; +static gchar **processed_args = NULL; +static GOptionContext *context = NULL; pcmk__supported_format_t formats[] = { PCMK__SUPPORTED_FORMAT_NONE, @@ -348,6 +350,14 @@ ipc_already_running(void) return true; } +static void +fenced_cleanup_cmdline(void) +{ + g_clear_pointer(&processed_args, g_strfreev); + g_clear_pointer(&context, g_option_context_free); + g_clear_pointer(&options.log_files, g_strfreev); +} + int main(int argc, char **argv) { @@ -356,9 +366,13 @@ main(int argc, char **argv) GError *error = NULL; GOptionGroup *output_group = NULL; - pcmk__common_args_t *args = pcmk__new_common_args(SUMMARY); - gchar **processed_args = pcmk__cmdline_preproc(argv, "l"); - GOptionContext *context = build_arg_context(args, &output_group); + pcmk__common_args_t *args = NULL; + + atexit(fenced_cleanup_cmdline); + + args = pcmk__new_common_args(SUMMARY); + processed_args = pcmk__cmdline_preproc(argv, "l"); + context = build_arg_context(args, &output_group); crm_log_preinit(NULL, argc, argv); @@ -448,11 +462,6 @@ main(int argc, char **argv) g_main_loop_run(mainloop); done: - g_strfreev(processed_args); - pcmk__free_arg_context(context); - - g_strfreev(options.log_files); - stonith_cleanup(); fenced_cluster_disconnect(); fenced_unregister_handlers(); From 860c5e240f89886a78eecda13e3c3c0042e53e7e Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Fri, 24 Jul 2026 09:36:34 -0400 Subject: [PATCH 29/60] Refactor: daemons: Improve the stonith_cleanup function. * Rename it to fenced_cleanup, inline with similar functions in other daemons. * Move it down closer to the one place it's used, and remove the unnecessary forward declaration. * Move more cleanup calls into it and rearrange them a bit. --- daemons/fenced/pacemaker-fenced.c | 33 +++++++++++++++---------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/daemons/fenced/pacemaker-fenced.c b/daemons/fenced/pacemaker-fenced.c index a79bb83d098..45060b07153 100644 --- a/daemons/fenced/pacemaker-fenced.c +++ b/daemons/fenced/pacemaker-fenced.c @@ -64,8 +64,6 @@ static struct { crm_exit_t exit_code = CRM_EX_OK; -static void stonith_cleanup(void); - void do_local_reply(const xmlNode *notify_src, pcmk__client_t *client, int call_options) @@ -272,17 +270,6 @@ stonith_shutdown(int nsig) } } -static void -stonith_cleanup(void) -{ - fenced_cib_cleanup(); - fenced_ipc_cleanup(); - free_stonith_remote_op_list(); - free_topology_list(); - fenced_free_device_table(); - free_metadata_cache(); -} - /* @COMPAT Deprecated since 2.1.8. Use pcmk_list_fence_attrs() or * crm_resource --list-options=fencing instead of querying daemon metadata. * @@ -358,6 +345,21 @@ fenced_cleanup_cmdline(void) g_clear_pointer(&options.log_files, g_strfreev); } +static void +fenced_cleanup(void) +{ + fenced_cib_cleanup(); + fenced_ipc_cleanup(); + fenced_unregister_handlers(); + fenced_cluster_disconnect(); + fenced_scheduler_cleanup(); + + free_stonith_remote_op_list(); + free_topology_list(); + fenced_free_device_table(); + free_metadata_cache(); +} + int main(int argc, char **argv) { @@ -462,10 +464,7 @@ main(int argc, char **argv) g_main_loop_run(mainloop); done: - stonith_cleanup(); - fenced_cluster_disconnect(); - fenced_unregister_handlers(); - fenced_scheduler_cleanup(); + fenced_cleanup(); pcmk__output_and_clear_error(&error, out); From 035871c1d267f9cdc2db36e8dd9a038395ae4e0d Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Fri, 24 Jul 2026 09:44:53 -0400 Subject: [PATCH 30/60] Refactor: daemons: Add pcmk__daemon_t to fenced for exit status. At the moment, we're not even calling pcmk__daemon_init on it, which should be fine because we're not using it for anything else. This is just to make it easier to follow the patches that convert fenced. --- daemons/fenced/fenced_cib.c | 2 +- daemons/fenced/pacemaker-fenced.c | 32 ++++++++++++++++--------------- daemons/fenced/pacemaker-fenced.h | 2 +- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/daemons/fenced/fenced_cib.c b/daemons/fenced/fenced_cib.c index 803cad11e33..b843de5f665 100644 --- a/daemons/fenced/fenced_cib.c +++ b/daemons/fenced/fenced_cib.c @@ -367,7 +367,7 @@ watchdog_device_update(void) rc = fenced_device_register(xml, true); pcmk__xml_free(xml); if (rc != pcmk_rc_ok) { - exit_code = CRM_EX_FATAL; + fenced.ec = CRM_EX_FATAL; pcmk__crit("Cannot register watchdog pseudo fence agent: %s", pcmk_rc_str(rc)); stonith_shutdown(0); diff --git a/daemons/fenced/pacemaker-fenced.c b/daemons/fenced/pacemaker-fenced.c index 45060b07153..7c2cd94b4c5 100644 --- a/daemons/fenced/pacemaker-fenced.c +++ b/daemons/fenced/pacemaker-fenced.c @@ -37,6 +37,11 @@ #define SUMMARY "daemon for executing fencing devices in a Pacemaker cluster" +pcmk__daemon_t fenced = { + .type = pcmk_ipc_fenced, + .ec = CRM_EX_OK, +}; + // @TODO This should be unsigned int long long fencing_watchdog_timeout_ms = 0; @@ -62,8 +67,6 @@ static struct { gchar **log_files; } options; -crm_exit_t exit_code = CRM_EX_OK; - void do_local_reply(const xmlNode *notify_src, pcmk__client_t *client, int call_options) @@ -380,14 +383,14 @@ main(int argc, char **argv) pcmk__register_formats(output_group, formats); if (!g_option_context_parse_strv(context, &processed_args, &error)) { - exit_code = CRM_EX_USAGE; + fenced.ec = CRM_EX_USAGE; goto done; } rc = pcmk__output_new(&out, args->output_ty, args->output_dest, argv); if ((rc != pcmk_rc_ok) || (out == NULL)) { - exit_code = CRM_EX_ERROR; - g_set_error(&error, PCMK__EXITC_ERROR, exit_code, + fenced.ec = CRM_EX_ERROR; + g_set_error(&error, PCMK__EXITC_ERROR, fenced.ec, "Error creating output format %s: %s", args->output_ty, pcmk_rc_str(rc)); goto done; @@ -403,8 +406,8 @@ main(int argc, char **argv) rc = fencer_metadata(); if (rc != pcmk_rc_ok) { - exit_code = CRM_EX_FATAL; - g_set_error(&error, PCMK__EXITC_ERROR, exit_code, + fenced.ec = CRM_EX_FATAL; + g_set_error(&error, PCMK__EXITC_ERROR, fenced.ec, "Unable to display metadata: %s", pcmk_rc_str(rc)); } goto done; @@ -419,8 +422,7 @@ main(int argc, char **argv) pcmk__notice("Starting Pacemaker fencer"); if (ipc_already_running()) { - exit_code = CRM_EX_OK; - g_set_error(&error, PCMK__EXITC_ERROR, exit_code, + g_set_error(&error, PCMK__EXITC_ERROR, fenced.ec, "Aborting start-up because a fencer instance is already active"); pcmk__crit("%s", error->message); goto done; @@ -432,15 +434,15 @@ main(int argc, char **argv) rc = fenced_scheduler_init(); if (rc != pcmk_rc_ok) { - exit_code = CRM_EX_FATAL; - g_set_error(&error, PCMK__EXITC_ERROR, exit_code, + fenced.ec = CRM_EX_FATAL; + g_set_error(&error, PCMK__EXITC_ERROR, fenced.ec, "Error initializing scheduler data: %s", pcmk_rc_str(rc)); goto done; } if (fenced_cluster_connect() != pcmk_rc_ok) { - exit_code = CRM_EX_FATAL; - g_set_error(&error, PCMK__EXITC_ERROR, exit_code, + fenced.ec = CRM_EX_FATAL; + g_set_error(&error, PCMK__EXITC_ERROR, fenced.ec, "Could not connect to the cluster"); goto done; } @@ -469,10 +471,10 @@ main(int argc, char **argv) pcmk__output_and_clear_error(&error, out); if (out != NULL) { - out->finish(out, exit_code, true, NULL); + out->finish(out, fenced.ec, true, NULL); pcmk__output_free(out); } pcmk__unregister_formats(); - crm_exit(exit_code); + crm_exit(fenced.ec); } diff --git a/daemons/fenced/pacemaker-fenced.h b/daemons/fenced/pacemaker-fenced.h index 6002c2f3601..cefb964ae32 100644 --- a/daemons/fenced/pacemaker-fenced.h +++ b/daemons/fenced/pacemaker-fenced.h @@ -405,6 +405,6 @@ extern GHashTable *topology; extern long long fencing_watchdog_timeout_ms; extern GList *stonith_watchdog_targets; extern GHashTable *stonith_remote_op_list; -extern crm_exit_t exit_code; extern gboolean stonith_shutdown_flag; extern pcmk_cluster_t *fenced_cluster; +extern pcmk__daemon_t fenced; From d377bb51e3e9c38d03cd8deca79ce2d53af951c8 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Fri, 24 Jul 2026 09:49:30 -0400 Subject: [PATCH 31/60] Refactor: daemons: Use pcmk__daemon_t for shutting_down in fenced. --- daemons/fenced/fenced_cib.c | 8 +++++--- daemons/fenced/fenced_ipc.c | 2 +- daemons/fenced/pacemaker-fenced.c | 11 ++++++++--- daemons/fenced/pacemaker-fenced.h | 1 - 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/daemons/fenced/fenced_cib.c b/daemons/fenced/fenced_cib.c index b843de5f665..3b631ba62f2 100644 --- a/daemons/fenced/fenced_cib.c +++ b/daemons/fenced/fenced_cib.c @@ -586,15 +586,17 @@ init_cib_cache_cb(xmlNode * msg, int call_id, int rc, xmlNode * output, void *us static void cib_connection_destroy(void *user_data) { - if (stonith_shutdown_flag) { + if (fenced.shutting_down) { pcmk__info("Connection to the CIB manager closed"); return; - } else { - pcmk__crit("Lost connection to the CIB manager, shutting down"); } + + pcmk__crit("Lost connection to the CIB manager, shutting down"); + if (cib_api) { cib_api->cmds->signoff(cib_api); } + stonith_shutdown(0); } diff --git a/daemons/fenced/fenced_ipc.c b/daemons/fenced/fenced_ipc.c index 34be2dd2b45..2653f42f290 100644 --- a/daemons/fenced/fenced_ipc.c +++ b/daemons/fenced/fenced_ipc.c @@ -65,7 +65,7 @@ static int32_t fenced_ipc_accept(qb_ipcs_connection_t *c, uid_t uid, gid_t gid) { pcmk__trace("New client connection %p", c); - if (stonith_shutdown_flag) { + if (fenced.shutting_down) { pcmk__info("Ignoring new connection from pid %d during shutdown", pcmk__client_pid(c)); return -ECONNREFUSED; diff --git a/daemons/fenced/pacemaker-fenced.c b/daemons/fenced/pacemaker-fenced.c index 7c2cd94b4c5..d5e869a816a 100644 --- a/daemons/fenced/pacemaker-fenced.c +++ b/daemons/fenced/pacemaker-fenced.c @@ -49,8 +49,6 @@ GList *stonith_watchdog_targets = NULL; static GMainLoop *mainloop = NULL; -gboolean stonith_shutdown_flag = FALSE; - static pcmk__output_t *out = NULL; static gchar **processed_args = NULL; static GOptionContext *context = NULL; @@ -267,7 +265,7 @@ void stonith_shutdown(int nsig) { pcmk__info("Terminating with %d clients", pcmk__ipc_client_count()); - stonith_shutdown_flag = TRUE; + fenced.shutting_down = true; if (mainloop != NULL && g_main_loop_is_running(mainloop)) { g_main_loop_quit(mainloop); } @@ -466,6 +464,13 @@ main(int argc, char **argv) g_main_loop_run(mainloop); done: + /* If we got here through any of the "goto done" calls instead of by the + * main loop quitting on SIGTERM, shutting_down will still be false. Set + * it here so fenced_cleanup -> fenced_cib_cleanup -> cib_connection_destroy + * doesn't call pcmk__daemon_quit with no main loop. + */ + fenced.shutting_down = true; + fenced_cleanup(); pcmk__output_and_clear_error(&error, out); diff --git a/daemons/fenced/pacemaker-fenced.h b/daemons/fenced/pacemaker-fenced.h index cefb964ae32..17d577ddf4a 100644 --- a/daemons/fenced/pacemaker-fenced.h +++ b/daemons/fenced/pacemaker-fenced.h @@ -405,6 +405,5 @@ extern GHashTable *topology; extern long long fencing_watchdog_timeout_ms; extern GList *stonith_watchdog_targets; extern GHashTable *stonith_remote_op_list; -extern gboolean stonith_shutdown_flag; extern pcmk_cluster_t *fenced_cluster; extern pcmk__daemon_t fenced; From 2c9e9760535c9465816f13aa214a7a8d3f9daff4 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Fri, 24 Jul 2026 09:50:47 -0400 Subject: [PATCH 32/60] Refactor: daemons: Use pcmk__daemon_t for stand_alone in fenced. --- daemons/fenced/pacemaker-fenced.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/daemons/fenced/pacemaker-fenced.c b/daemons/fenced/pacemaker-fenced.c index d5e869a816a..db1738da762 100644 --- a/daemons/fenced/pacemaker-fenced.c +++ b/daemons/fenced/pacemaker-fenced.c @@ -61,7 +61,6 @@ pcmk__supported_format_t formats[] = { }; static struct { - gboolean stand_alone; gchar **log_files; } options; @@ -292,7 +291,7 @@ fencer_metadata(void) static GOptionEntry entries[] = { { "stand-alone", 's', G_OPTION_FLAG_NONE, G_OPTION_ARG_NONE, - &options.stand_alone, N_("Intended for use in regression testing only"), + &fenced.stand_alone, N_("Intended for use in regression testing only"), NULL }, { "logfile", 'l', G_OPTION_FLAG_NONE, G_OPTION_ARG_FILENAME_ARRAY, @@ -449,7 +448,7 @@ main(int argc, char **argv) fenced_set_local_node(fenced_cluster->priv->node_name); - if (!options.stand_alone) { + if (!fenced.stand_alone) { setup_cib(); } From cf260eb79a02640462b55bf434fce79971a05d72 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Fri, 24 Jul 2026 10:09:13 -0400 Subject: [PATCH 33/60] Refactor: daemons: Finish converting fenced to using pcmk__daemon_t. This daemon was conveniently already shutting down how I wanted, so there's not much to do to complete this conversion. --- daemons/fenced/fenced_cib.c | 5 ++-- daemons/fenced/fenced_corosync.c | 2 +- daemons/fenced/pacemaker-fenced.c | 46 ++++++++++++++++++------------- daemons/fenced/pacemaker-fenced.h | 2 -- 4 files changed, 30 insertions(+), 25 deletions(-) diff --git a/daemons/fenced/fenced_cib.c b/daemons/fenced/fenced_cib.c index 3b631ba62f2..b4683027187 100644 --- a/daemons/fenced/fenced_cib.c +++ b/daemons/fenced/fenced_cib.c @@ -367,10 +367,9 @@ watchdog_device_update(void) rc = fenced_device_register(xml, true); pcmk__xml_free(xml); if (rc != pcmk_rc_ok) { - fenced.ec = CRM_EX_FATAL; pcmk__crit("Cannot register watchdog pseudo fence agent: %s", pcmk_rc_str(rc)); - stonith_shutdown(0); + pcmk__daemon_quit(&fenced, CRM_EX_FATAL); } } @@ -597,7 +596,7 @@ cib_connection_destroy(void *user_data) cib_api->cmds->signoff(cib_api); } - stonith_shutdown(0); + pcmk__daemon_quit(&fenced, CRM_EX_DISCONNECT); } /*! diff --git a/daemons/fenced/fenced_corosync.c b/daemons/fenced/fenced_corosync.c index 70fe9cd1fef..9beefcef6ed 100644 --- a/daemons/fenced/fenced_corosync.c +++ b/daemons/fenced/fenced_corosync.c @@ -170,7 +170,7 @@ static void fenced_cpg_destroy(void *unused) { pcmk__crit("Lost connection to cluster layer, shutting down"); - stonith_shutdown(0); + pcmk__daemon_quit(&fenced, CRM_EX_DISCONNECT); } #endif // SUPPORT_COROSYNC diff --git a/daemons/fenced/pacemaker-fenced.c b/daemons/fenced/pacemaker-fenced.c index db1738da762..23b9b3b99b2 100644 --- a/daemons/fenced/pacemaker-fenced.c +++ b/daemons/fenced/pacemaker-fenced.c @@ -47,8 +47,6 @@ long long fencing_watchdog_timeout_ms = 0; GList *stonith_watchdog_targets = NULL; -static GMainLoop *mainloop = NULL; - static pcmk__output_t *out = NULL; static gchar **processed_args = NULL; static GOptionContext *context = NULL; @@ -260,16 +258,6 @@ node_does_watchdog_fencing(const char *node) pcmk__str_in_list(node, stonith_watchdog_targets, pcmk__str_casei)); } -void -stonith_shutdown(int nsig) -{ - pcmk__info("Terminating with %d clients", pcmk__ipc_client_count()); - fenced.shutting_down = true; - if (mainloop != NULL && g_main_loop_is_running(mainloop)) { - g_main_loop_quit(mainloop); - } -} - /* @COMPAT Deprecated since 2.1.8. Use pcmk_list_fence_attrs() or * crm_resource --list-options=fencing instead of querying daemon metadata. * @@ -345,6 +333,21 @@ fenced_cleanup_cmdline(void) g_clear_pointer(&options.log_files, g_strfreev); } +/*! + * \internal + * \brief Quit the main loop and set the exit code to \c CRM_EX_OK + * + * \param[in] nsig Ignored + * + * \note This is a main loop signal handler function. + */ +static void +fenced_shutdown(int nsig) +{ + pcmk__info("Terminating with %d clients", pcmk__ipc_client_count()); + pcmk__daemon_quit(&fenced, CRM_EX_OK); +} + static void fenced_cleanup(void) { @@ -425,8 +428,6 @@ main(int argc, char **argv) goto done; } - mainloop_add_signal(SIGTERM, stonith_shutdown); - pcmk__cluster_init_node_caches(); rc = fenced_scheduler_init(); @@ -456,11 +457,18 @@ main(int argc, char **argv) init_topology_list(); fenced_ipc_init(); - // Create the mainloop and run it... - mainloop = g_main_loop_new(NULL, FALSE); - pcmk__notice("Pacemaker fencer successfully started and accepting " - "connections"); - g_main_loop_run(mainloop); + rc = pcmk__daemon_init(&fenced); + if (rc != pcmk_rc_ok) { + fenced.ec = CRM_EX_ERROR; + g_set_error(&error, PCMK__EXITC_ERROR, fenced.ec, + "Error initializing daemon object: %s", + pcmk_rc_str(rc)); + goto done; + } + + mainloop_add_signal(SIGTERM, fenced_shutdown); + + pcmk__daemon_run(&fenced); done: /* If we got here through any of the "goto done" calls instead of by the diff --git a/daemons/fenced/pacemaker-fenced.h b/daemons/fenced/pacemaker-fenced.h index 17d577ddf4a..4a0a6c35343 100644 --- a/daemons/fenced/pacemaker-fenced.h +++ b/daemons/fenced/pacemaker-fenced.h @@ -295,8 +295,6 @@ typedef struct { } stonith_topology_t; -void stonith_shutdown(int nsig); - void fenced_init_device_table(void); void fenced_free_device_table(void); bool fenced_has_watchdog_device(void); From 4422d47ee1a4d280152edcb55767de8ee23104d5 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Fri, 24 Jul 2026 10:11:31 -0400 Subject: [PATCH 34/60] Refactor: daemons: Get rid of the options struct in fenced. With just one member left in it, there's no point. --- daemons/fenced/pacemaker-fenced.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/daemons/fenced/pacemaker-fenced.c b/daemons/fenced/pacemaker-fenced.c index 23b9b3b99b2..651e2f01902 100644 --- a/daemons/fenced/pacemaker-fenced.c +++ b/daemons/fenced/pacemaker-fenced.c @@ -50,6 +50,7 @@ GList *stonith_watchdog_targets = NULL; static pcmk__output_t *out = NULL; static gchar **processed_args = NULL; static GOptionContext *context = NULL; +static gchar **log_files = NULL; pcmk__supported_format_t formats[] = { PCMK__SUPPORTED_FORMAT_NONE, @@ -58,10 +59,6 @@ pcmk__supported_format_t formats[] = { { NULL, NULL, NULL } }; -static struct { - gchar **log_files; -} options; - void do_local_reply(const xmlNode *notify_src, pcmk__client_t *client, int call_options) @@ -283,7 +280,7 @@ static GOptionEntry entries[] = { NULL }, { "logfile", 'l', G_OPTION_FLAG_NONE, G_OPTION_ARG_FILENAME_ARRAY, - &options.log_files, N_("Send logs to the additional named logfile"), NULL }, + &log_files, N_("Send logs to the additional named logfile"), NULL }, { NULL } }; @@ -330,7 +327,7 @@ fenced_cleanup_cmdline(void) { g_clear_pointer(&processed_args, g_strfreev); g_clear_pointer(&context, g_option_context_free); - g_clear_pointer(&options.log_files, g_strfreev); + g_clear_pointer(&log_files, g_strfreev); } /*! @@ -414,7 +411,7 @@ main(int argc, char **argv) } // Open additional log files - pcmk__add_logfiles(options.log_files, out); + pcmk__add_logfiles(log_files, out); crm_log_init(NULL, LOG_INFO + args->verbosity, TRUE, (args->verbosity > 0), argc, argv, FALSE); From 34625790d8698e24a8c780b3e5a8f86139b0e0a2 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Fri, 24 Jul 2026 10:15:34 -0400 Subject: [PATCH 35/60] Refactor: libcrmcommon,daemons: Add pcmk__generic_ipc_running. This function is useful for those daemons that have not yet been converted to the "new" pcmk_ipc_api_t interface. At the moment, only fenced can use this function. Once based and controld have been converted to use the pcmk__request_t server side interface (but have not yet been converted to use pcmk_ipc_api_t client side), they will need to use this function. Once everything is using pcmk_ipc_api_t on the client side, this function can go away. --- daemons/fenced/pacemaker-fenced.c | 38 ++++++------------------- include/crm/common/daemon_internal.h | 1 + lib/common/daemon.c | 42 +++++++++++++++++++++++++++- 3 files changed, 50 insertions(+), 31 deletions(-) diff --git a/daemons/fenced/pacemaker-fenced.c b/daemons/fenced/pacemaker-fenced.c index 651e2f01902..2c6e9aabbd9 100644 --- a/daemons/fenced/pacemaker-fenced.c +++ b/daemons/fenced/pacemaker-fenced.c @@ -37,9 +37,14 @@ #define SUMMARY "daemon for executing fencing devices in a Pacemaker cluster" +static pcmk__daemon_ipc_fns_t ipc_fns = { + .already_running = pcmk__generic_ipc_running, +}; + pcmk__daemon_t fenced = { .type = pcmk_ipc_fenced, .ec = CRM_EX_OK, + .ipc_fns = &ipc_fns, }; // @TODO This should be unsigned int @@ -295,33 +300,6 @@ build_arg_context(pcmk__common_args_t *args, GOptionGroup **group) return context; } -static bool -ipc_already_running(void) -{ - crm_ipc_t *old_instance = NULL; - int rc = pcmk_rc_ok; - - old_instance = crm_ipc_new("stonith-ng", 0); - if (old_instance == NULL) { - /* This is an error - memory allocation failed, etc. - but crm_ipc_new - * will have already logged an error message. - */ - return false; - } - - rc = pcmk__connect_generic_ipc(old_instance); - if (rc != pcmk_rc_ok) { - pcmk__debug("No existing stonith-ng instance found: %s", - pcmk_rc_str(rc)); - crm_ipc_destroy(old_instance); - return false; - } - - crm_ipc_close(old_instance); - crm_ipc_destroy(old_instance); - return true; -} - static void fenced_cleanup_cmdline(void) { @@ -416,15 +394,15 @@ main(int argc, char **argv) crm_log_init(NULL, LOG_INFO + args->verbosity, TRUE, (args->verbosity > 0), argc, argv, FALSE); - pcmk__notice("Starting Pacemaker fencer"); - - if (ipc_already_running()) { + if (fenced.ipc_fns->already_running(&fenced)) { g_set_error(&error, PCMK__EXITC_ERROR, fenced.ec, "Aborting start-up because a fencer instance is already active"); pcmk__crit("%s", error->message); goto done; } + pcmk__notice("Starting Pacemaker fencer"); + pcmk__cluster_init_node_caches(); rc = fenced_scheduler_init(); diff --git a/include/crm/common/daemon_internal.h b/include/crm/common/daemon_internal.h index 08117af92ec..742ae862f5c 100644 --- a/include/crm/common/daemon_internal.h +++ b/include/crm/common/daemon_internal.h @@ -98,6 +98,7 @@ struct pcmk__daemon_s { // IPC functions bool pcmk__daemon_ipc_running(pcmk__daemon_t *srv); +bool pcmk__generic_ipc_running(pcmk__daemon_t *srv); // Mainloop management functions diff --git a/lib/common/daemon.c b/lib/common/daemon.c index bc725f75e93..57a067b1fcb 100644 --- a/lib/common/daemon.c +++ b/lib/common/daemon.c @@ -16,7 +16,7 @@ #include // g_clear_pointer, g_main_loop_* -#include // pcmk_ipc_api_t, pcmk_*_ipc_api +#include // crm_ipc_*, pcmk_ipc_api_t, pcmk_*_ipc_api #include // CRM_CHECK #include // CRM_EX_*, crm_exit, pcmk_rc_* @@ -131,3 +131,43 @@ pcmk__daemon_run(pcmk__daemon_t *srv) g_main_loop_run(srv->mainloop); g_clear_pointer(&srv->mainloop, g_main_loop_unref); } + +/*! + * \internal + * \brief Determine if an instance of an IPC server is already running + * + * \param[in,out] srv The daemon object + * + * \return \c true if an instance of \p srv is already running, and \c false if not + * + * \note This function only works for older daemons that have not yet been + * converted to use the \c pcmk_ipc_api_t client interface. Once all have + * been updated, this function can be removed. + */ +bool +pcmk__generic_ipc_running(pcmk__daemon_t *srv) +{ + const char *ipc_name = pcmk__server_ipc_name(srv->type); + crm_ipc_t *old_instance = NULL; + int rc = pcmk_rc_ok; + + old_instance = crm_ipc_new(ipc_name, 0); + if (old_instance == NULL) { + /* This is an error - memory allocation failed, etc. - but crm_ipc_new + * will have already logged an error message. + */ + return false; + } + + rc = pcmk__connect_generic_ipc(old_instance); + if (rc != pcmk_rc_ok) { + pcmk__debug("No existing %s instance found: %s", ipc_name, + pcmk_rc_str(rc)); + crm_ipc_destroy(old_instance); + return false; + } + + crm_ipc_close(old_instance); + crm_ipc_destroy(old_instance); + return true; +} From 320e93e8e50f6853129292883b877144700abf8f Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 20 Jul 2026 16:42:27 -0400 Subject: [PATCH 36/60] Refactor: daemons: Consolidate cluster connection messages. Instead of having the success message in main() and the failure message in _cluster_connect, put both messages into the latter. This is just to clean the main functions up a little bit. --- daemons/attrd/attrd_corosync.c | 5 ++++- daemons/attrd/pacemaker-attrd.c | 2 -- daemons/based/based_corosync.c | 5 ++++- daemons/based/pacemaker-based.c | 2 -- daemons/fenced/fenced_corosync.c | 5 ++++- daemons/fenced/pacemaker-fenced.c | 2 -- 6 files changed, 12 insertions(+), 9 deletions(-) diff --git a/daemons/attrd/attrd_corosync.c b/daemons/attrd/attrd_corosync.c index b8b1e323613..2cb2fb849e3 100644 --- a/daemons/attrd/attrd_corosync.c +++ b/daemons/attrd/attrd_corosync.c @@ -510,7 +510,10 @@ attrd_cluster_connect(void) pcmk__cluster_set_status_callback(&attrd_peer_change_cb); rc = pcmk_cluster_connect(attrd_cluster); - if (rc != pcmk_rc_ok) { + + if (rc == pcmk_rc_ok) { + pcmk__info("Cluster connection active"); + } else { pcmk__err("Cluster connection failed"); } diff --git a/daemons/attrd/pacemaker-attrd.c b/daemons/attrd/pacemaker-attrd.c index 319da7ddd69..f63f704b4ce 100644 --- a/daemons/attrd/pacemaker-attrd.c +++ b/daemons/attrd/pacemaker-attrd.c @@ -193,8 +193,6 @@ main(int argc, char **argv) goto done; } - pcmk__info("Cluster connection active"); - // Initialization that requires the cluster to be connected attrd_election_init(); diff --git a/daemons/based/based_corosync.c b/daemons/based/based_corosync.c index cf8fc547d6c..b4fe16eafd2 100644 --- a/daemons/based/based_corosync.c +++ b/daemons/based/based_corosync.c @@ -115,7 +115,10 @@ based_cluster_connect(void) #endif // SUPPORT_COROSYNC rc = pcmk_cluster_connect(cluster); - if (rc != pcmk_rc_ok) { + + if (rc == pcmk_rc_ok) { + pcmk__info("Cluster connection active"); + } else { pcmk__err("Cluster connection failed"); } diff --git a/daemons/based/pacemaker-based.c b/daemons/based/pacemaker-based.c index 46206af7ffc..d8d9a145b64 100644 --- a/daemons/based/pacemaker-based.c +++ b/daemons/based/pacemaker-based.c @@ -405,8 +405,6 @@ main(int argc, char **argv) "Could not connect to the cluster"); goto done; } - - pcmk__info("Cluster connection active"); } // Run the main loop diff --git a/daemons/fenced/fenced_corosync.c b/daemons/fenced/fenced_corosync.c index 9beefcef6ed..a24721e4876 100644 --- a/daemons/fenced/fenced_corosync.c +++ b/daemons/fenced/fenced_corosync.c @@ -198,7 +198,10 @@ fenced_cluster_connect(void) pcmk__cluster_set_status_callback(&fenced_peer_change_cb); rc = pcmk_cluster_connect(fenced_cluster); - if (rc != pcmk_rc_ok) { + + if (rc == pcmk_rc_ok) { + pcmk__info("Cluster connection active"); + } else { pcmk__err("Cluster connection failed"); } diff --git a/daemons/fenced/pacemaker-fenced.c b/daemons/fenced/pacemaker-fenced.c index 2c6e9aabbd9..07811c52833 100644 --- a/daemons/fenced/pacemaker-fenced.c +++ b/daemons/fenced/pacemaker-fenced.c @@ -420,8 +420,6 @@ main(int argc, char **argv) goto done; } - pcmk__info("Cluster connection active"); - fenced_set_local_node(fenced_cluster->priv->node_name); if (!fenced.stand_alone) { From 96aee375072889763da589ec056cc024b0b20b6a Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Fri, 24 Jul 2026 10:23:41 -0400 Subject: [PATCH 37/60] Refactor: libcrmcommon,daemons: Don't exit in pcmk__serve_fenced_ipc. --- daemons/execd/remoted_proxy.c | 5 +++++ daemons/fenced/fenced_ipc.c | 3 ++- daemons/fenced/pacemaker-fenced.c | 6 +++++- daemons/fenced/pacemaker-fenced.h | 2 +- lib/common/ipc_server.c | 3 --- 5 files changed, 13 insertions(+), 6 deletions(-) diff --git a/daemons/execd/remoted_proxy.c b/daemons/execd/remoted_proxy.c index 4f46ea1a441..faad5ce9af1 100644 --- a/daemons/execd/remoted_proxy.c +++ b/daemons/execd/remoted_proxy.c @@ -534,6 +534,11 @@ ipc_proxy_init(void) } pcmk__serve_fenced_ipc(&fencer_ipcs, &fencer_proxy_callbacks); + if (fencer_ipcs == NULL) { + execd.ec = CRM_EX_FATAL; + return false; + } + pcmk__serve_pacemakerd_ipc(&pacemakerd_ipcs, &pacemakerd_proxy_callbacks); return true; } diff --git a/daemons/fenced/fenced_ipc.c b/daemons/fenced/fenced_ipc.c index 2653f42f290..cd346c8c5c1 100644 --- a/daemons/fenced/fenced_ipc.c +++ b/daemons/fenced/fenced_ipc.c @@ -276,8 +276,9 @@ fenced_ipc_cleanup(void) * \internal * \brief Set up fenced IPC communication */ -void +bool fenced_ipc_init(void) { pcmk__serve_fenced_ipc(&ipcs, &ipc_callbacks); + return ipcs != NULL; } diff --git a/daemons/fenced/pacemaker-fenced.c b/daemons/fenced/pacemaker-fenced.c index 07811c52833..8af9bbb4ece 100644 --- a/daemons/fenced/pacemaker-fenced.c +++ b/daemons/fenced/pacemaker-fenced.c @@ -428,7 +428,11 @@ main(int argc, char **argv) fenced_init_device_table(); init_topology_list(); - fenced_ipc_init(); + + if (!fenced_ipc_init()) { + fenced.ec = CRM_EX_FATAL; + goto done; + } rc = pcmk__daemon_init(&fenced); if (rc != pcmk_rc_ok) { diff --git a/daemons/fenced/pacemaker-fenced.h b/daemons/fenced/pacemaker-fenced.h index 4a0a6c35343..567cb239b71 100644 --- a/daemons/fenced/pacemaker-fenced.h +++ b/daemons/fenced/pacemaker-fenced.h @@ -373,7 +373,7 @@ const char *fenced_get_local_node(void); void fenced_scheduler_cleanup(void); void fenced_scheduler_run(xmlNode *cib); -void fenced_ipc_init(void); +bool fenced_ipc_init(void); void fenced_ipc_cleanup(void); int fenced_cluster_connect(void); diff --git a/lib/common/ipc_server.c b/lib/common/ipc_server.c index cf996555ba2..14d4f2bc729 100644 --- a/lib/common/ipc_server.c +++ b/lib/common/ipc_server.c @@ -1169,8 +1169,6 @@ pcmk__serve_execd_ipc(qb_ipcs_service_t **ipcs, * * \param[out] ipcs Where to store newly created IPC server * \param[in] cb IPC callbacks - * - * \note This function exits fatally on error. */ void pcmk__serve_fenced_ipc(qb_ipcs_service_t **ipcs, @@ -1186,7 +1184,6 @@ pcmk__serve_fenced_ipc(qb_ipcs_service_t **ipcs, pcmk__server_log_name(pcmk_ipc_fenced)); pcmk__crit("Verify pacemaker and pacemaker_remote are not both " "enabled"); - crm_exit(CRM_EX_FATAL); } } From cf7c0aa427629eedee7f2bb200ac4b443f0062a1 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Tue, 21 Jul 2026 08:57:19 -0400 Subject: [PATCH 38/60] Refactor: daemons: Add an atexit handler for certain cleanup in pacemakerd. See a previous commit to attrd for an explanation of this commit. We're just doing the same thing in pacemakerd that is now being done in attrd. --- daemons/pacemakerd/pacemakerd.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/daemons/pacemakerd/pacemakerd.c b/daemons/pacemakerd/pacemakerd.c index 286ff374ae4..99b1e0c4274 100644 --- a/daemons/pacemakerd/pacemakerd.c +++ b/daemons/pacemakerd/pacemakerd.c @@ -47,6 +47,8 @@ struct { } options; static pcmk__output_t *out = NULL; +static gchar **processed_args = NULL; +static GOptionContext *context = NULL; static pcmk__supported_format_t formats[] = { PCMK__SUPPORTED_FORMAT_NONE, @@ -339,6 +341,13 @@ handle_old_instance(gboolean shutdown) return rc; } +static void +pacemakerd_cleanup_cmdline(void) +{ + g_clear_pointer(&processed_args, g_strfreev); + g_clear_pointer(&context, g_option_context_free); +} + int main(int argc, char **argv) { @@ -348,9 +357,13 @@ main(int argc, char **argv) GError *error = NULL; GOptionGroup *output_group = NULL; - pcmk__common_args_t *args = pcmk__new_common_args(SUMMARY); - gchar **processed_args = pcmk__cmdline_preproc(argv, "p"); - GOptionContext *context = build_arg_context(args, &output_group); + pcmk__common_args_t * args = NULL; + + atexit(pacemakerd_cleanup_cmdline); + + args = pcmk__new_common_args(SUMMARY); + processed_args = pcmk__cmdline_preproc(argv, "p"); + context = build_arg_context(args, &output_group); subdaemon_check_progress = time(NULL); @@ -486,8 +499,6 @@ main(int argc, char **argv) #endif done: - g_strfreev(processed_args); - pcmk__free_arg_context(context); pcmk__output_and_clear_error(&error, out); From 6b70c961aaebd8d4400b7d063a5c292c3e78a8a5 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Tue, 21 Jul 2026 10:12:06 -0400 Subject: [PATCH 39/60] Refactor: daemons: Don't call crm_exit in create_pcmk_dirs. Instead, return a value and use that to decide what to do. In general, we want to limit the number of places crm_exit is called from in the daemons. --- daemons/pacemakerd/pacemakerd.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/daemons/pacemakerd/pacemakerd.c b/daemons/pacemakerd/pacemakerd.c index 99b1e0c4274..7a12315757f 100644 --- a/daemons/pacemakerd/pacemakerd.c +++ b/daemons/pacemakerd/pacemakerd.c @@ -155,7 +155,7 @@ pacemakerd_chown(const char *path, uid_t uid, gid_t gid) } } -static void +static int create_pcmk_dirs(void) { uid_t pcmk_uid = 0; @@ -174,7 +174,7 @@ create_pcmk_dirs(void) if (pcmk__daemon_user(&pcmk_uid, &pcmk_gid) != pcmk_rc_ok) { pcmk__err("Cluster user " CRM_DAEMON_USER " does not exist, aborting " "Pacemaker startup"); - crm_exit(CRM_EX_NOUSER); + return EINVAL; } // Used by some resource agents @@ -195,6 +195,8 @@ create_pcmk_dirs(void) pacemakerd_chown(dirs[i], pcmk_uid, pcmk_gid); } } + + return pcmk_rc_ok; } static void @@ -444,7 +446,10 @@ main(int argc, char **argv) mainloop = g_main_loop_new(NULL, FALSE); remove_core_file_limit(); - create_pcmk_dirs(); + if (create_pcmk_dirs() != pcmk_rc_ok) { + exit_code = CRM_EX_NOUSER; + goto done; + } pacemakerd_ipc_init(); #if SUPPORT_COROSYNC From a5d4ff65595e4f89b7bb3af83be706b9778e9543 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Tue, 21 Jul 2026 10:18:07 -0400 Subject: [PATCH 40/60] Refactor: daemons: Minor best practices around pacemakerd_read_config. * Rename to pcmkd_read_config, which is more consistent with other functions internal to this daemon. * Return bool instead of gboolean. * Unindent the do-while loop at the top of the function to make it easier to follow. * Make the call to crm_ipc_is_authentic_process easier to follow. * Minor cleanups at the call sites. --- daemons/pacemakerd/pacemakerd.c | 2 +- daemons/pacemakerd/pcmkd_corosync.c | 53 +++++++++++++++-------------- daemons/pacemakerd/pcmkd_corosync.h | 4 +-- 3 files changed, 31 insertions(+), 28 deletions(-) diff --git a/daemons/pacemakerd/pacemakerd.c b/daemons/pacemakerd/pacemakerd.c index 7a12315757f..73a6815106f 100644 --- a/daemons/pacemakerd/pacemakerd.c +++ b/daemons/pacemakerd/pacemakerd.c @@ -426,7 +426,7 @@ main(int argc, char **argv) } #if SUPPORT_COROSYNC - if (pacemakerd_read_config() == FALSE) { + if (!pcmkd_read_config()) { crm_exit(CRM_EX_UNAVAILABLE); } #endif diff --git a/daemons/pacemakerd/pcmkd_corosync.c b/daemons/pacemakerd/pcmkd_corosync.c index e085789d0cc..9972d4a294c 100644 --- a/daemons/pacemakerd/pcmkd_corosync.c +++ b/daemons/pacemakerd/pcmkd_corosync.c @@ -98,13 +98,14 @@ cluster_reconnect_cb(void *data) if (cluster_connect_cfg()) { g_clear_pointer(&reconnect_timer, mainloop_timer_del); pcmk__notice("Cluster reconnect succeeded"); - pacemakerd_read_config(); + pcmkd_read_config(); restart_cluster_subdaemons(); return G_SOURCE_REMOVE; - } else { - pcmk__info("Cluster reconnect failed (connection will be reattempted " - "once per second)"); } + + pcmk__info("Cluster reconnect failed (connection will be reattempted " + "once per second)"); + /* * In theory this will continue forever. In practice the CIB connection from * attrd will timeout and shut down Pacemaker when it gets bored. @@ -275,8 +276,8 @@ get_config_opt(uint64_t unused, cmap_handle_t object_handle, const char *key, ch return rc; } -gboolean -pacemakerd_read_config(void) +bool +pcmkd_read_config(void) { cs_error_t rc = CS_OK; int retries = 0; @@ -293,23 +294,21 @@ pacemakerd_read_config(void) // There can be only one possibility do { rc = pcmk__init_cmap(&local_handle); - if (rc != CS_OK) { - retries++; - pcmk__info("Could not connect to Corosync CMAP: %s " - "(retrying in %ds) " QB_XS " rc=%d", - pcmk_rc_str(pcmk__corosync2rc(rc)), retries, rc); - sleep(retries); - - } else { + if (rc == CS_OK) { break; } + retries++; + pcmk__info("Could not connect to Corosync CMAP: %s " + "(retrying in %ds) " QB_XS " rc=%d", + pcmk_rc_str(pcmk__corosync2rc(rc)), retries, rc); + sleep(retries); } while (retries < 5); if (rc != CS_OK) { pcmk__crit("Could not connect to Corosync CMAP: %s " QB_XS " rc=%d", pcmk_rc_str(pcmk__corosync2rc(rc)), rc); - return FALSE; + return false; } rc = cmap_fd_get(local_handle, &fd); @@ -317,23 +316,26 @@ pacemakerd_read_config(void) pcmk__crit("Could not get Corosync CMAP descriptor: %s " QB_XS " rc=%d", pcmk_rc_str(pcmk__corosync2rc(rc)), rc); cmap_finalize(local_handle); - return FALSE; + return false; } /* CMAP provider run as root (in given user namespace, anyway)? */ - if (!(rv = crm_ipc_is_authentic_process(fd, (uid_t) 0,(gid_t) 0, &found_pid, - &found_uid, &found_gid))) { + rv = crm_ipc_is_authentic_process(fd, (uid_t) 0,(gid_t) 0, &found_pid, + &found_uid, &found_gid); + if (rv == 0) { pcmk__crit("Rejecting Corosync CMAP provider because process %lld " "is running as uid %lld gid %lld, not root", (long long) PCMK__SPECIAL_PID_AS_0(found_pid), (long long) found_uid, (long long) found_gid); cmap_finalize(local_handle); - return FALSE; - } else if (rv < 0) { + return false; + } + + if (rv < 0) { pcmk__crit("Could not authenticate Corosync CMAP provider: %s " QB_XS " rc=%d", strerror(-rv), -rv); cmap_finalize(local_handle); - return FALSE; + return false; } cluster_layer = pcmk_get_cluster_layer(); @@ -343,7 +345,7 @@ pacemakerd_read_config(void) pcmk__crit("Expected Corosync cluster layer but detected %s " QB_XS " cluster_layer=%d", cluster_layer_s, cluster_layer); - return FALSE; + return false; } pcmk__info("Reading configuration for %s cluster layer", cluster_layer_s); @@ -369,8 +371,9 @@ pacemakerd_read_config(void) free(debug_enabled); } - if(local_handle){ + if (local_handle) { gid_t gid = 0; + if (pcmk__daemon_user(NULL, &gid) != pcmk_rc_ok) { pcmk__warn("Could not authorize group with Corosync " QB_XS " No group found for user " CRM_DAEMON_USER); @@ -389,7 +392,7 @@ pacemakerd_read_config(void) } } } - cmap_finalize(local_handle); - return TRUE; + cmap_finalize(local_handle); + return true; } diff --git a/daemons/pacemakerd/pcmkd_corosync.h b/daemons/pacemakerd/pcmkd_corosync.h index bad71022ab6..0fd75b37e43 100644 --- a/daemons/pacemakerd/pcmkd_corosync.h +++ b/daemons/pacemakerd/pcmkd_corosync.h @@ -1,5 +1,5 @@ /* - * Copyright 2010-2025 the Pacemaker project contributors + * Copyright 2010-2026 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -13,6 +13,6 @@ gboolean cluster_connect_cfg(void); void cluster_disconnect_cfg(void); -gboolean pacemakerd_read_config(void); +bool pcmkd_read_config(void); bool pcmkd_corosync_connected(void); void pcmkd_shutdown_corosync(void); From 98abe3a226904670f6a73ac97057390cbd238cdb Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Tue, 21 Jul 2026 10:21:10 -0400 Subject: [PATCH 41/60] Refactor: daemons: Don't call crm_exit if pcmkd_read_config fails. --- daemons/pacemakerd/pacemakerd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/daemons/pacemakerd/pacemakerd.c b/daemons/pacemakerd/pacemakerd.c index 73a6815106f..9a1fee8051b 100644 --- a/daemons/pacemakerd/pacemakerd.c +++ b/daemons/pacemakerd/pacemakerd.c @@ -427,7 +427,8 @@ main(int argc, char **argv) #if SUPPORT_COROSYNC if (!pcmkd_read_config()) { - crm_exit(CRM_EX_UNAVAILABLE); + exit_code = CRM_EX_UNAVAILABLE; + goto done; } #endif @@ -504,7 +505,6 @@ main(int argc, char **argv) #endif done: - pcmk__output_and_clear_error(&error, out); if (out != NULL) { From 6db78b9b6ba3d729a8daf879ecfa04939d15f935 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 29 Jul 2026 09:50:37 -0400 Subject: [PATCH 42/60] Refactor: daemons: Rename rc to cs_rc in pcmk_read_config. --- daemons/pacemakerd/pcmkd_corosync.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/daemons/pacemakerd/pcmkd_corosync.c b/daemons/pacemakerd/pcmkd_corosync.c index 9972d4a294c..bb406c706f2 100644 --- a/daemons/pacemakerd/pcmkd_corosync.c +++ b/daemons/pacemakerd/pcmkd_corosync.c @@ -279,7 +279,7 @@ get_config_opt(uint64_t unused, cmap_handle_t object_handle, const char *key, ch bool pcmkd_read_config(void) { - cs_error_t rc = CS_OK; + cs_error_t cs_rc = CS_OK; int retries = 0; cmap_handle_t local_handle; uint64_t config = 0; @@ -293,28 +293,28 @@ pcmkd_read_config(void) // There can be only one possibility do { - rc = pcmk__init_cmap(&local_handle); - if (rc == CS_OK) { + cs_rc = pcmk__init_cmap(&local_handle); + if (cs_rc == CS_OK) { break; } retries++; pcmk__info("Could not connect to Corosync CMAP: %s " "(retrying in %ds) " QB_XS " rc=%d", - pcmk_rc_str(pcmk__corosync2rc(rc)), retries, rc); + pcmk_rc_str(pcmk__corosync2rc(cs_rc)), retries, cs_rc); sleep(retries); } while (retries < 5); - if (rc != CS_OK) { + if (cs_rc != CS_OK) { pcmk__crit("Could not connect to Corosync CMAP: %s " - QB_XS " rc=%d", pcmk_rc_str(pcmk__corosync2rc(rc)), rc); + QB_XS " rc=%d", pcmk_rc_str(pcmk__corosync2rc(cs_rc)), cs_rc); return false; } - rc = cmap_fd_get(local_handle, &fd); - if (rc != CS_OK) { + cs_rc = cmap_fd_get(local_handle, &fd); + if (cs_rc != CS_OK) { pcmk__crit("Could not get Corosync CMAP descriptor: %s " QB_XS " rc=%d", - pcmk_rc_str(pcmk__corosync2rc(rc)), rc); + pcmk_rc_str(pcmk__corosync2rc(cs_rc)), cs_rc); cmap_finalize(local_handle); return false; } @@ -382,13 +382,13 @@ pcmkd_read_config(void) char *key = pcmk__assert_asprintf("uidgid.gid.%lld", (long long) gid); - rc = cmap_set_uint8(local_handle, key, 1); + cs_rc = cmap_set_uint8(local_handle, key, 1); free(key); - if (rc != CS_OK) { + if (cs_rc != CS_OK) { pcmk__warn("Could not authorize group with Corosync: %s " QB_XS " group=%u rc=%d", - pcmk_rc_str(pcmk__corosync2rc(rc)), gid, rc); + pcmk_rc_str(pcmk__corosync2rc(cs_rc)), gid, cs_rc); } } } From 3ea4aeaa9a2c465bfc57b5b1c001df5472161387 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 29 Jul 2026 09:56:29 -0400 Subject: [PATCH 43/60] Refactor: daemons: Call cmap_finalize on all paths in pcmkd_read_config. --- daemons/pacemakerd/pcmkd_corosync.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/daemons/pacemakerd/pcmkd_corosync.c b/daemons/pacemakerd/pcmkd_corosync.c index bb406c706f2..79f75866d97 100644 --- a/daemons/pacemakerd/pcmkd_corosync.c +++ b/daemons/pacemakerd/pcmkd_corosync.c @@ -288,6 +288,7 @@ pcmkd_read_config(void) gid_t found_gid = 0; pid_t found_pid = 0; int rv; + bool success = false; enum pcmk_cluster_layer cluster_layer = pcmk_cluster_layer_unknown; const char *cluster_layer_s = NULL; @@ -308,15 +309,14 @@ pcmkd_read_config(void) if (cs_rc != CS_OK) { pcmk__crit("Could not connect to Corosync CMAP: %s " QB_XS " rc=%d", pcmk_rc_str(pcmk__corosync2rc(cs_rc)), cs_rc); - return false; + return success; } cs_rc = cmap_fd_get(local_handle, &fd); if (cs_rc != CS_OK) { pcmk__crit("Could not get Corosync CMAP descriptor: %s " QB_XS " rc=%d", pcmk_rc_str(pcmk__corosync2rc(cs_rc)), cs_rc); - cmap_finalize(local_handle); - return false; + goto done; } /* CMAP provider run as root (in given user namespace, anyway)? */ @@ -327,15 +327,13 @@ pcmkd_read_config(void) "is running as uid %lld gid %lld, not root", (long long) PCMK__SPECIAL_PID_AS_0(found_pid), (long long) found_uid, (long long) found_gid); - cmap_finalize(local_handle); - return false; + goto done; } if (rv < 0) { pcmk__crit("Could not authenticate Corosync CMAP provider: %s " QB_XS " rc=%d", strerror(-rv), -rv); - cmap_finalize(local_handle); - return false; + goto done; } cluster_layer = pcmk_get_cluster_layer(); @@ -345,7 +343,7 @@ pcmkd_read_config(void) pcmk__crit("Expected Corosync cluster layer but detected %s " QB_XS " cluster_layer=%d", cluster_layer_s, cluster_layer); - return false; + goto done; } pcmk__info("Reading configuration for %s cluster layer", cluster_layer_s); @@ -393,6 +391,9 @@ pcmkd_read_config(void) } } + success = true; + +done: cmap_finalize(local_handle); - return true; + return success; } From c643f8f9ddb688acb198a2ea3eee53f7ec9239df Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Fri, 24 Jul 2026 12:34:27 -0400 Subject: [PATCH 44/60] Refactor: daemons: Move pacemakerd cleanup into its own function. This daemon doesn't do a lot of cleanup, but what it does should go into its own function just like all the other daemons. --- daemons/pacemakerd/pacemakerd.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/daemons/pacemakerd/pacemakerd.c b/daemons/pacemakerd/pacemakerd.c index 9a1fee8051b..8d3f325b744 100644 --- a/daemons/pacemakerd/pacemakerd.c +++ b/daemons/pacemakerd/pacemakerd.c @@ -350,6 +350,17 @@ pacemakerd_cleanup_cmdline(void) g_clear_pointer(&context, g_option_context_free); } +static void +pacemakerd_cleanup(void) +{ + pacemakerd_ipc_cleanup(); + pacemakerd_unregister_handlers(); + +#if SUPPORT_COROSYNC + cluster_disconnect_cfg(); +#endif +} + int main(int argc, char **argv) { @@ -496,15 +507,11 @@ main(int argc, char **argv) pcmk__notice("Pacemaker daemon successfully started and accepting " "connections"); g_main_loop_run(mainloop); - pacemakerd_ipc_cleanup(); - pacemakerd_unregister_handlers(); - g_main_loop_unref(mainloop); -#if SUPPORT_COROSYNC - cluster_disconnect_cfg(); -#endif done: + pacemakerd_cleanup(); + pcmk__output_and_clear_error(&error, out); if (out != NULL) { From 2a38d154e9c3f0a0ce3251e5f4e8621507e42653 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Fri, 24 Jul 2026 13:02:07 -0400 Subject: [PATCH 45/60] Refactor: daemons: Simplify a check in pacemakerd_event_cb. --- daemons/pacemakerd/pacemakerd.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/daemons/pacemakerd/pacemakerd.c b/daemons/pacemakerd/pacemakerd.c index 8d3f325b744..110de0f752d 100644 --- a/daemons/pacemakerd/pacemakerd.c +++ b/daemons/pacemakerd/pacemakerd.c @@ -247,12 +247,8 @@ pacemakerd_event_cb(pcmk_ipc_api_t *pacemakerd_api, { pcmk_pacemakerd_api_reply_t *reply = event_data; - switch (event_type) { - case pcmk_ipc_event_reply: - break; - - default: - return; + if (event_type != pcmk_ipc_event_reply) { + return; } if (status != CRM_EX_OK) { From a91bdad12b8c7424d63ca392da1eb3b2583afcc6 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Tue, 28 Jul 2026 14:22:15 -0400 Subject: [PATCH 46/60] Refactor: daemons: Add a comment to start_child. This is just so that as part of my ongoing war on crm_exit in daemons, I remember that it's okay to call it in this spot. We've fork()ed a child process, and so if starting execlp() fails, we want to exit instead of letting the child return and run whatever would happen next. --- daemons/pacemakerd/pcmkd_subdaemons.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/daemons/pacemakerd/pcmkd_subdaemons.c b/daemons/pacemakerd/pcmkd_subdaemons.c index 63152d29df0..45d07a4b662 100644 --- a/daemons/pacemakerd/pcmkd_subdaemons.c +++ b/daemons/pacemakerd/pcmkd_subdaemons.c @@ -537,9 +537,15 @@ start_child(pcmkd_child_t * child) execlp(path, path, (char *) NULL); } + /* If we reach this point, execlp has failed. It's okay to call crm_exit + * here to prevent the fork()ed child process from returning and continuing + * to run. + */ free(path); pcmk__crit("Could not execute subdaemon %s: %s", name, strerror(errno)); crm_exit(CRM_EX_FATAL); + + // Never reached, but makes static analysis happy return pcmk_rc_ok; // Never reached } From 34f114c5baace290e288352e5d21596433fea061 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Tue, 28 Jul 2026 15:03:15 -0400 Subject: [PATCH 47/60] Refactor: daemons: Don't call crm_exit on some easy cases. The only interesting thing happening here is the end of pcmk_shutdown_worker. It's okay to change the return value here because there's no way to reach the end of the function without quitting the main loop. --- daemons/pacemakerd/pacemakerd.c | 13 ++++++++++++- daemons/pacemakerd/pacemakerd.h | 3 ++- daemons/pacemakerd/pcmkd_subdaemons.c | 17 ++++++++++------- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/daemons/pacemakerd/pacemakerd.c b/daemons/pacemakerd/pacemakerd.c index 110de0f752d..1e51ce6e036 100644 --- a/daemons/pacemakerd/pacemakerd.c +++ b/daemons/pacemakerd/pacemakerd.c @@ -339,6 +339,17 @@ handle_old_instance(gboolean shutdown) return rc; } +void +pacemakerd_quit_main_loop(crm_exit_t ec) +{ + /* There's no way to get to this function without the main loop running, + * but check just in case someone adds one in the future + */ + CRM_CHECK((mainloop != NULL) && g_main_loop_is_running(mainloop), return); + + g_main_loop_quit(mainloop); +} + static void pacemakerd_cleanup_cmdline(void) { @@ -503,7 +514,7 @@ main(int argc, char **argv) pcmk__notice("Pacemaker daemon successfully started and accepting " "connections"); g_main_loop_run(mainloop); - g_main_loop_unref(mainloop); + g_clear_pointer(&mainloop, g_main_loop_unref); done: pacemakerd_cleanup(); diff --git a/daemons/pacemakerd/pacemakerd.h b/daemons/pacemakerd/pacemakerd.h index a2d91618bee..bd7eb174cdf 100644 --- a/daemons/pacemakerd/pacemakerd.h +++ b/daemons/pacemakerd/pacemakerd.h @@ -1,5 +1,5 @@ /* - * Copyright 2010-2025 the Pacemaker project contributors + * Copyright 2010-2026 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -29,6 +29,7 @@ int find_and_track_existing_processes(void); gboolean init_children_processes(void *user_data); void pcmk_shutdown(int nsig); void restart_cluster_subdaemons(void); +void pacemakerd_quit_main_loop(crm_exit_t ec); void pacemakerd_ipc_init(void); void pacemakerd_ipc_cleanup(void); diff --git a/daemons/pacemakerd/pcmkd_subdaemons.c b/daemons/pacemakerd/pcmkd_subdaemons.c index 45d07a4b662..19501228704 100644 --- a/daemons/pacemakerd/pcmkd_subdaemons.c +++ b/daemons/pacemakerd/pcmkd_subdaemons.c @@ -107,7 +107,6 @@ static int start_child(pcmkd_child_t *child); static void pcmk_child_exit(mainloop_child_t *p, int core, int signo, int exitcode); static void pcmk_process_exit(pcmkd_child_t *child); -static gboolean pcmk_shutdown_worker(void *user_data); static void stop_child(pcmkd_child_t *child, int signal); static void @@ -224,8 +223,8 @@ check_next_subdaemon(void *user_data) pcmk_process_exit(child); break; default: - crm_exit(CRM_EX_FATAL); - break; /* static analysis/noreturn */ + pacemakerd_quit_main_loop(CRM_EX_FATAL); + return G_SOURCE_REMOVE; } if (++next_child >= PCMK__NELEM(pcmk_children)) { @@ -409,17 +408,21 @@ pcmk_shutdown_worker(void *user_data) return G_SOURCE_CONTINUE; } - g_main_loop_quit(mainloop); - if (fatal_error) { + pacemakerd_quit_main_loop(CRM_EX_FATAL); pcmk__notice("Shutting down and staying down after fatal error"); + #if SUPPORT_COROSYNC + /* @FIXME Should this be moved to pacemakerd_cleanup? This is the only + * caller, so maybe not. + */ pcmkd_shutdown_corosync(); #endif - crm_exit(CRM_EX_FATAL); + } else { + pacemakerd_quit_main_loop(CRM_EX_OK); } - return G_SOURCE_CONTINUE; + return G_SOURCE_REMOVE; } /* TODO once libqb is taught to juggle with IPC end-points carried over as From 76b959f77579c473c958497238fe96dcadca0791 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Fri, 24 Jul 2026 15:37:11 -0400 Subject: [PATCH 48/60] Refactor: daemons: Add pcmk__daemon_t to pacemakerd for exit status. At the moment, we're not even calling pcmk__daemon_init on it, which should be fine because we're not using it for anything else. This is just to make it easier to follow the patches that convert pacemakerd. --- daemons/pacemakerd/pacemakerd.c | 35 +++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/daemons/pacemakerd/pacemakerd.c b/daemons/pacemakerd/pacemakerd.c index 1e51ce6e036..d8665bd5931 100644 --- a/daemons/pacemakerd/pacemakerd.c +++ b/daemons/pacemakerd/pacemakerd.c @@ -39,6 +39,11 @@ #define SUMMARY "pacemakerd - primary Pacemaker daemon that launches and monitors all subsidiary Pacemaker daemons" +pcmk__daemon_t pacemakerd = { + .type = pcmk_ipc_pacemakerd, + .ec = CRM_EX_OK, +}; + struct { gboolean features; gboolean foreground; @@ -372,7 +377,6 @@ int main(int argc, char **argv) { int rc = pcmk_rc_ok; - crm_exit_t exit_code = CRM_EX_OK; GError *error = NULL; @@ -395,14 +399,15 @@ main(int argc, char **argv) pcmk__register_formats(output_group, formats); if (!g_option_context_parse_strv(context, &processed_args, &error)) { - exit_code = CRM_EX_USAGE; + pacemakerd.ec = CRM_EX_USAGE; goto done; } rc = pcmk__output_new(&out, args->output_ty, args->output_dest, argv); if ((rc != pcmk_rc_ok) || (out == NULL)) { - exit_code = CRM_EX_ERROR; - g_set_error(&error, PCMK__EXITC_ERROR, exit_code, "Error creating output format %s: %s", + pacemakerd.ec = CRM_EX_ERROR; + g_set_error(&error, PCMK__EXITC_ERROR, pacemakerd.ec, + "Error creating output format %s: %s", args->output_ty, pcmk_rc_str(rc)); goto done; } @@ -411,7 +416,7 @@ main(int argc, char **argv) if (options.features) { out->message(out, "features"); - exit_code = CRM_EX_OK; + pacemakerd.ec = CRM_EX_OK; goto done; } @@ -430,22 +435,22 @@ main(int argc, char **argv) if ((rc == pcmk_rc_ok) && options.shutdown) { goto done; } else if (rc == pcmk_rc_already) { - exit_code = CRM_EX_FATAL; + pacemakerd.ec = CRM_EX_FATAL; goto done; } else if (rc != pcmk_rc_ok) { - exit_code = pcmk_rc2exitc(rc); + pacemakerd.ec = pcmk_rc2exitc(rc); goto done; } /* Don't allow any accidental output after this point. */ if (out != NULL) { - out->finish(out, exit_code, true, NULL); + out->finish(out, pacemakerd.ec, true, NULL); g_clear_pointer(&out, pcmk__output_free); } #if SUPPORT_COROSYNC if (!pcmkd_read_config()) { - exit_code = CRM_EX_UNAVAILABLE; + pacemakerd.ec = CRM_EX_UNAVAILABLE; goto done; } #endif @@ -466,7 +471,7 @@ main(int argc, char **argv) remove_core_file_limit(); if (create_pcmk_dirs() != pcmk_rc_ok) { - exit_code = CRM_EX_NOUSER; + pacemakerd.ec = CRM_EX_NOUSER; goto done; } pacemakerd_ipc_init(); @@ -474,7 +479,7 @@ main(int argc, char **argv) #if SUPPORT_COROSYNC /* Allows us to block shutdown */ if (!cluster_connect_cfg()) { - exit_code = CRM_EX_PROTOCOL; + pacemakerd.ec = CRM_EX_PROTOCOL; goto done; } #endif @@ -487,10 +492,10 @@ main(int argc, char **argv) case pcmk_rc_ok: break; case pcmk_rc_ipc_unauthorized: - exit_code = CRM_EX_CANTCREAT; + pacemakerd.ec = CRM_EX_CANTCREAT; goto done; default: - exit_code = CRM_EX_FATAL; + pacemakerd.ec = CRM_EX_FATAL; goto done; }; @@ -522,9 +527,9 @@ main(int argc, char **argv) pcmk__output_and_clear_error(&error, out); if (out != NULL) { - out->finish(out, exit_code, true, NULL); + out->finish(out, pacemakerd.ec, true, NULL); pcmk__output_free(out); } pcmk__unregister_formats(); - crm_exit(exit_code); + crm_exit(pacemakerd.ec); } From 870b1e4b535938923d11ebcfd0b666ba377d31cb Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 27 Jul 2026 09:00:28 -0400 Subject: [PATCH 49/60] Refactor: daemons: Finish converting pacemakerd to using pcmk__daemon_t. --- daemons/pacemakerd/pacemakerd.c | 36 +++++++++++---------------- daemons/pacemakerd/pacemakerd.h | 3 +-- daemons/pacemakerd/pcmkd_subdaemons.c | 8 +++--- 3 files changed, 19 insertions(+), 28 deletions(-) diff --git a/daemons/pacemakerd/pacemakerd.c b/daemons/pacemakerd/pacemakerd.c index d8665bd5931..e6565f59afd 100644 --- a/daemons/pacemakerd/pacemakerd.c +++ b/daemons/pacemakerd/pacemakerd.c @@ -344,17 +344,6 @@ handle_old_instance(gboolean shutdown) return rc; } -void -pacemakerd_quit_main_loop(crm_exit_t ec) -{ - /* There's no way to get to this function without the main loop running, - * but check just in case someone adds one in the future - */ - CRM_CHECK((mainloop != NULL) && g_main_loop_is_running(mainloop), return); - - g_main_loop_quit(mainloop); -} - static void pacemakerd_cleanup_cmdline(void) { @@ -394,8 +383,6 @@ main(int argc, char **argv) setenv("LC_ALL", "C", 1); // Ensure logs are in a common language crm_log_preinit(NULL, argc, argv); - mainloop_add_signal(SIGHUP, pcmk_ignore); - mainloop_add_signal(SIGQUIT, pcmk_sigquit); pcmk__register_formats(output_group, formats); if (!g_option_context_parse_strv(context, &processed_args, &error)) { @@ -467,7 +454,6 @@ main(int argc, char **argv) pcmk__notice("Starting Pacemaker " PACEMAKER_VERSION " " QB_XS " build=" BUILD_VERSION " features:" CRM_FEATURES); - mainloop = g_main_loop_new(NULL, FALSE); remove_core_file_limit(); if (create_pcmk_dirs() != pcmk_rc_ok) { @@ -499,9 +485,6 @@ main(int argc, char **argv) goto done; }; - mainloop_add_signal(SIGTERM, pcmk_shutdown); - mainloop_add_signal(SIGINT, pcmk_shutdown); - if ((running_with_sbd) && pcmk__get_sbd_sync_resource_startup()) { pcmk__notice("Waiting for startup-trigger from SBD"); pacemakerd_state = PCMK__VALUE_WAIT_FOR_PING; @@ -516,10 +499,21 @@ main(int argc, char **argv) init_children_processes(NULL); } - pcmk__notice("Pacemaker daemon successfully started and accepting " - "connections"); - g_main_loop_run(mainloop); - g_clear_pointer(&mainloop, g_main_loop_unref); + rc = pcmk__daemon_init(&pacemakerd); + if (rc != pcmk_rc_ok) { + pacemakerd.ec = CRM_EX_ERROR; + g_set_error(&error, PCMK__EXITC_ERROR, pacemakerd.ec, + "Error initializing daemon object: %s", + pcmk_rc_str(rc)); + goto done; + } + + mainloop_add_signal(SIGHUP, pcmk_ignore); + mainloop_add_signal(SIGINT, pcmk_shutdown); + mainloop_add_signal(SIGQUIT, pcmk_sigquit); + mainloop_add_signal(SIGTERM, pcmk_shutdown); + + pcmk__daemon_run(&pacemakerd); done: pacemakerd_cleanup(); diff --git a/daemons/pacemakerd/pacemakerd.h b/daemons/pacemakerd/pacemakerd.h index bd7eb174cdf..68c67b1ad08 100644 --- a/daemons/pacemakerd/pacemakerd.h +++ b/daemons/pacemakerd/pacemakerd.h @@ -16,7 +16,6 @@ #define MAX_RESPAWN 100 -extern GMainLoop *mainloop; extern const char *pacemakerd_state; extern bool running_with_sbd; extern bool shutdown_complete_state_reported_client_closed; @@ -24,12 +23,12 @@ extern unsigned int shutdown_complete_state_reported_to; extern crm_trigger_t *shutdown_trigger; extern crm_trigger_t *startup_trigger; extern time_t subdaemon_check_progress; +extern pcmk__daemon_t pacemakerd; int find_and_track_existing_processes(void); gboolean init_children_processes(void *user_data); void pcmk_shutdown(int nsig); void restart_cluster_subdaemons(void); -void pacemakerd_quit_main_loop(crm_exit_t ec); void pacemakerd_ipc_init(void); void pacemakerd_ipc_cleanup(void); diff --git a/daemons/pacemakerd/pcmkd_subdaemons.c b/daemons/pacemakerd/pcmkd_subdaemons.c index 19501228704..9bd868d8499 100644 --- a/daemons/pacemakerd/pcmkd_subdaemons.c +++ b/daemons/pacemakerd/pcmkd_subdaemons.c @@ -97,8 +97,6 @@ bool shutdown_complete_state_reported_client_closed = false; const char *pacemakerd_state = PCMK__VALUE_INIT; bool running_with_sbd = false; -GMainLoop *mainloop = NULL; - static bool fatal_error = false; static int child_liveness(pcmkd_child_t *child); @@ -223,7 +221,7 @@ check_next_subdaemon(void *user_data) pcmk_process_exit(child); break; default: - pacemakerd_quit_main_loop(CRM_EX_FATAL); + pcmk__daemon_quit(&pacemakerd, CRM_EX_FATAL); return G_SOURCE_REMOVE; } @@ -409,8 +407,8 @@ pcmk_shutdown_worker(void *user_data) } if (fatal_error) { - pacemakerd_quit_main_loop(CRM_EX_FATAL); pcmk__notice("Shutting down and staying down after fatal error"); + pcmk__daemon_quit(&pacemakerd, CRM_EX_FATAL); #if SUPPORT_COROSYNC /* @FIXME Should this be moved to pacemakerd_cleanup? This is the only @@ -419,7 +417,7 @@ pcmk_shutdown_worker(void *user_data) pcmkd_shutdown_corosync(); #endif } else { - pacemakerd_quit_main_loop(CRM_EX_OK); + pcmk__daemon_quit(&pacemakerd, CRM_EX_OK); } return G_SOURCE_REMOVE; From 4d1224b2f258af7ad8d649ec8ad09ca70bbb56af Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 27 Jul 2026 12:35:19 -0400 Subject: [PATCH 50/60] Refactor: libcrmcommon,daemons: Don't exit in pcmk__serve_pacemakerd_ipc. --- daemons/execd/remoted_proxy.c | 5 +++++ daemons/pacemakerd/pacemakerd.c | 6 +++++- daemons/pacemakerd/pacemakerd.h | 2 +- daemons/pacemakerd/pcmkd_ipc.c | 3 ++- lib/common/ipc_server.c | 9 --------- 5 files changed, 13 insertions(+), 12 deletions(-) diff --git a/daemons/execd/remoted_proxy.c b/daemons/execd/remoted_proxy.c index faad5ce9af1..dd1f09881a4 100644 --- a/daemons/execd/remoted_proxy.c +++ b/daemons/execd/remoted_proxy.c @@ -540,6 +540,11 @@ ipc_proxy_init(void) } pcmk__serve_pacemakerd_ipc(&pacemakerd_ipcs, &pacemakerd_proxy_callbacks); + if (pacemakerd_ipcs == NULL) { + execd.ec = CRM_EX_OSERR; + return false; + } + return true; } diff --git a/daemons/pacemakerd/pacemakerd.c b/daemons/pacemakerd/pacemakerd.c index e6565f59afd..b542c34df8f 100644 --- a/daemons/pacemakerd/pacemakerd.c +++ b/daemons/pacemakerd/pacemakerd.c @@ -460,7 +460,11 @@ main(int argc, char **argv) pacemakerd.ec = CRM_EX_NOUSER; goto done; } - pacemakerd_ipc_init(); + + if (!pacemakerd_ipc_init()) { + pacemakerd.ec = CRM_EX_OSERR; + goto done; + } #if SUPPORT_COROSYNC /* Allows us to block shutdown */ diff --git a/daemons/pacemakerd/pacemakerd.h b/daemons/pacemakerd/pacemakerd.h index 68c67b1ad08..658a683bf75 100644 --- a/daemons/pacemakerd/pacemakerd.h +++ b/daemons/pacemakerd/pacemakerd.h @@ -30,7 +30,7 @@ gboolean init_children_processes(void *user_data); void pcmk_shutdown(int nsig); void restart_cluster_subdaemons(void); -void pacemakerd_ipc_init(void); +bool pacemakerd_ipc_init(void); void pacemakerd_ipc_cleanup(void); void pacemakerd_unregister_handlers(void); void pacemakerd_handle_request(pcmk__request_t *request); diff --git a/daemons/pacemakerd/pcmkd_ipc.c b/daemons/pacemakerd/pcmkd_ipc.c index 41d823261c9..ab8a0ac24f3 100644 --- a/daemons/pacemakerd/pcmkd_ipc.c +++ b/daemons/pacemakerd/pcmkd_ipc.c @@ -199,8 +199,9 @@ pacemakerd_ipc_cleanup(void) * \internal * \brief Set up pacemakerd IPC communication */ -void +bool pacemakerd_ipc_init(void) { pcmk__serve_pacemakerd_ipc(&ipcs, &ipc_callbacks); + return ipcs != NULL; } diff --git a/lib/common/ipc_server.c b/lib/common/ipc_server.c index 14d4f2bc729..a414571baff 100644 --- a/lib/common/ipc_server.c +++ b/lib/common/ipc_server.c @@ -1193,8 +1193,6 @@ pcmk__serve_fenced_ipc(qb_ipcs_service_t **ipcs, * * \param[out] ipcs Where to store newly created IPC server * \param[in] cb IPC callbacks - * - * \note This function exits with CRM_EX_OSERR on error. */ void pcmk__serve_pacemakerd_ipc(qb_ipcs_service_t **ipcs, @@ -1210,13 +1208,6 @@ pcmk__serve_pacemakerd_ipc(qb_ipcs_service_t **ipcs, pcmk__server_log_name(pcmk_ipc_pacemakerd)); pcmk__crit("Verify pacemaker and pacemaker_remote are not both " "enabled"); - - /* sub-daemons are observed by pacemakerd. Thus we exit CRM_EX_FATAL - * if we want to prevent pacemakerd from restarting them. - * With pacemakerd we leave the exit-code shown to e.g. systemd - * to what it was prior to moving the code here from pacemakerd.c - */ - crm_exit(CRM_EX_OSERR); } } From 4e87b84f39f188faeebcd30a7e4ac944575eb76a Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 27 Jul 2026 11:32:47 -0400 Subject: [PATCH 51/60] Refactor: daemons: Add an atexit handler for certain cleanup in schedulerd. See a previous commit to attrd for an explanation of this commit. We're just doing the same thing in schedulerd that is now being done in attrd. --- daemons/schedulerd/pacemaker-schedulerd.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/daemons/schedulerd/pacemaker-schedulerd.c b/daemons/schedulerd/pacemaker-schedulerd.c index e3ce0ca7c4a..9ddfc9eac91 100644 --- a/daemons/schedulerd/pacemaker-schedulerd.c +++ b/daemons/schedulerd/pacemaker-schedulerd.c @@ -36,6 +36,8 @@ struct { pcmk__output_t *logger_out = NULL; static pcmk__output_t *out = NULL; +static gchar **processed_args = NULL; +static GOptionContext *context = NULL; static GMainLoop *mainloop = NULL; static crm_exit_t exit_code = CRM_EX_OK; @@ -80,6 +82,14 @@ build_arg_context(pcmk__common_args_t *args, GOptionGroup **group) { return context; } +static void +schedulerd_cleanup_cmdline(void) +{ + g_clear_pointer(&processed_args, g_strfreev); + g_clear_pointer(&context, g_option_context_free); + g_clear_pointer(&options.remainder, g_strfreev); +} + int main(int argc, char **argv) { @@ -87,9 +97,13 @@ main(int argc, char **argv) int rc = pcmk_rc_ok; GOptionGroup *output_group = NULL; - pcmk__common_args_t *args = pcmk__new_common_args(SUMMARY); - gchar **processed_args = pcmk__cmdline_preproc(argv, NULL); - GOptionContext *context = build_arg_context(args, &output_group); + pcmk__common_args_t *args = NULL; + + atexit(schedulerd_cleanup_cmdline); + + args = pcmk__new_common_args(SUMMARY); + processed_args = pcmk__cmdline_preproc(argv, NULL); + context = build_arg_context(args, &output_group); crm_log_preinit(NULL, argc, argv); mainloop_add_signal(SIGTERM, pengine_shutdown); @@ -166,9 +180,6 @@ main(int argc, char **argv) g_main_loop_run(mainloop); done: - g_strfreev(options.remainder); - g_strfreev(processed_args); - pcmk__free_arg_context(context); pcmk__output_and_clear_error(&error, out); pengine_shutdown(0); From 83ebdb4623fec4538883e79792097669daa2b326 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 27 Jul 2026 11:34:58 -0400 Subject: [PATCH 52/60] Refactor: daemons: Move schedulerd cleanup into its own function. This is more in line with what's happening in other daemons. Surprisingly, there's very little to clean up in this one. --- daemons/schedulerd/pacemaker-schedulerd.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/daemons/schedulerd/pacemaker-schedulerd.c b/daemons/schedulerd/pacemaker-schedulerd.c index 9ddfc9eac91..1995027795c 100644 --- a/daemons/schedulerd/pacemaker-schedulerd.c +++ b/daemons/schedulerd/pacemaker-schedulerd.c @@ -90,6 +90,13 @@ schedulerd_cleanup_cmdline(void) g_clear_pointer(&options.remainder, g_strfreev); } +static void +schedulerd_cleanup(void) +{ + schedulerd_ipc_cleanup(); + schedulerd_unregister_handlers(); +} + int main(int argc, char **argv) { @@ -180,6 +187,7 @@ main(int argc, char **argv) g_main_loop_run(mainloop); done: + schedulerd_cleanup(); pcmk__output_and_clear_error(&error, out); pengine_shutdown(0); @@ -188,9 +196,6 @@ main(int argc, char **argv) void pengine_shutdown(int nsig) { - schedulerd_ipc_cleanup(); - schedulerd_unregister_handlers(); - if (logger_out != NULL) { logger_out->finish(logger_out, exit_code, true, NULL); g_clear_pointer(&logger_out, pcmk__output_free); From 7d1a553cbc33af9788a099ce73ab6b7681194d79 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 27 Jul 2026 11:38:18 -0400 Subject: [PATCH 53/60] Refactor: daemons: Standardize how schedulerd exits. * Move everything out of pengine_shutdown and into the bottom of the main function, just like in other daemons. * Rename pengine_shutdown to schedulerd_shutdown and just have it quit the main loop. --- daemons/schedulerd/pacemaker-schedulerd.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/daemons/schedulerd/pacemaker-schedulerd.c b/daemons/schedulerd/pacemaker-schedulerd.c index 1995027795c..8cb04c58927 100644 --- a/daemons/schedulerd/pacemaker-schedulerd.c +++ b/daemons/schedulerd/pacemaker-schedulerd.c @@ -48,8 +48,6 @@ pcmk__supported_format_t formats[] = { { NULL, NULL, NULL } }; -void pengine_shutdown(int nsig); - /* @COMPAT Deprecated since 2.1.8. Use pcmk_list_cluster_options() or * crm_attribute --list-options=cluster instead of querying daemon metadata. * @@ -97,6 +95,12 @@ schedulerd_cleanup(void) schedulerd_unregister_handlers(); } +static void +schedulerd_shutdown(int nsig) +{ + g_main_loop_quit(mainloop); +} + int main(int argc, char **argv) { @@ -113,7 +117,7 @@ main(int argc, char **argv) context = build_arg_context(args, &output_group); crm_log_preinit(NULL, argc, argv); - mainloop_add_signal(SIGTERM, pengine_shutdown); + mainloop_add_signal(SIGTERM, schedulerd_shutdown); pcmk__register_formats(output_group, formats); if (!g_option_context_parse_strv(context, &processed_args, &error)) { @@ -185,17 +189,13 @@ main(int argc, char **argv) pcmk__notice("Pacemaker scheduler successfully started and accepting " "connections"); g_main_loop_run(mainloop); + g_clear_pointer(&mainloop, g_main_loop_unref); done: schedulerd_cleanup(); pcmk__output_and_clear_error(&error, out); - pengine_shutdown(0); -} -void -pengine_shutdown(int nsig) -{ if (logger_out != NULL) { logger_out->finish(logger_out, exit_code, true, NULL); g_clear_pointer(&logger_out, pcmk__output_free); From 596c6de948ed57776b777260f2ad5e2d54087f54 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 27 Jul 2026 11:40:07 -0400 Subject: [PATCH 54/60] Refactor: daemons: Get rid of the options struct in scheduler. --- daemons/schedulerd/pacemaker-schedulerd.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/daemons/schedulerd/pacemaker-schedulerd.c b/daemons/schedulerd/pacemaker-schedulerd.c index 8cb04c58927..8b8ea0b2344 100644 --- a/daemons/schedulerd/pacemaker-schedulerd.c +++ b/daemons/schedulerd/pacemaker-schedulerd.c @@ -29,10 +29,6 @@ #define SUMMARY PCMK__SERVER_SCHEDULERD " - daemon for calculating a " \ "Pacemaker cluster's response to events" -struct { - gchar **remainder; -} options; - pcmk__output_t *logger_out = NULL; static pcmk__output_t *out = NULL; @@ -40,6 +36,7 @@ static gchar **processed_args = NULL; static GOptionContext *context = NULL; static GMainLoop *mainloop = NULL; static crm_exit_t exit_code = CRM_EX_OK; +static gchar **remainder = NULL; pcmk__supported_format_t formats[] = { PCMK__SUPPORTED_FORMAT_NONE, @@ -68,7 +65,7 @@ build_arg_context(pcmk__common_args_t *args, GOptionGroup **group) { GOptionContext *context = NULL; GOptionEntry extra_prog_entries[] = { - { G_OPTION_REMAINING, 0, G_OPTION_FLAG_NONE, G_OPTION_ARG_STRING_ARRAY, &options.remainder, + { G_OPTION_REMAINING, 0, G_OPTION_FLAG_NONE, G_OPTION_ARG_STRING_ARRAY, &remainder, NULL, NULL }, @@ -85,7 +82,7 @@ schedulerd_cleanup_cmdline(void) { g_clear_pointer(&processed_args, g_strfreev); g_clear_pointer(&context, g_option_context_free); - g_clear_pointer(&options.remainder, g_strfreev); + g_clear_pointer(&remainder, g_strfreev); } static void @@ -136,9 +133,9 @@ main(int argc, char **argv) pe__register_messages(out); pcmk__register_lib_messages(out); - if (options.remainder) { - if (g_strv_length(options.remainder) == 1 && - pcmk__str_eq("metadata", options.remainder[0], pcmk__str_casei)) { + if (remainder != NULL) { + if (g_strv_length(remainder) == 1 && + pcmk__str_eq("metadata", remainder[0], pcmk__str_casei)) { rc = scheduler_metadata(out); if (rc != pcmk_rc_ok) { From 80865e84af6130a32d771ca7afb632dcb5fbab36 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 27 Jul 2026 11:42:38 -0400 Subject: [PATCH 55/60] Refactor: daemons: Add pcmk__daemon_t to schedulerd for exit status. At the moment, we're not even calling pcmk__daemon_init on it, which should be fine because we're not using it for anything else. This is just to make it easier to follow the patches that convert schedulerd. --- daemons/schedulerd/pacemaker-schedulerd.c | 33 +++++++++++++---------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/daemons/schedulerd/pacemaker-schedulerd.c b/daemons/schedulerd/pacemaker-schedulerd.c index 8b8ea0b2344..6bf6370ad33 100644 --- a/daemons/schedulerd/pacemaker-schedulerd.c +++ b/daemons/schedulerd/pacemaker-schedulerd.c @@ -29,13 +29,17 @@ #define SUMMARY PCMK__SERVER_SCHEDULERD " - daemon for calculating a " \ "Pacemaker cluster's response to events" +static pcmk__daemon_t schedulerd = { + .type = pcmk_ipc_schedulerd, + .ec = CRM_EX_OK, +}; + pcmk__output_t *logger_out = NULL; static pcmk__output_t *out = NULL; static gchar **processed_args = NULL; static GOptionContext *context = NULL; static GMainLoop *mainloop = NULL; -static crm_exit_t exit_code = CRM_EX_OK; static gchar **remainder = NULL; pcmk__supported_format_t formats[] = { @@ -118,14 +122,15 @@ main(int argc, char **argv) pcmk__register_formats(output_group, formats); if (!g_option_context_parse_strv(context, &processed_args, &error)) { - exit_code = CRM_EX_USAGE; + schedulerd.ec = CRM_EX_USAGE; goto done; } rc = pcmk__output_new(&out, args->output_ty, args->output_dest, argv); if ((rc != pcmk_rc_ok) || (out == NULL)) { - exit_code = CRM_EX_FATAL; - g_set_error(&error, PCMK__EXITC_ERROR, exit_code, "Error creating output format %s: %s", + schedulerd.ec = CRM_EX_FATAL; + g_set_error(&error, PCMK__EXITC_ERROR, schedulerd.ec, + "Error creating output format %s: %s", args->output_ty, pcmk_rc_str(rc)); goto done; } @@ -139,14 +144,14 @@ main(int argc, char **argv) rc = scheduler_metadata(out); if (rc != pcmk_rc_ok) { - exit_code = CRM_EX_FATAL; - g_set_error(&error, PCMK__EXITC_ERROR, exit_code, + schedulerd.ec = CRM_EX_FATAL; + g_set_error(&error, PCMK__EXITC_ERROR, schedulerd.ec, "Unable to display metadata: %s", pcmk_rc_str(rc)); } } else { - exit_code = CRM_EX_USAGE; - g_set_error(&error, PCMK__EXITC_ERROR, exit_code, + schedulerd.ec = CRM_EX_USAGE; + g_set_error(&error, PCMK__EXITC_ERROR, schedulerd.ec, "Unsupported extra command line parameters"); } goto done; @@ -164,8 +169,8 @@ main(int argc, char **argv) if (pcmk__daemon_can_write(PCMK_SCHEDULER_INPUT_DIR, NULL) == FALSE) { pcmk__err("Terminating due to bad permissions on " PCMK_SCHEDULER_INPUT_DIR); - exit_code = CRM_EX_FATAL; - g_set_error(&error, PCMK__EXITC_ERROR, exit_code, + schedulerd.ec = CRM_EX_FATAL; + g_set_error(&error, PCMK__EXITC_ERROR, schedulerd.ec, "ERROR: Bad permissions on %s (see logs for details)", PCMK_SCHEDULER_INPUT_DIR); goto done; @@ -174,7 +179,7 @@ main(int argc, char **argv) schedulerd_ipc_init(); if (pcmk__log_output_new(&logger_out) != pcmk_rc_ok) { - exit_code = CRM_EX_FATAL; + schedulerd.ec = CRM_EX_FATAL; goto done; } pe__register_messages(logger_out); @@ -194,15 +199,15 @@ main(int argc, char **argv) pcmk__output_and_clear_error(&error, out); if (logger_out != NULL) { - logger_out->finish(logger_out, exit_code, true, NULL); + logger_out->finish(logger_out, schedulerd.ec, true, NULL); g_clear_pointer(&logger_out, pcmk__output_free); } if (out != NULL) { - out->finish(out, exit_code, true, NULL); + out->finish(out, schedulerd.ec, true, NULL); g_clear_pointer(&out, pcmk__output_free); } pcmk__unregister_formats(); - crm_exit(exit_code); + crm_exit(schedulerd.ec); } From d7a6dabc330fc56382945ae6c60ad17387668904 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 27 Jul 2026 11:48:01 -0400 Subject: [PATCH 56/60] Refactor: daemons: Finish converting schedulerd to using pcmk__daemon_t. --- daemons/schedulerd/pacemaker-schedulerd.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/daemons/schedulerd/pacemaker-schedulerd.c b/daemons/schedulerd/pacemaker-schedulerd.c index 6bf6370ad33..ed82fbb25a5 100644 --- a/daemons/schedulerd/pacemaker-schedulerd.c +++ b/daemons/schedulerd/pacemaker-schedulerd.c @@ -39,7 +39,6 @@ pcmk__output_t *logger_out = NULL; static pcmk__output_t *out = NULL; static gchar **processed_args = NULL; static GOptionContext *context = NULL; -static GMainLoop *mainloop = NULL; static gchar **remainder = NULL; pcmk__supported_format_t formats[] = { @@ -99,7 +98,7 @@ schedulerd_cleanup(void) static void schedulerd_shutdown(int nsig) { - g_main_loop_quit(mainloop); + pcmk__daemon_quit(&schedulerd, CRM_EX_OK); } int @@ -118,7 +117,6 @@ main(int argc, char **argv) context = build_arg_context(args, &output_group); crm_log_preinit(NULL, argc, argv); - mainloop_add_signal(SIGTERM, schedulerd_shutdown); pcmk__register_formats(output_group, formats); if (!g_option_context_parse_strv(context, &processed_args, &error)) { @@ -186,12 +184,18 @@ main(int argc, char **argv) pcmk__register_lib_messages(logger_out); pcmk__output_set_log_level(logger_out, LOG_TRACE); - /* Create the mainloop and run it... */ - mainloop = g_main_loop_new(NULL, FALSE); - pcmk__notice("Pacemaker scheduler successfully started and accepting " - "connections"); - g_main_loop_run(mainloop); - g_clear_pointer(&mainloop, g_main_loop_unref); + rc = pcmk__daemon_init(&schedulerd); + if (rc != pcmk_rc_ok) { + schedulerd.ec = CRM_EX_ERROR; + g_set_error(&error, PCMK__EXITC_ERROR, schedulerd.ec, + "Error initializing daemon object: %s", + pcmk_rc_str(rc)); + goto done; + } + + mainloop_add_signal(SIGTERM, schedulerd_shutdown); + + pcmk__daemon_run(&schedulerd); done: schedulerd_cleanup(); From 385eba424e8a36d7e69f91e013453eb92f7f53fd Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 27 Jul 2026 11:51:28 -0400 Subject: [PATCH 57/60] Refactor: libcrmcommon,daemons: Don't exit in pcmk__serve_schedulerd_ipc. --- daemons/schedulerd/pacemaker-schedulerd.c | 5 ++++- daemons/schedulerd/pacemaker-schedulerd.h | 4 ++-- daemons/schedulerd/schedulerd_ipc.c | 3 ++- lib/common/ipc_server.c | 4 ---- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/daemons/schedulerd/pacemaker-schedulerd.c b/daemons/schedulerd/pacemaker-schedulerd.c index ed82fbb25a5..31ed94b57e8 100644 --- a/daemons/schedulerd/pacemaker-schedulerd.c +++ b/daemons/schedulerd/pacemaker-schedulerd.c @@ -174,7 +174,10 @@ main(int argc, char **argv) goto done; } - schedulerd_ipc_init(); + if (!schedulerd_ipc_init()) { + schedulerd.ec = CRM_EX_FATAL; + goto done; + } if (pcmk__log_output_new(&logger_out) != pcmk_rc_ok) { schedulerd.ec = CRM_EX_FATAL; diff --git a/daemons/schedulerd/pacemaker-schedulerd.h b/daemons/schedulerd/pacemaker-schedulerd.h index fff31e44a02..009d4d7ba97 100644 --- a/daemons/schedulerd/pacemaker-schedulerd.h +++ b/daemons/schedulerd/pacemaker-schedulerd.h @@ -1,5 +1,5 @@ /* - * Copyright 2004-2025 the Pacemaker project contributors + * Copyright 2004-2026 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -14,7 +14,7 @@ extern pcmk__output_t *logger_out; -void schedulerd_ipc_init(void); +bool schedulerd_ipc_init(void); void schedulerd_ipc_cleanup(void); void schedulerd_unregister_handlers(void); void schedulerd_handle_request(pcmk__request_t *request); diff --git a/daemons/schedulerd/schedulerd_ipc.c b/daemons/schedulerd/schedulerd_ipc.c index 102f4049329..adeba852712 100644 --- a/daemons/schedulerd/schedulerd_ipc.c +++ b/daemons/schedulerd/schedulerd_ipc.c @@ -206,8 +206,9 @@ schedulerd_ipc_cleanup(void) * \internal * \brief Set up schedulerd IPC communication */ -void +bool schedulerd_ipc_init(void) { pcmk__serve_schedulerd_ipc(&ipcs, &ipc_callbacks); + return ipcs != NULL; } diff --git a/lib/common/ipc_server.c b/lib/common/ipc_server.c index a414571baff..dd652625a3e 100644 --- a/lib/common/ipc_server.c +++ b/lib/common/ipc_server.c @@ -1217,9 +1217,6 @@ pcmk__serve_pacemakerd_ipc(qb_ipcs_service_t **ipcs, * * \param[out] ipcs Where to store newly created IPC server * \param[in] cb IPC callbacks - * - * \return Newly created IPC server - * \note This function exits fatally on error. */ void pcmk__serve_schedulerd_ipc(qb_ipcs_service_t **ipcs, @@ -1233,6 +1230,5 @@ pcmk__serve_schedulerd_ipc(qb_ipcs_service_t **ipcs, if (*ipcs == NULL) { pcmk__crit("Failed to create %s IPC server; shutting down", pcmk__server_log_name(pcmk_ipc_schedulerd)); - crm_exit(CRM_EX_FATAL); } } From 28bca873cc506d2652f8f917ca9d01fa06acd3c8 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 27 Jul 2026 12:55:32 -0400 Subject: [PATCH 58/60] Refactor: libcrmcommon,daemons: Don't exit in pcmk__serve_based_ipc. I'm not working on converting based over to using pcmk__daemon_t at the moment, but this can be called from execd which means there's still a path for execd to call crm_exit. So just do the bare minimum to ensure that doesn't happen without touching too much in based. --- daemons/based/based_ipc.c | 4 ++++ daemons/execd/remoted_proxy.c | 4 ++++ lib/common/ipc_server.c | 3 --- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/daemons/based/based_ipc.c b/daemons/based/based_ipc.c index 9e32dfdbd5f..56629b50d3b 100644 --- a/daemons/based/based_ipc.c +++ b/daemons/based/based_ipc.c @@ -306,6 +306,10 @@ based_ipc_init(void) { pcmk__serve_based_ipc(&ipcs_ro, &ipcs_rw, &ipc_ro_callbacks, &ipc_rw_callbacks); + + if ((ipcs_ro == NULL) || (ipcs_rw == NULL)) { + crm_exit(CRM_EX_FATAL); + } } /*! diff --git a/daemons/execd/remoted_proxy.c b/daemons/execd/remoted_proxy.c index dd1f09881a4..8be6223baac 100644 --- a/daemons/execd/remoted_proxy.c +++ b/daemons/execd/remoted_proxy.c @@ -520,6 +520,10 @@ ipc_proxy_init(void) pcmk__serve_based_ipc(&cib_ro, &cib_rw, &cib_proxy_callbacks_ro, &cib_proxy_callbacks_rw); + if ((cib_ro == NULL) || (cib_rw == NULL)) { + execd.ec = CRM_EX_FATAL; + return false; + } pcmk__serve_attrd_ipc(&attrd_ipcs, &attrd_proxy_callbacks); if (attrd_ipcs == NULL) { diff --git a/lib/common/ipc_server.c b/lib/common/ipc_server.c index dd652625a3e..7fdc8841333 100644 --- a/lib/common/ipc_server.c +++ b/lib/common/ipc_server.c @@ -1065,8 +1065,6 @@ pcmk__ipc_send_ack_as(const char *function, int line, pcmk__client_t *c, * \param[out] ipcs_rw New IPC server for read/write CIB manager API * \param[in] ro_cb IPC callbacks for read-only API * \param[in] rw_cb IPC callbacks for read/write and shared-memory APIs - * - * \note This function exits fatally on error. */ void pcmk__serve_based_ipc(qb_ipcs_service_t **ipcs_ro, qb_ipcs_service_t **ipcs_rw, @@ -1087,7 +1085,6 @@ pcmk__serve_based_ipc(qb_ipcs_service_t **ipcs_ro, qb_ipcs_service_t **ipcs_rw, pcmk__server_log_name(pcmk_ipc_based)); pcmk__crit("Verify pacemaker and pacemaker_remote are not both " "enabled"); - crm_exit(CRM_EX_FATAL); } } From 7d2c106ac3e24e8320d3a894c776f29348d8bd25 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 27 Jul 2026 15:13:31 -0400 Subject: [PATCH 59/60] Refactor: daemons: Don't allow a second schedulerd instance to start. For some reason, there was no check for a previous instance of schedulerd. This is easy to add since it already supports pcmk_ipc_api_t, so I might as well. It brings the daemons further in line with each other too. --- daemons/schedulerd/pacemaker-schedulerd.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/daemons/schedulerd/pacemaker-schedulerd.c b/daemons/schedulerd/pacemaker-schedulerd.c index 31ed94b57e8..d421353b1f5 100644 --- a/daemons/schedulerd/pacemaker-schedulerd.c +++ b/daemons/schedulerd/pacemaker-schedulerd.c @@ -29,9 +29,14 @@ #define SUMMARY PCMK__SERVER_SCHEDULERD " - daemon for calculating a " \ "Pacemaker cluster's response to events" +static pcmk__daemon_ipc_fns_t ipc_fns = { + .already_running = pcmk__daemon_ipc_running, +}; + static pcmk__daemon_t schedulerd = { .type = pcmk_ipc_schedulerd, .ec = CRM_EX_OK, + .ipc_fns = &ipc_fns, }; pcmk__output_t *logger_out = NULL; @@ -162,6 +167,16 @@ main(int argc, char **argv) pcmk__cli_init_logging(PCMK__SERVER_SCHEDULERD, args->verbosity); crm_log_init(NULL, LOG_INFO, TRUE, FALSE, argc, argv, FALSE); + + if (schedulerd.ipc_fns->already_running(&schedulerd)) { + schedulerd.ec = CRM_EX_OK; + g_set_error(&error, PCMK__EXITC_ERROR, schedulerd.ec, + "Aborting start-up because a scheduler instance is " + "already active"); + pcmk__crit("%s", error->message); + goto done; + } + pcmk__notice("Starting Pacemaker scheduler"); if (pcmk__daemon_can_write(PCMK_SCHEDULER_INPUT_DIR, NULL) == FALSE) { From e64e8dce449c5456f6d60ceb834495145d3faaf6 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Tue, 28 Jul 2026 12:08:50 -0400 Subject: [PATCH 60/60] Refactor: daemons: Remove unnecessary forward function declarations. --- daemons/execd/execd_commands.c | 1 - daemons/fenced/fenced_commands.c | 1 - 2 files changed, 2 deletions(-) diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c index 488cba0fe6e..67a98a172e5 100644 --- a/daemons/execd/execd_commands.c +++ b/daemons/execd/execd_commands.c @@ -101,7 +101,6 @@ typedef struct { GHashTable *params; } lrmd_cmd_t; -static void cmd_finalize(lrmd_cmd_t * cmd, lrmd_rsc_t * rsc); static gboolean execute_resource_action(void *user_data); static void cancel_all_recurring(lrmd_rsc_t * rsc, const char *client_id); diff --git a/daemons/fenced/fenced_commands.c b/daemons/fenced/fenced_commands.c index 233d8a9acc5..79ac34a4e2a 100644 --- a/daemons/fenced/fenced_commands.c +++ b/daemons/fenced/fenced_commands.c @@ -69,7 +69,6 @@ struct device_search_s { uint32_t support_action_only; }; -static gboolean stonith_device_dispatch(void *user_data); static void st_child_done(int pid, const pcmk__action_result_t *result, void *user_data);