From df587aaec07b4a08364d4024b3d0c73e6dede562 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Mon, 9 Nov 2020 14:55:42 -0600
Subject: [PATCH 1/9] Refactor: scheduler: simplify XML attribute filtering
function
---
lib/pengine/pe_digest.c | 52 ++++++++++++++++++++++++++-----------------------
1 file changed, 28 insertions(+), 24 deletions(-)
diff --git a/lib/pengine/pe_digest.c b/lib/pengine/pe_digest.c
index 5bcd22b..1e119f9 100644
--- a/lib/pengine/pe_digest.c
+++ b/lib/pengine/pe_digest.c
@@ -45,30 +45,38 @@ 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)
+filter_parameters(xmlNode *param_set, const char *param_string,
+ bool need_present)
{
- if (param_set && param_string) {
- xmlAttrPtr xIter = param_set->properties;
-
- while (xIter) {
- const char *prop_name = (const char *)xIter->name;
- char *name = crm_strdup_printf(" %s ", prop_name);
- char *match = strstr(param_string, name);
+ 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);
+ free(name);
- // Do now, because current entry might get removed below
- xIter = xIter->next;
+ // Do now, because current entry might get removed below
+ xIter = xIter->next;
- if (need_present && match == NULL) {
- crm_trace("%s not found in %s", prop_name, param_string);
- xml_remove_prop(param_set, prop_name);
+ if ((need_present && (match == NULL))
+ || (!need_present && (match != NULL))) {
- } else if (need_present == FALSE && match) {
- crm_trace("%s found in %s", prop_name, param_string);
- xml_remove_prop(param_set, prop_name);
- }
+ crm_trace("Filtering %s (%sfound in '%s')",
+ prop_name, (need_present? "not " : ""), param_string);
+ xml_remove_prop(param_set, prop_name);
}
}
}
@@ -203,9 +211,7 @@ calculate_secure_digest(op_digest_cache_t *data, pe_resource_t *rsc,
g_hash_table_foreach(overrides, hash2field, data->params_secure);
}
g_hash_table_foreach(rsc->parameters, hash2field, data->params_secure);
- if (secure_list != NULL) {
- filter_parameters(data->params_secure, secure_list, FALSE);
- }
+ filter_parameters(data->params_secure, secure_list, FALSE);
if (pcmk_is_set(pcmk_get_ra_caps(class),
pcmk_ra_cap_fence_params)) {
/* For stonith resources, Pacemaker adds special parameters,
@@ -259,9 +265,7 @@ 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);
- if (value != NULL) {
- filter_parameters(data->params_restart, value, TRUE);
- }
+ filter_parameters(data->params_restart, value, TRUE);
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 f030af8771601d46947ac9276538c46c6c296504 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Mon, 9 Nov 2020 18:37:37 -0600
Subject: [PATCH 2/9] Refactor: scheduler: remember whether action is probe
when unpacking
... to reduce code duplication and improve readability
---
lib/pengine/utils.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lib/pengine/utils.c b/lib/pengine/utils.c
index 04110be..b0922fa 100644
--- a/lib/pengine/utils.c
+++ b/lib/pengine/utils.c
@@ -995,6 +995,8 @@ unpack_operation(pe_action_t * action, xmlNode * xml_obj, pe_resource_t * contai
{
int timeout_ms = 0;
const char *value = NULL;
+ bool is_probe = pcmk__str_eq(action->task, RSC_STATUS, pcmk__str_casei)
+ && (interval_ms == 0);
#if ENABLE_VERSIONED_ATTRS
pe_rsc_action_details_t *rsc_details = NULL;
#endif
@@ -1026,8 +1028,7 @@ unpack_operation(pe_action_t * action, xmlNode * xml_obj, pe_resource_t * contai
action->meta, NULL, FALSE, data_set);
// Determine probe default timeout differently
- if (pcmk__str_eq(action->task, RSC_STATUS, pcmk__str_casei)
- && (interval_ms == 0)) {
+ if (is_probe) {
xmlNode *min_interval_mon = find_min_interval_mon(action->rsc, FALSE);
if (min_interval_mon) {
@@ -1099,8 +1100,7 @@ unpack_operation(pe_action_t * action, xmlNode * xml_obj, pe_resource_t * contai
if (pcmk_is_set(pcmk_get_ra_caps(rsc_rule_data.standard),
pcmk_ra_cap_fence_params)
&& (pcmk__str_eq(action->task, RSC_START, pcmk__str_casei)
- || (pcmk__str_eq(action->task, RSC_STATUS, pcmk__str_casei)
- && (interval_ms == 0)))
+ || is_probe)
&& action->rsc->parameters) {
value = g_hash_table_lookup(action->rsc->parameters,
--
1.8.3.1
From 9de547849697cd6a3581db3f83b04f68d0405d9d Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Tue, 10 Nov 2020 15:15:01 -0600
Subject: [PATCH 3/9] Refactor: scheduler: don't include originally unpacked
resource parameters in digest
Previously, when calculating an operation digest, calculate_main_digest() would
grab the following, in order of highest to lowest precedence:
* instance attributes evaluated for the appropriate node
* instance attributes specified with the operation
* instance attributes as originally unpacked (without evaluating for any node)
* resource meta-attributes
Adding the originally unpacked instance attributes was redundant, since
node-evaluated instance attributes would always be a superset of those and
would take precedence.
---
lib/pengine/pe_digest.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/lib/pengine/pe_digest.c b/lib/pengine/pe_digest.c
index 1e119f9..dd6b753 100644
--- a/lib/pengine/pe_digest.c
+++ b/lib/pengine/pe_digest.c
@@ -162,7 +162,6 @@ calculate_main_digest(op_digest_cache_t *data, pe_resource_t *rsc,
}
g_hash_table_foreach(local_rsc_params, hash2field, data->params_all);
g_hash_table_foreach(action->extra, hash2field, data->params_all);
- g_hash_table_foreach(rsc->parameters, hash2field, data->params_all);
g_hash_table_foreach(action->meta, hash2metafield, data->params_all);
#if ENABLE_VERSIONED_ATTRS
--
1.8.3.1
From e9921b2ab3e9eeab6227d97cc12b85fa04dfd187 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 13 Nov 2020 09:27:12 -0600
Subject: [PATCH 4/9] Refactor: scheduler: reuse existing function to check for
remote in XML
... to reduce code duplication and improve readability
---
lib/pengine/bundle.c | 21 +++------------------
1 file changed, 3 insertions(+), 18 deletions(-)
diff --git a/lib/pengine/bundle.c b/lib/pengine/bundle.c
index 76730c7..4f6eac3 100644
--- a/lib/pengine/bundle.c
+++ b/lib/pengine/bundle.c
@@ -959,28 +959,13 @@ pe__bundle_needs_remote_name(pe_resource_t *rsc)
const char *value;
if (rsc == NULL) {
- return FALSE;
+ return false;
}
value = g_hash_table_lookup(rsc->parameters, XML_RSC_ATTR_REMOTE_RA_ADDR);
- if (!pcmk__str_eq(value, "#uname", pcmk__str_casei)) {
- return FALSE;
- } else {
- const char *match[3][2] = {
- { XML_ATTR_TYPE, "remote" },
- { XML_AGENT_ATTR_CLASS, PCMK_RESOURCE_CLASS_OCF },
- { XML_AGENT_ATTR_PROVIDER, "pacemaker" },
- };
-
- for (int m = 0; m < 3; m++) {
- value = crm_element_value(rsc->xml, match[m][0]);
- if (!pcmk__str_eq(value, match[m][1], pcmk__str_casei)) {
- return FALSE;
- }
- }
- }
- return TRUE;
+ return pcmk__str_eq(value, "#uname", pcmk__str_casei)
+ && xml_contains_remote_node(rsc->xml);
}
const char *
--
1.8.3.1
From 0f220e1cd25ec095492ac4b346452520f4b71cf1 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Wed, 11 Nov 2020 15:29:46 -0600
Subject: [PATCH 5/9] Refactor: scheduler: remove dead code
Several internal expression-testing functions were unused. While they might
have had some potential future value, the abundance of similarly named
functions made debugging difficult. Comment blocks were added to the functions
they wrapped, which should add similar value.
Also, we had an internal wrapper for pe__eval_date_expr() that was used
only in crm_rule, so it was moved there.
---
include/crm/pengine/rules_internal.h | 10 +--
lib/pengine/rules.c | 130 +++++++++++------------------------
tools/crm_rule.c | 27 +++++++-
3 files changed, 66 insertions(+), 101 deletions(-)
diff --git a/include/crm/pengine/rules_internal.h b/include/crm/pengine/rules_internal.h
index f60263a..7380826 100644
--- a/include/crm/pengine/rules_internal.h
+++ b/include/crm/pengine/rules_internal.h
@@ -1,5 +1,5 @@
/*
- * Copyright 2015-2019 the Pacemaker project contributors
+ * Copyright 2015-2020 the Pacemaker project contributors
*
* The version control history for this file may have further details.
*
@@ -28,14 +28,6 @@ gboolean pe__eval_op_expr(xmlNodePtr expr, pe_rule_eval_data_t *rule_data);
gboolean pe__eval_role_expr(xmlNode *expr, pe_rule_eval_data_t *rule_data);
gboolean pe__eval_rsc_expr(xmlNodePtr expr, pe_rule_eval_data_t *rule_data);
-int pe_eval_date_expression(xmlNode *time_expr,
- crm_time_t *now,
- crm_time_t *next_change);
-gboolean pe_test_date_expression(xmlNode *time_expr, crm_time_t *now,
- crm_time_t *next_change);
int pe_cron_range_satisfied(crm_time_t * now, xmlNode * cron_spec);
-gboolean pe_test_attr_expression(xmlNode *expr, GHashTable *hash, crm_time_t *now,
- pe_match_data_t *match_data);
-gboolean pe_test_role_expression(xmlNode * expr, enum rsc_role_e role, crm_time_t * now);
#endif
diff --git a/lib/pengine/rules.c b/lib/pengine/rules.c
index 28562aa..be30e67 100644
--- a/lib/pengine/rules.c
+++ b/lib/pengine/rules.c
@@ -140,37 +140,6 @@ find_expression_type(xmlNode * expr)
return attr_expr;
}
-gboolean
-pe_test_role_expression(xmlNode *expr, enum rsc_role_e role, crm_time_t *now)
-{
- pe_rule_eval_data_t rule_data = {
- .node_hash = NULL,
- .role = role,
- .now = now,
- .match_data = NULL,
- .rsc_data = NULL,
- .op_data = NULL
- };
-
- return pe__eval_role_expr(expr, &rule_data);
-}
-
-gboolean
-pe_test_attr_expression(xmlNode *expr, GHashTable *hash, crm_time_t *now,
- pe_match_data_t *match_data)
-{
- pe_rule_eval_data_t rule_data = {
- .node_hash = hash,
- .role = RSC_ROLE_UNKNOWN,
- .now = now,
- .match_data = match_data,
- .rsc_data = NULL,
- .op_data = NULL
- };
-
- return pe__eval_attr_expr(expr, &rule_data);
-}
-
/* As per the nethack rules:
*
* moon period = 29.53058 days ~= 30, year = 365.2422 days
@@ -331,38 +300,6 @@ pe_parse_xml_duration(crm_time_t * start, xmlNode * duration_spec)
return end;
}
-/*!
- * \internal
- * \brief Test a date expression (pass/fail) for a specific time
- *
- * \param[in] time_expr date_expression XML
- * \param[in] now Time for which to evaluate expression
- * \param[out] next_change If not NULL, set to when evaluation will change
- *
- * \return TRUE if date expression is in effect at given time, FALSE otherwise
- */
-gboolean
-pe_test_date_expression(xmlNode *expr, crm_time_t *now, crm_time_t *next_change)
-{
- pe_rule_eval_data_t rule_data = {
- .node_hash = NULL,
- .role = RSC_ROLE_UNKNOWN,
- .now = now,
- .match_data = NULL,
- .rsc_data = NULL,
- .op_data = NULL
- };
-
- switch (pe__eval_date_expr(expr, &rule_data, next_change)) {
- case pcmk_rc_within_range:
- case pcmk_rc_ok:
- return TRUE;
-
- default:
- return FALSE;
- }
-}
-
// Set next_change to t if t is earlier
static void
crm_time_set_if_earlier(crm_time_t *next_change, crm_time_t *t)
@@ -375,31 +312,6 @@ crm_time_set_if_earlier(crm_time_t *next_change, crm_time_t *t)
}
}
-/*!
- * \internal
- * \brief Evaluate a date expression for a specific time
- *
- * \param[in] time_expr date_expression XML
- * \param[in] now Time for which to evaluate expression
- * \param[out] next_change If not NULL, set to when evaluation will change
- *
- * \return Standard Pacemaker return code
- */
-int
-pe_eval_date_expression(xmlNode *expr, crm_time_t *now, crm_time_t *next_change)
-{
- pe_rule_eval_data_t rule_data = {
- .node_hash = NULL,
- .role = RSC_ROLE_UNKNOWN,
- .now = now,
- .match_data = NULL,
- .rsc_data = NULL,
- .op_data = NULL
- };
-
- return pe__eval_date_expr(expr, &rule_data, next_change);
-}
-
// Information about a block of nvpair elements
typedef struct sorted_set_s {
int score; // This block's score for sorting
@@ -908,7 +820,16 @@ pe_eval_subexpr(xmlNode *expr, pe_rule_eval_data_t *rule_data, crm_time_t *next_
break;
case time_expr:
- accept = pe_test_date_expression(expr, rule_data->now, next_change);
+ switch (pe__eval_date_expr(expr, rule_data, next_change)) {
+ case pcmk_rc_within_range:
+ case pcmk_rc_ok:
+ accept = TRUE;
+ break;
+
+ default:
+ accept = FALSE;
+ break;
+ }
break;
case role_expr:
@@ -1104,6 +1025,16 @@ accept_attr_expr(const char *l_val, const char *r_val, const char *type,
return false; // Should never reach this point
}
+/*!
+ * \internal
+ * \brief Evaluate a node attribute expression based on #uname, #id, #kind,
+ * or a generic node attribute
+ *
+ * \param[in] expr XML of rule expression
+ * \param[in] rule_data The match_data and node_hash members are used
+ *
+ * \return TRUE if rule_data satisfies the expression, FALSE otherwise
+ */
gboolean
pe__eval_attr_expr(xmlNodePtr expr, pe_rule_eval_data_t *rule_data)
{
@@ -1169,8 +1100,16 @@ pe__eval_attr_expr(xmlNodePtr expr, pe_rule_eval_data_t *rule_data)
return accept_attr_expr(h_val, value, type, op);
}
-
-
+/*!
+ * \internal
+ * \brief Evaluate a date_expression
+ *
+ * \param[in] expr XML of rule expression
+ * \param[in] rule_data Only the now member is used
+ * \param[out] next_change If not NULL, set to when evaluation will change
+ *
+ * \return Standard Pacemaker return code
+ */
int
pe__eval_date_expr(xmlNodePtr expr, pe_rule_eval_data_t *rule_data, crm_time_t *next_change)
{
@@ -1285,6 +1224,15 @@ pe__eval_op_expr(xmlNodePtr expr, pe_rule_eval_data_t *rule_data) {
return TRUE;
}
+/*!
+ * \internal
+ * \brief Evaluate a node attribute expression based on #role
+ *
+ * \param[in] expr XML of rule expression
+ * \param[in] rule_data Only the role member is used
+ *
+ * \return TRUE if rule_data->role satisfies the expression, FALSE otherwise
+ */
gboolean
pe__eval_role_expr(xmlNodePtr expr, pe_rule_eval_data_t *rule_data)
{
diff --git a/tools/crm_rule.c b/tools/crm_rule.c
index 0e44828..2871f3d 100644
--- a/tools/crm_rule.c
+++ b/tools/crm_rule.c
@@ -75,6 +75,31 @@ mode_cb(const gchar *option_name, const gchar *optarg, gpointer data, GError **e
return TRUE;
}
+/*!
+ * \internal
+ * \brief Evaluate a date expression for a specific time
+ *
+ * \param[in] time_expr date_expression XML
+ * \param[in] now Time for which to evaluate expression
+ * \param[out] next_change If not NULL, set to when evaluation will change
+ *
+ * \return Standard Pacemaker return code
+ */
+static int
+eval_date_expression(xmlNode *expr, crm_time_t *now, crm_time_t *next_change)
+{
+ pe_rule_eval_data_t rule_data = {
+ .node_hash = NULL,
+ .role = RSC_ROLE_UNKNOWN,
+ .now = now,
+ .match_data = NULL,
+ .rsc_data = NULL,
+ .op_data = NULL
+ };
+
+ return pe__eval_date_expr(expr, &rule_data, next_change);
+}
+
static int
crm_rule_check(pe_working_set_t *data_set, const char *rule_id, crm_time_t *effective_date)
{
@@ -156,7 +181,7 @@ crm_rule_check(pe_working_set_t *data_set, const char *rule_id, crm_time_t *effe
CRM_ASSERT(match != NULL);
CRM_ASSERT(find_expression_type(match) == time_expr);
- rc = pe_eval_date_expression(match, effective_date, NULL);
+ rc = eval_date_expression(match, effective_date, NULL);
if (rc == pcmk_rc_within_range) {
printf("Rule %s is still in effect\n", rule_id);
--
1.8.3.1
From 91a6ec4bec86de7389fcd64cb27e315da760f0dd Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Wed, 11 Nov 2020 16:44:57 -0600
Subject: [PATCH 6/9] Refactor: scheduler: make constraint unpacking function
static
... for linker efficiency and readability. Also change the return type to void
since it was ignored (and the same for some related functions).
---
include/pcmki/pcmki_sched_allocate.h | 4 +--
lib/pacemaker/pcmk_sched_constraints.c | 46 ++++++++++++++++------------------
2 files changed, 23 insertions(+), 27 deletions(-)
diff --git a/include/pcmki/pcmki_sched_allocate.h b/include/pcmki/pcmki_sched_allocate.h
index efc0da6..a7f8c11 100644
--- a/include/pcmki/pcmki_sched_allocate.h
+++ b/include/pcmki/pcmki_sched_allocate.h
@@ -1,5 +1,5 @@
/*
- * Copyright 2004-2019 the Pacemaker project contributors
+ * Copyright 2004-2020 the Pacemaker project contributors
*
* The version control history for this file may have further details.
*
@@ -143,8 +143,6 @@ extern gboolean unpack_rsc_order(xmlNode * xml_obj, pe_working_set_t * data_set)
extern gboolean unpack_rsc_colocation(xmlNode * xml_obj, pe_working_set_t * data_set);
-extern gboolean unpack_location(xmlNode * xml_obj, pe_working_set_t * data_set);
-
extern gboolean unpack_rsc_ticket(xmlNode * xml_obj, pe_working_set_t * data_set);
void LogNodeActions(pe_working_set_t * data_set, gboolean terminal);
diff --git a/lib/pacemaker/pcmk_sched_constraints.c b/lib/pacemaker/pcmk_sched_constraints.c
index 6ed2d8c..121754d 100644
--- a/lib/pacemaker/pcmk_sched_constraints.c
+++ b/lib/pacemaker/pcmk_sched_constraints.c
@@ -48,6 +48,7 @@ static pe__location_t *generate_location_rule(pe_resource_t *rsc,
crm_time_t *next_change,
pe_working_set_t *data_set,
pe_match_data_t *match_data);
+static void unpack_location(xmlNode *xml_obj, pe_working_set_t *data_set);
static bool
evaluate_lifetime(xmlNode *lifetime, pe_working_set_t *data_set)
@@ -709,11 +710,13 @@ tag_to_set(xmlNode * xml_obj, xmlNode ** rsc_set, const char * attr,
return TRUE;
}
-static gboolean unpack_rsc_location(xmlNode * xml_obj, pe_resource_t * rsc_lh, const char * role,
- const char * score, pe_working_set_t * data_set, pe_match_data_t * match_data);
+static void unpack_rsc_location(xmlNode *xml_obj, pe_resource_t *rsc_lh,
+ const char *role, const char *score,
+ pe_working_set_t *data_set,
+ pe_match_data_t *match_data);
-static gboolean
-unpack_simple_location(xmlNode * xml_obj, pe_working_set_t * data_set)
+static void
+unpack_simple_location(xmlNode *xml_obj, pe_working_set_t *data_set)
{
const char *id = crm_element_value(xml_obj, XML_ATTR_ID);
const char *value = crm_element_value(xml_obj, XML_LOC_ATTR_SOURCE);
@@ -721,7 +724,7 @@ unpack_simple_location(xmlNode * xml_obj, pe_working_set_t * data_set)
if(value) {
pe_resource_t *rsc_lh = pe_find_constraint_resource(data_set->resources, value);
- return unpack_rsc_location(xml_obj, rsc_lh, NULL, NULL, data_set, NULL);
+ unpack_rsc_location(xml_obj, rsc_lh, NULL, NULL, data_set, NULL);
}
value = crm_element_value(xml_obj, XML_LOC_ATTR_SOURCE_PATTERN);
@@ -741,7 +744,7 @@ unpack_simple_location(xmlNode * xml_obj, pe_working_set_t * data_set)
" has invalid value '%s'", id, value);
regfree(r_patt);
free(r_patt);
- return FALSE;
+ return;
}
for (rIter = data_set->resources; rIter; rIter = rIter->next) {
@@ -787,13 +790,12 @@ unpack_simple_location(xmlNode * xml_obj, pe_working_set_t * data_set)
regfree(r_patt);
free(r_patt);
}
-
- return FALSE;
}
-static gboolean
-unpack_rsc_location(xmlNode * xml_obj, pe_resource_t * rsc_lh, const char * role,
- const char * score, pe_working_set_t * data_set, pe_match_data_t * match_data)
+static void
+unpack_rsc_location(xmlNode *xml_obj, pe_resource_t *rsc_lh, const char *role,
+ const char *score, pe_working_set_t *data_set,
+ pe_match_data_t *match_data)
{
pe__location_t *location = NULL;
const char *id_lh = crm_element_value(xml_obj, XML_LOC_ATTR_SOURCE);
@@ -804,7 +806,7 @@ unpack_rsc_location(xmlNode * xml_obj, pe_resource_t * rsc_lh, const char * role
if (rsc_lh == NULL) {
pcmk__config_warn("Ignoring constraint '%s' because resource '%s' "
"does not exist", id, id_lh);
- return FALSE;
+ return;
}
if (score == NULL) {
@@ -816,7 +818,7 @@ unpack_rsc_location(xmlNode * xml_obj, pe_resource_t * rsc_lh, const char * role
pe_node_t *match = pe_find_node(data_set->nodes, node);
if (!match) {
- return FALSE;
+ return;
}
location = rsc2node_new(id, rsc_lh, score_i, discovery, match, data_set);
@@ -850,7 +852,7 @@ unpack_rsc_location(xmlNode * xml_obj, pe_resource_t * rsc_lh, const char * role
pe__update_recheck_time(t, data_set);
}
crm_time_free(next_change);
- return TRUE;
+ return;
}
if (role == NULL) {
@@ -860,7 +862,7 @@ unpack_rsc_location(xmlNode * xml_obj, pe_resource_t * rsc_lh, const char * role
if (location && role) {
if (text2role(role) == RSC_ROLE_UNKNOWN) {
pe_err("Invalid constraint %s: Bad role %s", id, role);
- return FALSE;
+ return;
} else {
enum rsc_role_e r = text2role(role);
@@ -877,8 +879,6 @@ unpack_rsc_location(xmlNode * xml_obj, pe_resource_t * rsc_lh, const char * role
}
}
}
-
- return TRUE;
}
static gboolean
@@ -992,8 +992,8 @@ unpack_location_set(xmlNode * location, xmlNode * set, pe_working_set_t * data_s
return TRUE;
}
-gboolean
-unpack_location(xmlNode * xml_obj, pe_working_set_t * data_set)
+static void
+unpack_location(xmlNode *xml_obj, pe_working_set_t *data_set)
{
xmlNode *set = NULL;
gboolean any_sets = FALSE;
@@ -1002,7 +1002,7 @@ unpack_location(xmlNode * xml_obj, pe_working_set_t * data_set)
xmlNode *expanded_xml = NULL;
if (unpack_location_tags(xml_obj, &expanded_xml, data_set) == FALSE) {
- return FALSE;
+ return;
}
if (expanded_xml) {
@@ -1020,7 +1020,7 @@ unpack_location(xmlNode * xml_obj, pe_working_set_t * data_set)
if (expanded_xml) {
free_xml(expanded_xml);
}
- return FALSE;
+ return;
}
}
}
@@ -1031,10 +1031,8 @@ unpack_location(xmlNode * xml_obj, pe_working_set_t * data_set)
}
if (any_sets == FALSE) {
- return unpack_simple_location(xml_obj, data_set);
+ unpack_simple_location(xml_obj, data_set);
}
-
- return TRUE;
}
static int
--
1.8.3.1
From df842adafca8ba56ddb5b448490ffef54ea785d4 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 13 Nov 2020 09:19:51 -0600
Subject: [PATCH 7/9] Refactor: scheduler: trivial refactoring of nvpair
evaluation
... for readability
---
lib/pengine/rules.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/lib/pengine/rules.c b/lib/pengine/rules.c
index be30e67..86e7899 100644
--- a/lib/pengine/rules.c
+++ b/lib/pengine/rules.c
@@ -529,20 +529,19 @@ static GList *
make_pairs(xmlNode *top, xmlNode *xml_obj, const char *set_name,
const char *always_first)
{
- GListPtr unsorted = NULL;
- const char *score = NULL;
- sorted_set_t *pair = NULL;
- xmlNode *attr_set = NULL;
+ GList *unsorted = NULL;
if (xml_obj == NULL) {
return NULL;
}
- for (attr_set = pcmk__xe_first_child(xml_obj); attr_set != NULL;
+ for (xmlNode *attr_set = pcmk__xe_first_child(xml_obj); attr_set != NULL;
attr_set = pcmk__xe_next(attr_set)) {
- /* Uncertain if set_name == NULL check is strictly necessary here */
- if (pcmk__str_eq(set_name, (const char *)attr_set->name, pcmk__str_null_matches)) {
- pair = NULL;
+ if (pcmk__str_eq(set_name, (const char *) attr_set->name,
+ pcmk__str_null_matches)) {
+ const char *score = NULL;
+ sorted_set_t *pair = NULL;
+
attr_set = expand_idref(attr_set, top);
if (attr_set == NULL) {
continue;
--
1.8.3.1
From 373c224ac1c41bd47a928a7523744c6c678a6543 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 13 Nov 2020 09:21:35 -0600
Subject: [PATCH 8/9] Low: scheduler: correctly skip dangling id-ref
When evaluating XML nvpair blocks, make_pairs() previously reused the
"for" loop variable when expanding id-ref's. However if the id-ref was dangling
(only possible if schema enforcement is turned off), this would make it NULL
and thus exiting the loop instead of continuing.
---
lib/pengine/rules.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/lib/pengine/rules.c b/lib/pengine/rules.c
index 86e7899..1bd807f 100644
--- a/lib/pengine/rules.c
+++ b/lib/pengine/rules.c
@@ -541,18 +541,19 @@ make_pairs(xmlNode *top, xmlNode *xml_obj, const char *set_name,
pcmk__str_null_matches)) {
const char *score = NULL;
sorted_set_t *pair = NULL;
+ xmlNode *expanded_attr_set = expand_idref(attr_set, top);
- attr_set = expand_idref(attr_set, top);
- if (attr_set == NULL) {
+ if (expanded_attr_set == NULL) {
+ // Schema (if not "none") prevents this
continue;
}
pair = calloc(1, sizeof(sorted_set_t));
- pair->name = ID(attr_set);
+ pair->name = ID(expanded_attr_set);
pair->special_name = always_first;
- pair->attr_set = attr_set;
+ pair->attr_set = expanded_attr_set;
- score = crm_element_value(attr_set, XML_RULE_ATTR_SCORE);
+ score = crm_element_value(expanded_attr_set, XML_RULE_ATTR_SCORE);
pair->score = char2score(score);
unsorted = g_list_prepend(unsorted, pair);
--
1.8.3.1
From b59717751c168ec745bedbcc5696bee11036d931 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 12 Nov 2020 14:37:27 -0600
Subject: [PATCH 9/9] Low: scheduler: treat missing parameter as NULL in rules
with value-source
Previously, if value-source were set to "param" or "meta", and an affected
resource did not have the specified parameter, the parameter name would wrongly
be used as the value to compare against. Now, use NULL as the value to compare
against.
---
lib/pengine/rules.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/lib/pengine/rules.c b/lib/pengine/rules.c
index 1bd807f..e5d452f 100644
--- a/lib/pengine/rules.c
+++ b/lib/pengine/rules.c
@@ -1041,6 +1041,7 @@ 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;
@@ -1071,18 +1072,22 @@ pe__eval_attr_expr(xmlNodePtr expr, pe_rule_eval_data_t *rule_data)
}
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 (table) {
+ if (!literal) {
const char *param_name = value;
const char *param_value = NULL;
- if (param_name && param_name[0]) {
- if ((param_value = (const char *)g_hash_table_lookup(table, param_name))) {
+ 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;
}
}
--
1.8.3.1