Blob Blame History Raw
From 2c91b4a54d5b70204409ea2f7e92fbc7748e4257 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 17 Mar 2017 16:32:45 -0500
Subject: [PATCH 1/5] Log: various: minor improvements

add missing newline, reword to avoid singular/plural issue,
and correct misspellings (in log messages and comments)
---
 configure.ac                 | 2 +-
 crmd/join_dc.c               | 2 +-
 crmd/lrm.c                   | 4 ++--
 fencing/remote.c             | 4 ++--
 include/crm/pengine/status.h | 8 ++++----
 lib/cluster/election.c       | 2 +-
 lib/common/xml.c             | 2 +-
 lib/pengine/utils.c          | 4 ++--
 lrmd/lrmd.c                  | 5 +++--
 pengine/graph.c              | 2 +-
 pengine/native.c             | 2 +-
 tools/fake_transition.c      | 4 ++--
 12 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/configure.ac b/configure.ac
index b1465a3..687d3fe 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1759,7 +1759,7 @@ if test "$GCC" != yes; then
 else
         CFLAGS="$CFLAGS -ggdb"
 
-dnl when we don't have diagnostic push / pull we can't explicitely disable
+dnl When we don't have diagnostic push / pull, we can't explicitly disable
 dnl checking for nonliteral formats in the places where they occur on purpose
 dnl thus we disable nonliteral format checking globally as we are aborting
 dnl on warnings. 
diff --git a/crmd/join_dc.c b/crmd/join_dc.c
index 6af0884..d5ecc55 100644
--- a/crmd/join_dc.c
+++ b/crmd/join_dc.c
@@ -385,7 +385,7 @@ do_dc_join_finalize(long long action,
     /* This we can do straight away and avoid clients timing us out
      *  while we compute the latest CIB
      */
-    crm_debug("Finializing join-%d for %d clients",
+    crm_debug("Finalizing join-%d for %d clients",
               current_join_id, crmd_join_phase_count(crm_join_integrated));
 
     crmd_join_phase_log(LOG_INFO);
diff --git a/crmd/lrm.c b/crmd/lrm.c
index 2d553cd..f7aefbf 100644
--- a/crmd/lrm.c
+++ b/crmd/lrm.c
@@ -231,8 +231,8 @@ update_history_cache(lrm_state_t * lrm_state, lrmd_rsc_info_t * rsc, lrmd_event_
         }
 
     } else if (did_rsc_op_fail(op, target_rc)) {
-        /* We must store failed monitors here
-         * - otherwise the block below will cause them to be forgetten them when a stop happens
+        /* Store failed monitors here, otherwise the block below will cause them
+         * to be forgotten when a stop happens.
          */
         if (entry->failed) {
             lrmd_free_event(entry->failed);
diff --git a/fencing/remote.c b/fencing/remote.c
index 6c47f65..4a47d49 100644
--- a/fencing/remote.c
+++ b/fencing/remote.c
@@ -1558,13 +1558,13 @@ call_remote_stonith(remote_fencing_op_t * op, st_query_result_t * peer)
         }
 
         if (op->state == st_query) {
-           crm_info("None of the %d peers have devices capable of fencing (%s) %s for %s (%d)",
+           crm_info("No peers (out of %d) have devices capable of fencing (%s) %s for %s (%d)",
                    op->replies, op->action, op->target, op->client_name,
                    op->state);
 
             rc = -ENODEV;
         } else {
-           crm_info("None of the %d peers are capable of fencing (%s) %s for %s (%d)",
+           crm_info("No peers (out of %d) are capable of fencing (%s) %s for %s (%d)",
                    op->replies, op->action, op->target, op->client_name,
                    op->state);
         }
diff --git a/include/crm/pengine/status.h b/include/crm/pengine/status.h
index 79e4572..e748792 100644
--- a/include/crm/pengine/status.h
+++ b/include/crm/pengine/status.h
@@ -343,8 +343,8 @@ struct pe_action_s {
      * requires at minimum X number of cloned instances to be running
      * before an order dependency can run. Another option that uses
      * this is 'require-all=false' in ordering constrants. This option
-     * says "only required one instance of a resource to start before
-     * allowing dependencies to start" basicall require-all=false is
+     * says "only require one instance of a resource to start before
+     * allowing dependencies to start" -- basically, require-all=false is
      * the same as clone-min=1.
      */
 
@@ -354,8 +354,8 @@ struct pe_action_s {
      * to be considered runnable */ 
     int required_runnable_before;
 
-    GListPtr actions_before;    /* action_warpper_t* */
-    GListPtr actions_after;     /* action_warpper_t* */
+    GListPtr actions_before;    /* action_wrapper_t* */
+    GListPtr actions_after;     /* action_wrapper_t* */
 };
 
 struct ticket_s {
diff --git a/lib/cluster/election.c b/lib/cluster/election.c
index a8902d3..a5bb1da 100644
--- a/lib/cluster/election.c
+++ b/lib/cluster/election.c
@@ -461,7 +461,7 @@ election_count_vote(election_t *e, xmlNode *vote, bool can_win)
         int peers = 1 + g_hash_table_size(crm_peer_cache);
 
         /* If every node has to vote down every other node, thats N*(N-1) total elections
-         * Allow some leway before _really_ complaining
+         * Allow some leeway before _really_ complaining
          */
         election_wins++;
         if (election_wins > (peers * peers)) {
diff --git a/lib/common/xml.c b/lib/common/xml.c
index 6dce4cb..c566956 100644
--- a/lib/common/xml.c
+++ b/lib/common/xml.c
@@ -3190,7 +3190,7 @@ crm_xml_escape(const char *text)
      * converted back to their escape sequences.
      *
      * However xmlNodeDump() is randomly dog slow, even with the same
-     * input. So we need to replicate the escapeing in our custom
+     * input. So we need to replicate the escaping in our custom
      * version so that the result can be re-parsed by xmlCtxtReadDoc()
      * when necessary.
      */
diff --git a/lib/pengine/utils.c b/lib/pengine/utils.c
index 2b53999..74856fe 100644
--- a/lib/pengine/utils.c
+++ b/lib/pengine/utils.c
@@ -1099,8 +1099,8 @@ pe_free_action(action_t * action)
     if (action == NULL) {
         return;
     }
-    g_list_free_full(action->actions_before, free);     /* action_warpper_t* */
-    g_list_free_full(action->actions_after, free);      /* action_warpper_t* */
+    g_list_free_full(action->actions_before, free);     /* action_wrapper_t* */
+    g_list_free_full(action->actions_after, free);      /* action_wrapper_t* */
     if (action->extra) {
         g_hash_table_destroy(action->extra);
     }
diff --git a/lrmd/lrmd.c b/lrmd/lrmd.c
index 5669d34..c874aa5 100644
--- a/lrmd/lrmd.c
+++ b/lrmd/lrmd.c
@@ -1464,9 +1464,10 @@ free_rsc(gpointer data)
 
         if (is_stonith) {
             cmd->lrmd_op_status = PCMK_LRM_OP_CANCELLED;
-            /* if a stonith cmd is in-flight, mark just mark it as cancelled,
+            /* If a stonith command is in-flight, just mark it as cancelled;
              * it is not safe to finalize/free the cmd until the stonith api
-             * says it has either completed or timed out.*/ 
+             * says it has either completed or timed out.
+             */
             if (rsc->active != cmd) {
                 cmd_finalize(cmd, NULL);
             }
diff --git a/pengine/graph.c b/pengine/graph.c
index 81d8355..8098f9e 100644
--- a/pengine/graph.c
+++ b/pengine/graph.c
@@ -1005,7 +1005,7 @@ action2xml(action_t * action, gboolean as_input, pe_working_set_t *data_set)
                  *
                  * If anyone toggles the unique flag to 'on', the
                  * 'instance free' name will correspond to an orphan
-                 * and fall into the claus above instead
+                 * 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)) {
diff --git a/pengine/native.c b/pengine/native.c
index 52eba4f..a968fc7 100644
--- a/pengine/native.c
+++ b/pengine/native.c
@@ -1647,7 +1647,7 @@ colocation_match(resource_t * rsc_lh, resource_t * rsc_rh, rsc_colocation_t * co
 
     } else if (constraint->score < 0) {
         /* nothing to do:
-         *   anti-colocation with something thats not running
+         *   anti-colocation with something that is not running
          */
         return;
     }
diff --git a/tools/fake_transition.c b/tools/fake_transition.c
index 251f9bb..26f55a2 100644
--- a/tools/fake_transition.c
+++ b/tools/fake_transition.c
@@ -70,8 +70,8 @@ inject_transient_attr(xmlNode * cib_node, const char *name, const char *value)
     char *nvp_id = crm_concat(name, node_uuid, '-');
 
     node_path = xmlGetNodePath(cib_node);
-    quiet_log("Injecting attribute %s=%s into %s '%s'", name, value, node_path,
-             ID(cib_node));
+    quiet_log(" + Injecting attribute %s=%s into %s '%s'\n",
+              name, value, node_path, ID(cib_node));
     free(node_path);
 
     attrs = first_named_child(cib_node, XML_TAG_TRANSIENT_NODEATTRS);
-- 
1.8.3.1


From 9251c17d97250103679aada63729d54a9537874f Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 17 Mar 2017 16:58:48 -0500
Subject: [PATCH 2/5] Log: libcrmcluster,crmd: log join phase as text rather
 than integer

---
 crmd/join_dc.c        | 51 +++++++++++++++------------------------------------
 include/crm/cluster.h | 14 ++++++++++++++
 2 files changed, 29 insertions(+), 36 deletions(-)

diff --git a/crmd/join_dc.c b/crmd/join_dc.c
index d5ecc55..71311de 100644
--- a/crmd/join_dc.c
+++ b/crmd/join_dc.c
@@ -45,8 +45,9 @@ 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=%d", source, current_join_id, phase);
+        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;
     }
 
@@ -58,23 +59,21 @@ crm_update_peer_join(const char *source, crm_node_t * node, enum crm_join_phase
     last = node->join;
 
     if(phase == last) {
-        crm_trace("%s: Node %s[%u] - join-%u phase still %u",
-                  source, node->uname, node->id, current_join_id, last);
+        crm_trace("%s: Node %s[%u] - join-%u phase still %s",
+                  source, node->uname, node->id, current_join_id,
+                  crm_join_phase_str(last));
 
-    } else if (phase <= crm_join_none) {
+    } else if ((phase <= crm_join_none) || (phase == (last + 1))) {
         node->join = phase;
-        crm_info("%s: Node %s[%u] - join-%u phase %u -> %u",
-                 source, node->uname, node->id, current_join_id, last, phase);
+        crm_info("%s: Node %s[%u] - join-%u phase %s -> %s",
+                 source, node->uname, node->id, current_join_id,
+                 crm_join_phase_str(last), crm_join_phase_str(phase));
 
-    } else if(phase == last + 1) {
-        node->join = phase;
-        crm_info("%s: Node %s[%u] - join-%u phase %u -> %u",
-                 source, node->uname, node->id, current_join_id, last, phase);
     } else {
         crm_err("Could not update join for node %s because phase transition invalid "
-                CRM_XS " join-%u source=%s node_id=%u last=%u new=%u",
-                node->uname, current_join_id, source, node->id, last, phase);
-
+                CRM_XS " join-%u source=%s node_id=%u last=%s new=%s",
+                node->uname, current_join_id, source, node->id,
+                crm_join_phase_str(last), crm_join_phase_str(phase));
     }
 }
 
@@ -691,27 +690,7 @@ void crmd_join_phase_log(int level)
 
     g_hash_table_iter_init(&iter, crm_peer_cache);
     while (g_hash_table_iter_next(&iter, NULL, (gpointer *) &peer)) {
-        const char *state = "unknown";
-        switch(peer->join) {
-            case crm_join_nack:
-                state = "nack";
-                break;
-            case crm_join_none:
-                state = "none";
-                break;
-            case crm_join_welcomed:
-                state = "welcomed";
-                break;
-            case crm_join_integrated:
-                state = "integrated";
-                break;
-            case crm_join_finalized:
-                state = "finalized";
-                break;
-            case crm_join_confirmed:
-                state = "confirmed";
-                break;
-        }
-        do_crm_log(level, "join-%d: %s=%s", current_join_id, peer->uname, state);
+        do_crm_log(level, "join-%d: %s=%s", current_join_id, peer->uname,
+                   crm_join_phase_str(peer->join));
     }
 }
diff --git a/include/crm/cluster.h b/include/crm/cluster.h
index 27ee9eb..343ce08 100644
--- a/include/crm/cluster.h
+++ b/include/crm/cluster.h
@@ -235,4 +235,18 @@ char *pcmk_message_common_cs(cpg_handle_t handle, uint32_t nodeid, uint32_t pid,
                         uint32_t *kind, const char **from);
 #  endif
 
+static inline const char *
+crm_join_phase_str(enum crm_join_phase phase)
+{
+    switch (phase) {
+        case crm_join_nack:         return "nack";
+        case crm_join_none:         return "none";
+        case crm_join_welcomed:     return "welcomed";
+        case crm_join_integrated:   return "integrated";
+        case crm_join_finalized:    return "finalized";
+        case crm_join_confirmed:    return "confirmed";
+    }
+    return "invalid";
+}
+
 #endif
-- 
1.8.3.1


From dc977b614de9ce4018fa44c4f00c2b6b43c65c75 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 17 Mar 2017 17:23:22 -0500
Subject: [PATCH 3/5] Feature: libcrmcommon: add convenience functions for
 managing XML IDs

These handle ID sanitization and memory management for callers,
reducing the chance for mistakes.
---
 include/crm/common/xml.h |  4 ++++
 lib/common/xml.c         | 45 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/include/crm/common/xml.h b/include/crm/common/xml.h
index a948915..f896464 100644
--- a/include/crm/common/xml.h
+++ b/include/crm/common/xml.h
@@ -373,4 +373,8 @@ void save_xml_to_file(xmlNode * xml, const char *desc, const char *filename);
 char *xml_get_path(xmlNode *xml);
 
 char * crm_xml_escape(const char *text);
+void crm_xml_sanitize_id(char *id);
+void crm_xml_set_id(xmlNode *xml, const char *format, ...)
+    __attribute__ ((__format__ (__printf__, 2, 3)));
+
 #endif
diff --git a/lib/common/xml.c b/lib/common/xml.c
index c566956..0ea33d9 100644
--- a/lib/common/xml.c
+++ b/lib/common/xml.c
@@ -3036,6 +3036,51 @@ crm_xml_add_last_written(xmlNode *xml_node)
     return crm_xml_add(xml_node, XML_CIB_ATTR_WRITTEN, now_str);
 }
 
+/*!
+ * \brief Sanitize a string so it is usable as an XML ID
+ *
+ * \param[in,out] id  String to sanitize
+ */
+void
+crm_xml_sanitize_id(char *id)
+{
+    char *c;
+
+    for (c = id; *c; ++c) {
+        /* @TODO Sanitize more comprehensively */
+        switch (*c) {
+            case ':':
+            case '#':
+                *c = '.';
+        }
+    }
+}
+
+/*!
+ * \brief Set the ID of an XML element using a format
+ *
+ * \param[in,out] xml  XML element
+ * \param[in]     fmt  printf-style format
+ * \param[in]     ...  any arguments required by format
+ */
+void
+crm_xml_set_id(xmlNode *xml, const char *format, ...)
+{
+    va_list ap;
+    int len = 0;
+    char *id = NULL;
+
+    /* equivalent to crm_strdup_printf() */
+    va_start(ap, format);
+    len = vasprintf(&id, format, ap);
+    va_end(ap);
+    CRM_ASSERT(len > 0);
+
+    crm_xml_sanitize_id(id);
+    crm_xml_add(xml, XML_ATTR_ID, id);
+    free(id);
+}
+
 static int
 write_xml_stream(xmlNode * xml_node, const char *filename, FILE * stream, gboolean compress)
 {
-- 
1.8.3.1


From bf39184154405621a07ce05a201fded861db2357 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 17 Mar 2017 18:21:05 -0500
Subject: [PATCH 4/5] Refactor: various: use new convenience functions for
 generating XML IDs

---
 attrd/commands.c         | 41 +++++++++++-------------------------
 crmd/pengine.c           |  8 +++----
 lib/cluster/corosync.c   |  5 +----
 lib/pengine/unpack.c     | 37 ++++++++-------------------------
 tools/crm_node.c         | 12 +++--------
 tools/crm_resource_ban.c | 54 +++++++++++-------------------------------------
 tools/fake_transition.c  |  5 +----
 7 files changed, 41 insertions(+), 121 deletions(-)

diff --git a/attrd/commands.c b/attrd/commands.c
index 486efb6..98b4215 100644
--- a/attrd/commands.c
+++ b/attrd/commands.c
@@ -928,31 +928,9 @@ write_attributes(bool all, bool peer_discovered)
 static void
 build_update_element(xmlNode *parent, attribute_t *a, const char *nodeid, const char *value)
 {
-    char *set = NULL;
-    char *uuid = NULL;
+    const char *set = NULL;
     xmlNode *xml_obj = NULL;
 
-    if(a->set) {
-        set = strdup(a->set);
-    } else {
-        set = crm_strdup_printf("%s-%s", XML_CIB_TAG_STATUS, nodeid);
-    }
-
-    if(a->uuid) {
-        uuid = strdup(a->uuid);
-    } else {
-        int lpc;
-        uuid = crm_strdup_printf("%s-%s", set, a->id);
-
-        /* Minimal attempt at sanitizing automatic IDs */
-        for (lpc = 0; uuid[lpc] != 0; lpc++) {
-            switch (uuid[lpc]) {
-                case ':':
-                    uuid[lpc] = '.';
-            }
-        }
-    }
-
     xml_obj = create_xml_node(parent, XML_CIB_TAG_STATE);
     crm_xml_add(xml_obj, XML_ATTR_ID, nodeid);
 
@@ -960,10 +938,19 @@ build_update_element(xmlNode *parent, attribute_t *a, const char *nodeid, const
     crm_xml_add(xml_obj, XML_ATTR_ID, nodeid);
 
     xml_obj = create_xml_node(xml_obj, XML_TAG_ATTR_SETS);
-    crm_xml_add(xml_obj, XML_ATTR_ID, set);
+    if (a->set) {
+        crm_xml_set_id(xml_obj, "%s", a->set);
+    } else {
+        crm_xml_set_id(xml_obj, "%s-%s", XML_CIB_TAG_STATUS, nodeid);
+    }
+    set = ID(xml_obj);
 
     xml_obj = create_xml_node(xml_obj, XML_CIB_TAG_NVPAIR);
-    crm_xml_add(xml_obj, XML_ATTR_ID, uuid);
+    if (a->uuid) {
+        crm_xml_set_id(xml_obj, "%s", a->uuid);
+    } else {
+        crm_xml_set_id(xml_obj, "%s-%s", set, a->id);
+    }
     crm_xml_add(xml_obj, XML_NVPAIR_ATTR_NAME, a->id);
 
     if(value) {
@@ -973,9 +960,6 @@ build_update_element(xmlNode *parent, attribute_t *a, const char *nodeid, const
         crm_xml_add(xml_obj, XML_NVPAIR_ATTR_VALUE, "");
         crm_xml_add(xml_obj, "__delete__", XML_NVPAIR_ATTR_VALUE);
     }
-
-    free(uuid);
-    free(set);
 }
 
 void
diff --git a/crmd/pengine.c b/crmd/pengine.c
index 5a301c5..17613cf 100644
--- a/crmd/pengine.c
+++ b/crmd/pengine.c
@@ -264,12 +264,12 @@ force_local_option(xmlNode *xml, const char *attr_name, const char *attr_value)
     }
 
     if(max == 0) {
-        char *attr_id = crm_concat(CIB_OPTIONS_FIRST, attr_name, '-');
         xmlNode *configuration = NULL;
         xmlNode *crm_config = NULL;
         xmlNode *cluster_property_set = NULL;
 
-        crm_trace("Creating %s/%s = %s", attr_id, attr_name, attr_value);
+        crm_trace("Creating %s-%s for %s=%s",
+                  CIB_OPTIONS_FIRST, attr_name, attr_name, attr_value);
 
         configuration = find_entity(xml, XML_CIB_TAG_CONFIGURATION, NULL);
         if (configuration == NULL) {
@@ -289,11 +289,9 @@ force_local_option(xmlNode *xml, const char *attr_name, const char *attr_value)
 
         xml = create_xml_node(cluster_property_set, XML_CIB_TAG_NVPAIR);
 
-        crm_xml_add(xml, XML_ATTR_ID, attr_id);
+        crm_xml_set_id(xml, "%s-%s", CIB_OPTIONS_FIRST, attr_name);
         crm_xml_add(xml, XML_NVPAIR_ATTR_NAME, attr_name);
         crm_xml_add(xml, XML_NVPAIR_ATTR_VALUE, attr_value);
-
-        free(attr_id);
     }
     freeXpathObject(xpathObj);
 }
diff --git a/lib/cluster/corosync.c b/lib/cluster/corosync.c
index b02ae73..dfc781c 100644
--- a/lib/cluster/corosync.c
+++ b/lib/cluster/corosync.c
@@ -548,12 +548,9 @@ corosync_initialize_nodelist(void *cluster, gboolean force_member, xmlNode * xml
             any = TRUE;
 
             if (xml_parent) {
-                char buffer[64];
                 xmlNode *node = create_xml_node(xml_parent, XML_CIB_TAG_NODE);
 
-                if(snprintf(buffer, 63, "%u", nodeid) > 0) {
-                    crm_xml_add(node, XML_ATTR_ID, buffer);
-                }
+                crm_xml_set_id(node, "%u", nodeid);
                 crm_xml_add(node, XML_ATTR_UNAME, name);
                 if (force_member) {
                     crm_xml_add(node, XML_ATTR_TYPE, CRM_NODE_MEMBER);
diff --git a/lib/pengine/unpack.c b/lib/pengine/unpack.c
index e6a8f58..57aa5f2 100644
--- a/lib/pengine/unpack.c
+++ b/lib/pengine/unpack.c
@@ -373,7 +373,6 @@ expand_remote_rsc_meta(xmlNode *xml_obj, xmlNode *parent, GHashTable **rsc_name_
     const char *remote_port = NULL;
     const char *connect_timeout = "60s";
     const char *remote_allow_migrate=NULL;
-    char *tmp_id = NULL;
 
     for (attr_set = __xml_first_child(xml_obj); attr_set != NULL; attr_set = __xml_next_element(attr_set)) {
         if (safe_str_neq((const char *)attr_set->name, XML_TAG_META_SETS)) {
@@ -427,73 +426,55 @@ expand_remote_rsc_meta(xmlNode *xml_obj, xmlNode *parent, GHashTable **rsc_name_
     crm_xml_add(xml_rsc, XML_ATTR_TYPE, "remote");
 
     xml_tmp = create_xml_node(xml_rsc, XML_TAG_META_SETS);
-    tmp_id = crm_concat(remote_name, XML_TAG_META_SETS, '_');
-    crm_xml_add(xml_tmp, XML_ATTR_ID, tmp_id);
-    free(tmp_id);
+    crm_xml_set_id(xml_tmp, "%s_%s", remote_name, XML_TAG_META_SETS);
 
     attr = create_xml_node(xml_tmp, XML_CIB_TAG_NVPAIR);
-    tmp_id = crm_concat(remote_name, "meta-attributes-container", '_');
-    crm_xml_add(attr, XML_ATTR_ID, tmp_id);
+    crm_xml_set_id(attr, "%s_%s", remote_name, "meta-attributes-container");
     crm_xml_add(attr, XML_NVPAIR_ATTR_NAME, XML_RSC_ATTR_CONTAINER);
     crm_xml_add(attr, XML_NVPAIR_ATTR_VALUE, container_id);
-    free(tmp_id);
 
     attr = create_xml_node(xml_tmp, XML_CIB_TAG_NVPAIR);
-    tmp_id = crm_concat(remote_name, "meta-attributes-internal", '_');
-    crm_xml_add(attr, XML_ATTR_ID, tmp_id);
+    crm_xml_set_id(attr, "%s_%s", remote_name, "meta-attributes-internal");
     crm_xml_add(attr, XML_NVPAIR_ATTR_NAME, XML_RSC_ATTR_INTERNAL_RSC);
     crm_xml_add(attr, XML_NVPAIR_ATTR_VALUE, "true");
-    free(tmp_id);
 
     if (remote_allow_migrate) {
         attr = create_xml_node(xml_tmp, XML_CIB_TAG_NVPAIR);
-        tmp_id = crm_concat(remote_name, "meta-attributes-container", '_');
-        crm_xml_add(attr, XML_ATTR_ID, tmp_id);
+        crm_xml_set_id(attr, "%s_%s", remote_name, "meta-attributes-container");
         crm_xml_add(attr, XML_NVPAIR_ATTR_NAME, XML_OP_ATTR_ALLOW_MIGRATE);
         crm_xml_add(attr, XML_NVPAIR_ATTR_VALUE, remote_allow_migrate);
-        free(tmp_id);
     }
 
     xml_tmp = create_xml_node(xml_rsc, "operations");
     attr = create_xml_node(xml_tmp, XML_ATTR_OP);
-    tmp_id = crm_concat(remote_name, "monitor-interval-30s", '_');
-    crm_xml_add(attr, XML_ATTR_ID, tmp_id);
+    crm_xml_set_id(attr, "%s_%s", remote_name, "monitor-interval-30s");
     crm_xml_add(attr, XML_ATTR_TIMEOUT, "30s");
     crm_xml_add(attr, XML_LRM_ATTR_INTERVAL, "30s");
     crm_xml_add(attr, XML_NVPAIR_ATTR_NAME, "monitor");
-    free(tmp_id);
 
     if (connect_timeout) {
         attr = create_xml_node(xml_tmp, XML_ATTR_OP);
-        tmp_id = crm_concat(remote_name, "start-interval-0", '_');
-        crm_xml_add(attr, XML_ATTR_ID, tmp_id);
+        crm_xml_set_id(attr, "%s_%s", remote_name, "start-interval-0");
         crm_xml_add(attr, XML_ATTR_TIMEOUT, connect_timeout);
         crm_xml_add(attr, XML_LRM_ATTR_INTERVAL, "0");
         crm_xml_add(attr, XML_NVPAIR_ATTR_NAME, "start");
-        free(tmp_id);
     }
 
     if (remote_port || remote_server) {
         xml_tmp = create_xml_node(xml_rsc, XML_TAG_ATTR_SETS);
-        tmp_id = crm_concat(remote_name, XML_TAG_ATTR_SETS, '_');
-        crm_xml_add(xml_tmp, XML_ATTR_ID, tmp_id);
-        free(tmp_id);
+        crm_xml_set_id(xml_tmp, "%s_%s", remote_name, XML_TAG_ATTR_SETS);
 
         if (remote_server) {
             attr = create_xml_node(xml_tmp, XML_CIB_TAG_NVPAIR);
-            tmp_id = crm_concat(remote_name, "instance-attributes-addr", '_');
-            crm_xml_add(attr, XML_ATTR_ID, tmp_id);
+            crm_xml_set_id(attr, "%s_%s", remote_name, "instance-attributes-addr");
             crm_xml_add(attr, XML_NVPAIR_ATTR_NAME, "addr");
             crm_xml_add(attr, XML_NVPAIR_ATTR_VALUE, remote_server);
-            free(tmp_id);
         }
         if (remote_port) {
             attr = create_xml_node(xml_tmp, XML_CIB_TAG_NVPAIR);
-            tmp_id = crm_concat(remote_name, "instance-attributes-port", '_');
-            crm_xml_add(attr, XML_ATTR_ID, tmp_id);
+            crm_xml_set_id(attr, "%s_%s", remote_name, "instance-attributes-port");
             crm_xml_add(attr, XML_NVPAIR_ATTR_NAME, "port");
             crm_xml_add(attr, XML_NVPAIR_ATTR_VALUE, remote_port);
-            free(tmp_id);
         }
     }
 
diff --git a/tools/crm_node.c b/tools/crm_node.c
index 7092db4..9b60c55 100644
--- a/tools/crm_node.c
+++ b/tools/crm_node.c
@@ -105,11 +105,8 @@ cib_remove_node(uint32_t id, const char *name)
     crm_xml_add(node, XML_ATTR_UNAME, name);
     crm_xml_add(node_state, XML_ATTR_UNAME, name);
     if(id) {
-        char buffer[64];
-        if(snprintf(buffer, 63, "%u", id) > 0) {
-            crm_xml_add(node, XML_ATTR_ID, buffer);
-            crm_xml_add(node_state, XML_ATTR_ID, buffer);
-        }
+        crm_xml_set_id(node, "%u", id);
+        crm_xml_add(node_state, XML_ATTR_ID, ID(node));
     }
 
     cib = cib_new();
@@ -200,10 +197,7 @@ int tools_remove_node_cache(const char *node, const char *target)
         cmd = create_request(CRM_OP_RM_NODE_CACHE,
                              NULL, NULL, target, crm_system_name, admin_uuid);
         if (n) {
-            char buffer[64];
-            if(snprintf(buffer, 63, "%u", n) > 0) {
-                crm_xml_add(cmd, XML_ATTR_ID, buffer);
-            }
+            crm_xml_set_id(cmd, "%u", n);
         }
         crm_xml_add(cmd, XML_ATTR_UNAME, name);
     }
diff --git a/tools/crm_resource_ban.c b/tools/crm_resource_ban.c
index 2f7b366..69b49bf 100644
--- a/tools/crm_resource_ban.c
+++ b/tools/crm_resource_ban.c
@@ -62,7 +62,6 @@ cli_resource_ban(const char *rsc_id, const char *host, GListPtr allnodes, cib_t
 {
     char *later_s = NULL;
     int rc = pcmk_ok;
-    char *id = NULL;
     xmlNode *fragment = NULL;
     xmlNode *location = NULL;
 
@@ -83,10 +82,8 @@ cli_resource_ban(const char *rsc_id, const char *host, GListPtr allnodes, cib_t
 
     fragment = create_xml_node(NULL, XML_CIB_TAG_CONSTRAINTS);
 
-    id = crm_strdup_printf("cli-ban-%s-on-%s", rsc_id, host);
     location = create_xml_node(fragment, XML_CONS_TAG_RSC_LOCATION);
-    crm_xml_add(location, XML_ATTR_ID, id);
-    free(id);
+    crm_xml_set_id(location, "cli-ban-%s-on-%s", rsc_id, host);
 
     if (BE_QUIET == FALSE) {
         CMD_ERR("WARNING: Creating rsc_location constraint '%s'"
@@ -117,27 +114,18 @@ cli_resource_ban(const char *rsc_id, const char *host, GListPtr allnodes, cib_t
         xmlNode *rule = create_xml_node(location, XML_TAG_RULE);
         xmlNode *expr = create_xml_node(rule, XML_TAG_EXPRESSION);
 
-        id = crm_strdup_printf("cli-ban-%s-on-%s-rule", rsc_id, host);
-        crm_xml_add(rule, XML_ATTR_ID, id);
-        free(id);
-
+        crm_xml_set_id(rule, "cli-ban-%s-on-%s-rule", rsc_id, host);
         crm_xml_add(rule, XML_RULE_ATTR_SCORE, MINUS_INFINITY_S);
         crm_xml_add(rule, XML_RULE_ATTR_BOOLEAN_OP, "and");
 
-        id = crm_strdup_printf("cli-ban-%s-on-%s-expr", rsc_id, host);
-        crm_xml_add(expr, XML_ATTR_ID, id);
-        free(id);
-
+        crm_xml_set_id(expr, "cli-ban-%s-on-%s-expr", rsc_id, host);
         crm_xml_add(expr, XML_EXPR_ATTR_ATTRIBUTE, "#uname");
         crm_xml_add(expr, XML_EXPR_ATTR_OPERATION, "eq");
         crm_xml_add(expr, XML_EXPR_ATTR_VALUE, host);
         crm_xml_add(expr, XML_EXPR_ATTR_TYPE, "string");
 
         expr = create_xml_node(rule, "date_expression");
-        id = crm_strdup_printf("cli-ban-%s-on-%s-lifetime", rsc_id, host);
-        crm_xml_add(expr, XML_ATTR_ID, id);
-        free(id);
-
+        crm_xml_set_id(expr, "cli-ban-%s-on-%s-lifetime", rsc_id, host);
         crm_xml_add(expr, "operation", "lt");
         crm_xml_add(expr, "end", later_s);
     }
@@ -156,7 +144,6 @@ cli_resource_prefer(const char *rsc_id, const char *host, cib_t * cib_conn)
 {
     char *later_s = parse_cli_lifetime(move_lifetime);
     int rc = pcmk_ok;
-    char *id = NULL;
     xmlNode *location = NULL;
     xmlNode *fragment = NULL;
 
@@ -171,10 +158,8 @@ cli_resource_prefer(const char *rsc_id, const char *host, cib_t * cib_conn)
 
     fragment = create_xml_node(NULL, XML_CIB_TAG_CONSTRAINTS);
 
-    id = crm_strdup_printf("cli-prefer-%s", rsc_id);
     location = create_xml_node(fragment, XML_CONS_TAG_RSC_LOCATION);
-    crm_xml_add(location, XML_ATTR_ID, id);
-    free(id);
+    crm_xml_set_id(location, "cli-prefer-%s", rsc_id);
 
     crm_xml_add(location, XML_LOC_ATTR_SOURCE, rsc_id);
     if(scope_master) {
@@ -192,27 +177,18 @@ cli_resource_prefer(const char *rsc_id, const char *host, cib_t * cib_conn)
         xmlNode *rule = create_xml_node(location, XML_TAG_RULE);
         xmlNode *expr = create_xml_node(rule, XML_TAG_EXPRESSION);
 
-        id = crm_concat("cli-prefer-rule", rsc_id, '-');
-        crm_xml_add(rule, XML_ATTR_ID, id);
-        free(id);
-
+        crm_xml_set_id(rule, "cli-prefer-rule-%s", rsc_id);
         crm_xml_add(rule, XML_RULE_ATTR_SCORE, INFINITY_S);
         crm_xml_add(rule, XML_RULE_ATTR_BOOLEAN_OP, "and");
 
-        id = crm_concat("cli-prefer-expr", rsc_id, '-');
-        crm_xml_add(expr, XML_ATTR_ID, id);
-        free(id);
-
+        crm_xml_set_id(expr, "cli-prefer-expr-%s", rsc_id);
         crm_xml_add(expr, XML_EXPR_ATTR_ATTRIBUTE, "#uname");
         crm_xml_add(expr, XML_EXPR_ATTR_OPERATION, "eq");
         crm_xml_add(expr, XML_EXPR_ATTR_VALUE, host);
         crm_xml_add(expr, XML_EXPR_ATTR_TYPE, "string");
 
         expr = create_xml_node(rule, "date_expression");
-        id = crm_concat("cli-prefer-lifetime-end", rsc_id, '-');
-        crm_xml_add(expr, XML_ATTR_ID, id);
-        free(id);
-
+        crm_xml_set_id(expr, "cli-prefer-lifetime-end-%s", rsc_id);
         crm_xml_add(expr, "operation", "lt");
         crm_xml_add(expr, "end", later_s);
     }
@@ -228,7 +204,6 @@ cli_resource_prefer(const char *rsc_id, const char *host, cib_t * cib_conn)
 int
 cli_resource_clear(const char *rsc_id, const char *host, GListPtr allnodes, cib_t * cib_conn)
 {
-    char *id = NULL;
     int rc = pcmk_ok;
     xmlNode *fragment = NULL;
     xmlNode *location = NULL;
@@ -240,30 +215,25 @@ cli_resource_clear(const char *rsc_id, const char *host, GListPtr allnodes, cib_
     fragment = create_xml_node(NULL, XML_CIB_TAG_CONSTRAINTS);
 
     if(host) {
-        id = crm_strdup_printf("cli-ban-%s-on-%s", rsc_id, host);
         location = create_xml_node(fragment, XML_CONS_TAG_RSC_LOCATION);
-        crm_xml_add(location, XML_ATTR_ID, id);
-        free(id);
+        crm_xml_set_id(location, "cli-ban-%s-on-%s", rsc_id, host);
 
     } else {
         GListPtr n = allnodes;
         for(; n; n = n->next) {
             node_t *target = n->data;
 
-            id = crm_strdup_printf("cli-ban-%s-on-%s", rsc_id, target->details->uname);
             location = create_xml_node(fragment, XML_CONS_TAG_RSC_LOCATION);
-            crm_xml_add(location, XML_ATTR_ID, id);
-            free(id);
+            crm_xml_set_id(location, "cli-ban-%s-on-%s",
+                           rsc_id, target->details->uname);
         }
     }
 
-    id = crm_strdup_printf("cli-prefer-%s", rsc_id);
     location = create_xml_node(fragment, XML_CONS_TAG_RSC_LOCATION);
-    crm_xml_add(location, XML_ATTR_ID, id);
+    crm_xml_set_id(location, "cli-prefer-%s", rsc_id);
     if(host && do_force == FALSE) {
         crm_xml_add(location, XML_CIB_TAG_NODE, host);
     }
-    free(id);
 
     crm_log_xml_info(fragment, "Delete");
     rc = cib_conn->cmds->delete(cib_conn, XML_CIB_TAG_CONSTRAINTS, fragment, cib_options);
diff --git a/tools/fake_transition.c b/tools/fake_transition.c
index 26f55a2..9f9eaed 100644
--- a/tools/fake_transition.c
+++ b/tools/fake_transition.c
@@ -67,7 +67,6 @@ inject_transient_attr(xmlNode * cib_node, const char *name, const char *value)
     xmlNode *nvp = NULL;
     xmlChar *node_path;
     const char *node_uuid = ID(cib_node);
-    char *nvp_id = crm_concat(name, node_uuid, '-');
 
     node_path = xmlGetNodePath(cib_node);
     quiet_log(" + Injecting attribute %s=%s into %s '%s'\n",
@@ -87,11 +86,9 @@ inject_transient_attr(xmlNode * cib_node, const char *name, const char *value)
     }
 
     nvp = create_xml_node(container, XML_CIB_TAG_NVPAIR);
-    crm_xml_add(nvp, XML_ATTR_ID, nvp_id);
+    crm_xml_set_id(nvp, "%s-%s", name, node_uuid);
     crm_xml_add(nvp, XML_NVPAIR_ATTR_NAME, name);
     crm_xml_add(nvp, XML_NVPAIR_ATTR_VALUE, value);
-
-    free(nvp_id);
 }
 
 static void
-- 
1.8.3.1


From c37145733538ee92153dc26e5b40c6fcfad47fdb Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Mon, 20 Mar 2017 10:52:42 -0500
Subject: [PATCH 5/5] Refactor: libpe_status: add convenience functions for
 clone detection

---
 include/crm/pengine/status.h | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/include/crm/pengine/status.h b/include/crm/pengine/status.h
index e748792..ce3236b 100644
--- a/include/crm/pengine/status.h
+++ b/include/crm/pengine/status.h
@@ -19,6 +19,7 @@
 #  define PENGINE_STATUS__H
 
 #  include <glib.h>
+#  include <stdbool.h>
 #  include <crm/common/iso8601.h>
 #  include <crm/pengine/common.h>
 
@@ -433,4 +434,44 @@ node_t *pe_find_node_id(GListPtr node_list, const char *id);
 node_t *pe_find_node_any(GListPtr node_list, const char *id, const char *uname);
 GListPtr find_operations(const char *rsc, const char *node, gboolean active_filter,
                          pe_working_set_t * data_set);
+
+/*!
+ * \brief Check whether a resource is any clone type
+ *
+ * \param[in] rsc  Resource to check
+ *
+ * \return TRUE if resource is clone, FALSE otherwise
+ */
+static inline bool
+pe_rsc_is_clone(resource_t *rsc)
+{
+    return rsc && ((rsc->variant == pe_clone) || (rsc->variant == pe_master));
+}
+
+/*!
+ * \brief Check whether a resource is a globally unique clone
+ *
+ * \param[in] rsc  Resource to check
+ *
+ * \return TRUE if resource is unique clone, FALSE otherwise
+ */
+static inline bool
+pe_rsc_is_unique_clone(resource_t *rsc)
+{
+    return pe_rsc_is_clone(rsc) && is_set(rsc->flags, pe_rsc_unique);
+}
+
+/*!
+ * \brief Check whether a resource is an anonymous clone
+ *
+ * \param[in] rsc  Resource to check
+ *
+ * \return TRUE if resource is anonymous clone, FALSE otherwise
+ */
+static inline bool
+pe_rsc_is_anon_clone(resource_t *rsc)
+{
+    return pe_rsc_is_clone(rsc) && is_not_set(rsc->flags, pe_rsc_unique);
+}
+
 #endif
-- 
1.8.3.1