Hi Rafael, Comments inline.
On 2016/10/27 07:29 PM, Rafael Odzakow wrote: > Questions below. > > On 10/21/2016 01:31 PM, [email protected] wrote: >> osaf/services/saf/smfsv/smfd/SmfCampaignInit.cc | 17 +++++++++++++++-- >> 1 files changed, 15 insertions(+), 2 deletions(-) >> >> >> diff --git a/osaf/services/saf/smfsv/smfd/SmfCampaignInit.cc >> b/osaf/services/saf/smfsv/smfd/SmfCampaignInit.cc >> --- a/osaf/services/saf/smfsv/smfd/SmfCampaignInit.cc >> +++ b/osaf/services/saf/smfsv/smfd/SmfCampaignInit.cc >> @@ -225,11 +225,24 @@ SmfCampaignInit::execute() >> return false; >> } >> + TRACE("3. Read_IMM_long_DN_config_and_set_control_block()"); >> + if >> (!immUtil.read_IMM_long_DN_config_and_set_control_block(smfd_cb)) { >> + LOG_ER("SmfCampaignInit: reading long DN config from IMM >> FAILED"); >> + TRACE_LEAVE(); >> + return false; >> + } > [rafael] looks to me that this block is enough to have in one place. > There is nothing going on between the block above and the start of the > while loop. Yes, the above block may not be required. >> + >> std::list < SmfUpgradeAction * >::iterator upActiter; >> upActiter = m_campInitAction.begin(); >> while (upActiter != m_campInitAction.end()) { >> - SaAisErrorT rc = >> (*upActiter)->execute(SmfCampaignThread::instance()->getImmHandle(), >> - &initRollbackDn); >> + TRACE("4. %s: >> read_IMM_long_DN_config_and_set_control_block()",__FUNCTION__); >> + if >> (!immUtil.read_IMM_long_DN_config_and_set_control_block(smfd_cb)) { >> + LOG_ER("SmfCampaignInit: reading long DN config from IMM >> FAILED"); >> + TRACE_LEAVE(); >> + return false; >> + } > [rafael] Not sure why you would want to read the config before each > upgradeAction. Can you explain? The reading of IMM longdnconfig attribute is required here. The config attribute is read before each upgrade action, because There is a chance, that the londn attribute is enabled in previous campInitAction and longdn operation is performed in the next campInitAction. once the Longdn is enabled, then the IMM attribute is not read. (The function "read_IMM_long_DN_config_and_set_control_block" has a condition to check if longdn are already enabled) Eg: <campInitAction> <immCCB ccbFlags="0"> <modify operation="SA_IMM_ATTR_VALUES_REPLACE" objectDN="opensafImm=opensafImm,safApp=safImmService"> <attribute name="longDnsAllowed" type="SA_IMM_ATTR_SAUINT32T"> <value>1</value> </attribute> </modify> </immCCB> </campInitAction> <campInitAction> <immCCB ccbFlags="0"> <create objectClassName="OpenSafSmfMisc" parentObjectDN="="> <attribute name="openSafSmfMisc" type="SA_IMM_ATTR_SASTRINGT"> <value>openSafSmfMisc=ThisIsA300CharLongRDNaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa</value> </attribute> <attribute name="reason" type="SA_IMM_ATTR_SASTRINGT"> <value>LongDN 300 char parent object</value> </attribute> </create> </immCCB> </campInitAction> another review request is required or the patch can be pushed, without the first block (which is not required). Thanks, Neel. >> + SaAisErrorT rc = >> (*upActiter)->execute(SmfCampaignThread::instance()->getImmHandle(), >> + &initRollbackDn); >> if (rc != SA_AIS_OK) { >> LOG_ER("SmfCampaignInit init action %d failed, rc=%s", >> (*upActiter)->getId(), saf_error(rc)); >> TRACE_LEAVE(); > ------------------------------------------------------------------------------ The Command Line: Reinvented for Modern Developers Did the resurgence of CLI tooling catch you by surprise? Reconnect with the command line and become more productive. Learn the new .NET and ASP.NET CLI. Get your free copy! http://sdm.link/telerik _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
