Compare commits

..

No commits in common. "c8" and "imports/c9/pacemaker-2.1.6-10.1.el9_3" have entirely different histories.

27 changed files with 19871 additions and 14212 deletions

4
.gitignore vendored
View File

@ -1,2 +1,2 @@
SOURCES/nagios-agents-metadata-105ab8a.tar.gz
SOURCES/pacemaker-0f7f88312.tar.gz
SOURCES/nagios-agents-metadata-105ab8a7b2c16b9a29cf1c1596b80136eeef332b.tar.gz
SOURCES/pacemaker-6fdc9deea.tar.gz

View File

@ -1,2 +1,2 @@
ea6c0a27fd0ae8ce02f84a11f08a0d79377041c3 SOURCES/nagios-agents-metadata-105ab8a.tar.gz
88946a460e3be18852861269f8837aaaf339328c SOURCES/pacemaker-0f7f88312.tar.gz
2cbec94ad67dfbeba75e38d2c3c5c44961b3cd16 SOURCES/nagios-agents-metadata-105ab8a7b2c16b9a29cf1c1596b80136eeef332b.tar.gz
fbf71fb3fb42c76f9f1e98497505eb8521cab55e SOURCES/pacemaker-6fdc9deea.tar.gz

View File

@ -0,0 +1,402 @@
From cf53f523e691295879cd75cff1a86bc15664fa51 Mon Sep 17 00:00:00 2001
From: Chris Lumens <clumens@redhat.com>
Date: Tue, 2 May 2023 09:59:13 -0400
Subject: [PATCH 1/7] Feature: daemons: Add start state to LRMD handshake XML
This gets read out of /etc/sysconfig/pacemaker and set into the
environment. The remote node executor will then add that to the XML
that it sends to the controller upon startup.
Ref T183
---
daemons/execd/execd_commands.c | 5 +++++
include/crm_internal.h | 1 +
2 files changed, 6 insertions(+)
diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c
index fa2761e..9a783a5 100644
--- a/daemons/execd/execd_commands.c
+++ b/daemons/execd/execd_commands.c
@@ -1474,6 +1474,7 @@ process_lrmd_signon(pcmk__client_t *client, xmlNode *request, int call_id,
int rc = pcmk_ok;
time_t now = time(NULL);
const char *protocol_version = crm_element_value(request, F_LRMD_PROTOCOL_VERSION);
+ const char *start_state = pcmk__env_option(PCMK__ENV_NODE_START_STATE);
if (compare_version(protocol_version, LRMD_MIN_PROTOCOL_VERSION) < 0) {
crm_err("Cluster API version must be greater than or equal to %s, not %s",
@@ -1503,6 +1504,10 @@ process_lrmd_signon(pcmk__client_t *client, xmlNode *request, int call_id,
crm_xml_add(*reply, F_LRMD_PROTOCOL_VERSION, LRMD_PROTOCOL_VERSION);
crm_xml_add_ll(*reply, PCMK__XA_UPTIME, now - start_time);
+ if (start_state) {
+ crm_xml_add(*reply, PCMK__XA_NODE_START_STATE, start_state);
+ }
+
return rc;
}
diff --git a/include/crm_internal.h b/include/crm_internal.h
index 5f6531f..771bd26 100644
--- a/include/crm_internal.h
+++ b/include/crm_internal.h
@@ -84,6 +84,7 @@
#define PCMK__XA_GRAPH_ERRORS "graph-errors"
#define PCMK__XA_GRAPH_WARNINGS "graph-warnings"
#define PCMK__XA_MODE "mode"
+#define PCMK__XA_NODE_START_STATE "node_start_state"
#define PCMK__XA_TASK "task"
#define PCMK__XA_UPTIME "uptime"
#define PCMK__XA_CONN_HOST "connection_host"
--
2.31.1
From c950291742711b5c4c8986adc8e938fe6fef861c Mon Sep 17 00:00:00 2001
From: Chris Lumens <clumens@redhat.com>
Date: Tue, 2 May 2023 10:04:32 -0400
Subject: [PATCH 2/7] Feature: liblrmd: Save a remote node's requested start
state
Ref T183
---
include/crm/common/ipc_internal.h | 1 +
lib/lrmd/lrmd_client.c | 7 +++++++
2 files changed, 8 insertions(+)
diff --git a/include/crm/common/ipc_internal.h b/include/crm/common/ipc_internal.h
index 5099dda..d203924 100644
--- a/include/crm/common/ipc_internal.h
+++ b/include/crm/common/ipc_internal.h
@@ -112,6 +112,7 @@ struct pcmk__remote_s {
int tcp_socket;
mainloop_io_t *source;
time_t uptime;
+ char *start_state;
/* CIB-only */
char *token;
diff --git a/lib/lrmd/lrmd_client.c b/lib/lrmd/lrmd_client.c
index c565728..4239105 100644
--- a/lib/lrmd/lrmd_client.c
+++ b/lib/lrmd/lrmd_client.c
@@ -588,7 +588,9 @@ lrmd_tls_connection_destroy(gpointer userdata)
}
free(native->remote->buffer);
+ free(native->remote->start_state);
native->remote->buffer = NULL;
+ native->remote->start_state = NULL;
native->source = 0;
native->sock = 0;
native->psk_cred_c = NULL;
@@ -980,6 +982,7 @@ lrmd_handshake(lrmd_t * lrmd, const char *name)
const char *version = crm_element_value(reply, F_LRMD_PROTOCOL_VERSION);
const char *msg_type = crm_element_value(reply, F_LRMD_OPERATION);
const char *tmp_ticket = crm_element_value(reply, F_LRMD_CLIENTID);
+ const char *start_state = crm_element_value(reply, PCMK__XA_NODE_START_STATE);
long long uptime = -1;
crm_element_value_int(reply, F_LRMD_RC, &rc);
@@ -992,6 +995,10 @@ lrmd_handshake(lrmd_t * lrmd, const char *name)
crm_element_value_ll(reply, PCMK__XA_UPTIME, &uptime);
native->remote->uptime = uptime;
+ if (start_state) {
+ native->remote->start_state = strdup(start_state);
+ }
+
if (rc == -EPROTO) {
crm_err("Executor protocol version mismatch between client (%s) and server (%s)",
LRMD_PROTOCOL_VERSION, version);
--
2.31.1
From 7302014c7b7296be31b1f542b3f107d55b1fb2a0 Mon Sep 17 00:00:00 2001
From: Chris Lumens <clumens@redhat.com>
Date: Tue, 2 May 2023 10:05:13 -0400
Subject: [PATCH 3/7] Feature: liblrmd: Add lrmd__node_start_state.
This function is used to get the start state out of an lrmd_private_t
structure.
Ref T183
---
include/crm/lrmd_internal.h | 1 +
lib/lrmd/lrmd_client.c | 12 ++++++++++++
2 files changed, 13 insertions(+)
diff --git a/include/crm/lrmd_internal.h b/include/crm/lrmd_internal.h
index 5810554..d1cd25d 100644
--- a/include/crm/lrmd_internal.h
+++ b/include/crm/lrmd_internal.h
@@ -47,6 +47,7 @@ void lrmd__set_result(lrmd_event_data_t *event, enum ocf_exitcode rc,
void lrmd__reset_result(lrmd_event_data_t *event);
time_t lrmd__uptime(lrmd_t *lrmd);
+const char *lrmd__node_start_state(lrmd_t *lrmd);
/* Shared functions for IPC proxy back end */
diff --git a/lib/lrmd/lrmd_client.c b/lib/lrmd/lrmd_client.c
index 4239105..82434b9 100644
--- a/lib/lrmd/lrmd_client.c
+++ b/lib/lrmd/lrmd_client.c
@@ -2538,3 +2538,15 @@ lrmd__uptime(lrmd_t *lrmd)
return native->remote->uptime;
}
}
+
+const char *
+lrmd__node_start_state(lrmd_t *lrmd)
+{
+ lrmd_private_t *native = lrmd->lrmd_private;
+
+ if (native->remote == NULL) {
+ return NULL;
+ } else {
+ return native->remote->start_state;
+ }
+}
--
2.31.1
From e5e4d43f847da0930bae12f63c7e9d9c44c07cdf Mon Sep 17 00:00:00 2001
From: Chris Lumens <clumens@redhat.com>
Date: Tue, 2 May 2023 10:07:58 -0400
Subject: [PATCH 4/7] Refactor: controller: Make set_join_state a public
function.
This already does all the work of setting a node's start state. It just
needs to be made public and given arguments for what node to set instead
of reading globals.
Ref T183
---
daemons/controld/controld_join_client.c | 20 ++++++++++----------
daemons/controld/pacemaker-controld.h | 3 +++
2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/daemons/controld/controld_join_client.c b/daemons/controld/controld_join_client.c
index da6a9d6..07e2a27 100644
--- a/daemons/controld/controld_join_client.c
+++ b/daemons/controld/controld_join_client.c
@@ -195,32 +195,31 @@ join_query_callback(xmlNode * msg, int call_id, int rc, xmlNode * output, void *
free_xml(generation);
}
-static void
-set_join_state(const char * start_state)
+void
+set_join_state(const char *start_state, const char *node_name, const char *node_uuid)
{
if (pcmk__str_eq(start_state, "standby", pcmk__str_casei)) {
crm_notice("Forcing node %s to join in %s state per configured "
- "environment", controld_globals.our_nodename, start_state);
+ "environment", node_name, start_state);
cib__update_node_attr(controld_globals.logger_out,
controld_globals.cib_conn, cib_sync_call,
- XML_CIB_TAG_NODES, controld_globals.our_uuid,
+ XML_CIB_TAG_NODES, node_uuid,
NULL, NULL, NULL, "standby", "on", NULL, NULL);
} else if (pcmk__str_eq(start_state, "online", pcmk__str_casei)) {
crm_notice("Forcing node %s to join in %s state per configured "
- "environment", controld_globals.our_nodename, start_state);
+ "environment", node_name, start_state);
cib__update_node_attr(controld_globals.logger_out,
controld_globals.cib_conn, cib_sync_call,
- XML_CIB_TAG_NODES, controld_globals.our_uuid,
+ XML_CIB_TAG_NODES, node_uuid,
NULL, NULL, NULL, "standby", "off", NULL, NULL);
} else if (pcmk__str_eq(start_state, "default", pcmk__str_casei)) {
- crm_debug("Not forcing a starting state on node %s",
- controld_globals.our_nodename);
+ crm_debug("Not forcing a starting state on node %s", node_name);
} else {
crm_warn("Unrecognized start state '%s', using 'default' (%s)",
- start_state, controld_globals.our_nodename);
+ start_state, node_name);
}
}
@@ -335,7 +334,8 @@ do_cl_join_finalize_respond(long long action,
first_join = FALSE;
if (start_state) {
- set_join_state(start_state);
+ set_join_state(start_state, controld_globals.our_nodename,
+ controld_globals.our_uuid);
}
}
diff --git a/daemons/controld/pacemaker-controld.h b/daemons/controld/pacemaker-controld.h
index 1484a00..d8c2ddd 100644
--- a/daemons/controld/pacemaker-controld.h
+++ b/daemons/controld/pacemaker-controld.h
@@ -36,4 +36,7 @@ void controld_remove_voter(const char *uname);
void controld_election_fini(void);
void controld_stop_current_election_timeout(void);
+void set_join_state(const char *start_state, const char *node_name,
+ const char *node_uuid);
+
#endif
--
2.31.1
From 63d069adb344bba2c982013226f87dfd95afaff3 Mon Sep 17 00:00:00 2001
From: Chris Lumens <clumens@redhat.com>
Date: Tue, 2 May 2023 13:38:03 -0400
Subject: [PATCH 5/7] Refactor: controller: set_join_state needs to take a
remote parameter.
Without this parameter, we won't know what to pass to as node_type to
cib__update_node_attr. And without that, that function will not know to
update a remote node - it'll try to update a regular node by the same
name, which either doesn't exist or is not what we were hoping would
happen.
Ref T138
---
daemons/controld/controld_join_client.c | 11 +++++++----
daemons/controld/pacemaker-controld.h | 2 +-
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/daemons/controld/controld_join_client.c b/daemons/controld/controld_join_client.c
index 07e2a27..799d1b4 100644
--- a/daemons/controld/controld_join_client.c
+++ b/daemons/controld/controld_join_client.c
@@ -196,7 +196,8 @@ join_query_callback(xmlNode * msg, int call_id, int rc, xmlNode * output, void *
}
void
-set_join_state(const char *start_state, const char *node_name, const char *node_uuid)
+set_join_state(const char *start_state, const char *node_name, const char *node_uuid,
+ bool remote)
{
if (pcmk__str_eq(start_state, "standby", pcmk__str_casei)) {
crm_notice("Forcing node %s to join in %s state per configured "
@@ -204,7 +205,8 @@ set_join_state(const char *start_state, const char *node_name, const char *node_
cib__update_node_attr(controld_globals.logger_out,
controld_globals.cib_conn, cib_sync_call,
XML_CIB_TAG_NODES, node_uuid,
- NULL, NULL, NULL, "standby", "on", NULL, NULL);
+ NULL, NULL, NULL, "standby", "on", NULL,
+ remote ? "remote" : NULL);
} else if (pcmk__str_eq(start_state, "online", pcmk__str_casei)) {
crm_notice("Forcing node %s to join in %s state per configured "
@@ -212,7 +214,8 @@ set_join_state(const char *start_state, const char *node_name, const char *node_
cib__update_node_attr(controld_globals.logger_out,
controld_globals.cib_conn, cib_sync_call,
XML_CIB_TAG_NODES, node_uuid,
- NULL, NULL, NULL, "standby", "off", NULL, NULL);
+ NULL, NULL, NULL, "standby", "off", NULL,
+ remote ? "remote" : NULL);
} else if (pcmk__str_eq(start_state, "default", pcmk__str_casei)) {
crm_debug("Not forcing a starting state on node %s", node_name);
@@ -335,7 +338,7 @@ do_cl_join_finalize_respond(long long action,
first_join = FALSE;
if (start_state) {
set_join_state(start_state, controld_globals.our_nodename,
- controld_globals.our_uuid);
+ controld_globals.our_uuid, false);
}
}
diff --git a/daemons/controld/pacemaker-controld.h b/daemons/controld/pacemaker-controld.h
index d8c2ddd..2334cce 100644
--- a/daemons/controld/pacemaker-controld.h
+++ b/daemons/controld/pacemaker-controld.h
@@ -37,6 +37,6 @@ void controld_election_fini(void);
void controld_stop_current_election_timeout(void);
void set_join_state(const char *start_state, const char *node_name,
- const char *node_uuid);
+ const char *node_uuid, bool remote);
#endif
--
2.31.1
From 67274787898355065315f8c06d62458e2c2b0afe Mon Sep 17 00:00:00 2001
From: Chris Lumens <clumens@redhat.com>
Date: Tue, 2 May 2023 10:09:02 -0400
Subject: [PATCH 6/7] Feature: controller: When a remote node starts, apply any
start state.
If we were given a start state in the handshake XML, that is now stored
in the remote node cache's private data. Extract it and set the state
on the node with set_node_state.
Fixes T183
---
daemons/controld/controld_remote_ra.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/daemons/controld/controld_remote_ra.c b/daemons/controld/controld_remote_ra.c
index f24b755..8ab1e46 100644
--- a/daemons/controld/controld_remote_ra.c
+++ b/daemons/controld/controld_remote_ra.c
@@ -280,6 +280,7 @@ remote_node_up(const char *node_name)
int call_opt;
xmlNode *update, *state;
crm_node_t *node;
+ lrm_state_t *connection_rsc = NULL;
CRM_CHECK(node_name != NULL, return);
crm_info("Announcing Pacemaker Remote node %s", node_name);
@@ -301,6 +302,20 @@ remote_node_up(const char *node_name)
purge_remote_node_attrs(call_opt, node);
pcmk__update_peer_state(__func__, node, CRM_NODE_MEMBER, 0);
+ /* Apply any start state that we were given from the environment on the
+ * remote node.
+ */
+ connection_rsc = lrm_state_find(node->uname);
+
+ if (connection_rsc != NULL) {
+ lrmd_t *lrm = connection_rsc->conn;
+ const char *start_state = lrmd__node_start_state(lrm);
+
+ if (start_state) {
+ set_join_state(start_state, node->uname, node->uuid, true);
+ }
+ }
+
/* pacemaker_remote nodes don't participate in the membership layer,
* so cluster nodes don't automatically get notified when they come and go.
* We send a cluster message to the DC, and update the CIB node state entry,
--
2.31.1
From 91cdda7056c9b9254a0d7e7a016b30f788e3e3ff Mon Sep 17 00:00:00 2001
From: Chris Lumens <clumens@redhat.com>
Date: Tue, 2 May 2023 10:16:30 -0400
Subject: [PATCH 7/7] Doc: sysconfig: Remote nodes now respect start state.
Ref T183
---
etc/sysconfig/pacemaker.in | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/etc/sysconfig/pacemaker.in b/etc/sysconfig/pacemaker.in
index 3b03ad6..041da71 100644
--- a/etc/sysconfig/pacemaker.in
+++ b/etc/sysconfig/pacemaker.in
@@ -144,8 +144,7 @@
# By default, the local host will join the cluster in an online or standby
# state when Pacemaker first starts depending on whether it was previously put
# into standby mode. If this variable is set to "standby" or "online", it will
-# force the local host to join in the specified state. This has no effect on
-# Pacemaker Remote nodes.
+# force the local host to join in the specified state.
#
# Default: PCMK_node_start_state="default"
--
2.31.1

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

View File

@ -1,42 +0,0 @@
From a3bffc7c66bf6f796f977cffd44f223635b008c5 Mon Sep 17 00:00:00 2001
From: Reid Wahl <nrwahl@protonmail.com>
Date: Wed, 20 Dec 2023 13:33:47 -0800
Subject: [PATCH] Doc: Pacemaker Explained: Add replace for
PCMK__REMOTE_SCHEMA_DIR
So that the existing use in local-options.rst expands correctly.
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
---
doc/sphinx/Makefile.am | 1 +
doc/sphinx/conf.py.in | 1 +
3 files changed, 2 insertions(+)
create mode 100644 doc/sphinx/conf.py.in.rej
diff --git a/doc/sphinx/Makefile.am b/doc/sphinx/Makefile.am
index e48e19a..d0309ff 100644
--- a/doc/sphinx/Makefile.am
+++ b/doc/sphinx/Makefile.am
@@ -134,6 +134,7 @@ $(BOOKS:%=%/conf.py): conf.py.in
-e 's#%CRM_SCHEMA_DIRECTORY%#@CRM_SCHEMA_DIRECTORY@#g' \
-e 's#%PACEMAKER_CONFIG_DIR%#@PACEMAKER_CONFIG_DIR@#g' \
-e 's#%PCMK_GNUTLS_PRIORITIES%#@PCMK_GNUTLS_PRIORITIES@#g' \
+ -e 's#%PCMK__REMOTE_SCHEMA_DIR%#@PCMK__REMOTE_SCHEMA_DIR@#g' \
$(<) > "$@"
$(BOOK)/_build: $(STATIC_FILES) $(BOOK)/conf.py $(DEPS_$(BOOK)) $(wildcard $(srcdir)/$(BOOK)/*.rst)
diff --git a/doc/sphinx/conf.py.in b/doc/sphinx/conf.py.in
index 556eb72..511f029 100644
--- a/doc/sphinx/conf.py.in
+++ b/doc/sphinx/conf.py.in
@@ -40,6 +40,7 @@ rst_prolog="""
.. |PCMK_INIT_ENV_FILE| replace:: ``%PACEMAKER_CONFIG_DIR%/pcmk-init.env``
.. |PCMK_LOG_FILE| replace:: %CRM_LOG_DIR%/pacemaker.log
.. |PCMK_GNUTLS_PRIORITIES| replace:: %PCMK_GNUTLS_PRIORITIES%
+.. |PCMK__REMOTE_SCHEMA_DIR| replace:: %PCMK__REMOTE_SCHEMA_DIR%
.. |REMOTE_DISTRO| replace:: AlmaLinux
.. |REMOTE_DISTRO_VER| replace:: 9
"""
--
2.31.1

File diff suppressed because it is too large Load Diff

View File

@ -0,0 +1,35 @@
From 770d417e28dc9527fec8b8a00caaba8825995454 Mon Sep 17 00:00:00 2001
From: Grace Chin <gchin@redhat.com>
Date: Wed, 19 Jul 2023 10:25:55 -0400
Subject: [PATCH] Fix: tools: Fix a bug in clone resource description display
Previously, descriptions of resources running on multiple
nodes were displayed despite --full not being used (with pcs
status) or --show-detail not being used (with crm_mon).
For example, clone resources running on multiple nodes were
affected.
Now, --full and --show-detail must be used in order for resource
descriptions to be displayed, regardless of the number of nodes
the resource is run on.
see bz: 2106642
---
lib/pengine/pe_output.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/lib/pengine/pe_output.c b/lib/pengine/pe_output.c
index e0b43d997a..d1c9f6e226 100644
--- a/lib/pengine/pe_output.c
+++ b/lib/pengine/pe_output.c
@@ -20,8 +20,7 @@ pe__resource_description(const pe_resource_t *rsc, uint32_t show_opts)
{
const char * desc = NULL;
// User-supplied description
- if (pcmk_any_flags_set(show_opts, pcmk_show_rsc_only|pcmk_show_description)
- || pcmk__list_of_multiple(rsc->running_on)) {
+ if (pcmk_any_flags_set(show_opts, pcmk_show_rsc_only|pcmk_show_description)) {
desc = crm_element_value(rsc->xml, XML_ATTR_DESC);
}
return desc;

File diff suppressed because it is too large Load Diff

View File

@ -0,0 +1,26 @@
From ebac530c815a62f7c3a1c24f64e9a530d9753dbe Mon Sep 17 00:00:00 2001
From: Hideo Yamauchi <renayama19661014@ybb.ne.jp>
Date: Wed, 19 Jul 2023 18:21:07 +0900
Subject: [PATCH] High: tools: The dampen parameter is disabled when setting
values with attrd_updater.
---
tools/attrd_updater.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/attrd_updater.c b/tools/attrd_updater.c
index b615a3575..4688b9ff6 100644
--- a/tools/attrd_updater.c
+++ b/tools/attrd_updater.c
@@ -501,7 +501,7 @@ send_attrd_update(char command, const char *attr_node, const char *attr_name,
case 'U':
rc = pcmk__attrd_api_update(NULL, attr_node, attr_name, attr_value,
- NULL, attr_set, NULL,
+ attr_dampen, attr_set, NULL,
attr_options | pcmk__node_attr_value);
break;
--
2.41.0

View File

@ -1,276 +0,0 @@
From d50bbafc32428e873c0052a9defcf93d2e52667e Mon Sep 17 00:00:00 2001
From: Chris Lumens <clumens@redhat.com>
Date: Wed, 10 Jan 2024 11:35:11 -0500
Subject: [PATCH 1/3] Refactor: libcrmcommon: Split feature set check into its
own function.
---
include/crm/common/cib_internal.h | 4 +++-
lib/cib/cib_utils.c | 12 ++++++------
lib/common/cib.c | 18 +++++++++++++++++-
3 files changed, 26 insertions(+), 8 deletions(-)
diff --git a/include/crm/common/cib_internal.h b/include/crm/common/cib_internal.h
index c41c12e..fa65e58 100644
--- a/include/crm/common/cib_internal.h
+++ b/include/crm/common/cib_internal.h
@@ -1,5 +1,5 @@
/*
- * Copyright 2023 the Pacemaker project contributors
+ * Copyright 2023-2024 the Pacemaker project contributors
*
* The version control history for this file may have further details.
*
@@ -16,6 +16,8 @@ extern "C" {
const char *pcmk__cib_abs_xpath_for(const char *element);
+int pcmk__check_feature_set(const char *cib_version);
+
#ifdef __cplusplus
}
#endif
diff --git a/lib/cib/cib_utils.c b/lib/cib/cib_utils.c
index 0082eef..bf2982c 100644
--- a/lib/cib/cib_utils.c
+++ b/lib/cib/cib_utils.c
@@ -353,7 +353,6 @@ cib_perform_op(const char *op, int call_options, cib__op_fn_t fn, bool is_query,
xmlNode *patchset_cib = NULL;
xmlNode *local_diff = NULL;
- const char *new_version = NULL;
const char *user = crm_element_value(req, F_CIB_USER);
bool with_digest = false;
@@ -470,12 +469,13 @@ cib_perform_op(const char *op, int call_options, cib__op_fn_t fn, bool is_query,
}
if (scratch) {
- new_version = crm_element_value(scratch, XML_ATTR_CRM_VERSION);
+ const char *new_version = crm_element_value(scratch, XML_ATTR_CRM_VERSION);
- if (new_version && compare_version(new_version, CRM_FEATURE_SET) > 0) {
- crm_err("Discarding update with feature set '%s' greater than our own '%s'",
- new_version, CRM_FEATURE_SET);
- rc = -EPROTONOSUPPORT;
+ rc = pcmk__check_feature_set(new_version);
+ if (rc != pcmk_rc_ok) {
+ pcmk__config_err("Discarding update with feature set '%s' greater than our own '%s'",
+ new_version, CRM_FEATURE_SET);
+ rc = pcmk_rc2legacy(rc);
goto done;
}
}
diff --git a/lib/common/cib.c b/lib/common/cib.c
index fee7881..cbebc2e 100644
--- a/lib/common/cib.c
+++ b/lib/common/cib.c
@@ -1,6 +1,6 @@
/*
* Original copyright 2004 International Business Machines
- * Later changes copyright 2008-2023 the Pacemaker project contributors
+ * Later changes copyright 2008-2024 the Pacemaker project contributors
*
* The version control history for this file may have further details.
*
@@ -173,3 +173,19 @@ pcmk_find_cib_element(xmlNode *cib, const char *element_name)
{
return get_xpath_object(pcmk_cib_xpath_for(element_name), cib, LOG_TRACE);
}
+
+/*!
+ * \internal
+ * \brief Check that the feature set in the CIB is supported on this node
+ *
+ * \param[in] new_version XML_ATTR_CRM_VERSION attribute from the CIB
+ */
+int
+pcmk__check_feature_set(const char *cib_version)
+{
+ if (cib_version && compare_version(cib_version, CRM_FEATURE_SET) > 0) {
+ return EPROTONOSUPPORT;
+ }
+
+ return pcmk_rc_ok;
+}
--
2.31.1
From d89fd8336ae47d892201513c99773705d57f15f0 Mon Sep 17 00:00:00 2001
From: Chris Lumens <clumens@redhat.com>
Date: Wed, 10 Jan 2024 13:46:42 -0500
Subject: [PATCH 2/3] Feature: scheduler: Check the CIB feature set in
cluster_status.
This adds the check that was previously only in cib_perform_op to the
scheduler code, ensuring that any daemon or tool that calls the
scheduler will check that the feature set in the CIB is supported.
---
lib/pengine/status.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/lib/pengine/status.c b/lib/pengine/status.c
index e6ec237..1294803 100644
--- a/lib/pengine/status.c
+++ b/lib/pengine/status.c
@@ -14,6 +14,7 @@
#include <crm/crm.h>
#include <crm/msg_xml.h>
#include <crm/common/xml.h>
+#include <crm/common/cib_internal.h>
#include <glib.h>
@@ -70,12 +71,21 @@ pe_free_working_set(pcmk_scheduler_t *scheduler)
gboolean
cluster_status(pcmk_scheduler_t * scheduler)
{
+ const char *new_version = NULL;
xmlNode *section = NULL;
if ((scheduler == NULL) || (scheduler->input == NULL)) {
return FALSE;
}
+ new_version = crm_element_value(scheduler->input, XML_ATTR_CRM_VERSION);
+
+ if (pcmk__check_feature_set(new_version) != pcmk_rc_ok) {
+ pcmk__config_err("Can't process CIB with feature set '%s' greater than our own '%s'",
+ new_version, CRM_FEATURE_SET);
+ return FALSE;
+ }
+
crm_trace("Beginning unpack");
if (scheduler->failed != NULL) {
--
2.31.1
From a3428926d37af506014a6b462d1308d8541c5932 Mon Sep 17 00:00:00 2001
From: Chris Lumens <clumens@redhat.com>
Date: Wed, 10 Jan 2024 14:56:36 -0500
Subject: [PATCH 3/3] Low: libcib: Do not check CIB feature set for files in
cib_perform_op.
This is related to the previous feature for transferring schema files to
older remote nodes. In that case, the newer schema files may also have
a newer feature set than the node supports, so the transferred files are
still not usable.
However, the feature set only matters for the scheduler, not for most
command line tools (obviously, crm_simulate would still care). So in
those cases, we can just disable the feature set check if the CIB was
read in from a file. For the scheduler, the check is still performed as
part of cluster_status.
---
cts/cli/regression.tools.exp | 2 +-
daemons/based/based_callbacks.c | 4 ++--
include/crm/cib/internal.h | 4 ++--
lib/cib/cib_file.c | 2 +-
lib/cib/cib_utils.c | 15 +++++++++------
5 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/cts/cli/regression.tools.exp b/cts/cli/regression.tools.exp
index 417b5cd..c81c420 100644
--- a/cts/cli/regression.tools.exp
+++ b/cts/cli/regression.tools.exp
@@ -7939,7 +7939,7 @@ unpack_config warning: Blind faith: not fencing unseen nodes
=#=#=#= End test: Verbosely verify a file-specified invalid configuration, outputting as xml - Invalid configuration (78) =#=#=#=
* Passed: crm_verify - Verbosely verify a file-specified invalid configuration, outputting as xml
=#=#=#= Begin test: Verbosely verify another file-specified invalid configuration, outputting as xml =#=#=#=
-(cluster_status@status.c:113) warning: Fencing and resource management disabled due to lack of quorum
+(cluster_status@status.c:123) warning: Fencing and resource management disabled due to lack of quorum
<pacemaker-result api-version="X" request="crm_verify_invalid_no_stonith.xml --output-as=xml --verbose">
<status code="78" message="Invalid configuration">
<errors>
diff --git a/daemons/based/based_callbacks.c b/daemons/based/based_callbacks.c
index 5f3dc62..f16e4d9 100644
--- a/daemons/based/based_callbacks.c
+++ b/daemons/based/based_callbacks.c
@@ -1362,7 +1362,7 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation,
input = prepare_input(request, operation->type, &section);
if (!pcmk_is_set(operation->flags, cib__op_attr_modifies)) {
- rc = cib_perform_op(op, call_options, op_function, true, section,
+ rc = cib_perform_op(NULL, op, call_options, op_function, true, section,
request, input, false, &config_changed, &the_cib,
&result_cib, NULL, &output);
@@ -1395,7 +1395,7 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation,
}
// result_cib must not be modified after cib_perform_op() returns
- rc = cib_perform_op(op, call_options, op_function, false, section,
+ rc = cib_perform_op(NULL, op, call_options, op_function, false, section,
request, input, manage_counters, &config_changed,
&the_cib, &result_cib, cib_diff, &output);
diff --git a/include/crm/cib/internal.h b/include/crm/cib/internal.h
index 9d54d52..b6d6871 100644
--- a/include/crm/cib/internal.h
+++ b/include/crm/cib/internal.h
@@ -1,5 +1,5 @@
/*
- * Copyright 2004-2023 the Pacemaker project contributors
+ * Copyright 2004-2024 the Pacemaker project contributors
*
* The version control history for this file may have further details.
*
@@ -206,7 +206,7 @@ int cib__get_notify_patchset(const xmlNode *msg, const xmlNode **patchset);
bool cib__element_in_patchset(const xmlNode *patchset, const char *element);
-int cib_perform_op(const char *op, int call_options, cib__op_fn_t fn,
+int cib_perform_op(cib_t *cib, const char *op, int call_options, cib__op_fn_t fn,
bool is_query, const char *section, xmlNode *req,
xmlNode *input, bool manage_counters, bool *config_changed,
xmlNode **current_cib, xmlNode **result_cib, xmlNode **diff,
diff --git a/lib/cib/cib_file.c b/lib/cib/cib_file.c
index a279823..9dd952c 100644
--- a/lib/cib/cib_file.c
+++ b/lib/cib/cib_file.c
@@ -245,7 +245,7 @@ cib_file_process_request(cib_t *cib, xmlNode *request, xmlNode **output)
data = pcmk_find_cib_element(data, section);
}
- rc = cib_perform_op(op, call_options, op_function, read_only, section,
+ rc = cib_perform_op(cib, op, call_options, op_function, read_only, section,
request, data, true, &changed, &private->cib_xml,
&result_cib, &cib_diff, output);
diff --git a/lib/cib/cib_utils.c b/lib/cib/cib_utils.c
index bf2982c..9c3f9f1 100644
--- a/lib/cib/cib_utils.c
+++ b/lib/cib/cib_utils.c
@@ -339,11 +339,10 @@ should_copy_cib(const char *op, const char *section, int call_options)
}
int
-cib_perform_op(const char *op, int call_options, cib__op_fn_t fn, bool is_query,
- const char *section, xmlNode *req, xmlNode *input,
- bool manage_counters, bool *config_changed,
- xmlNode **current_cib, xmlNode **result_cib, xmlNode **diff,
- xmlNode **output)
+cib_perform_op(cib_t *cib, const char *op, int call_options, cib__op_fn_t fn,
+ bool is_query, const char *section, xmlNode *req, xmlNode *input,
+ bool manage_counters, bool *config_changed, xmlNode **current_cib,
+ xmlNode **result_cib, xmlNode **diff, xmlNode **output)
{
int rc = pcmk_ok;
bool check_schema = true;
@@ -468,7 +467,11 @@ cib_perform_op(const char *op, int call_options, cib__op_fn_t fn, bool is_query,
goto done;
}
- if (scratch) {
+ /* If the CIB is from a file, we don't need to check that the feature set is
+ * supported. All we care about in that case is the schema version, which
+ * is checked elsewhere.
+ */
+ if (scratch && (cib == NULL || cib->variant != cib_file)) {
const char *new_version = crm_element_value(scratch, XML_ATTR_CRM_VERSION);
rc = pcmk__check_feature_set(new_version);
--
2.31.1

View File

@ -0,0 +1,109 @@
From 3e31da0016795397bfeacb2f3d76ecfe35cc1f67 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Mon, 17 Jul 2023 14:52:42 -0500
Subject: [PATCH] Fix: libcrmcommon: wait for reply from appropriate controller
commands
ipc_controld.c:reply_expected() wrongly omitted PCMK__CONTROLD_CMD_NODES (which
hasn't been a problem because crm_node uses a mainloop instead of sync dispatch
for that) and CRM_OP_RM_NODE_CACHE (which can be sent via
ipc_client.c:pcmk_ipc_purge_node()).
Because CRM_OP_RM_NODE_CACHE gets only an ack and no further replies, we now
have to be careful not to return true from the controller's dispatch()
function, otherwise crm_node -R would wait forever for more data. That means
we have to check for whether any replies are expected, which means we have to
increment expected replies *before* sending a request (in case it's sync).
Regression introduced in 2.0.5 by ae14fa4a
Fixes T681
---
lib/common/ipc_controld.c | 49 ++++++++++++++-------------------------
1 file changed, 17 insertions(+), 32 deletions(-)
diff --git a/lib/common/ipc_controld.c b/lib/common/ipc_controld.c
index 3c3a98964..405fd0518 100644
--- a/lib/common/ipc_controld.c
+++ b/lib/common/ipc_controld.c
@@ -177,18 +177,16 @@ set_nodes_data(pcmk_controld_api_reply_t *data, xmlNode *msg_data)
static bool
reply_expected(pcmk_ipc_api_t *api, xmlNode *request)
{
- const char *command = crm_element_value(request, F_CRM_TASK);
-
- if (command == NULL) {
- return false;
- }
-
- // We only need to handle commands that functions in this file can send
- return !strcmp(command, CRM_OP_REPROBE)
- || !strcmp(command, CRM_OP_NODE_INFO)
- || !strcmp(command, CRM_OP_PING)
- || !strcmp(command, CRM_OP_LRM_FAIL)
- || !strcmp(command, CRM_OP_LRM_DELETE);
+ // We only need to handle commands that API functions can send
+ return pcmk__str_any_of(crm_element_value(request, F_CRM_TASK),
+ PCMK__CONTROLD_CMD_NODES,
+ CRM_OP_LRM_DELETE,
+ CRM_OP_LRM_FAIL,
+ CRM_OP_NODE_INFO,
+ CRM_OP_PING,
+ CRM_OP_REPROBE,
+ CRM_OP_RM_NODE_CACHE,
+ NULL);
}
static bool
@@ -202,22 +200,12 @@ dispatch(pcmk_ipc_api_t *api, xmlNode *reply)
pcmk_controld_reply_unknown, NULL, NULL,
};
- /* If we got an ACK, return true so the caller knows to expect more responses
- * from the IPC server. We do this before decrementing replies_expected because
- * ACKs are not going to be included in that value.
- *
- * Note that we cannot do the same kind of status checking here that we do in
- * ipc_pacemakerd.c. The ACK message we receive does not necessarily contain
- * a status attribute. That is, we may receive this:
- *
- * <ack function="crmd_remote_proxy_cb" line="556"/>
- *
- * Instead of this:
- *
- * <ack function="dispatch_controller_ipc" line="391" status="112"/>
- */
if (pcmk__str_eq(crm_element_name(reply), "ack", pcmk__str_none)) {
- return true; // More replies needed
+ /* ACKs are trivial responses that do not count toward expected replies,
+ * and do not have all the fields that validation requires, so skip that
+ * processing.
+ */
+ return private->replies_expected > 0;
}
if (private->replies_expected > 0) {
@@ -344,18 +332,15 @@ static int
send_controller_request(pcmk_ipc_api_t *api, xmlNode *request,
bool reply_is_expected)
{
- int rc;
-
if (crm_element_value(request, XML_ATTR_REFERENCE) == NULL) {
return EINVAL;
}
- rc = pcmk__send_ipc_request(api, request);
- if ((rc == pcmk_rc_ok) && reply_is_expected) {
+ if (reply_is_expected) {
struct controld_api_private_s *private = api->api_data;
private->replies_expected++;
}
- return rc;
+ return pcmk__send_ipc_request(api, request);
}
static xmlNode *
--
2.41.0

View File

@ -0,0 +1,163 @@
From 63f4bd4d5a324e6eb279340a42c7c36c8902ada7 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Wed, 2 Aug 2023 15:55:26 -0500
Subject: [PATCH 1/4] Fix: controller: don't try to execute agent action at
shutdown
Normally, agent execution is not possible at shutdown. However, when metadata
is needed for some action, the agent can be called asynchronously, and when the
metadata action returns, the original action is performed. If the metadata is
initiated before shutdown, but completes after shutdown has begun, do not try
to attempt the original action, so we avoid unnecessary error logs.
---
daemons/controld/controld_execd.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/daemons/controld/controld_execd.c b/daemons/controld/controld_execd.c
index 530e4346c8..a90e8d833e 100644
--- a/daemons/controld/controld_execd.c
+++ b/daemons/controld/controld_execd.c
@@ -1400,7 +1400,9 @@ metadata_complete(int pid, const pcmk__action_result_t *result, void *user_data)
md = controld_cache_metadata(lrm_state->metadata_cache, data->rsc,
result->action_stdout);
}
- do_lrm_rsc_op(lrm_state, data->rsc, data->input_xml, md);
+ if (!pcmk_is_set(controld_globals.fsa_input_register, R_HA_DISCONNECTED)) {
+ do_lrm_rsc_op(lrm_state, data->rsc, data->input_xml, md);
+ }
free_metadata_cb_data(data);
}
From 247d9534f36f690c1474e36cedaadb3934022a05 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Wed, 2 Aug 2023 16:16:31 -0500
Subject: [PATCH 2/4] Refactor: controller: de-functionize lrm_state_destroy()
It was a one-liner called once
---
daemons/controld/controld_execd_state.c | 8 +-------
daemons/controld/controld_lrm.h | 5 -----
2 files changed, 1 insertion(+), 12 deletions(-)
diff --git a/daemons/controld/controld_execd_state.c b/daemons/controld/controld_execd_state.c
index 8c68bfca08..4a87a9b332 100644
--- a/daemons/controld/controld_execd_state.c
+++ b/daemons/controld/controld_execd_state.c
@@ -132,12 +132,6 @@ lrm_state_create(const char *node_name)
return state;
}
-void
-lrm_state_destroy(const char *node_name)
-{
- g_hash_table_remove(lrm_state_table, node_name);
-}
-
static gboolean
remote_proxy_remove_by_node(gpointer key, gpointer value, gpointer user_data)
{
@@ -799,7 +793,7 @@ lrm_state_unregister_rsc(lrm_state_t * lrm_state,
}
if (is_remote_lrmd_ra(NULL, NULL, rsc_id)) {
- lrm_state_destroy(rsc_id);
+ g_hash_table_remove(lrm_state_table, rsc_id);
return pcmk_ok;
}
diff --git a/daemons/controld/controld_lrm.h b/daemons/controld/controld_lrm.h
index 25f3db3316..c3113e49c3 100644
--- a/daemons/controld/controld_lrm.h
+++ b/daemons/controld/controld_lrm.h
@@ -108,11 +108,6 @@ gboolean lrm_state_init_local(void);
*/
void lrm_state_destroy_all(void);
-/*!
- * \brief Destroy executor connection by node name
- */
-void lrm_state_destroy(const char *node_name);
-
/*!
* \brief Find lrm_state data by node name
*/
From 1b915f1ce38756431f7faa142565e3e07aade194 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Wed, 2 Aug 2023 15:58:09 -0500
Subject: [PATCH 3/4] Low: controller: guard lrm_state_table usage with NULL
check
It is NULL while draining the mainloop during the shutdown sequence.
---
daemons/controld/controld_execd_state.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/daemons/controld/controld_execd_state.c b/daemons/controld/controld_execd_state.c
index 4a87a9b332..b90cc5e635 100644
--- a/daemons/controld/controld_execd_state.c
+++ b/daemons/controld/controld_execd_state.c
@@ -301,7 +301,7 @@ lrm_state_destroy_all(void)
lrm_state_t *
lrm_state_find(const char *node_name)
{
- if (!node_name) {
+ if ((node_name == NULL) || (lrm_state_table == NULL)) {
return NULL;
}
return g_hash_table_lookup(lrm_state_table, node_name);
@@ -312,6 +312,8 @@ lrm_state_find_or_create(const char *node_name)
{
lrm_state_t *lrm_state;
+ CRM_CHECK(lrm_state_table != NULL, return NULL);
+
lrm_state = g_hash_table_lookup(lrm_state_table, node_name);
if (!lrm_state) {
lrm_state = lrm_state_create(node_name);
@@ -323,6 +325,9 @@ lrm_state_find_or_create(const char *node_name)
GList *
lrm_state_get_list(void)
{
+ if (lrm_state_table == NULL) {
+ return NULL;
+ }
return g_hash_table_get_values(lrm_state_table);
}
From 78581213ed3bf4183b0ec1f391b720d5d91f3f68 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Wed, 2 Aug 2023 15:48:36 -0500
Subject: [PATCH 4/4] Log: controller: improve messages for resource history
updates
---
daemons/controld/controld_cib.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/daemons/controld/controld_cib.c b/daemons/controld/controld_cib.c
index 22ac42486f..c9dde0b748 100644
--- a/daemons/controld/controld_cib.c
+++ b/daemons/controld/controld_cib.c
@@ -861,10 +861,17 @@ cib_rsc_callback(xmlNode * msg, int call_id, int rc, xmlNode * output, void *use
case pcmk_ok:
case -pcmk_err_diff_failed:
case -pcmk_err_diff_resync:
- crm_trace("Resource update %d complete: rc=%d", call_id, rc);
+ crm_trace("Resource history update completed (call=%d rc=%d)",
+ call_id, rc);
break;
default:
- crm_warn("Resource update %d failed: (rc=%d) %s", call_id, rc, pcmk_strerror(rc));
+ if (call_id > 0) {
+ crm_warn("Resource history update %d failed: %s "
+ CRM_XS " rc=%d", call_id, pcmk_strerror(rc), rc);
+ } else {
+ crm_warn("Resource history update failed: %s " CRM_XS " rc=%d",
+ pcmk_strerror(rc), rc);
+ }
}
if (call_id == pending_rsc_update) {

File diff suppressed because it is too large Load Diff

View File

@ -1,373 +0,0 @@
From 4823643bef8801b33688167b159bb531bcdf8911 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 4 Jan 2024 17:10:08 -0600
Subject: [PATCH 1/5] Refactor: pacemaker-attrd: drop redundant argument from
update_attr_on_host()
It can check for a force-write via its xml argument, to simplify the caller
---
daemons/attrd/attrd_corosync.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/daemons/attrd/attrd_corosync.c b/daemons/attrd/attrd_corosync.c
index 158d82f..1b56923 100644
--- a/daemons/attrd/attrd_corosync.c
+++ b/daemons/attrd/attrd_corosync.c
@@ -266,7 +266,7 @@ record_peer_nodeid(attribute_value_t *v, const char *host)
static void
update_attr_on_host(attribute_t *a, const crm_node_t *peer, const xmlNode *xml,
const char *attr, const char *value, const char *host,
- bool filter, int is_force_write)
+ bool filter)
{
attribute_value_t *v = NULL;
@@ -309,6 +309,10 @@ update_attr_on_host(attribute_t *a, const crm_node_t *peer, const xmlNode *xml,
}
} else {
+ int is_force_write = 0;
+
+ crm_element_value_int(xml, PCMK__XA_ATTR_FORCE, &is_force_write);
+
if (is_force_write == 1 && a->timeout_ms && a->timer) {
/* Save forced writing and set change flag. */
/* The actual attribute is written by Writer after election. */
@@ -338,15 +342,12 @@ attrd_peer_update_one(const crm_node_t *peer, xmlNode *xml, bool filter)
const char *attr = crm_element_value(xml, PCMK__XA_ATTR_NAME);
const char *value = crm_element_value(xml, PCMK__XA_ATTR_VALUE);
const char *host = crm_element_value(xml, PCMK__XA_ATTR_NODE_NAME);
- int is_force_write = 0;
if (attr == NULL) {
crm_warn("Could not update attribute: peer did not specify name");
return;
}
- crm_element_value_int(xml, PCMK__XA_ATTR_FORCE, &is_force_write);
-
a = attrd_populate_attribute(xml, attr);
if (a == NULL) {
return;
@@ -361,12 +362,12 @@ attrd_peer_update_one(const crm_node_t *peer, xmlNode *xml, bool filter)
g_hash_table_iter_init(&vIter, a->values);
while (g_hash_table_iter_next(&vIter, (gpointer *) & host, NULL)) {
- update_attr_on_host(a, peer, xml, attr, value, host, filter, is_force_write);
+ update_attr_on_host(a, peer, xml, attr, value, host, filter);
}
} else {
// Update attribute value for the given host
- update_attr_on_host(a, peer, xml, attr, value, host, filter, is_force_write);
+ update_attr_on_host(a, peer, xml, attr, value, host, filter);
}
/* If this is a message from some attrd instance broadcasting its protocol
--
2.31.1
From c7a1ab819b25e3225c185c1630a7139a96fb5c71 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Tue, 9 Jan 2024 16:48:37 -0600
Subject: [PATCH 2/5] Refactor: pacemaker-attrd: drop unused argument from
attrd_peer_sync()
---
daemons/attrd/attrd_corosync.c | 10 ++++++++--
daemons/attrd/attrd_elections.c | 2 +-
daemons/attrd/attrd_messages.c | 2 +-
daemons/attrd/pacemaker-attrd.h | 2 +-
4 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/daemons/attrd/attrd_corosync.c b/daemons/attrd/attrd_corosync.c
index 1b56923..088f00c 100644
--- a/daemons/attrd/attrd_corosync.c
+++ b/daemons/attrd/attrd_corosync.c
@@ -233,7 +233,7 @@ attrd_peer_change_cb(enum crm_status_type kind, crm_node_t *peer, const void *da
*/
if (attrd_election_won()
&& !pcmk_is_set(peer->flags, crm_remote_node)) {
- attrd_peer_sync(peer, NULL);
+ attrd_peer_sync(peer);
}
} else {
// Remove all attribute values associated with lost nodes
@@ -535,8 +535,14 @@ attrd_peer_remove(const char *host, bool uncache, const char *source)
}
}
+/*!
+ * \internal
+ * \brief Send all known attributes and values to a peer
+ *
+ * \param[in] peer Peer to send sync to (if NULL, broadcast to all peers)
+ */
void
-attrd_peer_sync(crm_node_t *peer, xmlNode *xml)
+attrd_peer_sync(crm_node_t *peer)
{
GHashTableIter aIter;
GHashTableIter vIter;
diff --git a/daemons/attrd/attrd_elections.c b/daemons/attrd/attrd_elections.c
index 82fbe8a..9dbf133 100644
--- a/daemons/attrd/attrd_elections.c
+++ b/daemons/attrd/attrd_elections.c
@@ -23,7 +23,7 @@ attrd_election_cb(gpointer user_data)
attrd_declare_winner();
/* Update the peers after an election */
- attrd_peer_sync(NULL, NULL);
+ attrd_peer_sync(NULL);
/* After winning an election, update the CIB with the values of all
* attributes as the winner knows them.
diff --git a/daemons/attrd/attrd_messages.c b/daemons/attrd/attrd_messages.c
index 5525d4b..13ac01f 100644
--- a/daemons/attrd/attrd_messages.c
+++ b/daemons/attrd/attrd_messages.c
@@ -180,7 +180,7 @@ handle_sync_request(pcmk__request_t *request)
crm_node_t *peer = pcmk__get_node(0, request->peer, NULL,
pcmk__node_search_cluster);
- attrd_peer_sync(peer, request->xml);
+ attrd_peer_sync(peer);
pcmk__set_result(&request->result, CRM_EX_OK, PCMK_EXEC_DONE, NULL);
return NULL;
} else {
diff --git a/daemons/attrd/pacemaker-attrd.h b/daemons/attrd/pacemaker-attrd.h
index 7384188..bacaad6 100644
--- a/daemons/attrd/pacemaker-attrd.h
+++ b/daemons/attrd/pacemaker-attrd.h
@@ -175,7 +175,7 @@ extern GHashTable *peer_protocol_vers;
int attrd_cluster_connect(void);
void attrd_peer_update(const crm_node_t *peer, xmlNode *xml, const char *host,
bool filter);
-void attrd_peer_sync(crm_node_t *peer, xmlNode *xml);
+void attrd_peer_sync(crm_node_t *peer);
void attrd_peer_remove(const char *host, bool uncache, const char *source);
void attrd_peer_clear_failure(pcmk__request_t *request);
void attrd_peer_sync_response(const crm_node_t *peer, bool peer_won,
--
2.31.1
From abafae0068e10abb135b0496086947728365299a Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 11 Jan 2024 17:31:17 -0600
Subject: [PATCH 3/5] Refactor: pacemaker-attrd: de-functionize
attrd_lookup_or_create_value()
... to make planned changes easier
---
daemons/attrd/attrd_corosync.c | 62 +++++++++++++---------------------
1 file changed, 24 insertions(+), 38 deletions(-)
diff --git a/daemons/attrd/attrd_corosync.c b/daemons/attrd/attrd_corosync.c
index 088f00c..59e6a26 100644
--- a/daemons/attrd/attrd_corosync.c
+++ b/daemons/attrd/attrd_corosync.c
@@ -168,40 +168,6 @@ broadcast_local_value(const attribute_t *a)
#define state_text(state) pcmk__s((state), "in unknown state")
-/*!
- * \internal
- * \brief Return a node's value from hash table (creating one if needed)
- *
- * \param[in,out] values Hash table of values
- * \param[in] node_name Name of node to look up
- * \param[in] xml XML describing the attribute
- *
- * \return Pointer to new or existing hash table entry
- */
-static attribute_value_t *
-attrd_lookup_or_create_value(GHashTable *values, const char *node_name,
- const xmlNode *xml)
-{
- attribute_value_t *v = g_hash_table_lookup(values, node_name);
- int is_remote = 0;
-
- if (v == NULL) {
- v = calloc(1, sizeof(attribute_value_t));
- CRM_ASSERT(v != NULL);
-
- pcmk__str_update(&v->nodename, node_name);
- g_hash_table_replace(values, v->nodename, v);
- }
-
- crm_element_value_int(xml, PCMK__XA_ATTR_IS_REMOTE, &is_remote);
- if (is_remote) {
- attrd_set_value_flags(v, attrd_value_remote);
- CRM_ASSERT(crm_remote_peer_get(node_name) != NULL);
- }
-
- return(v);
-}
-
static void
attrd_peer_change_cb(enum crm_status_type kind, crm_node_t *peer, const void *data)
{
@@ -268,18 +234,38 @@ update_attr_on_host(attribute_t *a, const crm_node_t *peer, const xmlNode *xml,
const char *attr, const char *value, const char *host,
bool filter)
{
+ int is_remote = 0;
+ bool changed = false;
attribute_value_t *v = NULL;
- v = attrd_lookup_or_create_value(a->values, host, xml);
+ // Create entry for value if not already existing
+ v = g_hash_table_lookup(a->values, host);
+ if (v == NULL) {
+ v = calloc(1, sizeof(attribute_value_t));
+ CRM_ASSERT(v != NULL);
+
+ pcmk__str_update(&v->nodename, host);
+ g_hash_table_replace(a->values, v->nodename, v);
+ }
+
+ // If value is for a Pacemaker Remote node, remember that
+ crm_element_value_int(xml, PCMK__XA_ATTR_IS_REMOTE, &is_remote);
+ if (is_remote) {
+ attrd_set_value_flags(v, attrd_value_remote);
+ CRM_ASSERT(crm_remote_peer_get(host) != NULL);
+ }
+
+ // Check whether the value changed
+ changed = !pcmk__str_eq(v->current, value, pcmk__str_casei);
- if (filter && !pcmk__str_eq(v->current, value, pcmk__str_casei)
- && pcmk__str_eq(host, attrd_cluster->uname, pcmk__str_casei)) {
+ if (changed && filter && pcmk__str_eq(host, attrd_cluster->uname,
+ pcmk__str_casei)) {
crm_notice("%s[%s]: local value '%s' takes priority over '%s' from %s",
attr, host, v->current, value, peer->uname);
v = broadcast_local_value(a);
- } else if (!pcmk__str_eq(v->current, value, pcmk__str_casei)) {
+ } else if (changed) {
crm_notice("Setting %s[%s]%s%s: %s -> %s "
CRM_XS " from %s with %s write delay",
attr, host, a->set_type ? " in " : "",
--
2.31.1
From 72529ec512fb4938bd8dbbd2caf44bbb1a616826 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 11 Jan 2024 18:04:33 -0600
Subject: [PATCH 4/5] Refactor: pacemaker-attrd: minor shuffling to make
planned changes easier
---
daemons/attrd/attrd_cib.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/daemons/attrd/attrd_cib.c b/daemons/attrd/attrd_cib.c
index bdc0a10..481fea7 100644
--- a/daemons/attrd/attrd_cib.c
+++ b/daemons/attrd/attrd_cib.c
@@ -51,6 +51,7 @@ attrd_cib_updated_cb(const char *event, xmlNode *msg)
{
const xmlNode *patchset = NULL;
const char *client_name = NULL;
+ bool status_changed = false;
if (attrd_shutting_down(true)) {
return;
@@ -64,20 +65,22 @@ attrd_cib_updated_cb(const char *event, xmlNode *msg)
mainloop_set_trigger(attrd_config_read);
}
- if (!attrd_election_won()) {
- // Don't write attributes if we're not the writer
- return;
- }
+ status_changed = cib__element_in_patchset(patchset, XML_CIB_TAG_STATUS);
client_name = crm_element_value(msg, F_CIB_CLIENTNAME);
if (!cib__client_triggers_refresh(client_name)) {
- // The CIB is still accurate
+ /* This change came from a source that ensured the CIB is consistent
+ * with our attributes table, so we don't need to write anything out.
+ */
return;
}
- if (cib__element_in_patchset(patchset, XML_CIB_TAG_NODES)
- || cib__element_in_patchset(patchset, XML_CIB_TAG_STATUS)) {
-
+ if (!attrd_election_won()) {
+ // Don't write attributes if we're not the writer
+ return;
+ }
+
+ if (status_changed || cib__element_in_patchset(patchset, XML_CIB_TAG_NODES)) {
/* An unsafe client modified the nodes or status section. Write
* transient attributes to ensure they're up-to-date in the CIB.
*/
--
2.31.1
From b83c2567fb450eec5b18882ded16403831d2c3c0 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 11 Jan 2024 17:53:55 -0600
Subject: [PATCH 5/5] Log: pacemaker-attrd: make sure we don't try to log NULL
---
daemons/attrd/attrd_corosync.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/daemons/attrd/attrd_corosync.c b/daemons/attrd/attrd_corosync.c
index 59e6a26..b348d52 100644
--- a/daemons/attrd/attrd_corosync.c
+++ b/daemons/attrd/attrd_corosync.c
@@ -229,6 +229,11 @@ record_peer_nodeid(attribute_value_t *v, const char *host)
}
}
+#define readable_value(rv_v) pcmk__s((rv_v)->current, "(unset)")
+
+#define readable_peer(p) \
+ (((p) == NULL)? "all peers" : pcmk__s((p)->uname, "unknown peer"))
+
static void
update_attr_on_host(attribute_t *a, const crm_node_t *peer, const xmlNode *xml,
const char *attr, const char *value, const char *host,
@@ -262,14 +267,14 @@ update_attr_on_host(attribute_t *a, const crm_node_t *peer, const xmlNode *xml,
pcmk__str_casei)) {
crm_notice("%s[%s]: local value '%s' takes priority over '%s' from %s",
- attr, host, v->current, value, peer->uname);
+ attr, host, readable_value(v), value, peer->uname);
v = broadcast_local_value(a);
} else if (changed) {
crm_notice("Setting %s[%s]%s%s: %s -> %s "
CRM_XS " from %s with %s write delay",
attr, host, a->set_type ? " in " : "",
- pcmk__s(a->set_type, ""), pcmk__s(v->current, "(unset)"),
+ pcmk__s(a->set_type, ""), readable_value(v),
pcmk__s(value, "(unset)"), peer->uname,
(a->timeout_ms == 0)? "no" : pcmk__readable_interval(a->timeout_ms));
pcmk__str_update(&v->current, value);
@@ -543,12 +548,14 @@ attrd_peer_sync(crm_node_t *peer)
while (g_hash_table_iter_next(&aIter, NULL, (gpointer *) & a)) {
g_hash_table_iter_init(&vIter, a->values);
while (g_hash_table_iter_next(&vIter, NULL, (gpointer *) & v)) {
- crm_debug("Syncing %s[%s] = %s to %s", a->id, v->nodename, v->current, peer?peer->uname:"everyone");
+ crm_debug("Syncing %s[%s]='%s' to %s",
+ a->id, v->nodename, readable_value(v),
+ readable_peer(peer));
attrd_add_value_xml(sync, a, v, false);
}
}
- crm_debug("Syncing values to %s", peer?peer->uname:"everyone");
+ crm_debug("Syncing values to %s", readable_peer(peer));
attrd_send_message(peer, sync, false);
free_xml(sync);
}
--
2.31.1

View File

@ -0,0 +1,45 @@
From f5263c9401c9c38d4e039149deddcc0da0c184ba Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 3 Aug 2023 12:17:08 -0500
Subject: [PATCH] Fix: attrd: avoid race condition when shutting down
This addresses a race condition that can occur when the DC and the attribute
writer are different nodes, and shutting down at the same time. When the DC
controller leaves its Corosync process group, the remaining nodes erase its
transient node attributes (including "shutdown") from the CIB. However if the
(former) DC's attrd is still up, it can win the attribute writer election
called after the original writer leaves. As the election winner, it writes out
all its attributes to the CIB, including "shutdown". The next time it rejoins
the cluster, it will be immediately shut down.
Fixes T138
---
daemons/attrd/attrd_elections.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/daemons/attrd/attrd_elections.c b/daemons/attrd/attrd_elections.c
index 3b6b55a0f59..6f4916888a9 100644
--- a/daemons/attrd/attrd_elections.c
+++ b/daemons/attrd/attrd_elections.c
@@ -22,12 +22,20 @@ attrd_election_cb(gpointer user_data)
{
attrd_declare_winner();
+ if (attrd_requesting_shutdown() || attrd_shutting_down()) {
+ /* This node is shutting down or about to, meaning its attributes will
+ * be removed (and may have already been removed from the CIB by a
+ * controller). Don't sync or write its attributes in this case.
+ */
+ return G_SOURCE_REMOVE;
+ }
+
/* Update the peers after an election */
attrd_peer_sync(NULL, NULL);
/* Update the CIB after an election */
attrd_write_attributes(true, false);
- return FALSE;
+ return G_SOURCE_REMOVE;
}
void

View File

@ -1,385 +0,0 @@
From 84d4a0d5f562df91baa0fece45d06ad3732f941c Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Tue, 16 Jan 2024 11:20:53 -0600
Subject: [PATCH 1/5] Low: pacemaker-attrd: properly validate attribute set
type
The sense of the test was accidentally reversed in 26471a52689
---
daemons/attrd/attrd_attributes.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/daemons/attrd/attrd_attributes.c b/daemons/attrd/attrd_attributes.c
index 8f32988..f059059 100644
--- a/daemons/attrd/attrd_attributes.c
+++ b/daemons/attrd/attrd_attributes.c
@@ -40,9 +40,9 @@ attrd_create_attribute(xmlNode *xml)
* attributes are not written.
*/
crm_element_value_int(xml, PCMK__XA_ATTR_IS_PRIVATE, &is_private);
- if ((is_private != 0)
- && !pcmk__str_any_of(set_type, XML_TAG_ATTR_SETS, XML_TAG_UTILIZATION,
- NULL)) {
+ if (!is_private && !pcmk__str_any_of(set_type,
+ XML_TAG_ATTR_SETS,
+ XML_TAG_UTILIZATION, NULL)) {
crm_warn("Ignoring attribute %s with invalid set type %s",
pcmk__s(name, "(unidentified)"), set_type);
return NULL;
--
2.31.1
From d0d0511e71fe983a2d89589c39810b79fb48a8ca Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Tue, 16 Jan 2024 12:13:42 -0600
Subject: [PATCH 2/5] Fix: pacemaker-attrd: sync utilization attributes to
peers correctly
Include the set type with attribute syncs.
Previously, utilization attributes would have the correct set_type on the node
where they were set, but peers would store it as a regular node attribute. If
one of those peers became writer, the attribute would get written to the wrong
set.
---
daemons/attrd/attrd_attributes.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/daemons/attrd/attrd_attributes.c b/daemons/attrd/attrd_attributes.c
index f059059..0ad9630 100644
--- a/daemons/attrd/attrd_attributes.c
+++ b/daemons/attrd/attrd_attributes.c
@@ -139,6 +139,7 @@ attrd_add_value_xml(xmlNode *parent, const attribute_t *a,
xmlNode *xml = create_xml_node(parent, __func__);
crm_xml_add(xml, PCMK__XA_ATTR_NAME, a->id);
+ crm_xml_add(xml, PCMK__XA_ATTR_SET_TYPE, a->set_type);
crm_xml_add(xml, PCMK__XA_ATTR_SET, a->set_id);
crm_xml_add(xml, PCMK__XA_ATTR_UUID, a->uuid);
crm_xml_add(xml, PCMK__XA_ATTR_USER, a->user);
--
2.31.1
From 4479ff8507dd69f5946d31cf83c7e47fe15d3bdb Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Tue, 16 Jan 2024 12:18:40 -0600
Subject: [PATCH 3/5] Refactor: pacemaker-attrd: functionize getting attribute
set ID
... for future reuse
---
daemons/attrd/attrd_attributes.c | 38 ++++++++++++++++++++++++++++++++
daemons/attrd/attrd_cib.c | 9 +-------
daemons/attrd/pacemaker-attrd.h | 3 ++-
3 files changed, 41 insertions(+), 9 deletions(-)
diff --git a/daemons/attrd/attrd_attributes.c b/daemons/attrd/attrd_attributes.c
index 0ad9630..5727ab8 100644
--- a/daemons/attrd/attrd_attributes.c
+++ b/daemons/attrd/attrd_attributes.c
@@ -210,3 +210,41 @@ attrd_populate_attribute(xmlNode *xml, const char *attr)
return a;
}
+
+/*!
+ * \internal
+ * \brief Get the XML ID used to write out an attribute set
+ *
+ * \param[in] attr Attribute to get set ID for
+ * \param[in] node_state_id XML ID of node state that attribute value is for
+ *
+ * \return Newly allocated string with XML ID to use for \p attr set
+ */
+char *
+attrd_set_id(const attribute_t *attr, const char *node_state_id)
+{
+ char *set_id = NULL;
+
+ CRM_ASSERT((attr != NULL) && (node_state_id != NULL));
+
+ if (attr->set_id == NULL) {
+ /* @COMPAT This should really take the set type into account. Currently
+ * we use the same XML ID for transient attributes and utilization
+ * attributes. It doesn't cause problems because the status section is
+ * not limited by the schema in any way, but it's still unfortunate.
+ * For backward compatibility reasons, we can't change this.
+ */
+ set_id = crm_strdup_printf("%s-%s", XML_CIB_TAG_STATUS, node_state_id);
+ } else {
+ /* @COMPAT When the user specifies a set ID for an attribute, it is the
+ * same for every node. That is less than ideal, but again, the schema
+ * doesn't enforce anything for the status section. We couldn't change
+ * it without allowing the set ID to vary per value rather than per
+ * attribute, which would break backward compatibility, pose design
+ * challenges, and potentially cause problems in rolling upgrades.
+ */
+ pcmk__str_update(&set_id, attr->set_id);
+ }
+ crm_xml_sanitize_id(set_id);
+ return set_id;
+}
diff --git a/daemons/attrd/attrd_cib.c b/daemons/attrd/attrd_cib.c
index 481fea7..08d3425 100644
--- a/daemons/attrd/attrd_cib.c
+++ b/daemons/attrd/attrd_cib.c
@@ -423,17 +423,10 @@ add_unset_attr_update(const attribute_t *attr, const char *attr_id,
static int
add_attr_update(const attribute_t *attr, const char *value, const char *node_id)
{
- char *set_id = NULL;
+ char *set_id = attrd_set_id(attr, node_id);
char *attr_id = NULL;
int rc = pcmk_rc_ok;
- if (attr->set_id != NULL) {
- pcmk__str_update(&set_id, attr->set_id);
- } else {
- set_id = crm_strdup_printf("%s-%s", XML_CIB_TAG_STATUS, node_id);
- }
- crm_xml_sanitize_id(set_id);
-
if (attr->uuid != NULL) {
pcmk__str_update(&attr_id, attr->uuid);
} else {
diff --git a/daemons/attrd/pacemaker-attrd.h b/daemons/attrd/pacemaker-attrd.h
index bacaad6..3da7f8d 100644
--- a/daemons/attrd/pacemaker-attrd.h
+++ b/daemons/attrd/pacemaker-attrd.h
@@ -1,5 +1,5 @@
/*
- * Copyright 2013-2023 the Pacemaker project contributors
+ * Copyright 2013-2024 the Pacemaker project contributors
*
* The version control history for this file may have further details.
*
@@ -195,6 +195,7 @@ void attrd_clear_value_seen(void);
void attrd_free_attribute(gpointer data);
void attrd_free_attribute_value(gpointer data);
attribute_t *attrd_populate_attribute(xmlNode *xml, const char *attr);
+char *attrd_set_id(const attribute_t *attr, const char *node_state_id);
enum attrd_write_options {
attrd_write_changed = 0,
--
2.31.1
From eee2169ac348b8ed26ac0b78cb11ddc5cef9384e Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Tue, 16 Jan 2024 12:25:59 -0600
Subject: [PATCH 4/5] Refactor: pacemaker-attrd: functionize getting attribute
nvpair ID
... for future reuse
---
daemons/attrd/attrd_attributes.c | 28 ++++++++++++++++++++++++++++
daemons/attrd/attrd_cib.c | 17 +++++------------
daemons/attrd/pacemaker-attrd.h | 1 +
3 files changed, 34 insertions(+), 12 deletions(-)
diff --git a/daemons/attrd/attrd_attributes.c b/daemons/attrd/attrd_attributes.c
index 5727ab8..23de2e2 100644
--- a/daemons/attrd/attrd_attributes.c
+++ b/daemons/attrd/attrd_attributes.c
@@ -248,3 +248,31 @@ attrd_set_id(const attribute_t *attr, const char *node_state_id)
crm_xml_sanitize_id(set_id);
return set_id;
}
+
+/*!
+ * \internal
+ * \brief Get the XML ID used to write out an attribute value
+ *
+ * \param[in] attr Attribute to get value XML ID for
+ * \param[in] node_state_id UUID of node that attribute value is for
+ *
+ * \return Newly allocated string with XML ID of \p attr value
+ */
+char *
+attrd_nvpair_id(const attribute_t *attr, const char *node_state_id)
+{
+ char *nvpair_id = NULL;
+
+ if (attr->uuid != NULL) {
+ pcmk__str_update(&nvpair_id, attr->uuid);
+
+ } else if (attr->set_id != NULL) {
+ nvpair_id = crm_strdup_printf("%s-%s", attr->set_id, attr->id);
+
+ } else {
+ nvpair_id = crm_strdup_printf(XML_CIB_TAG_STATUS "-%s-%s",
+ node_state_id, attr->id);
+ }
+ crm_xml_sanitize_id(nvpair_id);
+ return nvpair_id;
+}
diff --git a/daemons/attrd/attrd_cib.c b/daemons/attrd/attrd_cib.c
index 08d3425..d42345f 100644
--- a/daemons/attrd/attrd_cib.c
+++ b/daemons/attrd/attrd_cib.c
@@ -424,23 +424,16 @@ static int
add_attr_update(const attribute_t *attr, const char *value, const char *node_id)
{
char *set_id = attrd_set_id(attr, node_id);
- char *attr_id = NULL;
+ char *nvpair_id = attrd_nvpair_id(attr, node_id);
int rc = pcmk_rc_ok;
- if (attr->uuid != NULL) {
- pcmk__str_update(&attr_id, attr->uuid);
+ if (value == NULL) {
+ rc = add_unset_attr_update(attr, nvpair_id, node_id, set_id);
} else {
- attr_id = crm_strdup_printf("%s-%s", set_id, attr->id);
- }
- crm_xml_sanitize_id(attr_id);
-
- if (value != NULL) {
- rc = add_set_attr_update(attr, attr_id, node_id, set_id, value);
- } else {
- rc = add_unset_attr_update(attr, attr_id, node_id, set_id);
+ rc = add_set_attr_update(attr, nvpair_id, node_id, set_id, value);
}
free(set_id);
- free(attr_id);
+ free(nvpair_id);
return rc;
}
diff --git a/daemons/attrd/pacemaker-attrd.h b/daemons/attrd/pacemaker-attrd.h
index 3da7f8d..deec790 100644
--- a/daemons/attrd/pacemaker-attrd.h
+++ b/daemons/attrd/pacemaker-attrd.h
@@ -196,6 +196,7 @@ void attrd_free_attribute(gpointer data);
void attrd_free_attribute_value(gpointer data);
attribute_t *attrd_populate_attribute(xmlNode *xml, const char *attr);
char *attrd_set_id(const attribute_t *attr, const char *node_state_id);
+char *attrd_nvpair_id(const attribute_t *attr, const char *node_state_id);
enum attrd_write_options {
attrd_write_changed = 0,
--
2.31.1
From 2abde6cb87d2e3d31a370c74656f6f7c0818c185 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 18 Jan 2024 10:01:56 -0600
Subject: [PATCH 5/5] Log: pacemaker-attrd: improve some messages for debugging
---
daemons/attrd/attrd_attributes.c | 8 +++++---
daemons/attrd/attrd_cib.c | 13 +++++++++----
daemons/attrd/attrd_corosync.c | 10 ++++++----
3 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/daemons/attrd/attrd_attributes.c b/daemons/attrd/attrd_attributes.c
index 23de2e2..68b9585 100644
--- a/daemons/attrd/attrd_attributes.c
+++ b/daemons/attrd/attrd_attributes.c
@@ -60,13 +60,10 @@ attrd_create_attribute(xmlNode *xml)
a->values = pcmk__strikey_table(NULL, attrd_free_attribute_value);
a->user = crm_element_value_copy(xml, PCMK__XA_ATTR_USER);
- crm_trace("Performing all %s operations as user '%s'", a->id, a->user);
if (dampen_s != NULL) {
dampen = crm_get_msec(dampen_s);
}
- crm_trace("Created attribute %s with %s write delay", a->id,
- (a->timeout_ms == 0)? "no" : pcmk__readable_interval(a->timeout_ms));
if(dampen > 0) {
a->timeout_ms = dampen;
@@ -75,6 +72,11 @@ attrd_create_attribute(xmlNode *xml)
crm_warn("Ignoring invalid delay %s for attribute %s", dampen_s, a->id);
}
+ crm_trace("Created attribute %s with %s write delay and %s CIB user",
+ a->id,
+ ((dampen > 0)? pcmk__readable_interval(a->timeout_ms) : "no"),
+ pcmk__s(a->user, "default"));
+
g_hash_table_replace(attributes, a->id, a);
return a;
}
diff --git a/daemons/attrd/attrd_cib.c b/daemons/attrd/attrd_cib.c
index d42345f..cae6846 100644
--- a/daemons/attrd/attrd_cib.c
+++ b/daemons/attrd/attrd_cib.c
@@ -54,6 +54,7 @@ attrd_cib_updated_cb(const char *event, xmlNode *msg)
bool status_changed = false;
if (attrd_shutting_down(true)) {
+ crm_debug("Ignoring CIB change during shutdown");
return;
}
@@ -278,11 +279,13 @@ attrd_cib_callback(xmlNode *msg, int call_id, int rc, xmlNode *output, void *use
g_hash_table_iter_init(&iter, a->values);
while (g_hash_table_iter_next(&iter, (gpointer *) & peer, (gpointer *) & v)) {
- do_crm_log(level, "* %s[%s]=%s",
- a->id, peer, pcmk__s(v->requested, "(null)"));
if (rc == pcmk_ok) {
+ crm_info("* Wrote %s[%s]=%s",
+ a->id, peer, pcmk__s(v->requested, "(unset)"));
pcmk__str_update(&(v->requested), NULL);
} else {
+ do_crm_log(level, "* Could not write %s[%s]=%s",
+ a->id, peer, pcmk__s(v->requested, "(unset)"));
a->changed = true; // Reattempt write below if we are still writer
}
}
@@ -292,6 +295,7 @@ attrd_cib_callback(xmlNode *msg, int call_id, int rc, xmlNode *output, void *use
/* We deferred a write of a new update because this update was in
* progress. Write out the new value without additional delay.
*/
+ crm_debug("Pending update for %s can be written now", a->id);
write_attribute(a, false);
/* We're re-attempting a write because the original failed; delay
@@ -593,8 +597,9 @@ write_attribute(attribute_t *a, bool ignore_delay)
continue;
}
- crm_debug("Updating %s[%s]=%s (node uuid=%s id=%" PRIu32 ")",
- a->id, v->nodename, v->current, uuid, v->nodeid);
+ crm_debug("Writing %s[%s]=%s (node-state-id=%s node-id=%" PRIu32 ")",
+ a->id, v->nodename, pcmk__s(v->current, "(unset)"),
+ uuid, v->nodeid);
cib_updates++;
/* Preservation of the attribute to transmit alert */
diff --git a/daemons/attrd/attrd_corosync.c b/daemons/attrd/attrd_corosync.c
index b348d52..6fb847b 100644
--- a/daemons/attrd/attrd_corosync.c
+++ b/daemons/attrd/attrd_corosync.c
@@ -293,7 +293,8 @@ update_attr_on_host(attribute_t *a, const crm_node_t *peer, const xmlNode *xml,
// Write out new value or start dampening timer
if (a->timeout_ms && a->timer) {
- crm_trace("Delayed write out (%dms) for %s", a->timeout_ms, attr);
+ crm_trace("Delaying write of %s %s for dampening",
+ attr, pcmk__readable_interval(a->timeout_ms));
mainloop_timer_start(a->timer);
} else {
attrd_write_or_elect_attribute(a);
@@ -307,11 +308,12 @@ update_attr_on_host(attribute_t *a, const crm_node_t *peer, const xmlNode *xml,
if (is_force_write == 1 && a->timeout_ms && a->timer) {
/* Save forced writing and set change flag. */
/* The actual attribute is written by Writer after election. */
- crm_trace("Unchanged %s[%s] from %s is %s(Set the forced write flag)",
- attr, host, peer->uname, value);
+ crm_trace("%s[%s] from %s is unchanged (%s), forcing write",
+ attr, host, peer->uname, pcmk__s(value, "unset"));
a->force_write = TRUE;
} else {
- crm_trace("Unchanged %s[%s] from %s is %s", attr, host, peer->uname, value);
+ crm_trace("%s[%s] from %s is unchanged (%s)",
+ attr, host, peer->uname, pcmk__s(value, "unset"));
}
}
--
2.31.1

View File

@ -0,0 +1,210 @@
From 83e547cc64f2586031a007ab58e91fc22cd1a68a Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 24 Aug 2023 12:18:23 -0500
Subject: [PATCH] Refactor: attrd: use enum instead of bools for
attrd_write_attributes()
---
daemons/attrd/attrd_cib.c | 24 ++++++++++++++++++------
daemons/attrd/attrd_corosync.c | 2 +-
daemons/attrd/attrd_elections.c | 2 +-
daemons/attrd/attrd_ipc.c | 2 +-
daemons/attrd/attrd_utils.c | 2 +-
daemons/attrd/pacemaker-attrd.h | 8 +++++++-
6 files changed, 29 insertions(+), 11 deletions(-)
diff --git a/daemons/attrd/attrd_cib.c b/daemons/attrd/attrd_cib.c
index 928c0133745..9c787fe1024 100644
--- a/daemons/attrd/attrd_cib.c
+++ b/daemons/attrd/attrd_cib.c
@@ -343,16 +343,23 @@ attrd_write_attribute(attribute_t *a, bool ignore_delay)
free_xml(xml_top);
}
+/*!
+ * \internal
+ * \brief Write out attributes
+ *
+ * \param[in] options Group of enum attrd_write_options
+ */
void
-attrd_write_attributes(bool all, bool ignore_delay)
+attrd_write_attributes(uint32_t options)
{
GHashTableIter iter;
attribute_t *a = NULL;
- crm_debug("Writing out %s attributes", all? "all" : "changed");
+ crm_debug("Writing out %s attributes",
+ pcmk_is_set(options, attrd_write_all)? "all" : "changed");
g_hash_table_iter_init(&iter, attributes);
while (g_hash_table_iter_next(&iter, NULL, (gpointer *) & a)) {
- if (!all && a->unknown_peer_uuids) {
+ if (!pcmk_is_set(options, attrd_write_all) && a->unknown_peer_uuids) {
// Try writing this attribute again, in case peer ID was learned
a->changed = true;
} else if (a->force_write) {
@@ -360,9 +367,14 @@ attrd_write_attributes(bool all, bool ignore_delay)
a->changed = true;
}
- if(all || a->changed) {
- /* When forced write flag is set, ignore delay. */
- attrd_write_attribute(a, (a->force_write ? true : ignore_delay));
+ if (pcmk_is_set(options, attrd_write_all) || a->changed) {
+ bool ignore_delay = pcmk_is_set(options, attrd_write_no_delay);
+
+ if (a->force_write) {
+ // Always ignore delay when forced write flag is set
+ ignore_delay = true;
+ }
+ attrd_write_attribute(a, ignore_delay);
} else {
crm_trace("Skipping unchanged attribute %s", a->id);
}
diff --git a/daemons/attrd/attrd_corosync.c b/daemons/attrd/attrd_corosync.c
index 1aec35a054e..49631df6e44 100644
--- a/daemons/attrd/attrd_corosync.c
+++ b/daemons/attrd/attrd_corosync.c
@@ -285,7 +285,7 @@ record_peer_nodeid(attribute_value_t *v, const char *host)
crm_trace("Learned %s has node id %s", known_peer->uname, known_peer->uuid);
if (attrd_election_won()) {
- attrd_write_attributes(false, false);
+ attrd_write_attributes(attrd_write_changed);
}
}
diff --git a/daemons/attrd/attrd_elections.c b/daemons/attrd/attrd_elections.c
index c25a41a4492..01341db18e4 100644
--- a/daemons/attrd/attrd_elections.c
+++ b/daemons/attrd/attrd_elections.c
@@ -34,7 +34,7 @@ attrd_election_cb(gpointer user_data)
attrd_peer_sync(NULL, NULL);
/* Update the CIB after an election */
- attrd_write_attributes(true, false);
+ attrd_write_attributes(attrd_write_all);
return G_SOURCE_REMOVE;
}
diff --git a/daemons/attrd/attrd_ipc.c b/daemons/attrd/attrd_ipc.c
index 4be789de7f9..05c4a696a19 100644
--- a/daemons/attrd/attrd_ipc.c
+++ b/daemons/attrd/attrd_ipc.c
@@ -232,7 +232,7 @@ attrd_client_refresh(pcmk__request_t *request)
crm_info("Updating all attributes");
attrd_send_ack(request->ipc_client, request->ipc_id, request->ipc_flags);
- attrd_write_attributes(true, true);
+ attrd_write_attributes(attrd_write_all|attrd_write_no_delay);
pcmk__set_result(&request->result, CRM_EX_OK, PCMK_EXEC_DONE, NULL);
return NULL;
diff --git a/daemons/attrd/attrd_utils.c b/daemons/attrd/attrd_utils.c
index c43eac1695a..bfd51368890 100644
--- a/daemons/attrd/attrd_utils.c
+++ b/daemons/attrd/attrd_utils.c
@@ -156,7 +156,7 @@ attrd_cib_replaced_cb(const char *event, xmlNode * msg)
if (attrd_election_won()) {
if (change_section & (cib_change_section_nodes | cib_change_section_status)) {
crm_notice("Updating all attributes after %s event", event);
- attrd_write_attributes(true, false);
+ attrd_write_attributes(attrd_write_all);
}
}
diff --git a/daemons/attrd/pacemaker-attrd.h b/daemons/attrd/pacemaker-attrd.h
index 41f31d97b3b..2d781d11394 100644
--- a/daemons/attrd/pacemaker-attrd.h
+++ b/daemons/attrd/pacemaker-attrd.h
@@ -176,8 +176,14 @@ void attrd_free_attribute(gpointer data);
void attrd_free_attribute_value(gpointer data);
attribute_t *attrd_populate_attribute(xmlNode *xml, const char *attr);
+enum attrd_write_options {
+ attrd_write_changed = 0,
+ attrd_write_all = (1 << 0),
+ attrd_write_no_delay = (1 << 1),
+};
+
void attrd_write_attribute(attribute_t *a, bool ignore_delay);
-void attrd_write_attributes(bool all, bool ignore_delay);
+void attrd_write_attributes(uint32_t options);
void attrd_write_or_elect_attribute(attribute_t *a);
extern int minimum_protocol_version;
From 58400e272cfc51f02eec69cdd0ed0d27a30e78a3 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 24 Aug 2023 12:27:53 -0500
Subject: [PATCH] Fix: attrd: avoid race condition at writer election
f5263c94 was not a complete fix. The issue may also occur if a remaining node
(not the original DC or writer) wins the attribute writer election after the
original DC's controller has exited but before its attribute manger has exited.
The long-term solution will be to have the attribute manager (instead of the
controller) be in control of erasing transient attributes from the CIB when a
node leaves. This short-term workaround simply has new attribute writers skip
shutdown attributes when writing out all attributes.
Fixes T138
---
daemons/attrd/attrd_cib.c | 5 +++++
daemons/attrd/attrd_elections.c | 14 ++++++++++++--
daemons/attrd/pacemaker-attrd.h | 1 +
3 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/daemons/attrd/attrd_cib.c b/daemons/attrd/attrd_cib.c
index 9c787fe102..2c910b4c64 100644
--- a/daemons/attrd/attrd_cib.c
+++ b/daemons/attrd/attrd_cib.c
@@ -359,6 +359,11 @@ attrd_write_attributes(uint32_t options)
pcmk_is_set(options, attrd_write_all)? "all" : "changed");
g_hash_table_iter_init(&iter, attributes);
while (g_hash_table_iter_next(&iter, NULL, (gpointer *) & a)) {
+ if (pcmk_is_set(options, attrd_write_skip_shutdown)
+ && pcmk__str_eq(a->id, XML_CIB_ATTR_SHUTDOWN, pcmk__str_none)) {
+ continue;
+ }
+
if (!pcmk_is_set(options, attrd_write_all) && a->unknown_peer_uuids) {
// Try writing this attribute again, in case peer ID was learned
a->changed = true;
diff --git a/daemons/attrd/attrd_elections.c b/daemons/attrd/attrd_elections.c
index 01341db18e..a95cd44cbd 100644
--- a/daemons/attrd/attrd_elections.c
+++ b/daemons/attrd/attrd_elections.c
@@ -33,8 +33,18 @@ attrd_election_cb(gpointer user_data)
/* Update the peers after an election */
attrd_peer_sync(NULL, NULL);
- /* Update the CIB after an election */
- attrd_write_attributes(attrd_write_all);
+ /* After winning an election, update the CIB with the values of all
+ * attributes as the winner knows them.
+ *
+ * However, do not write out any "shutdown" attributes. A node that is
+ * shutting down will have all its transient attributes removed from the CIB
+ * when its controller exits, and from the attribute manager's memory (on
+ * remaining nodes) when its attribute manager exits; if an election is won
+ * between when those two things happen, we don't want to write the shutdown
+ * attribute back out, which would cause the node to immediately shut down
+ * the next time it rejoins.
+ */
+ attrd_write_attributes(attrd_write_all|attrd_write_skip_shutdown);
return G_SOURCE_REMOVE;
}
diff --git a/daemons/attrd/pacemaker-attrd.h b/daemons/attrd/pacemaker-attrd.h
index 2d781d1139..2e35bd7ec5 100644
--- a/daemons/attrd/pacemaker-attrd.h
+++ b/daemons/attrd/pacemaker-attrd.h
@@ -180,6 +180,7 @@ enum attrd_write_options {
attrd_write_changed = 0,
attrd_write_all = (1 << 0),
attrd_write_no_delay = (1 << 1),
+ attrd_write_skip_shutdown = (1 << 2),
};
void attrd_write_attribute(attribute_t *a, bool ignore_delay);

View File

@ -1,53 +0,0 @@
From 9c13ce6fe95812308443c188ace8f897e6bce942 Mon Sep 17 00:00:00 2001
From: Reid Wahl <nrwahl@protonmail.com>
Date: Mon, 29 Jan 2024 11:14:25 -0800
Subject: [PATCH] Fix: tools: crm_attribute emits garbage for --node localhost
or auto
This happens because pcmk__node_attr_target() returns its argument if
its argument is NULL, "auto", or "localhost" and no relevant environment
variables are found. Then crm_attribute frees the return value, makes a
copy of it, and assigns it back to options.dest_uname.
The fix is to check whether the return value is equal to the argument.
Fixes RHEL-23065
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
---
tools/crm_attribute.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/tools/crm_attribute.c b/tools/crm_attribute.c
index d221ab85d..636d03dbd 100644
--- a/tools/crm_attribute.c
+++ b/tools/crm_attribute.c
@@ -766,8 +766,23 @@ main(int argc, char **argv)
const char *target = pcmk__node_attr_target(options.dest_uname);
if (target != NULL) {
- g_free(options.dest_uname);
- options.dest_uname = g_strdup(target);
+ /* If options.dest_uname is "auto" or "localhost", then
+ * pcmk__node_attr_target() may return it, depending on environment
+ * variables. In that case, attribute lookups will fail for "auto"
+ * (unless there's a node named "auto"). attrd maps "localhost" to
+ * the true local node name for queries.
+ *
+ * @TODO
+ * * Investigate whether "localhost" is mapped to a real node name
+ * for non-query commands. If not, possibly modify it so that it
+ * is.
+ * * Map "auto" to "localhost" (probably).
+ */
+ if (target != (const char *) options.dest_uname) {
+ g_free(options.dest_uname);
+ options.dest_uname = g_strdup(target);
+ }
+
} else if (getenv("CIB_file") != NULL && options.dest_uname == NULL) {
get_node_name_from_local();
}
--
2.41.0

View File

@ -0,0 +1,62 @@
From 2e81e0db9a716c486805e0760f78be65ca79eeae Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Tue, 17 Oct 2023 15:28:27 -0500
Subject: [PATCH] Fix: attrd: avoid regression by reverting 58400e27
Fixes T714
---
daemons/attrd/attrd_cib.c | 5 -----
daemons/attrd/attrd_elections.c | 10 +---------
daemons/attrd/pacemaker-attrd.h | 1 -
3 files changed, 1 insertion(+), 15 deletions(-)
diff --git a/daemons/attrd/attrd_cib.c b/daemons/attrd/attrd_cib.c
index 2de37a7cb6..9ce2872715 100644
--- a/daemons/attrd/attrd_cib.c
+++ b/daemons/attrd/attrd_cib.c
@@ -641,11 +641,6 @@ attrd_write_attributes(uint32_t options)
pcmk_is_set(options, attrd_write_all)? "all" : "changed");
g_hash_table_iter_init(&iter, attributes);
while (g_hash_table_iter_next(&iter, NULL, (gpointer *) & a)) {
- if (pcmk_is_set(options, attrd_write_skip_shutdown)
- && pcmk__str_eq(a->id, XML_CIB_ATTR_SHUTDOWN, pcmk__str_none)) {
- continue;
- }
-
if (!pcmk_is_set(options, attrd_write_all) && a->unknown_peer_uuids) {
// Try writing this attribute again, in case peer ID was learned
a->changed = true;
diff --git a/daemons/attrd/attrd_elections.c b/daemons/attrd/attrd_elections.c
index a95cd44cbd..62310ed1d8 100644
--- a/daemons/attrd/attrd_elections.c
+++ b/daemons/attrd/attrd_elections.c
@@ -35,16 +35,8 @@ attrd_election_cb(gpointer user_data)
/* After winning an election, update the CIB with the values of all
* attributes as the winner knows them.
- *
- * However, do not write out any "shutdown" attributes. A node that is
- * shutting down will have all its transient attributes removed from the CIB
- * when its controller exits, and from the attribute manager's memory (on
- * remaining nodes) when its attribute manager exits; if an election is won
- * between when those two things happen, we don't want to write the shutdown
- * attribute back out, which would cause the node to immediately shut down
- * the next time it rejoins.
*/
- attrd_write_attributes(attrd_write_all|attrd_write_skip_shutdown);
+ attrd_write_attributes(attrd_write_all);
return G_SOURCE_REMOVE;
}
diff --git a/daemons/attrd/pacemaker-attrd.h b/daemons/attrd/pacemaker-attrd.h
index e3c369b5bc..a95bb54367 100644
--- a/daemons/attrd/pacemaker-attrd.h
+++ b/daemons/attrd/pacemaker-attrd.h
@@ -181,7 +181,6 @@ enum attrd_write_options {
attrd_write_changed = 0,
attrd_write_all = (1 << 0),
attrd_write_no_delay = (1 << 1),
- attrd_write_skip_shutdown = (1 << 2),
};
void attrd_write_attribute(attribute_t *a, bool ignore_delay);

View File

@ -1,31 +0,0 @@
From 8dc0d9b43343919edf4f4011ceecfd6b6765b4a4 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Wed, 8 May 2024 11:18:50 -0500
Subject: [PATCH] Low: libcib: avoid memory leak in async calls
Never in a release
---
lib/cib/cib_native.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/lib/cib/cib_native.c b/lib/cib/cib_native.c
index 0e502155bc..b014223112 100644
--- a/lib/cib/cib_native.c
+++ b/lib/cib/cib_native.c
@@ -94,9 +94,10 @@ cib_native_perform_op_delegate(cib_t *cib, const char *op, const char *host,
if (!(call_options & cib_sync_call)) {
crm_trace("Async call, returning %d", cib->call_id);
- CRM_CHECK(cib->call_id != 0, return -ENOMSG);
- free_xml(op_reply);
- return cib->call_id;
+ CRM_CHECK(cib->call_id != 0,
+ rc = -ENOMSG; goto done);
+ rc = cib->call_id;
+ goto done;
}
rc = pcmk_ok;
--
2.41.0

View File

@ -0,0 +1,34 @@
From 14b87a38786ae5b4dc12fc1581e5d39a274fced2 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Mon, 30 Oct 2023 12:21:24 -0500
Subject: [PATCH] Fix: attrd: revert faulty T138 fix
f5263c9401 created a timing issue where a node could get a shutdown attribute,
the original writer leaves the cluster before writing it out, then the
shutting-down node wins the writer election. In that case, it would skip the
write-out and the scheduler would never shut it down.
Reopens T138
---
daemons/attrd/attrd_elections.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/daemons/attrd/attrd_elections.c b/daemons/attrd/attrd_elections.c
index 62310ed1d8..82fbe8affc 100644
--- a/daemons/attrd/attrd_elections.c
+++ b/daemons/attrd/attrd_elections.c
@@ -22,14 +22,6 @@
{
attrd_declare_winner();
- if (attrd_requesting_shutdown() || attrd_shutting_down()) {
- /* This node is shutting down or about to, meaning its attributes will
- * be removed (and may have already been removed from the CIB by a
- * controller). Don't sync or write its attributes in this case.
- */
- return G_SOURCE_REMOVE;
- }
-
/* Update the peers after an election */
attrd_peer_sync(NULL, NULL);

View File

@ -1,121 +0,0 @@
From d7c233090057d4f660fa458a2ff97896b15ea951 Mon Sep 17 00:00:00 2001
From: Reid Wahl <nrwahl@protonmail.com>
Date: Thu, 11 Jul 2024 12:43:49 -0700
Subject: [PATCH] Refactor: various: Don't set cluster-layer node ID as XML ID
Currently, we call the pcmk__xe_set_id() function using a stringified
version of the numeric cluster-layer node ID. However, pcmk__xe_set_id()
tries to sanitize its input to a valid XML ID. An XML ID cannot begin
with a digit.
crm_xml_set_id() does not sanitize comprehensively, and in particular,
it does not care whether its argument begins with a digit. So the
current code doesn't cause a problem.
Still, as a best practice, set the PCMK_XA_ID attribute using
crm_xml_add_ll() instead.
Ref T848
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
---
daemons/controld/controld_messages.c | 6 +++++-
lib/cluster/corosync.c | 2 +-
lib/common/ipc_client.c | 2 +-
lib/common/ipc_controld.c | 9 ++++++---
tools/crm_node.c | 4 ++--
5 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/daemons/controld/controld_messages.c b/daemons/controld/controld_messages.c
index bd5237e..0b0f25b 100644
--- a/daemons/controld/controld_messages.c
+++ b/daemons/controld/controld_messages.c
@@ -893,7 +893,11 @@ handle_node_info_request(const xmlNode *msg)
pcmk_is_set(controld_globals.flags,
controld_has_quorum));
- // Check whether client requested node info by ID and/or name
+ /* Check whether client requested node info by ID and/or name
+ *
+ * @TODO A Corosync-layer node ID is of type uint32_t. We should be able to
+ * handle legitimate node IDs greater than INT_MAX, but currently we do not.
+ */
crm_element_value_int(msg, XML_ATTR_ID, &node_id);
if (node_id < 0) {
node_id = 0;
diff --git a/lib/cluster/corosync.c b/lib/cluster/corosync.c
index 374250f..fc33cce 100644
--- a/lib/cluster/corosync.c
+++ b/lib/cluster/corosync.c
@@ -650,7 +650,7 @@ pcmk__corosync_add_nodes(xmlNode *xml_parent)
if (xml_parent) {
xmlNode *node = create_xml_node(xml_parent, XML_CIB_TAG_NODE);
- crm_xml_set_id(node, "%u", nodeid);
+ crm_xml_add_ll(node, XML_ATTR_ID, (long long) nodeid);
crm_xml_add(node, XML_ATTR_UNAME, name);
}
}
diff --git a/lib/common/ipc_client.c b/lib/common/ipc_client.c
index 5e64e23..7696a69 100644
--- a/lib/common/ipc_client.c
+++ b/lib/common/ipc_client.c
@@ -769,7 +769,7 @@ create_purge_node_request(const pcmk_ipc_api_t *api, const char *node_name,
request = create_request(CRM_OP_RM_NODE_CACHE, NULL, NULL,
pcmk_ipc_name(api, false), client, NULL);
if (nodeid > 0) {
- crm_xml_set_id(request, "%lu", (unsigned long) nodeid);
+ crm_xml_add_ll(request, XML_ATTR_ID, (unsigned long) nodeid);
}
crm_xml_add(request, XML_ATTR_UNAME, node_name);
break;
diff --git a/lib/common/ipc_controld.c b/lib/common/ipc_controld.c
index 8e2016e..e4284f5 100644
--- a/lib/common/ipc_controld.c
+++ b/lib/common/ipc_controld.c
@@ -9,9 +9,12 @@
#include <crm_internal.h>
-#include <stdio.h>
-#include <stdbool.h>
#include <errno.h>
+#include <inttypes.h> // PRIu32
+#include <stdbool.h>
+#include <stdint.h> // uint32_t
+#include <stdio.h>
+
#include <libxml/tree.h>
#include <crm/crm.h>
@@ -412,7 +415,7 @@ pcmk_controld_api_node_info(pcmk_ipc_api_t *api, uint32_t nodeid)
return EINVAL;
}
if (nodeid > 0) {
- crm_xml_set_id(request, "%lu", (unsigned long) nodeid);
+ crm_xml_add_ll(request, XML_ATTR_ID, (unsigned long) nodeid);
}
rc = send_controller_request(api, request, true);
diff --git a/tools/crm_node.c b/tools/crm_node.c
index 1e7ce6c..ad8c459 100644
--- a/tools/crm_node.c
+++ b/tools/crm_node.c
@@ -552,7 +552,7 @@ remove_from_section(cib_t *cib, const char *element, const char *section,
}
crm_xml_add(xml, XML_ATTR_UNAME, node_name);
if (node_id > 0) {
- crm_xml_set_id(xml, "%ld", node_id);
+ crm_xml_add_ll(xml, XML_ATTR_ID, node_id);
}
rc = cib->cmds->remove(cib, section, xml, cib_transaction);
free_xml(xml);
@@ -691,7 +691,7 @@ purge_node_from_fencer(const char *node_name, long node_id)
cmd = create_request(CRM_OP_RM_NODE_CACHE, NULL, NULL, "stonith-ng",
crm_system_name, NULL);
if (node_id > 0) {
- crm_xml_set_id(cmd, "%ld", node_id);
+ crm_xml_add_ll(cmd, XML_ATTR_ID, node_id);
}
crm_xml_add(cmd, XML_ATTR_UNAME, node_name);

View File

@ -1,89 +0,0 @@
From 22e093a5bff608c86d0ea68588078ca747a6d945 Mon Sep 17 00:00:00 2001
From: Reid Wahl <nrwahl@protonmail.com>
Date: Thu, 11 Jul 2024 12:29:34 -0700
Subject: [PATCH] Fix: tools: crm_node -i must initialize nodeid before passing
pointer
This is a regression introduced in 2.1.7 via a27f099.
Currently, crm_node -i passes a pointer to the uninitialized uint32_t
nodeid variable, to pcmk__query_node_info(). Since the pointer is
non-NULL, pcmk__query_node_info() dereferences it. Whatever garbage
value resides there gets passed as the ID to query.
The controller parses the node ID from the request as an int. If the
garbage value is greater than INT_MAX, it overflows to a negative int
value, and the controller (in handle_node_info_request()) defaults it to
0. In that case, there's no problem: we search for the local node name
instead of the garbage node ID.
If the garbage value is less than or equal to INT_MAX, we search for it
directly. We won't find a matching node unless one happens to exist with
that garbage node ID. In the case of no match, crm_node -i outputs "Node
is not known to cluster" instead of the local node's cluster-layer ID.
Thanks to Artur Novik for the report:
https://lists.clusterlabs.org/pipermail/users/2024-July/036270.html
Fixes T847
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
---
lib/pacemaker/pcmk_cluster_queries.c | 22 +++++++++++-----------
tools/crm_node.c | 2 +-
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/lib/pacemaker/pcmk_cluster_queries.c b/lib/pacemaker/pcmk_cluster_queries.c
index 3229fae3eff..8404584580e 100644
--- a/lib/pacemaker/pcmk_cluster_queries.c
+++ b/lib/pacemaker/pcmk_cluster_queries.c
@@ -586,25 +586,25 @@ pcmk_designated_controller(xmlNodePtr *xml, unsigned int message_timeout_ms)
* the controller
*
* \param[in,out] out Output object
- * \param[in,out] node_id ID of node whose name to get. If \p NULL
- * or 0, get the local node name. If not
- * \p NULL, store the true node ID here on
+ * \param[in,out] node_id ID of node whose info to get. If \p NULL
+ * or 0, get the local node's info. If not
+ * \c NULL, store the true node ID here on
* success.
- * \param[out] node_name If not \p NULL, where to store the node
+ * \param[out] node_name If not \c NULL, where to store the node
* name
- * \param[out] uuid If not \p NULL, where to store the node
+ * \param[out] uuid If not \c NULL, where to store the node
* UUID
- * \param[out] state If not \p NULL, where to store the
+ * \param[out] state If not \c NULL, where to store the
* membership state
- * \param[out] is_remote If not \p NULL, where to store whether the
+ * \param[out] is_remote If not \c NULL, where to store whether the
* node is a Pacemaker Remote node
- * \param[out] have_quorum If not \p NULL, where to store whether the
+ * \param[out] have_quorum If not \c NULL, where to store whether the
* node has quorum
* \param[in] show_output Whether to show the node info
* \param[in] message_timeout_ms How long to wait for a reply from the
- * \p pacemaker-controld API. If 0,
- * \p pcmk_ipc_dispatch_sync will be used.
- * Otherwise, \p pcmk_ipc_dispatch_poll will
+ * \c pacemaker-controld API. If 0,
+ * \c pcmk_ipc_dispatch_sync will be used.
+ * Otherwise, \c pcmk_ipc_dispatch_poll will
* be used.
*
* \return Standard Pacemaker return code
diff --git a/tools/crm_node.c b/tools/crm_node.c
index d4153605a69..8aa8d3d29c7 100644
--- a/tools/crm_node.c
+++ b/tools/crm_node.c
@@ -434,7 +434,7 @@ run_controller_mainloop(void)
static void
print_node_id(void)
{
- uint32_t nodeid;
+ uint32_t nodeid = 0;
int rc = pcmk__query_node_info(out, &nodeid, NULL, NULL, NULL, NULL, NULL,
false, 0);

File diff suppressed because it is too large Load Diff