Comments:
- the commit message says comp but its all about hctype, remve comp I guess?
- strange indentation in new code, neither linux or google:
- space between if and (, opening brace on same line etc
See inline
Thanks,
Hans
On 10 April 2014 13:25, Hans Nordeback <hans.nordeb...@ericsson.com> wrote:
> osaf/libs/common/amf/include/amf_defs.h | 7 +
> osaf/services/saf/amf/amfd/hlttype.cc | 70 +++++++++++-
> osaf/services/saf/amf/amfnd/di.cc | 3 +
> osaf/services/saf/amf/amfnd/hcdb.cc | 161
> ++++++++++++++++++++++++++
> osaf/services/saf/amf/amfnd/include/avnd_hc.h | 1 +
> 5 files changed, 241 insertions(+), 1 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,15 @@
> *
> */
>
> +#include <string>
> +#include <map>
> #include <string.h>
>
> #include <logtrace.h>
> #include <immutil.h>
> #include <ncsgl_defs.h>
> #include <imm.h>
> +#include "comp.h"
>
> /**
> * Validates proposed change in comptype
> @@ -56,6 +59,71 @@ done:
> return rc;
> }
>
> +static void ccb_apply_modify_hdlr(CcbUtilOperationData_t *opdata)
opdata should be const
> +{
> + const SaImmAttrModificationT_2 *attr_mod;
> + int i;
> + const AVD_COMP_TYPE *comp_type;
> + SaNameT comp_type_dn;
> + char *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
> + comp_type_name = strstr((char *)opdata->objectName.value,
> "safVersion");
you should use avsv_sanamet_init() instead, it does the same thing.
> + osafassert(comp_type_name);
> +
> + comp_type_dn.length =
> + snprintf((char *)comp_type_dn.value, SA_MAX_NAME_LENGTH,
> "%s", comp_type_name);
> +
> + if ((comp_type = avd_comptype_get(&comp_type_dn)) == NULL) {
> + LOG_ER("Internal error: %s not found", comp_type_dn.value);
> + return;
> + }
> +
> + // Create a map of nodes where components "may" be using the given
> SaAmfHealthcheckType.
I don't understand why you use a map, feels like you get more code
than needed. Try use a vector instead then the "name thing" will go
away which anyway seems unused.
> + // 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::map<std::string, avd_avnd_tag*> node_map;
> + AVD_COMP *comp = comp_type->list_of_comp;
> + while (comp != NULL) {
> + std::string name((const
> char*)comp->su->su_on_node->name.value, comp->su->su_on_node->name.length);
> + node_map.insert(std::make_pair(name, 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::map<std::string, avd_avnd_tag*>::iterator it;
> + for (it = node_map.begin(); it != node_map.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->second->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->second->name.value);
> + param.attr_id = saAmfHctDefMaxDuration_ID;
> + } else
> + osafassert(0);
can't have assert here, just log at warning level (I guess)
> +
> + avd_snd_op_req_msg(avd_cb, it->second, ¶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/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,101 @@ SaAisErrorT avnd_hctype_config_get(SaImm
> return error;
> }
>
> +//
> +std::string get_healtcheck_key(const std::string& dn)
static, doxygen function comment
> +{
> + std::string err_str;
> +
> + std::string::size_type idx = dn.find("=");
> +
> + // found?
> + if(idx == std::string::npos)
> + {
> + return err_str;
> + }
> +
> + std::string part1 = dn.substr(0, idx);
> + if (part1 != "safHealthcheckKey")
> + {
> + return err_str;
> + }
> +
> + std::string part2 = dn.substr(idx + 1);
> +
> + idx = part2.find(",");
> +
> + if(idx == std::string::npos)
> + {
> + return err_str;
> + }
> +
> + std::string value = part2.substr(0, idx);
> +
> + return value;
> +
> +}
> +
> +
> +static void comp_hctype_update_compdb(AVND_CB *cb, AVSV_PARAM_INFO *param)
remove cb, add const for param, doxygen function comment
> +{
> + 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 = get_healtcheck_key((const
> char*)param->name.value);
> + if (hlt_chk_key.size() == 0)
> + {
> + 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);
> + }
> + }
> + }
> + }
> + 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 +497,68 @@ done:
> return rc;
> }
>
> +uint32_t avnd_hctype_oper_req(AVND_CB *cb, AVSV_PARAM_INFO *param)
remove avnd_ prefix, remove cb parameter, add const for 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) {
what convention do we have to check ptrs?
I try to stick to: if (hctype == NULL)
maybe we should decide and document?
> + 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);
guess you should log some DN here?
> + goto done;
> + }
> + break;
> + }
> +
> + case AVSV_OBJ_OPR_DEL: {
> + if (hctype != NULL) {
> + rc = ncs_patricia_tree_del(&hctypedb,
> &hctype->tree_node);
> + osafassert(rc == NCSCC_RC_SUCCESS);
> + LOG_IN("Deleted '%s'", param->name.value);
> + } 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 */
>
> ------------------------------------------------------------------------------
> Put Bad Developers to Shame
> Dominate Development with Jenkins Continuous Integration
> Continuously Automate Build, Test & Deployment
> Start a new project now. Try Jenkins in the cloud.
> http://p.sf.net/sfu/13600_Cloudbees
> _______________________________________________
> Opensaf-devel mailing list
> Opensaf-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/opensaf-devel
------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos. Get
unparalleled scalability from the best Selenium testing platform available.
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
