From 6482197daf0ca300d89c5126db95e405a07d4c38 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Mon, 5 Jan 2026 11:32:48 -0800 Subject: [PATCH 01/59] Refactor: libcrmcommon: Include action_relation_internal.h at right spot Signed-off-by: Reid Wahl --- include/crm/common/internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/crm/common/internal.h b/include/crm/common/internal.h index 874cd162a89..25f9731870a 100644 --- a/include/crm/common/internal.h +++ b/include/crm/common/internal.h @@ -12,8 +12,8 @@ #define PCMK__INCLUDED_CRM_COMMON_INTERNAL_H -#include #include +#include #include #include #include From 59653a699e58673dbad1079280dc9d5b6f5a40a5 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 20 Dec 2025 14:13:05 -0800 Subject: [PATCH 02/59] Refactor: based: Drop cib.pam file It was added in 2007 by commit f3ec7ec. No motivation was given, the file doesn't get installed, and it appears that nothing has ever used it. Signed-off-by: Reid Wahl --- daemons/based/Makefile.am | 2 -- daemons/based/cib.pam | 6 ------ 2 files changed, 8 deletions(-) delete mode 100644 daemons/based/cib.pam diff --git a/daemons/based/Makefile.am b/daemons/based/Makefile.am index 44e5ce4a5aa..d7c18906c7b 100644 --- a/daemons/based/Makefile.am +++ b/daemons/based/Makefile.am @@ -10,8 +10,6 @@ include $(top_srcdir)/mk/common.mk include $(top_srcdir)/mk/man.mk -EXTRA_DIST = cib.pam - halibdir = $(CRM_DAEMON_DIR) halib_PROGRAMS = pacemaker-based diff --git a/daemons/based/cib.pam b/daemons/based/cib.pam deleted file mode 100644 index 5d0f6553acc..00000000000 --- a/daemons/based/cib.pam +++ /dev/null @@ -1,6 +0,0 @@ -# login: auth account password session -# may require permission to read /etc/shadow -auth include common-auth -account include common-account -password include common-password -session include common-session From eb16c755029d939d4119db77819103550ec61815 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 20 Dec 2025 14:44:37 -0800 Subject: [PATCH 03/59] Refactor: based: New based_io_init() and make cib_writer static Also rename cib_writer to write_trigger and create a new based_io.h. Signed-off-by: Reid Wahl --- daemons/based/Makefile.am | 3 ++- daemons/based/based_io.c | 22 ++++++++++++++++++---- daemons/based/based_io.h | 15 +++++++++++++++ daemons/based/pacemaker-based.c | 4 ++-- daemons/based/pacemaker-based.h | 4 ++-- 5 files changed, 39 insertions(+), 9 deletions(-) create mode 100644 daemons/based/based_io.h diff --git a/daemons/based/Makefile.am b/daemons/based/Makefile.am index d7c18906c7b..6850b70c496 100644 --- a/daemons/based/Makefile.am +++ b/daemons/based/Makefile.am @@ -14,7 +14,8 @@ halibdir = $(CRM_DAEMON_DIR) halib_PROGRAMS = pacemaker-based -noinst_HEADERS = based_transaction.h +noinst_HEADERS = based_io.h +noinst_HEADERS += based_transaction.h noinst_HEADERS += pacemaker-based.h pacemaker_based_CFLAGS = $(CFLAGS_HARDENED_EXE) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 981db4ffd7b..c4fd5ad0a2e 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -1,5 +1,5 @@ /* - * Copyright 2004-2025 the Pacemaker project contributors + * Copyright 2004-2026 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -36,10 +36,24 @@ #include -crm_trigger_t *cib_writer = NULL; +static crm_trigger_t *write_trigger = NULL; int write_cib_contents(gpointer p); +/*! + * \internal + * \brief Initialize data structures for \c pacemaker-based I/O + * + * Currently there is only one, but this may be expanded later, and the name + * clarifies its purpose in the caller. + */ +void +based_io_init(void) +{ + write_trigger = mainloop_add_trigger(G_PRIORITY_LOW, write_cib_contents, + NULL); +} + static void cib_rename(const char *old) { @@ -306,7 +320,7 @@ activateCibXml(xmlNode *new_cib, bool to_disk, const char *op) pcmk__xml_free(saved_cib); if (cib_writes_enabled && cib_status == pcmk_rc_ok && to_disk) { pcmk__debug("Triggering CIB write for %s op", op); - mainloop_set_trigger(cib_writer); + mainloop_set_trigger(write_trigger); } return pcmk_ok; } @@ -343,7 +357,7 @@ cib_diskwrite_complete(mainloop_child_t * p, pid_t pid, int core, int signo, int ((core != 0)? " and dumped core" : "")); } - mainloop_trigger_complete(cib_writer); + mainloop_trigger_complete(write_trigger); } int diff --git a/daemons/based/based_io.h b/daemons/based/based_io.h new file mode 100644 index 00000000000..c4cd524980c --- /dev/null +++ b/daemons/based/based_io.h @@ -0,0 +1,15 @@ +/* + * 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 BASED_IO__H +#define BASED_IO__H + +void based_io_init(void); + +#endif // BASED_IO__H diff --git a/daemons/based/pacemaker-based.c b/daemons/based/pacemaker-based.c index 41712b43af7..317456d1ea4 100644 --- a/daemons/based/pacemaker-based.c +++ b/daemons/based/pacemaker-based.c @@ -1,5 +1,5 @@ /* - * Copyright 2004-2025 the Pacemaker project contributors + * Copyright 2004-2026 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -208,7 +208,7 @@ main(int argc, char **argv) mainloop_add_signal(SIGTERM, cib_shutdown); mainloop_add_signal(SIGPIPE, cib_enable_writes); - cib_writer = mainloop_add_trigger(G_PRIORITY_LOW, write_cib_contents, NULL); + based_io_init(); if ((g_strv_length(processed_args) >= 2) && pcmk__str_eq(processed_args[1], "metadata", pcmk__str_none)) { diff --git a/daemons/based/pacemaker-based.h b/daemons/based/pacemaker-based.h index 8ef9396f123..5f2ad5b86ee 100644 --- a/daemons/based/pacemaker-based.h +++ b/daemons/based/pacemaker-based.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. * @@ -30,6 +30,7 @@ #include #include +#include "based_io.h" #include "based_transaction.h" #include @@ -48,7 +49,6 @@ enum cib_client_flags { extern bool based_is_primary; extern GHashTable *config_hash; extern xmlNode *the_cib; -extern crm_trigger_t *cib_writer; extern gboolean cib_writes_enabled; extern GMainLoop *mainloop; From 8cb305657fb6be18c7e3cf44dce8aec177ea2567 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 20 Dec 2025 14:47:47 -0800 Subject: [PATCH 04/59] Refactor: based: Make write_cib_contents() static No code changes aside from moving two function definitions to a position above based_io_init() and dropping an extern declaration. Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 212 ++++++++++++++++---------------- daemons/based/pacemaker-based.c | 1 - 2 files changed, 105 insertions(+), 108 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index c4fd5ad0a2e..7a5a14bdf32 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -38,7 +38,111 @@ static crm_trigger_t *write_trigger = NULL; -int write_cib_contents(gpointer p); +static void +cib_diskwrite_complete(mainloop_child_t * p, pid_t pid, int core, int signo, int exitcode) +{ + const char *errmsg = "Could not write CIB to disk"; + + if ((exitcode != 0) && cib_writes_enabled) { + cib_writes_enabled = FALSE; + errmsg = "Disabling CIB disk writes after failure"; + } + + if ((signo == 0) && (exitcode == 0)) { + pcmk__trace("Disk write [%d] succeeded", (int) pid); + + } else if (signo == 0) { + pcmk__err("%s: process %d exited %d", errmsg, (int) pid, exitcode); + + } else { + pcmk__err("%s: process %d terminated with signal %d (%s)%s", + errmsg, (int) pid, signo, strsignal(signo), + ((core != 0)? " and dumped core" : "")); + } + + mainloop_trigger_complete(write_trigger); +} + +static int +write_cib_contents(gpointer p) +{ + int exit_rc = pcmk_ok; + xmlNode *cib_local = NULL; + + /* Make a copy of the CIB to write (possibly in a forked child) */ + if (p) { + /* Synchronous write out */ + cib_local = pcmk__xml_copy(NULL, p); + + } else { + int pid = 0; + int bb_state = qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_STATE_GET, 0); + + /* Turn it off before the fork() to avoid: + * - 2 processes writing to the same shared mem + * - the child needing to disable it + * (which would close it from underneath the parent) + * This way, the shared mem files are already closed + */ + qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_ENABLED, QB_FALSE); + + pid = fork(); + if (pid < 0) { + pcmk__err("Disabling disk writes after fork failure: %s", + pcmk_rc_str(errno)); + cib_writes_enabled = FALSE; + return FALSE; + } + + if (pid) { + /* Parent */ + mainloop_child_add(pid, 0, "disk-writer", NULL, cib_diskwrite_complete); + if (bb_state == QB_LOG_STATE_ENABLED) { + /* Re-enable now that it it safe */ + qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_ENABLED, QB_TRUE); + } + + return -1; /* -1 means 'still work to do' */ + } + + /* Asynchronous write-out after a fork() */ + + /* In theory, we can scribble on the_cib here and not affect the parent, + * but let's be safe anyway. + */ + cib_local = pcmk__xml_copy(NULL, the_cib); + } + + /* Write the CIB */ + exit_rc = cib_file_write_with_digest(cib_local, cib_root, "cib.xml"); + + /* A nonzero exit code will cause further writes to be disabled */ + pcmk__xml_free(cib_local); + if (p == NULL) { + crm_exit_t exit_code = CRM_EX_OK; + + switch (exit_rc) { + case pcmk_ok: + exit_code = CRM_EX_OK; + break; + case pcmk_err_cib_modified: + exit_code = CRM_EX_DIGEST; // Existing CIB doesn't match digest + break; + case pcmk_err_cib_backup: // Existing CIB couldn't be backed up + case pcmk_err_cib_save: // New CIB couldn't be saved + exit_code = CRM_EX_CANTCREAT; + break; + default: + exit_code = CRM_EX_ERROR; + break; + } + + /* Use _exit() because exit() could affect the parent adversely */ + pcmk_common_cleanup(); + _exit(exit_code); + } + return exit_rc; +} /*! * \internal @@ -334,109 +438,3 @@ activateCibXml(xmlNode *new_cib, bool to_disk, const char *op) } return -ENODATA; } - -static void -cib_diskwrite_complete(mainloop_child_t * p, pid_t pid, int core, int signo, int exitcode) -{ - const char *errmsg = "Could not write CIB to disk"; - - if ((exitcode != 0) && cib_writes_enabled) { - cib_writes_enabled = FALSE; - errmsg = "Disabling CIB disk writes after failure"; - } - - if ((signo == 0) && (exitcode == 0)) { - pcmk__trace("Disk write [%d] succeeded", (int) pid); - - } else if (signo == 0) { - pcmk__err("%s: process %d exited %d", errmsg, (int) pid, exitcode); - - } else { - pcmk__err("%s: process %d terminated with signal %d (%s)%s", - errmsg, (int) pid, signo, strsignal(signo), - ((core != 0)? " and dumped core" : "")); - } - - mainloop_trigger_complete(write_trigger); -} - -int -write_cib_contents(gpointer p) -{ - int exit_rc = pcmk_ok; - xmlNode *cib_local = NULL; - - /* Make a copy of the CIB to write (possibly in a forked child) */ - if (p) { - /* Synchronous write out */ - cib_local = pcmk__xml_copy(NULL, p); - - } else { - int pid = 0; - int bb_state = qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_STATE_GET, 0); - - /* Turn it off before the fork() to avoid: - * - 2 processes writing to the same shared mem - * - the child needing to disable it - * (which would close it from underneath the parent) - * This way, the shared mem files are already closed - */ - qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_ENABLED, QB_FALSE); - - pid = fork(); - if (pid < 0) { - pcmk__err("Disabling disk writes after fork failure: %s", - pcmk_rc_str(errno)); - cib_writes_enabled = FALSE; - return FALSE; - } - - if (pid) { - /* Parent */ - mainloop_child_add(pid, 0, "disk-writer", NULL, cib_diskwrite_complete); - if (bb_state == QB_LOG_STATE_ENABLED) { - /* Re-enable now that it it safe */ - qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_ENABLED, QB_TRUE); - } - - return -1; /* -1 means 'still work to do' */ - } - - /* Asynchronous write-out after a fork() */ - - /* In theory, we can scribble on the_cib here and not affect the parent, - * but let's be safe anyway. - */ - cib_local = pcmk__xml_copy(NULL, the_cib); - } - - /* Write the CIB */ - exit_rc = cib_file_write_with_digest(cib_local, cib_root, "cib.xml"); - - /* A nonzero exit code will cause further writes to be disabled */ - pcmk__xml_free(cib_local); - if (p == NULL) { - crm_exit_t exit_code = CRM_EX_OK; - - switch (exit_rc) { - case pcmk_ok: - exit_code = CRM_EX_OK; - break; - case pcmk_err_cib_modified: - exit_code = CRM_EX_DIGEST; // Existing CIB doesn't match digest - break; - case pcmk_err_cib_backup: // Existing CIB couldn't be backed up - case pcmk_err_cib_save: // New CIB couldn't be saved - exit_code = CRM_EX_CANTCREAT; - break; - default: - exit_code = CRM_EX_ERROR; - break; - } - - /* Use _exit() because exit() could affect the parent adversely */ - pcmk_common_cleanup(); - _exit(exit_code); - } - return exit_rc; -} diff --git a/daemons/based/pacemaker-based.c b/daemons/based/pacemaker-based.c index 317456d1ea4..b614ecfe1d8 100644 --- a/daemons/based/pacemaker-based.c +++ b/daemons/based/pacemaker-based.c @@ -50,7 +50,6 @@ GHashTable *config_hash = NULL; static void cib_init(void); void cib_shutdown(int nsig); static bool startCib(const char *filename); -extern int write_cib_contents(gpointer p); static crm_exit_t exit_code = CRM_EX_OK; From ffec81b72c90ebf1fcaa7faf931f744a69a678ab Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 20 Dec 2025 15:19:01 -0800 Subject: [PATCH 05/59] Refactor: based: Unindent code in write_cib_contents() No other code changes, except moving a variable declaration. The p argument is always NULL, though that was not the case when this function was created. Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 109 ++++++++++++++++++--------------------- 1 file changed, 51 insertions(+), 58 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 7a5a14bdf32..2f0284d20f4 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -68,79 +68,72 @@ write_cib_contents(gpointer p) { int exit_rc = pcmk_ok; xmlNode *cib_local = NULL; + crm_exit_t exit_code = CRM_EX_OK; /* Make a copy of the CIB to write (possibly in a forked child) */ - if (p) { - /* Synchronous write out */ - cib_local = pcmk__xml_copy(NULL, p); + int pid = 0; + int bb_state = qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_STATE_GET, 0); + + /* Turn it off before the fork() to avoid: + * - 2 processes writing to the same shared mem + * - the child needing to disable it + * (which would close it from underneath the parent) + * This way, the shared mem files are already closed + */ + qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_ENABLED, QB_FALSE); + + pid = fork(); + if (pid < 0) { + pcmk__err("Disabling disk writes after fork failure: %s", + pcmk_rc_str(errno)); + cib_writes_enabled = FALSE; + return FALSE; + } - } else { - int pid = 0; - int bb_state = qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_STATE_GET, 0); - - /* Turn it off before the fork() to avoid: - * - 2 processes writing to the same shared mem - * - the child needing to disable it - * (which would close it from underneath the parent) - * This way, the shared mem files are already closed - */ - qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_ENABLED, QB_FALSE); - - pid = fork(); - if (pid < 0) { - pcmk__err("Disabling disk writes after fork failure: %s", - pcmk_rc_str(errno)); - cib_writes_enabled = FALSE; - return FALSE; + if (pid) { + /* Parent */ + mainloop_child_add(pid, 0, "disk-writer", NULL, cib_diskwrite_complete); + if (bb_state == QB_LOG_STATE_ENABLED) { + /* Re-enable now that it it safe */ + qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_ENABLED, QB_TRUE); } - if (pid) { - /* Parent */ - mainloop_child_add(pid, 0, "disk-writer", NULL, cib_diskwrite_complete); - if (bb_state == QB_LOG_STATE_ENABLED) { - /* Re-enable now that it it safe */ - qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_ENABLED, QB_TRUE); - } - - return -1; /* -1 means 'still work to do' */ - } + return -1; /* -1 means 'still work to do' */ + } - /* Asynchronous write-out after a fork() */ + /* Asynchronous write-out after a fork() */ - /* In theory, we can scribble on the_cib here and not affect the parent, - * but let's be safe anyway. - */ - cib_local = pcmk__xml_copy(NULL, the_cib); - } + /* In theory, we can scribble on the_cib here and not affect the parent, + * but let's be safe anyway. + */ + cib_local = pcmk__xml_copy(NULL, the_cib); /* Write the CIB */ exit_rc = cib_file_write_with_digest(cib_local, cib_root, "cib.xml"); /* A nonzero exit code will cause further writes to be disabled */ pcmk__xml_free(cib_local); - if (p == NULL) { - crm_exit_t exit_code = CRM_EX_OK; - - switch (exit_rc) { - case pcmk_ok: - exit_code = CRM_EX_OK; - break; - case pcmk_err_cib_modified: - exit_code = CRM_EX_DIGEST; // Existing CIB doesn't match digest - break; - case pcmk_err_cib_backup: // Existing CIB couldn't be backed up - case pcmk_err_cib_save: // New CIB couldn't be saved - exit_code = CRM_EX_CANTCREAT; - break; - default: - exit_code = CRM_EX_ERROR; - break; - } - /* Use _exit() because exit() could affect the parent adversely */ - pcmk_common_cleanup(); - _exit(exit_code); + switch (exit_rc) { + case pcmk_ok: + exit_code = CRM_EX_OK; + break; + case pcmk_err_cib_modified: + exit_code = CRM_EX_DIGEST; // Existing CIB doesn't match digest + break; + case pcmk_err_cib_backup: // Existing CIB couldn't be backed up + case pcmk_err_cib_save: // New CIB couldn't be saved + exit_code = CRM_EX_CANTCREAT; + break; + default: + exit_code = CRM_EX_ERROR; + break; } + + /* Use _exit() because exit() could affect the parent adversely */ + pcmk_common_cleanup(); + _exit(exit_code); + return exit_rc; } From 863f5bb1cd38f53ff5146272474da6604a119e55 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 20 Dec 2025 15:25:00 -0800 Subject: [PATCH 06/59] Refactor: based: Use G_SOURCE_{CONTINUE,REMOVE} in write_cib_contents() Also, drop the return statement after calling _exit(), and rename exit_rc to rc. Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 2f0284d20f4..bd553195df4 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -66,7 +66,7 @@ cib_diskwrite_complete(mainloop_child_t * p, pid_t pid, int core, int signo, int static int write_cib_contents(gpointer p) { - int exit_rc = pcmk_ok; + int rc = pcmk_ok; xmlNode *cib_local = NULL; crm_exit_t exit_code = CRM_EX_OK; @@ -87,7 +87,7 @@ write_cib_contents(gpointer p) pcmk__err("Disabling disk writes after fork failure: %s", pcmk_rc_str(errno)); cib_writes_enabled = FALSE; - return FALSE; + return G_SOURCE_REMOVE; } if (pid) { @@ -98,7 +98,7 @@ write_cib_contents(gpointer p) qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_ENABLED, QB_TRUE); } - return -1; /* -1 means 'still work to do' */ + return G_SOURCE_CONTINUE; } /* Asynchronous write-out after a fork() */ @@ -109,12 +109,12 @@ write_cib_contents(gpointer p) cib_local = pcmk__xml_copy(NULL, the_cib); /* Write the CIB */ - exit_rc = cib_file_write_with_digest(cib_local, cib_root, "cib.xml"); + rc = cib_file_write_with_digest(cib_local, cib_root, "cib.xml"); /* A nonzero exit code will cause further writes to be disabled */ pcmk__xml_free(cib_local); - switch (exit_rc) { + switch (rc) { case pcmk_ok: exit_code = CRM_EX_OK; break; @@ -133,8 +133,6 @@ write_cib_contents(gpointer p) /* Use _exit() because exit() could affect the parent adversely */ pcmk_common_cleanup(); _exit(exit_code); - - return exit_rc; } /*! From 7f3c8273cb515d5a6ca7d2a7201713001201379a Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 20 Dec 2025 15:34:31 -0800 Subject: [PATCH 07/59] Refactor: based: Drop exit_code variable in write_cib_contents() And drop comments that seem redundant. Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index bd553195df4..c17722102e6 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -68,7 +68,6 @@ write_cib_contents(gpointer p) { int rc = pcmk_ok; xmlNode *cib_local = NULL; - crm_exit_t exit_code = CRM_EX_OK; /* Make a copy of the CIB to write (possibly in a forked child) */ int pid = 0; @@ -114,25 +113,23 @@ write_cib_contents(gpointer p) /* A nonzero exit code will cause further writes to be disabled */ pcmk__xml_free(cib_local); + pcmk_common_cleanup(); + + /* Use _exit() because exit() could affect the parent adversely */ switch (rc) { case pcmk_ok: - exit_code = CRM_EX_OK; - break; + _exit(CRM_EX_OK); + case pcmk_err_cib_modified: - exit_code = CRM_EX_DIGEST; // Existing CIB doesn't match digest - break; - case pcmk_err_cib_backup: // Existing CIB couldn't be backed up - case pcmk_err_cib_save: // New CIB couldn't be saved - exit_code = CRM_EX_CANTCREAT; - break; + _exit(CRM_EX_DIGEST); + + case pcmk_err_cib_backup: + case pcmk_err_cib_save: + _exit(CRM_EX_CANTCREAT); + default: - exit_code = CRM_EX_ERROR; - break; + _exit(CRM_EX_ERROR); } - - /* Use _exit() because exit() could affect the parent adversely */ - pcmk_common_cleanup(); - _exit(exit_code); } /*! From bd69ea62a6ffda1f3e3438a13bbc8adf56d0972d Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 20 Dec 2025 15:41:08 -0800 Subject: [PATCH 08/59] Refactor: based: Don't copy the CIB in write_cib_contents() Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index c17722102e6..111770c7ce3 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -67,9 +67,6 @@ static int write_cib_contents(gpointer p) { int rc = pcmk_ok; - xmlNode *cib_local = NULL; - - /* Make a copy of the CIB to write (possibly in a forked child) */ int pid = 0; int bb_state = qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_STATE_GET, 0); @@ -100,22 +97,25 @@ write_cib_contents(gpointer p) return G_SOURCE_CONTINUE; } - /* Asynchronous write-out after a fork() */ - - /* In theory, we can scribble on the_cib here and not affect the parent, - * but let's be safe anyway. + /* Write the CIB. Note that this modifies the_cib, but this child is about + * to exit. The parent's copy of the_cib won't be affected. */ - cib_local = pcmk__xml_copy(NULL, the_cib); - - /* Write the CIB */ - rc = cib_file_write_with_digest(cib_local, cib_root, "cib.xml"); - - /* A nonzero exit code will cause further writes to be disabled */ - pcmk__xml_free(cib_local); + rc = cib_file_write_with_digest(the_cib, cib_root, "cib.xml"); pcmk_common_cleanup(); - /* Use _exit() because exit() could affect the parent adversely */ + /* A nonzero exit code will cause further writes to be disabled. Use _exit() + * because exit() could affect the parent adversely. + * + * @TODO Investigate whether _exit() instead of exit() is really necessary. + * This goes back to commit 58cb43dc, which states that exit() may close + * things it shoudn't close. There is no explanation of what these things + * might be. The exit(2) man page states that exit() calls atexit/on_exit + * handlers and flushes open stdio streams. The exit(3) man page states that + * file created with tmpfile() are removed. But neither Pacemaker nor libqb + * uses atexit or on_exit, and it's not clear why we'd be worried about + * stdio streams. + */ switch (rc) { case pcmk_ok: _exit(CRM_EX_OK); From e71deb8b2afe3a902eb17826e1723e6ff85f96ab Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 20 Dec 2025 15:42:33 -0800 Subject: [PATCH 09/59] Refactor: based: Use pid_t in write_cib_contents() Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 111770c7ce3..c2808f26db7 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -67,7 +67,7 @@ static int write_cib_contents(gpointer p) { int rc = pcmk_ok; - int pid = 0; + pid_t pid = 0; int bb_state = qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_STATE_GET, 0); /* Turn it off before the fork() to avoid: @@ -86,7 +86,7 @@ write_cib_contents(gpointer p) return G_SOURCE_REMOVE; } - if (pid) { + if (pid > 0) { /* Parent */ mainloop_child_add(pid, 0, "disk-writer", NULL, cib_diskwrite_complete); if (bb_state == QB_LOG_STATE_ENABLED) { From 2c428b86088c3d768b6a92bf10ed99c7ce6148e7 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 20 Dec 2025 16:20:07 -0800 Subject: [PATCH 10/59] Refactor: based: Clarify blackbox state save-and-restore somewhat But not much. Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index c2808f26db7..b6cac69964b 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -68,13 +68,13 @@ write_cib_contents(gpointer p) { int rc = pcmk_ok; pid_t pid = 0; - int bb_state = qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_STATE_GET, 0); + int blackbox_state = qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_STATE_GET, 0); - /* Turn it off before the fork() to avoid: - * - 2 processes writing to the same shared mem - * - the child needing to disable it - * (which would close it from underneath the parent) - * This way, the shared mem files are already closed + /* Disable blackbox logging before the fork to avoid two processes writing + * to the same shared memory. The disable should not be done in the child, + * because this would close shared memory files in the parent. + * + * @TODO How? What is meant by this last sentence? */ qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_ENABLED, QB_FALSE); @@ -87,10 +87,10 @@ write_cib_contents(gpointer p) } if (pid > 0) { - /* Parent */ + // Parent mainloop_child_add(pid, 0, "disk-writer", NULL, cib_diskwrite_complete); - if (bb_state == QB_LOG_STATE_ENABLED) { - /* Re-enable now that it it safe */ + + if (blackbox_state == QB_LOG_STATE_ENABLED) { qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_ENABLED, QB_TRUE); } From 9d7ce3bd01f69be56b6d3368223126ad0706229b Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 20 Dec 2025 16:23:07 -0800 Subject: [PATCH 11/59] Refactor: based: Convert to standard return code in write_cib_contents() Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index b6cac69964b..1b37e519f12 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -66,7 +66,7 @@ cib_diskwrite_complete(mainloop_child_t * p, pid_t pid, int core, int signo, int static int write_cib_contents(gpointer p) { - int rc = pcmk_ok; + int rc = pcmk_rc_ok; pid_t pid = 0; int blackbox_state = qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_STATE_GET, 0); @@ -101,6 +101,7 @@ write_cib_contents(gpointer p) * to exit. The parent's copy of the_cib won't be affected. */ rc = cib_file_write_with_digest(the_cib, cib_root, "cib.xml"); + rc = pcmk_legacy2rc(rc); pcmk_common_cleanup(); @@ -117,14 +118,14 @@ write_cib_contents(gpointer p) * stdio streams. */ switch (rc) { - case pcmk_ok: + case pcmk_rc_ok: _exit(CRM_EX_OK); - case pcmk_err_cib_modified: + case pcmk_rc_cib_modified: _exit(CRM_EX_DIGEST); - case pcmk_err_cib_backup: - case pcmk_err_cib_save: + case pcmk_rc_cib_backup: + case pcmk_rc_cib_save: _exit(CRM_EX_CANTCREAT); default: From 38f563b870759f3e8fe60293db94f3eeeba20b1b Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 20 Dec 2025 16:36:18 -0800 Subject: [PATCH 12/59] Refactor: based: Rename write_cib_contents() to write_cib_async() And add Doxygen, and call strerror(errno) since we know it's a system errno and we don't have to worry about a Pacemaker return code. Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 1b37e519f12..8c677b005ee 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -63,8 +63,18 @@ cib_diskwrite_complete(mainloop_child_t * p, pid_t pid, int core, int signo, int mainloop_trigger_complete(write_trigger); } +/*! + * \internal + * \brief Write the CIB to disk in a forked child + * + * This avoids blocking in the parent. The child writes synchronously. The + * parent tracks the child via the mainloop and runs a callback when the child + * exits. + * + * \param[in] user_data Ignored + */ static int -write_cib_contents(gpointer p) +write_cib_async(gpointer user_data) { int rc = pcmk_rc_ok; pid_t pid = 0; @@ -81,7 +91,7 @@ write_cib_contents(gpointer p) pid = fork(); if (pid < 0) { pcmk__err("Disabling disk writes after fork failure: %s", - pcmk_rc_str(errno)); + strerror(errno)); cib_writes_enabled = FALSE; return G_SOURCE_REMOVE; } @@ -143,8 +153,7 @@ write_cib_contents(gpointer p) void based_io_init(void) { - write_trigger = mainloop_add_trigger(G_PRIORITY_LOW, write_cib_contents, - NULL); + write_trigger = mainloop_add_trigger(G_PRIORITY_LOW, write_cib_async, NULL); } static void From 0a90c2b9e619be4be707c39b9da19b5b59814804 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 20 Dec 2025 16:59:34 -0800 Subject: [PATCH 13/59] Refactor: libcrmcommon: Don't pass pid in mainloop child callback It's accessible via the mainloop_child_t object, now that we're exposing its definition internally. We could use the existing mainloop_child_pid() function. However, nothing is using that yet, so I would rather deprecate it in the future. Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 8 ++++---- daemons/execd/remoted_schemas.c | 7 ++++--- daemons/pacemakerd/pcmkd_subdaemons.c | 19 ++++++++++--------- include/crm/common/mainloop.h | 6 ++++-- include/crm/common/mainloop_internal.h | 19 +++++++++++++++++-- lib/common/mainloop.c | 21 +++++---------------- lib/services/services_linux.c | 6 ++---- 7 files changed, 46 insertions(+), 40 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 8c677b005ee..04fe79c25e8 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -39,7 +39,7 @@ static crm_trigger_t *write_trigger = NULL; static void -cib_diskwrite_complete(mainloop_child_t * p, pid_t pid, int core, int signo, int exitcode) +cib_diskwrite_complete(mainloop_child_t *p, int core, int signo, int exitcode) { const char *errmsg = "Could not write CIB to disk"; @@ -49,14 +49,14 @@ cib_diskwrite_complete(mainloop_child_t * p, pid_t pid, int core, int signo, int } if ((signo == 0) && (exitcode == 0)) { - pcmk__trace("Disk write [%d] succeeded", (int) pid); + pcmk__trace("Disk write [%d] succeeded", (int) p->pid); } else if (signo == 0) { - pcmk__err("%s: process %d exited %d", errmsg, (int) pid, exitcode); + pcmk__err("%s: process %d exited %d", errmsg, (int) p->pid, exitcode); } else { pcmk__err("%s: process %d terminated with signal %d (%s)%s", - errmsg, (int) pid, signo, strsignal(signo), + errmsg, (int) p->pid, signo, strsignal(signo), ((core != 0)? " and dumped core" : "")); } diff --git a/daemons/execd/remoted_schemas.c b/daemons/execd/remoted_schemas.c index 40f4fa284b3..da4fece9267 100644 --- a/daemons/execd/remoted_schemas.c +++ b/daemons/execd/remoted_schemas.c @@ -201,7 +201,8 @@ get_schema_files(void) * saving them to disk. */ static void -get_schema_files_complete(mainloop_child_t *p, pid_t pid, int core, int signo, int exitcode) +get_schema_files_complete(mainloop_child_t *p, int core, int signo, + int exitcode) { const char *errmsg = "Could not load additional schema files"; @@ -217,12 +218,12 @@ get_schema_files_complete(mainloop_child_t *p, pid_t pid, int core, int signo, i } else { if (signo == 0) { - pcmk__err("%s: process %lld exited %d", errmsg, (long long) pid, + pcmk__err("%s: process %lld exited %d", errmsg, (long long) p->pid, exitcode); } else { pcmk__err("%s: process %lld terminated with signal %d (%s)%s", - errmsg, (long long) pid, signo, strsignal(signo), + errmsg, (long long) p->pid, signo, strsignal(signo), ((core != 0)? " and dumped core" : "")); } diff --git a/daemons/pacemakerd/pcmkd_subdaemons.c b/daemons/pacemakerd/pcmkd_subdaemons.c index 7e785159cfc..954b922faf4 100644 --- a/daemons/pacemakerd/pcmkd_subdaemons.c +++ b/daemons/pacemakerd/pcmkd_subdaemons.c @@ -104,7 +104,8 @@ static bool fatal_error = false; static int child_liveness(pcmkd_child_t *child); static gboolean escalate_shutdown(gpointer data); static int start_child(pcmkd_child_t *child); -static void pcmk_child_exit(mainloop_child_t * p, pid_t pid, int core, int signo, int exitcode); +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(gpointer user_data); static void stop_child(pcmkd_child_t *child, int signal); @@ -253,7 +254,7 @@ escalate_shutdown(gpointer data) } static void -pcmk_child_exit(mainloop_child_t * p, pid_t pid, int core, int signo, int exitcode) +pcmk_child_exit(mainloop_child_t *p, int core, int signo, int exitcode) { pcmkd_child_t *child = mainloop_child_userdata(p); const char *name = mainloop_child_name(p); @@ -262,7 +263,7 @@ pcmk_child_exit(mainloop_child_t * p, pid_t pid, int core, int signo, int exitco // cts-lab looks for this message do_crm_log(((signo == SIGKILL)? LOG_WARNING : LOG_ERR), "%s[%d] terminated with signal %d (%s)%s", - name, pid, signo, strsignal(signo), + name, p->pid, signo, strsignal(signo), (core? " and dumped core" : "")); pcmk_process_exit(child); return; @@ -270,13 +271,13 @@ pcmk_child_exit(mainloop_child_t * p, pid_t pid, int core, int signo, int exitco switch(exitcode) { case CRM_EX_OK: - pcmk__info("%s[%d] exited with status %d (%s)", name, pid, exitcode, - crm_exit_str(exitcode)); + pcmk__info("%s[%d] exited with status %d (%s)", name, p->pid, + exitcode, crm_exit_str(exitcode)); break; case CRM_EX_FATAL: pcmk__warn("Shutting cluster down because %s[%d] had fatal failure", - name, pid); + name, p->pid); child->flags &= ~child_respawn; fatal_error = true; pcmk_shutdown(SIGTERM); @@ -289,7 +290,7 @@ pcmk_child_exit(mainloop_child_t * p, pid_t pid, int core, int signo, int exitco child->flags &= ~child_respawn; fatal_error = true; msg = pcmk__assert_asprintf("Subdaemon %s[%d] requested panic", - name, pid); + name, p->pid); pcmk__panic(msg); // Should never get here @@ -300,8 +301,8 @@ pcmk_child_exit(mainloop_child_t * p, pid_t pid, int core, int signo, int exitco default: // cts-lab looks for this message - pcmk__err("%s[%d] exited with status %d (%s)", name, pid, exitcode, - crm_exit_str(exitcode)); + pcmk__err("%s[%d] exited with status %d (%s)", name, p->pid, + exitcode, crm_exit_str(exitcode)); break; } diff --git a/include/crm/common/mainloop.h b/include/crm/common/mainloop.h index 30bde33111a..a680c44bd04 100644 --- a/include/crm/common/mainloop.h +++ b/include/crm/common/mainloop.h @@ -171,14 +171,16 @@ void mainloop_child_add(pid_t pid, int timeout, const char *desc, void *userdata, - void (*callback) (mainloop_child_t * p, pid_t pid, int core, int signo, int exitcode)); + void (*callback)(mainloop_child_t *p, int core, + int signo, int exitcode)); void mainloop_child_add_with_flags(pid_t pid, int timeout, const char *desc, void *userdata, enum mainloop_child_flags, - void (*callback) (mainloop_child_t * p, pid_t pid, int core, int signo, int exitcode)); + void (*callback)(mainloop_child_t *p, int core, + int signo, int exitcode)); void *mainloop_child_userdata(mainloop_child_t * child); int mainloop_child_timeout(mainloop_child_t * child); diff --git a/include/crm/common/mainloop_internal.h b/include/crm/common/mainloop_internal.h index 3b5d07ec1d7..05c3f45672e 100644 --- a/include/crm/common/mainloop_internal.h +++ b/include/crm/common/mainloop_internal.h @@ -1,5 +1,5 @@ /* - * Copyright 2015-2026 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,9 @@ #ifndef PCMK__CRM_COMMON_MAINLOOP_INTERNAL__H #define PCMK__CRM_COMMON_MAINLOOP_INTERNAL__H -#include // guint +#include // pid_t + +#include // gboolean, guint #include // crm_ipc_t #include // ipc_client_callbacks, mainloop_* @@ -23,6 +25,19 @@ extern "C" { #endif +struct mainloop_child_s { + pid_t pid; + char *desc; + unsigned timerid; + gboolean timeout; + void *privatedata; + + enum mainloop_child_flags flags; + + /* Called when a process dies */ + void (*callback)(mainloop_child_t *p, int core, int signo, int exitcode); +}; + int pcmk__add_mainloop_ipc(crm_ipc_t *ipc, int priority, void *userdata, const struct ipc_client_callbacks *callbacks, mainloop_io_t **source); diff --git a/lib/common/mainloop.c b/lib/common/mainloop.c index b38ed21dfb7..5652fe89bb7 100644 --- a/lib/common/mainloop.c +++ b/lib/common/mainloop.c @@ -23,19 +23,6 @@ #include -struct mainloop_child_s { - pid_t pid; - char *desc; - unsigned timerid; - gboolean timeout; - void *privatedata; - - enum mainloop_child_flags flags; - - /* Called when a process dies */ - void (*callback) (mainloop_child_t * p, pid_t pid, int core, int signo, int exitcode); -}; - struct trigger_s { GSource source; gboolean running; @@ -1172,7 +1159,7 @@ child_waitpid(mainloop_child_t *child, int flags) } if (callback_needed && child->callback) { - child->callback(child, child->pid, core, signo, exitcode); + child->callback(child, core, signo, exitcode); } return callback_needed; } @@ -1265,7 +1252,8 @@ mainloop_child_kill(pid_t pid) */ void mainloop_child_add_with_flags(pid_t pid, int timeout, const char *desc, void *privatedata, enum mainloop_child_flags flags, - void (*callback) (mainloop_child_t * p, pid_t pid, int core, int signo, int exitcode)) + void (*callback)(mainloop_child_t *p, int core, + int signo, int exitcode)) { static bool need_init = TRUE; mainloop_child_t *child = pcmk__assert_alloc(1, sizeof(mainloop_child_t)); @@ -1296,7 +1284,8 @@ mainloop_child_add_with_flags(pid_t pid, int timeout, const char *desc, void *pr void mainloop_child_add(pid_t pid, int timeout, const char *desc, void *privatedata, - void (*callback) (mainloop_child_t * p, pid_t pid, int core, int signo, int exitcode)) + void (*callback)(mainloop_child_t *p, int core, int signo, + int exitcode)) { mainloop_child_add_with_flags(pid, timeout, desc, privatedata, 0, callback); } diff --git a/lib/services/services_linux.c b/lib/services/services_linux.c index 2c1218a96d1..28057b002e3 100644 --- a/lib/services/services_linux.c +++ b/lib/services/services_linux.c @@ -705,19 +705,17 @@ parse_exit_reason_from_stderr(svc_action_t *op) * \brief Process the completion of an asynchronous child process * * \param[in,out] p Child process that completed - * \param[in] pid Process ID of child * \param[in] core (Unused) * \param[in] signo Signal that interrupted child, if any * \param[in] exitcode Exit status of child process */ static void -async_action_complete(mainloop_child_t *p, pid_t pid, int core, int signo, - int exitcode) +async_action_complete(mainloop_child_t *p, int core, int signo, int exitcode) { svc_action_t *op = mainloop_child_userdata(p); mainloop_clear_child_userdata(p); - CRM_CHECK(op->pid == pid, + CRM_CHECK(op->pid == p->pid, services__set_result(op, services__generic_error(op), PCMK_EXEC_ERROR, "Bug in mainloop handling"); return); From 71b630a717ba363e2c5e0534ba6f56df93a5a781 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 20 Dec 2025 17:13:52 -0800 Subject: [PATCH 14/59] Refactor: based: Best practices in cib_diskwrite_complete() Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 04fe79c25e8..5086c1f921b 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -38,25 +38,35 @@ static crm_trigger_t *write_trigger = NULL; +/*! + * \internal + * \brief Process the exit status of a child forked from \c write_cib_async() + * + * \param[in] child Mainloop child data + * \param[in] core If set to 1, the child process dumped core + * \param[in] signo Signal that the child process exited with + * \param[in] exit_code Child process's exit code + */ static void -cib_diskwrite_complete(mainloop_child_t *p, int core, int signo, int exitcode) +write_cib_cb(mainloop_child_t *child, int core, int signo, int exit_code) { - const char *errmsg = "Could not write CIB to disk"; + const char *error = "Could not write CIB to disk"; - if ((exitcode != 0) && cib_writes_enabled) { + if ((exit_code != 0) && cib_writes_enabled) { cib_writes_enabled = FALSE; - errmsg = "Disabling CIB disk writes after failure"; + error = "Disabling CIB disk writes after failure"; } - if ((signo == 0) && (exitcode == 0)) { - pcmk__trace("Disk write [%d] succeeded", (int) p->pid); + if ((signo == 0) && (exit_code == 0)) { + pcmk__trace("Disk write [%lld] succeeded", (long long) child->pid); } else if (signo == 0) { - pcmk__err("%s: process %d exited %d", errmsg, (int) p->pid, exitcode); + pcmk__err("%s: process %lld exited with code %d", error, + (long long) child->pid, exit_code); } else { - pcmk__err("%s: process %d terminated with signal %d (%s)%s", - errmsg, (int) p->pid, signo, strsignal(signo), + pcmk__err("%s: process %lld terminated with signal %d (%s)%s", + error, (long long) child->pid, signo, strsignal(signo), ((core != 0)? " and dumped core" : "")); } @@ -98,7 +108,7 @@ write_cib_async(gpointer user_data) if (pid > 0) { // Parent - mainloop_child_add(pid, 0, "disk-writer", NULL, cib_diskwrite_complete); + mainloop_child_add(pid, 0, "disk-writer", NULL, write_cib_cb); if (blackbox_state == QB_LOG_STATE_ENABLED) { qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_ENABLED, QB_TRUE); From 0b216df119d18c4aabed50249805c0db8cc47b02 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 20 Dec 2025 22:23:27 -0800 Subject: [PATCH 15/59] Refactor: based: Make cib_writes_enabled static to based_io.c Also use bool instead of gboolean. The --disk-writes command line option seems unnecessary, but we can deal with that later. But see T227. Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 34 +++++++++++++++++++++++---------- daemons/based/based_io.h | 1 + daemons/based/pacemaker-based.c | 22 ++++++++++----------- daemons/based/pacemaker-based.h | 1 - 4 files changed, 35 insertions(+), 23 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 5086c1f921b..2f8748563d4 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -36,6 +36,7 @@ #include +static bool writes_enabled = true; static crm_trigger_t *write_trigger = NULL; /*! @@ -52,8 +53,8 @@ write_cib_cb(mainloop_child_t *child, int core, int signo, int exit_code) { const char *error = "Could not write CIB to disk"; - if ((exit_code != 0) && cib_writes_enabled) { - cib_writes_enabled = FALSE; + if ((exit_code != 0) && writes_enabled) { + writes_enabled = false; error = "Disabling CIB disk writes after failure"; } @@ -102,7 +103,7 @@ write_cib_async(gpointer user_data) if (pid < 0) { pcmk__err("Disabling disk writes after fork failure: %s", strerror(errno)); - cib_writes_enabled = FALSE; + writes_enabled = false; return G_SOURCE_REMOVE; } @@ -155,14 +156,27 @@ write_cib_async(gpointer user_data) /*! * \internal - * \brief Initialize data structures for \c pacemaker-based I/O + * \brief Enable CIB writes to disk (signal handler) * - * Currently there is only one, but this may be expanded later, and the name - * clarifies its purpose in the caller. + * \param[in] nsig Ignored + */ +void +based_enable_writes(int nsig) +{ + pcmk__info("(Re)enabling disk writes"); + writes_enabled = true; +} + +/*! + * \internal + * \brief Initialize data structures for \c pacemaker-based I/O */ void based_io_init(void) { + writes_enabled = !stand_alone; + mainloop_add_signal(SIGPIPE, based_enable_writes); + write_trigger = mainloop_add_trigger(G_PRIORITY_LOW, write_cib_async, NULL); } @@ -179,7 +193,7 @@ cib_rename(const char *old) pcmk__err("Couldn't archive unusable file %s (disabling disk writes " "and continuing)", old); - cib_writes_enabled = FALSE; + writes_enabled = false; } else { pcmk__err("Archived unusable file %s as %s", old, new); } @@ -350,11 +364,11 @@ readCibXmlFile(const char *dir, const char *file, bool discard_status) pcmk__warn("Continuing with an empty configuration"); } - if (cib_writes_enabled + if (writes_enabled && pcmk__env_option_enabled(PCMK__SERVER_BASED, PCMK__ENV_VALGRIND_ENABLED)) { - cib_writes_enabled = FALSE; + writes_enabled = false; pcmk__err("*** Disabling disk writes to avoid confusing Valgrind ***"); } @@ -430,7 +444,7 @@ activateCibXml(xmlNode *new_cib, bool to_disk, const char *op) pcmk__assert(new_cib != saved_cib); the_cib = new_cib; pcmk__xml_free(saved_cib); - if (cib_writes_enabled && cib_status == pcmk_rc_ok && to_disk) { + if (to_disk && writes_enabled && (cib_status == pcmk_rc_ok)) { pcmk__debug("Triggering CIB write for %s op", op); mainloop_set_trigger(write_trigger); } diff --git a/daemons/based/based_io.h b/daemons/based/based_io.h index c4cd524980c..84806aa43d9 100644 --- a/daemons/based/based_io.h +++ b/daemons/based/based_io.h @@ -11,5 +11,6 @@ #define BASED_IO__H void based_io_init(void); +void based_enable_writes(int nsig); #endif // BASED_IO__H diff --git a/daemons/based/pacemaker-based.c b/daemons/based/pacemaker-based.c index b614ecfe1d8..4efb88915d6 100644 --- a/daemons/based/pacemaker-based.c +++ b/daemons/based/pacemaker-based.c @@ -39,7 +39,6 @@ GMainLoop *mainloop = NULL; gchar *cib_root = NULL; static bool preserve_status = false; -gboolean cib_writes_enabled = TRUE; gboolean stand_alone = FALSE; int remote_fd = 0; @@ -53,13 +52,6 @@ static bool startCib(const char *filename); static crm_exit_t exit_code = CRM_EX_OK; -static void -cib_enable_writes(int nsig) -{ - pcmk__info("(Re)enabling disk writes"); - cib_writes_enabled = TRUE; -} - /*! * \internal * \brief Set up options, users, and groups for stand-alone mode @@ -76,7 +68,6 @@ setup_stand_alone(GError **error) int rc = pcmk_rc_ok; preserve_status = true; - cib_writes_enabled = FALSE; rc = pcmk__daemon_user(&uid, &gid); if (rc != pcmk_rc_ok) { @@ -134,12 +125,20 @@ based_metadata(pcmk__output_t *out) pcmk__opt_based); } +static gboolean +disk_writes_cb(const gchar *option_name, const gchar *optarg, gpointer data, + GError **error) +{ + based_enable_writes(0); + return TRUE; +} + static GOptionEntry entries[] = { { "stand-alone", 's', G_OPTION_FLAG_NONE, G_OPTION_ARG_NONE, &stand_alone, "(Advanced use only) Run in stand-alone mode", NULL }, - { "disk-writes", 'w', G_OPTION_FLAG_NONE, G_OPTION_ARG_NONE, - &cib_writes_enabled, + { "disk-writes", 'w', G_OPTION_FLAG_NO_ARG, G_OPTION_ARG_CALLBACK, + disk_writes_cb, "(Advanced use only) Enable disk writes (enabled by default unless in " "stand-alone mode)", NULL }, @@ -205,7 +204,6 @@ main(int argc, char **argv) } mainloop_add_signal(SIGTERM, cib_shutdown); - mainloop_add_signal(SIGPIPE, cib_enable_writes); based_io_init(); diff --git a/daemons/based/pacemaker-based.h b/daemons/based/pacemaker-based.h index 5f2ad5b86ee..ad8ddc9d12f 100644 --- a/daemons/based/pacemaker-based.h +++ b/daemons/based/pacemaker-based.h @@ -49,7 +49,6 @@ enum cib_client_flags { extern bool based_is_primary; extern GHashTable *config_hash; extern xmlNode *the_cib; -extern gboolean cib_writes_enabled; extern GMainLoop *mainloop; extern pcmk_cluster_t *crm_cluster; From 1cbff2138253dcb855721cc9051efb3e9143d722 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 20 Dec 2025 22:37:18 -0800 Subject: [PATCH 16/59] Refactor: based: Don't compare against FALSE Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 2f8748563d4..3bc3cc1c69a 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -310,8 +310,10 @@ readCibXmlFile(const char *dir, const char *file, bool discard_status) xmlNode *status = NULL; sigfile = pcmk__assert_asprintf("%s.sig", file); - if (pcmk__daemon_can_write(dir, file) == FALSE - || pcmk__daemon_can_write(dir, sigfile) == FALSE) { + + if (!pcmk__daemon_can_write(dir, file) + || !pcmk__daemon_can_write(dir, sigfile)) { + cib_status = EACCES; return NULL; } From c3fa5b755b028301b1e6734edd4a324255acacc3 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 20 Dec 2025 22:48:32 -0800 Subject: [PATCH 17/59] Refactor: based: Unindent retrieveCib() Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 3bc3cc1c69a..d947a51873e 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -216,14 +216,15 @@ retrieveCib(const char *filename, const char *sigfile) if (rc == pcmk_ok) { pcmk__info("Loaded CIB from %s (with digest %s)", filename, sigfile); - } else { - pcmk__warn("Continuing but NOT using CIB from %s (with digest %s): %s", - filename, sigfile, pcmk_strerror(rc)); - if (rc == -pcmk_err_cib_modified) { - // Archive the original files so the contents are not lost - cib_rename(filename); - cib_rename(sigfile); - } + return root; + } + + pcmk__warn("Continuing but NOT using CIB from %s (with digest %s): %s", + filename, sigfile, pcmk_strerror(rc)); + if (rc == -pcmk_err_cib_modified) { + // Archive the original files so the contents are not lost + cib_rename(filename); + cib_rename(sigfile); } return root; } From 6244cb384a574d3745f8eccef622dcaed59c88b9 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 20 Dec 2025 22:56:52 -0800 Subject: [PATCH 18/59] Refactor: based: Clarify variable names in readCibXmlFile() Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index d947a51873e..d91d1d6dabf 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -296,37 +296,37 @@ static int cib_archive_sort(const struct dirent ** a, const struct dirent **b) } xmlNode * -readCibXmlFile(const char *dir, const char *file, bool discard_status) +readCibXmlFile(const char *dir, const char *cibfile, bool discard_status) { struct dirent **namelist = NULL; int lpc = 0; + char *cibfile_path = NULL; char *sigfile = NULL; - char *sigfilepath = NULL; - char *filename = NULL; + char *sigfile_path = NULL; const char *name = NULL; const char *value = NULL; xmlNode *root = NULL; xmlNode *status = NULL; - sigfile = pcmk__assert_asprintf("%s.sig", file); + sigfile = pcmk__assert_asprintf("%s.sig", cibfile); - if (!pcmk__daemon_can_write(dir, file) + if (!pcmk__daemon_can_write(dir, cibfile) || !pcmk__daemon_can_write(dir, sigfile)) { cib_status = EACCES; return NULL; } - filename = pcmk__assert_asprintf("%s/%s", dir, file); - sigfilepath = pcmk__assert_asprintf("%s/%s", dir, sigfile); + cibfile_path = pcmk__assert_asprintf("%s/%s", dir, cibfile); + sigfile_path = pcmk__assert_asprintf("%s/%s", dir, sigfile); free(sigfile); cib_status = pcmk_rc_ok; - root = retrieveCib(filename, sigfilepath); - free(filename); - free(sigfilepath); + root = retrieveCib(cibfile_path, sigfile_path); + free(cibfile_path); + free(sigfile_path); if (root == NULL) { lpc = scandir(cib_root, &namelist, cib_archive_filter, cib_archive_sort); @@ -341,24 +341,23 @@ readCibXmlFile(const char *dir, const char *file, bool discard_status) lpc--; - filename = pcmk__assert_asprintf("%s/%s", cib_root, - namelist[lpc]->d_name); - sigfile = pcmk__assert_asprintf("%s.sig", filename); + cibfile_path = pcmk__assert_asprintf("%s/%s", cib_root, + namelist[lpc]->d_name); + sigfile_path = pcmk__assert_asprintf("%s.sig", cibfile_path); - rc = cib_file_read_and_verify(filename, sigfile, &root); + rc = cib_file_read_and_verify(cibfile_path, sigfile_path, &root); if (rc == pcmk_ok) { pcmk__notice("Loaded CIB from last valid backup %s (with digest " - "%s)", - filename, sigfile); + "%s)", cibfile_path, sigfile_path); } else { pcmk__warn("Not using next most recent CIB backup from %s (with " - "digest %s): %s", - filename, sigfile, pcmk_strerror(rc)); + "digest %s): %s", cibfile_path, sigfile_path, + pcmk_strerror(rc)); } free(namelist[lpc]); - free(filename); - free(sigfile); + free(cibfile_path); + free(sigfile_path); } free(namelist); From c955771e2d5833097c05995d5c026ca793a4f59c Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 20 Dec 2025 22:57:59 -0800 Subject: [PATCH 19/59] Refactor: based: Clarify argument names in retrieveCib() Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index d91d1d6dabf..96691f3670e 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -209,22 +209,23 @@ cib_rename(const char *old) */ static xmlNode * -retrieveCib(const char *filename, const char *sigfile) +retrieveCib(const char *cibfile_path, const char *sigfile_path) { xmlNode *root = NULL; - int rc = cib_file_read_and_verify(filename, sigfile, &root); + int rc = cib_file_read_and_verify(cibfile_path, sigfile_path, &root); if (rc == pcmk_ok) { - pcmk__info("Loaded CIB from %s (with digest %s)", filename, sigfile); + pcmk__info("Loaded CIB from %s (with digest %s)", cibfile_path, + sigfile_path); return root; } pcmk__warn("Continuing but NOT using CIB from %s (with digest %s): %s", - filename, sigfile, pcmk_strerror(rc)); + cibfile_path, sigfile_path, pcmk_strerror(rc)); if (rc == -pcmk_err_cib_modified) { // Archive the original files so the contents are not lost - cib_rename(filename); - cib_rename(sigfile); + cib_rename(cibfile_path); + cib_rename(sigfile_path); } return root; } From 3b6ff1ae74d2a2b80506ce0cb1bd836c50d1dd82 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 20 Dec 2025 23:02:28 -0800 Subject: [PATCH 20/59] Refactor: based: Move more functionality into retrieveCib() This function handles the case "try to get the CIB from the requested file first." Then we'll try the directory, and finally we'll fall back to creating an empty CIB. I want to make readCibXmlFile() shorter and reduce the number of times we're allocating and freeing variables there, which is confusing. Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 49 +++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 96691f3670e..500eb38950c 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -209,24 +209,44 @@ cib_rename(const char *old) */ static xmlNode * -retrieveCib(const char *cibfile_path, const char *sigfile_path) +retrieveCib(const char *dir, const char *cibfile) { + char *sigfile = pcmk__assert_asprintf("%s.sig", cibfile); + char *cibfile_path = pcmk__assert_asprintf("%s/%s", dir, cibfile); + char *sigfile_path = pcmk__assert_asprintf("%s/%s", dir, sigfile); + xmlNode *root = NULL; - int rc = cib_file_read_and_verify(cibfile_path, sigfile_path, &root); + int rc = pcmk_ok; + + if (!pcmk__daemon_can_write(dir, cibfile) + || !pcmk__daemon_can_write(dir, sigfile)) { + + cib_status = EACCES; + goto done; + } + cib_status = pcmk_rc_ok; + + rc = cib_file_read_and_verify(cibfile_path, sigfile_path, &root); if (rc == pcmk_ok) { pcmk__info("Loaded CIB from %s (with digest %s)", cibfile_path, sigfile_path); - return root; + goto done; } pcmk__warn("Continuing but NOT using CIB from %s (with digest %s): %s", cibfile_path, sigfile_path, pcmk_strerror(rc)); + if (rc == -pcmk_err_cib_modified) { // Archive the original files so the contents are not lost cib_rename(cibfile_path); cib_rename(sigfile_path); } + +done: + free(sigfile); + free(cibfile_path); + free(sigfile_path); return root; } @@ -302,32 +322,13 @@ readCibXmlFile(const char *dir, const char *cibfile, bool discard_status) struct dirent **namelist = NULL; int lpc = 0; - char *cibfile_path = NULL; - char *sigfile = NULL; - char *sigfile_path = NULL; const char *name = NULL; const char *value = NULL; xmlNode *root = NULL; xmlNode *status = NULL; - sigfile = pcmk__assert_asprintf("%s.sig", cibfile); - - if (!pcmk__daemon_can_write(dir, cibfile) - || !pcmk__daemon_can_write(dir, sigfile)) { - - cib_status = EACCES; - return NULL; - } - - cibfile_path = pcmk__assert_asprintf("%s/%s", dir, cibfile); - sigfile_path = pcmk__assert_asprintf("%s/%s", dir, sigfile); - free(sigfile); - - cib_status = pcmk_rc_ok; - root = retrieveCib(cibfile_path, sigfile_path); - free(cibfile_path); - free(sigfile_path); + root = retrieveCib(dir, cibfile); if (root == NULL) { lpc = scandir(cib_root, &namelist, cib_archive_filter, cib_archive_sort); @@ -338,6 +339,8 @@ readCibXmlFile(const char *dir, const char *cibfile, bool discard_status) } while (root == NULL && lpc > 1) { + char *cibfile_path = NULL; + char *sigfile_path = NULL; int rc = pcmk_ok; lpc--; From a3dd84399ec4750e4c43c5f83c45699ac3021daa Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 00:31:37 -0800 Subject: [PATCH 21/59] Refactor: based: Drop startCib() argument Signed-off-by: Reid Wahl --- daemons/based/pacemaker-based.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/daemons/based/pacemaker-based.c b/daemons/based/pacemaker-based.c index 4efb88915d6..d75a14c0afc 100644 --- a/daemons/based/pacemaker-based.c +++ b/daemons/based/pacemaker-based.c @@ -48,7 +48,7 @@ GHashTable *config_hash = NULL; static void cib_init(void); void cib_shutdown(int nsig); -static bool startCib(const char *filename); +static bool startCib(void); static crm_exit_t exit_code = CRM_EX_OK; @@ -381,7 +381,7 @@ cib_init(void) config_hash = pcmk__strkey_table(free, free); - if (!startCib("cib.xml")) { + if (!startCib()) { pcmk__crit("Cannot start CIB... terminating"); crm_exit(CRM_EX_NOINPUT); } @@ -404,9 +404,9 @@ cib_init(void) } static bool -startCib(const char *filename) +startCib(void) { - xmlNode *cib = readCibXmlFile(cib_root, filename, !preserve_status); + xmlNode *cib = readCibXmlFile(cib_root, "cib.xml", !preserve_status); int port = 0; if (activateCibXml(cib, true, "start") != 0) { From c86f35077d02fc9bc9f047571eeea3b6bfa37bcc Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 00:47:06 -0800 Subject: [PATCH 22/59] Refactor: based: Drop readCibXmlFile() cibfile argument The sole call sets it to "cib.xml". Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 4 ++-- daemons/based/based_io.h | 5 +++++ daemons/based/pacemaker-based.c | 2 +- daemons/based/pacemaker-based.h | 1 - 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 500eb38950c..a517fe169a1 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -317,7 +317,7 @@ static int cib_archive_sort(const struct dirent ** a, const struct dirent **b) } xmlNode * -readCibXmlFile(const char *dir, const char *cibfile, bool discard_status) +based_read_cib(const char *dir, bool discard_status) { struct dirent **namelist = NULL; @@ -328,7 +328,7 @@ readCibXmlFile(const char *dir, const char *cibfile, bool discard_status) xmlNode *root = NULL; xmlNode *status = NULL; - root = retrieveCib(dir, cibfile); + root = retrieveCib(dir, "cib.xml"); if (root == NULL) { lpc = scandir(cib_root, &namelist, cib_archive_filter, cib_archive_sort); diff --git a/daemons/based/based_io.h b/daemons/based/based_io.h index 84806aa43d9..dd15edd3810 100644 --- a/daemons/based/based_io.h +++ b/daemons/based/based_io.h @@ -10,7 +10,12 @@ #ifndef BASED_IO__H #define BASED_IO__H +#include + +#include // xmlNode + void based_io_init(void); void based_enable_writes(int nsig); +xmlNode *based_read_cib(const char *dir, bool discard_status); #endif // BASED_IO__H diff --git a/daemons/based/pacemaker-based.c b/daemons/based/pacemaker-based.c index d75a14c0afc..55635436495 100644 --- a/daemons/based/pacemaker-based.c +++ b/daemons/based/pacemaker-based.c @@ -406,7 +406,7 @@ cib_init(void) static bool startCib(void) { - xmlNode *cib = readCibXmlFile(cib_root, "cib.xml", !preserve_status); + xmlNode *cib = based_read_cib(cib_root, !preserve_status); int port = 0; if (activateCibXml(cib, true, "start") != 0) { diff --git a/daemons/based/pacemaker-based.h b/daemons/based/pacemaker-based.h index ad8ddc9d12f..a16485320b8 100644 --- a/daemons/based/pacemaker-based.h +++ b/daemons/based/pacemaker-based.h @@ -75,7 +75,6 @@ void cib_shutdown(int nsig); void terminate_cib(int exit_status); void uninitializeCib(void); -xmlNode *readCibXmlFile(const char *dir, const char *file, bool discard_status); int activateCibXml(xmlNode *doc, bool to_disk, const char *op); int cib_process_shutdown_req(const char *op, int options, const char *section, From b3a66ca9368332e53b80ee88c9c0e0f2b8d190f0 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 00:49:45 -0800 Subject: [PATCH 23/59] Refactor: based: Drop based_read_cib() discard_status argument It's always set to !preserve_status, and preserve_status is always set to the same value as stand_alone. So we can use !stand_alone in place of discard_status. Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 6 +++--- daemons/based/based_io.h | 4 +--- daemons/based/pacemaker-based.c | 5 +---- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index a517fe169a1..2cd467f4a32 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -317,7 +317,7 @@ static int cib_archive_sort(const struct dirent ** a, const struct dirent **b) } xmlNode * -based_read_cib(const char *dir, bool discard_status) +based_read_cib(const char *dir) { struct dirent **namelist = NULL; @@ -379,7 +379,7 @@ based_read_cib(const char *dir, bool discard_status) } status = pcmk__xe_first_child(root, PCMK_XE_STATUS, NULL, NULL); - if (discard_status && status != NULL) { + if (!stand_alone && (status != NULL)) { // Strip out the PCMK_XE_STATUS section if there is one pcmk__xml_free(status); status = NULL; @@ -414,7 +414,7 @@ based_read_cib(const char *dir, bool discard_status) // Unset (DC should set appropriate value) pcmk__xe_remove_attr(root, PCMK_XA_DC_UUID); - if (discard_status) { + if (!stand_alone) { pcmk__log_xml_trace(root, "[on-disk]"); } diff --git a/daemons/based/based_io.h b/daemons/based/based_io.h index dd15edd3810..8f4a502523a 100644 --- a/daemons/based/based_io.h +++ b/daemons/based/based_io.h @@ -10,12 +10,10 @@ #ifndef BASED_IO__H #define BASED_IO__H -#include - #include // xmlNode void based_io_init(void); void based_enable_writes(int nsig); -xmlNode *based_read_cib(const char *dir, bool discard_status); +xmlNode *based_read_cib(const char *dir); #endif // BASED_IO__H diff --git a/daemons/based/pacemaker-based.c b/daemons/based/pacemaker-based.c index 55635436495..3bc75be1927 100644 --- a/daemons/based/pacemaker-based.c +++ b/daemons/based/pacemaker-based.c @@ -37,7 +37,6 @@ pcmk_cluster_t *crm_cluster = NULL; GMainLoop *mainloop = NULL; gchar *cib_root = NULL; -static bool preserve_status = false; gboolean stand_alone = FALSE; @@ -67,8 +66,6 @@ setup_stand_alone(GError **error) gid_t gid = 0; int rc = pcmk_rc_ok; - preserve_status = true; - rc = pcmk__daemon_user(&uid, &gid); if (rc != pcmk_rc_ok) { exit_code = CRM_EX_FATAL; @@ -406,7 +403,7 @@ cib_init(void) static bool startCib(void) { - xmlNode *cib = based_read_cib(cib_root, !preserve_status); + xmlNode *cib = based_read_cib(cib_root); int port = 0; if (activateCibXml(cib, true, "start") != 0) { From d741bd4dcb5817a3c1491377144f64aefe5c121d Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 00:53:09 -0800 Subject: [PATCH 24/59] Refactor: based: Drop retrieveCib() cibfile argument The sole call sets it to "cib.xml". Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 2cd467f4a32..711128a1780 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -208,17 +208,19 @@ cib_rename(const char *old) * It is the callers responsibility to free the output of this function */ +#define CIBFILE "cib.xml" + static xmlNode * -retrieveCib(const char *dir, const char *cibfile) +retrieveCib(const char *dir) { - char *sigfile = pcmk__assert_asprintf("%s.sig", cibfile); - char *cibfile_path = pcmk__assert_asprintf("%s/%s", dir, cibfile); + char *sigfile = pcmk__assert_asprintf("%s.sig", CIBFILE); + char *cibfile_path = pcmk__assert_asprintf("%s/%s", dir, CIBFILE); char *sigfile_path = pcmk__assert_asprintf("%s/%s", dir, sigfile); xmlNode *root = NULL; int rc = pcmk_ok; - if (!pcmk__daemon_can_write(dir, cibfile) + if (!pcmk__daemon_can_write(dir, CIBFILE) || !pcmk__daemon_can_write(dir, sigfile)) { cib_status = EACCES; @@ -328,7 +330,7 @@ based_read_cib(const char *dir) xmlNode *root = NULL; xmlNode *status = NULL; - root = retrieveCib(dir, "cib.xml"); + root = retrieveCib(dir); if (root == NULL) { lpc = scandir(cib_root, &namelist, cib_archive_filter, cib_archive_sort); From 3f0458598f4d99aaaefafeabf118e58b9d79ef14 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 01:00:01 -0800 Subject: [PATCH 25/59] Refactor: based: Drop based_read_cib() and retrieveCib() dir arguments They're always set to cib_root. Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 14 +++++++------- daemons/based/based_io.h | 2 +- daemons/based/pacemaker-based.c | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 711128a1780..b43a9219b44 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -211,17 +211,17 @@ cib_rename(const char *old) #define CIBFILE "cib.xml" static xmlNode * -retrieveCib(const char *dir) +retrieveCib(void) { char *sigfile = pcmk__assert_asprintf("%s.sig", CIBFILE); - char *cibfile_path = pcmk__assert_asprintf("%s/%s", dir, CIBFILE); - char *sigfile_path = pcmk__assert_asprintf("%s/%s", dir, sigfile); + char *cibfile_path = pcmk__assert_asprintf("%s/%s", cib_root, CIBFILE); + char *sigfile_path = pcmk__assert_asprintf("%s/%s", cib_root, sigfile); xmlNode *root = NULL; int rc = pcmk_ok; - if (!pcmk__daemon_can_write(dir, CIBFILE) - || !pcmk__daemon_can_write(dir, sigfile)) { + if (!pcmk__daemon_can_write(cib_root, CIBFILE) + || !pcmk__daemon_can_write(cib_root, sigfile)) { cib_status = EACCES; goto done; @@ -319,7 +319,7 @@ static int cib_archive_sort(const struct dirent ** a, const struct dirent **b) } xmlNode * -based_read_cib(const char *dir) +based_read_cib(void) { struct dirent **namelist = NULL; @@ -330,7 +330,7 @@ based_read_cib(const char *dir) xmlNode *root = NULL; xmlNode *status = NULL; - root = retrieveCib(dir); + root = retrieveCib(); if (root == NULL) { lpc = scandir(cib_root, &namelist, cib_archive_filter, cib_archive_sort); diff --git a/daemons/based/based_io.h b/daemons/based/based_io.h index 8f4a502523a..2c3fab0ffea 100644 --- a/daemons/based/based_io.h +++ b/daemons/based/based_io.h @@ -14,6 +14,6 @@ void based_io_init(void); void based_enable_writes(int nsig); -xmlNode *based_read_cib(const char *dir); +xmlNode *based_read_cib(void); #endif // BASED_IO__H diff --git a/daemons/based/pacemaker-based.c b/daemons/based/pacemaker-based.c index 3bc75be1927..28662d690c0 100644 --- a/daemons/based/pacemaker-based.c +++ b/daemons/based/pacemaker-based.c @@ -403,7 +403,7 @@ cib_init(void) static bool startCib(void) { - xmlNode *cib = based_read_cib(cib_root); + xmlNode *cib = based_read_cib(); int port = 0; if (activateCibXml(cib, true, "start") != 0) { From fdbd2b96621f411a128a4f2c4422579791913bbc Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 19:24:10 -0800 Subject: [PATCH 26/59] Refactor: based: Make cib_archive_sort() behavior match comment cib_archive_sort() sorts directory entries in the reverse order from the one documented. The code still worked as intended, because the while loop iterated in reverse order. This commit sorts directory entries from newest to oldest and then iterates starting at index 0. Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index b43a9219b44..f0c6b2d6983 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -306,10 +306,12 @@ static int cib_archive_sort(const struct dirent ** a, const struct dirent **b) free(a_path); free(b_path); - if(a_age > b_age) { - rc = 1; - } else if(a_age < b_age) { + if (a_age > b_age) { + // a newer than b rc = -1; + } else if (a_age < b_age) { + // a older than b + rc = 1; } pcmk__trace("%s (%lu) vs. %s (%lu) : %d", @@ -323,7 +325,7 @@ based_read_cib(void) { struct dirent **namelist = NULL; - int lpc = 0; + int num_files = 0; const char *name = NULL; const char *value = NULL; @@ -333,22 +335,21 @@ based_read_cib(void) root = retrieveCib(); if (root == NULL) { - lpc = scandir(cib_root, &namelist, cib_archive_filter, cib_archive_sort); - if (lpc < 0) { + num_files = scandir(cib_root, &namelist, cib_archive_filter, + cib_archive_sort); + if (num_files < 0) { pcmk__err("Could not check for CIB backups in %s: %s", cib_root, pcmk_rc_str(errno)); } } - while (root == NULL && lpc > 1) { + for (int i = 0; i < num_files; i++) { char *cibfile_path = NULL; char *sigfile_path = NULL; int rc = pcmk_ok; - lpc--; - cibfile_path = pcmk__assert_asprintf("%s/%s", cib_root, - namelist[lpc]->d_name); + namelist[i]->d_name); sigfile_path = pcmk__assert_asprintf("%s.sig", cibfile_path); rc = cib_file_read_and_verify(cibfile_path, sigfile_path, &root); @@ -361,9 +362,13 @@ based_read_cib(void) pcmk_strerror(rc)); } - free(namelist[lpc]); + free(namelist[i]); free(cibfile_path); free(sigfile_path); + + if (rc == pcmk_ok) { + break; + } } free(namelist); From 1108e260719cbb229b882c143d7d4a3823f8859f Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 01:28:34 -0800 Subject: [PATCH 27/59] Refactor: based: Functionize finding and reading a backup CIB file Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 69 +++++++++++++++++++++++++--------------- 1 file changed, 43 insertions(+), 26 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index f0c6b2d6983..d091717b050 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -320,39 +320,34 @@ static int cib_archive_sort(const struct dirent ** a, const struct dirent **b) return rc; } -xmlNode * -based_read_cib(void) +/*! + * \internal + * \brief Read CIB XML from the last valid backup file in \c cib_root + * + * \return CIB XML parsed from the last valid backup file, or \c NULL if none + * was found + */ +static xmlNode * +read_backup_cib(void) { + xmlNode *cib_xml = NULL; struct dirent **namelist = NULL; - - int num_files = 0; - const char *name = NULL; - const char *value = NULL; - - xmlNode *root = NULL; - xmlNode *status = NULL; - - root = retrieveCib(); - - if (root == NULL) { - num_files = scandir(cib_root, &namelist, cib_archive_filter, + int num_files = scandir(cib_root, &namelist, cib_archive_filter, cib_archive_sort); - if (num_files < 0) { - pcmk__err("Could not check for CIB backups in %s: %s", cib_root, - pcmk_rc_str(errno)); - } + + if (num_files < 0) { + pcmk__err("Could not check for CIB backups in %s: %s", cib_root, + pcmk_rc_str(errno)); + goto done; } for (int i = 0; i < num_files; i++) { - char *cibfile_path = NULL; - char *sigfile_path = NULL; - int rc = pcmk_ok; + const char *cibfile = namelist[i]->d_name; + char *cibfile_path = pcmk__assert_asprintf("%s/%s", cib_root, cibfile); + char *sigfile_path = pcmk__assert_asprintf("%s.sig", cibfile_path); - cibfile_path = pcmk__assert_asprintf("%s/%s", cib_root, - namelist[i]->d_name); - sigfile_path = pcmk__assert_asprintf("%s.sig", cibfile_path); + int rc = cib_file_read_and_verify(cibfile_path, sigfile_path, &cib_xml); - rc = cib_file_read_and_verify(cibfile_path, sigfile_path, &root); if (rc == pcmk_ok) { pcmk__notice("Loaded CIB from last valid backup %s (with digest " "%s)", cibfile_path, sigfile_path); @@ -362,7 +357,6 @@ based_read_cib(void) pcmk_strerror(rc)); } - free(namelist[i]); free(cibfile_path); free(sigfile_path); @@ -370,8 +364,31 @@ based_read_cib(void) break; } } + +done: + for (int i = 0; i < num_files; i++) { + free(namelist[i]); + } free(namelist); + return cib_xml; +} + +xmlNode * +based_read_cib(void) +{ + const char *name = NULL; + const char *value = NULL; + + xmlNode *root = NULL; + xmlNode *status = NULL; + + root = retrieveCib(); + + if (root == NULL) { + root = read_backup_cib(); + } + if (root == NULL) { root = createEmptyCib(0); pcmk__warn("Continuing with an empty configuration"); From f636cd77ca8ecd3565fc8366a67b801e2d72c9ae Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 01:32:55 -0800 Subject: [PATCH 28/59] Refactor: based: Rename retrieveCib() to read_current_cib() And add Doxygen. Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index d091717b050..4be92a9a558 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -204,14 +204,20 @@ cib_rename(const char *old) free(new); } -/* - * It is the callers responsibility to free the output of this function - */ - #define CIBFILE "cib.xml" +/*! + * \internal + * \brief Read CIB XML from \c CIBFILE in the \c cib_root directory + * + * \return CIB XML parsed from \c CIBFILE in \c cib_root , or \c NULL if the + * file was not found or if parsing failed + * + * \note The caller is responsible for freeing the return value using + * \c pcmk__xml_free(). + */ static xmlNode * -retrieveCib(void) +read_current_cib(void) { char *sigfile = pcmk__assert_asprintf("%s.sig", CIBFILE); char *cibfile_path = pcmk__assert_asprintf("%s/%s", cib_root, CIBFILE); @@ -383,7 +389,7 @@ based_read_cib(void) xmlNode *root = NULL; xmlNode *status = NULL; - root = retrieveCib(); + root = read_current_cib(); if (root == NULL) { root = read_backup_cib(); From 107e8f1b813388d0cc6e6796360673a93d124555 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 14:54:44 -0800 Subject: [PATCH 29/59] Refactor: based: Don't allocate sigfile string Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 4be92a9a558..238da64dc0f 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -219,9 +219,9 @@ cib_rename(const char *old) static xmlNode * read_current_cib(void) { - char *sigfile = pcmk__assert_asprintf("%s.sig", CIBFILE); char *cibfile_path = pcmk__assert_asprintf("%s/%s", cib_root, CIBFILE); - char *sigfile_path = pcmk__assert_asprintf("%s/%s", cib_root, sigfile); + char *sigfile_path = pcmk__assert_asprintf("%s.sig", cibfile_path); + const char *sigfile = strrchr(sigfile_path, '/') + 1; xmlNode *root = NULL; int rc = pcmk_ok; @@ -252,7 +252,6 @@ read_current_cib(void) } done: - free(sigfile); free(cibfile_path); free(sigfile_path); return root; From e48b6499b2c20bb7bd55465ece84a32f25be3c45 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 01:37:12 -0800 Subject: [PATCH 30/59] Refactor: based: Rename root variable to cib_xml in based_read_cib() ...and in read_current_cib(). Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 42 ++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 238da64dc0f..00ac427f9bf 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -223,7 +223,7 @@ read_current_cib(void) char *sigfile_path = pcmk__assert_asprintf("%s.sig", cibfile_path); const char *sigfile = strrchr(sigfile_path, '/') + 1; - xmlNode *root = NULL; + xmlNode *cib_xml = NULL; int rc = pcmk_ok; if (!pcmk__daemon_can_write(cib_root, CIBFILE) @@ -235,7 +235,7 @@ read_current_cib(void) cib_status = pcmk_rc_ok; - rc = cib_file_read_and_verify(cibfile_path, sigfile_path, &root); + rc = cib_file_read_and_verify(cibfile_path, sigfile_path, &cib_xml); if (rc == pcmk_ok) { pcmk__info("Loaded CIB from %s (with digest %s)", cibfile_path, sigfile_path); @@ -254,7 +254,7 @@ read_current_cib(void) done: free(cibfile_path); free(sigfile_path); - return root; + return cib_xml; } static int cib_archive_filter(const struct dirent * a) @@ -385,17 +385,17 @@ based_read_cib(void) const char *name = NULL; const char *value = NULL; - xmlNode *root = NULL; + xmlNode *cib_xml = NULL; xmlNode *status = NULL; - root = read_current_cib(); + cib_xml = read_current_cib(); - if (root == NULL) { - root = read_backup_cib(); + if (cib_xml == NULL) { + cib_xml = read_backup_cib(); } - if (root == NULL) { - root = createEmptyCib(0); + if (cib_xml == NULL) { + cib_xml = createEmptyCib(0); pcmk__warn("Continuing with an empty configuration"); } @@ -407,50 +407,50 @@ based_read_cib(void) pcmk__err("*** Disabling disk writes to avoid confusing Valgrind ***"); } - status = pcmk__xe_first_child(root, PCMK_XE_STATUS, NULL, NULL); + status = pcmk__xe_first_child(cib_xml, PCMK_XE_STATUS, NULL, NULL); if (!stand_alone && (status != NULL)) { // Strip out the PCMK_XE_STATUS section if there is one pcmk__xml_free(status); status = NULL; } if (status == NULL) { - pcmk__xe_create(root, PCMK_XE_STATUS); + pcmk__xe_create(cib_xml, PCMK_XE_STATUS); } /* Do this before schema validation happens */ /* fill in some defaults */ - value = pcmk__xe_get(root, PCMK_XA_ADMIN_EPOCH); + value = pcmk__xe_get(cib_xml, PCMK_XA_ADMIN_EPOCH); if (value == NULL) { // Not possible with schema validation enabled pcmk__warn("Defaulting missing " PCMK_XA_ADMIN_EPOCH " to 0, but " "cluster may get confused about which node's configuration " "is most recent"); - pcmk__xe_set_int(root, PCMK_XA_ADMIN_EPOCH, 0); + pcmk__xe_set_int(cib_xml, PCMK_XA_ADMIN_EPOCH, 0); } name = PCMK_XA_EPOCH; - value = pcmk__xe_get(root, name); + value = pcmk__xe_get(cib_xml, name); if (value == NULL) { - pcmk__xe_set_int(root, name, 0); + pcmk__xe_set_int(cib_xml, name, 0); } name = PCMK_XA_NUM_UPDATES; - value = pcmk__xe_get(root, name); + value = pcmk__xe_get(cib_xml, name); if (value == NULL) { - pcmk__xe_set_int(root, name, 0); + pcmk__xe_set_int(cib_xml, name, 0); } // Unset (DC should set appropriate value) - pcmk__xe_remove_attr(root, PCMK_XA_DC_UUID); + pcmk__xe_remove_attr(cib_xml, PCMK_XA_DC_UUID); if (!stand_alone) { - pcmk__log_xml_trace(root, "[on-disk]"); + pcmk__log_xml_trace(cib_xml, "[on-disk]"); } - if (!pcmk__configured_schema_validates(root)) { + if (!pcmk__configured_schema_validates(cib_xml)) { cib_status = pcmk_rc_schema_validation; } - return root; + return cib_xml; } void From b87b110dcd27780d261cb0df626adf18b03cdbef Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 14:58:46 -0800 Subject: [PATCH 31/59] Refactor: based: Convert to standard RC in read_{current,backup}_cib() Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 00ac427f9bf..5300d65a811 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -224,7 +224,7 @@ read_current_cib(void) const char *sigfile = strrchr(sigfile_path, '/') + 1; xmlNode *cib_xml = NULL; - int rc = pcmk_ok; + int rc = pcmk_rc_ok; if (!pcmk__daemon_can_write(cib_root, CIBFILE) || !pcmk__daemon_can_write(cib_root, sigfile)) { @@ -236,16 +236,18 @@ read_current_cib(void) cib_status = pcmk_rc_ok; rc = cib_file_read_and_verify(cibfile_path, sigfile_path, &cib_xml); - if (rc == pcmk_ok) { + rc = pcmk_legacy2rc(rc); + + if (rc == pcmk_rc_ok) { pcmk__info("Loaded CIB from %s (with digest %s)", cibfile_path, sigfile_path); goto done; } pcmk__warn("Continuing but NOT using CIB from %s (with digest %s): %s", - cibfile_path, sigfile_path, pcmk_strerror(rc)); + cibfile_path, sigfile_path, pcmk_rc_str(rc)); - if (rc == -pcmk_err_cib_modified) { + if (rc == pcmk_rc_cib_modified) { // Archive the original files so the contents are not lost cib_rename(cibfile_path); cib_rename(sigfile_path); @@ -353,19 +355,21 @@ read_backup_cib(void) int rc = cib_file_read_and_verify(cibfile_path, sigfile_path, &cib_xml); - if (rc == pcmk_ok) { + rc = pcmk_legacy2rc(rc); + + if (rc == pcmk_rc_ok) { pcmk__notice("Loaded CIB from last valid backup %s (with digest " "%s)", cibfile_path, sigfile_path); } else { pcmk__warn("Not using next most recent CIB backup from %s (with " "digest %s): %s", cibfile_path, sigfile_path, - pcmk_strerror(rc)); + pcmk_rc_str(rc)); } free(cibfile_path); free(sigfile_path); - if (rc == pcmk_ok) { + if (rc == pcmk_rc_ok) { break; } } From 87c25986f14bf2658d63afb9ebc186ccfda30949 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 01:38:53 -0800 Subject: [PATCH 32/59] Refactor: based: Drop name/value variables from based_read_cib() Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 5300d65a811..466369f275a 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -386,9 +386,6 @@ read_backup_cib(void) xmlNode * based_read_cib(void) { - const char *name = NULL; - const char *value = NULL; - xmlNode *cib_xml = NULL; xmlNode *status = NULL; @@ -421,39 +418,46 @@ based_read_cib(void) pcmk__xe_create(cib_xml, PCMK_XE_STATUS); } - /* Do this before schema validation happens */ + /* Default the three version attributes to 0 if unset. The schema requires + * them to be set, so: + * * It's not possible for them to be unset if schema validation was enabled + * when the CIB file was generated, or if it was generated by Pacemaker + * and then unmodified. + * * We need to set these defaults before schema validation happens. + */ - /* fill in some defaults */ - value = pcmk__xe_get(cib_xml, PCMK_XA_ADMIN_EPOCH); - if (value == NULL) { // Not possible with schema validation enabled + if (pcmk__xe_get(cib_xml, PCMK_XA_ADMIN_EPOCH) == NULL) { pcmk__warn("Defaulting missing " PCMK_XA_ADMIN_EPOCH " to 0, but " "cluster may get confused about which node's configuration " "is most recent"); pcmk__xe_set_int(cib_xml, PCMK_XA_ADMIN_EPOCH, 0); } - name = PCMK_XA_EPOCH; - value = pcmk__xe_get(cib_xml, name); - if (value == NULL) { - pcmk__xe_set_int(cib_xml, name, 0); + if (pcmk__xe_get(cib_xml, PCMK_XA_EPOCH) == NULL) { + pcmk__warn("Defaulting missing " PCMK_XA_EPOCH " to 0, but cluster " + "may get confused about which node's configuration is most " + "recent"); + pcmk__xe_set_int(cib_xml, PCMK_XA_EPOCH, 0); } - name = PCMK_XA_NUM_UPDATES; - value = pcmk__xe_get(cib_xml, name); - if (value == NULL) { - pcmk__xe_set_int(cib_xml, name, 0); + if (pcmk__xe_get(cib_xml, PCMK_XA_NUM_UPDATES) == NULL) { + pcmk__warn("Defaulting missing " PCMK_XA_NUM_UPDATES " to 0, but " + "cluster may get confused about which node's configuration " + "is most recent"); + pcmk__xe_set_int(cib_xml, PCMK_XA_NUM_UPDATES, 0); } - // Unset (DC should set appropriate value) + // The DC should set appropriate value for PCMK_XA_DC_UUID pcmk__xe_remove_attr(cib_xml, PCMK_XA_DC_UUID); if (!stand_alone) { - pcmk__log_xml_trace(cib_xml, "[on-disk]"); + pcmk__log_xml_trace(cib_xml, "on-disk"); } if (!pcmk__configured_schema_validates(cib_xml)) { cib_status = pcmk_rc_schema_validation; } + return cib_xml; } From 34b86b62687ac8aa4b572fd6769691e2686e1793 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 03:00:35 -0800 Subject: [PATCH 33/59] Refactor: based: Functionize setting default version attributes Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 47 +++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 466369f275a..ddb65b1c44c 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -383,9 +383,34 @@ read_backup_cib(void) return cib_xml; } +/*! + * \internal + * \brief Set the given CIB version attribute to 0 if it's not already set + * + * \param[in,out] cib_xml CIB XML + * \param[in] version_attr Version attribute + */ +static void +set_default_if_unset(xmlNode *cib_xml, const char *version_attr) +{ + if (pcmk__xe_get(cib_xml, version_attr) != NULL) { + return; + } + + pcmk__warn("Defaulting missing %s to 0, but cluster may get confused about " + "which node's configuration is most recent", version_attr); + pcmk__xe_set_int(cib_xml, version_attr, 0); +} + xmlNode * based_read_cib(void) { + static const char *version_attrs[] = { + PCMK_XA_ADMIN_EPOCH, + PCMK_XA_EPOCH, + PCMK_XA_NUM_UPDATES, + }; + xmlNode *cib_xml = NULL; xmlNode *status = NULL; @@ -425,26 +450,8 @@ based_read_cib(void) * and then unmodified. * * We need to set these defaults before schema validation happens. */ - - if (pcmk__xe_get(cib_xml, PCMK_XA_ADMIN_EPOCH) == NULL) { - pcmk__warn("Defaulting missing " PCMK_XA_ADMIN_EPOCH " to 0, but " - "cluster may get confused about which node's configuration " - "is most recent"); - pcmk__xe_set_int(cib_xml, PCMK_XA_ADMIN_EPOCH, 0); - } - - if (pcmk__xe_get(cib_xml, PCMK_XA_EPOCH) == NULL) { - pcmk__warn("Defaulting missing " PCMK_XA_EPOCH " to 0, but cluster " - "may get confused about which node's configuration is most " - "recent"); - pcmk__xe_set_int(cib_xml, PCMK_XA_EPOCH, 0); - } - - if (pcmk__xe_get(cib_xml, PCMK_XA_NUM_UPDATES) == NULL) { - pcmk__warn("Defaulting missing " PCMK_XA_NUM_UPDATES " to 0, but " - "cluster may get confused about which node's configuration " - "is most recent"); - pcmk__xe_set_int(cib_xml, PCMK_XA_NUM_UPDATES, 0); + for (int i = 0; i < PCMK__NELEM(version_attrs); i++) { + set_default_if_unset(cib_xml, version_attrs[i]); } // The DC should set appropriate value for PCMK_XA_DC_UUID From 874c3437a544b99257f97ccefa885175b1d7e093 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 03:18:55 -0800 Subject: [PATCH 34/59] Refactor: based: Functionize setting empty status element Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 41 +++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index ddb65b1c44c..842499a7c7d 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -383,6 +383,32 @@ read_backup_cib(void) return cib_xml; } +/*! + * \internal + * \brief Set the CIB XML's \c PCMK_XE_STATUS element to empty if appropriate + * + * Delete the current \c PCMK_XE_STATUS element if not running in stand-alone + * mode. Then create an empty \c PCMK_XE_STATUS child if either of the following + * is true: + * * not running in stand-alone mode + * * running in stand-alone mode with no \c PCMK_XE_STATUS element + * + * \param[in,out] cib_xml CIB XML + */ +static void +set_empty_status(xmlNode *cib_xml) +{ + xmlNode *status = pcmk__xe_first_child(cib_xml, PCMK_XE_STATUS, NULL, NULL); + + if (!stand_alone) { + g_clear_pointer(&status, pcmk__xml_free); + } + + if (status == NULL) { + pcmk__xe_create(cib_xml, PCMK_XE_STATUS); + } +} + /*! * \internal * \brief Set the given CIB version attribute to 0 if it's not already set @@ -411,10 +437,7 @@ based_read_cib(void) PCMK_XA_NUM_UPDATES, }; - xmlNode *cib_xml = NULL; - xmlNode *status = NULL; - - cib_xml = read_current_cib(); + xmlNode *cib_xml = read_current_cib(); if (cib_xml == NULL) { cib_xml = read_backup_cib(); @@ -433,15 +456,7 @@ based_read_cib(void) pcmk__err("*** Disabling disk writes to avoid confusing Valgrind ***"); } - status = pcmk__xe_first_child(cib_xml, PCMK_XE_STATUS, NULL, NULL); - if (!stand_alone && (status != NULL)) { - // Strip out the PCMK_XE_STATUS section if there is one - pcmk__xml_free(status); - status = NULL; - } - if (status == NULL) { - pcmk__xe_create(cib_xml, PCMK_XE_STATUS); - } + set_empty_status(cib_xml); /* Default the three version attributes to 0 if unset. The schema requires * them to be set, so: From 9d6c5dc44b5f6f3e41e7315166fad0d42c135fca Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 03:21:29 -0800 Subject: [PATCH 35/59] Refactor: based: Move disabling writes for valgrind to based_io_init() This has nothing to do with reading the CIB from a file, so it should not be part of based_read_cib(). Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 842499a7c7d..032562c6623 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -175,6 +175,17 @@ void based_io_init(void) { writes_enabled = !stand_alone; + if (writes_enabled + && pcmk__env_option_enabled(PCMK__SERVER_BASED, + PCMK__ENV_VALGRIND_ENABLED)) { + + writes_enabled = false; + pcmk__err("*** Disabling disk writes to avoid confusing Valgrind ***"); + } + + /* @TODO Should we be setting this up if we've explicitly disabled writes + * already? + */ mainloop_add_signal(SIGPIPE, based_enable_writes); write_trigger = mainloop_add_trigger(G_PRIORITY_LOW, write_cib_async, NULL); @@ -448,14 +459,6 @@ based_read_cib(void) pcmk__warn("Continuing with an empty configuration"); } - if (writes_enabled - && pcmk__env_option_enabled(PCMK__SERVER_BASED, - PCMK__ENV_VALGRIND_ENABLED)) { - - writes_enabled = false; - pcmk__err("*** Disabling disk writes to avoid confusing Valgrind ***"); - } - set_empty_status(cib_xml); /* Default the three version attributes to 0 if unset. The schema requires From da102e8917ed71d75029f66b63661456131e068e Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 15:35:52 -0800 Subject: [PATCH 36/59] Refactor: based: Best practices in cib_rename() * Rename function to archive_unusable_file(). * Add Doxygen. * Improve log messages. * Rename new_fd variable to fd. * Allow fd == 0. This should never happen in practice, but it's odd to check (fd < 0) for error and (fd > 0) for success when 0 is a valid file descriptor. * Close fd sooner on success. mkstemp() is the safe and idiomatic way to generate a temp file name, because it avoids race conditions that can occur when generating a file name and then creating a file. However, we don't need the file to be open, and we don't need the fd except for the purpose of closing the file. By separating the (fd < 0) case from the (rename(...) < 0) case, we can be sure that (fd >= 0) when we call close(). * Rename old and new variables to old_file and new_file, respectively. I suspect that the use of "new" may be problematic when compiling as C++. Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 52 ++++++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 032562c6623..f4d994d0177 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -191,28 +191,50 @@ based_io_init(void) write_trigger = mainloop_add_trigger(G_PRIORITY_LOW, write_cib_async, NULL); } +/*! + * \internal + * \brief Archive a CIB file or its saved digest file + * + * When a CIB file's calculated digest doesn't match its saved one, we archive + * both the CIB file and its digest (".sig") file. This way the contents can be + * inspected for troubleshooting purposes. + * + * The file is renamed with a unique name using the \c mkstemp() template + * \c "cib.auto.XXXXXX", in the same directory (\c cib_root). + * + * \param[in] old_file Original file path + */ static void -cib_rename(const char *old) +archive_unusable_file(const char *old_file) { - int new_fd; - char *new = pcmk__assert_asprintf("%s/cib.auto.XXXXXX", cib_root); + int fd = 0; + char *new_file = pcmk__assert_asprintf("%s/cib.auto.XXXXXX", cib_root); umask(S_IWGRP | S_IWOTH | S_IROTH); - new_fd = mkstemp(new); + fd = mkstemp(new_file); - if ((new_fd < 0) || (rename(old, new) < 0)) { - pcmk__err("Couldn't archive unusable file %s (disabling disk writes " - "and continuing)", - old); + if (fd < 0) { + pcmk__err("Failed to create a temp file to archive unusable file %s: " + "%s. Disabling disk writes and continuing.", old_file, + strerror(errno)); writes_enabled = false; - } else { - pcmk__err("Archived unusable file %s as %s", old, new); + goto done; } - if (new_fd > 0) { - close(new_fd); + close(fd); + + if (rename(old_file, new_file) < 0) { + pcmk__err("Failed to archive unusable file %s as %s: %s. Disabling " + "disk writes and continuing.", old_file, new_file, + strerror(errno)); + writes_enabled = false; + goto done; } - free(new); + + pcmk__err("Archived unusable file %s as %s", old_file, new_file); + +done: + free(new_file); } #define CIBFILE "cib.xml" @@ -260,8 +282,8 @@ read_current_cib(void) if (rc == pcmk_rc_cib_modified) { // Archive the original files so the contents are not lost - cib_rename(cibfile_path); - cib_rename(sigfile_path); + archive_unusable_file(cibfile_path); + archive_unusable_file(sigfile_path); } done: From c3b756105c9bb3f2479a14be7aa9078ee75f0172 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 17:07:25 -0800 Subject: [PATCH 37/59] Refactor: based: Archive CIB file and digest file in subdirectory Previously, in case of a digest mismatch, we renamed the CIB file and the digest file in cib_root to new unique names generated by mkstemp(). Without checking the error logs, the file names did not make it clear that the two files were related, or which one was the CIB file and which one was the digest file. Now, we create a new subdirectory of cib_root using mkdtemp(). Then we move both the CIB file and the digest file to the new subdirectory. They keep the base names "cib.xml" and "cib.xml.sig". It's not necessary for archive_on_digest_mismatch() to take arguments, but this avoids the need to allocate redundant strings for the old paths. Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 78 ++++++++++++++++++++++++++++------------ 1 file changed, 55 insertions(+), 23 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index f4d994d0177..c6d11f5b134 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -193,52 +193,85 @@ based_io_init(void) /*! * \internal - * \brief Archive a CIB file or its saved digest file + * \brief Rename a CIB or digest file after digest mismatch + * + * This is just a wrapper for logging an error. The caller should disable writes + * on error. + * + * \param[in] old_path Original file path + * \param[in] new_path New file path + * + * \return Standard Pacemaker return code + */ +static int +rename_one(const char *old_path, const char *new_path) +{ + int rc = rename(old_path, new_path); + + if (rc == 0) { + return pcmk_rc_ok; + } + + rc = errno; + pcmk__err("Failed to rename %s to %s after digest mismatch: %s. Disabling " + "disk writes.", old_path, new_path, strerror(rc)); + return rc; +} + +#define CIBFILE "cib.xml" + +/*! + * \internal + * \brief Archive the current CIB file in \c cib_root with its saved digest file * * When a CIB file's calculated digest doesn't match its saved one, we archive * both the CIB file and its digest (".sig") file. This way the contents can be * inspected for troubleshooting purposes. * - * The file is renamed with a unique name using the \c mkstemp() template - * \c "cib.auto.XXXXXX", in the same directory (\c cib_root). + * A subdirectory with a unique name is created in \c cib_root, using the + * \c mkdtemp() template \c "cib.auto.XXXXXX". Then \c CIB_FILE and + * CIB_FILE ".sig" are moved to that directory. * - * \param[in] old_file Original file path + * \param[in] old_cibfile_path Original path of CIB file + * \param[in] old_sigfile_path Original path of digest file */ static void -archive_unusable_file(const char *old_file) +archive_on_digest_mismatch(const char *old_cibfile_path, + const char *old_sigfile_path) { - int fd = 0; - char *new_file = pcmk__assert_asprintf("%s/cib.auto.XXXXXX", cib_root); + char *new_dir = pcmk__assert_asprintf("%s/cib.auto.XXXXXX", cib_root); + char *new_cibfile_path = NULL; + char *new_sigfile_path = NULL; umask(S_IWGRP | S_IWOTH | S_IROTH); - fd = mkstemp(new_file); - if (fd < 0) { - pcmk__err("Failed to create a temp file to archive unusable file %s: " - "%s. Disabling disk writes and continuing.", old_file, - strerror(errno)); + if (mkdtemp(new_dir) == NULL) { + pcmk__err("Failed to create directory to archive %s and %s after " + "digest mismatch: %s. Disabling disk writes.", + old_cibfile_path, old_sigfile_path, strerror(errno)); writes_enabled = false; goto done; } - close(fd); + new_cibfile_path = pcmk__assert_asprintf("%s/%s", new_dir, CIBFILE); + new_sigfile_path = pcmk__assert_asprintf("%s.sig", new_cibfile_path); + + if ((rename_one(old_cibfile_path, new_cibfile_path) != pcmk_rc_ok) + || (rename_one(old_sigfile_path, new_sigfile_path) != pcmk_rc_ok)) { - if (rename(old_file, new_file) < 0) { - pcmk__err("Failed to archive unusable file %s as %s: %s. Disabling " - "disk writes and continuing.", old_file, new_file, - strerror(errno)); writes_enabled = false; goto done; } - pcmk__err("Archived unusable file %s as %s", old_file, new_file); + pcmk__err("Archived %s and %s in %s after digest mismatch", + old_cibfile_path, old_sigfile_path, new_dir); done: - free(new_file); + free(new_dir); + free(new_cibfile_path); + free(new_sigfile_path); } -#define CIBFILE "cib.xml" - /*! * \internal * \brief Read CIB XML from \c CIBFILE in the \c cib_root directory @@ -282,8 +315,7 @@ read_current_cib(void) if (rc == pcmk_rc_cib_modified) { // Archive the original files so the contents are not lost - archive_unusable_file(cibfile_path); - archive_unusable_file(sigfile_path); + archive_on_digest_mismatch(cibfile_path, sigfile_path); } done: From 039d1c1651f5f52205ea817661668c8a19495ca3 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 18:32:03 -0800 Subject: [PATCH 38/59] Refactor: based: Simplify cib_archive_filter() Also add Doxygen and rename to backup_cib_filter(). The trace/debug messages don't seem especially useful, and we recently got rid of similar ones that were commented out of schema_filter(). Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 50 +++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index c6d11f5b134..e898530d77d 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -324,36 +324,34 @@ read_current_cib(void) return cib_xml; } -static int cib_archive_filter(const struct dirent * a) +/*! + * \internal + * \brief \c scandir() filter for backup CIB files in \c cib_root + * + * \param[in] entry Directory entry + * + * \retval 1 if the entry is a regular file whose name begins with \c "cib-" and + * does not end with ".sig" + * \retval 0 otherwise + */ +static int +backup_cib_filter(const struct dirent *entry) { - int rc = 0; - // Looking for regular files starting with "cib-" and not ending in .sig - struct stat s; - char *a_path = pcmk__assert_asprintf("%s/%s", cib_root, a->d_name); + char *path = pcmk__assert_asprintf("%s/%s", cib_root, entry->d_name); + struct stat sb; + int rc = stat(path, &sb); - if(stat(a_path, &s) != 0) { - rc = errno; - pcmk__trace("%s - stat failed: %s (%d)", a->d_name, pcmk_rc_str(rc), - rc); - rc = 0; + free(path); - } else if (!S_ISREG(s.st_mode)) { - pcmk__trace("%s - wrong type (%#o)", a->d_name, - (unsigned int) (s.st_mode & S_IFMT)); - - } else if (!g_str_has_prefix(a->d_name, "cib-")) { - pcmk__trace("%s - wrong prefix", a->d_name); - - } else if (g_str_has_suffix(a->d_name, ".sig")) { - pcmk__trace("%s - wrong suffix", a->d_name); - - } else { - pcmk__debug("%s - candidate", a->d_name); - rc = 1; + if (rc != 0) { + pcmk__warn("Filtering %s/%s during scan for backup CIB: stat() failed: " + "%s", cib_root, entry->d_name, strerror(errno)); + return 0; } - free(a_path); - return rc; + return S_ISREG(sb.st_mode) + && g_str_has_prefix(entry->d_name, "cib-") + && !g_str_has_suffix(entry->d_name, ".sig"); } static int cib_archive_sort(const struct dirent ** a, const struct dirent **b) @@ -404,7 +402,7 @@ read_backup_cib(void) { xmlNode *cib_xml = NULL; struct dirent **namelist = NULL; - int num_files = scandir(cib_root, &namelist, cib_archive_filter, + int num_files = scandir(cib_root, &namelist, backup_cib_filter, cib_archive_sort); if (num_files < 0) { From 9d2f5b2ec41a0139bcae7f2da01292a17def3595 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 19:19:11 -0800 Subject: [PATCH 39/59] Refactor: based: Simplify cib_archive_sort() Also add Doxygen, rename to compare_backup_cibs(), and create a helper function. Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 85 +++++++++++++++++++++++++++------------- 1 file changed, 58 insertions(+), 27 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index e898530d77d..c99ff1d3641 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -354,40 +354,71 @@ backup_cib_filter(const struct dirent *entry) && !g_str_has_suffix(entry->d_name, ".sig"); } -static int cib_archive_sort(const struct dirent ** a, const struct dirent **b) +/*! + * \internal + * \brief Get a file's last change time (\c ctime) + * + * The file is assumed to be a backup CIB file in the \c cib_root directory. + * + * \param[in] file Base name of file + * + * \return Last change time of \p file, or 0 on \c stat() failure + */ +static time_t +get_backup_cib_ctime(const char *file) { - /* Order by creation date - most recently created file first */ - int rc = 0; - struct stat buf; - - time_t a_age = 0; - time_t b_age = 0; + char *path = pcmk__assert_asprintf("%s/%s", cib_root, file); + struct stat sb; + int rc = stat(path, &sb); - char *a_path = pcmk__assert_asprintf("%s/%s", cib_root, a[0]->d_name); - char *b_path = pcmk__assert_asprintf("%s/%s", cib_root, b[0]->d_name); + free(path); - if(stat(a_path, &buf) == 0) { - a_age = buf.st_ctime; - } - if(stat(b_path, &buf) == 0) { - b_age = buf.st_ctime; + if (rc != 0) { + pcmk__warn("Failed to stat() %s/%s while sorting backup CIBs: %s", + cib_root, file, strerror(errno)); + return 0; } - free(a_path); - free(b_path); + return sb.st_ctime; +} + +/*! + * \internal + * \brief Compare directory entries based on their last change times + * + * The entries are assumed to be CIB files in the \c cib_root directory. + * + * \param[in] entry1 First directory entry to compare + * \param[in] entry2 Second directory entry to compare + * + * \retval -1 if \p entry1 was changed more recently than \p entry2 + * \retval 0 if \p entry1 was last changed at the same timestamp as \p entry2 + * \retval 1 if \p entry1 was changed less recently than \p entry2 + */ +static int +compare_backup_cibs(const struct dirent **entry1, const struct dirent **entry2) +{ + time_t ctime1 = get_backup_cib_ctime((*entry1)->d_name); + time_t ctime2 = get_backup_cib_ctime((*entry2)->d_name); + + if (ctime1 > ctime2) { + pcmk__trace("%s/%s (%lld) newer than %s/%s (%lld)", + cib_root, (*entry1)->d_name, (long long) ctime1, + cib_root, (*entry2)->d_name, (long long) ctime2); + return -1; + } - if (a_age > b_age) { - // a newer than b - rc = -1; - } else if (a_age < b_age) { - // a older than b - rc = 1; + if (ctime1 < ctime2) { + pcmk__trace("%s/%s (%lld) older than %s/%s (%lld)", + cib_root, (*entry1)->d_name, (long long) ctime1, + cib_root, (*entry2)->d_name, (long long) ctime2); + return 1; } - pcmk__trace("%s (%lu) vs. %s (%lu) : %d", - a[0]->d_name, (unsigned long)a_age, - b[0]->d_name, (unsigned long)b_age, rc); - return rc; + pcmk__trace("%s/%s (%lld) same age as %s/%s (%lld)", + cib_root, (*entry1)->d_name, (long long) ctime1, + cib_root, (*entry2)->d_name, (long long) ctime2); + return 0; } /*! @@ -403,7 +434,7 @@ read_backup_cib(void) xmlNode *cib_xml = NULL; struct dirent **namelist = NULL; int num_files = scandir(cib_root, &namelist, backup_cib_filter, - cib_archive_sort); + compare_backup_cibs); if (num_files < 0) { pcmk__err("Could not check for CIB backups in %s: %s", cib_root, From 1f549ed80cf070e443db1c82ca69075a88f2c411 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 19:42:52 -0800 Subject: [PATCH 40/59] Doc: based: Add Doxygen for based_read_cib() Note that the return value is guaranteed not to be NULL, because the createEmptyCib() return value is guaranteed not to be NULL. Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index c99ff1d3641..66080ffd7f3 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -522,6 +522,25 @@ set_default_if_unset(xmlNode *cib_xml, const char *version_attr) pcmk__xe_set_int(cib_xml, version_attr, 0); } +/*! + * \internal + * \brief Read the most recent CIB from a file in \c cib_root + * + * This function first tries to read the CIB from a file called \c "cib.xml" in + * the \c cib_root directory. + * + * If that fails or there is a digest mismatch, it tries all the backup CIB + * files in \c cib_root, in order from most recently changed to least, moving to + * the next backup file on failure or digest mismatch. + * + * If no valid CIB file is found, this function generates an empty CIB. + * + * \return The most current CIB XML available, or an empty CIB if none is + * available (guaranteed not to be \c NULL) + * + * \note The caller is responsible for freeing the return value using + * \c pcmk__xml_free(). + */ xmlNode * based_read_cib(void) { From 08b57b2071d35cc8d182ce1ae0e76f645001643b Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 19:44:37 -0800 Subject: [PATCH 41/59] Refactor: based: Drop uninitializeCib() As long as the_cib is a global variable, this function is unnecessary. Also, it was too complicated -- it can be a one-liner, as shown here. Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 2 +- daemons/based/based_io.c | 13 ------------- daemons/based/pacemaker-based.h | 1 - 3 files changed, 1 insertion(+), 15 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index f02bfe74042..0ac24a326b6 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -1363,7 +1363,7 @@ terminate_cib(int exit_status) remote_tls_fd = 0; } - uninitializeCib(); + g_clear_pointer(&the_cib, pcmk__xml_free); // Exit immediately on error if (exit_status > CRM_EX_OK) { diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 66080ffd7f3..da77bcfdba7 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -588,19 +588,6 @@ based_read_cib(void) return cib_xml; } -void -uninitializeCib(void) -{ - xmlNode *tmp_cib = the_cib; - - if (tmp_cib == NULL) { - return; - } - - the_cib = NULL; - pcmk__xml_free(tmp_cib); -} - /* * This method will free the old CIB pointer on success and the new one * on failure. diff --git a/daemons/based/pacemaker-based.h b/daemons/based/pacemaker-based.h index a16485320b8..eb70cb4230e 100644 --- a/daemons/based/pacemaker-based.h +++ b/daemons/based/pacemaker-based.h @@ -74,7 +74,6 @@ int cib_process_request(xmlNode *request, bool privileged, void cib_shutdown(int nsig); void terminate_cib(int exit_status); -void uninitializeCib(void); int activateCibXml(xmlNode *doc, bool to_disk, const char *op); int cib_process_shutdown_req(const char *op, int options, const char *section, From 29cfde2ede19a90179c957c6e7ca586abfae70a9 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 19:47:08 -0800 Subject: [PATCH 42/59] Refactor: based: activateCibXml() returns a standard return code Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 1 - daemons/based/based_io.c | 4 ++-- daemons/based/pacemaker-based.c | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 0ac24a326b6..1330b1d5c87 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -1128,7 +1128,6 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation, (config_changed? " changed" : "")); rc = activateCibXml(result_cib, config_changed, op); - rc = pcmk_legacy2rc(rc); if (rc != pcmk_rc_ok) { pcmk__err("Failed to activate new CIB: %s", pcmk_rc_str(rc)); } diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index da77bcfdba7..118e67ed663 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -605,7 +605,7 @@ activateCibXml(xmlNode *new_cib, bool to_disk, const char *op) pcmk__debug("Triggering CIB write for %s op", op); mainloop_set_trigger(write_trigger); } - return pcmk_ok; + return pcmk_rc_ok; } pcmk__err("Ignoring invalid CIB"); @@ -615,5 +615,5 @@ activateCibXml(xmlNode *new_cib, bool to_disk, const char *op) pcmk__crit("Could not write out new CIB and no saved version to revert " "to"); } - return -ENODATA; + return ENODATA; } diff --git a/daemons/based/pacemaker-based.c b/daemons/based/pacemaker-based.c index 28662d690c0..49b53a6073d 100644 --- a/daemons/based/pacemaker-based.c +++ b/daemons/based/pacemaker-based.c @@ -406,7 +406,7 @@ startCib(void) xmlNode *cib = based_read_cib(); int port = 0; - if (activateCibXml(cib, true, "start") != 0) { + if (activateCibXml(cib, true, "start") != pcmk_rc_ok) { return false; } From 0ce2428aeceba34238376347d0104d367a966f12 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 19:53:39 -0800 Subject: [PATCH 43/59] Refactor: based: activateCibXml() CRM_CHECKs that new_cib != NULL There are two callers. The argument in startCib() is clearly non-NULL. The argument in cib_process_command() is unsurprisingly less clear. result_cib gets set by cib_perform_op(), and we call activateCibXml() only if we're performing an operation that modifies the CIB, rc is pcmk_rc_ok, and (result_cib != the_cib). result_cib should always be non-NULL in that case. And if it ever is NULL, then this is something I'd like to find out about via an assertion log. Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 118e67ed663..1e8abafe78f 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -595,25 +595,15 @@ based_read_cib(void) int activateCibXml(xmlNode *new_cib, bool to_disk, const char *op) { - if (new_cib) { - xmlNode *saved_cib = the_cib; - - pcmk__assert(new_cib != saved_cib); - the_cib = new_cib; - pcmk__xml_free(saved_cib); - if (to_disk && writes_enabled && (cib_status == pcmk_rc_ok)) { - pcmk__debug("Triggering CIB write for %s op", op); - mainloop_set_trigger(write_trigger); - } - return pcmk_rc_ok; - } + xmlNode *saved_cib = the_cib; - pcmk__err("Ignoring invalid CIB"); - if (the_cib) { - pcmk__warn("Reverting to last known CIB"); - } else { - pcmk__crit("Could not write out new CIB and no saved version to revert " - "to"); + CRM_CHECK((new_cib != NULL) && (new_cib != the_cib), return ENODATA); + + the_cib = new_cib; + pcmk__xml_free(saved_cib); + if (to_disk && writes_enabled && (cib_status == pcmk_rc_ok)) { + pcmk__debug("Triggering CIB write for %s op", op); + mainloop_set_trigger(write_trigger); } - return ENODATA; + return pcmk_rc_ok; } From 98f8c2dffbe70f25c0d56b2e19671cf7c8748c41 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 20:10:27 -0800 Subject: [PATCH 44/59] Refactor: based: Best practices in activateCibXml() Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 2 +- daemons/based/based_io.c | 28 +++++++++++++++++++++------- daemons/based/based_io.h | 3 +++ daemons/based/pacemaker-based.c | 2 +- daemons/based/pacemaker-based.h | 2 -- 5 files changed, 26 insertions(+), 11 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index 1330b1d5c87..c1c7d8721f0 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -1127,7 +1127,7 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation, pcmk__xe_get(result_cib, PCMK_XA_NUM_UPDATES), (config_changed? " changed" : "")); - rc = activateCibXml(result_cib, config_changed, op); + rc = based_activate_cib(result_cib, config_changed, op); if (rc != pcmk_rc_ok) { pcmk__err("Failed to activate new CIB: %s", pcmk_rc_str(rc)); } diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 1e8abafe78f..6eec84af668 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -588,22 +588,36 @@ based_read_cib(void) return cib_xml; } -/* - * This method will free the old CIB pointer on success and the new one - * on failure. +/*! + * \internal + * \brief Activate new CIB XML + * + * This function frees the existing \c the_cib and points it to \p new_cib. + * + * \param[in] new_cib CIB XML to activate (must not be \c NULL or equal to + * \c the_cib) + * \param[in] to_disk If \c true and if the CIB status is OK and writes are + * enabled, trigger the new CIB to be written to disk + * \param[in] op Operation that triggered the activation (for logging + * only) + * + * \return Standard Pacemaker return code + * + * \note This function takes ownership of \p new_cib by assigning it to + * \c the_cib. The caller should not free it. */ int -activateCibXml(xmlNode *new_cib, bool to_disk, const char *op) +based_activate_cib(xmlNode *new_cib, bool to_disk, const char *op) { - xmlNode *saved_cib = the_cib; - CRM_CHECK((new_cib != NULL) && (new_cib != the_cib), return ENODATA); + pcmk__xml_free(the_cib); the_cib = new_cib; - pcmk__xml_free(saved_cib); + if (to_disk && writes_enabled && (cib_status == pcmk_rc_ok)) { pcmk__debug("Triggering CIB write for %s op", op); mainloop_set_trigger(write_trigger); } + return pcmk_rc_ok; } diff --git a/daemons/based/based_io.h b/daemons/based/based_io.h index 2c3fab0ffea..f9bb5011a1a 100644 --- a/daemons/based/based_io.h +++ b/daemons/based/based_io.h @@ -10,10 +10,13 @@ #ifndef BASED_IO__H #define BASED_IO__H +#include + #include // xmlNode void based_io_init(void); void based_enable_writes(int nsig); xmlNode *based_read_cib(void); +int based_activate_cib(xmlNode *new_cib, bool to_disk, const char *op); #endif // BASED_IO__H diff --git a/daemons/based/pacemaker-based.c b/daemons/based/pacemaker-based.c index 49b53a6073d..c1da0103f5a 100644 --- a/daemons/based/pacemaker-based.c +++ b/daemons/based/pacemaker-based.c @@ -406,7 +406,7 @@ startCib(void) xmlNode *cib = based_read_cib(); int port = 0; - if (activateCibXml(cib, true, "start") != pcmk_rc_ok) { + if (based_activate_cib(cib, true, "start") != pcmk_rc_ok) { return false; } diff --git a/daemons/based/pacemaker-based.h b/daemons/based/pacemaker-based.h index eb70cb4230e..8ebb8f4f4fd 100644 --- a/daemons/based/pacemaker-based.h +++ b/daemons/based/pacemaker-based.h @@ -74,8 +74,6 @@ int cib_process_request(xmlNode *request, bool privileged, void cib_shutdown(int nsig); void terminate_cib(int exit_status); -int activateCibXml(xmlNode *doc, bool to_disk, const char *op); - int cib_process_shutdown_req(const char *op, int options, const char *section, xmlNode *req, xmlNode *input, xmlNode *existing_cib, xmlNode **result_cib, From 9e5583a26a46363b27ca4fb047d9f17a06045841 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 22:54:57 -0800 Subject: [PATCH 45/59] Refactor: based: Drop unused includes from based_operation.c Also include stddef.h for NULL, and rename cib_op_functions to op_functions so that it doesn't look like a public libcib symbol. Signed-off-by: Reid Wahl --- daemons/based/based_operation.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/daemons/based/based_operation.c b/daemons/based/based_operation.c index 6c8e93f4b83..881464517de 100644 --- a/daemons/based/based_operation.c +++ b/daemons/based/based_operation.c @@ -1,5 +1,5 @@ /* - * Copyright 2008-2024 the Pacemaker project contributors + * Copyright 2008-2025 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -9,13 +9,11 @@ #include -#include +#include // NULL -#include -#include #include -static const cib__op_fn_t cib_op_functions[] = { +static const cib__op_fn_t op_functions[] = { [cib__op_abs_delete] = cib_process_delete_absolute, [cib__op_apply_patch] = cib_server_process_diff, [cib__op_bump] = cib_process_bump, @@ -53,8 +51,8 @@ based_get_op_function(const cib__operation_t *operation) pcmk__assert(type >= 0); - if (type >= PCMK__NELEM(cib_op_functions)) { + if (type >= PCMK__NELEM(op_functions)) { return NULL; } - return cib_op_functions[type]; + return op_functions[type]; } From 033142fe257db05c6b3a6ebae5aa91a73c0c953c Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 22:59:40 -0800 Subject: [PATCH 46/59] Refactor: based: New based_operation.h Signed-off-by: Reid Wahl --- daemons/based/Makefile.am | 1 + daemons/based/based_operation.h | 17 +++++++++++++++++ daemons/based/pacemaker-based.h | 2 +- 3 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 daemons/based/based_operation.h diff --git a/daemons/based/Makefile.am b/daemons/based/Makefile.am index 6850b70c496..3ae8e2e60b6 100644 --- a/daemons/based/Makefile.am +++ b/daemons/based/Makefile.am @@ -15,6 +15,7 @@ halibdir = $(CRM_DAEMON_DIR) halib_PROGRAMS = pacemaker-based noinst_HEADERS = based_io.h +noinst_HEADERS += based_operation.h noinst_HEADERS += based_transaction.h noinst_HEADERS += pacemaker-based.h diff --git a/daemons/based/based_operation.h b/daemons/based/based_operation.h new file mode 100644 index 00000000000..ee016466531 --- /dev/null +++ b/daemons/based/based_operation.h @@ -0,0 +1,17 @@ +/* + * Copyright 2023-2025 the Pacemaker project contributors + * + * The version control history for this file may have further details. + * + * This source code is licensed under the GNU Lesser General Public License + * version 2.1 or later (LGPLv2.1+) WITHOUT ANY WARRANTY. + */ + +#ifndef BASED_OPERATION__H +#define BASED_OPERATION__H + +#include // cib__* + +cib__op_fn_t based_get_op_function(const cib__operation_t *operation); + +#endif // BASED_OPERATION__H diff --git a/daemons/based/pacemaker-based.h b/daemons/based/pacemaker-based.h index 8ebb8f4f4fd..cba8a045f5e 100644 --- a/daemons/based/pacemaker-based.h +++ b/daemons/based/pacemaker-based.h @@ -31,6 +31,7 @@ #include #include "based_io.h" +#include "based_operation.h" #include "based_transaction.h" #include @@ -118,7 +119,6 @@ int cib_process_schemas(const char *op, int options, const char *section, void send_sync_request(const char *host); int sync_our_cib(xmlNode *request, bool all); -cib__op_fn_t based_get_op_function(const cib__operation_t *operation); void cib_diff_notify(const char *op, int result, const char *call_id, const char *client_id, const char *client_name, const char *origin, xmlNode *update, xmlNode *diff); From 3980a997b6d37076fbe3a5c4ed4da16b521c07d6 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 23:07:42 -0800 Subject: [PATCH 47/59] Refactor: based: Avoid xmlNodePtr and clean up includes This is just some miscellaneous cleanup. For the most part, we use xmlNode * instead of xmlNodePtr nowadays, because "const xmlNodePtr" doesn't work. Also drop some unused includes and add some that were missing in based_transaction.{c,h}. Signed-off-by: Reid Wahl --- daemons/based/based_transaction.c | 13 ++++++------- daemons/based/based_transaction.h | 11 +++++------ 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/daemons/based/based_transaction.c b/daemons/based/based_transaction.c index 258861cdea6..a9df2950d53 100644 --- a/daemons/based/based_transaction.c +++ b/daemons/based/based_transaction.c @@ -11,8 +11,7 @@ #include -#include -#include +#include // xmlNode #include "pacemaker-based.h" @@ -54,8 +53,8 @@ based_transaction_source_str(const pcmk__client_t *client, const char *origin) * \return Standard Pacemaker return code */ static int -process_transaction_requests(xmlNodePtr transaction, - const pcmk__client_t *client, const char *source) +process_transaction_requests(xmlNode *transaction, const pcmk__client_t *client, + const char *source) { for (xmlNode *request = pcmk__xe_first_child(transaction, PCMK__XE_CIB_COMMAND, NULL, @@ -116,10 +115,10 @@ process_transaction_requests(xmlNodePtr transaction, * success, and for freeing it on failure. */ int -based_commit_transaction(xmlNodePtr transaction, const pcmk__client_t *client, - const char *origin, xmlNodePtr *result_cib) +based_commit_transaction(xmlNode *transaction, const pcmk__client_t *client, + const char *origin, xmlNode **result_cib) { - xmlNodePtr saved_cib = the_cib; + xmlNode *saved_cib = the_cib; int rc = pcmk_rc_ok; char *source = NULL; diff --git a/daemons/based/based_transaction.h b/daemons/based/based_transaction.h index 9935c736a68..19dc01ba529 100644 --- a/daemons/based/based_transaction.h +++ b/daemons/based/based_transaction.h @@ -1,5 +1,5 @@ /* - * Copyright 2023 the Pacemaker project contributors + * Copyright 2023-2025 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -10,15 +10,14 @@ #ifndef BASED_TRANSACTION__H #define BASED_TRANSACTION__H -#include +#include // xmlNode -#include +#include // pcmk__client_t char *based_transaction_source_str(const pcmk__client_t *client, const char *origin); -int based_commit_transaction(xmlNodePtr transaction, - const pcmk__client_t *client, - const char *origin, xmlNodePtr *result_cib); +int based_commit_transaction(xmlNode *transaction, const pcmk__client_t *client, + const char *origin, xmlNode **result_cib); #endif // BASED_TRANSACTION__H From f7810a1fabfa76ddf237280eb511c60032e0be61 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 23:16:58 -0800 Subject: [PATCH 48/59] Refactor: based: New based_notify.h Signed-off-by: Reid Wahl --- daemons/based/Makefile.am | 1 + daemons/based/based_callbacks.c | 4 ++-- daemons/based/based_notify.c | 6 +++--- daemons/based/based_notify.h | 19 +++++++++++++++++++ daemons/based/pacemaker-based.h | 5 +---- 5 files changed, 26 insertions(+), 9 deletions(-) create mode 100644 daemons/based/based_notify.h diff --git a/daemons/based/Makefile.am b/daemons/based/Makefile.am index 3ae8e2e60b6..187a8a59cbc 100644 --- a/daemons/based/Makefile.am +++ b/daemons/based/Makefile.am @@ -15,6 +15,7 @@ halibdir = $(CRM_DAEMON_DIR) halib_PROGRAMS = pacemaker-based noinst_HEADERS = based_io.h +noinst_HEADERS += based_notify.h noinst_HEADERS += based_operation.h noinst_HEADERS += based_transaction.h noinst_HEADERS += pacemaker-based.h diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index c1c7d8721f0..b06e3e4d576 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -1182,8 +1182,8 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation, cib_dryrun|cib_inhibit_notify|cib_transaction)) { pcmk__trace("Sending notifications %d", pcmk__is_set(call_options, cib_dryrun)); - cib_diff_notify(op, pcmk_rc2legacy(rc), call_id, client_id, client_name, - originator, input, cib_diff); + based_diff_notify(op, pcmk_rc2legacy(rc), call_id, client_id, + client_name, originator, input, cib_diff); } pcmk__log_xml_patchset(LOG_TRACE, cib_diff); diff --git a/daemons/based/based_notify.c b/daemons/based/based_notify.c index dba81da44c3..91dc28e9b3c 100644 --- a/daemons/based/based_notify.c +++ b/daemons/based/based_notify.c @@ -150,9 +150,9 @@ cib_notify_send(const xmlNode *xml) } void -cib_diff_notify(const char *op, int result, const char *call_id, - const char *client_id, const char *client_name, - const char *origin, xmlNode *update, xmlNode *diff) +based_diff_notify(const char *op, int result, const char *call_id, + const char *client_id, const char *client_name, + const char *origin, xmlNode *update, xmlNode *diff) { int add_updates = 0; int add_epoch = 0; diff --git a/daemons/based/based_notify.h b/daemons/based/based_notify.h new file mode 100644 index 00000000000..0943d8d9dd3 --- /dev/null +++ b/daemons/based/based_notify.h @@ -0,0 +1,19 @@ +/* + * Copyright 2025 the Pacemaker project contributors + * + * The version control history for this file may have further details. + * + * This source code is licensed under the GNU Lesser General Public License + * version 2.1 or later (LGPLv2.1+) WITHOUT ANY WARRANTY. + */ + +#ifndef BASED_NOTIFY__H +#define BASED_NOTIFY__H + +#include // xmlNode + +void based_diff_notify(const char *op, int result, const char *call_id, + const char *client_id, const char *client_name, + const char *origin, xmlNode *update, xmlNode *diff); + +#endif // BASED_NOTIFY__H diff --git a/daemons/based/pacemaker-based.h b/daemons/based/pacemaker-based.h index cba8a045f5e..78b5f9655b5 100644 --- a/daemons/based/pacemaker-based.h +++ b/daemons/based/pacemaker-based.h @@ -32,6 +32,7 @@ #include "based_io.h" #include "based_operation.h" +#include "based_notify.h" #include "based_transaction.h" #include @@ -119,10 +120,6 @@ int cib_process_schemas(const char *op, int options, const char *section, void send_sync_request(const char *host); int sync_our_cib(xmlNode *request, bool all); -void cib_diff_notify(const char *op, int result, const char *call_id, - const char *client_id, const char *client_name, - const char *origin, xmlNode *update, xmlNode *diff); - static inline const char * cib_config_lookup(const char *opt) { From 8b6e487d9710496729844b07f494c25fb6ec56fa Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sun, 21 Dec 2025 23:37:22 -0800 Subject: [PATCH 49/59] Refactor: libcib: Drop cib_diff_version_details() It's a simple wrapper around pcmk__xml_patchset_versions but requires seven arguments. Signed-off-by: Reid Wahl --- daemons/based/based_messages.c | 17 +++++------------ daemons/based/based_notify.c | 34 +++++++++++++--------------------- include/crm/cib/internal.h | 3 --- lib/cib/cib_utils.c | 20 -------------------- 4 files changed, 18 insertions(+), 56 deletions(-) diff --git a/daemons/based/based_messages.c b/daemons/based/based_messages.c index 0aaedec5001..0e4dd7b8bfc 100644 --- a/daemons/based/based_messages.c +++ b/daemons/based/based_messages.c @@ -307,23 +307,16 @@ cib_server_process_diff(const char *op, int options, const char *section, xmlNod // The primary instance should never ignore a diff if (sync_in_progress && !based_is_primary) { - int diff_add_updates = 0; - int diff_add_epoch = 0; - int diff_add_admin_epoch = 0; + int source[] = { 0, 0, 0 }; + int target[] = { 0, 0, 0 }; - int diff_del_updates = 0; - int diff_del_epoch = 0; - int diff_del_admin_epoch = 0; - - cib_diff_version_details(input, - &diff_add_admin_epoch, &diff_add_epoch, &diff_add_updates, - &diff_del_admin_epoch, &diff_del_epoch, &diff_del_updates); + pcmk__xml_patchset_versions(input, source, target); sync_in_progress++; pcmk__notice("Not applying diff %d.%d.%d -> %d.%d.%d (sync in " "progress)", - diff_del_admin_epoch, diff_del_epoch, diff_del_updates, - diff_add_admin_epoch, diff_add_epoch, diff_add_updates); + source[0], source[1], source[2], + target[0], target[1], target[2]); return -pcmk_err_diff_resync; } diff --git a/daemons/based/based_notify.c b/daemons/based/based_notify.c index 91dc28e9b3c..c00c35a4f8c 100644 --- a/daemons/based/based_notify.c +++ b/daemons/based/based_notify.c @@ -154,13 +154,8 @@ based_diff_notify(const char *op, int result, const char *call_id, const char *client_id, const char *client_name, const char *origin, xmlNode *update, xmlNode *diff) { - int add_updates = 0; - int add_epoch = 0; - int add_admin_epoch = 0; - - int del_updates = 0; - int del_epoch = 0; - int del_admin_epoch = 0; + int source[] = { 0, 0, 0 }; + int target[] = { 0, 0, 0 }; uint8_t log_level = LOG_TRACE; @@ -175,31 +170,28 @@ based_diff_notify(const char *op, int result, const char *call_id, log_level = LOG_WARNING; } - cib_diff_version_details(diff, &add_admin_epoch, &add_epoch, &add_updates, - &del_admin_epoch, &del_epoch, &del_updates); + /* @TODO Check return code? How should we handle an error? Are these log + * messages even useful? + */ + pcmk__xml_patchset_versions(diff, source, target); - if ((add_admin_epoch != del_admin_epoch) - || (add_epoch != del_epoch) - || (add_updates != del_updates)) { + if ((source[0] != target[0]) + || (source[1] != target[1]) + || (source[2] != target[2])) { do_crm_log(log_level, "Updated CIB generation %d.%d.%d to %d.%d.%d from client " "%s%s%s (%s) (%s)", - del_admin_epoch, del_epoch, del_updates, - add_admin_epoch, add_epoch, add_updates, - client_name, + source[0], source[1], source[2], + target[0], target[1], target[2], client_name, ((call_id != NULL)? " call " : ""), pcmk__s(call_id, ""), pcmk__s(origin, "unspecified peer"), pcmk_strerror(result)); - } else if ((add_admin_epoch != 0) - || (add_epoch != 0) - || (add_updates != 0)) { - + } else if ((target[0] != 0) || (target[1] != 0) || (target[2] != 0)) { do_crm_log(log_level, "Local-only change to CIB generation %d.%d.%d from client " "%s%s%s (%s) (%s)", - add_admin_epoch, add_epoch, add_updates, - client_name, + target[0], target[1], target[2], client_name, ((call_id != NULL)? " call " : ""), pcmk__s(call_id, ""), pcmk__s(origin, "unspecified peer"), pcmk_strerror(result)); } diff --git a/include/crm/cib/internal.h b/include/crm/cib/internal.h index 4c4c54fd473..d493b69e2a2 100644 --- a/include/crm/cib/internal.h +++ b/include/crm/cib/internal.h @@ -95,9 +95,6 @@ enum cib__op_type { cib__op_schemas, }; -gboolean cib_diff_version_details(xmlNode * diff, int *admin_epoch, int *epoch, int *updates, - int *_admin_epoch, int *_epoch, int *_updates); - gboolean cib_read_config(GHashTable * options, xmlNode * current_cib); typedef int (*cib__op_fn_t)(const char *, int, const char *, xmlNode *, diff --git a/lib/cib/cib_utils.c b/lib/cib/cib_utils.c index 043c57e14ec..70c409ba1f8 100644 --- a/lib/cib/cib_utils.c +++ b/lib/cib/cib_utils.c @@ -39,26 +39,6 @@ cib_version_details(xmlNode * cib, int *admin_epoch, int *epoch, int *updates) return TRUE; } -gboolean -cib_diff_version_details(xmlNode * diff, int *admin_epoch, int *epoch, int *updates, - int *_admin_epoch, int *_epoch, int *_updates) -{ - int add[] = { 0, 0, 0 }; - int del[] = { 0, 0, 0 }; - - pcmk__xml_patchset_versions(diff, del, add); - - *admin_epoch = add[0]; - *epoch = add[1]; - *updates = add[2]; - - *_admin_epoch = del[0]; - *_epoch = del[1]; - *_updates = del[2]; - - return TRUE; -} - /*! * \internal * \brief Get the XML patchset from a CIB diff notification From 2c1859994f9e0649d1c26c8d6f0b55d98ca6c774 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 3 Jan 2026 18:08:13 -0800 Subject: [PATCH 50/59] Log: based: Drop redundant messages from based_diff_notify() There's a message near the end of cib_process_request() that tells us almost exactly the same things. The only thing that seems to be missing is the pre-operation CIB versions. I don't think that's a big loss. The only caller of based_diff_notify() is cib_process_command(). The message in cib_process_request() gets logged shortly after the sole call to cib_process_command(). So any time the based_diff_notify() message would have been logged, the remaining one in cib_process_request() will be too. Signed-off-by: Reid Wahl --- daemons/based/based_notify.c | 37 +----------------------------------- 1 file changed, 1 insertion(+), 36 deletions(-) diff --git a/daemons/based/based_notify.c b/daemons/based/based_notify.c index c00c35a4f8c..a05eb1066c5 100644 --- a/daemons/based/based_notify.c +++ b/daemons/based/based_notify.c @@ -1,5 +1,5 @@ /* - * Copyright 2004-2025 the Pacemaker project contributors + * Copyright 2004-2026 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -154,11 +154,6 @@ based_diff_notify(const char *op, int result, const char *call_id, const char *client_id, const char *client_name, const char *origin, xmlNode *update, xmlNode *diff) { - int source[] = { 0, 0, 0 }; - int target[] = { 0, 0, 0 }; - - uint8_t log_level = LOG_TRACE; - xmlNode *update_msg = NULL; xmlNode *wrapper = NULL; @@ -166,36 +161,6 @@ based_diff_notify(const char *op, int result, const char *call_id, return; } - if (result != pcmk_ok) { - log_level = LOG_WARNING; - } - - /* @TODO Check return code? How should we handle an error? Are these log - * messages even useful? - */ - pcmk__xml_patchset_versions(diff, source, target); - - if ((source[0] != target[0]) - || (source[1] != target[1]) - || (source[2] != target[2])) { - - do_crm_log(log_level, - "Updated CIB generation %d.%d.%d to %d.%d.%d from client " - "%s%s%s (%s) (%s)", - source[0], source[1], source[2], - target[0], target[1], target[2], client_name, - ((call_id != NULL)? " call " : ""), pcmk__s(call_id, ""), - pcmk__s(origin, "unspecified peer"), pcmk_strerror(result)); - - } else if ((target[0] != 0) || (target[1] != 0) || (target[2] != 0)) { - do_crm_log(log_level, - "Local-only change to CIB generation %d.%d.%d from client " - "%s%s%s (%s) (%s)", - target[0], target[1], target[2], client_name, - ((call_id != NULL)? " call " : ""), pcmk__s(call_id, ""), - pcmk__s(origin, "unspecified peer"), pcmk_strerror(result)); - } - update_msg = pcmk__xe_create(NULL, PCMK__XE_NOTIFY); pcmk__xe_set(update_msg, PCMK__XA_T, PCMK__VALUE_CIB_NOTIFY); From 24ad657aaf61f034d0563a52e90d46163dc3a6f0 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 6 Jan 2026 11:22:57 -0800 Subject: [PATCH 51/59] Refactor: libcrmcommon: New pcmk__mainloop_child_exit_fn_t typedef Signed-off-by: Reid Wahl --- include/crm/common/mainloop.h | 21 +++++++++------------ include/crm/common/mainloop_internal.h | 2 +- lib/common/mainloop.c | 18 +++++++++--------- 3 files changed, 19 insertions(+), 22 deletions(-) diff --git a/include/crm/common/mainloop.h b/include/crm/common/mainloop.h index a680c44bd04..e10e0625976 100644 --- a/include/crm/common/mainloop.h +++ b/include/crm/common/mainloop.h @@ -44,6 +44,10 @@ typedef struct mainloop_child_s mainloop_child_t; // NOTE: sbd (as of at least 1.5.2) uses this typedef struct mainloop_timer_s mainloop_timer_t; +//! \deprecated This has been for internal use only since its creation. +typedef void (*pcmk__mainloop_child_exit_fn_t)(mainloop_child_t *p, int core, + int signo, int exitcode); + void mainloop_cleanup(void); // NOTE: sbd (as of at least 1.5.2) uses this @@ -167,20 +171,13 @@ void mainloop_del_fd(mainloop_io_t * client); * Create a new tracked process * To track a process group, use -pid */ -void mainloop_child_add(pid_t pid, - int timeout, - const char *desc, +void mainloop_child_add(pid_t pid, int timeout, const char *desc, void *userdata, - void (*callback)(mainloop_child_t *p, int core, - int signo, int exitcode)); + pcmk__mainloop_child_exit_fn_t exit_fn); -void mainloop_child_add_with_flags(pid_t pid, - int timeout, - const char *desc, - void *userdata, - enum mainloop_child_flags, - void (*callback)(mainloop_child_t *p, int core, - int signo, int exitcode)); +void mainloop_child_add_with_flags(pid_t pid, int timeout, const char *desc, + void *userdata, enum mainloop_child_flags, + pcmk__mainloop_child_exit_fn_t exit_fn); void *mainloop_child_userdata(mainloop_child_t * child); int mainloop_child_timeout(mainloop_child_t * child); diff --git a/include/crm/common/mainloop_internal.h b/include/crm/common/mainloop_internal.h index 05c3f45672e..db3a49b64b4 100644 --- a/include/crm/common/mainloop_internal.h +++ b/include/crm/common/mainloop_internal.h @@ -35,7 +35,7 @@ struct mainloop_child_s { enum mainloop_child_flags flags; /* Called when a process dies */ - void (*callback)(mainloop_child_t *p, int core, int signo, int exitcode); + pcmk__mainloop_child_exit_fn_t exit_fn; }; int pcmk__add_mainloop_ipc(crm_ipc_t *ipc, int priority, void *userdata, diff --git a/lib/common/mainloop.c b/lib/common/mainloop.c index 5652fe89bb7..4d3dd9ae6ac 100644 --- a/lib/common/mainloop.c +++ b/lib/common/mainloop.c @@ -1158,8 +1158,8 @@ child_waitpid(mainloop_child_t *child, int flags) callback_needed = false; } - if (callback_needed && child->callback) { - child->callback(child, core, signo, exitcode); + if (callback_needed && child->exit_fn) { + child->exit_fn(child, core, signo, exitcode); } return callback_needed; } @@ -1251,9 +1251,10 @@ mainloop_child_kill(pid_t pid) * completed process. */ void -mainloop_child_add_with_flags(pid_t pid, int timeout, const char *desc, void *privatedata, enum mainloop_child_flags flags, - void (*callback)(mainloop_child_t *p, int core, - int signo, int exitcode)) +mainloop_child_add_with_flags(pid_t pid, int timeout, const char *desc, + void *privatedata, + enum mainloop_child_flags flags, + pcmk__mainloop_child_exit_fn_t exit_fn) { static bool need_init = TRUE; mainloop_child_t *child = pcmk__assert_alloc(1, sizeof(mainloop_child_t)); @@ -1262,7 +1263,7 @@ mainloop_child_add_with_flags(pid_t pid, int timeout, const char *desc, void *pr child->timerid = 0; child->timeout = FALSE; child->privatedata = privatedata; - child->callback = callback; + child->exit_fn = exit_fn; child->flags = flags; child->desc = pcmk__str_copy(desc); @@ -1284,10 +1285,9 @@ mainloop_child_add_with_flags(pid_t pid, int timeout, const char *desc, void *pr void mainloop_child_add(pid_t pid, int timeout, const char *desc, void *privatedata, - void (*callback)(mainloop_child_t *p, int core, int signo, - int exitcode)) + pcmk__mainloop_child_exit_fn_t exit_fn) { - mainloop_child_add_with_flags(pid, timeout, desc, privatedata, 0, callback); + mainloop_child_add_with_flags(pid, timeout, desc, privatedata, 0, exit_fn); } static gboolean From c4755534f1c4fa680630124b0371cbdd7a7038be Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 6 Jan 2026 14:09:03 -0800 Subject: [PATCH 52/59] Refactor: based: Clean up includes in based_callbacks.c Using include-what-you-use. Signed-off-by: Reid Wahl --- daemons/based/based_callbacks.c | 56 +++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 23 deletions(-) diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c index b06e3e4d576..6017682c065 100644 --- a/daemons/based/based_callbacks.c +++ b/daemons/based/based_callbacks.c @@ -1,5 +1,5 @@ /* - * Copyright 2004-2025 the Pacemaker project contributors + * Copyright 2004-2026 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -9,29 +9,39 @@ #include -#include -#include -#include -#include - +#include // EACCES, ECONNREFUSED +#include // PRIu64 #include -#include -#include // uint32_t, uint64_t, UINT64_C() -#include -#include -#include // PRIu64 - -#include -#include -#include // xmlXPathObject, etc. - -#include -#include -#include - -#include - -#include +#include // NULL, size_t +#include // u?int*_t, UINT64_C +#include // snprintf +#include // free +#include // gid_t, uid_t +#include // LOG_INFO, LOG_DEBUG +#include // time_t +#include // close + +#include // gboolean, gpointer, g_*, etc. +#include // xmlNode +#include // xmlXPath* +#include // QB_FALSE +#include // qb_ipcs_connection_t +#include // LOG_TRACE + +#include // cib_call_options values +#include // cib__* +#include // pcmk_cluster_disconnect +#include // pcmk__cluster_send_message +#include // pcmk_find_cib_element +#include // pcmk__s, pcmk__str_eq +#include // crm_ipc_*, pcmk_ipc_* +#include // CRM_LOG_ASSERT, CRM_CHECK +#include // mainloop_* +#include // pcmk_rc_* +#include // PCMK_XA_*, PCMK_XE_* +#include // CRM_OP_* + +#include "pacemaker-based.h" #define EXIT_ESCALATION_MS 10000 From 0a56e1e5320ea5db23590168741708e85c798d3a Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 6 Jan 2026 13:55:36 -0800 Subject: [PATCH 53/59] Refactor: based: Clean up includes in based_io.c Using include-what-you-use. Signed-off-by: Reid Wahl --- daemons/based/based_io.c | 52 +++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/daemons/based/based_io.c b/daemons/based/based_io.c index 6eec84af668..5524358b7d8 100644 --- a/daemons/based/based_io.c +++ b/daemons/based/based_io.c @@ -9,32 +9,34 @@ #include +#include // dirent, scandir +#include // errno, EACCES, ENODATA +#include // SIGPIPE #include -#include -#include -#include -#include -#include -#include -#include - -#include -#include -#include -#include - -#include -#include - -#include - -#include -#include -#include -#include -#include - -#include +#include // NULL +#include // rename +#include // free, mkdtemp +#include // strerror, strrchr, etc. +#include // stat, umask, etc. +#include // pid_t +#include // time_t +#include // _exit, fork + +#include // g_*, G_* +#include // xmlNode +#include // QB_FALSE, QB_TRUE +#include // qb_log_* + +#include // cib_file_* +#include // createEmptyCib +#include // pcmk__assert_asprintf, PCMK__XE_*, etc. +#include // CRM_CHECK +#include // mainloop_add_signal +#include // pcmk_legacy2rc, pcmk_rc_* +#include // pcmk_common_cleanup +#include // PCMK_XA_*, PCMK_XE_* + +#include "pacemaker-based.h" static bool writes_enabled = true; static crm_trigger_t *write_trigger = NULL; From 450cb055bd3c0bc5b214135a81d508d2dcda6d99 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 6 Jan 2026 14:15:14 -0800 Subject: [PATCH 54/59] Refactor: based: Clean up includes in based_messages.c Using include-what-you-use. Signed-off-by: Reid Wahl --- daemons/based/based_messages.c | 40 ++++++++++++++++------------------ 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/daemons/based/based_messages.c b/daemons/based/based_messages.c index 0e4dd7b8bfc..cf6d1ad7a85 100644 --- a/daemons/based/based_messages.c +++ b/daemons/based/based_messages.c @@ -1,5 +1,5 @@ /* - * Copyright 2004-2025 the Pacemaker project contributors + * Copyright 2004-2026 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -9,27 +9,25 @@ #include +#include // EINVAL, ENOTCONN, EPROTO #include -#include -#include -#include -#include -#include -#include - -#include -#include - -#include -#include - -#include -#include - -#include -#include - -#include +#include // NULL +#include // free + +#include // g_list_free_full, GList +#include // xmlNode +#include // QB_XS + +#include // PCMK__CIB_REQUEST_UPGRADE +#include // pcmk__cluster_send_message +#include // pcmk__info, pcmk__xml_free, etc. +#include // pcmk_ipc_server +#include // CRM_CHECK +#include // pcmk_err, pcmk_ok, pcmk_rc* +#include // PCMK_XA_*, PCMK_XE_* +#include // CRM_FEATURE_SET + +#include "pacemaker-based.h" /* Maximum number of diffs to ignore while waiting for a resync */ #define MAX_DIFF_RETRY 5 From f9afa95f0ad32ab55edf2293dfb81be618f18ac9 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 6 Jan 2026 14:20:57 -0800 Subject: [PATCH 55/59] Refactor: based: Clean up includes in based_notify.c Using include-what-you-use. Signed-off-by: Reid Wahl --- daemons/based/based_notify.c | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/daemons/based/based_notify.c b/daemons/based/based_notify.c index a05eb1066c5..d987973b145 100644 --- a/daemons/based/based_notify.c +++ b/daemons/based/based_notify.c @@ -9,27 +9,23 @@ #include -#include +#include // EAGAIN +#include // PRIx64 #include -#include -#include -#include -#include // PRIx64 +#include // NULL +#include // int32_t, uint16_t +#include // ssize_t -#include -#include -#include +#include // gpointer, g_string_free +#include // xmlNode +#include // QB_XS -#include +#include // pcmk__client_t, etc. +#include // pcmk_free_ipc_event +#include // CRM_LOG_ASSERT +#include // pcmk_rc_* -#include -#include - -#include -#include - -#include -#include +#include "pacemaker-based.h" struct cib_notification_s { const xmlNode *msg; From 88cc3b8531723f894f5d7f203e7ae37bf71f1101 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 6 Jan 2026 14:23:27 -0800 Subject: [PATCH 56/59] Refactor: based: Clean up includes in based_operation.c Using include-what-you-use. Signed-off-by: Reid Wahl --- daemons/based/based_operation.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/daemons/based/based_operation.c b/daemons/based/based_operation.c index 881464517de..d8ac4047b15 100644 --- a/daemons/based/based_operation.c +++ b/daemons/based/based_operation.c @@ -1,5 +1,5 @@ /* - * Copyright 2008-2025 the Pacemaker project contributors + * Copyright 2008-2026 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -9,9 +9,12 @@ #include -#include // NULL +#include // NULL -#include +#include // cib__* +#include // pcmk__assert, PCMK__NELEM + +#include "pacemaker-based.h" static const cib__op_fn_t op_functions[] = { [cib__op_abs_delete] = cib_process_delete_absolute, From 8b495ea0fd781235d2ce36001f4669383f0c82da Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 6 Jan 2026 14:35:07 -0800 Subject: [PATCH 57/59] Refactor: based: Clean up includes in based_remote.c Using include-what-you-use. Signed-off-by: Reid Wahl --- daemons/based/based_remote.c | 59 ++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/daemons/based/based_remote.c b/daemons/based/based_remote.c index c51d788d7f7..1c86a2ae91d 100644 --- a/daemons/based/based_remote.c +++ b/daemons/based/based_remote.c @@ -1,5 +1,5 @@ /* - * Copyright 2004-2025 the Pacemaker project contributors + * Copyright 2004-2026 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -8,42 +8,41 @@ */ #include -#include -#include +#include // htons +#include // errno, EAGAIN +#include // getgrgid, getgrnam, group +#include // PRIx64 +#include // sockaddr_in, INADDR_ANY #include -#include -#include -#include -#include -#include // PRIx64 -#include -#include - -#include - -#include -#include - -#include -#include - -#include -#include -#include +#include // NULL +#include // calloc, free, getenv +#include // memset +#include // sockaddr{,_storage}, AF_INET, etc. +#include // gid_t +#include // close + +#include // gboolean, gpointer, g_source_remove, etc. +#include // gnutls_bye, gnutls_deinit +#include // xmlNode +#include // QB_XS + +#include // CRM_DAEMON_GROUP +#include // pcmk__client_t, etc. +#include // CRM_CHECK +#include // mainloop_* +#include // pcmk_rc_* +#include // PCMK_XA_* +#include // CRM_OP_REGISTER #include "pacemaker-based.h" -#include - -#include -#include #if HAVE_SECURITY_PAM_APPL_H -# include -# define HAVE_PAM 1 +#include // pam_*, PAM_* +#define HAVE_PAM 1 #elif HAVE_PAM_PAM_APPL_H -# include -# define HAVE_PAM 1 +#include // pam_*, PAM_* +#define HAVE_PAM 1 #endif static pcmk__tls_t *tls = NULL; From 9fea5f5e58263ed0e4d5b389ee9fe3f4483098c8 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 6 Jan 2026 14:38:15 -0800 Subject: [PATCH 58/59] Refactor: based: Clean up includes in based_transaction.c Using include-what-you-use. Signed-off-by: Reid Wahl --- daemons/based/based_transaction.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/daemons/based/based_transaction.c b/daemons/based/based_transaction.c index a9df2950d53..021e9073798 100644 --- a/daemons/based/based_transaction.c +++ b/daemons/based/based_transaction.c @@ -1,5 +1,5 @@ /* - * Copyright 2023-2025 the Pacemaker project contributors + * Copyright 2023-2026 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -9,10 +9,18 @@ #include +#include // EOPNOTSUPP #include +#include // NULL +#include // free #include // xmlNode +#include // cib__* +#include // pcmk__client_t, pcmk__s, pcmk__xe_*, etc. +#include // CRM_CHECK +#include // pcmk_rc_* + #include "pacemaker-based.h" /*! From 29a080137edce52e04ac900bdb50df6027200352 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Tue, 6 Jan 2026 14:49:15 -0800 Subject: [PATCH 59/59] Refactor: based: Clean up includes in pacemaker-based.c Using include-what-you-use. Signed-off-by: Reid Wahl --- daemons/based/pacemaker-based.c | 41 +++++++++++++++++++-------------- daemons/based/pacemaker-based.h | 28 +++++++--------------- 2 files changed, 32 insertions(+), 37 deletions(-) diff --git a/daemons/based/pacemaker-based.c b/daemons/based/pacemaker-based.c index c1da0103f5a..2392ea2fcee 100644 --- a/daemons/based/pacemaker-based.c +++ b/daemons/based/pacemaker-based.c @@ -9,24 +9,31 @@ #include +#include // errno +#include // initgroups +#include // SIGTERM #include -#include -#include -#include -#include -#include -#include - -#include -#include - -#include -#include -#include -#include -#include - -#include +#include // NULL, size_t +#include // free +#include // LOG_INFO +#include // gid_t, uid_t +#include // setgid, setuid + +#include // cpg_* +#include // g_*, G_*, etc. +#include // xmlNode + +#include // CRM_CONFIG_DIR, CRM_DAEMON_USER +#include // cib_read_config +#include // pcmk_cluster_* +#include // pcmk__node_update, etc. +#include // crm_ipc_* +#include // crm_log_* +#include // mainloop_add_signal +#include // CRM_EX_*, pcmk_rc_* +#include // PCMK_XA_REMOTE_*_PORT + +#include "pacemaker-based.h" #define SUMMARY "daemon for managing the configuration of a Pacemaker cluster" diff --git a/daemons/based/pacemaker-based.h b/daemons/based/pacemaker-based.h index 78b5f9655b5..2e018ec77b4 100644 --- a/daemons/based/pacemaker-based.h +++ b/daemons/based/pacemaker-based.h @@ -11,32 +11,20 @@ # define PACEMAKER_BASED__H #include -#include -#include -#include -#include -#include -#include -#include - -#include -#include - -#include -#include -#include -#include -#include -#include -#include +#include // uint32_t, UINT64_C + +#include // GHashTable, g_hash_table_lookup +#include // xmlNode +#include // qb_ipcs_service_t + +#include // pcmk_cluster_t +#include // pcmk__client_t #include "based_io.h" #include "based_operation.h" #include "based_notify.h" #include "based_transaction.h" -#include - #define OUR_NODENAME (stand_alone? "localhost" : crm_cluster->priv->node_name) // CIB-specific client flags