ACK
On 10/10/2016 01:46 PM, Neelakanta Reddy wrote:
> Hi Alex,
>
> Reviewed and tested the patch.
> Ack.
>
> /Neel.
>
> On 2016/10/07 11:10 PM, Alex Jones wrote:
>> osaf/services/saf/smfsv/smfd/SmfStepState.cc | 13 ++++
>> osaf/services/saf/smfsv/smfd/SmfUpgradeProcedure.cc | 68
>> +++++++++++++++++---
>> osaf/services/saf/smfsv/smfd/SmfUpgradeProcedure.hh | 6 +
>> 3 files changed, 76 insertions(+), 11 deletions(-)
>>
>>
>> Sep 27 00:34:14 q50-s1 osafsmfd[6667]: NO SA_AMF_ADMIN_SI_SWAP [rc=1]
>> successfully initiated
>> Sep 27 00:34:15 q50-s1 osafimmnd[6571]: NO ERR_BAD_OPERATION:
>> Mismatch on administrative owner '' != 'SMFSERVICE'
>> Sep 27 00:34:17 q50-s1 osafsmfd[6667]: NO Fail to invoke admin
>> operation, rc=SA_AIS_ERR_BAD_OPERATION (20).
>> dn=[safSi=SC-2N,safApp=OpenSAF], opId=[7]
>> Sep 27 00:34:17 q50-s1 osafsmfd[6667]: NO Admin op
>> SA_AMF_ADMIN_SI_SWAP fail [rc = 20]
>> Sep 27 00:34:17 q50-s1 osafsmfd[6667]: NO CAMP: Procedure
>> safSmfProc=RollingUpgrade returned FAILED
>> Sep 27 00:36:14 q50-s1 osafsmfd[6667]: NO Campaign thread does not
>> disappear within 120 seconds after SA_AMF_ADMIN_SI_SWAP, the
>> operation was assumed failed.
>> Sep 27 00:36:14 q50-s1 kernel: [14934029.531187] osafsmfd[32024]:
>> segfault at 4 ip 00000000004425b6 sp 00007f67f7ffe1c0 error 4 in
>> osafsmfd[400000+9a000]
>> Sep 27 00:36:14 q50-s1 osafamfnd[6649]: NO
>> 'safComp=SMF,safSu=SC-1,safSg=2N,safApp=OpenSAF' faulted due to
>> 'avaDown' : Recovery is 'nodeFailfast'
>> Sep 27 00:36:14 q50-s1 osafamfnd[6649]: ER
>> safComp=SMF,safSu=SC-1,safSg=2N,safApp=OpenSAF Faulted due to:avaDown
>> Recovery is:nodeFailfast
>>
>> There are a few problems here. One is that the SmfSwapThread is
>> pointing to a
>> deleted procedure when the original active controller is reassigned
>> active. The
>> second problem is that a new SmfSwapThread is created when the
>> original active
>> controller is reassigned active, so now there are two running. The
>> first thread
>> tries to use its proc pointer (which has been deleted when the
>> original active
>> goes to quiesced) and causes the segfault.
>>
>> The proposed solution is a little different from that proposed in the
>> ticket
>> description. This solution proposes to use the existence of the
>> SmfSwapThread as
>> a test. When the original active controller is reassigned active
>> because the
>> si-swap failed, it will still remove the RestartIndicator as it does
>> now. But,
>> if the SmfSwapThread is still running, it will not create a new one,
>> but update
>> it with the recreated procedure pointer, and let it handle the
>> si-swap timeout.
>> Then it will report the error. I believe this solution is backwards
>> compatible
>> because no IMM changes are made like the ones proposed in the ticket.
>>
>> diff --git a/osaf/services/saf/smfsv/smfd/SmfStepState.cc
>> b/osaf/services/saf/smfsv/smfd/SmfStepState.cc
>> --- a/osaf/services/saf/smfsv/smfd/SmfStepState.cc
>> +++ b/osaf/services/saf/smfsv/smfd/SmfStepState.cc
>> @@ -424,6 +424,19 @@ SmfStepStateUndone::execute(SmfUpgradeSt
>> {
>> TRACE_ENTER();
>> + if (i_step->calculateStepType() != SA_AIS_OK) {
>> + LOG_ER("SmfStepStateUndone: Failed to calculate step
>> type");
>> + changeState(i_step, SmfStepStateFailed::instance());
>> + TRACE_LEAVE();
>> + return SMF_STEP_FAILED;
>> + }
>> +
>> + if (i_step->getSwitchOver() == true) {
>> + TRACE("Switch over is needed in this step");
>> + TRACE_LEAVE();
>> + return SMF_STEP_SWITCHOVER;
>> + }
>> +
>> i_step->setRetryCount(0); /* Reset the retry counter */
>> changeState(i_step, SmfStepStateExecuting::instance());
>> diff --git a/osaf/services/saf/smfsv/smfd/SmfUpgradeProcedure.cc
>> b/osaf/services/saf/smfsv/smfd/SmfUpgradeProcedure.cc
>> --- a/osaf/services/saf/smfsv/smfd/SmfUpgradeProcedure.cc
>> +++ b/osaf/services/saf/smfsv/smfd/SmfUpgradeProcedure.cc
>> @@ -482,12 +482,17 @@ SmfUpgradeProcedure::switchOver()
>> osafassert(0);
>> }
>> - TRACE("SmfUpgradeProcedure::switchOver: Create the restart
>> indicator");
>> -
>> SmfCampaignThread::instance()->campaign()->getUpgradeCampaign()->createSmfRestartIndicator();
>> -
>> - SmfSwapThread *swapThread = new SmfSwapThread(this);
>> - TRACE("SmfUpgradeProcedure::switchOver, Starting SI_SWAP thread");
>> - swapThread->start();
>> + if (!SmfSwapThread::running()) {
>> + TRACE("SmfUpgradeProcedure::switchOver: Create the restart
>> indicator");
>> +
>> SmfCampaignThread::instance()->campaign()->getUpgradeCampaign()->createSmfRestartIndicator();
>> +
>> + SmfSwapThread *swapThread = new SmfSwapThread(this);
>> + TRACE("SmfUpgradeProcedure::switchOver, Starting SI_SWAP
>> thread");
>> + swapThread->start();
>> + } else {
>> + TRACE("SmfUpgradeProcedure::switchOver, SI_SWAP thread
>> already running");
>> + SmfSwapThread::setProc(this);
>> + }
>> TRACE_LEAVE();
>> }
>> @@ -4156,6 +4161,31 @@ SmfUpgradeProcedure::resetProcCounter()
>> /* Static
>> methods */
>> /*====================================================================*/
>> +SmfSwapThread *SmfSwapThread::me(0);
>> +std::mutex SmfSwapThread::m_mutex;
>> +
>> +/**
>> + * SmfSmfSwapThread::running
>> + * Is the thread currently running?
>> + */
>> +bool
>> +SmfSwapThread::running(void)
>> +{
>> + std::lock_guard<std::mutex> guard(m_mutex);
>> + return me ? true : false;
>> +}
>> +
>> +/**
>> + * SmfSmfSwapThread::setProc
>> + * Set the procedure pointer to the newly created procedure
>> + */
>> +void
>> +SmfSwapThread::setProc(SmfUpgradeProcedure *newProc)
>> +{
>> + std::lock_guard<std::mutex> guard(m_mutex);
>> + me->m_proc = newProc;
>> +}
>> +
>> /**
>> * SmfSmfSwapThread::main
>> * static main for the thread
>> @@ -4181,6 +4211,8 @@ SmfSwapThread::SmfSwapThread(SmfUpgradeP
>> m_proc(i_proc)
>> {
>> sem_init(&m_semaphore, 0, 0);
>> + std::lock_guard<std::mutex> guard(m_mutex);
>> + me = this;
>> }
>> /**
>> @@ -4188,6 +4220,8 @@ SmfSwapThread::SmfSwapThread(SmfUpgradeP
>> */
>> SmfSwapThread::~SmfSwapThread()
>> {
>> + std::lock_guard<std::mutex> guard(m_mutex);
>> + me = 0;
>> }
>> /**
>> @@ -4309,13 +4343,25 @@ SmfSwapThread::main(void)
>> exit_error:
>> if (SmfCampaignThread::instance() != NULL) {
>> - SmfProcStateExecFailed::instance()->changeState(m_proc,
>> SmfProcStateExecFailed::instance());
>> - }
>> -
>> - if (SmfCampaignThread::instance() != NULL) {
>> + std::lock_guard<std::mutex> guard(m_mutex);
>> +
>> + SmfProcStateExecuting::instance()->changeState(m_proc,
>> SmfProcStateStepUndone::instance());
>> +
>> + // find the failed upgrade step and set it to Undone
>> + std::vector<SmfUpgradeStep *>&
>> upgradeSteps(m_proc->getProcSteps());
>> + for (std::vector<SmfUpgradeStep *>::iterator
>> it(upgradeSteps.begin()); it != upgradeSteps.end(); ++it) {
>> + if ((*it)->getSwitchOver()) {
>> + (*it)->changeState(SmfStepStateUndone::instance());
>> + break;
>> + }
>> + }
>> +
>> + std::string error("si-swap of middleware failed");
>> + SmfCampaignThread::instance()->campaign()->setError(error);
>> +
>> CAMPAIGN_EVT *evt = new CAMPAIGN_EVT();
>> evt->type = CAMPAIGN_EVT_PROCEDURE_RC;
>> - evt->event.procResult.rc = SMF_PROC_FAILED;
>> + evt->event.procResult.rc = SMF_PROC_STEPUNDONE;
>> evt->event.procResult.procedure = m_proc;
>> SmfCampaignThread::instance()->send(evt);
>> }
>> diff --git a/osaf/services/saf/smfsv/smfd/SmfUpgradeProcedure.hh
>> b/osaf/services/saf/smfsv/smfd/SmfUpgradeProcedure.hh
>> --- a/osaf/services/saf/smfsv/smfd/SmfUpgradeProcedure.hh
>> +++ b/osaf/services/saf/smfsv/smfd/SmfUpgradeProcedure.hh
>> @@ -29,6 +29,7 @@
>> #include <vector>
>> #include <list>
>> #include <map>
>> +#include <mutex>
>> #include <saSmf.h>
>> #include <saImmOi.h>
>> @@ -791,7 +792,12 @@ class SmfSwapThread {
>> ~SmfSwapThread();
>> int start(void);
>> + static bool running(void);
>> + static void setProc(SmfUpgradeProcedure *);
>> +
>> private:
>> + static SmfSwapThread *me;
>> + static std::mutex m_mutex;
>> void main(void);
>> int init(void);
>>
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
