Hi HansF, I have incorporated your comments. Regarding if the set is
unique, I realized after sending the patch that a template
specialization was missing and changed node.cc accordingly:
bool operator<(const AVD_AVND &lhs, AVD_AVND &rhs) {
if (strncmp((const char*) lhs.name.value, (const char*)
rhs.name.value, lhs.name.length) < 0)
return true;
else
return false;
}
namespace std {
template <>
struct less<AVD_AVND*>
{
bool operator() (const AVD_AVND* lhs, const AVD_AVND* rhs)
{
return *lhs < *rhs;
}
};
}
i.e before avd_avnd pointers were compared for uniqueness as the
template specialization above were missing, with that added, "less than"
operator is called and it compares name
in avd_avnd.
/Regards HansN
On 06/18/14 15:16, Hans Feldt wrote:
> General comment, use NULL instead of 0
> See inline for some comments.
> /HansF
>
>> -----Original Message-----
>> From: Hans Nordebäck
>> Sent: den 12 juni 2014 15:51
>> To: Hans Feldt; praveen.malv...@oracle.com; nagendr...@oracle.com
>> Cc: opensaf-devel@lists.sourceforge.net
>> Subject: [PATCH 1 of 1] AMF: support immediate effect when changing hc-type
>> attributes V4 [#819]
>>
>> osaf/libs/common/amf/include/amf_defs.h | 7 +
>> osaf/services/saf/amf/amfd/hlttype.cc | 72 ++++++++++++-
>> osaf/services/saf/amf/amfd/include/node.h | 2 +
>> osaf/services/saf/amf/amfd/node.cc | 8 +
>> osaf/services/saf/amf/amfnd/di.cc | 3 +
>> osaf/services/saf/amf/amfnd/hcdb.cc | 150
>> ++++++++++++++++++++++++++
>> osaf/services/saf/amf/amfnd/include/avnd_hc.h | 1 +
>> 7 files changed, 241 insertions(+), 2 deletions(-)
>>
>>
>> diff --git a/osaf/libs/common/amf/include/amf_defs.h
>> b/osaf/libs/common/amf/include/amf_defs.h
>> --- a/osaf/libs/common/amf/include/amf_defs.h
>> +++ b/osaf/libs/common/amf/include/amf_defs.h
>> @@ -254,6 +254,13 @@ typedef enum
>> saAmfHealthcheckMaxDuration_ID = 2,
>> } AVSV_AMF_HEALTHCHECK_ATTR_ID;
>>
>> +/* Attribute ID enum for the SaAmfHealthcheckType class */
>> +typedef enum
>> +{
>> + saAmfHctDefPeriod_ID = 1,
>> + saAmfHctDefMaxDuration_ID = 2,
>> +} AVSV_AMF_HEALTHCHECK_TYPE_ATTR_ID;
>> +
>>
>> #define AVSV_COMMON_SUB_ID_DEFAULT_VAL 1
>> #define SA_AMF_PRESENCE_ORPHANED (SA_AMF_PRESENCE_TERMINATION_FAILED+1)
>> diff --git a/osaf/services/saf/amf/amfd/hlttype.cc
>> b/osaf/services/saf/amf/amfd/hlttype.cc
>> --- a/osaf/services/saf/amf/amfd/hlttype.cc
>> +++ b/osaf/services/saf/amf/amfd/hlttype.cc
>> @@ -15,12 +15,16 @@
>> *
>> */
>>
>> +#include <string>
>> +#include <set>
>> #include <string.h>
>> -
>> +#include "util.h"
>> +#include "node.h"
>> #include <logtrace.h>
>> #include <immutil.h>
>> #include <ncsgl_defs.h>
>> #include <imm.h>
>> +#include "comp.h"
>>
>> /**
>> * Validates proposed change in comptype
>> @@ -56,6 +60,70 @@ done:
>> return rc;
>> }
>>
>> +static void ccb_apply_modify_hdlr(const CcbUtilOperationData_t *opdata)
>> +{
>> + const SaImmAttrModificationT_2 *attr_mod;
>> + int i;
>> + const AVD_COMP_TYPE *comp_type;
>> + SaNameT comp_type_dn;
>> + SaNameT comp_type_name;
>> +
>> + TRACE_ENTER2("CCB ID %llu, '%s'", opdata->ccbId,
>> opdata->objectName.value);
>> +
>> + // input example: opdata.objectName.value,
>> safHealthcheckKey=AmfDemo,safVersion=1,safCompType=AmfDemo1
>> + avsv_sanamet_init(&opdata->objectName, &comp_type_name, "safVersion=");
> [Hans] I think you should clear comp_type_name first with a memset
>> +
>> + comp_type_dn.length =
>> + snprintf((char *)comp_type_dn.value, SA_MAX_NAME_LENGTH, "%s",
>> (const char*) comp_type_name.value);
>> +
>> + if ((comp_type = comptype_db->find(Amf::to_string(&comp_type_dn))) ==
>> 0) {
>> + LOG_ER("Internal error: %s not found", comp_type_dn.value);
>> + return;
>> + }
>> +
>> + // Create a set of nodes where components "may" be using the given
>> SaAmfHealthcheckType.
>> + // A msg will be sent to the related node regarding this change. If a
>> component has an
>> + // SaAmfHealthcheck record that overrides this SaAmfHealthcheckType it
>> will be handled by the amfnd.
>> + std::set<AVD_AVND*> node_set;
> [Hans] Will this set really be unique?
>
>> +
>> + AVD_COMP *comp = comp_type->list_of_comp;
>> + while (comp != NULL) {
>> + node_set.insert(comp->su->su_on_node);
>> + TRACE("comp name %s on node %s", comp->comp_info.name.value,
>> comp->su->su_on_node->name.value);
>> + comp = comp->comp_type_list_comp_next;
>> + }
>> +
>> + std::set<AVD_AVND*>::iterator it;
>> + for (it = node_set.begin(); it != node_set.end(); ++it) {
>> + i = 0;
>> + while ((attr_mod = opdata->param.modify.attrMods[i++]) != NULL)
>> {
>> + AVSV_PARAM_INFO param;
>> + const SaImmAttrValuesT_2 *attribute =
>> &attr_mod->modAttr;
>> + SaTimeT *param_val = (SaTimeT
>> *)attribute->attrValues[0];
>> +
>> + memset(¶m, 0, sizeof(param));
>> + param.class_id = AVSV_SA_AMF_HEALTH_CHECK_TYPE;
>> + param.act = AVSV_OBJ_OPR_MOD;
>> + param.name = opdata->objectName;
>> + param.value_len = sizeof(*param_val);
>> + memcpy(param.value, param_val, param.value_len);
>> +
>> + if (!strcmp(attribute->attrName, "saAmfHctDefPeriod")) {
>> + TRACE("saAmfHctDefPeriod modified to '%llu' for
>> CompType '%s' on node '%s'", *param_val,
>> + opdata->objectName.value,
>> (*it)->name.value);
>> + param.attr_id = saAmfHctDefPeriod_ID;
>> + } else if (!strcmp(attribute->attrName,
>> "saAmfHctDefMaxDuration")) {
>> + TRACE("saAmfHctDefMaxDuration modified to
>> '%llu' for CompType '%s' on node '%s", *param_val,
>> + opdata->objectName.value,
>> (*it)->name.value);
>> + param.attr_id = saAmfHctDefMaxDuration_ID;
>> + } else
>> + LOG_WA("Unexpected attribute name: %s",
>> attribute->attrName);
>> +
>> + avd_snd_op_req_msg(avd_cb, *it, ¶m);
>> + }
>> + }
>> +}
>> +
>> static SaAisErrorT hct_ccb_completed_cb(CcbUtilOperationData_t *opdata)
>> {
>> SaAisErrorT rc = SA_AIS_ERR_BAD_OPERATION;
>> @@ -90,7 +158,7 @@ static void hct_ccb_apply_cb(CcbUtilOper
>> case CCBUTIL_DELETE:
>> break;
>> case CCBUTIL_MODIFY:
>> - // values not used, no need to reinitialize type
>> + ccb_apply_modify_hdlr(opdata);
>> break;
>> default:
>> osafassert(0);
>> diff --git a/osaf/services/saf/amf/amfd/include/node.h
>> b/osaf/services/saf/amf/amfd/include/node.h
>> --- a/osaf/services/saf/amf/amfd/include/node.h
>> +++ b/osaf/services/saf/amf/amfd/include/node.h
>> @@ -139,6 +139,8 @@ typedef struct avd_avnd_tag {
>> bool recvr_fail_sw; /* to indicate there was node reboot because of
>> node failover/switchover.*/
>> } AVD_AVND;
>>
>> +bool operator<(const avd_avnd_tag& t1, const avd_avnd_tag& t2);
>> +
>> extern AmfDb<std::string, AVD_AVND> *node_name_db;
>> extern AmfDb<uint32_t, AVD_AVND> *node_id_db;
>>
>> diff --git a/osaf/services/saf/amf/amfd/node.cc
>> b/osaf/services/saf/amf/amfd/node.cc
>> --- a/osaf/services/saf/amf/amfd/node.cc
>> +++ b/osaf/services/saf/amf/amfd/node.cc
>> @@ -28,6 +28,14 @@
>> AmfDb<std::string, AVD_AVND> *node_name_db = 0; /* SaNameT index */
>> AmfDb<uint32_t, AVD_AVND> *node_id_db = 0; /* SaClmNodeIdT index */
>>
>> +bool operator<(const avd_avnd_tag& t1, const avd_avnd_tag& t2)
>> +{
>> + if (strncmp((const char*) t1.name.value, (const char*) t2.name.value,
>> t1.name.length) < 0)
>> + return true;
>> + else
>> + return false;
>> +}
>> +
>> //
>> // TODO(HANO) Temporary use this function instead of strdup which uses
>> malloc.
>> // Later on remove this function and use std::string instead
>> diff --git a/osaf/services/saf/amf/amfnd/di.cc
>> b/osaf/services/saf/amf/amfnd/di.cc
>> --- a/osaf/services/saf/amf/amfnd/di.cc
>> +++ b/osaf/services/saf/amf/amfnd/di.cc
>> @@ -228,6 +228,9 @@ uint32_t avnd_evt_avd_operation_request_
>> case AVSV_SA_AMF_HEALTH_CHECK:
>> rc = avnd_hc_oper_req(cb, param);
>> break;
>> + case AVSV_SA_AMF_HEALTH_CHECK_TYPE:
>> + rc = avnd_hctype_oper_req(cb, param);
>> + break;
>> default:
>> LOG_NO("%s: Unknown class ID %u", __FUNCTION__,
>> param->class_id);
>> rc = NCSCC_RC_FAILURE;
>> diff --git a/osaf/services/saf/amf/amfnd/hcdb.cc
>> b/osaf/services/saf/amf/amfnd/hcdb.cc
>> --- a/osaf/services/saf/amf/amfnd/hcdb.cc
>> +++ b/osaf/services/saf/amf/amfnd/hcdb.cc
>> @@ -36,6 +36,7 @@
>> #include "avnd.h"
>> #include <saImmOm.h>
>> #include <immutil.h>
>> +#include <string>
>>
>> static NCS_PATRICIA_TREE hctypedb; /* healthcheck type db */
>>
>> @@ -339,6 +340,90 @@ SaAisErrorT avnd_hctype_config_get(SaImm
>> return error;
>> }
>>
>> +// Search for a given key and return its value or empty string if not
>> found.
>> +static std::string search_key(const std::string& str, const std::string&
>> key)
>> +{
>> + std::string value;
>> +
>> + std::string::size_type idx1;
>> +
>> + idx1 = str.find(key);
>> +
>> + // if key was found
>> + if (idx1 != std::string::npos) {
>> + // get value, idx2 points to value
>> + idx1 += key.length();
>> + std::string part2 = str.substr(idx1);
>> +
>> + idx1 = part2.find(",");
>> +
>> + // get value
>> + value = part2.substr(0, idx1);
>> + }
>> +
>> + return value;
>> +}
>> +
>> +static void comp_hctype_update_compdb(AVND_CB *cb, AVSV_PARAM_INFO *param)
>> +{
>> + AVND_COMP_HC_REC *comp_hc_rec;
>> + AVND_COMP * comp;
>> + char *comp_type_name;
>> + AVSV_HLT_KEY hlt_chk;
>> + AVND_COMP_HC_REC tmp_hc_rec;
>> +
>> + // 1. find component from componentType,
>> + // input example, param->name.value =
>> safHealthcheckKey=AmfDemo,safVersion=1,safCompType=AmfDemo1
>> + comp_type_name = strstr((char *)param->name.value, "safVersion");
>> + TRACE("comp_type_name: %s", comp_type_name);
>> + osafassert(comp_type_name);
>> +
>> + // 2. search each component for a matching compType
>> + comp = (AVND_COMP *)ncs_patricia_tree_getnext(&cb->compdb, (uint8_t
>> *)0);
>> + while (comp != 0) {
>> + if (strncmp((const char*) comp->saAmfCompType.value,
>> comp_type_name, comp->saAmfCompType.length) == 0) {
>> +
>> + // 3. matching compType found, check that component
>> does not have a SaAmfHealthcheck rec (specialization)
>> + std::string hlt_chk_key = search_key((const char*)
>> param->name.value, "safHealthcheckKey=");
>> + if (hlt_chk_key.size() == 0)
> [Hans] place opening brace end of line
>> + {
>> + LOG_ER("%s: failed to get healthcheckKey from
>> %s", __FUNCTION__, param->name.value);
>> + return;
>> + }
>> +
>> + memset(&hlt_chk, 0, sizeof(AVSV_HLT_KEY));
>> + hlt_chk.comp_name.length = comp->name.length;
>> + memcpy(hlt_chk.comp_name.value, comp->name.value,
>> hlt_chk.comp_name.length);
>> + hlt_chk.key_len = hlt_chk_key.size();
>> + memcpy(hlt_chk.name.key, hlt_chk_key.c_str(),
>> hlt_chk_key.size());
>> + hlt_chk.name.keyLen = hlt_chk.key_len;
>> + TRACE("comp_name %s key %s keyLen %u",
>> hlt_chk.comp_name.value, hlt_chk.name.key,
>> hlt_chk.name.keyLen);
>> + if (avnd_hcdb_rec_get(cb, &hlt_chk) == 0) {
>> + TRACE("comp uses healthcheckType rec");
>> + // 4. found a component that uses the
>> healthcheckType record, update the comp_hc_rec
>> + memset(&tmp_hc_rec, '\0',
>> sizeof(AVND_COMP_HC_REC));
>> + tmp_hc_rec.key = hlt_chk.name;
>> + tmp_hc_rec.req_hdl = comp->reg_hdl;
>> + TRACE("tmp_hc_rec: key %s req_hdl %llu",
>> tmp_hc_rec.key.key, tmp_hc_rec.req_hdl);
>> + if ((comp_hc_rec =
>> m_AVND_COMPDB_REC_HC_GET(*comp, tmp_hc_rec)) != 0) {
>> + TRACE("comp_hc_rec: period %llu max_dur
>> %llu", comp_hc_rec->period, comp_hc_rec-
>>> max_dur);
>> + switch (param->attr_id) {
>> + case saAmfHctDefPeriod_ID:
>> + comp_hc_rec->period =
>> *((SaTimeT *)param->value);
>> + break;
>> + case saAmfHctDefMaxDuration_ID:
>> + comp_hc_rec->max_dur =
>> *((SaTimeT *)param->value);
>> + break;
>> + default:
>> + osafassert(0);
> [Hans] anyway add a break here for analysis tools...
>> + }
>> + }
>> + }
>> + }
>> + comp = (AVND_COMP *) ncs_patricia_tree_getnext(&cb->compdb,
>> (uint8_t *)&comp->name);
>> + }
>> +}
>> +
>> uint32_t avnd_hc_oper_req(AVND_CB *cb, AVSV_PARAM_INFO *param)
>> {
>> uint32_t rc = NCSCC_RC_FAILURE;
>> @@ -401,3 +486,68 @@ done:
>> return rc;
>> }
>>
>> +uint32_t avnd_hctype_oper_req(AVND_CB *cb, AVSV_PARAM_INFO *param)
>> +{
>> + uint32_t rc = NCSCC_RC_FAILURE;
>> +
>> + TRACE_ENTER2("'%s'", param->name.value);
>> + AVND_HCTYPE *hctype = (AVND_HCTYPE *)ncs_patricia_tree_get(&hctypedb,
>> (uint8_t *)¶m->name);
>> +
>> + switch (param->act) {
>> + case AVSV_OBJ_OPR_MOD: {
>> + if (!hctype) {
>> + LOG_ER("%s: failed to get %s", __FUNCTION__,
>> param->name.value);
>> + goto done;
>> + }
>> +
>> + switch (param->attr_id) {
>> + case saAmfHctDefPeriod_ID:
>> + osafassert(sizeof(SaTimeT) == param->value_len);
>> + hctype->saAmfHctDefPeriod = *((SaTimeT *)param->value);
>> + comp_hctype_update_compdb(cb, param);
>> + //m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, hc,
>> AVND_CKPT_HC_PERIOD);
>> + break;
>> +
>> + case saAmfHctDefMaxDuration_ID:
>> + osafassert(sizeof(SaTimeT) == param->value_len);
>> + hctype->saAmfHctDefMaxDuration = *((SaTimeT
>> *)param->value);
>> + comp_hctype_update_compdb(cb, param);
>> + //m_AVND_SEND_CKPT_UPDT_ASYNC_UPDT(cb, hc,
>> AVND_CKPT_HC_MAX_DUR);
>> + break;
>> +
>> + default:
>> + LOG_NO("%s: Unsupported attribute %u", __FUNCTION__,
>> param->attr_id);
>> + goto done;
>> + }
>> + break;
>> + }
>> +
>> + case AVSV_OBJ_OPR_DEL: {
> [Hans] I am not sure this should be here. How are these objects deleted today?
>
>> + if (hctype != NULL) {
>> + rc = ncs_patricia_tree_del(&hctypedb,
>> &hctype->tree_node);
>> + osafassert(rc == NCSCC_RC_SUCCESS);
>> + LOG_IN("Deleted '%s'", param->name.value);
> [Hans] missing free of hctype?
>
>> + } else {
>> + /*
>> + ** Normal case that happens if a parent of this HC was
>> + ** the real delete target for the CCB.
>> + */
>> + TRACE("already deleted!");
>> + }
>> +
>> + break;
>> + }
>> + default:
>> + LOG_NO("%s: Unsupported action %u", __FUNCTION__, param->act);
>> + goto done;
>> + }
>> +
>> + rc = NCSCC_RC_SUCCESS;
>> +
>> +done:
>> + rc = NCSCC_RC_SUCCESS;
>> +
>> + TRACE_LEAVE();
>> + return rc;
>> +}
>> +
>> diff --git a/osaf/services/saf/amf/amfnd/include/avnd_hc.h
>> b/osaf/services/saf/amf/amfnd/include/avnd_hc.h
>> --- a/osaf/services/saf/amf/amfnd/include/avnd_hc.h
>> +++ b/osaf/services/saf/amf/amfnd/include/avnd_hc.h
>> @@ -59,5 +59,6 @@ extern SaAisErrorT avnd_hc_config_get(st
>> extern SaAisErrorT avnd_hctype_config_get(SaImmHandleT immOmHandle, const
>> SaNameT *comptype_dn);
>> extern AVND_HCTYPE *avnd_hctypedb_rec_get(const SaNameT *comp_type_dn,
>> const SaAmfHealthcheckKeyT *key);
>> extern uint32_t avnd_hc_oper_req(struct avnd_cb_tag *, AVSV_PARAM_INFO
>> *param);
>> +extern uint32_t avnd_hctype_oper_req(struct avnd_cb_tag *, AVSV_PARAM_INFO
>> *param);
>>
>> #endif /* !AVND_HC_H */
------------------------------------------------------------------------------
HPCC Systems Open Source Big Data Platform from LexisNexis Risk Solutions
Find What Matters Most in Your Big Data with HPCC Systems
Open Source. Fast. Scalable. Simple. Ideal for Dirty Data.
Leverages Graph Analysis for Fast Processing & Easy Data Exploration
http://p.sf.net/sfu/hpccsystems
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
