From 2dd1537211458902a240061db120baae2e24153a Mon Sep 17 00:00:00 2001 From: Ken Gaillot Date: Feb 05 2021 06:23:10 +0000 Subject: Refactor: libcrmcommon,libpe_status: use new attribute removing function ... where appropriate, for readability and efficiency --- diff --git a/lib/common/operations.c b/lib/common/operations.c index 7e6bf5a..f3a11be 100644 --- a/lib/common/operations.c +++ b/lib/common/operations.c @@ -24,6 +24,7 @@ #include #include #include +#include #include static regex_t *notify_migrate_re = NULL; @@ -361,6 +362,24 @@ decode_transition_key(const char *key, char **uuid, int *transition_id, int *act return TRUE; } +#define CRM_META_LEN (sizeof(CRM_META) - 1) + +// Return true if a is an attribute that should be filtered +static bool +should_filter_for_digest(xmlAttrPtr a, void *user_data) +{ + // @TODO CRM_META check should be case-sensitive + return (strncasecmp((const char *) a->name, CRM_META, CRM_META_LEN) == 0) + || pcmk__str_any_of((const char *) a->name, + XML_ATTR_ID, + XML_ATTR_CRM_VERSION, + XML_LRM_ATTR_OP_DIGEST, + XML_LRM_ATTR_TARGET, + XML_LRM_ATTR_TARGET_UUID, + "pcmk_external_ip", + NULL); +} + /*! * \internal * \brief Remove XML attributes not needed for operation digest @@ -374,52 +393,31 @@ pcmk__filter_op_for_digest(xmlNode *param_set) char *timeout = NULL; guint interval_ms = 0; - const char *attr_filter[] = { - XML_ATTR_ID, - XML_ATTR_CRM_VERSION, - XML_LRM_ATTR_OP_DIGEST, - XML_LRM_ATTR_TARGET, - XML_LRM_ATTR_TARGET_UUID, - "pcmk_external_ip" - }; - - const int meta_len = strlen(CRM_META); - if (param_set == NULL) { return; } - // Remove the specific attributes listed in attr_filter - for (int lpc = 0; lpc < DIMOF(attr_filter); lpc++) { - xml_remove_prop(param_set, attr_filter[lpc]); - } - + /* Timeout is useful for recurring operation digests, so grab it before + * removing meta-attributes + */ key = crm_meta_name(XML_LRM_ATTR_INTERVAL_MS); if (crm_element_value_ms(param_set, key, &interval_ms) != pcmk_ok) { interval_ms = 0; } free(key); - - key = crm_meta_name(XML_ATTR_TIMEOUT); - timeout = crm_element_value_copy(param_set, key); - - // Remove all CRM_meta_* attributes - for (xmlAttrPtr xIter = param_set->properties; xIter != NULL; ) { - const char *prop_name = (const char *) (xIter->name); - - xIter = xIter->next; - - // @TODO Why is this case-insensitive? - if (strncasecmp(prop_name, CRM_META, meta_len) == 0) { - xml_remove_prop(param_set, prop_name); - } + key = NULL; + if (interval_ms != 0) { + key = crm_meta_name(XML_ATTR_TIMEOUT); + timeout = crm_element_value_copy(param_set, key); } - if ((interval_ms != 0) && (timeout != NULL)) { - // Add the timeout back, it's useful for recurring operation digests + // Remove all CRM_meta_* attributes and certain other attributes + pcmk__xe_remove_matching_attrs(param_set, should_filter_for_digest, NULL); + + // Add timeout back for recurring operation digests + if (timeout != NULL) { crm_xml_add(param_set, key, timeout); } - free(timeout); free(key); } diff --git a/lib/common/patchset.c b/lib/common/patchset.c index 15cbe07..fae7046 100644 --- a/lib/common/patchset.c +++ b/lib/common/patchset.c @@ -638,13 +638,19 @@ xml_log_patchset(uint8_t log_level, const char *function, xmlNode *patchset) } } +// Return true if attribute name is not "id" +static bool +not_id(xmlAttrPtr attr, void *user_data) +{ + return strcmp((const char *) attr->name, XML_ATTR_ID) != 0; +} + // Apply the removals section of an v1 patchset to an XML node static void process_v1_removals(xmlNode *target, xmlNode *patch) { xmlNode *patch_child = NULL; xmlNode *cIter = NULL; - xmlAttrPtr xIter = NULL; char *id = NULL; const char *name = NULL; @@ -677,15 +683,8 @@ process_v1_removals(xmlNode *target, xmlNode *patch) return; } - for (xIter = pcmk__xe_first_attr(patch); xIter != NULL; - xIter = xIter->next) { - const char *p_name = (const char *)xIter->name; - - // Removing then restoring id would change ordering of properties - if (!pcmk__str_eq(p_name, XML_ATTR_ID, pcmk__str_casei)) { - xml_remove_prop(target, p_name); - } - } + // Removing then restoring id would change ordering of properties + pcmk__xe_remove_matching_attrs(patch, not_id, NULL); // Changes to child objects cIter = pcmk__xml_first_child(target); @@ -1204,7 +1203,6 @@ apply_v2_patchset(xmlNode *xml, xmlNode *patchset) free_xml(match); } else if (strcmp(op, "modify") == 0) { - xmlAttr *pIter = pcmk__xe_first_attr(match); xmlNode *attrs = NULL; attrs = pcmk__xml_first_child(first_named_child(change, @@ -1213,14 +1211,9 @@ apply_v2_patchset(xmlNode *xml, xmlNode *patchset) rc = ENOMSG; continue; } - while (pIter != NULL) { - const char *name = (const char *)pIter->name; - - pIter = pIter->next; - xml_remove_prop(match, name); - } + pcmk__xe_remove_matching_attrs(match, NULL, NULL); // Remove all - for (pIter = pcmk__xe_first_attr(attrs); pIter != NULL; + for (xmlAttrPtr pIter = pcmk__xe_first_attr(attrs); pIter != NULL; pIter = pIter->next) { const char *name = (const char *) pIter->name; const char *value = crm_element_value(attrs, name); diff --git a/lib/common/xml.c b/lib/common/xml.c index 39c5e53..869ed51 100644 --- a/lib/common/xml.c +++ b/lib/common/xml.c @@ -328,32 +328,31 @@ pcmk__xml_position(xmlNode *xml, enum xml_private_flags ignore_if_set) return position; } -// Remove all attributes marked as deleted from an XML node -static void -accept_attr_deletions(xmlNode *xml) +// This also clears attribute's flags if not marked as deleted +static bool +marked_as_deleted(xmlAttrPtr a, void *user_data) { - xmlNode *cIter = NULL; - xmlAttr *pIter = NULL; - xml_private_t *p = xml->_private; + xml_private_t *p = a->_private; + if (pcmk_is_set(p->flags, xpf_deleted)) { + return true; + } p->flags = xpf_none; - pIter = pcmk__xe_first_attr(xml); - - while (pIter != NULL) { - const xmlChar *name = pIter->name; - - p = pIter->_private; - pIter = pIter->next; + return false; +} - if(p->flags & xpf_deleted) { - xml_remove_prop(xml, (const char *)name); +// Remove all attributes marked as deleted from an XML node +static void +accept_attr_deletions(xmlNode *xml) +{ + // Clear XML node's flags + ((xml_private_t *) xml->_private)->flags = xpf_none; - } else { - p->flags = xpf_none; - } - } + // Remove this XML node's attributes that were marked as deleted + pcmk__xe_remove_matching_attrs(xml, marked_as_deleted, NULL); - for (cIter = pcmk__xml_first_child(xml); cIter != NULL; + // Recursively do the same for this XML node's children + for (xmlNodePtr cIter = pcmk__xml_first_child(xml); cIter != NULL; cIter = pcmk__xml_next(cIter)) { accept_attr_deletions(cIter); } diff --git a/lib/pengine/pe_digest.c b/lib/pengine/pe_digest.c index dd6b753..e8cb108 100644 --- a/lib/pengine/pe_digest.c +++ b/lib/pengine/pe_digest.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include "pe_status_private.h" @@ -45,40 +46,36 @@ pe__free_digests(gpointer ptr) } } -/*! - * \internal - * \brief Remove named attributes from an XML element - * - * \param[in,out] param_set XML to be filtered - * \param[in] param_string Space-separated list of attribute names - * \param[in] need_present Whether to remove attributes that match, - * or those that don't match - */ -static void -filter_parameters(xmlNode *param_set, const char *param_string, - bool need_present) +// Return true if XML attribute name is substring of a given string +static bool +attr_in_string(xmlAttrPtr a, void *user_data) { - if ((param_set == NULL) || (param_string == NULL)) { - return; - } - for (xmlAttrPtr xIter = param_set->properties; xIter; ) { - const char *prop_name = (const char *) xIter->name; - char *name = crm_strdup_printf(" %s ", prop_name); - char *match = strstr(param_string, name); - - free(name); + bool filter = false; + char *name = crm_strdup_printf(" %s ", (const char *) a->name); - // Do now, because current entry might get removed below - xIter = xIter->next; + if (strstr((const char *) user_data, name) == NULL) { + crm_trace("Filtering %s (not found in '%s')", + (const char *) a->name, (const char *) user_data); + filter = true; + } + free(name); + return filter; +} - if ((need_present && (match == NULL)) - || (!need_present && (match != NULL))) { +// Return true if XML attribute name is not substring of a given string +static bool +attr_not_in_string(xmlAttrPtr a, void *user_data) +{ + bool filter = false; + char *name = crm_strdup_printf(" %s ", (const char *) a->name); - crm_trace("Filtering %s (%sfound in '%s')", - prop_name, (need_present? "not " : ""), param_string); - xml_remove_prop(param_set, prop_name); - } + if (strstr((const char *) user_data, name) != NULL) { + crm_trace("Filtering %s (found in '%s')", + (const char *) a->name, (const char *) user_data); + filter = true; } + free(name); + return filter; } #if ENABLE_VERSIONED_ATTRS @@ -177,6 +174,13 @@ calculate_main_digest(op_digest_cache_t *data, pe_resource_t *rsc, op_version); } +// Return true if XML attribute name is a Pacemaker-defined fencing parameter +static bool +is_fence_param(xmlAttrPtr attr, void *user_data) +{ + return pcmk_stonith_param((const char *) attr->name); +} + /*! * \internal * \brief Add secure digest to a digest cache entry @@ -209,8 +213,12 @@ calculate_secure_digest(op_digest_cache_t *data, pe_resource_t *rsc, if (overrides != NULL) { g_hash_table_foreach(overrides, hash2field, data->params_secure); } + g_hash_table_foreach(rsc->parameters, hash2field, data->params_secure); - filter_parameters(data->params_secure, secure_list, FALSE); + if (secure_list != NULL) { + pcmk__xe_remove_matching_attrs(data->params_secure, attr_not_in_string, + (void *) secure_list); + } if (pcmk_is_set(pcmk_get_ra_caps(class), pcmk_ra_cap_fence_params)) { /* For stonith resources, Pacemaker adds special parameters, @@ -218,15 +226,8 @@ calculate_secure_digest(op_digest_cache_t *data, pe_resource_t *rsc, * controller will not hash them. That means we have to filter * them out before calculating our hash for comparison. */ - for (xmlAttrPtr iter = data->params_secure->properties; - iter != NULL; ) { - const char *prop_name = (const char *) iter->name; - - iter = iter->next; // Grab next now in case we remove current - if (pcmk_stonith_param(prop_name)) { - xml_remove_prop(data->params_secure, prop_name); - } - } + pcmk__xe_remove_matching_attrs(data->params_secure, is_fence_param, + NULL); } data->digest_secure_calc = calculate_operation_digest(data->params_secure, op_version); @@ -264,7 +265,10 @@ calculate_restart_digest(op_digest_cache_t *data, xmlNode *xml_op, // Then filter out reloadable parameters, if any value = crm_element_value(xml_op, XML_LRM_ATTR_OP_RESTART); - filter_parameters(data->params_restart, value, TRUE); + if (value != NULL) { + pcmk__xe_remove_matching_attrs(data->params_restart, attr_in_string, + (void *) value); + } value = crm_element_value(xml_op, XML_ATTR_CRM_VERSION); data->digest_restart_calc = calculate_operation_digest(data->params_restart,