From 9e4addbcb67ea8e36ba853f1e401d8a6cb6a0aa3 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Fri, 20 Dec 2019 11:34:06 -0600

Subject: [PATCH 1/8] Refactor: scheduler: reduce code duplication when

 displaying resources

Refactor native_output_string() to use GString, for readability and

maintainability. Refactor common_print() to use it, to reduce duplication and

ensure displays are consistent.

This makes a couple small changes in how things are shown:

* If pe_print_dev is enabled (a debugging flag not actually used by anything),

  the additional resource fields are shown with the resource flags rather than

  their own parenthesized list.

* The new output model is now consistent with the legacy print model in

  displaying resource flags with commas (not spaces) between them.

---

 include/crm/pengine/common.h |  24 +--

 lib/pengine/native.c         | 410 +++++++++++++++++--------------------------

 2 files changed, 168 insertions(+), 266 deletions(-)

diff --git a/include/crm/pengine/common.h b/include/crm/pengine/common.h

index e497f9c..48c2b66 100644

--- a/include/crm/pengine/common.h

+++ b/include/crm/pengine/common.h

@@ -1,22 +1,12 @@

-/* 

- * Copyright 2004-2018 the Pacemaker project contributors

+/*

+ * Copyright 2004-2019 the Pacemaker project contributors

  *

  * The version control history for this file may have further details.

- * 

- * This program is free software; you can redistribute it and/or

- * modify it under the terms of the GNU Lesser General Public

- * License as published by the Free Software Foundation; either

- * version 2 of the License, or (at your option) any later version.

- * 

- * This software is distributed in the hope that it will be useful,

- * but WITHOUT ANY WARRANTY; without even the implied warranty of

- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU

- * General Public License for more details.

- * 

- * You should have received a copy of the GNU Lesser General Public

- * License along with this library; if not, write to the Free Software

- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA

+ *

+ * This source code is licensed under the GNU Lesser General Public License

+ * version 2.1 or later (LGPLv2.1+) WITHOUT ANY WARRANTY.

  */

+

 #ifndef PE_COMMON__H

 #  define PE_COMMON__H

@@ -104,7 +94,7 @@ enum pe_print_options {

     pe_print_html           = 0x0002,

     pe_print_ncurses        = 0x0004,

     pe_print_printf         = 0x0008,

-    pe_print_dev            = 0x0010,

+    pe_print_dev            = 0x0010, // Debugging (@COMPAT probably not useful)

     pe_print_details        = 0x0020,

     pe_print_max_details    = 0x0040,

     pe_print_rsconly        = 0x0080,

diff --git a/lib/pengine/native.c b/lib/pengine/native.c

index fdb98e0..8fd98bc 100644

--- a/lib/pengine/native.c

+++ b/lib/pengine/native.c

@@ -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.

  *

@@ -490,165 +490,172 @@ native_print_xml(resource_t * rsc, const char *pre_text, long options, void *pri

     }

 }

-/* making this inline rather than a macro prevents a coverity "unreachable"

- * warning on the first usage

- */

-static inline const char *

-comma_if(int i)

+// Append a flag to resource description string's flags list

+static bool

+add_output_flag(GString *s, const char *flag_desc, bool have_flags)

 {

-    return i? ", " : "";

+    g_string_append(s, (have_flags? ", " : " ("));

+    g_string_append(s, flag_desc);

+    return true;

 }

-static char *

-flags_string(pe_resource_t *rsc, pe_node_t *node, long options,

-             const char *target_role)

+// Append a node name to resource description string's node list

+static bool

+add_output_node(GString *s, const char *node, bool have_nodes)

 {

-    char *flags[6] = { NULL, };

-    char *result = NULL;

-    int ndx = 0;

+    g_string_append(s, (have_nodes? " " : " [ "));

+    g_string_append(s, node);

+    return true;

+}

+

+/*!

+ * \internal

+ * \brief Create a string description of a resource

+ *

+ * \param[in] rsc          Resource to describe

+ * \param[in] name         Desired identifier for the resource

+ * \param[in] node         If not NULL, node that resource is "on"

+ * \param[in] options      Bitmask of pe_print_*

+ * \param[in] target_role  Resource's target role

+ * \param[in] show_nodes   Whether to display nodes when multiply active

+ *

+ * \return Newly allocated string description of resource

+ * \note Caller must free the result with g_free().

+ */

+static gchar *

+native_output_string(pe_resource_t *rsc, const char *name, pe_node_t *node,

+                     long options, const char *target_role, bool show_nodes)

+{

+    const char *class = crm_element_value(rsc->xml, XML_AGENT_ATTR_CLASS);

+    const char *provider = NULL;

+    const char *kind = crm_element_value(rsc->xml, XML_ATTR_TYPE);

+    char *retval = NULL;

+    GString *outstr = NULL;

+    bool have_flags = false;

+

+    CRM_CHECK(name != NULL, name = "unknown");

+    CRM_CHECK(kind != NULL, kind = "unknown");

+    CRM_CHECK(class != NULL, class = "unknown");

+

+    if (is_set(pcmk_get_ra_caps(class), pcmk_ra_cap_provider)) {

+        provider = crm_element_value(rsc->xml, XML_AGENT_ATTR_PROVIDER);

+    }

-    if (node && node->details->online == FALSE && node->details->unclean) {

-        flags[ndx++] = strdup("UNCLEAN");

+    if (is_set(options, pe_print_rsconly)

+        || pcmk__list_of_multiple(rsc->running_on)) {

+        node = NULL;

     }

+    // We need a string of at least this size

+    outstr = g_string_sized_new(strlen(name) + strlen(class) + strlen(kind)

+                                + (provider? (strlen(provider) + 2) : 0)

+                                + (node? strlen(node->details->uname) + 1 : 0)

+                                + 11);

+

+    // Resource name and agent

+    g_string_printf(outstr, "%s\t(%s%s%s:%s):\t", name, class,

+                    /* @COMPAT This should be a single ':' (see CLBZ#5395) but

+                     * to avoid breaking anything relying on it, we're keeping

+                     * it like this until the next minor version bump.

+                     */

+                    (provider? "::" : ""), (provider? provider : ""), kind);

+

+    // State on node

+    if (is_set(rsc->flags, pe_rsc_orphan)) {

+        g_string_append(outstr, " ORPHANED");

+    }

+    if (is_set(rsc->flags, pe_rsc_failed)) {

+        enum rsc_role_e role = native_displayable_role(rsc);

+

+        if (role > RSC_ROLE_SLAVE) {

+            g_string_append_printf(outstr, " FAILED %s", role2text(role));

+        } else {

+            g_string_append(outstr, " FAILED");

+        }

+    } else {

+        g_string_append(outstr, native_displayable_state(rsc, options));

+    }

+    if (node) {

+        g_string_append_printf(outstr, " %s", node->details->uname);

+    }

+

+    // Flags, as: (<flag> [...])

+    if (node && !(node->details->online) && node->details->unclean) {

+        have_flags = add_output_flag(outstr, "UNCLEAN", have_flags);

+    }

     if (is_set(options, pe_print_pending)) {

         const char *pending_task = native_pending_task(rsc);

         if (pending_task) {

-            flags[ndx++] = strdup(pending_task);

+            have_flags = add_output_flag(outstr, pending_task, have_flags);

         }

     }

-

     if (target_role) {

         enum rsc_role_e target_role_e = text2role(target_role);

-        /* Ignore target role Started, as it is the default anyways

-         * (and would also allow a Master to be Master).

-         * Show if target role limits our abilities. */

+        /* Only show target role if it limits our abilities (i.e. ignore

+         * Started, as it is the default anyways, and doesn't prevent the

+         * resource from becoming Master).

+         */

         if (target_role_e == RSC_ROLE_STOPPED) {

-            flags[ndx++] = strdup("disabled");

+            have_flags = add_output_flag(outstr, "disabled", have_flags);

         } else if (is_set(uber_parent(rsc)->flags, pe_rsc_promotable)

                    && target_role_e == RSC_ROLE_SLAVE) {

-            flags[ndx++] = crm_strdup_printf("target-role:%s", target_role);

+            have_flags = add_output_flag(outstr, "target-role:", have_flags);

+            g_string_append(outstr, target_role);

         }

     }

-

     if (is_set(rsc->flags, pe_rsc_block)) {

-        flags[ndx++] = strdup("blocked");

-

+        have_flags = add_output_flag(outstr, "blocked", have_flags);

     } else if (is_not_set(rsc->flags, pe_rsc_managed)) {

-        flags[ndx++] = strdup("unmanaged");

+        have_flags = add_output_flag(outstr, "unmanaged", have_flags);

     }

-

     if (is_set(rsc->flags, pe_rsc_failure_ignored)) {

-        flags[ndx++] = strdup("failure ignored");

+        have_flags = add_output_flag(outstr, "failure ignored", have_flags);

     }

-

-    if (ndx > 0) {

-        char *total = g_strjoinv(" ", flags);

-

-        result = crm_strdup_printf(" (%s)", total);

-        g_free(total);

-    }

-

-    while (--ndx >= 0) {

-        free(flags[ndx]);

-    }

-    return result;

-}

-

-static char *

-native_output_string(resource_t *rsc, const char *name, node_t *node, long options,

-                     const char *target_role) {

-    const char *desc = NULL;

-    const char *class = crm_element_value(rsc->xml, XML_AGENT_ATTR_CLASS);

-    const char *kind = crm_element_value(rsc->xml, XML_ATTR_TYPE);

-    enum rsc_role_e role = native_displayable_role(rsc);

-

-    char *retval = NULL;

-

-    char *unames = NULL;

-    char *provider = NULL;

-    const char *orphan = NULL;

-    char *role_s = NULL;

-    char *node_s = NULL;

-    char *print_dev_s = NULL;

-    char *flags_s = NULL;

-

-    CRM_ASSERT(kind != NULL);

-

-    if (is_set(pcmk_get_ra_caps(class), pcmk_ra_cap_provider)) {

-        provider = crm_strdup_printf("::%s", crm_element_value(rsc->xml, XML_AGENT_ATTR_PROVIDER));

+    if (is_set(options, pe_print_dev)) {

+        if (is_set(options, pe_rsc_provisional)) {

+            have_flags = add_output_flag(outstr, "provisional", have_flags);

+        }

+        if (is_not_set(options, pe_rsc_runnable)) {

+            have_flags = add_output_flag(outstr, "non-startable", have_flags);

+        }

+        have_flags = add_output_flag(outstr, "variant:", have_flags);

+        g_string_append_printf(outstr, "%s priority:%f",

+                                       crm_element_name(rsc->xml),

+                                       (double) (rsc->priority));

     }

-

-    if (is_set(rsc->flags, pe_rsc_orphan)) {

-        orphan = " ORPHANED";

+    if (have_flags) {

+        g_string_append(outstr, ")");

     }

-    if (role > RSC_ROLE_SLAVE && is_set(rsc->flags, pe_rsc_failed)) {

-        role_s = crm_strdup_printf(" FAILED %s", role2text(role));

-    } else if (is_set(rsc->flags, pe_rsc_failed)) {

-        role_s = crm_strdup_printf(" FAILED");

-    } else {

-        role_s = crm_strdup_printf(" %s", native_displayable_state(rsc, options));

-    }

+    // User-supplied description

+    if (is_set(options, pe_print_rsconly)

+        || pcmk__list_of_multiple(rsc->running_on)) {

+        const char *desc = crm_element_value(rsc->xml, XML_ATTR_DESC);

-    if (node) {

-        node_s = crm_strdup_printf(" %s", node->details->uname);

+        if (desc) {

+            g_string_append_printf(outstr, " %s", desc);

+        }

     }

-    if (is_set(options, pe_print_rsconly) || g_list_length(rsc->running_on) > 1) {

-        desc = crm_element_value(rsc->xml, XML_ATTR_DESC);

-    }

+    if (show_nodes && is_not_set(options, pe_print_rsconly)

+        && pcmk__list_of_multiple(rsc->running_on)) {

+        bool have_nodes = false;

-    if (is_not_set(options, pe_print_rsconly) && g_list_length(rsc->running_on) > 1) {

-        GListPtr gIter = rsc->running_on;

-        gchar **arr = calloc(g_list_length(rsc->running_on)+1, sizeof(gchar *));

-        int i = 0;

-        char *total = NULL;

+        for (GList *iter = rsc->running_on; iter != NULL; iter = iter->next) {

+            pe_node_t *n = (pe_node_t *) iter->data;

-        for (; gIter != NULL; gIter = gIter->next) {

-            node_t *n = (node_t *) gIter->data;

-            arr[i] = (gchar *) strdup(n->details->uname);

-            i++;

+            have_nodes = add_output_node(outstr, n->details->uname, have_nodes);

+        }

+        if (have_nodes) {

+            g_string_append(outstr, " ]");

         }

-

-        total = g_strjoinv(" ", arr);

-        unames = crm_strdup_printf(" [ %s ]", total);

-

-        g_free(total);

-        g_strfreev(arr);

     }

-    if (is_set(options, pe_print_dev)) {

-        print_dev_s = crm_strdup_printf(" (%s%svariant=%s, priority=%f)",

-                                        is_set(rsc->flags, pe_rsc_provisional) ? "provisional, " : "",

-                                        is_set(rsc->flags, pe_rsc_runnable) ? "" : "non-startable, ",

-                                        crm_element_name(rsc->xml), (double)rsc->priority);

-    }

-

-    flags_s = flags_string(rsc, node, options, target_role);

-

-    retval = crm_strdup_printf("%s\t(%s%s:%s):\t%s%s%s%s%s%s%s%s",

-                               name, class,

-                               provider ? provider : "",

-                               kind,

-                               orphan ? orphan : "",

-                               role_s,

-                               node_s ? node_s : "",

-                               print_dev_s ? print_dev_s : "",

-                               flags_s ? flags_s : "",

-                               desc ? " " : "", desc ? desc : "",

-                               unames ? unames : "");

-

-    free(provider);

-    free(role_s);

-    free(node_s);

-    free(unames);

-    free(print_dev_s);

-    free(flags_s);

-

+    retval = outstr->str;

+    g_string_free(outstr, FALSE);

     return retval;

 }

@@ -656,7 +663,6 @@ void

 pe__common_output_html(pcmk__output_t *out, resource_t * rsc,

                        const char *name, node_t *node, long options)

 {

-    char *s = NULL;

     const char *kind = crm_element_value(rsc->xml, XML_ATTR_TYPE);

     const char *target_role = NULL;

@@ -675,10 +681,6 @@ pe__common_output_html(pcmk__output_t *out, resource_t * rsc,

         target_role = g_hash_table_lookup(rsc->meta, XML_RSC_ATTR_TARGET_ROLE);

     }

-    if ((options & pe_print_rsconly) || g_list_length(rsc->running_on) > 1) {

-        node = NULL;

-    }

-

     if (is_not_set(rsc->flags, pe_rsc_managed)) {

         cl = "rsc-managed";

@@ -698,10 +700,14 @@ pe__common_output_html(pcmk__output_t *out, resource_t * rsc,

         cl = "rsc-ok";

     }

-    s = native_output_string(rsc, name, node, options, target_role);

-    list_node = pcmk__output_create_html_node(out, "li", NULL, NULL, NULL);

-    pcmk_create_html_node(list_node, "span", NULL, cl, s);

-    free(s);

+    {

+        gchar *s = native_output_string(rsc, name, node, options, target_role,

+                                        true);

+

+        list_node = pcmk__output_create_html_node(out, "li", NULL, NULL, NULL);

+        pcmk_create_html_node(list_node, "span", NULL, cl, s);

+        g_free(s);

+    }

     if (is_set(options, pe_print_details)) {

         GHashTableIter iter;

@@ -744,7 +750,6 @@ void

 pe__common_output_text(pcmk__output_t *out, resource_t * rsc,

                        const char *name, node_t *node, long options)

 {

-    char *s = NULL;

     const char *target_role = NULL;

     CRM_ASSERT(rsc->variant == pe_native);

@@ -758,13 +763,13 @@ pe__common_output_text(pcmk__output_t *out, resource_t * rsc,

         target_role = g_hash_table_lookup(rsc->meta, XML_RSC_ATTR_TARGET_ROLE);

     }

-    if (is_set(options, pe_print_rsconly) || g_list_length(rsc->running_on) > 1) {

-        node = NULL;

-    }

+    {

+        gchar *s = native_output_string(rsc, name, node, options, target_role,

+                                        true);

-    s = native_output_string(rsc, name, node, options, target_role);

-    out->list_item(out, NULL, "%s", s);

-    free(s);

+        out->list_item(out, NULL, "%s", s);

+        g_free(s);

+    }

     if (is_set(options, pe_print_details)) {

         GHashTableIter iter;

@@ -806,22 +811,14 @@ pe__common_output_text(pcmk__output_t *out, resource_t * rsc,

 void

 common_print(resource_t * rsc, const char *pre_text, const char *name, node_t *node, long options, void *print_data)

 {

-    const char *desc = NULL;

-    const char *class = crm_element_value(rsc->xml, XML_AGENT_ATTR_CLASS);

-    const char *kind = crm_element_value(rsc->xml, XML_ATTR_TYPE);

     const char *target_role = NULL;

-    enum rsc_role_e role = native_displayable_role(rsc);

-

-    int offset = 0;

-    int flagOffset = 0;

-    char buffer[LINE_MAX];

-    char flagBuffer[LINE_MAX];

     CRM_ASSERT(rsc->variant == pe_native);

-    CRM_ASSERT(kind != NULL);

     if (rsc->meta) {

-        const char *is_internal = g_hash_table_lookup(rsc->meta, XML_RSC_ATTR_INTERNAL_RSC);

+        const char *is_internal = g_hash_table_lookup(rsc->meta,

+                                                      XML_RSC_ATTR_INTERNAL_RSC);

+

         if (crm_is_true(is_internal) && is_not_set(options, pe_print_implicit)) {

             crm_trace("skipping print of internal resource %s", rsc->id);

             return;

@@ -829,17 +826,13 @@ common_print(resource_t * rsc, const char *pre_text, const char *name, node_t *n

         target_role = g_hash_table_lookup(rsc->meta, XML_RSC_ATTR_TARGET_ROLE);

     }

-    if (pre_text == NULL && (options & pe_print_printf)) {

-        pre_text = " ";

-    }

-

     if (options & pe_print_xml) {

         native_print_xml(rsc, pre_text, options, print_data);

         return;

     }

-    if ((options & pe_print_rsconly) || g_list_length(rsc->running_on) > 1) {

-        node = NULL;

+    if ((pre_text == NULL) && (options & pe_print_printf)) {

+        pre_text = " ";

     }

     if (options & pe_print_html) {

@@ -849,10 +842,10 @@ common_print(resource_t * rsc, const char *pre_text, const char *name, node_t *n

         } else if (is_set(rsc->flags, pe_rsc_failed)) {

             status_print("<font color=\"red\">");

-        } else if (rsc->variant == pe_native && (rsc->running_on == NULL)) {

+        } else if (rsc->running_on == NULL) {

             status_print("<font color=\"red\">");

-        } else if (g_list_length(rsc->running_on) > 1) {

+        } else if (pcmk__list_of_multiple(rsc->running_on)) {

             status_print("<font color=\"orange\">");

         } else if (is_set(rsc->flags, pe_rsc_failure_ignored)) {

@@ -863,106 +856,29 @@ common_print(resource_t * rsc, const char *pre_text, const char *name, node_t *n

         }

     }

-    if(pre_text) {

-        offset += snprintf(buffer + offset, LINE_MAX - offset, "%s", pre_text);

-    }

-    offset += snprintf(buffer + offset, LINE_MAX - offset, "%s", name);

-    offset += snprintf(buffer + offset, LINE_MAX - offset, "\t(%s", class);

-    if (is_set(pcmk_get_ra_caps(class), pcmk_ra_cap_provider)) {

-        const char *prov = crm_element_value(rsc->xml, XML_AGENT_ATTR_PROVIDER);

-        offset += snprintf(buffer + offset, LINE_MAX - offset, "::%s", prov);

-    }

-    offset += snprintf(buffer + offset, LINE_MAX - offset, ":%s):\t", kind);

-    if(is_set(rsc->flags, pe_rsc_orphan)) {

-        offset += snprintf(buffer + offset, LINE_MAX - offset, " ORPHANED ");

-    }

-    if(role > RSC_ROLE_SLAVE && is_set(rsc->flags, pe_rsc_failed)) {

-        offset += snprintf(buffer + offset, LINE_MAX - offset, "FAILED %s", role2text(role));

-    } else if(is_set(rsc->flags, pe_rsc_failed)) {

-        offset += snprintf(buffer + offset, LINE_MAX - offset, "FAILED");

-    } else {

-        const char *rsc_state = native_displayable_state(rsc, options);

-

-        offset += snprintf(buffer + offset, LINE_MAX - offset, "%s", rsc_state);

-    }

-

-    if(node) {

-        offset += snprintf(buffer + offset, LINE_MAX - offset, " %s", node->details->uname);

-

-        if (node->details->online == FALSE && node->details->unclean) {

-            flagOffset += snprintf(flagBuffer + flagOffset, LINE_MAX - flagOffset,

-                                   "%sUNCLEAN", comma_if(flagOffset));

-        }

-    }

-

-    if (options & pe_print_pending) {

-        const char *pending_task = native_pending_task(rsc);

-

-        if (pending_task) {

-            flagOffset += snprintf(flagBuffer + flagOffset, LINE_MAX - flagOffset,

-                                   "%s%s", comma_if(flagOffset), pending_task);

-        }

-    }

-

-    if (target_role) {

-        enum rsc_role_e target_role_e = text2role(target_role);

-

-        /* Ignore target role Started, as it is the default anyways

-         * (and would also allow a Master to be Master).

-         * Show if target role limits our abilities. */

-        if (target_role_e == RSC_ROLE_STOPPED) {

-            flagOffset += snprintf(flagBuffer + flagOffset, LINE_MAX - flagOffset,

-                                   "%sdisabled", comma_if(flagOffset));

-

-        } else if (is_set(uber_parent(rsc)->flags, pe_rsc_promotable)

-                   && target_role_e == RSC_ROLE_SLAVE) {

-            flagOffset += snprintf(flagBuffer + flagOffset, LINE_MAX - flagOffset,

-                                   "%starget-role:%s", comma_if(flagOffset), target_role);

-        }

-    }

-

-    if (is_set(rsc->flags, pe_rsc_block)) {

-        flagOffset += snprintf(flagBuffer + flagOffset, LINE_MAX - flagOffset,

-                               "%sblocked", comma_if(flagOffset));

-

-    } else if (is_not_set(rsc->flags, pe_rsc_managed)) {

-        flagOffset += snprintf(flagBuffer + flagOffset, LINE_MAX - flagOffset,

-                               "%sunmanaged", comma_if(flagOffset));

-    }

-

-    if(is_set(rsc->flags, pe_rsc_failure_ignored)) {

-        flagOffset += snprintf(flagBuffer + flagOffset, LINE_MAX - flagOffset,

-                               "%sfailure ignored", comma_if(flagOffset));

-    }

-

-    if ((options & pe_print_rsconly) || g_list_length(rsc->running_on) > 1) {

-        desc = crm_element_value(rsc->xml, XML_ATTR_DESC);

-    }

-

-    CRM_LOG_ASSERT(offset > 0);

-    if(flagOffset > 0) {

-        status_print("%s (%s)%s%s", buffer, flagBuffer, desc?" ":"", desc?desc:"");

-    } else {

-        status_print("%s%s%s", buffer, desc?" ":"", desc?desc:"");

+    {

+        gchar *resource_s = native_output_string(rsc, name, node, options,

+                                                 target_role, false);

+        status_print("%s%s", (pre_text? pre_text : ""), resource_s);

+        g_free(resource_s);

     }

 #if CURSES_ENABLED

-    if ((options & pe_print_rsconly) || g_list_length(rsc->running_on) > 1) {

-        /* Done */

-

-    } else if (options & pe_print_ncurses) {

+    if (is_set(options, pe_print_ncurses)

+        && is_not_set(options, pe_print_rsconly)

+        && !pcmk__list_of_multiple(rsc->running_on)) {

         /* coverity[negative_returns] False positive */

         move(-1, 0);

     }

 #endif

-    if (options & pe_print_html) {

+    if (is_set(options, pe_print_html)) {

         status_print(" </font> ");

     }

-    if ((options & pe_print_rsconly)) {

+    if (is_not_set(options, pe_print_rsconly)

+        && pcmk__list_of_multiple(rsc->running_on)) {

-    } else if (g_list_length(rsc->running_on) > 1) {

         GListPtr gIter = rsc->running_on;

         int counter = 0;

@@ -1025,10 +941,6 @@ common_print(resource_t * rsc, const char *pre_text, const char *name, node_t *n

         GHashTableIter iter;

         node_t *n = NULL;

-        status_print("%s\t(%s%svariant=%s, priority=%f)", pre_text,

-                     is_set(rsc->flags, pe_rsc_provisional) ? "provisional, " : "",

-                     is_set(rsc->flags, pe_rsc_runnable) ? "" : "non-startable, ",

-                     crm_element_name(rsc->xml), (double)rsc->priority);

         status_print("%s\tAllowed Nodes", pre_text);

         g_hash_table_iter_init(&iter, rsc->allowed_nodes);

         while (g_hash_table_iter_next(&iter, NULL, (void **)&n)) {

-- 

1.8.3.1

From 41e911be8ea9151b3f0758c2c22c0e69b8b78d93 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Thu, 19 Dec 2019 17:18:41 -0600

Subject: [PATCH 2/8] Log: scheduler: drop redundant trace messages

We logged "applying placement constraints" three times.

---

 lib/pacemaker/pcmk_sched_allocate.c | 17 ++++-------------

 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/lib/pacemaker/pcmk_sched_allocate.c b/lib/pacemaker/pcmk_sched_allocate.c

index ca43c71..dde8b69 100644

--- a/lib/pacemaker/pcmk_sched_allocate.c

+++ b/lib/pacemaker/pcmk_sched_allocate.c

@@ -623,21 +623,15 @@ check_actions(pe_working_set_t * data_set)

     }

 }

-static gboolean

+static void

 apply_placement_constraints(pe_working_set_t * data_set)

 {

-    GListPtr gIter = NULL;

-

-    crm_trace("Applying constraints...");

-

-    for (gIter = data_set->placement_constraints; gIter != NULL; gIter = gIter->next) {

+    for (GList *gIter = data_set->placement_constraints;

+         gIter != NULL; gIter = gIter->next) {

         pe__location_t *cons = gIter->data;

         cons->rsc_lh->cmds->rsc_location(cons->rsc_lh, cons);

     }

-

-    return TRUE;

-

 }

 static gboolean

@@ -994,10 +988,7 @@ stage2(pe_working_set_t * data_set)

 {

     GListPtr gIter = NULL;

-    crm_trace("Applying placement constraints");

-

-    gIter = data_set->nodes;

-    for (; gIter != NULL; gIter = gIter->next) {

+    for (gIter = data_set->nodes; gIter != NULL; gIter = gIter->next) {

         node_t *node = (node_t *) gIter->data;

         if (node == NULL) {

-- 

1.8.3.1

From 7fe136e19b5018d609beb8bad4e34234739572c9 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Sat, 7 Dec 2019 12:13:11 -0600

Subject: [PATCH 3/8] Refactor: libcrmcommon: convenience functions for list

 length comparisons

... for efficiency and readability

---

 include/crm/common/internal.h | 14 ++++++++++++++

 1 file changed, 14 insertions(+)

diff --git a/include/crm/common/internal.h b/include/crm/common/internal.h

index da2c7d7..484c836 100644

--- a/include/crm/common/internal.h

+++ b/include/crm/common/internal.h

@@ -126,6 +126,20 @@ crm_getpid_s()

     return crm_strdup_printf("%lu", (unsigned long) getpid());

 }

+// More efficient than g_list_length(list) == 1

+static inline bool

+pcmk__list_of_1(GList *list)

+{

+    return list && (list->next == NULL);

+}

+

+// More efficient than g_list_length(list) > 1

+static inline bool

+pcmk__list_of_multiple(GList *list)

+{

+    return list && (list->next != NULL);

+}

+

 /* convenience functions for failure-related node attributes */

 #define CRM_FAIL_COUNT_PREFIX   "fail-count"