285 lines
8.7 KiB
Diff
285 lines
8.7 KiB
Diff
From 66e5e4d83e90be3cecab7bf5f50d0e10fbaa7cea 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(-)
|
|
|
|
diff --git a/lib/common/xml.c b/lib/common/xml.c
|
|
index 66b5f66..d815a48 100644
|
|
--- a/lib/common/xml.c
|
|
+++ b/lib/common/xml.c
|
|
@@ -1466,11 +1466,40 @@ __xml_find_path(xmlNode *top, const char *key, int target_position)
|
|
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)
|
|
{
|
|
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);
|
|
@@ -1482,6 +1511,7 @@ xml_apply_patchset_v2(xmlNode *xml, xmlNode *patchset)
|
|
continue;
|
|
}
|
|
|
|
+ // "delete" changes for XML comments are generated with "position"
|
|
if(strcmp(op, "delete") == 0) {
|
|
crm_element_value_int(change, XML_DIFF_POSITION, &position);
|
|
}
|
|
@@ -1497,7 +1527,71 @@ xml_apply_patchset_v2(xmlNode *xml, xmlNode *patchset)
|
|
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 = pcmk__first_xml_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 = pcmk__first_xml_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;
|
|
@@ -1565,36 +1659,10 @@ xml_apply_patchset_v2(xmlNode *xml, xmlNode *patchset)
|
|
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 = pcmk__first_xml_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 = pcmk__first_xml_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;
|
|
}
|
|
|
|
--
|
|
1.8.3.1
|
|
|
|
|
|
From f8d008d8d3a29900ee0c6decbb71a243fa4c2d8c Mon Sep 17 00:00:00 2001
|
|
From: "Gao,Yan" <ygao@suse.com>
|
|
Date: Tue, 30 Apr 2019 00:15:03 +0200
|
|
Subject: [PATCH 2/3] Fix: libcrmcommon: avoid possible use-of-NULL when
|
|
applying XML diffs
|
|
|
|
---
|
|
lib/common/xml.c | 3 ++-
|
|
1 file changed, 2 insertions(+), 1 deletion(-)
|
|
|
|
diff --git a/lib/common/xml.c b/lib/common/xml.c
|
|
index d815a48..fe87de6 100644
|
|
--- a/lib/common/xml.c
|
|
+++ b/lib/common/xml.c
|
|
@@ -1506,11 +1506,12 @@ xml_apply_patchset_v2(xmlNode *xml, xmlNode *patchset)
|
|
const char *xpath = crm_element_value(change, XML_DIFF_PATH);
|
|
int position = -1;
|
|
|
|
- crm_trace("Processing %s %s", change->name, op);
|
|
if(op == NULL) {
|
|
continue;
|
|
}
|
|
|
|
+ crm_trace("Processing %s %s", change->name, op);
|
|
+
|
|
// "delete" changes for XML comments are generated with "position"
|
|
if(strcmp(op, "delete") == 0) {
|
|
crm_element_value_int(change, XML_DIFF_POSITION, &position);
|
|
--
|
|
1.8.3.1
|
|
|
|
|
|
From e6b2bf0cf7e7ed839583d529b190a7a6cd1bd594 Mon Sep 17 00:00:00 2001
|
|
From: "Gao,Yan" <ygao@suse.com>
|
|
Date: Tue, 30 Apr 2019 00:19:46 +0200
|
|
Subject: [PATCH 3/3] Fix: libcrmcommon: return error when applying XML diffs
|
|
containing unknown operations
|
|
|
|
---
|
|
lib/common/xml.c | 1 +
|
|
1 file changed, 1 insertion(+)
|
|
|
|
diff --git a/lib/common/xml.c b/lib/common/xml.c
|
|
index fe87de6..940c4b9 100644
|
|
--- a/lib/common/xml.c
|
|
+++ b/lib/common/xml.c
|
|
@@ -1573,6 +1573,7 @@ xml_apply_patchset_v2(xmlNode *xml, xmlNode *patchset)
|
|
|
|
} else {
|
|
crm_err("Unknown operation: %s", op);
|
|
+ rc = -pcmk_err_diff_failed;
|
|
}
|
|
}
|
|
|
|
--
|
|
1.8.3.1
|
|
|