Hello,
looks good to me. Got a question and a comment below under the [rafael] tag.
On 10/31/2016 09:31 PM, Alex Jones wrote:
> osaf/services/saf/smfsv/smfd/SmfCampaignThread.cc | 175
> +++++++++++++++++++++-
> osaf/services/saf/smfsv/smfd/SmfCampaignThread.hh | 3 +
> osaf/services/saf/smfsv/smfd/SmfStepTypes.cc | 32 ++-
> 3 files changed, 191 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);
> + }
> +
> + 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,142 @@ 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;
> +}
> +
> +void SmfCampaignThread::ntfNotificationCallback(
> + SaNtfSubscriptionIdT subId,
> + const SaNtfNotificationsT *notification) {
> + TRACE_ENTER();
> +
> + do {
> + if (subId != operStateSubId) {
> + TRACE("unknown subscription id received in ntfNotificationCallback:
> %d", subId);
> + break;
> + }
> +
> + // check if an SU was disabled after it had been upgraded
> + if (notification->notificationType == SA_NTF_TYPE_STATE_CHANGE) {
> + const SaNtfStateChangeNotificationT& stateChangeNotification(
> + notification->notification.stateChangeNotification);
> +
> + if (stateChangeNotification.notificationHeader.eventType &&
> + *stateChangeNotification.notificationHeader.eventType ==
> + SA_NTF_OBJECT_STATE_CHANGE) {
> + if (stateChangeNotification.notificationHeader.notificationClassId &&
> +
> stateChangeNotification.notificationHeader.notificationClassId->vendorId ==
> + SA_NTF_VENDOR_ID_SAF &&
> +
> stateChangeNotification.notificationHeader.notificationClassId->majorId ==
> + SA_SVC_AMF &&
> +
> stateChangeNotification.notificationHeader.notificationClassId->minorId ==
> + SA_AMF_NTFID_SU_OP_STATE) {
[rafael] If possible try to improve the readability. This would be
applicable here. Specially for the do while and nested if clauses.
http://www.informit.com/articles/article.aspx?p=1398607
> + for (SaUint16T i(0); i < stateChangeNotification.numStateChanges;
> i++) {
> + if (stateChangeNotification.changedStates[i].newState ==
> + SA_AMF_OPERATIONAL_DISABLED &&
> + stateChangeNotification.notificationHeader.additionalInfo) {
> + for (SaUint16T j(0);
> + j <
> stateChangeNotification.notificationHeader.numAdditionalInfo;
> + j++) {
> + if
> (stateChangeNotification.notificationHeader.additionalInfo[j].infoId
> + == SA_AMF_MAINTENANCE_CAMPAIGN_DN &&
> +
> stateChangeNotification.notificationHeader.additionalInfo[j].infoType
> + == SA_NTF_VALUE_LDAP_NAME) {
> + SaNameT *name(0);
> + SaUint16T dataSize(0);
> +
> + SaAisErrorT rc(saNtfPtrValGet(
> + stateChangeNotification.notificationHandle,
> +
> &stateChangeNotification.notificationHeader.additionalInfo[j].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.notificationObject));
> +
> 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("saNtfPtrValGet failed: %d", rc);
> + }
> + }
> + }
> + }
> + }
> + }
> + }
> + }
> + } while (false);
> +
> + TRACE_LEAVE();
> +}
> /**
> * SmfCampaignThread::send
> * send event to the thread.
> @@ -672,18 +815,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 +855,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,12 @@ class SmfCampaignThread {
>
> SaAisErrorT createImmHandle(SmfCampaign * i_campaign);
> SaAisErrorT deleteImmHandle();
> + SaAisErrorT initNtfSubscriptions(void);
>
> + 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;
> + }
> +
[rafael] Why was setting the maintenance status moved to after the node up?
> /* 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
