diff --git a/009-attrd-cache-3.patch b/009-attrd-cache-3.patch new file mode 100644 index 0000000..6150218 --- /dev/null +++ b/009-attrd-cache-3.patch @@ -0,0 +1,385 @@ +From 84d4a0d5f562df91baa0fece45d06ad3732f941c Mon Sep 17 00:00:00 2001 +From: Ken Gaillot +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 +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 +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 +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 +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 + diff --git a/010-crm_attribute-free.patch b/010-crm_attribute-free.patch new file mode 100644 index 0000000..d1e4831 --- /dev/null +++ b/010-crm_attribute-free.patch @@ -0,0 +1,53 @@ +From 9c13ce6fe95812308443c188ace8f897e6bce942 Mon Sep 17 00:00:00 2001 +From: Reid Wahl +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 +--- + 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 + diff --git a/pacemaker.spec b/pacemaker.spec index 6fcffd5..3960db8 100644 --- a/pacemaker.spec +++ b/pacemaker.spec @@ -36,7 +36,7 @@ ## can be incremented to build packages reliably considered "newer" ## than previously built packages with the same pcmkversion) %global pcmkversion 2.1.7 -%global specversion 3 +%global specversion 4 ## Upstream commit (full commit ID, abbreviated commit ID, or tag) to build %global commit 0f7f88312f7a1ccedee60bf768aba79ee13d41e0 @@ -255,6 +255,8 @@ Patch005: 005-attrd-cache-2.patch Patch006: 006-cib-file-feature-set.patch Patch007: 007-option-metadata.patch Patch008: 008-attrd-prep.patch +Patch009: 009-attrd-cache-3.patch +Patch010: 010-crm_attribute-free.patch Requires: resource-agents Requires: %{pkgname_pcmk_libs}%{?_isa} = %{version}-%{release} @@ -913,6 +915,13 @@ exit 0 %license %{nagios_name}-%{nagios_hash}/COPYING %changelog +* Wed Jan 31 2024 Chris Lumens - 2.1.7-4 +- Properly validate attribute set type in pacemaker-attrd +- Fix `crm_attribute -t nodes --node localhost` +- Resolves: RHEL-13216 +- Resolves: RHEL-17225 +- Resolves: RHEL-23498 + * Tue Jan 16 2024 Chris Lumens - 2.1.7-3 - Rebase on upstream 2.1.7 final release - Fix documentation for Pacemaker Remote schema transfers