Hi Alex > On 22 Apr 2017, at 7:51 pm, Alex Jones <[email protected]> wrote: > > Hi Gary, > > I guess I'm OK if you want to put this in regardless, but I have some > comments. > > (1) The behavior should be ON by default because it is part of the spec.
We could do that. Let's see what other maintainers think? > (2) Do we really want to allow this value to be changed while a campaign is > executing? That could cause some problems. This is another reason why I think > this should really be handled by SMF. > (3) If this does end up being handled by SMF, what does having this > configuration in AMF buy us? What would be the point of using it? My understanding is changing SMF not to set the campaign is non-backwards-compatible; some existing applications are using this attribute. I had also wanted this configuration to be in SMF as it would mean no changes required in AMF. Thanks Gary > > Alex > From: Gary Lee [[email protected]] > Sent: Friday, April 21, 2017 9:24 AM > To: Alex Jones > Cc: praveen malviya; [email protected] > Subject: > > NOTICE: This email was received from an EXTERNAL sender > > Hi > > I'm neutral as to whether this should be done in SMF or AMF. > But most of the code changes are related to introducing a configuration > object for AMF which is long overdue anyway. We have hard coded values and > environment variables which would be better placed in such an object. > > Gary > > > On 21 Apr 2017, at 11:07 pm, Alex Jones <[email protected]> wrote: > > > > In my opinion this is not the right thing to do for a couple of reasons. > > > > (1) If the behavior is defined in the spec it should be ON by default. > > Anders and I have already discussed this. > > > > (2) Implementing this in AMF is too complicated. It really should be > > implemented in SMF. SMF shouldn't set the maintenance campaign if it > > doesn't want the new behavior. > > > > (3) It is also much simpler to implement in SMF. > > > > Alex > > > >> > >> Message: 3 > >> Date: Fri, 21 Apr 2017 19:51:33 +1000 > >> From: Gary Lee <[email protected]> > >> Subject: [devel] [PATCH 0/1] Review Request for amfd: make auto repair > >> restriction configurable [#2435] > >> To: [email protected] <mailto:[email protected]> > >> Cc: [email protected] > >> <mailto:[email protected]> > >> Message-ID: > >> <[email protected]> > >> Content-Type: text/plain; charset=UTF-8 > >> > >> Summary: amfd: make auto repair restriction configurable [#2435] > >> Review request for Ticket(s): 2435 > >> Peer Reviewer(s): AMF devs > >> Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** > >> Affected branch(es): develop > >> Development branch: ticket-2435 > >> Base revision: 66970f59421f9d4338ee6d13134afca9082c1e91 > >> Personal repository: git://git.code.sf.net/u/userid-2226215/review > >> > >> -------------------------------- > >> Impacted area Impact y/n > >> -------------------------------- > >> Docs n > >> Build system n > >> RPM/packaging n > >> Configuration files n > >> Startup scripts n > >> SAF services y > >> OpenSAF services n > >> Core libraries n > >> Samples n > >> Tests n > >> Other n > >> > >> > >> Comments (indicate scope for each "y" above): > >> --------------------------------------------- > >> *** EXPLAIN/COMMENT THE PATCH SERIES HERE *** > >> > >> revision be7b51d723db24721c19b0b4953e794215f86d88 > >> Author: Gary Lee <[email protected]> > >> Date: Fri, 21 Apr 2017 19:36:51 +1000 > >> > >> amfd: make auto repair restriction configurable [#2435] > >> > >> This adds a configuration object for AMF at > >> amfConfig=1,safApp=safAmfService. > >> > >> A configuration attribute 'amfRestrictAutoRepairEnable' is added. > >> This determines if 'suMaintenanceCampaign' should be ignored to maintain > >> legacy AMF behaviour. The default behaviour is not to support auto repair > >> restriction. > >> > >> To enable restriction: > >> immcfg -a amfRestrictAutoRepairEnable=1 amfConfig=1,safApp=safAmfService > >> > >> To disable restriction: > >> immcfg -a amfRestrictAutoRepairEnable=0 amfConfig=1,safApp=safAmfService > >> > >> > >> > >> Added Files: > >> ------------ > >> src/amf/amfd/config.cc > >> src/amf/amfd/config.h > >> > >> > >> Complete diffstat: > >> ------------------ > >> src/amf/Makefile.am | 3 + > >> src/amf/amfd/comp.cc | 2 +- > >> src/amf/amfd/config.cc | 179 +++++++++++++++++++++++++++++++++++++++++ > >> src/amf/amfd/config.h | 21 +++++ > >> src/amf/amfd/imm.cc | 35 ++++++-- > >> src/amf/amfd/ndproc.cc | 4 +- > >> src/amf/amfd/node.cc | 4 +- > >> src/amf/amfd/sgproc.cc | 14 ++-- > >> src/amf/amfd/su.cc | 37 +++++++-- > >> src/amf/amfd/su.h | 3 +- > >> src/amf/common/amf_defs.h | 3 + > >> src/amf/config/amf_classes.xml | 15 ++++ > >> src/amf/config/amf_objects.xml | 7 ++ > >> 13 files changed, 300 insertions(+), 27 deletions(-) > >> > >> > >> Testing Commands: > >> ----------------- > >> Test 1) > >> Turn on tracing for AMFND > >> > >> Ensure ?restrict auto repair? is disabled:?immcfg -a > >> amfRestrictAutoRepairEnable=0 amfConfig=1,safApp=safAmfService > >> > >> Set campaign on an SU:?immcfg -a saAmfSUMaintenanceCampaign="foo" > >> safSu=PL-5,safSg=NoRed,safApp=OpenSAF > >> > >> Enable ?restrict auto repair':?immcfg -a amfRestrictAutoRepairEnable=1 > >> amfConfig=1,safApp=safAmfService > >> > >> Ensure the change is propagated to AMFND on PL-5 by looking for this in > >> the trace: > >> TRACE("suMaintenanceCampaign for '%s' changed to '%s'",? > >> su->name.c_str(), su->suMaintenanceCampaign.c_str()); > >> > >> Test 2) > >> 1. Enable 'restrict auto repair' > >> 2. "amf-adm restart <comp>" should work when suMaintenence is set for > >> enclosing SU > >> 3. kill <comp> when suMaintenance is set, auto repair will not occur > >> > >> > >> Testing, Expected Results: > >> -------------------------- > >> *** PASTE COMMAND OUTPUTS / TEST RESULTS *** > >> > >> > >> Conditions of Submission: > >> ------------------------- > >> *** HOW MANY DAYS BEFORE PUSHING, CONSENSUS ETC *** > >> > >> > >> Arch Built Started Linux distro > >> ------------------------------------------- > >> mips n n > >> mips64 n n > >> x86 n n > >> x86_64 y y > >> powerpc n n > >> powerpc64 n n > >> > >> > >> Reviewer Checklist: > >> ------------------- > >> [Submitters: make sure that your review doesn't trigger any checkmarks!] > >> > >> > >> Your checkin has not passed review because (see checked entries): > >> > >> ___ Your RR template is generally incomplete; it has too many blank entries > >> that need proper data filled in. > >> > >> ___ You have failed to nominate the proper persons for review and push. > >> > >> ___ Your patches do not have proper short+long header > >> > >> ___ You have grammar/spelling in your header that is unacceptable. > >> > >> ___ You have exceeded a sensible line length in your headers/comments/text. > >> > >> ___ You have failed to put in a proper Trac Ticket # into your commits. > >> > >> ___ You have incorrectly put/left internal data in your comments/files > >> (i.e. internal bug tracking tool IDs, product names etc) > >> > >> ___ You have not given any evidence of testing beyond basic build tests. > >> Demonstrate some level of runtime or other sanity testing. > >> > >> ___ You have ^M present in some of your files. These have to be removed. > >> > >> ___ You have needlessly changed whitespace or added whitespace crimes > >> like trailing spaces, or spaces before tabs. > >> > >> ___ You have mixed real technical changes with whitespace and other > >> cosmetic code cleanup changes. These have to be separate commits. > >> > >> ___ You need to refactor your submission into logical chunks; there is > >> too much content into a single commit. > >> > >> ___ You have extraneous garbage in your review (merge commits etc) > >> > >> ___ You have giant attachments which should never have been sent; > >> Instead you should place your content in a public tree to be pulled. > >> > >> ___ You have too many commits attached to an e-mail; resend as threaded > >> commits, or place in a public tree for a pull. > >> > >> ___ You have resent this content multiple times without a clear indication > >> of what has changed between each re-send. > >> > >> ___ You have failed to adequately and individually address all of the > >> comments and change requests that were proposed in the initial review. > >> > >> ___ You have a misconfigured ~/.gitconfig file (i.e. user.name, > >> user.email etc) > >> > >> ___ Your computer have a badly configured date and time; confusing the > >> the threaded patch review. > >> > >> ___ Your changes affect IPC mechanism, and you don't present any results > >> for in-service upgradability test. > >> > >> ___ Your changes affect user manual and documentation, your patch series > >> do not contain the patch that updates the Doxygen manual. > >> > >> > >> > >> > >> ------------------------------ > >> > >> Message: 4 > >> Date: Fri, 21 Apr 2017 19:51:34 +1000 > >> From: Gary Lee <[email protected]> > >> Subject: [devel] [PATCH 1/1] amfd: make auto repair restriction > >> configurable [#2435] > >> To: [email protected] <mailto:[email protected]> > >> Cc: [email protected] > >> <mailto:[email protected]> > >> Message-ID: > >> <[email protected]> > >> > >> This adds a configuration object for AMF at > >> amfConfig=1,safApp=safAmfService. > >> > >> A configuration attribute 'amfRestrictAutoRepairEnable' is added. > >> This determines if 'suMaintenanceCampaign' should be ignored to maintain > >> legacy AMF behaviour. The default behaviour is not to support auto repair > >> restriction. > >> > >> To enable restriction: > >> immcfg -a amfRestrictAutoRepairEnable=1 amfConfig=1,safApp=safAmfService > >> > >> To disable restriction: > >> immcfg -a amfRestrictAutoRepairEnable=0 amfConfig=1,safApp=safAmfService > >> --- > >> src/amf/Makefile.am | 3 + > >> src/amf/amfd/comp.cc | 2 +- > >> src/amf/amfd/config.cc | 179 +++++++++++++++++++++++++++++++++++++++++ > >> src/amf/amfd/config.h | 21 +++++ > >> src/amf/amfd/imm.cc | 35 ++++++-- > >> src/amf/amfd/ndproc.cc | 4 +- > >> src/amf/amfd/node.cc | 4 +- > >> src/amf/amfd/sgproc.cc | 14 ++-- > >> src/amf/amfd/su.cc | 37 +++++++-- > >> src/amf/amfd/su.h | 3 +- > >> src/amf/common/amf_defs.h | 3 + > >> src/amf/config/amf_classes.xml | 15 ++++ > >> src/amf/config/amf_objects.xml | 7 ++ > >> 13 files changed, 300 insertions(+), 27 deletions(-) > >> create mode 100644 src/amf/amfd/config.cc > >> create mode 100644 src/amf/amfd/config.h > >> > >> diff --git a/src/amf/Makefile.am b/src/amf/Makefile.am > >> index 8c175c2..1d6ca60 100644 > >> --- a/src/amf/Makefile.am > >> +++ b/src/amf/Makefile.am > >> @@ -103,6 +103,7 @@ noinst_HEADERS += \ > >> src/amf/amfd/clm.h \ > >> src/amf/amfd/cluster.h \ > >> src/amf/amfd/comp.h \ > >> + src/amf/amfd/config.h \ > >> src/amf/amfd/csi.h \ > >> src/amf/amfd/def.h \ > >> src/amf/amfd/evt.h \ > >> @@ -213,6 +214,7 @@ bin_testamfd_LDFLAGS = \ > >> src/amf/amfd/bin_osafamfd-ckpt_updt.o \ > >> src/amf/amfd/bin_osafamfd-clm.o \ > >> src/amf/amfd/bin_osafamfd-cluster.o \ > >> + src/amf/amfd/bin_osafamfd-config.o \ > >> src/amf/amfd/bin_osafamfd-comp.o \ > >> src/amf/amfd/bin_osafamfd-compcstype.o \ > >> src/amf/amfd/bin_osafamfd-comptype.o \ > >> @@ -300,6 +302,7 @@ bin_osafamfd_SOURCES = \ > >> src/amf/amfd/comp.cc \ > >> src/amf/amfd/compcstype.cc \ > >> src/amf/amfd/comptype.cc \ > >> + src/amf/amfd/config.cc \ > >> src/amf/amfd/csi.cc \ > >> src/amf/amfd/csiattr.cc \ > >> src/amf/amfd/cstype.cc \ > >> diff --git a/src/amf/amfd/comp.cc b/src/amf/amfd/comp.cc > >> index d4b51a6..3e0dc5d 100644 > >> --- a/src/amf/amfd/comp.cc > >> +++ b/src/amf/amfd/comp.cc > >> @@ -152,7 +152,7 @@ void > >> AVD_COMP::avd_comp_pres_state_set(SaAmfPresenceStateT pres_state) { > >> (saAmfCompPresenceState == SA_AMF_PRESENCE_TERMINATION_FAILED)) || > >> ((node->saAmfNodeFailfastOnInstantiationFailure == true) && > >> (saAmfCompPresenceState == SA_AMF_PRESENCE_INSTANTIATION_FAILED))) && > >> - (su->saAmfSUMaintenanceCampaign.empty())) { > >> + (su->restrict_auto_repair() == false)) { > >> saflog(LOG_NOTICE, amfSvcUsrName, "%s PresenceState %s => %s", > >> osaf_extended_name_borrow(&comp_info.name), > >> avd_pres_state_name[old_state], avd_pres_state_name[pres_state]); > >> diff --git a/src/amf/amfd/config.cc b/src/amf/amfd/config.cc > >> new file mode 100644 > >> index 0000000..bdb07d2 > >> --- /dev/null > >> +++ b/src/amf/amfd/config.cc > >> @@ -0,0 +1,179 @@ > >> +#include "amf/amfd/util.h" > >> +#include "amf/common/amf_util.h" > >> +#include "amf/amfd/imm.h" > >> +#include "amf/amfd/node.h" > >> +#include "amf/amfd/config.h" > >> + > >> +static Configuration _configuration; > >> +Configuration *configuration = &_configuration; > >> + > >> +static void ccb_apply_modify_hdlr(struct CcbUtilOperationData *opdata) { > >> + TRACE_ENTER(); > >> + const SaImmAttrModificationT_2 *attr_mod; > >> + int i = 0; > >> + > >> + while ((attr_mod = opdata->param.modify.attrMods[i++]) != nullptr) { > >> + if (!strcmp(attr_mod->modAttr.attrName, "amfRestrictAutoRepairEnable")) { > >> + bool enabled = false; // default to disabled > >> + if (attr_mod->modType != SA_IMM_ATTR_VALUES_DELETE && > >> + attr_mod->modAttr.attrValues != nullptr) { > >> + enabled = > >> + static_cast<bool>(*((SaUint32T *)attr_mod->modAttr.attrValues[0])); > >> + } > >> + TRACE("amfRestrictAutoRepairEnable changed to '%d'", enabled); > >> + configuration->restrict_auto_repair(enabled); > >> + } > >> + } > >> + TRACE_LEAVE(); > >> +} > >> + > >> +static SaAisErrorT ccb_completed_modify_hdlr(CcbUtilOperationData_t > >> *opdata) { > >> + TRACE_ENTER(); > >> + SaAisErrorT rc = SA_AIS_OK; > >> + TRACE_LEAVE(); > >> + return rc; > >> +} > >> + > >> +static SaAisErrorT configuration_ccb_completed_cb( > >> + CcbUtilOperationData_t *opdata) { > >> + SaAisErrorT rc = SA_AIS_ERR_BAD_OPERATION; > >> + > >> + TRACE_ENTER2("CCB ID %llu, '%s'", opdata->ccbId, > >> + osaf_extended_name_borrow(&opdata->objectName)); > >> + > >> + switch (opdata->operationType) { > >> + case CCBUTIL_CREATE: > >> + report_ccb_validation_error(opdata, > >> + "OpenSafAmfConfig objects cannot be created"); > >> + goto done; > >> + break; > >> + case CCBUTIL_MODIFY: > >> + rc = ccb_completed_modify_hdlr(opdata); > >> + break; > >> + case CCBUTIL_DELETE: > >> + report_ccb_validation_error(opdata, > >> + "OpenSafAmfConfig objects cannot be deleted"); > >> + goto done; > >> + break; > >> + default: > >> + osafassert(0); > >> + break; > >> + } > >> + > >> +done: > >> + TRACE_LEAVE(); > >> + return rc; > >> +} > >> + > >> +static void configuration_ccb_apply_cb(CcbUtilOperationData_t *opdata) { > >> + TRACE_ENTER2("CCB ID %llu, '%s'", opdata->ccbId, > >> + osaf_extended_name_borrow(&opdata->objectName)); > >> + > >> + switch (opdata->operationType) { > >> + case CCBUTIL_MODIFY: > >> + ccb_apply_modify_hdlr(opdata); > >> + break; > >> + case CCBUTIL_CREATE: > >> + case CCBUTIL_DELETE: > >> + /* fall through */ > >> + default: > >> + osafassert(0); > >> + break; > >> + } > >> + TRACE_LEAVE(); > >> +} > >> + > >> +static void configuration_admin_op_cb(SaImmOiHandleT immOiHandle, > >> + SaInvocationT invocation, > >> + const SaNameT *object_name, > >> + SaImmAdminOperationIdT op_id, > >> + const SaImmAdminOperationParamsT_2 **params) { > >> + TRACE_ENTER(); > >> + switch (op_id) { > >> + default: > >> + report_admin_op_error(immOiHandle, invocation, SA_AIS_ERR_NOT_SUPPORTED, > >> + nullptr, "Not supported"); > >> + break; > >> + } > >> + TRACE_LEAVE(); > >> +} > >> + > >> +/** > >> + * Get configuration for the AMF configuration object from IMM > >> + */ > >> +SaAisErrorT Configuration::get_config(void) { > >> + SaAisErrorT error = SA_AIS_ERR_FAILED_OPERATION; > >> + SaImmSearchHandleT searchHandle; > >> + SaImmSearchParametersT_2 searchParam; > >> + SaNameT dn; > >> + const SaImmAttrValuesT_2 **attributes; > >> + const char *className = "OpenSafAmfConfig"; > >> + > >> + TRACE_ENTER(); > >> + > >> + searchParam.searchOneAttr.attrName = > >> + const_cast<SaImmAttrNameT>("SaImmAttrClassName"); > >> + searchParam.searchOneAttr.attrValueType = SA_IMM_ATTR_SASTRINGT; > >> + searchParam.searchOneAttr.attrValue = &className; > >> + > >> + if (immutil_saImmOmSearchInitialize_2( > >> + avd_cb->immOmHandle, nullptr, SA_IMM_SUBTREE, > >> + SA_IMM_SEARCH_ONE_ATTR | SA_IMM_SEARCH_GET_ALL_ATTR, &searchParam, > >> + nullptr, &searchHandle) != SA_AIS_OK) { > >> + LOG_WA("saImmOmSearchInitialize_2 failed: %u", error); > >> + goto done1; > >> + } > >> + > >> + while (immutil_saImmOmSearchNext_2(searchHandle, &dn, > >> + (SaImmAttrValuesT_2 ***)&attributes) == SA_AIS_OK) { > >> + uint32_t value; > >> + TRACE("reading configuration '%s'", osaf_extended_name_borrow(&dn)); > >> + if (immutil_getAttr("amfRestrictAutoRepairEnable", attributes, 0, &value) > >> + == SA_AIS_OK) { > >> + configuration->restrict_auto_repair(static_cast<bool>(value)); > >> + } > >> + } > >> + > >> + error = SA_AIS_OK; > >> + TRACE("amfRestrictAutoRepairEnable set to '%d'", > >> + restrict_auto_repair_enabled()); > >> + > >> + (void)immutil_saImmOmSearchFinalize(searchHandle); > >> +done1: > >> + TRACE_LEAVE(); > >> + return error; > >> +} > >> + > >> +Configuration::Configuration() : restrict_auto_repair_(false) // default > >> +{ > >> + avd_class_impl_set("OpenSafAmfConfig", nullptr, > >> configuration_admin_op_cb, > >> + configuration_ccb_completed_cb, configuration_ccb_apply_cb); > >> +} > >> + > >> +Configuration::~Configuration() > >> +{ > >> +} > >> + > >> +void Configuration::restrict_auto_repair(bool enable) > >> +{ > >> + TRACE_ENTER(); > >> + restrict_auto_repair_ = enable; > >> + > >> + // notify AMFNDs of this ... > >> + for (const auto &value : *node_name_db) { > >> + AVD_AVND *avnd = value.second; > >> + osafassert(avnd != nullptr); > >> + for (const auto &su : avnd->list_of_ncs_su) { > >> + su->set_su_maintenance_campaign(); > >> + } > >> + for (const auto &su : avnd->list_of_su) { > >> + su->set_su_maintenance_campaign(); > >> + } > >> + } > >> + TRACE_LEAVE(); > >> +} > >> + > >> +bool Configuration::restrict_auto_repair_enabled() > >> +{ > >> + return restrict_auto_repair_; > >> +} > >> diff --git a/src/amf/amfd/config.h b/src/amf/amfd/config.h > >> new file mode 100644 > >> index 0000000..6a110db > >> --- /dev/null > >> +++ b/src/amf/amfd/config.h > >> @@ -0,0 +1,21 @@ > >> +#ifndef AMFD_CONFIG_H_ > >> +#define AMFD_CONFIG_H_ > >> + > >> +#include "osaf/saf/saAis.h" > >> + > >> +class Configuration { > >> + public: > >> + Configuration(); > >> + ~Configuration(); > >> + SaAisErrorT get_config(void); > >> + bool restrict_auto_repair_enabled(); > >> + void restrict_auto_repair(bool enable); > >> + private: > >> + bool restrict_auto_repair_; > >> + Configuration(const Configuration&) = delete; > >> + Configuration& operator=(const Configuration&) = delete; > >> +}; > >> + > >> +extern Configuration *configuration; > >> + > >> +#endif > >> diff --git a/src/amf/amfd/imm.cc b/src/amf/amfd/imm.cc > >> index d59c2c4..dd842d9 100644 > >> --- a/src/amf/amfd/imm.cc > >> +++ b/src/amf/amfd/imm.cc > >> @@ -49,6 +49,7 @@ > >> #include "amf/amfd/si.h" > >> #include "amf/amfd/csi.h" > >> #include "amf/amfd/si_dep.h" > >> +#include "amf/amfd/config.h" > >> #include "base/osaf_utility.h" > >> > >> #include "base/osaf_time.h" > >> @@ -107,7 +108,8 @@ type_map amf_type_map = { > >> {"safRdn", AVSV_SA_AMF_COMP_GLOBAL_ATTR}, > >> {"safHealthcheckKey", AVSV_SA_AMF_CLASS_INVALID}, > >> /* Common Version Related */ > >> - {"safVersion", AVSV_SA_AMF_CLASS_INVALID}}; > >> + {"safVersion", AVSV_SA_AMF_CLASS_INVALID}, > >> + {"amfConfig", AVSV_SA_AMF_CONFIGURATION}}; > >> > >> type_map versioned_types = {{"safAppType", AVSV_SA_AMF_APP_TYPE}, > >> {"safSgType", AVSV_SA_AMF_SG_TYPE}, > >> @@ -554,7 +556,9 @@ static const char *avd_class_names[] = {"Invalid", > >> "SaAmfSIRankedSU", > >> > >> "SaAmfSIAssignment", > >> - "SaAmfCSIAssignment"}; > >> + "SaAmfCSIAssignment", > >> + > >> + "OpenSafAmfConfig"}; > >> > >> static AvdImmOiCcbApplyCallbackT ccb_apply_callback[AVSV_SA_AMF_CLASS_MAX]; > >> static AvdImmOiCcbCompletedCallbackT > >> @@ -1482,13 +1486,20 @@ SaAisErrorT avd_imm_impl_set(void) { > >> if ((nullptr != ccb_completed_callback[i]) && > >> (rc = immutil_saImmOiClassImplementerSet( > >> avd_cb->immOiHandle, avd_class_names[i])) != SA_AIS_OK) { > >> - LOG_ER("Impl Set Failed for %s, returned %d", avd_class_names[i], rc); > >> - break; > >> + > >> + if (rc == SA_AIS_ERR_NOT_EXIST) { > >> + LOG_WA( > >> + "Impl Set Failed for %s, returned %d. Please check IMM schema is > >> up-to-date.", > >> + avd_class_names[i], rc); > >> + rc = SA_AIS_OK; > >> + } else { > >> + LOG_ER("Impl Set Failed for %s, returned %d", avd_class_names[i], rc); > >> + break; > >> + } > >> } > >> } > >> > >> avd_cb->is_implementer = true; > >> - > >> TRACE_LEAVE2("%u", rc); > >> return rc; > >> } > >> @@ -1521,8 +1532,16 @@ SaAisErrorT avd_imm_applier_set(void) { > >> if ((nullptr != ccb_completed_callback[i]) && > >> (rc = immutil_saImmOiClassImplementerSet( > >> avd_cb->immOiHandle, avd_class_names[i])) != SA_AIS_OK) { > >> - LOG_ER("Impl Set Failed for %s, returned %d", avd_class_names[i], rc); > >> - break; > >> + > >> + if (rc == SA_AIS_ERR_NOT_EXIST) { > >> + LOG_WA( > >> + "Impl Set Failed for %s, returned %d. Please check IMM schema is > >> up-to-date.", > >> + avd_class_names[i], rc); > >> + rc = SA_AIS_OK; > >> + } else { > >> + LOG_ER("Impl Set Failed for %s, returned %d", avd_class_names[i], rc); > >> + break; > >> + } > >> } > >> } > >> > >> @@ -1651,6 +1670,8 @@ unsigned int avd_imm_config_get(void) { > >> /* retrieve hydra configuration from IMM */ > >> if (hydra_config_get() != SA_AIS_OK) goto done; > >> > >> + if (configuration->get_config() != SA_AIS_OK) goto done; > >> + > >> // SGs needs to adjust configuration once all instances have been added > >> { > >> for (const auto &value : *sg_db) { > >> diff --git a/src/amf/amfd/ndproc.cc b/src/amf/amfd/ndproc.cc > >> index 148e929..e80a0b3 100644 > >> --- a/src/amf/amfd/ndproc.cc > >> +++ b/src/amf/amfd/ndproc.cc > >> @@ -159,13 +159,13 @@ void avd_reg_su_evh(AVD_CL_CB *cb, AVD_EVT *evt) { > >> > >> // Before any SU gets instantiated, send Maintenance campaign update. > >> for (const auto &su : node->list_of_ncs_su) { > >> - if (su->saAmfSUMaintenanceCampaign.empty() == false) { > >> + if (su->restrict_auto_repair() == true) { > >> TRACE("Sending Maintenance campaign info for '%s'", su->name.c_str()); > >> su->set_su_maintenance_campaign(); > >> } > >> } > >> for (const auto &su : node->list_of_su) { > >> - if (su->saAmfSUMaintenanceCampaign.empty() == false) { > >> + if (su->restrict_auto_repair() == true) { > >> TRACE("Sending Maintenance campaign info for '%s'", su->name.c_str()); > >> su->set_su_maintenance_campaign(); > >> } > >> diff --git a/src/amf/amfd/node.cc b/src/amf/amfd/node.cc > >> index 1e364de..f537596 100644 > >> --- a/src/amf/amfd/node.cc > >> +++ b/src/amf/amfd/node.cc > >> @@ -1652,11 +1652,11 @@ void avd_node_constructor(void) { > >> bool AVD_AVND::is_campaign_set_for_all_sus() const { > >> if (std::all_of(list_of_ncs_su.begin(), list_of_ncs_su.end(), > >> [&](AVD_SU *su) -> bool { > >> - return su->saAmfSUMaintenanceCampaign.empty() == false; > >> + return su->restrict_auto_repair() == true; > >> })) { > >> if (std::all_of(list_of_su.begin(), list_of_su.end(), > >> [&](AVD_SU *su) -> bool { > >> - return su->saAmfSUMaintenanceCampaign.empty() == false; > >> + return su->restrict_auto_repair() == true; > >> })) { > >> return true; > >> } else { > >> diff --git a/src/amf/amfd/sgproc.cc b/src/amf/amfd/sgproc.cc > >> index 3698017..cd95fe8 100644 > >> --- a/src/amf/amfd/sgproc.cc > >> +++ b/src/amf/amfd/sgproc.cc > >> @@ -309,7 +309,7 @@ void su_try_repair(const AVD_SU *su) { > >> (su->saAmfSUOperState == SA_AMF_OPERATIONAL_DISABLED) && > >> (su->saAmfSUPresenceState != SA_AMF_PRESENCE_INSTANTIATION_FAILED) && > >> (su->saAmfSUPresenceState != SA_AMF_PRESENCE_TERMINATION_FAILED) && > >> - (su->saAmfSUMaintenanceCampaign.empty())) { > >> + (su->restrict_auto_repair() == false)) { > >> saflog(LOG_NOTICE, amfSvcUsrName, > >> "Ordering Auto repair of '%s' as sufailover repair action", > >> su->name.c_str()); > >> @@ -639,7 +639,7 @@ static void perform_nodeswitchover_recovery(AVD_AVND > >> *node) { > >> for (const auto &su : node->list_of_su) { > >> if (su->list_of_susi == nullptr) continue; > >> > >> - if (!su->saAmfSUMaintenanceCampaign.empty()) node_reboot = false; > >> + if (su->restrict_auto_repair() == true) node_reboot = false; > >> > >> if (su_recover_from_fault(su) == NCSCC_RC_FAILURE) { > >> LOG_ER("%s:%d %s", __FUNCTION__, __LINE__, su->name.c_str()); > >> @@ -787,7 +787,7 @@ void avd_su_oper_state_evh(AVD_CL_CB *cb, AVD_EVT > >> *evt) { > >> TRACE("Component in %s requested FAILFAST", su->name.c_str()); > >> } > >> > >> - if (!su->saAmfSUMaintenanceCampaign.empty()) { > >> + if (su->restrict_auto_repair() == true) { > >> saflog( > >> LOG_NOTICE, amfSvcUsrName, > >> "Node Fail-Fast disabled because maintenance campaign %s is set for su: > >> %s", > >> @@ -839,7 +839,7 @@ void avd_su_oper_state_evh(AVD_CL_CB *cb, AVD_EVT > >> *evt) { > >> if (su->sg_of_su->sg_ncs_spec == true) { > >> su->set_oper_state(SA_AMF_OPERATIONAL_DISABLED); > >> > >> - if (!su->saAmfSUMaintenanceCampaign.empty()) { > >> + if (su->restrict_auto_repair() == true) { > >> saflog( > >> LOG_NOTICE, amfSvcUsrName, > >> "Node Fail-Fast disabled because maintenance campaign %s is set for su: > >> %s", > >> @@ -890,13 +890,13 @@ void avd_su_oper_state_evh(AVD_CL_CB *cb, AVD_EVT > >> *evt) { > >> node->recvr_fail_sw = true; > >> > >> // if maintenance campaign is ongoing, disable node reboot > >> - if (!su->saAmfSUMaintenanceCampaign.empty()) node_reboot_req = false; > >> + if (su->restrict_auto_repair() == true) node_reboot_req = false; > >> > >> switch (n2d_msg->msg_info.n2d_opr_state.rec_rcvr.raw) { > >> case SA_AMF_NODE_FAILOVER: > >> if ((node->node_info.nodeId == cb->node_id_avd) && > >> (node->saAmfNodeAutoRepair) && > >> - (su->saAmfSUMaintenanceCampaign.empty())) { > >> + (su->restrict_auto_repair() == false)) { > >> /* This is a case when Act ctlr is rebooting. Don't do appl > >> failover as of now because during appl failover if Act > >> controller reboots, then there may be packet losses. Anyway, > >> @@ -1731,7 +1731,7 @@ void avd_su_si_assign_evh(AVD_CL_CB *cb, AVD_EVT > >> *evt) { > >> are in no_red model. We are doing the same thing for controller also. */ > >> bool maintenanceCampaignSet(false); > >> > >> - if (su && !su->saAmfSUMaintenanceCampaign.empty()) > >> + if (su && su->restrict_auto_repair() == true) > >> maintenanceCampaignSet = true; > >> > >> for (const auto &temp_su : node->list_of_su) { > >> diff --git a/src/amf/amfd/su.cc b/src/amf/amfd/su.cc > >> index 84c05b7..fac1188 100644 > >> --- a/src/amf/amfd/su.cc > >> +++ b/src/amf/amfd/su.cc > >> @@ -30,6 +30,7 @@ > >> #include "amf/amfd/proc.h" > >> #include "amf/amfd/csi.h" > >> #include "amf/amfd/cluster.h" > >> +#include "config.h" > >> #include <algorithm> > >> > >> AmfDb<std::string, AVD_SU> *su_db = nullptr; > >> @@ -766,7 +767,7 @@ void AVD_SU::set_pres_state(SaAmfPresenceStateT > >> pres_state) { > >> (su_on_node->saAmfNodeFailfastOnTerminationFailure == true) && > >> (sg_of_su->saAmfSGAutoRepair == true) && > >> (su_on_node->saAmfNodeAutoRepair == true) && > >> - (saAmfSUMaintenanceCampaign.empty())) > >> + (restrict_auto_repair() == false)) > >> /* According to AMF B.04.01 Section 4.8 Page 214 if user configures > >> saAmfNodeFailfastOnTerminationFailure = true, AMF has to perform > >> node failfast recovery action. So mark SU to > >> @@ -777,7 +778,7 @@ void AVD_SU::set_pres_state(SaAmfPresenceStateT > >> pres_state) { > >> (su_on_node->saAmfNodeFailfastOnInstantiationFailure == true) && > >> (sg_of_su->saAmfSGAutoRepair == true) && > >> (su_on_node->saAmfNodeAutoRepair == true) && > >> - (saAmfSUMaintenanceCampaign.empty())) > >> + (restrict_auto_repair() == false)) > >> /* According to AMF B.04.01 Section 4.6 Page 212 if user configures > >> saAmfNodeFailfastOnInstantiationFailure = true, AMF has to perform > >> node failfast recovery action. So mark SU to > >> @@ -839,8 +840,15 @@ void AVD_SU::set_oper_state(SaAmfOperationalStateT > >> oper_state) { > >> > >> saAmfSUOperState = oper_state; > >> > >> - avd_send_oper_chg_ntf(name, SA_AMF_NTFID_SU_OP_STATE, old_state, > >> - saAmfSUOperState, &saAmfSUMaintenanceCampaign); > >> + if (restrict_auto_repair() == true) { > >> + avd_send_oper_chg_ntf(name, SA_AMF_NTFID_SU_OP_STATE, old_state, > >> + saAmfSUOperState, &saAmfSUMaintenanceCampaign); > >> + } else { > >> + // if restrict auto repair is not enabled, *do not* send > >> + // campaign in notification to SMF, for backwards compatability > >> + avd_send_oper_chg_ntf(name, SA_AMF_NTFID_SU_OP_STATE, old_state, > >> + saAmfSUOperState); > >> + } > >> > >> avd_saImmOiRtObjectUpdate(name, "saAmfSUOperState", SA_IMM_ATTR_SAUINT32T, > >> &saAmfSUOperState); > >> @@ -2194,9 +2202,14 @@ void > >> AVD_SU::send_attribute_update(AVSV_AMF_SU_ATTR_ID attrib_id) { > >> } > >> case saAmfSUMaintenanceCampaign_ID: { > >> param.attr_id = saAmfSUMaintenanceCampaign_ID; > >> - param.value_len = saAmfSUMaintenanceCampaign.length(); > >> - memcpy(¶m.value[0], saAmfSUMaintenanceCampaign.data(), > >> - param.value_len); > >> + if (restrict_auto_repair() == true) { > >> + param.value_len = saAmfSUMaintenanceCampaign.length(); > >> + memcpy(¶m.value[0], saAmfSUMaintenanceCampaign.data(), > >> + param.value_len); > >> + } else { > >> + param.value_len = 0; > >> + param.value[0] = '\0'; > >> + } > >> break; > >> } > >> default: > >> @@ -2710,3 +2723,13 @@ void > >> AVD_SU::update_susis_in_imm_and_ntf(SaAmfHAStateT ha_state) const { > >> avd_gen_su_ha_state_changed_ntf(avd_cb, susi); > >> } > >> } > >> + > >> +bool AVD_SU::restrict_auto_repair() const > >> +{ > >> + if (saAmfSUMaintenanceCampaign.empty() == false && > >> + configuration->restrict_auto_repair_enabled()) { > >> + return true; > >> + } > >> + > >> + return false; > >> +} > >> \ No newline at end of file > >> diff --git a/src/amf/amfd/su.h b/src/amf/amfd/su.h > >> index 351365b..600b8f5 100644 > >> --- a/src/amf/amfd/su.h > >> +++ b/src/amf/amfd/su.h > >> @@ -55,7 +55,7 @@ class AVD_SU { > >> bool saAmfSUFailover; > >> /* true when user has configured saAmfSUFailover */ > >> bool saAmfSUFailover_configured; > >> - std::string saAmfSUMaintenanceCampaign; > >> + bool restrict_auto_repair() const; > >> > >> /* runtime attributes */ > >> SaBoolT saAmfSUPreInstantiable; // TODO(hafe) change to bool > >> @@ -96,6 +96,7 @@ class AVD_SU { > >> > >> AVD_SUTYPE *su_type; > >> AVD_SU *su_list_su_type_next; > >> + std::string saAmfSUMaintenanceCampaign; > >> > >> void set_su_failover(bool value); > >> void set_su_maintenance_campaign(void); > >> diff --git a/src/amf/common/amf_defs.h b/src/amf/common/amf_defs.h > >> index 8433214..24549b3 100644 > >> --- a/src/amf/common/amf_defs.h > >> +++ b/src/amf/common/amf_defs.h > >> @@ -154,6 +154,9 @@ typedef enum { > >> AVSV_SA_AMF_SI_ASSIGNMENT = 33, > >> AVSV_SA_AMF_CSI_ASSIGNMENT = 34, > >> > >> + /* AMF configuration */ > >> + AVSV_SA_AMF_CONFIGURATION = 35, > >> + > >> AVSV_SA_AMF_CLASS_MAX > >> } AVSV_AMF_CLASS_ID; > >> > >> diff --git a/src/amf/config/amf_classes.xml > >> b/src/amf/config/amf_classes.xml > >> index ee0f185..19a6232 100644 > >> --- a/src/amf/config/amf_classes.xml > >> +++ b/src/amf/config/amf_classes.xml > >> @@ -1438,4 +1438,19 @@ > >> <flag>SA_WRITABLE</flag> > >> </attr> > >> </class> > >> + <class name="OpenSafAmfConfig"> > >> + <category>SA_CONFIG</category> > >> + <rdn> > >> + <name>amfConfig</name> > >> + <type>SA_STRING_T</type> > >> + <category>SA_CONFIG</category> > >> + <flag>SA_INITIALIZED</flag> > >> + </rdn> > >> + <attr> > >> + <name>amfRestrictAutoRepairEnable</name> > >> + <type>SA_UINT32_T</type> > >> + <category>SA_CONFIG</category> > >> + <flag>SA_WRITABLE</flag> > >> + </attr> > >> + </class> > >> </imm:IMM-contents> > >> diff --git a/src/amf/config/amf_objects.xml > >> b/src/amf/config/amf_objects.xml > >> index 502fc2f..31fc3e5 100644 > >> --- a/src/amf/config/amf_objects.xml > >> +++ b/src/amf/config/amf_objects.xml > >> @@ -1,5 +1,12 @@ > >> <?xml version="1.0" encoding="utf-8"?> > >> <imm:IMM-contents xmlns:imm="http://www.saforum.org/IMMSchema"; > >> xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; > >> xsi:noNamespaceSchemaLocation="SAI-AIS-IMM-XSD-A.02.13.xsd"> > >> + <object class="OpenSafAmfConfig"> > >> + <dn>amfConfig=1,safApp=safAmfService</dn> > >> + <attr> > >> + <name>amfRestrictAutoRepairEnable</name> > >> + <value>0</value> > >> + </attr> > >> + </object> > >> <object class="SaAmfAppBaseType"> > >> <dn>safAppType=OpenSafApplicationType</dn> > >> </object> > >> -- > >> 1.9.1 > >> > > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
