File bug-1024037_pacemaker-libcrmcommon-delete-XML-comments.patch of Package pacemaker.8397
commit 362f02874b6be46bf2648fb45ebc6bc636d48965
Author: Gao,Yan <ygao@suse.com>
Date: Mon Feb 13 17:47:23 2017 +0100
Fix: libcrmcommon: Correctly delete XML comments according to their positions
Previously, v2 patchset was not able to define which exact XML comments were
expected to be deleted in a CIB XML element. It was a problem if there
was more than one comment at the same level within a CIB XML element.
This commit resolves it by adding and handling a "position" field in the
diff operation "delete" for XML comments.
diff --git a/lib/common/xml.c b/lib/common/xml.c
index fd80fe189..6dce4cb7f 100644
--- a/lib/common/xml.c
+++ b/lib/common/xml.c
@@ -78,7 +78,7 @@ typedef struct xml_private_s
uint32_t flags;
char *user;
GListPtr acls;
- GListPtr deleted_paths;
+ GListPtr deleted_objs;
} xml_private_t;
typedef struct xml_acl_s {
@@ -86,6 +86,11 @@ typedef struct xml_acl_s {
char *xpath;
} xml_acl_t;
+typedef struct xml_deleted_obj_s {
+ char *path;
+ int position;
+} xml_deleted_obj_t;
+
/* *INDENT-OFF* */
static filter_t filter[] = {
@@ -275,6 +280,17 @@ __xml_acl_free(void *data)
}
static void
+__xml_deleted_obj_free(void *data)
+{
+ if(data) {
+ xml_deleted_obj_t *deleted_obj = data;
+
+ free(deleted_obj->path);
+ free(deleted_obj);
+ }
+}
+
+static void
__xml_private_clean(xml_private_t *p)
{
if(p) {
@@ -288,9 +304,9 @@ __xml_private_clean(xml_private_t *p)
p->acls = NULL;
}
- if(p->deleted_paths) {
- g_list_free_full(p->deleted_paths, free);
- p->deleted_paths = NULL;
+ if(p->deleted_objs) {
+ g_list_free_full(p->deleted_objs, __xml_deleted_obj_free);
+ p->deleted_objs = NULL;
}
}
}
@@ -1094,10 +1110,10 @@ is_config_change(xmlNode *xml)
if(xml->doc && xml->doc->_private) {
p = xml->doc->_private;
- for(gIter = p->deleted_paths; gIter; gIter = gIter->next) {
- char *path = gIter->data;
+ for(gIter = p->deleted_objs; gIter; gIter = gIter->next) {
+ xml_deleted_obj_t *deleted_obj = gIter->data;
- if(strstr(path, "/"XML_TAG_CIB"/"XML_CIB_TAG_CONFIGURATION) != NULL) {
+ if(strstr(deleted_obj->path, "/"XML_TAG_CIB"/"XML_CIB_TAG_CONFIGURATION) != NULL) {
return TRUE;
}
}
@@ -1241,11 +1257,15 @@ xml_create_patchset_v2(xmlNode *source, xmlNode *target)
crm_xml_add(v, vfields[lpc], value);
}
- for(gIter = doc->deleted_paths; gIter; gIter = gIter->next) {
+ for(gIter = doc->deleted_objs; gIter; gIter = gIter->next) {
+ xml_deleted_obj_t *deleted_obj = gIter->data;
xmlNode *change = create_xml_node(patchset, XML_DIFF_CHANGE);
crm_xml_add(change, XML_DIFF_OP, "delete");
- crm_xml_add(change, XML_DIFF_PATH, gIter->data);
+ crm_xml_add(change, XML_DIFF_PATH, deleted_obj->path);
+ if (deleted_obj->position >= 0) {
+ crm_xml_add_int(change, XML_DIFF_POSITION, deleted_obj->position);
+ }
}
__xml_build_changes(target, patchset);
@@ -1473,7 +1493,15 @@ xml_log_patchset(uint8_t log_level, const char *function, xmlNode * patchset)
}
} else if(strcmp(op, "delete") == 0) {
- do_crm_log_alias(log_level, __FILE__, function, __LINE__, "-- %s", xpath);
+ int position = -1;
+
+ crm_element_value_int(change, XML_DIFF_POSITION, &position);
+ if (position >= 0) {
+ do_crm_log_alias(log_level, __FILE__, function, __LINE__, "-- %s (%d)", xpath, position);
+
+ } else {
+ do_crm_log_alias(log_level, __FILE__, function, __LINE__, "-- %s", xpath);
+ }
}
}
return;
@@ -1521,8 +1549,17 @@ xml_log_changes(uint8_t log_level, const char *function, xmlNode * xml)
return;
}
- for(gIter = doc->deleted_paths; gIter; gIter = gIter->next) {
- do_crm_log_alias(log_level, __FILE__, function, __LINE__, "-- %s", (char*)gIter->data);
+ for(gIter = doc->deleted_objs; gIter; gIter = gIter->next) {
+ xml_deleted_obj_t *deleted_obj = gIter->data;
+
+ if (deleted_obj->position >= 0) {
+ do_crm_log_alias(log_level, __FILE__, function, __LINE__, "-- %s (%d)",
+ deleted_obj->path, deleted_obj->position);
+
+ } else {
+ do_crm_log_alias(log_level, __FILE__, function, __LINE__, "-- %s",
+ deleted_obj->path);
+ }
}
log_data_element(log_level, __FILE__, function, __LINE__, "+ ", xml, 0,
@@ -1881,7 +1918,7 @@ xml_apply_patchset_v1(xmlNode *xml, xmlNode *patchset, bool check_version)
}
static xmlNode *
-__first_xml_child_match(xmlNode *parent, const char *name, const char *id)
+__first_xml_child_match(xmlNode *parent, const char *name, const char *id, int position)
{
xmlNode *cIter = NULL;
@@ -1894,13 +1931,21 @@ __first_xml_child_match(xmlNode *parent, const char *name, const char *id)
continue;
}
}
+
+ /* The "position" makes sense only for XML comments for now */
+ if (cIter->type == XML_COMMENT_NODE
+ && position >= 0
+ && __xml_offset(cIter) != position) {
+ continue;
+ }
+
return cIter;
}
return NULL;
}
static xmlNode *
-__xml_find_path(xmlNode *top, const char *key)
+__xml_find_path(xmlNode *top, const char *key, int target_position)
{
xmlNode *target = (xmlNode*)top->doc;
char *id = malloc(XML_BUFFER_SIZE);
@@ -1923,13 +1968,19 @@ __xml_find_path(xmlNode *top, const char *key)
} else if(tag && section) {
int f = sscanf (section, "%[^[][@id='%[^']", tag, id);
+ int current_position = -1;
+
+ /* The "target_position" is for the target tag */
+ if (rc == 1 && target_position >= 0) {
+ current_position = target_position;
+ }
switch(f) {
case 1:
- target = __first_xml_child_match(target, tag, NULL);
+ target = __first_xml_child_match(target, tag, NULL, current_position);
break;
case 2:
- target = __first_xml_child_match(target, tag, id);
+ target = __first_xml_child_match(target, tag, id, current_position);
break;
default:
crm_trace("Aborting on %s", section);
@@ -1975,16 +2026,20 @@ xml_apply_patchset_v2(xmlNode *xml, xmlNode *patchset, bool check_version)
xmlNode *match = NULL;
const char *op = crm_element_value(change, XML_DIFF_OP);
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;
}
+ if(strcmp(op, "delete") == 0) {
+ crm_element_value_int(change, XML_DIFF_POSITION, &position);
+ }
#if 0
match = get_xpath_object(xpath, xml, LOG_TRACE);
#else
- match = __xml_find_path(xml, xpath);
+ match = __xml_find_path(xml, xpath, position);
#endif
crm_trace("Performing %s on %s with %p", op, xpath, match);
@@ -2583,8 +2638,8 @@ xml_get_path(xmlNode *xml)
return NULL;
}
-void
-free_xml(xmlNode * child)
+static void
+free_xml_with_position(xmlNode * child, int position)
{
if (child != NULL) {
xmlNode *top = NULL;
@@ -2613,9 +2668,25 @@ free_xml(xmlNode * child)
char buffer[XML_BUFFER_SIZE];
if(__get_prefix(NULL, child, buffer, offset) > 0) {
+ xml_deleted_obj_t *deleted_obj = calloc(1, sizeof(xml_deleted_obj_t));
+
crm_trace("Deleting %s %p from %p", buffer, child, doc);
+
+ deleted_obj->path = strdup(buffer);
+
+ deleted_obj->position = -1;
+ /* Record the "position" only for XML comments for now */
+ if (child->type == XML_COMMENT_NODE) {
+ if (position >= 0) {
+ deleted_obj->position = position;
+
+ } else {
+ deleted_obj->position = __xml_offset(child);
+ }
+ }
+
p = doc->_private;
- p->deleted_paths = g_list_append(p->deleted_paths, strdup(buffer));
+ p->deleted_objs = g_list_append(p->deleted_objs, deleted_obj);
set_doc_flag(child, xpf_dirty);
}
}
@@ -2629,6 +2700,13 @@ free_xml(xmlNode * child)
}
}
+
+void
+free_xml(xmlNode * child)
+{
+ free_xml_with_position(child, -1);
+}
+
xmlNode *
copy_xml(xmlNode * src)
{
@@ -4039,7 +4117,8 @@ __xml_diff_object(xmlNode * old, xmlNode * new)
__xml_node_clean(candidate);
__xml_acl_apply(top); /* Make sure any ACLs are applied to 'candidate' */
- free_xml(candidate);
+ /* Record the old position */
+ free_xml_with_position(candidate, __xml_offset(old_child));
if (find_element(new, old_child, TRUE) == NULL) {
xml_private_t *p = old_child->_private;
