Hi Alex After I posted this ack I and Rafael found a possible Non backwards compatibility (NBC) problem. If so this must be solved before pushing.
I believe this change introduce a new behavior when campaigns are executed if a component is failing. If I am right this could be solved by introducing a new attribute in the SMF configuration class that makes it possible to switch on/off this new behavior. The default must be that it is switched off (legacy behavior) also the absence of the new attribute must be interpreted as off (legacy) Thanks Lennart > -----Original Message----- > From: Lennart Lund [mailto:lennart.l...@ericsson.com] > Sent: den 14 november 2016 13:56 > To: Alex Jones <ajo...@genband.com>; reddy.neelaka...@oracle.com; > Rafael Odzakow <rafael.odza...@ericsson.com> > Cc: opensaf-devel@lists.sourceforge.net > Subject: Re: [devel] [PATCH 1 of 1] smfd: add support for asynchronous > detection of failed AMF entities [#2145] > > 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 > > 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 ------------------------------------------------------------------------------ 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
