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
