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;