Hi Alex I think that calling ntfFinalize() is enough. If you want to add a comment about that all subscriptions are unistalled it's ok. I hope you saw my mail about a possible NBC
Thanks Lennart > -----Original Message----- > From: Alex Jones [mailto:ajo...@genband.com] > Sent: den 14 november 2016 14:58 > To: Lennart Lund <lennart.l...@ericsson.com>; > reddy.neelaka...@oracle.com; Rafael Odzakow > <rafael.odza...@ericsson.com> > Cc: opensaf-devel@lists.sourceforge.net > Subject: Re: [PATCH 1 of 1] smfd: add support for asynchronous detection of > failed AMF entities [#2145] > > Hi Lennart, > > Yes, ntfFinalize does uninstall any subscriptions. I'm just being > explicit here, by putting it in the destructor. > > Do you want me to remove it before pushing? > > Alex > > On 11/14/2016 07:55 AM, Lennart Lund wrote: > > ------------------------------------------------------------------------ > > NOTICE: This email was received from an EXTERNAL sender > > ------------------------------------------------------------------------ > > > > Hi Alex > > > > Ack with comment > > See my comments/questions inline [Lennart] > > > > Thanks > > Lennart > > > >> -----Original Message----- > >> From: Alex Jones [mailto:ajo...@genband.com] > >> Sent: den 11 november 2016 20:50 > >> To: reddy.neelaka...@oracle.com; Lennart Lund > >> <lennart.l...@ericsson.com>; Rafael Odzakow > >> <rafael.odza...@ericsson.com> > >> Cc: opensaf-devel@lists.sourceforge.net > > <mailto:opensaf-devel@lists.sourceforge.net> > >> Subject: [PATCH 1 of 1] smfd: add support for asynchronous detection of > >> failed AMF entities [#2145] > >> > >> osaf/services/saf/smfsv/smfd/SmfCampaignThread.cc | 235 > >> +++++++++++++++++++++- > >> osaf/services/saf/smfsv/smfd/SmfCampaignThread.hh | 15 + > >> osaf/services/saf/smfsv/smfd/SmfStepTypes.cc | 32 +- > >> 3 files changed, 263 insertions(+), 19 deletions(-) > >> > >> > >> This patch adds support for section 4.2.1.3 of SMF A.01.02 spec. > >> > >> diff --git a/osaf/services/saf/smfsv/smfd/SmfCampaignThread.cc > >> b/osaf/services/saf/smfsv/smfd/SmfCampaignThread.cc > >> --- a/osaf/services/saf/smfsv/smfd/SmfCampaignThread.cc > >> +++ b/osaf/services/saf/smfsv/smfd/SmfCampaignThread.cc > >> @@ -39,6 +39,7 @@ > >> #include "SmfUtils.hh" > >> > >> SmfCampaignThread *SmfCampaignThread::s_instance = NULL; > >> +SaNtfSubscriptionIdT SmfCampaignThread::operStateSubId(1); > >> > >> #define SMF_RDA_RETRY_COUNT 25 /* This is taken as the csi's are > assigned > >> in parallel */ > >> > >> @@ -140,7 +141,12 @@ void SmfCampaignThread::main(NCSCONTEXT > >> SmfCampaignThread::~SmfCampaignThread() > >> { > >> TRACE_ENTER(); > >> - SaAisErrorT rc = saNtfFinalize(m_ntfHandle); > >> + SaAisErrorT rc(saNtfNotificationUnsubscribe(operStateSubId)); > >> + if (rc != SA_AIS_OK) { > >> + LOG_ER("Failed to unsubscribe to oper state notifications > >> %u", rc); > >> + } > > [Lennart] Why is this needed? Does not saNtfFinalize remove SMF as a NTF > > client including all subscriptions? > > > >> + > >> + rc = saNtfFinalize(m_ntfHandle); > >> if (rc != SA_AIS_OK) { > >> LOG_ER("Failed to finalize NTF handle %u", rc); > >> } > >> @@ -303,10 +309,14 @@ int SmfCampaignThread::initNtf(void) > >> { > >> SaAisErrorT rc = SA_AIS_ERR_TRY_AGAIN; > >> SaVersionT ntfVersion = { 'A', 1, 1 }; > >> + SaNtfCallbacksT callbacks = { > >> + ntfNotificationCallback, > >> + 0 > >> + }; > >> unsigned int numOfTries = 50; > >> > >> while (rc == SA_AIS_ERR_TRY_AGAIN && numOfTries > 0) { > >> - rc = saNtfInitialize(&m_ntfHandle, NULL, &ntfVersion); > >> + rc = saNtfInitialize(&m_ntfHandle, &callbacks, &ntfVersion); > >> if (rc != SA_AIS_ERR_TRY_AGAIN) { > >> break; > >> } > >> @@ -319,9 +329,202 @@ int SmfCampaignThread::initNtf(void) > >> return -1; > >> } > >> > >> + // subscribe to operational state change notifications generated by AMF > >> + rc = initNtfSubscriptions(); > >> + if (rc != SA_AIS_OK) { > >> + LOG_ER("initNtfSubscriptions FAILED rc=%s", saf_error(rc)); > >> + return -1; > >> + } > >> + > >> return 0; > >> } > >> > >> +SaAisErrorT SmfCampaignThread::initNtfSubscriptions(void) { > >> + TRACE_ENTER(); > >> + SaAisErrorT rc(SA_AIS_OK); > >> + > >> + do { > >> + SaNtfStateChangeNotificationFilterT stateChangeFilter; > >> + > >> + SaAisErrorT rc(saNtfStateChangeNotificationFilterAllocate( > >> + m_ntfHandle, > >> + &stateChangeFilter, > >> + 0, > >> + 0, > >> + 0, > >> + 0, > >> + 0, > >> + 0)); > >> + > >> + if (rc != SA_AIS_OK) { > >> + LOG_ER("saNtfAttributeChangeNotificationFilterAllocate FAILED rc=%s", > >> + saf_error(rc)); > >> + break; > >> + } > >> + > >> + SaNtfNotificationTypeFilterHandlesT notificationFilterHandles = { > >> + 0, > >> + 0, > >> + stateChangeFilter.notificationFilterHandle, > >> + 0, > >> + 0 > >> + }; > >> + > >> + rc = saNtfNotificationSubscribe(¬ificationFilterHandles, > >> operStateSubId); > >> + > >> + if (rc != SA_AIS_OK) { > >> + LOG_ER("saNtfNotificationSubscribe FAILED rc=%s", saf_error(rc)); > >> + break; > >> + } > >> + > >> + rc = > >> saNtfNotificationFilterFree(stateChangeFilter.notificationFilterHandle); > >> + if (rc != SA_AIS_OK) { > >> + LOG_ER("saNtfNotificationFilterFree FAILED rc=%s", saf_error(rc)); > >> + break; > >> + } > >> + } while (false); > >> + > >> + TRACE_LEAVE(); > >> + return rc; > >> +} > >> + > >> +bool SmfCampaignThread::isAMFOperState(const SaNtfClassIdT& > classId) > >> const { > >> + TRACE_ENTER(); > >> + bool status(false); > >> + > >> + if (classId.vendorId == SA_NTF_VENDOR_ID_SAF && > >> + classId.majorId == SA_SVC_AMF && > >> + classId.minorId == SA_AMF_NTFID_SU_OP_STATE) { > >> + status = true; > >> + } > >> + > >> + TRACE_LEAVE2("%i", status); > >> + return status; > >> +} > >> + > >> +void SmfCampaignThread::handleStateChangeNotification( > >> + const SaNtfStateChangeNotificationT& stateChangeNotification) { > >> + TRACE_ENTER(); > >> + if (stateChangeNotification.notificationHeader.eventType) { > >> + if (*stateChangeNotification.notificationHeader.eventType == > >> + SA_NTF_OBJECT_STATE_CHANGE) { > >> + handleObjectStateChangeNotification(stateChangeNotification); > >> + } else { > >> + TRACE("ignoring state change notification with event type: %i", > >> + *stateChangeNotification.notificationHeader.eventType); > >> + } > >> + } > >> + TRACE_LEAVE(); > >> +} > >> + > >> +void SmfCampaignThread::handleAmfObjectStateChangeNotification( > >> + const SaNtfStateChangeNotificationT& stateChangeNotification) { > >> + TRACE_ENTER(); > >> + > >> + do { > >> + bool disabled(false); > >> + > >> + // see if this is a failure of an upgraded SU > >> + for (SaUint16T i(0); i < stateChangeNotification.numStateChanges; i++) { > >> + if (stateChangeNotification.changedStates[i].newState == > >> + SA_AMF_OPERATIONAL_DISABLED) { > >> + disabled = true; > >> + break; > >> + } > >> + } > >> + > >> + if (!disabled) { > >> + TRACE("state change is not to DISABLED -- ignoring"); > >> + break; > >> + } > >> + > >> + for (SaUint16T i(0); > >> + i < stateChangeNotification.notificationHeader.numAdditionalInfo; > >> + i++) { > >> + if > > (stateChangeNotification.notificationHeader.additionalInfo[i].infoId != > >> + SA_AMF_MAINTENANCE_CAMPAIGN_DN || > >> + stateChangeNotification.notificationHeader.additionalInfo[i].infoType > >> + != SA_NTF_VALUE_LDAP_NAME) { > >> + continue; > >> + } > >> + > >> + SaNameT *name(0); > >> + SaUint16T dataSize(0); > >> + > >> + SaAisErrorT rc(saNtfPtrValGet( > >> + stateChangeNotification.notificationHandle, > >> + > >> &stateChangeNotification.notificationHeader.additionalInfo[i].infoValue, > >> + reinterpret_cast<void **>(&name), > >> + &dataSize)); > >> + > >> + if (rc == SA_AIS_OK) { > >> + SaConstStringT maintenanceCampaign(saAisNameBorrow(name)); > >> + > >> + if (!strcmp(maintenanceCampaign, > >> + s_instance->m_campaign->getDn().c_str())) { > >> + LOG_ER("SU: %s failed after upgrade in campaign", > >> + > >> > saAisNameBorrow(stateChangeNotification.notificationHeader.notificationO > >> bject)); > >> + s_instance->m_campaign->getUpgradeCampaign()->suspend(); > >> + s_instance->m_campaign->getUpgradeCampaign()->setCampState( > >> + SA_SMF_CMPG_ERROR_DETECTED); > >> + > >> + std::string error(saAisNameBorrow( > >> + stateChangeNotification.notificationHeader.notificationObject)); > >> + > >> + error += " failed after upgrade"; > >> + s_instance->m_campaign->setError(error); > >> + } else { > >> + LOG_ER("maintenance campaign received from AMF (%s) is not the " > >> + "same as ours (%s)", > >> + maintenanceCampaign, > >> + s_instance->m_campaign->getDn().c_str()); > >> + } > >> + } else { > >> + LOG_ER("saNtfPtrValGet failed: %d", rc); > >> + } > >> + } > >> + } while (false); > >> + > >> + TRACE_LEAVE(); > >> +} > >> + > >> +void SmfCampaignThread::handleObjectStateChangeNotification( > >> + const SaNtfStateChangeNotificationT& stateChangeNotification) { > >> + TRACE_ENTER(); > >> + > >> + if (stateChangeNotification.notificationHeader.notificationClassId && > >> + > >> > isAMFOperState(*stateChangeNotification.notificationHeader.notificationCl > >> assId)) { > >> + handleAmfObjectStateChangeNotification(stateChangeNotification); > >> + } else { > >> + TRACE("ignoring non-AMF state change notification"); > >> + } > >> + > >> + TRACE_LEAVE(); > >> +} > >> + > >> +void SmfCampaignThread::ntfNotificationCallback( > >> + SaNtfSubscriptionIdT subId, > >> + const SaNtfNotificationsT *notification) { > >> + TRACE_ENTER(); > >> + > >> + do { > >> + if (subId != operStateSubId) { > >> + TRACE("unknown subscription id received in ntfNotificationCallback: > >> %d", > >> + subId); > >> + break; > >> + } > >> + > >> + if (notification->notificationType == SA_NTF_TYPE_STATE_CHANGE) { > >> + s_instance->handleStateChangeNotification( > >> + notification->notification.stateChangeNotification); > >> + } else { > >> + TRACE("ignoring NTF notification of type: %i", > >> + notification->notificationType); > >> + } > >> + } while (false); > >> + > >> + TRACE_LEAVE(); > >> +} > >> /** > >> * SmfCampaignThread::send > >> * send event to the thread. > >> @@ -672,18 +875,31 @@ int SmfCampaignThread::handleEvents(void > >> { > >> TRACE_ENTER(); > >> NCS_SEL_OBJ mbx_fd = ncs_ipc_get_sel_obj(&m_mbx); > >> - struct pollfd fds[1]; > >> + > >> + SaSelectionObjectT ntfSelObj(0); > >> + > >> + SaAisErrorT rc(saNtfSelectionObjectGet(m_ntfHandle, &ntfSelObj)); > >> + if (rc != SA_AIS_OK) { > >> + LOG_ER("saNtfSelectionObjectGet FAILED - %s", saf_error(rc)); > >> + return 1; > >> + } > >> + > >> + struct pollfd fds[2]; > >> > >> /* Set up all file descriptors to listen to */ > >> fds[0].fd = mbx_fd.rmv_obj; > >> fds[0].events = POLLIN; > >> fds[0].revents = 0; > >> > >> + fds[1].fd = ntfSelObj; > >> + fds[1].events = POLLIN; > >> + fds[1].revents = 0; > >> + > >> TRACE("Campaign thread %s waiting for events", m_campaign- > >> >getDn().c_str()); > >> > >> while (m_running) { > >> > >> - int ret = poll(fds, 1, SMF_UPDATE_ELAPSED_TIME_INTERVAL); > >> + int ret = poll(fds, sizeof(fds) / sizeof(pollfd), > >> SMF_UPDATE_ELAPSED_TIME_INTERVAL); > >> > >> if (ret == -1) { > >> if (errno == EINTR) > >> @@ -699,7 +915,16 @@ int SmfCampaignThread::handleEvents(void > >> processEvt(); > >> } > >> > >> - m_campaign->updateElapsedTime(); > >> + if (fds[1].revents & POLLIN) { > >> + // dispatch NTF events > >> + rc = saNtfDispatch(m_ntfHandle, SA_DISPATCH_ALL); > >> + > >> + if (rc != SA_AIS_OK) { > >> + LOG_ER("saNtfDispatch FAILED - %s", saf_error(rc)); > >> + } > >> + } > >> + > >> + m_campaign->updateElapsedTime(); > >> } > >> TRACE_LEAVE(); > >> return 0; > >> diff --git a/osaf/services/saf/smfsv/smfd/SmfCampaignThread.hh > >> b/osaf/services/saf/smfsv/smfd/SmfCampaignThread.hh > >> --- a/osaf/services/saf/smfsv/smfd/SmfCampaignThread.hh > >> +++ b/osaf/services/saf/smfsv/smfd/SmfCampaignThread.hh > >> @@ -190,9 +190,24 @@ class SmfCampaignThread { > >> > >> SaAisErrorT createImmHandle(SmfCampaign * i_campaign); > >> SaAisErrorT deleteImmHandle(); > >> + SaAisErrorT initNtfSubscriptions(void); > >> + > >> + void handleStateChangeNotification(const > >> SaNtfStateChangeNotificationT&); > >> + > >> + void handleObjectStateChangeNotification( > >> + const SaNtfStateChangeNotificationT&); > >> + > >> + void handleAmfObjectStateChangeNotification( > >> + const SaNtfStateChangeNotificationT&); > >> + > >> + bool isAMFOperState(const SaNtfClassIdT&) const; > >> + > >> + static void ntfNotificationCallback(SaNtfSubscriptionIdT, > >> + const SaNtfNotificationsT *); > >> > >> static void main(NCSCONTEXT info); > >> static SmfCampaignThread *s_instance; > >> + static SaNtfSubscriptionIdT operStateSubId; > >> > >> NCSCONTEXT m_task_hdl; > >> SYSF_MBX m_mbx; /* mailbox */ > >> diff --git a/osaf/services/saf/smfsv/smfd/SmfStepTypes.cc > >> b/osaf/services/saf/smfsv/smfd/SmfStepTypes.cc > >> --- a/osaf/services/saf/smfsv/smfd/SmfStepTypes.cc > >> +++ b/osaf/services/saf/smfsv/smfd/SmfStepTypes.cc > >> @@ -1247,8 +1247,8 @@ SmfStepTypeNodeReboot::execute() > >> return false; > >> } > >> > >> - /* Modify information model and set maintenance status */ > >> - LOG_NO("STEP: Modify information model and set maintenance > >> status"); > >> + /* Modify information model */ > >> + LOG_NO("STEP: Modify information model"); > >> if (m_step->modifyInformationModel() != SA_AIS_OK) { > >> LOG_ER("Failed to Modify information model in step=%s",m_step- > >> >getRdn().c_str()); > >> return false; > >> @@ -1261,11 +1261,6 @@ SmfStepTypeNodeReboot::execute() > >> return false; > >> } > >> > >> - if (m_step->setMaintenanceStateActUnits() == false) { > >> - LOG_ER("Failed to set maintenance state in step=%s",m_step- > >> >getRdn().c_str()); > >> - return false; > >> - } > >> - > >> /* The action below is an add on to > > SMF.--------------------------------------- > >> */ > >> /* See if any of the software bundles installed at online installation has > >> the */ > >> /* saSmfBundleInstallOfflineScope attribute set to > >> SA_SMF_CMD_SCOPE_PLM_EE */ > >> @@ -1372,6 +1367,13 @@ SmfStepTypeNodeReboot::execute() > >> /* the units in the same state as before locking */ > >> m_step->copyDuInitStateToAu(); > >> > >> + /* Now that the node is up, set the maintenance status */ > >> + LOG_NO("STEP: Set Maintenance Status"); > >> + if (m_step->setMaintenanceStateActUnits() == false) { > >> + LOG_ER("Failed to set maintenance state in step=%s",m_step- > >> >getRdn().c_str()); > >> + return false; > >> + } > >> + > >> /* Instantiate activation units */ > >> LOG_NO("STEP: Instantiate activation units"); > >> if (m_step->instantiateActivationUnits() == false) { > >> @@ -1685,8 +1687,8 @@ SmfStepTypeNodeRebootAct::execute() > >> return false; > >> } > >> > >> - /* Modify information model and set maintenance status */ > >> - LOG_NO("STEP: Modify information model and set maintenance > >> status"); > >> + /* Modify information model */ > >> + LOG_NO("STEP: Modify information model"); > >> if (m_step->modifyInformationModel() != SA_AIS_OK) { > >> LOG_ER("Failed to Modify information model in step=%s",m_step- > >> >getRdn().c_str()); > >> return false; > >> @@ -1699,11 +1701,6 @@ SmfStepTypeNodeRebootAct::execute() > >> return false; > >> } > >> > >> - if (m_step->setMaintenanceStateActUnits() == false) { > >> - LOG_ER("Failed to set maintenance state in step=%s",m_step- > >> >getRdn().c_str()); > >> - return false; > >> - } > >> - > >> /* Offline installation of new software */ > >> LOG_NO("STEP: Offline installation of new software"); > >> if (m_step->offlineInstallNewBundles() == false) { > >> @@ -1746,6 +1743,13 @@ SmfStepTypeNodeRebootAct::execute() > >> /* the units in the same state as before locking */ > >> m_step->copyDuInitStateToAu(); > >> > >> + /* Now that the node is up, set the maintenance status */ > >> + LOG_NO("STEP: Set Maintenance Status"); > >> + if (m_step->setMaintenanceStateActUnits() == false) { > >> + LOG_ER("Failed to set maintenance state in step=%s",m_step- > >> >getRdn().c_str()); > >> + return false; > >> + } > >> + > >> /* Instantiate activation units */ > >> LOG_NO("STEP: Instantiate activation units"); > >> if (m_step->instantiateActivationUnits() == false) { ------------------------------------------------------------------------------ Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today. http://sdm.link/xeonphi _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
