From d4954fccb6fa02d2cc37d0ee6e5dbe7a24fb0048 Mon Sep 17 00:00:00 2001
From: Chris Lumens <clumens@redhat.com>
Date: Fri, 11 Jan 2019 12:18:45 -0500
Subject: [PATCH 2/4] Bug: tools: Clear all prefer constraints when performing
 a move

A move is implemented in terms of perfer constraints.  If those
constraints contain something like a lifetime expression, and older
prefer constraints are not cleared out, the result is a mess.  The XML
that is attempted to insert into the CIB will contain both the older
constraint and then the new lifetime expression as sub-nodes of that
constraint.  This is invalid, so the CIB will throw it out.

The fix is to make sure there are no prefer constraints for any nodes
when a move is done.

Most ban constraints are left alone, because they may still be valid -
you may want to move a resource to one node while preserving the ban on
another node.  Taking care of this is the bulk of the complexity in this
patch.

One further note - any ban constraints on the destination still need to
be removed.  Having both a ban and a prefer constraint on the same node
may technically be valid XML, but doesn't make any sense.

See rhbz#1648620
---
 tools/crm_resource.c         |  4 ++--
 tools/crm_resource.h         |  3 ++-
 tools/crm_resource_ban.c     | 17 +++++++++++------
 tools/crm_resource_runtime.c | 11 +++++++----
 4 files changed, 22 insertions(+), 13 deletions(-)

Index: pacemaker-1.1.16+20170320.77ea74d/tools/crm_resource.c
===================================================================
--- pacemaker-1.1.16+20170320.77ea74d.orig/tools/crm_resource.c
+++ pacemaker-1.1.16+20170320.77ea74d/tools/crm_resource.c
@@ -967,10 +967,10 @@ main(int argc, char **argv)
                 rc = -ENXIO;
                 goto bail;
             }
-            rc = cli_resource_clear(rsc_id, dest->details->uname, NULL, cib_conn);
+            rc = cli_resource_clear(rsc_id, dest->details->uname, NULL, cib_conn, TRUE);
 
         } else {
-            rc = cli_resource_clear(rsc_id, NULL, data_set.nodes, cib_conn);
+            rc = cli_resource_clear(rsc_id, NULL, data_set.nodes, cib_conn, TRUE);
         }
 
     } else if (rsc_cmd == 'M' && host_uname) {
Index: pacemaker-1.1.16+20170320.77ea74d/tools/crm_resource.h
===================================================================
--- pacemaker-1.1.16+20170320.77ea74d.orig/tools/crm_resource.h
+++ pacemaker-1.1.16+20170320.77ea74d/tools/crm_resource.h
@@ -52,7 +52,8 @@ resource_t *find_rsc_or_clone(const char
 /* ban */
 int cli_resource_prefer(const char *rsc_id, const char *host, cib_t * cib_conn);
 int cli_resource_ban(const char *rsc_id, const char *host, GListPtr allnodes, cib_t * cib_conn);
-int cli_resource_clear(const char *rsc_id, const char *host, GListPtr allnodes, cib_t * cib_conn);
+int cli_resource_clear(const char *rsc_id, const char *host, GListPtr allnodes, cib_t * cib_conn,
+                       bool clear_ban_constraints);
 
 /* print */
 void cli_resource_print_cts(resource_t * rsc);
Index: pacemaker-1.1.16+20170320.77ea74d/tools/crm_resource_ban.c
===================================================================
--- pacemaker-1.1.16+20170320.77ea74d.orig/tools/crm_resource_ban.c
+++ pacemaker-1.1.16+20170320.77ea74d/tools/crm_resource_ban.c
@@ -238,15 +238,19 @@ resource_clear_node_in_expr(const char *
 }
 
 static int
-resource_clear_node_in_location(const char *rsc_id, const char *host, cib_t * cib_conn)
+resource_clear_node_in_location(const char *rsc_id, const char *host, cib_t * cib_conn,
+                                bool clear_ban_constraints)
 {
     int rc = pcmk_ok;
     xmlNode *fragment = NULL;
     xmlNode *location = NULL;
 
     fragment = create_xml_node(NULL, XML_CIB_TAG_CONSTRAINTS);
-    location = create_xml_node(fragment, XML_CONS_TAG_RSC_LOCATION);
-    crm_xml_set_id(location, "cli-ban-%s-on-%s", rsc_id, host);
+
+    if (clear_ban_constraints == TRUE) {
+        location = create_xml_node(fragment, XML_CONS_TAG_RSC_LOCATION);
+        crm_xml_set_id(location, "cli-ban-%s-on-%s", rsc_id, host);
+    }
 
     location = create_xml_node(fragment, XML_CONS_TAG_RSC_LOCATION);
     crm_xml_set_id(location, "cli-prefer-%s", rsc_id);
@@ -265,7 +269,8 @@ resource_clear_node_in_location(const ch
 }
 
 int
-cli_resource_clear(const char *rsc_id, const char *host, GListPtr allnodes, cib_t * cib_conn)
+cli_resource_clear(const char *rsc_id, const char *host, GListPtr allnodes, cib_t * cib_conn,
+                   bool clear_ban_constraints)
 {
     int rc = pcmk_ok;
 
@@ -281,7 +286,7 @@ cli_resource_clear(const char *rsc_id, c
          * to try the second clear method.
          */
         if (rc == pcmk_ok) {
-            rc = resource_clear_node_in_location(rsc_id, host, cib_conn);
+            rc = resource_clear_node_in_location(rsc_id, host, cib_conn, clear_ban_constraints);
         }
 
     } else {
@@ -293,7 +298,7 @@ cli_resource_clear(const char *rsc_id, c
         for(; n; n = n->next) {
             node_t *target = n->data;
 
-            rc = cli_resource_clear(rsc_id, target->details->uname, NULL, cib_conn);
+            rc = cli_resource_clear(rsc_id, target->details->uname, NULL, cib_conn, clear_ban_constraints);
             if (rc != pcmk_ok) {
                 break;
             }
Index: pacemaker-1.1.16+20170320.77ea74d/tools/crm_resource_runtime.c
===================================================================
--- pacemaker-1.1.16+20170320.77ea74d.orig/tools/crm_resource_runtime.c
+++ pacemaker-1.1.16+20170320.77ea74d/tools/crm_resource_runtime.c
@@ -1247,7 +1247,7 @@ cli_resource_restart(resource_t * rsc, c
     }
 
     if(is_clone && host) {
-        rc = cli_resource_clear(rsc_id, host, NULL, cib);
+        rc = cli_resource_clear(rsc_id, host, NULL, cib, TRUE);
 
     } else if (orig_target_role) {
         rc = cli_resource_update_attribute(rsc_id, NULL, NULL,
@@ -1328,7 +1328,7 @@ cli_resource_restart(resource_t * rsc, c
 
   failure:
     if(is_clone && host) {
-        cli_resource_clear(rsc_id, host, NULL, cib);
+        cli_resource_clear(rsc_id, host, NULL, cib, TRUE);
     } else if (orig_target_role) {
         cli_resource_update_attribute(rsc_id, NULL, NULL,
                                       XML_RSC_ATTR_TARGET_ROLE,
@@ -1739,8 +1739,11 @@ cli_resource_move(const char *rsc_id, co
         }
     }
 
-    /* Clear any previous constraints for 'dest' */
-    cli_resource_clear(rsc_id, dest->details->uname, data_set->nodes, cib);
+    /* Clear any previous prefer constraints across all nodes. */
+    cli_resource_clear(rsc_id, NULL, data_set->nodes, cib, FALSE);
+
+    /* Clear any previous ban constraints on 'dest'. */
+    cli_resource_clear(rsc_id, dest->details->uname, data_set->nodes, cib, TRUE);
 
     /* Record an explicit preference for 'dest' */
     rc = cli_resource_prefer(rsc_id, dest->details->uname, cib);
