From 11e8a3b9c8e35301b197724658ec2d243aec3336 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Fri, 22 Nov 2019 16:39:54 -0600

Subject: [PATCH 01/11] Refactor: controller: rename struct recurring_op_s to

 active_op_t

... because it holds both recurring and pending non-recurring actions,

and the name was confusing

---

 crmd/crmd_lrm.h  |  8 ++++----

 crmd/lrm.c       | 18 +++++++++---------

 crmd/lrm_state.c |  4 ++--

 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/crmd/crmd_lrm.h b/crmd/crmd_lrm.h

index 0870817..7c6a3c4 100644

--- a/crmd/crmd_lrm.h

+++ b/crmd/crmd_lrm.h

@@ -42,8 +42,8 @@ typedef struct resource_history_s {

 void history_free(gpointer data);

-/* TODO - Replace this with lrmd_event_data_t */

-struct recurring_op_s {

+// In-flight action (recurring or pending)

+typedef struct active_op_s {

     int call_id;

     int interval;

     gboolean remove;

@@ -54,7 +54,7 @@ struct recurring_op_s {

     char *op_key;

     char *user_data;

     GHashTable *params;

-};

+} active_op_t;

 typedef struct lrm_state_s {

     const char *node_name;

@@ -171,4 +171,4 @@ void remote_ra_process_maintenance_nodes(xmlNode *xml);

 gboolean remote_ra_controlling_guest(lrm_state_t * lrm_state);

 void process_lrm_event(lrm_state_t *lrm_state, lrmd_event_data_t *op,

-                       struct recurring_op_s *pending, xmlNode *action_xml);

+                       active_op_t *pending, xmlNode *action_xml);

diff --git a/crmd/lrm.c b/crmd/lrm.c

index 437840f..e459465 100644

--- a/crmd/lrm.c

+++ b/crmd/lrm.c

@@ -400,7 +400,7 @@ lrm_state_verify_stopped(lrm_state_t * lrm_state, enum crmd_fsa_state cur_state,

     GHashTableIter gIter;

     const char *key = NULL;

     rsc_history_t *entry = NULL;

-    struct recurring_op_s *pending = NULL;

+    active_op_t *pending = NULL;

     crm_debug("Checking for active resources before exit");

@@ -917,7 +917,7 @@ static gboolean

 lrm_remove_deleted_op(gpointer key, gpointer value, gpointer user_data)

 {

     const char *rsc = user_data;

-    struct recurring_op_s *pending = value;

+    active_op_t *pending = value;

     if (crm_str_eq(rsc, pending->rsc_id, TRUE)) {

         crm_info("Removing op %s:%d for deleted resource %s",

@@ -1148,7 +1148,7 @@ cancel_op(lrm_state_t * lrm_state, const char *rsc_id, const char *key, int op,

 {

     int rc = pcmk_ok;

     char *local_key = NULL;

-    struct recurring_op_s *pending = NULL;

+    active_op_t *pending = NULL;

     CRM_CHECK(op != 0, return FALSE);

     CRM_CHECK(rsc_id != NULL, return FALSE);

@@ -1213,7 +1213,7 @@ cancel_action_by_key(gpointer key, gpointer value, gpointer user_data)

 {

     gboolean remove = FALSE;

     struct cancel_data *data = user_data;

-    struct recurring_op_s *op = (struct recurring_op_s *)value;

+    active_op_t *op = value;

     if (crm_str_eq(op->op_key, data->key, TRUE)) {

         data->done = TRUE;

@@ -2104,7 +2104,7 @@ stop_recurring_action_by_rsc(gpointer key, gpointer value, gpointer user_data)

 {

     gboolean remove = FALSE;

     struct stop_recurring_action_s *event = user_data;

-    struct recurring_op_s *op = (struct recurring_op_s *)value;

+    active_op_t *op = value;

     if (op->interval != 0 && crm_str_eq(op->rsc_id, event->rsc->id, TRUE)) {

         crm_debug("Cancelling op %d for %s (%s)", op->call_id, op->rsc_id, (char*)key);

@@ -2119,7 +2119,7 @@ stop_recurring_actions(gpointer key, gpointer value, gpointer user_data)

 {

     gboolean remove = FALSE;

     lrm_state_t *lrm_state = user_data;

-    struct recurring_op_s *op = (struct recurring_op_s *)value;

+    active_op_t *op = value;

     if (op->interval != 0) {

         crm_info("Cancelling op %d for %s (%s)", op->call_id, op->rsc_id, key);

@@ -2294,9 +2294,9 @@ do_lrm_rsc_op(lrm_state_t * lrm_state, lrmd_rsc_info_t * rsc, const char *operat

          * for them to complete during shutdown

          */

         char *call_id_s = make_stop_id(rsc->id, call_id);

-        struct recurring_op_s *pending = NULL;

+        active_op_t *pending = NULL;

-        pending = calloc(1, sizeof(struct recurring_op_s));

+        pending = calloc(1, sizeof(active_op_t));

         crm_trace("Recording pending op: %d - %s %s", call_id, op_id, call_id_s);

         pending->call_id = call_id;

@@ -2517,7 +2517,7 @@ did_lrm_rsc_op_fail(lrm_state_t *lrm_state, const char * rsc_id,

 void

 process_lrm_event(lrm_state_t *lrm_state, lrmd_event_data_t *op,

-                  struct recurring_op_s *pending, xmlNode *action_xml)

+                  active_op_t *pending, xmlNode *action_xml)

 {

     char *op_id = NULL;

     char *op_key = NULL;

diff --git a/crmd/lrm_state.c b/crmd/lrm_state.c

index 8d07ef5..0f39c3d 100644

--- a/crmd/lrm_state.c

+++ b/crmd/lrm_state.c

@@ -58,7 +58,7 @@ free_deletion_op(gpointer value)

 static void

 free_recurring_op(gpointer value)

 {

-    struct recurring_op_s *op = (struct recurring_op_s *)value;

+    active_op_t *op = value;

     free(op->user_data);

     free(op->rsc_id);

@@ -75,7 +75,7 @@ fail_pending_op(gpointer key, gpointer value, gpointer user_data)

 {

     lrmd_event_data_t event = { 0, };

     lrm_state_t *lrm_state = user_data;

-    struct recurring_op_s *op = (struct recurring_op_s *)value;

+    active_op_t *op = value;

     crm_trace("Pre-emptively failing %s_%s_%d on %s (call=%s, %s)",

               op->rsc_id, op->op_type, op->interval,

-- 

1.8.3.1

From 9795f5401957563de2307f94c393dc83ce41d3d1 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Fri, 22 Nov 2019 16:45:31 -0600

Subject: [PATCH 02/11] Refactor: controller: convert active_op_t booleans to

 bitmask

---

 crmd/crmd_lrm.h |  8 ++++++--

 crmd/lrm.c      | 11 +++++------

 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/crmd/crmd_lrm.h b/crmd/crmd_lrm.h

index 7c6a3c4..eb27d84 100644

--- a/crmd/crmd_lrm.h

+++ b/crmd/crmd_lrm.h

@@ -42,12 +42,16 @@ typedef struct resource_history_s {

 void history_free(gpointer data);

+enum active_op_e {

+    active_op_remove    = (1 << 0),

+    active_op_cancelled = (1 << 1),

+};

+

 // In-flight action (recurring or pending)

 typedef struct active_op_s {

     int call_id;

     int interval;

-    gboolean remove;

-    gboolean cancelled;

+    uint32_t flags; // bitmask of active_op_e

     unsigned int start_time;

     char *rsc_id;

     char *op_type;

diff --git a/crmd/lrm.c b/crmd/lrm.c

index e459465..2ae0d85 100644

--- a/crmd/lrm.c

+++ b/crmd/lrm.c

@@ -1159,18 +1159,17 @@ cancel_op(lrm_state_t * lrm_state, const char *rsc_id, const char *key, int op,

     pending = g_hash_table_lookup(lrm_state->pending_ops, key);

     if (pending) {

-        if (remove && pending->remove == FALSE) {

-            pending->remove = TRUE;

+        if (remove && is_not_set(pending->flags, active_op_remove)) {

+            set_bit(pending->flags, active_op_remove);

             crm_debug("Scheduling %s for removal", key);

         }

-        if (pending->cancelled) {

+        if (is_set(pending->flags, active_op_cancelled)) {

             crm_debug("Operation %s already cancelled", key);

             free(local_key);

             return FALSE;

         }

-

-        pending->cancelled = TRUE;

+        set_bit(pending->flags, active_op_cancelled);

     } else {

         crm_info("No pending op found for %s", key);

@@ -2636,7 +2635,7 @@ process_lrm_event(lrm_state_t *lrm_state, lrmd_event_data_t *op,

         crm_err("Recurring operation %s was cancelled without transition information",

                 op_key);

-    } else if (pending->remove) {

+    } else if (is_set(pending->flags, active_op_remove)) {

         /* This recurring operation was cancelled (by us) and pending, and we

          * have been waiting for it to finish.

          */

-- 

1.8.3.1

From 620ff6cb923e4fe6c3d1a9e3345a26ff5180d56d Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Fri, 22 Nov 2019 16:58:25 -0600

Subject: [PATCH 03/11] Refactor: controller: remove unused argument

---

 crmd/lrm.c | 10 +++++-----

 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/crmd/lrm.c b/crmd/lrm.c

index 2ae0d85..80fcd69 100644

--- a/crmd/lrm.c

+++ b/crmd/lrm.c

@@ -44,8 +44,8 @@ static int delete_rsc_status(lrm_state_t * lrm_state, const char *rsc_id, int ca

 static lrmd_event_data_t *construct_op(lrm_state_t * lrm_state, xmlNode * rsc_op,

                                        const char *rsc_id, const char *operation);

-static void do_lrm_rsc_op(lrm_state_t * lrm_state, lrmd_rsc_info_t * rsc, const char *operation,

-                          xmlNode * msg, xmlNode * request);

+static void do_lrm_rsc_op(lrm_state_t *lrm_state, lrmd_rsc_info_t *rsc,

+                          const char *operation, xmlNode *msg);

 void send_direct_ack(const char *to_host, const char *to_sys,

                      lrmd_rsc_info_t * rsc, lrmd_event_data_t * op, const char *rsc_id);

@@ -1851,7 +1851,7 @@ do_lrm_invoke(long long action,

                           crm_rsc_delete, user_name);

         } else {

-            do_lrm_rsc_op(lrm_state, rsc, operation, input->xml, input->msg);

+            do_lrm_rsc_op(lrm_state, rsc, operation, input->xml);

         }

         lrmd_free_rsc_info(rsc);

@@ -2167,8 +2167,8 @@ record_pending_op(const char *node_name, lrmd_rsc_info_t *rsc, lrmd_event_data_t

 }

 static void

-do_lrm_rsc_op(lrm_state_t * lrm_state, lrmd_rsc_info_t * rsc, const char *operation, xmlNode * msg,

-              xmlNode * request)

+do_lrm_rsc_op(lrm_state_t *lrm_state, lrmd_rsc_info_t *rsc,

+              const char *operation, xmlNode *msg)

 {

     int call_id = 0;

     char *op_id = NULL;

-- 

1.8.3.1

From 2716f2f4c334b927f9e253979dce27b56bbff46a Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Fri, 6 Dec 2019 12:15:05 -0600

Subject: [PATCH 04/11] Refactor: scheduler: combine two "if" statements

... for readability, and ease of adding another block later

---

 pengine/graph.c | 120 ++++++++++++++++++++++++++++----------------------------

 1 file changed, 60 insertions(+), 60 deletions(-)

diff --git a/pengine/graph.c b/pengine/graph.c

index 9edd1a1..cba30d0 100644

--- a/pengine/graph.c

+++ b/pengine/graph.c

@@ -1095,71 +1095,71 @@ action2xml(action_t * action, gboolean as_input, pe_working_set_t *data_set)

         return action_xml;

     }

-    /* List affected resource */

-    if (action->rsc) {

-        if (is_set(action->flags, pe_action_pseudo) == FALSE) {

-            int lpc = 0;

-

-            xmlNode *rsc_xml = create_xml_node(action_xml, crm_element_name(action->rsc->xml));

-

-            const char *attr_list[] = {

-                XML_AGENT_ATTR_CLASS,

-                XML_AGENT_ATTR_PROVIDER,

-                XML_ATTR_TYPE

-            };

-

-            if (is_set(action->rsc->flags, pe_rsc_orphan) && action->rsc->clone_name) {

-                /* Do not use the 'instance free' name here as that

-                 * might interfere with the instance we plan to keep.

-                 * Ie. if there are more than two named /anonymous/

-                 * instances on a given node, we need to make sure the

-                 * command goes to the right one.

-                 *

-                 * Keep this block, even when everyone is using

-                 * 'instance free' anonymous clone names - it means

-                 * we'll do the right thing if anyone toggles the

-                 * unique flag to 'off'

-                 */

-                crm_debug("Using orphan clone name %s instead of %s", action->rsc->id,

-                          action->rsc->clone_name);

-                crm_xml_add(rsc_xml, XML_ATTR_ID, action->rsc->clone_name);

-                crm_xml_add(rsc_xml, XML_ATTR_ID_LONG, action->rsc->id);

+    if (action->rsc && is_not_set(action->flags, pe_action_pseudo)) {

+        int lpc = 0;

+        xmlNode *rsc_xml = NULL;

+        const char *attr_list[] = {

+            XML_AGENT_ATTR_CLASS,

+            XML_AGENT_ATTR_PROVIDER,

+            XML_ATTR_TYPE

+        };

+

+        // List affected resource

+

+        rsc_xml = create_xml_node(action_xml,

+                                  crm_element_name(action->rsc->xml));

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

+            && action->rsc->clone_name) {

+            /* Do not use the 'instance free' name here as that

+             * might interfere with the instance we plan to keep.

+             * Ie. if there are more than two named /anonymous/

+             * instances on a given node, we need to make sure the

+             * command goes to the right one.

+             *

+             * Keep this block, even when everyone is using

+             * 'instance free' anonymous clone names - it means

+             * we'll do the right thing if anyone toggles the

+             * unique flag to 'off'

+             */

+            crm_debug("Using orphan clone name %s instead of %s", action->rsc->id,

+                      action->rsc->clone_name);

+            crm_xml_add(rsc_xml, XML_ATTR_ID, action->rsc->clone_name);

+            crm_xml_add(rsc_xml, XML_ATTR_ID_LONG, action->rsc->id);

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

-                const char *xml_id = ID(action->rsc->xml);

-

-                crm_debug("Using anonymous clone name %s for %s (aka. %s)", xml_id, action->rsc->id,

-                          action->rsc->clone_name);

-

-                /* ID is what we'd like client to use

-                 * ID_LONG is what they might know it as instead

-                 *

-                 * ID_LONG is only strictly needed /here/ during the

-                 * transition period until all nodes in the cluster

-                 * are running the new software /and/ have rebooted

-                 * once (meaning that they've only ever spoken to a DC

-                 * supporting this feature).

-                 *

-                 * If anyone toggles the unique flag to 'on', the

-                 * 'instance free' name will correspond to an orphan

-                 * and fall into the clause above instead

-                 */

-                crm_xml_add(rsc_xml, XML_ATTR_ID, xml_id);

-                if (action->rsc->clone_name && safe_str_neq(xml_id, action->rsc->clone_name)) {

-                    crm_xml_add(rsc_xml, XML_ATTR_ID_LONG, action->rsc->clone_name);

-                } else {

-                    crm_xml_add(rsc_xml, XML_ATTR_ID_LONG, action->rsc->id);

-                }

+        } else if (is_not_set(action->rsc->flags, pe_rsc_unique)) {

+            const char *xml_id = ID(action->rsc->xml);

+

+            crm_debug("Using anonymous clone name %s for %s (aka. %s)", xml_id, action->rsc->id,

+                      action->rsc->clone_name);

+            /* ID is what we'd like client to use

+             * ID_LONG is what they might know it as instead

+             *

+             * ID_LONG is only strictly needed /here/ during the

+             * transition period until all nodes in the cluster

+             * are running the new software /and/ have rebooted

+             * once (meaning that they've only ever spoken to a DC

+             * supporting this feature).

+             *

+             * If anyone toggles the unique flag to 'on', the

+             * 'instance free' name will correspond to an orphan

+             * and fall into the clause above instead

+             */

+            crm_xml_add(rsc_xml, XML_ATTR_ID, xml_id);

+            if (action->rsc->clone_name && safe_str_neq(xml_id, action->rsc->clone_name)) {

+                crm_xml_add(rsc_xml, XML_ATTR_ID_LONG, action->rsc->clone_name);

             } else {

-                CRM_ASSERT(action->rsc->clone_name == NULL);

-                crm_xml_add(rsc_xml, XML_ATTR_ID, action->rsc->id);

+                crm_xml_add(rsc_xml, XML_ATTR_ID_LONG, action->rsc->id);

             }

-            for (lpc = 0; lpc < DIMOF(attr_list); lpc++) {

-                crm_xml_add(rsc_xml, attr_list[lpc],

-                            g_hash_table_lookup(action->rsc->meta, attr_list[lpc]));

-            }

+        } else {

+            CRM_ASSERT(action->rsc->clone_name == NULL);

+            crm_xml_add(rsc_xml, XML_ATTR_ID, action->rsc->id);

+        }

+

+        for (lpc = 0; lpc < DIMOF(attr_list); lpc++) {

+            crm_xml_add(rsc_xml, attr_list[lpc],

+                        g_hash_table_lookup(action->rsc->meta, attr_list[lpc]));

         }

     }

-- 

1.8.3.1

From dc73907eeae769d70b04e4a8feae208c12018d83 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

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

Subject: [PATCH 05/11] Log: scheduler: drop redundant trace messages

We logged "applying placement constraints" three times.

---

 pengine/allocate.c | 17 ++++-------------

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

diff --git a/pengine/allocate.c b/pengine/allocate.c

index b819af3..30d29e1 100644

--- a/pengine/allocate.c

+++ b/pengine/allocate.c

@@ -652,21 +652,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

@@ -1026,10 +1020,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 ae404a4e04b50e6177942256c5b8ff57a36af309 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

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

Subject: [PATCH 06/11] 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 0d225f5..0f7158c 100644

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

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

@@ -84,6 +84,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"

-- 

1.8.3.1

From 78a1d57ae4028e3068b13d3526840f24c7798140 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Mon, 16 Dec 2019 14:13:30 -0600

Subject: [PATCH 07/11] Refactor: libcrmcommon: add convenience macros for

 plurals

I've avoided making s_if_plural() an official API due to its hackiness, but

it really is the best solution for now. Promote it to pcmk__plural_s(), along

with a companion macro pcmk__plural_alt() for more complicated plurals.

---

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

 1 file changed, 23 insertions(+)

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

index 0f7158c..5bbf01b 100644

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

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

@@ -72,6 +72,29 @@ bool crm_compress_string(const char *data, int length, int max, char **result,

                          unsigned int *result_len);

 gint crm_alpha_sort(gconstpointer a, gconstpointer b);

+/* Correctly displaying singular or plural is complicated; consider "1 node has"

+ * vs. "2 nodes have". A flexible solution is to pluralize entire strings, e.g.

+ *

+ * if (a == 1) {

+ *     crm_info("singular message"):

+ * } else {

+ *     crm_info("plural message");

+ * }

+ *

+ * though even that's not sufficient for all languages besides English (if we

+ * ever desire to do translations of output and log messages). But the following

+ * convenience macros are "good enough" and more concise for many cases.

+ */

+

+/* Example:

+ * crm_info("Found %d %s", nentries,

+ *          pcmk__plural_alt(nentries, "entry", "entries"));

+ */

+#define pcmk__plural_alt(i, s1, s2) (((i) == 1)? (s1) : (s2))

+

+// Example: crm_info("Found %d node%s", nnodes, pcmk__plural_s(nnodes));

+#define pcmk__plural_s(i) pcmk__plural_alt(i, "", "s")

+

 static inline int

 crm_strlen_zero(const char *s)

 {

-- 

1.8.3.1

From e33b5e297c24182c15ace7142efef1add7664643 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Thu, 12 Dec 2019 20:50:50 -0600

Subject: [PATCH 08/11] Log: controller: improve join messages

---

 crmd/fsa.c     |  79 +++++++-----

 crmd/join_dc.c | 375 +++++++++++++++++++++++++++++++++------------------------

 2 files changed, 263 insertions(+), 191 deletions(-)

diff --git a/crmd/fsa.c b/crmd/fsa.c

index 9b1189a..7f6a3ac 100644

--- a/crmd/fsa.c

+++ b/crmd/fsa.c

@@ -469,12 +469,53 @@ log_fsa_input(fsa_data_t * stored_msg)

     }

 }

+static void

+check_join_counts(fsa_data_t *msg_data)

+{

+    int count;

+    guint npeers;

+

+    count = crmd_join_phase_count(crm_join_finalized);

+    if (count > 0) {

+        crm_err("%d cluster node%s failed to confirm join",

+                count, pcmk__plural_s(count));

+        crmd_join_phase_log(LOG_NOTICE);

+        return;

+    }

+

+    npeers = crm_active_peers();

+    count = crmd_join_phase_count(crm_join_confirmed);

+    if (count == npeers) {

+        if (npeers == 1) {

+            crm_debug("Sole active cluster node is fully joined");

+        } else {

+            crm_debug("All %d active cluster nodes are fully joined", count);

+        }

+

+    } else if (count > npeers) {

+        crm_err("New election needed because more nodes confirmed join "

+                "than are in membership (%d > %u)", count, npeers);

+        register_fsa_input(C_FSA_INTERNAL, I_ELECTION, NULL);

+

+    } else if (saved_ccm_membership_id != crm_peer_seq) {

+        crm_info("New join needed because membership changed (%llu -> %llu)",

+                 saved_ccm_membership_id, crm_peer_seq);

+        register_fsa_input_before(C_FSA_INTERNAL, I_NODE_JOIN, NULL);

+

+    } else {

+        crm_warn("Only %d of %u active cluster nodes fully joined "

+                 "(%d did not respond to offer)",

+                 count, npeers, crmd_join_phase_count(crm_join_welcomed));

+    }

+}

+

 long long

 do_state_transition(long long actions,

                     enum crmd_fsa_state cur_state,

                     enum crmd_fsa_state next_state, fsa_data_t * msg_data)

 {

     int level = LOG_INFO;

+    int count = 0;

     long long tmp = actions;

     gboolean clear_recovery_bit = TRUE;

@@ -572,13 +613,14 @@ do_state_transition(long long actions,

                 crm_warn("Progressed to state %s after %s",

                          fsa_state2string(next_state), fsa_cause2string(cause));

             }

-            if (crmd_join_phase_count(crm_join_welcomed) > 0) {

-                crm_warn("%u cluster nodes failed to respond"

-                         " to the join offer.", crmd_join_phase_count(crm_join_welcomed));

+            count = crmd_join_phase_count(crm_join_welcomed);

+            if (count > 0) {

+                crm_warn("%d cluster node%s failed to respond to join offer",

+                         count, pcmk__plural_s(count));

                 crmd_join_phase_log(LOG_NOTICE);

             } else {

-                crm_debug("All %d cluster nodes responded to the join offer.",

+                crm_debug("All cluster nodes (%d) responded to join offer",

                           crmd_join_phase_count(crm_join_integrated));

             }

             break;

@@ -590,34 +632,7 @@ do_state_transition(long long actions,

                 crm_info("Progressed to state %s after %s",

                          fsa_state2string(next_state), fsa_cause2string(cause));

             }

-

-            if (crmd_join_phase_count(crm_join_finalized) > 0) {

-                crm_err("%u cluster nodes failed to confirm their join.",

-                        crmd_join_phase_count(crm_join_finalized));

-                crmd_join_phase_log(LOG_NOTICE);

-

-            } else if (crmd_join_phase_count(crm_join_confirmed)

-                       == crm_active_peers()) {

-                crm_debug("All %u cluster nodes are"

-                          " eligible to run resources.", crm_active_peers());

-

-            } else if (crmd_join_phase_count(crm_join_confirmed) > crm_active_peers()) {

-                crm_err("We have more confirmed nodes than our membership does: %d vs. %d",

-                        crmd_join_phase_count(crm_join_confirmed), crm_active_peers());

-                register_fsa_input(C_FSA_INTERNAL, I_ELECTION, NULL);

-

-            } else if (saved_ccm_membership_id != crm_peer_seq) {

-                crm_info("Membership changed: %llu -> %llu - join restart",

-                         saved_ccm_membership_id, crm_peer_seq);

-                register_fsa_input_before(C_FSA_INTERNAL, I_NODE_JOIN, NULL);

-

-            } else {

-                crm_warn("Only %u of %u cluster "

-                         "nodes are eligible to run resources - continue %d",

-                         crmd_join_phase_count(crm_join_confirmed),

-                         crm_active_peers(), crmd_join_phase_count(crm_join_welcomed));

-            }

-/* 			initialize_join(FALSE); */

+            check_join_counts(msg_data);

             break;

         case S_STOPPING:

diff --git a/crmd/join_dc.c b/crmd/join_dc.c

index 857e760..cdb3f77 100644

--- a/crmd/join_dc.c

+++ b/crmd/join_dc.c

@@ -36,7 +36,11 @@ void finalize_join_for(gpointer key, gpointer value, gpointer user_data);

 void finalize_sync_callback(xmlNode * msg, int call_id, int rc, xmlNode * output, void *user_data);

 gboolean check_join_state(enum crmd_fsa_state cur_state, const char *source);

+/* Numeric counter used to identify join rounds (an unsigned int would be

+ * appropriate, except we get and set it in XML as int)

+ */

 static int current_join_id = 0;

+

 unsigned long long saved_ccm_membership_id = 0;

 void

@@ -44,12 +48,7 @@ crm_update_peer_join(const char *source, crm_node_t * node, enum crm_join_phase

 {

     enum crm_join_phase last = 0;

-    if(node == NULL) {

-        crm_err("Could not update join because node not specified"

-                CRM_XS " join-%u source=%s phase=%s",

-                current_join_id, source, crm_join_phase_str(phase));

-        return;

-    }

+    CRM_CHECK(node != NULL, return);

     /* Remote nodes do not participate in joins */