Credits for this patch should go to Karin Holm @ ericsson! /Hans > -----Original Message----- > From: Mathivanan Naickan Palanivelu [mailto:[email protected]] > Sent: den 14 augusti 2014 10:21 > To: Ingvar Bergström > Cc: [email protected] > Subject: Re: [devel] [PATCH 0 of 1] Review Request for smf: update campaign > state before restoring pbe in the completed state > [#944] > > Hi Ingvar, > > ACK (to the patch and to your familiarity with the system on which the > problem was detected) , > Mathi. > > > >-----Original Message----- > >From: Ingvar Bergström [mailto:[email protected]] > >Sent: Wednesday, August 13, 2014 11:30 AM > >To: Mathivanan Naickan Palanivelu > >Cc: [email protected] > >Subject: RE: [PATCH 0 of 1] Review Request for smf: update campaign state > >before restoring pbe in the completed state [#944] > > > >Hi, > >I totally missed this review request, it was in my huge mailbox after my > >vacation. Sorry for that. > >I have made some modification to the patch (new patch attached in text > >below) -The state change is made before the camp complete actions in the > >forward direction. It has shown that existing applications have problems with > >some background upgrade tasks started in the camp complete section which > >fail when PBE is turned on. > >-The state change is made before campaign init rollback if the campaign is > >rolled back. > >-SMF overall retry is extended (shown to be needed in some very slow > >systems) > > > >Thanks > >Ingvar > > > >=========================================================== > >======================================= > > > ># HG changeset patch > ># User Ingvar Bergstrom <[email protected]> # Date > >1407852641 -7200 > ># Tue Aug 12 16:10:41 2014 +0200 > ># Branch opensaf-4.4.x > ># Node ID 3c354b334da12461a9879cec06b9d7fe9a9c101a > ># Parent 830e6a0b68343d581a814cd37e4e01471af871ed > >ingvar_944_patch > > > >diff --git a/osaf/services/saf/smfsv/smfd/SmfCampState.cc > >b/osaf/services/saf/smfsv/smfd/SmfCampState.cc > >--- a/osaf/services/saf/smfsv/smfd/SmfCampState.cc > >+++ b/osaf/services/saf/smfsv/smfd/SmfCampState.cc > >@@ -830,6 +830,14 @@ SmfCampStateExecuting::executeWrapup(Smf > > { > > TRACE_ENTER(); > > > >+ //Activate IMM BPE if active when campaign was started. > >+ i_camp->restorePbe(); > >+ > >+ // Wait for IMM to become writable again by keep on writing until it > >+ // succeeds. Write the same value, just for synchronization purposes. > >+ changeState(i_camp, SmfCampStateExecuting::instance()); > >+ > >+ // IMM is writeable again so let's execute campCompleteAction > > if (i_camp->m_campWrapup.executeCampComplete() == false) { > > std::string error = "Campaign wrapup executing campaign > >complete actions failed"; > > LOG_ER("%s", error.c_str()); > >@@ -838,9 +846,6 @@ SmfCampStateExecuting::executeWrapup(Smf > > return SMF_CAMP_FAILED; > > } > > > >- //Activate IMM BPE if active when campaign was started. > >- i_camp->restorePbe(); > >- > > // TODO Start wait to complete timer > > LOG_NO("CAMP: Start wait to complete timer (not implemented > >yet)"); > > > >@@ -1737,6 +1742,12 @@ SmfCampRollingBack::rollbackInit(SmfUpgr > > TRACE_ENTER(); > > > > TRACE("SmfCampRollingBack::rollbackInit implementation"); > >+ //Activate IMM BPE if active when campaign was started. > >+ i_camp->restorePbe(); > >+ > >+ // Wait for IMM to become writable again by keep on writing until it > >+ // succeeds. Write the same value, just for synchronization purposes. > >+ changeState(i_camp, SmfCampRollingBack::instance()); > > > > if (i_camp->m_campInit.rollback() != SA_AIS_OK) { > > std::string error = "Campaign init rollback failed"; @@ -1745,9 > >+1756,6 @@ SmfCampRollingBack::rollbackInit(SmfUpgr > > changeState(i_camp, SmfCampRollbackFailed::instance()); > > return SMF_CAMP_FAILED; > > } > >- > >- //Activate IMM BPE if active when campaign was started. > >- i_camp->restorePbe(); > > > > changeState(i_camp, SmfCampRollbackCompleted::instance()); > > LOG_NO("CAMP: Upgrade campaign rollback completed %s", i_camp- > >>getCampaignName().c_str()); > >diff --git a/osaf/services/saf/smfsv/smfd/smfd_campaign_oi.cc > >b/osaf/services/saf/smfsv/smfd/smfd_campaign_oi.cc > >--- a/osaf/services/saf/smfsv/smfd/smfd_campaign_oi.cc > >+++ b/osaf/services/saf/smfsv/smfd/smfd_campaign_oi.cc > >@@ -633,7 +633,8 @@ uint32_t create_campaign_objects(smfd_cb > > */ > > uint32_t updateImmAttr(const char *dn, SaImmAttrNameT attributeName, > >SaImmValueTypeT attrValueType, void *value) { > >- (void)immutil_update_one_rattr(smfd_cb->campaignOiHandle, dn, > >attributeName, attrValueType, value); > >+ SaAisErrorT rc = immutil_update_one_rattr(smfd_cb- > >>campaignOiHandle, dn, attributeName, attrValueType, value); > >+ osafassert(rc == SA_AIS_OK); > > > > return NCSCC_RC_SUCCESS; > > } > >diff --git a/osaf/services/saf/smfsv/smfd/smfd_main.c > >b/osaf/services/saf/smfsv/smfd/smfd_main.c > >--- a/osaf/services/saf/smfsv/smfd/smfd_main.c > >+++ b/osaf/services/saf/smfsv/smfd/smfd_main.c > >@@ -165,8 +165,8 @@ static uint32_t initialize_smfd(void) > > > > /* Set the behaviour of SMF-IMM interactions */ > > immutilWrapperProfile.errorsAreFatal = 0; /* False, no reboot when > >fail */ > >- immutilWrapperProfile.nTries = 500; /* Times */ > >- immutilWrapperProfile.retryInterval = 400; /* MS */ > >+ immutilWrapperProfile.nTries = 600; /* Times */ > >+ immutilWrapperProfile.retryInterval = 1000; /* MS */ > > > > if (ncs_agents_startup() != NCSCC_RC_SUCCESS) { > > LOG_ER("ncs_agents_startup FAILED"); > > > >=========================================================== > >======================================= > > > >-----Original Message----- > >From: [email protected] [mailto:[email protected]] > >Sent: den 24 juni 2014 03:16 > >To: Ingvar Bergström > >Cc: [email protected] > >Subject: [PATCH 0 of 1] Review Request for smf: update campaign state > >before restoring pbe in the completed state [#944] > > > >Summary: smf: update campaign state before restoring pbe in the completed > >state Review request for Trac Ticket(s): #944 Peer Reviewer(s): > >[email protected] Pull request to: <<LIST THE PERSON WITH > >PUSH ACCESS HERE>> Affected branch(es): 4.4.x, default Development > >branch: <<IF ANY GIVE THE REPO URL>> > > > >-------------------------------- > >Impacted area Impact y/n > >-------------------------------- > > Docs n > > Build system n > > RPM/packaging n > > Configuration files n > > Startup scripts n > > SAF services y > > OpenSAF services n > > Core libraries n > > Samples n > > Tests n > > Other n > > > > > >Comments (indicate scope for each "y" above): > >--------------------------------------------- > > > >changeset d2846c8fed95dcec256faf787bb9a257652cb585 > >Author: Mathivanan N.P.<[email protected]> > >Date: Mon, 23 Jun 2014 21:12:27 -0400 > > > > smf: update campaign state before restoring pbe in the completed > >state > > [#944] By way of ticket #677 we restore the pbe in the campaign > >completed > > state itself. i.e. In SmfCampStateExecuting::executeWrapup() { > > > > a) we first restore the pbe i.e. i_camp->restorePbe(); > > > > b) And, then subsequently update the campaign state, i.e. > > changeState(i_camp, SmfCampStateExecCompleted::instance()); > > > > } However, it is possible that when the pbe is restored, the pbe could > >take > > more time (than the immutil wait time) to become functionally ready. > >And in > > such sitautions, the updation to the campaign state will not succeed > >until > > the PBE is really ready. The patch updates the campaign state first (to > >imm) > > inthe completed state and subsequently restores the pbe. > > > > > >Complete diffstat: > >------------------ > > osaf/services/saf/smfsv/smfd/SmfCampState.cc | 7 ++++--- > > 1 files changed, 4 insertions(+), 3 deletions(-) > > > > > >Testing Commands: > >----------------- > >The patch requires simulation of a scenario were pberestore takes a lot of > >time. > >Trigger an upgrade. > >Upgrades should work fine. > > > >Testing, Expected Results: > >-------------------------- > >Same as above. > > > >Conditions of Submission: > >------------------------- > >Ack from Ingvar. > > > >Arch Built Started Linux distro > >------------------------------------------- > >mips n n > >mips64 n n > >x86 n n > >x86_64 n n > >powerpc n n > >powerpc64 n n > > > > > >Reviewer Checklist: > >------------------- > >[Submitters: make sure that your review doesn't trigger any checkmarks!] > > > > > >Your checkin has not passed review because (see checked entries): > > > >___ Your RR template is generally incomplete; it has too many blank entries > > that need proper data filled in. > > > >___ You have failed to nominate the proper persons for review and push. > > > >___ Your patches do not have proper short+long header > > > >___ You have grammar/spelling in your header that is unacceptable. > > > >___ You have exceeded a sensible line length in your > >headers/comments/text. > > > >___ You have failed to put in a proper Trac Ticket # into your commits. > > > >___ You have incorrectly put/left internal data in your comments/files > > (i.e. internal bug tracking tool IDs, product names etc) > > > >___ You have not given any evidence of testing beyond basic build tests. > > Demonstrate some level of runtime or other sanity testing. > > > >___ You have ^M present in some of your files. These have to be removed. > > > >___ You have needlessly changed whitespace or added whitespace crimes > > like trailing spaces, or spaces before tabs. > > > >___ You have mixed real technical changes with whitespace and other > > cosmetic code cleanup changes. These have to be separate commits. > > > >___ You need to refactor your submission into logical chunks; there is > > too much content into a single commit. > > > >___ You have extraneous garbage in your review (merge commits etc) > > > >___ You have giant attachments which should never have been sent; > > Instead you should place your content in a public tree to be pulled. > > > >___ You have too many commits attached to an e-mail; resend as threaded > > commits, or place in a public tree for a pull. > > > >___ You have resent this content multiple times without a clear indication > > of what has changed between each re-send. > > > >___ You have failed to adequately and individually address all of the > > comments and change requests that were proposed in the initial review. > > > >___ You have a misconfigured ~/.hgrc file (i.e. username, email etc) > > > >___ Your computer have a badly configured date and time; confusing the > > the threaded patch review. > > > >___ Your changes affect IPC mechanism, and you don't present any results > > for in-service upgradability test. > > > >___ Your changes affect user manual and documentation, your patch series > > do not contain the patch that updates the Doxygen manual. > > > > ------------------------------------------------------------------------------ > _______________________________________________ > Opensaf-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/opensaf-devel
------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
