File bsc#1127716-0001-Fix-libcrmcommon-correctly-app... of Package pacemaker.13547

Overview Repositories Revisions Requests Users Attributes Meta

File bsc#1127716-0001-Fix-libcrmcommon-correctly-apply-XML-diffs-with-mult-1.1.patch of Package pacemaker.13547

From 96244e2c9bb4f32dc38bd4fa1c091c9f0d5bbf57 Mon Sep 17 00:00:00 2001
From: "Gao,Yan" <ygao@suse.com>
Date: Fri, 26 Apr 2019 11:52:59 +0200
Subject: [PATCH 1/3] Fix: libcrmcommon: correctly apply XML diffs with
 multiple move/create changes

Given a resource group:
```
<group id="dummies">
  <primitive id="dummy0"/>
  <primitive id="dummy1"/>
  <primitive id="dummy2"/>
  <primitive id="dummy3"/>
  <primitive id="dummy4"/>
</group>
```

, if we'd like to change it to:
```
<group id="dummies">
  <primitive id="dummy3"/>
  <primitive id="dummy4"/>
  <primitive id="dummy2"/>
  <primitive id="dummy0"/>
  <primitive id="dummy1"/>
</group>
```

, the generated XML diff would be like:
```
<diff format="2">
  <change operation="move" path="//primitive[@id=dummy3]" position="0"/>
  <change operation="move" path="//primitive[@id=dummy4]" position="1"/>
  <change operation="move" path="//primitive[@id=dummy0]" position="3"/>
  <change operation="move" path="//primitive[@id=dummy1]" position="4"/>
</diff>
```

Previously after applying the XML diff, the resulting XML would be a mess:
```
<group id="dummies">
  <primitive id="dummy3"/>
  <primitive id="dummy4"/>
  <primitive id="dummy0"/>
  <primitive id="dummy2"/>
  <primitive id="dummy1"/>
</group>
```
It's because the positions of the already moved XML objects could be
affected by the later moved objects.

This commit fixes it by temporarily putting "move" objects after the
last sibling and also delaying the adding of any "create" objects, then
placing them to the target positions in the right order.
---
 lib/common/xml.c | 126 ++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 97 insertions(+), 29 deletions(-)

Index: pacemaker-1.1.16+20170320.77ea74d/lib/common/xml.c
===================================================================
--- pacemaker-1.1.16+20170320.77ea74d.orig/lib/common/xml.c
+++ pacemaker-1.1.16+20170320.77ea74d/lib/common/xml.c
@@ -2018,11 +2018,40 @@ __xml_find_path(xmlNode *top, const char
     return target;
 }
 
+typedef struct xml_change_obj_s {
+    xmlNode *change;
+    xmlNode *match;
+} xml_change_obj_t;
+
+static gint
+sort_change_obj_by_position(gconstpointer a, gconstpointer b)
+{
+    const xml_change_obj_t *change_obj_a = a;
+    const xml_change_obj_t *change_obj_b = b;
+    int position_a = -1;
+    int position_b = -1;
+
+    crm_element_value_int(change_obj_a->change, XML_DIFF_POSITION, &position_a);
+    crm_element_value_int(change_obj_b->change, XML_DIFF_POSITION, &position_b);
+
+    if (position_a < position_b) {
+        return -1;
+
+    } else if (position_a > position_b) {
+        return 1;
+    }
+
+    return 0;
+}
+
 static int
 xml_apply_patchset_v2(xmlNode *xml, xmlNode *patchset, bool check_version) 
 {
     int rc = pcmk_ok;
     xmlNode *change = NULL;
+    GListPtr change_objs = NULL;
+    GListPtr gIter = NULL;
+
     for (change = __xml_first_child(patchset); change != NULL; change = __xml_next(change)) {
         xmlNode *match = NULL;
         const char *op = crm_element_value(change, XML_DIFF_OP);
@@ -2034,6 +2063,7 @@ xml_apply_patchset_v2(xmlNode *xml, xmlN
             continue;
         }
 
+        // "delete" changes for XML comments are generated with "position"
         if(strcmp(op, "delete") == 0) {
             crm_element_value_int(change, XML_DIFF_POSITION, &position);
         }
@@ -2053,7 +2083,71 @@ xml_apply_patchset_v2(xmlNode *xml, xmlN
             rc = -pcmk_err_diff_failed;
             continue;
 
-        } else if(strcmp(op, "create") == 0) {
+        } else if (strcmp(op, "create") == 0 || strcmp(op, "move") == 0) {
+            // Delay the adding of a "create" object
+            xml_change_obj_t *change_obj = calloc(1, sizeof(xml_change_obj_t));
+
+            CRM_ASSERT(change_obj != NULL);
+
+            change_obj->change = change;
+            change_obj->match = match;
+
+            change_objs = g_list_append(change_objs, change_obj);
+
+            if (strcmp(op, "move") == 0) {
+                // Temporarily put the "move" object after the last sibling
+                if (match->parent != NULL && match->parent->last != NULL) {
+                    xmlAddNextSibling(match->parent->last, match);
+                }
+            }
+
+        } else if(strcmp(op, "delete") == 0) {
+            free_xml(match);
+
+        } else if(strcmp(op, "modify") == 0) {
+            xmlAttr *pIter = crm_first_attr(match);
+            xmlNode *attrs = __xml_first_child(first_named_child(change, XML_DIFF_RESULT));
+
+            if(attrs == NULL) {
+                rc = -ENOMSG;
+                continue;
+            }
+            while(pIter != NULL) {
+                const char *name = (const char *)pIter->name;
+
+                pIter = pIter->next;
+                xml_remove_prop(match, name);
+            }
+
+            for (pIter = crm_first_attr(attrs); pIter != NULL; pIter = pIter->next) {
+                const char *name = (const char *)pIter->name;
+                const char *value = crm_element_value(attrs, name);
+
+                crm_xml_add(match, name, value);
+            }
+
+        } else {
+            crm_err("Unknown operation: %s", op);
+        }
+    }
+
+    // Changes should be generated in the right order. Double checking.
+    change_objs = g_list_sort(change_objs, sort_change_obj_by_position);
+
+    for (gIter = change_objs; gIter; gIter = gIter->next) {
+        xml_change_obj_t *change_obj = gIter->data;
+        xmlNode *match = change_obj->match;
+        const char *op = NULL;
+        const char *xpath = NULL;
+
+        change = change_obj->change;
+
+        op = crm_element_value(change, XML_DIFF_OP);
+        xpath = crm_element_value(change, XML_DIFF_PATH);
+
+        crm_trace("Continue performing %s on %s with %p", op, xpath, match);
+
+        if(strcmp(op, "create") == 0) {
             int position = 0;
             xmlNode *child = NULL;
             xmlNode *match_child = NULL;
@@ -2121,36 +2215,10 @@ xml_apply_patchset_v2(xmlNode *xml, xmlN
                         match->name, ID(match), __xml_offset(match), position, match->prev);
                 rc = -pcmk_err_diff_failed;
             }
-
-        } else if(strcmp(op, "delete") == 0) {
-            free_xml(match);
-
-        } else if(strcmp(op, "modify") == 0) {
-            xmlAttr *pIter = crm_first_attr(match);
-            xmlNode *attrs = __xml_first_child(first_named_child(change, XML_DIFF_RESULT));
-
-            if(attrs == NULL) {
-                rc = -ENOMSG;
-                continue;
-            }
-            while(pIter != NULL) {
-                const char *name = (const char *)pIter->name;
-
-                pIter = pIter->next;
-                xml_remove_prop(match, name);
-            }
-
-            for (pIter = crm_first_attr(attrs); pIter != NULL; pIter = pIter->next) {
-                const char *name = (const char *)pIter->name;
-                const char *value = crm_element_value(attrs, name);
-
-                crm_xml_add(match, name, value);
-            }
-
-        } else {
-            crm_err("Unknown operation: %s", op);
         }
     }
+
+    g_list_free_full(change_objs, free);
     return rc;
 }

Places

File bsc#1127716-0001-Fix-libcrmcommon-correctly-apply-XML-diffs-with-mult-1.1.patch of Package pacemaker.13547

Places