Hi Rafael, Ack
This includes some nice cleanup in the SmfNodeSwLoadThread class :) One more thing that can be done is to simplify iterations over lists by using C++11 constructions when modifying them or when new ones are created. Also class variable initiation could be done the C++11 way Thanks Lennart > -----Original Message----- > From: Rafael Odzakow > Sent: den 19 april 2016 17:50 > To: Lennart Lund > Cc: opensaf-devel@lists.sourceforge.net > Subject: [PATCH 1 of 1] smfd: single step rollback does not call bundle > scripts > [#1771] > > osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc | 41 +++++++++++------- > ------- > osaf/services/saf/smfsv/smfd/SmfUpgradeStep.hh | 4 +- > 2 files changed, 21 insertions(+), 24 deletions(-) > > > Fixes a bug introduced with opensaf change: 7354. The effect of the bug is > that bundle scripts do not get executed when rolling back. In the software > loading thread a bundle list is passed down. > > diff --git a/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc > b/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc > --- a/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc > +++ b/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc > @@ -1942,9 +1942,7 @@ SmfUpgradeStep::callActivationCmd() > //Start a load thread for each node > std::list <std::string>::iterator nodeIt; > for (nodeIt = swNodeList.begin(); nodeIt != > swNodeList.end(); ++nodeIt) { > - SmfNodeSwLoadThread > *swLoadThread = new SmfNodeSwLoadThread(this, > - > > *nodeIt, > - > > SMF_ACTIVATE_ALL); > + SmfNodeSwLoadThread > *swLoadThread = new SmfNodeSwLoadThread(this, *nodeIt, > SMF_ACTIVATE_ALL); > swLoadThread->start(); > } > > @@ -2027,9 +2025,7 @@ SmfUpgradeStep::callBundleScript(SmfInst > //Start a load thread for each node > std::list <std::string>::iterator nodeIt; > for (nodeIt = swNodeList.begin(); nodeIt != > swNodeList.end(); ++nodeIt) { > - SmfNodeSwLoadThread > *swLoadThread = new SmfNodeSwLoadThread(this, > - > > *nodeIt, > - > > i_order); > + SmfNodeSwLoadThread > *swLoadThread = new SmfNodeSwLoadThread(this, *nodeIt, i_order, > &i_bundleList); > swLoadThread->start(); > } > > @@ -2845,14 +2841,20 @@ SmfNodeSwLoadThread::main(NCSCONTEXT inf > > /** > * Constructor > + * > + * This class operates in two main modes: activate and install/remove. > When > + * install/remove is used a bundle list must be specified. The reason for not > + * using the bundle list directly from the step is that install and remove > + * operations are switched during rollback. > */ > -SmfNodeSwLoadThread::SmfNodeSwLoadThread(SmfUpgradeStep * > i_step, > - > std::string i_nodeName, > - > SmfUpgradeStep::SmfInstallRemoveT i_order): > +SmfNodeSwLoadThread::SmfNodeSwLoadThread(SmfUpgradeStep * > i_step, std::string i_nodeName, > + > SmfUpgradeStep::SmfInstallRemoveT i_order, > + > const std::list<SmfBundleRef> *i_bundleList): > m_task_hdl(0), > m_step(i_step), > m_amfNode(i_nodeName), > - m_order(i_order) > + m_order(i_order), > + m_bundleList(i_bundleList) > { > sem_init(&m_semaphore, 0, 0); > } > @@ -2914,7 +2916,6 @@ void > SmfNodeSwLoadThread::main(void) > { > std::list < SmfBundleRef >::const_iterator bundleit; > - std::list < SmfBundleRef > bundleList; > std::string command; > std::string cmdAttr; > std::string argsAttr; > @@ -2932,15 +2933,8 @@ SmfNodeSwLoadThread::main(void) > > TRACE("Bundle command thread for node [%s] started", > m_amfNode.c_str()); > uint32_t rc = 0; > - if ((m_order == > SmfUpgradeStep::SMF_STEP_OFFLINE_INSTALL) || > - (m_order == > SmfUpgradeStep::SMF_STEP_ONLINE_INSTALL)) { > - bundleList = m_step->getSwAddList(); > - } > - else if ((m_order == > SmfUpgradeStep::SMF_STEP_OFFLINE_REMOVE) || > - (m_order == > SmfUpgradeStep::SMF_STEP_ONLINE_REMOVE)) { > - bundleList = m_step->getSwRemoveList(); > - } > - else if (m_order == SmfUpgradeStep::SMF_ACTIVATE_ALL) { > + > + if (m_order == SmfUpgradeStep::SMF_ACTIVATE_ALL) { > command = smfd_cb->nodeBundleActCmd; > swNodeList.push_back(m_amfNode); > std::list<std::string>::const_iterator n; > @@ -2968,13 +2962,14 @@ SmfNodeSwLoadThread::main(void) > } > goto done; > } > - else { > - LOG_NO("Unknown bundle command order"); > + > + if (m_bundleList == NULL) { > + LOG_ER("Bundle list must be set for > installation/removal"); > rc = 1; > goto done; > } > > - for (bundleit = bundleList.begin(); bundleit != > bundleList.end(); ++bundleit) { > + for (bundleit = m_bundleList->begin(); bundleit != > m_bundleList->end(); ++bundleit) { > /* Get bundle object from IMM */ > if > (immUtil.getObject((*bundleit).getBundleDn(), &attributes) == false) { > LOG_NO("Fail to read bundle > object for bundle DN [%s]", > diff --git a/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.hh > b/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.hh > --- a/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.hh > +++ b/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.hh > @@ -808,7 +808,8 @@ class SmfUpgradeStep { > ////////////////////////////////////////////////// > class SmfNodeSwLoadThread { > public: > - SmfNodeSwLoadThread(SmfUpgradeStep * i_step, std::string > i_nodeName, SmfUpgradeStep::SmfInstallRemoveT i_order); > + SmfNodeSwLoadThread(SmfUpgradeStep * i_step, std::string > i_nodeName, SmfUpgradeStep::SmfInstallRemoveT i_order, > + const std::list<SmfBundleRef> > *i_bundleList=NULL); > ~SmfNodeSwLoadThread(); > int start(void); > > @@ -824,6 +825,7 @@ class SmfNodeSwLoadThread { > std::string m_amfNode; > SmfUpgradeStep::SmfInstallRemoveT m_order; > sem_t m_semaphore; > + const std::list < SmfBundleRef > *m_bundleList; > }; > > #endif // > SMFUPGRADESTEP_HH ------------------------------------------------------------------------------ Find and fix application performance issues faster with Applications Manager Applications Manager provides deep performance insights into multiple tiers of your business applications. It resolves application problems quickly and reduces your MTTR. Get your free trial! https://ad.doubleclick.net/ddm/clk/302982198;130105516;z _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel