Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
openSUSE
pacemaker.8750
pacemaker-crmd-validate-CIB-diffs-better.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File pacemaker-crmd-validate-CIB-diffs-better.patch of Package pacemaker.8750
commit 8c286edcef4e2b1dcaf4c2560574324903fa18ef Author: Ken Gaillot <kgaillot@redhat.com> Date: Mon Feb 26 11:44:19 2018 -0600 Low: crmd: validate CIB diffs better This contains refactoring and logging changes to make the diff processing code more readable, validate CIB diffs better, and log diff issues consistently. diff --git a/crmd/te_callbacks.c b/crmd/te_callbacks.c index fd75368cb..976e57e62 100644 --- a/crmd/te_callbacks.c +++ b/crmd/te_callbacks.c @@ -66,7 +66,7 @@ update_stonith_max_attempts(const char* value) } } static void -te_legacy_update_diff(const char *event, xmlNode * diff) +te_update_diff_v1(const char *event, xmlNode *diff) { int lpc, max; xmlXPathObject *xpathObj = NULL; @@ -144,46 +144,40 @@ te_legacy_update_diff(const char *event, xmlNode * diff) freeXpathObject(xpathObj); /* - * Check for and fast-track the processing of LRM refreshes - * In large clusters this can result in _huge_ speedups + * Updates by, or in response to, TE actions will never contain updates + * for more than one resource at a time, so such updates indicate an + * LRM refresh. * - * Unfortunately we can only do so when there are no pending actions - * Otherwise we could miss updates we're waiting for and stall + * In that case, start a new transition rather than check each result + * individually, which can result in _huge_ speedups in large clusters. * + * Unfortunately, we can only do so when there are no pending actions. + * Otherwise, we could mistakenly throw away those results here, and + * the cluster will stall waiting for them and time out the operation. */ - xpathObj = NULL; if (transition_graph->pending == 0) { - xpathObj = - xpath_search(diff, - "//" F_CIB_UPDATE_RESULT "//" XML_TAG_DIFF_ADDED "//" - XML_LRM_TAG_RESOURCE); - } - - max = numXpathResults(xpathObj); - if (max > 1) { - /* Updates by, or in response to, TE actions will never contain updates - * for more than one resource at a time - */ - crm_debug("Detected LRM refresh - %d resources updated: Skipping all resource events", max); - crm_log_xml_trace(diff, "lrm-refresh"); - abort_transition(INFINITY, tg_restart, "LRM Refresh", NULL); - goto bail; + xpathObj = xpath_search(diff, + "//" F_CIB_UPDATE_RESULT + "//" XML_TAG_DIFF_ADDED + "//" XML_LRM_TAG_RESOURCE); + max = numXpathResults(xpathObj); + if (max > 1) { + crm_debug("Ignoring resource operation updates due to LRM refresh of %d resources", + max); + crm_log_xml_trace(diff, "lrm-refresh"); + abort_transition(INFINITY, tg_restart, "LRM Refresh", NULL); + goto bail; + } + freeXpathObject(xpathObj); } - freeXpathObject(xpathObj); /* Process operation updates */ xpathObj = xpath_search(diff, "//" F_CIB_UPDATE_RESULT "//" XML_TAG_DIFF_ADDED "//" XML_LRM_TAG_RSC_OP); - if (numXpathResults(xpathObj)) { -/* - <status> - <node_state id="node1" state=CRMD_JOINSTATE_MEMBER exp_state="active"> - <lrm> - <lrm_resources> - <rsc_state id="" rsc_id="rsc4" node_id="node1" rsc_state="stopped"/> -*/ - int lpc = 0, max = numXpathResults(xpathObj); + max = numXpathResults(xpathObj); + if (max > 0) { + int lpc = 0; for (lpc = 0; lpc < max; lpc++) { xmlNode *rsc_op = getXpathResult(xpathObj, lpc); @@ -241,38 +235,50 @@ te_legacy_update_diff(const char *event, xmlNode * diff) freeXpathObject(xpathObj); } -static void process_resource_updates( - const char *node, xmlNode *xml, xmlNode *change, const char *op, const char *xpath) +static void +process_lrm_resource_diff(xmlNode *lrm_resource, const char *node) +{ + for (xmlNode *rsc_op = __xml_first_child(lrm_resource); rsc_op != NULL; + rsc_op = __xml_next(rsc_op)) { + process_graph_event(rsc_op, node); + } +} + +static void +process_resource_updates(const char *node, xmlNode *xml, xmlNode *change, + const char *op, const char *xpath) { xmlNode *cIter = NULL; xmlNode *rsc = NULL; - xmlNode *rsc_op = NULL; int num_resources = 0; - if(xml == NULL) { + if (xml == NULL) { return; - } else if(strcmp((const char*)xml->name, XML_CIB_TAG_LRM) == 0) { + } else if (strcmp((const char*)xml->name, XML_CIB_TAG_LRM) == 0) { xml = first_named_child(xml, XML_LRM_TAG_RESOURCES); crm_trace("Got %p in %s", xml, XML_CIB_TAG_LRM); } CRM_ASSERT(strcmp((const char*)xml->name, XML_LRM_TAG_RESOURCES) == 0); - for(cIter = xml->children; cIter; cIter = cIter->next) { + for (cIter = xml->children; cIter; cIter = cIter->next) { num_resources++; } - if(num_resources > 1) { + if (num_resources > 1) { /* - * Check for and fast-track the processing of LRM refreshes - * In large clusters this can result in _huge_ speedups + * Updates by, or in response to, TE actions will never contain updates + * for more than one resource at a time, so such updates indicate an + * LRM refresh. * - * Unfortunately we can only do so when there are no pending actions - * Otherwise we could miss updates we're waiting for and stall + * In that case, start a new transition rather than check each result + * individually, which can result in _huge_ speedups in large clusters. * + * Unfortunately, we can only do so when there are no pending actions. + * Otherwise, we could mistakenly throw away those results here, and + * the cluster will stall waiting for them and time out the operation. */ - crm_debug("Detected LRM refresh - %d resources updated", num_resources); crm_log_xml_trace(change, "lrm-refresh"); abort_transition(INFINITY, tg_restart, "LRM Refresh", NULL); @@ -281,10 +287,7 @@ static void process_resource_updates( for (rsc = __xml_first_child(xml); rsc != NULL; rsc = __xml_next(rsc)) { crm_trace("Processing %s", ID(rsc)); - for (rsc_op = __xml_first_child(rsc); rsc_op != NULL; rsc_op = __xml_next(rsc_op)) { - crm_trace("Processing %s", ID(rsc_op)); - process_graph_event(rsc_op, node); - } + process_lrm_resource_diff(rsc, node); } } @@ -361,84 +364,142 @@ abort_unless_down(const char *xpath, const char *op, xmlNode *change, free(node_uuid); } -void -te_update_diff(const char *event, xmlNode * msg) +static void +process_op_deletion(const char *xpath, xmlNode *change) { - int rc = -EINVAL; - int format = 1; - xmlNode *change = NULL; - const char *op = NULL; - - xmlNode *diff = NULL; + char *mutable_key = strdup(xpath); + char *key; + char *node_uuid; + crm_action_t *cancel = NULL; + + // Extract the part of xpath between last pair of single quotes + key = strrchr(mutable_key, '\''); + if (key != NULL) { + *key = '\0'; + key = strrchr(mutable_key, '\''); + } + if (key == NULL) { + crm_warn("Ignoring malformed CIB update (resource deletion of %s)", + xpath); + free(mutable_key); + return; + } + ++key; - int p_add[] = { 0, 0, 0 }; - int p_del[] = { 0, 0, 0 }; + node_uuid = extract_node_uuid(xpath); + cancel = get_cancel_action(key, node_uuid); + if (cancel) { + crm_info("Cancellation of %s on %s confirmed (%d)", + key, node_uuid, cancel->id); + stop_te_timer(cancel->timer); + te_action_confirmed(cancel); + update_graph(transition_graph, cancel); + trigger_graph(); + } else { + abort_transition(INFINITY, tg_restart, "Resource operation removal", + change); + } + free(mutable_key); + free(node_uuid); +} - CRM_CHECK(msg != NULL, return); - crm_element_value_int(msg, F_CIB_RC, &rc); +static void +process_delete_diff(const char *xpath, const char *op, xmlNode *change) +{ + if (strstr(xpath, "/" XML_LRM_TAG_RSC_OP "[")) { + process_op_deletion(xpath, change); - if (transition_graph == NULL) { - crm_trace("No graph"); - return; + } else if (strstr(xpath, "/" XML_CIB_TAG_LRM "[")) { + abort_unless_down(xpath, op, change, "Resource state removal"); - } else if (rc < pcmk_ok) { - crm_trace("Filter rc=%d (%s)", rc, pcmk_strerror(rc)); - return; + } else if (strstr(xpath, "/" XML_CIB_TAG_STATE "[")) { + abort_unless_down(xpath, op, change, "Node state removal"); - } else if (transition_graph->complete == TRUE - && fsa_state != S_IDLE - && fsa_state != S_TRANSITION_ENGINE && fsa_state != S_POLICY_ENGINE) { - crm_trace("Filter state=%s, complete=%d", fsa_state2string(fsa_state), - transition_graph->complete); - return; + } else { + crm_trace("Ignoring delete of %s", xpath); } +} - op = crm_element_value(msg, F_CIB_OPERATION); - diff = get_message_xml(msg, F_CIB_UPDATE_RESULT); +static void +process_node_state_diff(xmlNode *state, xmlNode *change, const char *op, + const char *xpath) +{ + xmlNode *lrm = first_named_child(state, XML_CIB_TAG_LRM); - xml_patch_versions(diff, p_add, p_del); - crm_debug("Processing (%s) diff: %d.%d.%d -> %d.%d.%d (%s)", op, - p_del[0], p_del[1], p_del[2], p_add[0], p_add[1], p_add[2], - fsa_state2string(fsa_state)); + process_resource_updates(ID(state), lrm, change, op, xpath); +} - crm_element_value_int(diff, "format", &format); - switch(format) { - case 1: - te_legacy_update_diff(event, diff); - return; - case 2: - /* Cool, we know what to do here */ - crm_log_xml_trace(diff, "Patch:Raw"); - break; - default: - crm_warn("Unknown patch format: %d", format); - return; +static void +process_status_diff(xmlNode *status, xmlNode *change, const char *op, + const char *xpath) +{ + for (xmlNode *state = __xml_first_child(status); state != NULL; + state = __xml_next(state)) { + process_node_state_diff(state, change, op, xpath); + } +} + +static void +process_cib_diff(xmlNode *cib, xmlNode *change, const char *op, + const char *xpath) +{ + xmlNode *status = first_named_child(cib, XML_CIB_TAG_STATUS); + xmlNode *config = first_named_child(cib, XML_CIB_TAG_CONFIGURATION); + + if (status) { + process_status_diff(status, change, op, xpath); } + if (config) { + abort_transition(INFINITY, tg_restart, + "Non-status-only change", change); + } +} - for (change = __xml_first_child(diff); change != NULL; change = __xml_next(change)) { +static void +te_update_diff_v2(xmlNode *diff) +{ + crm_log_xml_trace(diff, "Patch:Raw"); + + for (xmlNode *change = __xml_first_child(diff); change != NULL; + change = __xml_next(change)) { + + xmlNode *match = NULL; const char *name = NULL; - const char *op = crm_element_value(change, XML_DIFF_OP); const char *xpath = crm_element_value(change, XML_DIFF_PATH); - xmlNode *match = NULL; - const char *node = NULL; - if(op == NULL) { + // Possible ops: create, modify, delete, move + const char *op = crm_element_value(change, XML_DIFF_OP); + + // Ignore uninteresting updates + if (op == NULL) { continue; - } else if(strcmp(op, "create") == 0) { - match = change->children; + } else if (xpath == NULL) { + crm_trace("Ignoring %s change for version field", op); + continue; - } else if(strcmp(op, "move") == 0) { + } else if (strcmp(op, "move") == 0) { + crm_trace("Ignoring move change at %s", xpath); continue; + } - } else if(strcmp(op, "modify") == 0) { + // Find the result of create/modify ops + if (strcmp(op, "create") == 0) { + match = change->children; + + } else if (strcmp(op, "modify") == 0) { match = first_named_child(change, XML_DIFF_RESULT); if(match) { match = match->children; } + + } else if (strcmp(op, "delete") != 0) { + crm_warn("Ignoring malformed CIB update (%s operation on %s is unrecognized)", + op, xpath); + continue; } - if(match) { + if (match) { if (match->type == XML_COMMENT_NODE) { crm_trace("Ignoring %s operation for comment at %s", op, xpath); continue; @@ -449,130 +510,117 @@ te_update_diff(const char *event, xmlNode * msg) crm_trace("Handling %s operation for %s%s%s", op, (xpath? xpath : "CIB"), (name? " matched by " : ""), (name? name : "")); - if(xpath == NULL) { - /* Version field, ignore */ - } else if(strstr(xpath, "/cib/configuration")) { - abort_transition(INFINITY, tg_restart, "Configuration change", change); - break; /* Won't be packaged with any resource operations we may be waiting for */ + if (strstr(xpath, "/" XML_TAG_CIB "/" XML_CIB_TAG_CONFIGURATION)) { + abort_transition(INFINITY, tg_restart, "Configuration change", + change); + break; // Won't be packaged with operation results we may be waiting for - } else if(strstr(xpath, "/"XML_CIB_TAG_TICKETS) || safe_str_eq(name, XML_CIB_TAG_TICKETS)) { + } else if (strstr(xpath, "/" XML_CIB_TAG_TICKETS) + || safe_str_eq(name, XML_CIB_TAG_TICKETS)) { abort_transition(INFINITY, tg_restart, "Ticket attribute change", change); - break; /* Won't be packaged with any resource operations we may be waiting for */ + break; // Won't be packaged with operation results we may be waiting for - } else if(strstr(xpath, "/"XML_TAG_TRANSIENT_NODEATTRS"[") || safe_str_eq(name, XML_TAG_TRANSIENT_NODEATTRS)) { + } else if (strstr(xpath, "/" XML_TAG_TRANSIENT_NODEATTRS "[") + || safe_str_eq(name, XML_TAG_TRANSIENT_NODEATTRS)) { abort_unless_down(xpath, op, change, "Transient attribute change"); - break; /* Won't be packaged with any resource operations we may be waiting for */ - - } else if(strstr(xpath, "/"XML_LRM_TAG_RSC_OP"[") && safe_str_eq(op, "delete")) { - crm_action_t *cancel = NULL; - char *mutable_key = strdup(xpath); - char *key, *node_uuid; - - /* Extract the part of xpath between last pair of single quotes */ - key = strrchr(mutable_key, '\''); - if (key != NULL) { - *key = '\0'; - key = strrchr(mutable_key, '\''); - } - if (key == NULL) { - crm_warn("Ignoring malformed CIB update (resource deletion)"); - free(mutable_key); - continue; - } - ++key; - - node_uuid = extract_node_uuid(xpath); - cancel = get_cancel_action(key, node_uuid); - if (cancel == NULL) { - abort_transition(INFINITY, tg_restart, "Resource operation removal", change); + break; // Won't be packaged with operation results we may be waiting for - } else { - crm_info("Cancellation of %s on %s confirmed (%d)", key, node_uuid, cancel->id); - stop_te_timer(cancel->timer); - te_action_confirmed(cancel); - - update_graph(transition_graph, cancel); - trigger_graph(); - - } - free(mutable_key); - free(node_uuid); - - } else if(strstr(xpath, "/"XML_CIB_TAG_LRM"[") && safe_str_eq(op, "delete")) { - abort_unless_down(xpath, op, change, "Resource state removal"); - - } else if(strstr(xpath, "/"XML_CIB_TAG_STATE"[") && safe_str_eq(op, "delete")) { - abort_unless_down(xpath, op, change, "Node state removal"); - - } else if(name == NULL) { - crm_debug("No result for %s operation to %s", op, xpath); - CRM_ASSERT(strcmp(op, "delete") == 0 || strcmp(op, "move") == 0); - - } else if(strcmp(name, XML_TAG_CIB) == 0) { - xmlNode *state = NULL; - xmlNode *status = first_named_child(match, XML_CIB_TAG_STATUS); - xmlNode *config = first_named_child(match, XML_CIB_TAG_CONFIGURATION); - - for (state = __xml_first_child(status); state != NULL; state = __xml_next(state)) { - xmlNode *lrm = first_named_child(state, XML_CIB_TAG_LRM); - - node = ID(state); - process_resource_updates(node, lrm, change, op, xpath); - } - - if(config) { - abort_transition(INFINITY, tg_restart, "Non-status-only change", change); - } + } else if (strcmp(op, "delete") == 0) { + process_delete_diff(xpath, op, change); - } else if(strcmp(name, XML_CIB_TAG_STATUS) == 0) { - xmlNode *state = NULL; + } else if (name == NULL) { + crm_warn("Ignoring malformed CIB update (%s at %s has no result)", + op, xpath); - for (state = __xml_first_child(match); state != NULL; state = __xml_next(state)) { - xmlNode *lrm = first_named_child(state, XML_CIB_TAG_LRM); - - node = ID(state); - process_resource_updates(node, lrm, change, op, xpath); - } + } else if (strcmp(name, XML_TAG_CIB) == 0) { + process_cib_diff(match, change, op, xpath); - } else if(strcmp(name, XML_CIB_TAG_STATE) == 0) { - xmlNode *lrm = first_named_child(match, XML_CIB_TAG_LRM); + } else if (strcmp(name, XML_CIB_TAG_STATUS) == 0) { + process_status_diff(match, change, op, xpath); - node = ID(match); - process_resource_updates(node, lrm, change, op, xpath); + } else if (strcmp(name, XML_CIB_TAG_STATE) == 0) { + process_node_state_diff(match, change, op, xpath); - } else if(strcmp(name, XML_CIB_TAG_LRM) == 0) { - node = ID(match); - process_resource_updates(node, match, change, op, xpath); + } else if (strcmp(name, XML_CIB_TAG_LRM) == 0) { + process_resource_updates(ID(match), match, change, op, xpath); - } else if(strcmp(name, XML_LRM_TAG_RESOURCES) == 0) { + } else if (strcmp(name, XML_LRM_TAG_RESOURCES) == 0) { char *local_node = get_node_from_xpath(xpath); process_resource_updates(local_node, match, change, op, xpath); free(local_node); - } else if(strcmp(name, XML_LRM_TAG_RESOURCE) == 0) { - - xmlNode *rsc_op; + } else if (strcmp(name, XML_LRM_TAG_RESOURCE) == 0) { char *local_node = get_node_from_xpath(xpath); - for (rsc_op = __xml_first_child(match); rsc_op != NULL; rsc_op = __xml_next(rsc_op)) { - process_graph_event(rsc_op, local_node); - } + process_lrm_resource_diff(match, local_node); free(local_node); - } else if(strcmp(name, XML_LRM_TAG_RSC_OP) == 0) { + } else if (strcmp(name, XML_LRM_TAG_RSC_OP) == 0) { char *local_node = get_node_from_xpath(xpath); process_graph_event(match, local_node); free(local_node); } else { - crm_err("Ignoring %s operation for %s %p, %s", op, xpath, match, name); + crm_warn("Ignoring malformed CIB update (%s at %s has unrecognized result %s)", + op, xpath, name); } } } +void +te_update_diff(const char *event, xmlNode * msg) +{ + xmlNode *diff = NULL; + const char *op = NULL; + int rc = -EINVAL; + int format = 1; + int p_add[] = { 0, 0, 0 }; + int p_del[] = { 0, 0, 0 }; + + CRM_CHECK(msg != NULL, return); + crm_element_value_int(msg, F_CIB_RC, &rc); + + if (transition_graph == NULL) { + crm_trace("No graph"); + return; + + } else if (rc < pcmk_ok) { + crm_trace("Filter rc=%d (%s)", rc, pcmk_strerror(rc)); + return; + + } else if (transition_graph->complete + && fsa_state != S_IDLE + && fsa_state != S_TRANSITION_ENGINE + && fsa_state != S_POLICY_ENGINE) { + crm_trace("Filter state=%s, complete=%d", fsa_state2string(fsa_state), + transition_graph->complete); + return; + } + + op = crm_element_value(msg, F_CIB_OPERATION); + diff = get_message_xml(msg, F_CIB_UPDATE_RESULT); + + xml_patch_versions(diff, p_add, p_del); + crm_debug("Processing (%s) diff: %d.%d.%d -> %d.%d.%d (%s)", op, + p_del[0], p_del[1], p_del[2], p_add[0], p_add[1], p_add[2], + fsa_state2string(fsa_state)); + + crm_element_value_int(diff, "format", &format); + switch (format) { + case 1: + te_update_diff_v1(event, diff); + break; + case 2: + te_update_diff_v2(diff); + break; + default: + crm_warn("Ignoring malformed CIB update (unknown patch format %d)", + format); + } +} gboolean process_te_message(xmlNode * msg, xmlNode * xml_data)
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
