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(&notificationFilterHandles, 
> 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

Reply via email to