Blob Blame History Raw
From 07fc025fb0998ed7f3b0bbfb10a6740e420cae31 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Tue, 20 Jun 2017 15:47:38 -0500
Subject: [PATCH 1/2] Fix: libpe_status: disallow resources on bundle nodes

---
 lib/pengine/container.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/lib/pengine/container.c b/lib/pengine/container.c
index 836b482..c37f421 100644
--- a/lib/pengine/container.c
+++ b/lib/pengine/container.c
@@ -342,6 +342,7 @@ create_remote_resource(
 {
     if (tuple->child && valid_network(data)) {
         GHashTableIter gIter;
+        GListPtr rsc_iter = NULL;
         node_t *node = NULL;
         xmlNode *xml_obj = NULL;
         xmlNode *xml_remote = NULL;
@@ -402,6 +403,26 @@ create_remote_resource(
             node->weight = -INFINITY;
         }
 
+        /* unpack_remote_nodes() ensures that each remote node and guest node
+         * has a node_t entry. Ideally, it would do the same for bundle nodes.
+         * Unfortunately, a bundle has to be mostly unpacked before it's obvious
+         * what nodes will be needed, so we do it just above.
+         *
+         * Worse, that means that the node may have been utilized while
+         * unpacking other resources, without our weight correction. The most
+         * likely place for this to happen is when common_unpack() calls
+         * resource_location() to set a default score in symmetric clusters.
+         * This adds a node *copy* to each resource's allowed nodes, and these
+         * copies will have the wrong weight.
+         *
+         * As a hacky workaround, clear those copies here.
+         */
+        for (rsc_iter = data_set->resources; rsc_iter; rsc_iter = rsc_iter->next) {
+            resource_t *rsc = (resource_t *) rsc_iter->data;
+
+            g_hash_table_remove(rsc->allowed_nodes, nodeid);
+        }
+
         tuple->node = node_copy(node);
         tuple->node->weight = 500;
         nodeid = NULL;
-- 
1.8.3.1


From ac906cbf1b46c0037fed634158874679824ce635 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Tue, 20 Jun 2017 20:23:12 -0500
Subject: [PATCH 2/2] Fix: libpe_status: avoid memory leaks when creating
 bundle remote resource

---
 lib/pengine/container.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/lib/pengine/container.c b/lib/pengine/container.c
index c37f421..e916446 100644
--- a/lib/pengine/container.c
+++ b/lib/pengine/container.c
@@ -346,22 +346,28 @@ create_remote_resource(
         node_t *node = NULL;
         xmlNode *xml_obj = NULL;
         xmlNode *xml_remote = NULL;
-        char *nodeid = crm_strdup_printf("%s-%d", data->prefix, tuple->offset);
-        char *id = NULL;
+        char *id = crm_strdup_printf("%s-%d", data->prefix, tuple->offset);
+        const char *uname = NULL;
 
-        if (remote_id_conflict(nodeid, data_set)) {
+        if (remote_id_conflict(id, data_set)) {
+            free(id);
             // The biggest hammer we have
             id = crm_strdup_printf("pcmk-internal-%s-remote-%d", tuple->child->id, tuple->offset);
             CRM_ASSERT(remote_id_conflict(id, data_set) == FALSE);
-        } else {
-            id = strdup(nodeid);
         }
 
         xml_remote = create_resource(id, "pacemaker", "remote");
+
+        /* Abandon our created ID, and pull the copy from the XML, because we
+         * need something that will get freed during data set cleanup to use as
+         * the node ID and uname.
+         */
         free(id);
+        id = NULL;
+        uname = ID(xml_remote);
 
         xml_obj = create_xml_node(xml_remote, "operations");
-        create_op(xml_obj, ID(xml_remote), "monitor", "60s");
+        create_op(xml_obj, uname, "monitor", "60s");
 
         xml_obj = create_xml_node(xml_remote, XML_TAG_ATTR_SETS);
         crm_xml_set_id(xml_obj, "%s-attributes-%d", data->prefix, tuple->offset);
@@ -376,7 +382,10 @@ create_remote_resource(
         if(data->control_port) {
             create_nvp(xml_obj, "port", data->control_port);
         } else {
-            create_nvp(xml_obj, "port", crm_itoa(DEFAULT_REMOTE_PORT));
+            char *port_s = crm_itoa(DEFAULT_REMOTE_PORT);
+
+            create_nvp(xml_obj, "port", port_s);
+            free(port_s);
         }
 
         xml_obj = create_xml_node(xml_remote, XML_TAG_META_SETS);
@@ -395,9 +404,9 @@ create_remote_resource(
          * been, if it has a permanent node attribute), and ensure its weight is
          * -INFINITY so no other resources can run on it.
          */
-        node = pe_find_node(data_set->nodes, nodeid);
+        node = pe_find_node(data_set->nodes, uname);
         if (node == NULL) {
-            node = pe_create_node(strdup(nodeid), nodeid, "remote", "-INFINITY",
+            node = pe_create_node(uname, uname, "remote", "-INFINITY",
                                   data_set);
         } else {
             node->weight = -INFINITY;
@@ -420,13 +429,11 @@ create_remote_resource(
         for (rsc_iter = data_set->resources; rsc_iter; rsc_iter = rsc_iter->next) {
             resource_t *rsc = (resource_t *) rsc_iter->data;
 
-            g_hash_table_remove(rsc->allowed_nodes, nodeid);
+            g_hash_table_remove(rsc->allowed_nodes, uname);
         }
 
         tuple->node = node_copy(node);
         tuple->node->weight = 500;
-        nodeid = NULL;
-        id = NULL;
 
         if (common_unpack(xml_remote, &tuple->remote, parent, data_set) == FALSE) {
             return FALSE;
-- 
1.8.3.1