From 08ce507927ff497b0e8f125050f67d59c74d674c Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 19 Nov 2020 15:52:14 -0600
Subject: [PATCH 01/12] Refactor: libpe_rules: functionize value-source
expansion
... for readability
---
lib/pengine/rules.c | 61 +++++++++++++++++++++++++++++++----------------------
1 file changed, 36 insertions(+), 25 deletions(-)
diff --git a/lib/pengine/rules.c b/lib/pengine/rules.c
index e5d452f..aa5d6ab 100644
--- a/lib/pengine/rules.c
+++ b/lib/pengine/rules.c
@@ -1027,6 +1027,36 @@ accept_attr_expr(const char *l_val, const char *r_val, const char *type,
/*!
* \internal
+ * \brief Get correct value according to value-source
+ *
+ * \param[in] value value given in rule expression
+ * \param[in] value_source value-source given in rule expressions
+ * \param[in] match_data If not NULL, resource back-references and params
+ */
+static const char *
+expand_value_source(const char *value, const char *value_source,
+ pe_match_data_t *match_data)
+{
+ GHashTable *table = NULL;
+
+ if (pcmk__str_eq(value_source, "param", pcmk__str_casei)) {
+ table = match_data->params;
+
+ } else if (pcmk__str_eq(value_source, "meta", pcmk__str_casei)) {
+ table = match_data->meta;
+
+ } else { // literal
+ return value;
+ }
+
+ if ((table == NULL) || pcmk__str_empty(value)) {
+ return NULL;
+ }
+ return (const char *) g_hash_table_lookup(table, value);
+}
+
+/*!
+ * \internal
* \brief Evaluate a node attribute expression based on #uname, #id, #kind,
* or a generic node attribute
*
@@ -1040,8 +1070,6 @@ pe__eval_attr_expr(xmlNodePtr expr, pe_rule_eval_data_t *rule_data)
{
gboolean attr_allocated = FALSE;
const char *h_val = NULL;
- GHashTable *table = NULL;
- bool literal = true;
const char *op = NULL;
const char *type = NULL;
@@ -1061,36 +1089,19 @@ pe__eval_attr_expr(xmlNodePtr expr, pe_rule_eval_data_t *rule_data)
return FALSE;
}
- if (rule_data->match_data) {
- if (rule_data->match_data->re) {
+ if (rule_data->match_data != NULL) {
+ // Expand any regular expression submatches (%0-%9) in attribute name
+ if (rule_data->match_data->re != NULL) {
char *resolved_attr = pe_expand_re_matches(attr, rule_data->match_data->re);
- if (resolved_attr) {
+ if (resolved_attr != NULL) {
attr = (const char *) resolved_attr;
attr_allocated = TRUE;
}
}
- if (pcmk__str_eq(value_source, "param", pcmk__str_casei)) {
- literal = false;
- table = rule_data->match_data->params;
- } else if (pcmk__str_eq(value_source, "meta", pcmk__str_casei)) {
- literal = false;
- table = rule_data->match_data->meta;
- }
- }
-
- if (!literal) {
- const char *param_name = value;
- const char *param_value = NULL;
-
- value = NULL;
- if ((table != NULL) && !pcmk__str_empty(param_name)) {
- param_value = (const char *)g_hash_table_lookup(table, param_name);
- if (param_value != NULL) {
- value = param_value;
- }
- }
+ // Get value appropriate to value-source
+ value = expand_value_source(value, value_source, rule_data->match_data);
}
if (rule_data->node_hash != NULL) {
--
1.8.3.1
From 8864e8596cc44fa275f68d58fea9b7d3d5236106 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 19 Nov 2020 11:59:05 -0600
Subject: [PATCH 02/12] Refactor: libcrmcommon: expose "first XML attribute"
function
... for future reuse. Also, rename to reflect that only XML element nodes may
have attributes.
---
include/crm/common/xml_internal.h | 14 +++++++
lib/common/crmcommon_private.h | 6 ---
lib/common/nvpair.c | 4 +-
lib/common/patchset.c | 20 +++++-----
lib/common/xml.c | 81 +++++++++++++++++----------------------
5 files changed, 61 insertions(+), 64 deletions(-)
diff --git a/include/crm/common/xml_internal.h b/include/crm/common/xml_internal.h
index 13157c6..b2ff529 100644
--- a/include/crm/common/xml_internal.h
+++ b/include/crm/common/xml_internal.h
@@ -255,4 +255,18 @@ void
pcmk__xe_set_props(xmlNodePtr node, ...)
G_GNUC_NULL_TERMINATED;
+/*!
+ * \internal
+ * \brief Get first attribute of an XML element
+ *
+ * \param[in] xe XML element to check
+ *
+ * \return First attribute of \p xe (or NULL if \p xe is NULL or has none)
+ */
+static inline xmlAttr *
+pcmk__xe_first_attr(const xmlNode *xe)
+{
+ return (xe == NULL)? NULL : xe->properties;
+}
+
#endif // PCMK__XML_INTERNAL__H
diff --git a/lib/common/crmcommon_private.h b/lib/common/crmcommon_private.h
index 1654cba..c2f334d 100644
--- a/lib/common/crmcommon_private.h
+++ b/lib/common/crmcommon_private.h
@@ -141,12 +141,6 @@ void pcmk__mark_xml_attr_dirty(xmlAttr *a);
G_GNUC_INTERNAL
bool pcmk__xa_filterable(const char *name);
-static inline xmlAttr *
-pcmk__first_xml_attr(const xmlNode *xml)
-{
- return xml? xml->properties : NULL;
-}
-
static inline const char *
pcmk__xml_attr_value(const xmlAttr *attr)
{
diff --git a/lib/common/nvpair.c b/lib/common/nvpair.c
index 0ed7a7d..9bd87af 100644
--- a/lib/common/nvpair.c
+++ b/lib/common/nvpair.c
@@ -163,7 +163,7 @@ pcmk_xml_attrs2nvpairs(xmlNode *xml)
{
GSList *result = NULL;
- for (xmlAttrPtr iter = pcmk__first_xml_attr(xml); iter != NULL;
+ for (xmlAttrPtr iter = pcmk__xe_first_attr(xml); iter != NULL;
iter = iter->next) {
result = pcmk_prepend_nvpair(result,
@@ -925,7 +925,7 @@ xml2list(xmlNode *parent)
crm_log_xml_trace(nvpair_list, "Unpacking");
- for (pIter = pcmk__first_xml_attr(nvpair_list); pIter != NULL;
+ for (pIter = pcmk__xe_first_attr(nvpair_list); pIter != NULL;
pIter = pIter->next) {
const char *p_name = (const char *)pIter->name;
diff --git a/lib/common/patchset.c b/lib/common/patchset.c
index f3dab03..15cbe07 100644
--- a/lib/common/patchset.c
+++ b/lib/common/patchset.c
@@ -112,7 +112,7 @@ add_xml_changes_to_patchset(xmlNode *xml, xmlNode *patchset)
}
// Check each of the XML node's attributes for changes
- for (pIter = pcmk__first_xml_attr(xml); pIter != NULL;
+ for (pIter = pcmk__xe_first_attr(xml); pIter != NULL;
pIter = pIter->next) {
xmlNode *attr = NULL;
@@ -156,7 +156,7 @@ add_xml_changes_to_patchset(xmlNode *xml, xmlNode *patchset)
change = create_xml_node(change->parent, XML_DIFF_RESULT);
result = create_xml_node(change, (const char *)xml->name);
- for (pIter = pcmk__first_xml_attr(xml); pIter != NULL;
+ for (pIter = pcmk__xe_first_attr(xml); pIter != NULL;
pIter = pIter->next) {
p = pIter->_private;
if (!pcmk_is_set(p->flags, xpf_deleted)) {
@@ -677,7 +677,7 @@ process_v1_removals(xmlNode *target, xmlNode *patch)
return;
}
- for (xIter = pcmk__first_xml_attr(patch); xIter != NULL;
+ for (xIter = pcmk__xe_first_attr(patch); xIter != NULL;
xIter = xIter->next) {
const char *p_name = (const char *)xIter->name;
@@ -745,7 +745,7 @@ process_v1_additions(xmlNode *parent, xmlNode *target, xmlNode *patch)
return);
CRM_CHECK(pcmk__str_eq(ID(target), ID(patch), pcmk__str_casei), return);
- for (xIter = pcmk__first_xml_attr(patch); xIter != NULL;
+ for (xIter = pcmk__xe_first_attr(patch); xIter != NULL;
xIter = xIter->next) {
const char *p_name = (const char *) xIter->name;
const char *p_value = crm_element_value(patch, p_name);
@@ -1204,7 +1204,7 @@ apply_v2_patchset(xmlNode *xml, xmlNode *patchset)
free_xml(match);
} else if (strcmp(op, "modify") == 0) {
- xmlAttr *pIter = pcmk__first_xml_attr(match);
+ xmlAttr *pIter = pcmk__xe_first_attr(match);
xmlNode *attrs = NULL;
attrs = pcmk__xml_first_child(first_named_child(change,
@@ -1220,7 +1220,7 @@ apply_v2_patchset(xmlNode *xml, xmlNode *patchset)
xml_remove_prop(match, name);
}
- for (pIter = pcmk__first_xml_attr(attrs); pIter != NULL;
+ for (pIter = pcmk__xe_first_attr(attrs); pIter != NULL;
pIter = pIter->next) {
const char *name = (const char *) pIter->name;
const char *value = crm_element_value(attrs, name);
@@ -1553,7 +1553,7 @@ subtract_xml_object(xmlNode *parent, xmlNode *left, xmlNode *right,
} else if (full) {
xmlAttrPtr pIter = NULL;
- for (pIter = pcmk__first_xml_attr(left); pIter != NULL;
+ for (pIter = pcmk__xe_first_attr(left); pIter != NULL;
pIter = pIter->next) {
const char *p_name = (const char *)pIter->name;
const char *p_value = pcmk__xml_attr_value(pIter);
@@ -1566,7 +1566,7 @@ subtract_xml_object(xmlNode *parent, xmlNode *left, xmlNode *right,
}
// Changes to name/value pairs
- for (xIter = pcmk__first_xml_attr(left); xIter != NULL;
+ for (xIter = pcmk__xe_first_attr(left); xIter != NULL;
xIter = xIter->next) {
const char *prop_name = (const char *) xIter->name;
xmlAttrPtr right_attr = NULL;
@@ -1594,7 +1594,7 @@ subtract_xml_object(xmlNode *parent, xmlNode *left, xmlNode *right,
if (full) {
xmlAttrPtr pIter = NULL;
- for (pIter = pcmk__first_xml_attr(left); pIter != NULL;
+ for (pIter = pcmk__xe_first_attr(left); pIter != NULL;
pIter = pIter->next) {
const char *p_name = (const char *) pIter->name;
const char *p_value = pcmk__xml_attr_value(pIter);
@@ -1624,7 +1624,7 @@ subtract_xml_object(xmlNode *parent, xmlNode *left, xmlNode *right,
crm_trace("Changes detected to %s in <%s id=%s>", prop_name,
crm_element_name(left), id);
- for (pIter = pcmk__first_xml_attr(left); pIter != NULL;
+ for (pIter = pcmk__xe_first_attr(left); pIter != NULL;
pIter = pIter->next) {
const char *p_name = (const char *) pIter->name;
const char *p_value = pcmk__xml_attr_value(pIter);
diff --git a/lib/common/xml.c b/lib/common/xml.c
index abb120c..f2f48e9 100644
--- a/lib/common/xml.c
+++ b/lib/common/xml.c
@@ -337,7 +337,7 @@ accept_attr_deletions(xmlNode *xml)
xml_private_t *p = xml->_private;
p->flags = xpf_none;
- pIter = pcmk__first_xml_attr(xml);
+ pIter = pcmk__xe_first_attr(xml);
while (pIter != NULL) {
const xmlChar *name = pIter->name;
@@ -528,11 +528,9 @@ copy_in_properties(xmlNode * target, xmlNode * src)
crm_err("No node to copy properties into");
} else {
- xmlAttrPtr pIter = NULL;
-
- for (pIter = pcmk__first_xml_attr(src); pIter != NULL; pIter = pIter->next) {
- const char *p_name = (const char *)pIter->name;
- const char *p_value = pcmk__xml_attr_value(pIter);
+ for (xmlAttrPtr a = pcmk__xe_first_attr(src); a != NULL; a = a->next) {
+ const char *p_name = (const char *) a->name;
+ const char *p_value = pcmk__xml_attr_value(a);
expand_plus_plus(target, p_name, p_value);
}
@@ -546,11 +544,10 @@ fix_plus_plus_recursive(xmlNode * target)
{
/* TODO: Remove recursion and use xpath searches for value++ */
xmlNode *child = NULL;
- xmlAttrPtr pIter = NULL;
- for (pIter = pcmk__first_xml_attr(target); pIter != NULL; pIter = pIter->next) {
- const char *p_name = (const char *)pIter->name;
- const char *p_value = pcmk__xml_attr_value(pIter);
+ for (xmlAttrPtr a = pcmk__xe_first_attr(target); a != NULL; a = a->next) {
+ const char *p_name = (const char *) a->name;
+ const char *p_value = pcmk__xml_attr_value(a);
expand_plus_plus(target, p_name, p_value);
}
@@ -1429,7 +1426,6 @@ pcmk__xe_log(int log_level, const char *file, const char *function, int line,
const char *hidden = NULL;
xmlNode *child = NULL;
- xmlAttrPtr pIter = NULL;
if ((data == NULL) || (log_level == LOG_NEVER)) {
return;
@@ -1449,10 +1445,12 @@ pcmk__xe_log(int log_level, const char *file, const char *function, int line,
buffer_print(buffer, max, offset, "<%s", name);
hidden = crm_element_value(data, "hidden");
- for (pIter = pcmk__first_xml_attr(data); pIter != NULL; pIter = pIter->next) {
- xml_private_t *p = pIter->_private;
- const char *p_name = (const char *)pIter->name;
- const char *p_value = pcmk__xml_attr_value(pIter);
+ for (xmlAttrPtr a = pcmk__xe_first_attr(data); a != NULL;
+ a = a->next) {
+
+ xml_private_t *p = a->_private;
+ const char *p_name = (const char *) a->name;
+ const char *p_value = pcmk__xml_attr_value(a);
char *p_copy = NULL;
if (pcmk_is_set(p->flags, xpf_deleted)) {
@@ -1526,7 +1524,6 @@ log_xml_changes(int log_level, const char *file, const char *function, int line,
xml_private_t *p;
char *prefix_m = NULL;
xmlNode *child = NULL;
- xmlAttrPtr pIter = NULL;
if ((data == NULL) || (log_level == LOG_NEVER)) {
return;
@@ -1566,10 +1563,10 @@ log_xml_changes(int log_level, const char *file, const char *function, int line,
pcmk__xe_log(log_level, file, function, line, flags, data, depth,
options|xml_log_option_open);
- for (pIter = pcmk__first_xml_attr(data); pIter != NULL; pIter = pIter->next) {
- const char *aname = (const char*)pIter->name;
+ for (xmlAttrPtr a = pcmk__xe_first_attr(data); a != NULL; a = a->next) {
+ const char *aname = (const char*) a->name;
- p = pIter->_private;
+ p = a->_private;
if (pcmk_is_set(p->flags, xpf_deleted)) {
const char *value = crm_element_value(data, aname);
flags = prefix_del;
@@ -1684,11 +1681,9 @@ log_data_element(int log_level, const char *file, const char *function, int line
static void
dump_filtered_xml(xmlNode * data, int options, char **buffer, int *offset, int *max)
{
- xmlAttrPtr xIter = NULL;
-
- for (xIter = pcmk__first_xml_attr(data); xIter != NULL; xIter = xIter->next) {
- if (!pcmk__xa_filterable((const char *) (xIter->name))) {
- dump_xml_attr(xIter, options, buffer, offset, max);
+ for (xmlAttrPtr a = pcmk__xe_first_attr(data); a != NULL; a = a->next) {
+ if (!pcmk__xa_filterable((const char *) (a->name))) {
+ dump_xml_attr(a, options, buffer, offset, max);
}
}
}
@@ -1722,10 +1717,8 @@ dump_xml_element(xmlNode * data, int options, char **buffer, int *offset, int *m
dump_filtered_xml(data, options, buffer, offset, max);
} else {
- xmlAttrPtr xIter = NULL;
-
- for (xIter = pcmk__first_xml_attr(data); xIter != NULL; xIter = xIter->next) {
- dump_xml_attr(xIter, options, buffer, offset, max);
+ for (xmlAttrPtr a = pcmk__xe_first_attr(data); a != NULL; a = a->next) {
+ dump_xml_attr(a, options, buffer, offset, max);
}
}
@@ -2062,7 +2055,7 @@ save_xml_to_file(xmlNode * xml, const char *desc, const char *filename)
static void
set_attrs_flag(xmlNode *xml, enum xml_private_flags flag)
{
- for (xmlAttr *attr = pcmk__first_xml_attr(xml); attr; attr = attr->next) {
+ for (xmlAttr *attr = pcmk__xe_first_attr(xml); attr; attr = attr->next) {
pcmk__set_xml_flags((xml_private_t *) (attr->_private), flag);
}
}
@@ -2152,7 +2145,7 @@ mark_attr_moved(xmlNode *new_xml, const char *element, xmlAttr *old_attr,
static void
xml_diff_old_attrs(xmlNode *old_xml, xmlNode *new_xml)
{
- xmlAttr *attr_iter = pcmk__first_xml_attr(old_xml);
+ xmlAttr *attr_iter = pcmk__xe_first_attr(old_xml);
while (attr_iter != NULL) {
xmlAttr *old_attr = attr_iter;
@@ -2194,7 +2187,7 @@ xml_diff_old_attrs(xmlNode *old_xml, xmlNode *new_xml)
static void
mark_created_attrs(xmlNode *new_xml)
{
- xmlAttr *attr_iter = pcmk__first_xml_attr(new_xml);
+ xmlAttr *attr_iter = pcmk__xe_first_attr(new_xml);
while (attr_iter != NULL) {
xmlAttr *new_attr = attr_iter;
@@ -2371,7 +2364,6 @@ gboolean
can_prune_leaf(xmlNode * xml_node)
{
xmlNode *cIter = NULL;
- xmlAttrPtr pIter = NULL;
gboolean can_prune = TRUE;
const char *name = crm_element_name(xml_node);
@@ -2380,8 +2372,8 @@ can_prune_leaf(xmlNode * xml_node)
return FALSE;
}
- for (pIter = pcmk__first_xml_attr(xml_node); pIter != NULL; pIter = pIter->next) {
- const char *p_name = (const char *)pIter->name;
+ for (xmlAttrPtr a = pcmk__xe_first_attr(xml_node); a != NULL; a = a->next) {
+ const char *p_name = (const char *) a->name;
if (strcmp(p_name, XML_ATTR_ID) == 0) {
continue;
@@ -2558,15 +2550,13 @@ pcmk__xml_update(xmlNode *parent, xmlNode *target, xmlNode *update,
} else {
/* No need for expand_plus_plus(), just raw speed */
- xmlAttrPtr pIter = NULL;
-
- for (pIter = pcmk__first_xml_attr(update); pIter != NULL; pIter = pIter->next) {
- const char *p_name = (const char *)pIter->name;
- const char *p_value = pcmk__xml_attr_value(pIter);
+ for (xmlAttrPtr a = pcmk__xe_first_attr(update); a != NULL;
+ a = a->next) {
+ const char *p_value = pcmk__xml_attr_value(a);
/* Remove it first so the ordering of the update is preserved */
- xmlUnsetProp(target, (pcmkXmlStr) p_name);
- xmlSetProp(target, (pcmkXmlStr) p_name, (pcmkXmlStr) p_value);
+ xmlUnsetProp(target, a->name);
+ xmlSetProp(target, a->name, (pcmkXmlStr) p_value);
}
}
@@ -2681,11 +2671,10 @@ replace_xml_child(xmlNode * parent, xmlNode * child, xmlNode * update, gboolean
can_delete = FALSE;
}
if (can_delete && delete_only) {
- xmlAttrPtr pIter = NULL;
-
- for (pIter = pcmk__first_xml_attr(update); pIter != NULL; pIter = pIter->next) {
- const char *p_name = (const char *)pIter->name;
- const char *p_value = pcmk__xml_attr_value(pIter);
+ for (xmlAttrPtr a = pcmk__xe_first_attr(update); a != NULL;
+ a = a->next) {
+ const char *p_name = (const char *) a->name;
+ const char *p_value = pcmk__xml_attr_value(a);
right_val = crm_element_value(child, p_name);
if (!pcmk__str_eq(p_value, right_val, pcmk__str_casei)) {
--
1.8.3.1
From 68e35a6f715f26a8a3b033cc91875f6f7001f106 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 19 Nov 2020 10:31:36 -0600
Subject: [PATCH 03/12] Refactor: libcrmcommon: new internal function for
removing XML attributes
---
include/crm/common/xml_internal.h | 4 ++++
lib/common/xml.c | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 38 insertions(+)
diff --git a/include/crm/common/xml_internal.h b/include/crm/common/xml_internal.h
index b2ff529..1e80bc6 100644
--- a/include/crm/common/xml_internal.h
+++ b/include/crm/common/xml_internal.h
@@ -136,6 +136,10 @@ const char *pcmk__xe_add_last_written(xmlNode *xe);
xmlNode *pcmk__xe_match(xmlNode *parent, const char *node_name,
const char *attr_n, const char *attr_v);
+void pcmk__xe_remove_matching_attrs(xmlNode *element,
+ bool (*match)(xmlAttrPtr, void *),
+ void *user_data);
+
/*!
* \internal
* \brief Get the root directory to scan XML artefacts of given kind for
diff --git a/lib/common/xml.c b/lib/common/xml.c
index f2f48e9..39c5e53 100644
--- a/lib/common/xml.c
+++ b/lib/common/xml.c
@@ -618,6 +618,40 @@ expand_plus_plus(xmlNode * target, const char *name, const char *value)
return;
}
+/*!
+ * \internal
+ * \brief Remove an XML element's attributes that match some criteria
+ *
+ * \param[in,out] element XML element to modify
+ * \param[in] match If not NULL, only remove attributes for which
+ * this function returns true
+ * \param[in] user_data Data to pass to \p match
+ */
+void
+pcmk__xe_remove_matching_attrs(xmlNode *element,
+ bool (*match)(xmlAttrPtr, void *),
+ void *user_data)
+{
+ xmlAttrPtr next = NULL;
+
+ for (xmlAttrPtr a = pcmk__xe_first_attr(element); a != NULL; a = next) {
+ next = a->next; // Grab now because attribute might get removed
+ if ((match == NULL) || match(a, user_data)) {
+ if (!pcmk__check_acl(element, NULL, xpf_acl_write)) {
+ crm_trace("ACLs prevent removal of %s attribute from %s element",
+ (const char *) a->name, (const char *) element->name);
+
+ } else if (pcmk__tracking_xml_changes(element, false)) {
+ // Leave (marked for removal) until after diff is calculated
+ set_parent_flag(element, xpf_dirty);
+ pcmk__set_xml_flags((xml_private_t *) a->_private, xpf_deleted);
+ } else {
+ xmlRemoveProp(a);
+ }
+ }
+ }
+}
+
xmlDoc *
getDocPtr(xmlNode * node)
{
--
1.8.3.1
From a6e1804156dfd1f6c3bc1ffbf888fbd6421aa464 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 19 Nov 2020 11:10:52 -0600
Subject: [PATCH 04/12] Refactor: libcrmcommon,libpe_status: use new attribute
removing function
... where appropriate, for readability and efficiency
---
lib/common/operations.c | 64 ++++++++++++++++++-------------------
lib/common/patchset.c | 29 +++++++----------
lib/common/xml.c | 39 +++++++++++------------
lib/pengine/pe_digest.c | 84 ++++++++++++++++++++++++++-----------------------
4 files changed, 105 insertions(+), 111 deletions(-)
diff --git a/lib/common/operations.c b/lib/common/operations.c
index 7e6bf5a..f3a11be 100644
--- a/lib/common/operations.c
+++ b/lib/common/operations.c
@@ -24,6 +24,7 @@
#include <crm/lrmd.h>
#include <crm/msg_xml.h>
#include <crm/common/xml.h>
+#include <crm/common/xml_internal.h>
#include <crm/common/util.h>
static regex_t *notify_migrate_re = NULL;
@@ -361,6 +362,24 @@ decode_transition_key(const char *key, char **uuid, int *transition_id, int *act
return TRUE;
}
+#define CRM_META_LEN (sizeof(CRM_META) - 1)
+
+// Return true if a is an attribute that should be filtered
+static bool
+should_filter_for_digest(xmlAttrPtr a, void *user_data)
+{
+ // @TODO CRM_META check should be case-sensitive
+ return (strncasecmp((const char *) a->name, CRM_META, CRM_META_LEN) == 0)
+ || pcmk__str_any_of((const char *) a->name,
+ XML_ATTR_ID,
+ XML_ATTR_CRM_VERSION,
+ XML_LRM_ATTR_OP_DIGEST,
+ XML_LRM_ATTR_TARGET,
+ XML_LRM_ATTR_TARGET_UUID,
+ "pcmk_external_ip",
+ NULL);
+}
+
/*!
* \internal
* \brief Remove XML attributes not needed for operation digest
@@ -374,52 +393,31 @@ pcmk__filter_op_for_digest(xmlNode *param_set)
char *timeout = NULL;
guint interval_ms = 0;
- const char *attr_filter[] = {
- XML_ATTR_ID,
- XML_ATTR_CRM_VERSION,
- XML_LRM_ATTR_OP_DIGEST,
- XML_LRM_ATTR_TARGET,
- XML_LRM_ATTR_TARGET_UUID,
- "pcmk_external_ip"
- };
-
- const int meta_len = strlen(CRM_META);
-
if (param_set == NULL) {
return;
}
- // Remove the specific attributes listed in attr_filter
- for (int lpc = 0; lpc < DIMOF(attr_filter); lpc++) {
- xml_remove_prop(param_set, attr_filter[lpc]);
- }
-
+ /* Timeout is useful for recurring operation digests, so grab it before
+ * removing meta-attributes
+ */
key = crm_meta_name(XML_LRM_ATTR_INTERVAL_MS);
if (crm_element_value_ms(param_set, key, &interval_ms) != pcmk_ok) {
interval_ms = 0;
}
free(key);
-
- key = crm_meta_name(XML_ATTR_TIMEOUT);
- timeout = crm_element_value_copy(param_set, key);
-
- // Remove all CRM_meta_* attributes
- for (xmlAttrPtr xIter = param_set->properties; xIter != NULL; ) {
- const char *prop_name = (const char *) (xIter->name);
-
- xIter = xIter->next;
-
- // @TODO Why is this case-insensitive?
- if (strncasecmp(prop_name, CRM_META, meta_len) == 0) {
- xml_remove_prop(param_set, prop_name);
- }
+ key = NULL;
+ if (interval_ms != 0) {
+ key = crm_meta_name(XML_ATTR_TIMEOUT);
+ timeout = crm_element_value_copy(param_set, key);
}
- if ((interval_ms != 0) && (timeout != NULL)) {
- // Add the timeout back, it's useful for recurring operation digests
+ // Remove all CRM_meta_* attributes and certain other attributes
+ pcmk__xe_remove_matching_attrs(param_set, should_filter_for_digest, NULL);
+
+ // Add timeout back for recurring operation digests
+ if (timeout != NULL) {
crm_xml_add(param_set, key, timeout);
}
-
free(timeout);
free(key);
}
diff --git a/lib/common/patchset.c b/lib/common/patchset.c
index 15cbe07..fae7046 100644
--- a/lib/common/patchset.c
+++ b/lib/common/patchset.c
@@ -638,13 +638,19 @@ xml_log_patchset(uint8_t log_level, const char *function, xmlNode *patchset)
}
}
+// Return true if attribute name is not "id"
+static bool
+not_id(xmlAttrPtr attr, void *user_data)
+{
+ return strcmp((const char *) attr->name, XML_ATTR_ID) != 0;
+}
+
// Apply the removals section of an v1 patchset to an XML node
static void
process_v1_removals(xmlNode *target, xmlNode *patch)
{
xmlNode *patch_child = NULL;
xmlNode *cIter = NULL;
- xmlAttrPtr xIter = NULL;
char *id = NULL;
const char *name = NULL;
@@ -677,15 +683,8 @@ process_v1_removals(xmlNode *target, xmlNode *patch)
return;
}
- for (xIter = pcmk__xe_first_attr(patch); xIter != NULL;
- xIter = xIter->next) {
- const char *p_name = (const char *)xIter->name;
-
- // Removing then restoring id would change ordering of properties
- if (!pcmk__str_eq(p_name, XML_ATTR_ID, pcmk__str_casei)) {
- xml_remove_prop(target, p_name);
- }
- }
+ // Removing then restoring id would change ordering of properties
+ pcmk__xe_remove_matching_attrs(patch, not_id, NULL);
// Changes to child objects
cIter = pcmk__xml_first_child(target);
@@ -1204,7 +1203,6 @@ apply_v2_patchset(xmlNode *xml, xmlNode *patchset)
free_xml(match);
} else if (strcmp(op, "modify") == 0) {
- xmlAttr *pIter = pcmk__xe_first_attr(match);
xmlNode *attrs = NULL;
attrs = pcmk__xml_first_child(first_named_child(change,
@@ -1213,14 +1211,9 @@ apply_v2_patchset(xmlNode *xml, xmlNode *patchset)
rc = ENOMSG;
continue;
}
- while (pIter != NULL) {
- const char *name = (const char *)pIter->name;
-
- pIter = pIter->next;
- xml_remove_prop(match, name);
- }
+ pcmk__xe_remove_matching_attrs(match, NULL, NULL); // Remove all
- for (pIter = pcmk__xe_first_attr(attrs); pIter != NULL;
+ for (xmlAttrPtr pIter = pcmk__xe_first_attr(attrs); pIter != NULL;
pIter = pIter->next) {
const char *name = (const char *) pIter->name;
const char *value = crm_element_value(attrs, name);
diff --git a/lib/common/xml.c b/lib/common/xml.c
index 39c5e53..869ed51 100644
--- a/lib/common/xml.c
+++ b/lib/common/xml.c
@@ -328,32 +328,31 @@ pcmk__xml_position(xmlNode *xml, enum xml_private_flags ignore_if_set)
return position;
}
-// Remove all attributes marked as deleted from an XML node
-static void
-accept_attr_deletions(xmlNode *xml)
+// This also clears attribute's flags if not marked as deleted
+static bool
+marked_as_deleted(xmlAttrPtr a, void *user_data)
{
- xmlNode *cIter = NULL;
- xmlAttr *pIter = NULL;
- xml_private_t *p = xml->_private;
+ xml_private_t *p = a->_private;
+ if (pcmk_is_set(p->flags, xpf_deleted)) {
+ return true;
+ }
p->flags = xpf_none;
- pIter = pcmk__xe_first_attr(xml);
-
- while (pIter != NULL) {
- const xmlChar *name = pIter->name;
-
- p = pIter->_private;
- pIter = pIter->next;
+ return false;
+}
- if(p->flags & xpf_deleted) {
- xml_remove_prop(xml, (const char *)name);
+// Remove all attributes marked as deleted from an XML node
+static void
+accept_attr_deletions(xmlNode *xml)
+{
+ // Clear XML node's flags
+ ((xml_private_t *) xml->_private)->flags = xpf_none;
- } else {
- p->flags = xpf_none;
- }
- }
+ // Remove this XML node's attributes that were marked as deleted
+ pcmk__xe_remove_matching_attrs(xml, marked_as_deleted, NULL);
- for (cIter = pcmk__xml_first_child(xml); cIter != NULL;
+ // Recursively do the same for this XML node's children
+ for (xmlNodePtr cIter = pcmk__xml_first_child(xml); cIter != NULL;
cIter = pcmk__xml_next(cIter)) {
accept_attr_deletions(cIter);
}
diff --git a/lib/pengine/pe_digest.c b/lib/pengine/pe_digest.c
index dd6b753..e8cb108 100644
--- a/lib/pengine/pe_digest.c
+++ b/lib/pengine/pe_digest.c
@@ -15,6 +15,7 @@
#include <crm/crm.h>
#include <crm/msg_xml.h>
#include <crm/common/xml.h>
+#include <crm/common/xml_internal.h>
#include <crm/pengine/internal.h>
#include "pe_status_private.h"
@@ -45,40 +46,36 @@ pe__free_digests(gpointer ptr)
}
}
-/*!
- * \internal
- * \brief Remove named attributes from an XML element
- *
- * \param[in,out] param_set XML to be filtered
- * \param[in] param_string Space-separated list of attribute names
- * \param[in] need_present Whether to remove attributes that match,
- * or those that don't match
- */
-static void
-filter_parameters(xmlNode *param_set, const char *param_string,
- bool need_present)
+// Return true if XML attribute name is substring of a given string
+static bool
+attr_in_string(xmlAttrPtr a, void *user_data)
{
- if ((param_set == NULL) || (param_string == NULL)) {
- return;
- }
- for (xmlAttrPtr xIter = param_set->properties; xIter; ) {
- const char *prop_name = (const char *) xIter->name;
- char *name = crm_strdup_printf(" %s ", prop_name);
- char *match = strstr(param_string, name);
-
- free(name);
+ bool filter = false;
+ char *name = crm_strdup_printf(" %s ", (const char *) a->name);
- // Do now, because current entry might get removed below
- xIter = xIter->next;
+ if (strstr((const char *) user_data, name) == NULL) {
+ crm_trace("Filtering %s (not found in '%s')",
+ (const char *) a->name, (const char *) user_data);
+ filter = true;
+ }
+ free(name);
+ return filter;
+}
- if ((need_present && (match == NULL))
- || (!need_present && (match != NULL))) {
+// Return true if XML attribute name is not substring of a given string
+static bool
+attr_not_in_string(xmlAttrPtr a, void *user_data)
+{
+ bool filter = false;
+ char *name = crm_strdup_printf(" %s ", (const char *) a->name);
- crm_trace("Filtering %s (%sfound in '%s')",
- prop_name, (need_present? "not " : ""), param_string);
- xml_remove_prop(param_set, prop_name);
- }
+ if (strstr((const char *) user_data, name) != NULL) {
+ crm_trace("Filtering %s (found in '%s')",
+ (const char *) a->name, (const char *) user_data);
+ filter = true;
}
+ free(name);
+ return filter;
}
#if ENABLE_VERSIONED_ATTRS
@@ -177,6 +174,13 @@ calculate_main_digest(op_digest_cache_t *data, pe_resource_t *rsc,
op_version);
}
+// Return true if XML attribute name is a Pacemaker-defined fencing parameter
+static bool
+is_fence_param(xmlAttrPtr attr, void *user_data)
+{
+ return pcmk_stonith_param((const char *) attr->name);
+}
+
/*!
* \internal
* \brief Add secure digest to a digest cache entry
@@ -209,8 +213,12 @@ calculate_secure_digest(op_digest_cache_t *data, pe_resource_t *rsc,
if (overrides != NULL) {
g_hash_table_foreach(overrides, hash2field, data->params_secure);
}
+
g_hash_table_foreach(rsc->parameters, hash2field, data->params_secure);
- filter_parameters(data->params_secure, secure_list, FALSE);
+ if (secure_list != NULL) {
+ pcmk__xe_remove_matching_attrs(data->params_secure, attr_not_in_string,
+ (void *) secure_list);
+ }
if (pcmk_is_set(pcmk_get_ra_caps(class),
pcmk_ra_cap_fence_params)) {
/* For stonith resources, Pacemaker adds special parameters,
@@ -218,15 +226,8 @@ calculate_secure_digest(op_digest_cache_t *data, pe_resource_t *rsc,
* controller will not hash them. That means we have to filter
* them out before calculating our hash for comparison.
*/
- for (xmlAttrPtr iter = data->params_secure->properties;
- iter != NULL; ) {
- const char *prop_name = (const char *) iter->name;
-
- iter = iter->next; // Grab next now in case we remove current
- if (pcmk_stonith_param(prop_name)) {
- xml_remove_prop(data->params_secure, prop_name);
- }
- }
+ pcmk__xe_remove_matching_attrs(data->params_secure, is_fence_param,
+ NULL);
}
data->digest_secure_calc = calculate_operation_digest(data->params_secure,
op_version);
@@ -264,7 +265,10 @@ calculate_restart_digest(op_digest_cache_t *data, xmlNode *xml_op,
// Then filter out reloadable parameters, if any
value = crm_element_value(xml_op, XML_LRM_ATTR_OP_RESTART);
- filter_parameters(data->params_restart, value, TRUE);
+ if (value != NULL) {
+ pcmk__xe_remove_matching_attrs(data->params_restart, attr_in_string,
+ (void *) value);
+ }
value = crm_element_value(xml_op, XML_ATTR_CRM_VERSION);
data->digest_restart_calc = calculate_operation_digest(data->params_restart,
--
1.8.3.1
From 579875b70c2399f6cbf15d7afd210a2d4e2ed9df Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 19 Nov 2020 11:47:20 -0600
Subject: [PATCH 05/12] Refactor: libcrmcommon,libpe_status: use "first XML
attribute" function
... where appropriate. It checks for NULL, so that doesn't need to be
duplicated.
---
lib/common/patchset.c | 11 +++--------
lib/pengine/complex.c | 12 ++++--------
2 files changed, 7 insertions(+), 16 deletions(-)
diff --git a/lib/common/patchset.c b/lib/common/patchset.c
index fae7046..46d136a 100644
--- a/lib/common/patchset.c
+++ b/lib/common/patchset.c
@@ -279,15 +279,10 @@ xml_repair_v1_diff(xmlNode *last, xmlNode *next, xmlNode *local_diff,
crm_xml_add(diff_child, vfields[lpc], value);
}
- if (next) {
- xmlAttrPtr xIter = NULL;
+ for (xmlAttrPtr a = pcmk__xe_first_attr(next); a != NULL; a = a->next) {
+ const char *p_value = crm_element_value(next, (const char *) a->name);
- for (xIter = next->properties; xIter; xIter = xIter->next) {
- const char *p_name = (const char *) xIter->name;
- const char *p_value = crm_element_value(next, p_name);
-
- xmlSetProp(cib, (pcmkXmlStr) p_name, (pcmkXmlStr) p_value);
- }
+ xmlSetProp(cib, a->name, (pcmkXmlStr) p_value);
}
crm_log_xml_explicit(local_diff, "Repaired-diff");
diff --git a/lib/pengine/complex.c b/lib/pengine/complex.c
index a087e5e..5f484ad 100644
--- a/lib/pengine/complex.c
+++ b/lib/pengine/complex.c
@@ -159,15 +159,11 @@ get_meta_attributes(GHashTable * meta_hash, pe_resource_t * rsc,
rule_data.node_hash = node->details->attrs;
}
- if (rsc->xml) {
- xmlAttrPtr xIter = NULL;
+ for (xmlAttrPtr a = pcmk__xe_first_attr(rsc->xml); a != NULL; a = a->next) {
+ const char *prop_name = (const char *) a->name;
+ const char *prop_value = crm_element_value(rsc->xml, prop_name);
- for (xIter = rsc->xml->properties; xIter; xIter = xIter->next) {
- const char *prop_name = (const char *)xIter->name;
- const char *prop_value = crm_element_value(rsc->xml, prop_name);
-
- add_hash_param(meta_hash, prop_name, prop_value);
- }
+ add_hash_param(meta_hash, prop_name, prop_value);
}
pe__unpack_dataset_nvpairs(rsc->xml, XML_TAG_META_SETS, &rule_data,
--
1.8.3.1
From a30d60e6344129e03fcab9cd8c9523bf24951a92 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 19 Nov 2020 13:37:37 -0600
Subject: [PATCH 06/12] Low: libcrmcommon: compare CRM_meta_ properly
Previously, when filtering XML attributes for digests, we would filter
attributes starting with "CRM_meta" case-insensitively. Now, compare
against "CRM_meta_" case-sensitively.
This could potentially cause resource restarts after upgrading to a version
with this commit, for any resource that has instance attributes that start with
something like "crm_meta" -- a highly unlikely scenario.
---
lib/common/operations.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/common/operations.c b/lib/common/operations.c
index f3a11be..421aaac 100644
--- a/lib/common/operations.c
+++ b/lib/common/operations.c
@@ -362,14 +362,14 @@ decode_transition_key(const char *key, char **uuid, int *transition_id, int *act
return TRUE;
}
-#define CRM_META_LEN (sizeof(CRM_META) - 1)
+// String length of CRM_META"_"
+#define CRM_META_LEN sizeof(CRM_META)
// Return true if a is an attribute that should be filtered
static bool
should_filter_for_digest(xmlAttrPtr a, void *user_data)
{
- // @TODO CRM_META check should be case-sensitive
- return (strncasecmp((const char *) a->name, CRM_META, CRM_META_LEN) == 0)
+ return (strncmp((const char *) a->name, CRM_META "_", CRM_META_LEN) == 0)
|| pcmk__str_any_of((const char *) a->name,
XML_ATTR_ID,
XML_ATTR_CRM_VERSION,
--
1.8.3.1
From cb81616eaf53e7029a0c196ffab6203d60389386 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Mon, 23 Nov 2020 11:04:08 -0600
Subject: [PATCH 07/12] Low: scheduler: filter non-private parameters properly
for digest
Do the same filtering for non-private parameter digests as for all-parameter
digests. This matters when a filterable parameter is specified in overrides.
---
lib/pengine/pe_digest.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/pengine/pe_digest.c b/lib/pengine/pe_digest.c
index e8cb108..03aa3bc 100644
--- a/lib/pengine/pe_digest.c
+++ b/lib/pengine/pe_digest.c
@@ -229,6 +229,7 @@ calculate_secure_digest(op_digest_cache_t *data, pe_resource_t *rsc,
pcmk__xe_remove_matching_attrs(data->params_secure, is_fence_param,
NULL);
}
+ pcmk__filter_op_for_digest(data->params_secure);
data->digest_secure_calc = calculate_operation_digest(data->params_secure,
op_version);
}
--
1.8.3.1
From fe4aee37ceca64fd8a512e7cae57bae0745c7ace Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Mon, 23 Nov 2020 17:17:59 -0600
Subject: [PATCH 08/12] Refactor: scheduler: pull interval from digest
overrides if appropriate
When calculating an operation digest, we previously used an operation key
provided by the caller, along with a table of parameter overrides. However, if
CRM_meta_interval is one of the overrides, that should be used in the operation
key instead of the caller-provided key.
Now, the caller passes in a pointer to the interval, and if it's overridden,
pe__calculate_digests() will use the overridden value and reset the caller's
interval to it.
As of this commit, no caller passes overrides, so it has no effect, but it will
be useful for an upcoming feature.
---
include/crm/pengine/internal.h | 2 +-
lib/pengine/pe_digest.c | 117 +++++++++++++++++++++++++----------------
2 files changed, 73 insertions(+), 46 deletions(-)
diff --git a/include/crm/pengine/internal.h b/include/crm/pengine/internal.h
index a4f8086..1e5aee1 100644
--- a/include/crm/pengine/internal.h
+++ b/include/crm/pengine/internal.h
@@ -503,7 +503,7 @@ typedef struct op_digest_cache_s {
} op_digest_cache_t;
op_digest_cache_t *pe__calculate_digests(pe_resource_t *rsc, const char *task,
- const char *key, pe_node_t *node,
+ guint *interval_ms, pe_node_t *node,
xmlNode *xml_op, GHashTable *overrides,
bool calc_secure,
pe_working_set_t *data_set);
diff --git a/lib/pengine/pe_digest.c b/lib/pengine/pe_digest.c
index 03aa3bc..b608f22 100644
--- a/lib/pengine/pe_digest.c
+++ b/lib/pengine/pe_digest.c
@@ -121,19 +121,19 @@ append_all_versioned_params(pe_resource_t *rsc, pe_node_t *node,
* \internal
* \brief Add digest of all parameters to a digest cache entry
*
- * \param[out] data Digest cache entry to modify
- * \param[in] rsc Resource that action was for
- * \param[in] node Node action was performed on
- * \param[in] task Name of action performed
- * \param[in] key Action's task key
- * \param[in] xml_op XML of operation in CIB status (if available)
- * \param[in] op_version CRM feature set to use for digest calculation
- * \param[in] overrides Key/value hash table to override resource parameters
- * \param[in] data_set Cluster working set
+ * \param[out] data Digest cache entry to modify
+ * \param[in] rsc Resource that action was for
+ * \param[in] node Node action was performed on
+ * \param[in] task Name of action performed
+ * \param[in,out] interval_ms Action's interval (will be reset if in overrides)
+ * \param[in] xml_op XML of operation in CIB status (if available)
+ * \param[in] op_version CRM feature set to use for digest calculation
+ * \param[in] overrides Key/value table to override resource parameters
+ * \param[in] data_set Cluster working set
*/
static void
calculate_main_digest(op_digest_cache_t *data, pe_resource_t *rsc,
- pe_node_t *node, const char *task, const char *key,
+ pe_node_t *node, const char *task, guint *interval_ms,
xmlNode *xml_op, const char *op_version,
GHashTable *overrides, pe_working_set_t *data_set)
{
@@ -153,7 +153,24 @@ calculate_main_digest(op_digest_cache_t *data, pe_resource_t *rsc,
rsc->id, node->details->uname);
}
- action = custom_action(rsc, strdup(key), task, node, TRUE, FALSE, data_set);
+ // If interval was overridden, reset it
+ if (overrides != NULL) {
+ const char *interval_s = g_hash_table_lookup(overrides, CRM_META "_"
+ XML_LRM_ATTR_INTERVAL);
+
+ if (interval_s != NULL) {
+ long long value_ll;
+
+ errno = 0;
+ value_ll = crm_parse_ll(interval_s, NULL);
+ if ((errno == 0) && (value_ll >= 0) && (value_ll <= G_MAXUINT)) {
+ *interval_ms = (guint) value_ll;
+ }
+ }
+ }
+
+ action = custom_action(rsc, pcmk__op_key(rsc->id, task, *interval_ms),
+ task, node, TRUE, FALSE, data_set);
if (overrides != NULL) {
g_hash_table_foreach(overrides, hash2field, data->params_all);
}
@@ -280,21 +297,21 @@ calculate_restart_digest(op_digest_cache_t *data, xmlNode *xml_op,
* \internal
* \brief Create a new digest cache entry with calculated digests
*
- * \param[in] rsc Resource that action was for
- * \param[in] task Name of action performed
- * \param[in] key Action's task key
- * \param[in] node Node action was performed on
- * \param[in] xml_op XML of operation in CIB status (if available)
- * \param[in] overrides Key/value hash table to override resource parameters
- * \param[in] calc_secure Whether to calculate secure digest
- * \param[in] data_set Cluster working set
+ * \param[in] rsc Resource that action was for
+ * \param[in] task Name of action performed
+ * \param[in,out] interval_ms Action's interval (will be reset if in overrides)
+ * \param[in] node Node action was performed on
+ * \param[in] xml_op XML of operation in CIB status (if available)
+ * \param[in] overrides Key/value table to override resource parameters
+ * \param[in] calc_secure Whether to calculate secure digest
+ * \param[in] data_set Cluster working set
*
* \return Pointer to new digest cache entry (or NULL on memory error)
* \note It is the caller's responsibility to free the result using
* pe__free_digests().
*/
op_digest_cache_t *
-pe__calculate_digests(pe_resource_t *rsc, const char *task, const char *key,
+pe__calculate_digests(pe_resource_t *rsc, const char *task, guint *interval_ms,
pe_node_t *node, xmlNode *xml_op, GHashTable *overrides,
bool calc_secure, pe_working_set_t *data_set)
{
@@ -307,8 +324,9 @@ pe__calculate_digests(pe_resource_t *rsc, const char *task, const char *key,
if (xml_op != NULL) {
op_version = crm_element_value(xml_op, XML_ATTR_CRM_VERSION);
}
- calculate_main_digest(data, rsc, node, task, key, xml_op, op_version,
- overrides, data_set);
+
+ calculate_main_digest(data, rsc, node, task, interval_ms, xml_op,
+ op_version, overrides, data_set);
if (calc_secure) {
calculate_secure_digest(data, rsc, xml_op, op_version, overrides);
}
@@ -322,7 +340,7 @@ pe__calculate_digests(pe_resource_t *rsc, const char *task, const char *key,
*
* \param[in] rsc Resource that action was for
* \param[in] task Name of action performed
- * \param[in] key Action's task key
+ * \param[in] interval_ms Action's interval
* \param[in] node Node action was performed on
* \param[in] xml_op XML of operation in CIB status (if available)
* \param[in] calc_secure Whether to calculate secure digest
@@ -331,29 +349,40 @@ pe__calculate_digests(pe_resource_t *rsc, const char *task, const char *key,
* \return Pointer to node's digest cache entry
*/
static op_digest_cache_t *
-rsc_action_digest(pe_resource_t *rsc, const char *task, const char *key,
+rsc_action_digest(pe_resource_t *rsc, const char *task, guint interval_ms,
pe_node_t *node, xmlNode *xml_op, bool calc_secure,
pe_working_set_t *data_set)
{
op_digest_cache_t *data = NULL;
+ char *key = pcmk__op_key(rsc->id, task, interval_ms);
data = g_hash_table_lookup(node->details->digest_cache, key);
if (data == NULL) {
- data = pe__calculate_digests(rsc, task, key, node, xml_op, NULL,
- calc_secure, data_set);
+ data = pe__calculate_digests(rsc, task, &interval_ms, node, xml_op,
+ NULL, calc_secure, data_set);
CRM_ASSERT(data != NULL);
g_hash_table_insert(node->details->digest_cache, strdup(key), data);
}
+ free(key);
return data;
}
+/*!
+ * \internal
+ * \brief Calculate operation digests and compare against an XML history entry
+ *
+ * \param[in] rsc Resource to check
+ * \param[in] xml_op Resource history XML
+ * \param[in] node Node to use for digest calculation
+ * \param[in] data_set Cluster working set
+ *
+ * \return Pointer to node's digest cache entry, with comparison result set
+ */
op_digest_cache_t *
rsc_action_digest_cmp(pe_resource_t * rsc, xmlNode * xml_op, pe_node_t * node,
pe_working_set_t * data_set)
{
op_digest_cache_t *data = NULL;
-
- char *key = NULL;
guint interval_ms = 0;
const char *op_version;
@@ -368,17 +397,18 @@ rsc_action_digest_cmp(pe_resource_t * rsc, xmlNode * xml_op, pe_node_t * node,
digest_restart = crm_element_value(xml_op, XML_LRM_ATTR_RESTART_DIGEST);
crm_element_value_ms(xml_op, XML_LRM_ATTR_INTERVAL_MS, &interval_ms);
- key = pcmk__op_key(rsc->id, task, interval_ms);
- data = rsc_action_digest(rsc, task, key, node, xml_op,
+ data = rsc_action_digest(rsc, task, interval_ms, node, xml_op,
pcmk_is_set(data_set->flags, pe_flag_sanitized),
data_set);
data->rc = RSC_DIGEST_MATCH;
if (digest_restart && data->digest_restart_calc && strcmp(data->digest_restart_calc, digest_restart) != 0) {
- pe_rsc_info(rsc, "Parameters to %s on %s changed: was %s vs. now %s (restart:%s) %s",
- key, node->details->uname,
- crm_str(digest_restart), data->digest_restart_calc,
- op_version, crm_element_value(xml_op, XML_ATTR_TRANSITION_MAGIC));
+ pe_rsc_info(rsc, "Parameters to %ums-interval %s action for %s on %s "
+ "changed: hash was %s vs. now %s (restart:%s) %s",
+ interval_ms, task, rsc->id, node->details->uname,
+ crm_str(digest_restart), data->digest_restart_calc,
+ op_version,
+ crm_element_value(xml_op, XML_ATTR_TRANSITION_MAGIC));
data->rc = RSC_DIGEST_RESTART;
} else if (digest_all == NULL) {
@@ -386,15 +416,15 @@ rsc_action_digest_cmp(pe_resource_t * rsc, xmlNode * xml_op, pe_node_t * node,
data->rc = RSC_DIGEST_UNKNOWN;
} else if (strcmp(digest_all, data->digest_all_calc) != 0) {
- pe_rsc_info(rsc, "Parameters to %s on %s changed: was %s vs. now %s (%s:%s) %s",
- key, node->details->uname,
- crm_str(digest_all), data->digest_all_calc,
- (interval_ms > 0)? "reschedule" : "reload",
- op_version, crm_element_value(xml_op, XML_ATTR_TRANSITION_MAGIC));
+ pe_rsc_info(rsc, "Parameters to %ums-interval %s action for %s on %s "
+ "changed: hash was %s vs. now %s (%s:%s) %s",
+ interval_ms, task, rsc->id, node->details->uname,
+ crm_str(digest_all), data->digest_all_calc,
+ (interval_ms > 0)? "reschedule" : "reload",
+ op_version,
+ crm_element_value(xml_op, XML_ATTR_TRANSITION_MAGIC));
data->rc = RSC_DIGEST_ALL;
}
-
- free(key);
return data;
}
@@ -483,12 +513,9 @@ pe__compare_fencing_digest(pe_resource_t *rsc, const char *agent,
const char *node_summary = NULL;
// Calculate device's current parameter digests
- char *key = pcmk__op_key(rsc->id, STONITH_DIGEST_TASK, 0);
- op_digest_cache_t *data = rsc_action_digest(rsc, STONITH_DIGEST_TASK, key,
+ op_digest_cache_t *data = rsc_action_digest(rsc, STONITH_DIGEST_TASK, 0U,
node, NULL, TRUE, data_set);
- free(key);
-
// Check whether node has special unfencing summary node attribute
node_summary = pe_node_attribute_raw(node, CRM_ATTR_DIGESTS_ALL);
if (node_summary == NULL) {
--
1.8.3.1
From bf20b166b6e7dcf87edd398f2edfc384cb640886 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Mon, 23 Nov 2020 17:48:34 -0600
Subject: [PATCH 09/12] Low: scheduler: don't include timeout in secure digests
... to match what the controller does
---
lib/pengine/pe_digest.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/lib/pengine/pe_digest.c b/lib/pengine/pe_digest.c
index b608f22..f55c896 100644
--- a/lib/pengine/pe_digest.c
+++ b/lib/pengine/pe_digest.c
@@ -247,6 +247,17 @@ calculate_secure_digest(op_digest_cache_t *data, pe_resource_t *rsc,
NULL);
}
pcmk__filter_op_for_digest(data->params_secure);
+
+ /* CRM_meta_timeout *should* be part of a digest for recurring operations.
+ * However, currently the controller does not add timeout to secure digests,
+ * because it only includes parameters declared by the resource agent.
+ * Remove any timeout that made it this far, to match.
+ *
+ * @TODO Update the controller to add the timeout (which will require
+ * bumping the feature set and checking that here).
+ */
+ xml_remove_prop(data->params_secure, CRM_META "_" XML_ATTR_TIMEOUT);
+
data->digest_secure_calc = calculate_operation_digest(data->params_secure,
op_version);
}
--
1.8.3.1
From a956e32a536942b0fc1f2f058e441b3faf2abdd3 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 7 Jan 2021 12:08:56 -0600
Subject: [PATCH 10/12] Low: scheduler: treat NULL and empty string the same in
literal attribute comparisons
Previously, expand_value_source() returned NULL if the value was the empty
string ("") only when value-source was "param" or "meta". If value-source was
literal, it would return the empty string.
This behavior shouldn't depend on value-source, so it now returns NULL when a
literal value is the empty string.
This could change the behavior for "defined"/"not_defined" checks, and
comparisons against another NULL or empty string value (NULL compares less than
empty strings). But the consistency seems worth it.
(Another question not addressed here is whether NULL and empty string should
compare as equal.)
---
lib/pengine/rules.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/lib/pengine/rules.c b/lib/pengine/rules.c
index aa5d6ab..d69f449 100644
--- a/lib/pengine/rules.c
+++ b/lib/pengine/rules.c
@@ -1,5 +1,5 @@
/*
- * Copyright 2004-2020 the Pacemaker project contributors
+ * Copyright 2004-2021 the Pacemaker project contributors
*
* The version control history for this file may have further details.
*
@@ -1039,7 +1039,10 @@ expand_value_source(const char *value, const char *value_source,
{
GHashTable *table = NULL;
- if (pcmk__str_eq(value_source, "param", pcmk__str_casei)) {
+ if (pcmk__str_empty(value)) {
+ return NULL; // value_source is irrelevant
+
+ } else if (pcmk__str_eq(value_source, "param", pcmk__str_casei)) {
table = match_data->params;
} else if (pcmk__str_eq(value_source, "meta", pcmk__str_casei)) {
@@ -1049,7 +1052,7 @@ expand_value_source(const char *value, const char *value_source,
return value;
}
- if ((table == NULL) || pcmk__str_empty(value)) {
+ if (table == NULL) {
return NULL;
}
return (const char *) g_hash_table_lookup(table, value);
--
1.8.3.1
From 2d7cd78340b47045b5987002c9b2d221ccb01ee9 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 7 Jan 2021 13:01:29 -0600
Subject: [PATCH 11/12] Refactor: libcrmcommon: bail early if ACLs block
attribute removal
... for efficiency and fewer trace messages
---
lib/common/xml.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/lib/common/xml.c b/lib/common/xml.c
index 869ed51..8b71911 100644
--- a/lib/common/xml.c
+++ b/lib/common/xml.c
@@ -1,5 +1,5 @@
/*
- * Copyright 2004-2020 the Pacemaker project contributors
+ * Copyright 2004-2021 the Pacemaker project contributors
*
* The version control history for this file may have further details.
*
@@ -637,10 +637,13 @@ pcmk__xe_remove_matching_attrs(xmlNode *element,
next = a->next; // Grab now because attribute might get removed
if ((match == NULL) || match(a, user_data)) {
if (!pcmk__check_acl(element, NULL, xpf_acl_write)) {
- crm_trace("ACLs prevent removal of %s attribute from %s element",
+ crm_trace("ACLs prevent removal of attributes (%s and "
+ "possibly others) from %s element",
(const char *) a->name, (const char *) element->name);
+ return; // ACLs apply to element, not particular attributes
+ }
- } else if (pcmk__tracking_xml_changes(element, false)) {
+ if (pcmk__tracking_xml_changes(element, false)) {
// Leave (marked for removal) until after diff is calculated
set_parent_flag(element, xpf_dirty);
pcmk__set_xml_flags((xml_private_t *) a->_private, xpf_deleted);
--
1.8.3.1
From 459f7a58424b05b7c586906d904129a6408d6206 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 7 Jan 2021 13:30:40 -0600
Subject: [PATCH 12/12] Refactor: libcrmcommon: drop a constant
It was only used once, and the code is actually more readable without it
---
lib/common/operations.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/lib/common/operations.c b/lib/common/operations.c
index 421aaac..420f078 100644
--- a/lib/common/operations.c
+++ b/lib/common/operations.c
@@ -1,5 +1,5 @@
/*
- * Copyright 2004-2020 the Pacemaker project contributors
+ * Copyright 2004-2021 the Pacemaker project contributors
*
* The version control history for this file may have further details.
*
@@ -362,22 +362,22 @@ decode_transition_key(const char *key, char **uuid, int *transition_id, int *act
return TRUE;
}
-// String length of CRM_META"_"
-#define CRM_META_LEN sizeof(CRM_META)
-
// Return true if a is an attribute that should be filtered
static bool
should_filter_for_digest(xmlAttrPtr a, void *user_data)
{
- return (strncmp((const char *) a->name, CRM_META "_", CRM_META_LEN) == 0)
- || pcmk__str_any_of((const char *) a->name,
- XML_ATTR_ID,
- XML_ATTR_CRM_VERSION,
- XML_LRM_ATTR_OP_DIGEST,
- XML_LRM_ATTR_TARGET,
- XML_LRM_ATTR_TARGET_UUID,
- "pcmk_external_ip",
- NULL);
+ if (strncmp((const char *) a->name, CRM_META "_",
+ sizeof(CRM_META " ") - 1) == 0) {
+ return true;
+ }
+ return pcmk__str_any_of((const char *) a->name,
+ XML_ATTR_ID,
+ XML_ATTR_CRM_VERSION,
+ XML_LRM_ATTR_OP_DIGEST,
+ XML_LRM_ATTR_TARGET,
+ XML_LRM_ATTR_TARGET_UUID,
+ "pcmk_external_ip",
+ NULL);
}
/*!
--
1.8.3.1