Hi Rafael, Thanks for your thoughts.
Yes, this condition check exists even for the regular single step campaigns. But, in that case it is obvious (as you too rightly point out) that Node must be specified if reboot is required for activation, That is the reason why this patch targets only the merge related scenario. Iam glad we are having this discussion because originally I was hesistant to even support merging of such a campaign to start with. But, since the merge feature is an OpenSAF extension, I thought defining the boundaries of this feature is in our control and particularly if there are active users who want to merge such kind of 'non-symmetric' procedures. w.r.t your pointer to the enhancement ticket https://sourceforge.net/p/opensaf/tickets/1787/ that would need further consideration, I suppose. Because, it is assumed/implicit that activation is achieved by rebooting the node. If reboot is not desired then the smfNodeBundleActCmd feature could be of use. Having said that, it may not be much of an overhead to add a new type of 'step' to the SMF spec, especially if we have users for such kind of 'steps' who expect both - an immediate activation as well as a reboot as a part of the step! w.r.t this patch, your suggestion to simply avoid the condition-check in the case of merge, iam okay with that. Going by your suggestion, this patch would become much simpler, except that we may be missing error handling (if any) when the user did not specify any node or PLMee at all, but I guess that is acceptable and the campaign designer can take care of! If we agree on that, i can send a v2 patch that avoids looking up the nodes, swAdd, swRemove lists but instead simply avoids doing this error checking if The merge feature is enabled. Thanks, Mathi. > -----Original Message----- > From: Rafael Odzakow [mailto:[email protected]] > Sent: Wednesday, May 11, 2016 8:12 PM > To: Mathivanan Naickan Palanivelu; [email protected]; Reddy > Neelakanta Reddy Peddavandla > Cc: [email protected] > Subject: Re: [PATCH 1 of 1] smfd: choose the AU type with highest scope in > merged procedure [#1810] > > Hi, Mathi > > I am wondering about this change. I am seeing a similar problem when > running single step campaigns with reboot. They get stuck on this error when > the AU is not a node: > > from calculateStepType: > > LOG_NO("A software bundle requires reboot but the AU is a SU (%s)", > firstAuDu.c_str()); > return SA_AIS_ERR_FAILED_OPERATION; > > > For single step campaigns having a node as the AU is necessary when > bundle requires reboot. You can also not specify an AU and that would > work but without any activation done by SMF. Here is a ticket related to > this > > https://sourceforge.net/p/opensaf/tickets/1787/ > > > So regarding this patch. In MERGE_TO_SINGLESTEP we are merging different > procedures to a new single step procedure. This new procedure has read > all the AU and created a new AU list. > > As I understand it this patch does its work after this step and reads in > the AU to try to find a Node so it can set className to SaAmfNode. This > is in order to not fail the above "LOG_NO(A software bundle..." check. > > I suggest that instead of doing this searching at a this late stage. The > new AU list could be ordered with nodes first already when merging the > procedures unless there is some unknown problem having nodes first. > > Check in: SmfUpgradeProcedure::mergeStepIntoSingleStep() > > And this line: > > newStep->addDeactivationUnits(nodeLevelDU); //Add the node level DU > > Alternative is to skip this error when there is a MERGE_TO_SINGLESTEP > and the AU is not a node. > > > What do you think? > > > On 05/10/2016 04:29 PM, [email protected] wrote: > > osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc | 70 > ++++++++++++++++++++++++++ > > 1 files changed, 70 insertions(+), 0 deletions(-) > > > > > > When SMF_MERGE_TO_SINGLE_STEP feature is enabled and the step > requires reboot, > > i.e. when saSmfBundleInstallOfflineScope or > saSmfBundleRemoveOfflineScope is > > set to SA_SMF_CMD_SCOPE_PLM_EE and when > smfSSAffectedNodesEnable is true then > > currently SMF uses the first occurrence of the AU's actedOn entry and > throws > > the following error: > > - A software bundle requires reboot but the AU is a SU > > This patch changes this behaviour and chooses the AU/DU type that has > the highest scope > > i.e. safAmfNode scope for enabling node reboot. > > Therefore, If first entry is not of type safAmfNode, look for it in the > > rest of > the actedOn list. > > If not found in actedOn list, look in swAdd and swRemove bundles > accordingly > > to choose the scope. > > > > 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 > > @@ -31,6 +31,7 @@ > > #include "SmfUpgradeStep.hh" > > #include "SmfCampaign.hh" > > #include "SmfUpgradeProcedure.hh" > > +#include "SmfUpgradeCampaign.hh" > > #include "SmfUpgradeMethod.hh" > > #include "SmfProcedureThread.hh" > > #include "SmfStepState.hh" > > @@ -1362,13 +1363,82 @@ SmfUpgradeStep::calculateStepType() > > //If rolling upgrade check the first AU of the step > > std::string className; > > std::string firstAuDu; > > + std::list < unitNameAndState >::const_iterator unitIt; > > + > > if (this->getProcedure()->getUpgradeMethod()- > >getUpgradeMethod() == SA_SMF_SINGLE_STEP) { > > //Single step > > //Try the activation unit list, if empty try the deactivation > > unit > list > > if (!this->getActivationUnitList().empty()) { > > firstAuDu = this- > >getActivationUnitList().front().name; > > + // When SMF_MERGE_TO_SINGLE_STEP feature is > enabled and the step requires reboot, > > + // i.e. when saSmfBundleInstallOfflineScope or > saSmfBundleRemoveOfflineScope is > > + // set to SA_SMF_CMD_SCOPE_PLM_EE and when > smfSSAffectedNodesEnable is true then > > + // Choose the type with the highest scope i.e. > safAmfNode for enabling node reboot. > > + // Therefore, If first entry is not of type safAmfNode, > look for it in the rest of the actedOn list. > > + // Note: ignore matches for comp, su since node is a > part of their DNs. > > + if (rebootNeeded && > !(firstAuDu.find("safAmfNode") == 0) && > > + SmfCampaignThread::instance()- > >campaign()->getUpgradeCampaign()->getProcExecutionMode() > > + == > SMF_MERGE_TO_SINGLE_STEP) { > [rafael] The above if statement is hard to read. Try getting the > ProcExecutionMode earlier (after unitIt) and use it here (and later) > instead. > > + for (unitIt = this->getActivationUnitList().begin(); > > + unitIt != this->getActivationUnitList().end(); > ++unitIt) { > [rafael] c++11 and the auto keyword would simplify this loop, same goes > for the rest of the for loops. > > + if(!((*unitIt).name.find("safComp") == 0) && > !((*unitIt).name.find("safSu") == 0) > > + && > (*unitIt).name.find("safAmfNode") == 0){ > > + firstAuDu = unitIt->name; > > + break; > > + } > > + } > > + if (!firstAuDu.find("safAmfNode") == 0) { > > + // There is no safAmfNode in actedOn list, > now walk through swAdd Bundles list. > > + std::list < SmfBundleRef >::const_iterator > bundleit; > > + for (bundleit = m_swAddList.begin(); bundleit != > m_swAddList.end(); ++bundleit) { > > + > > std::list<SmfPlmExecEnv>::const_iterator ee; > > + for (ee = bundleit- > >getPlmExecEnvList().begin(); > > + ee != bundleit- > >getPlmExecEnvList().end(); ee++) { > > + std::string const& amfnode = ee- > >getAmfNode(); > > + if (amfnode.length() != 0) { > > + firstAuDu = amfnode; > > + break; > > + } > > + } > > + if (firstAuDu.find("safAmfNode")) > > + break; > > + } //End - walk through swAdd bundle list > > + } > > + } //End - Look ahead in the lists of actedOn and > swAdd bundles to find for safAmfNode > > } else if (!this->getDeactivationUnitList().empty()) { > > firstAuDu = this- > >getDeactivationUnitList().front().name; > > + // When SMF_MERGE_TO_SINGLE_STEP feature is > enabled and reboot is enabled, > > + // choose the highest of the DU types for letting > reboot happen. > > + //If first entry is not of type safAmfNode, find it in > the rest of the list. > > + if (rebootNeeded && > !(firstAuDu.find("safAmfNode") == 0) && > > + SmfCampaignThread::instance()- > >campaign()->getUpgradeCampaign()->getProcExecutionMode() > > + == > SMF_MERGE_TO_SINGLE_STEP) { > [rafael] Same issue as the first comment. > > + for (unitIt = this->getDeactivationUnitList().begin(); > > + unitIt != this- > >getDeactivationUnitList().end(); ++unitIt) { > > + if(!((*unitIt).name.find("safComp") == 0) && > !((*unitIt).name.find("safSu") == 0) > > + && > (*unitIt).name.find("safAmfNode") == 0){ > > + firstAuDu = unitIt->name; > > + break; > > + } > > + } > > + if (!firstAuDu.find("safAmfNode") == 0) { > > + // There is no safAmfNode in actedOn list, > > now walk > through swRemove Bundles list. > > + std::list < SmfBundleRef >::const_iterator > > bundleit; > > + for (bundleit = m_swRemoveList.begin(); > > bundleit != > m_swRemoveList.end(); ++bundleit) { > > + > > std::list<SmfPlmExecEnv>::const_iterator ee; > > + for (ee = > > bundleit->getPlmExecEnvList().begin(); > > + ee != > > bundleit->getPlmExecEnvList().end(); > ee++) { > > + std::string const& amfnode > > = ee->getAmfNode(); > > + if (amfnode.length() != 0) > > { > > + firstAuDu = > > amfnode; > > + break; > > + } > > + } > > + if (firstAuDu.find("safAmfNode")) > > + break; > > + } //End - walk through swRemove bundle list > > + } > > + } //End - Look ahead in the lists of actedOn and > swRemove bundles to find for safAmfNode > > } else { > > //No activation/deactivation, just SW installation > > className = "SaAmfNode"; > ------------------------------------------------------------------------------ Mobile security can be enabling, not merely restricting. Employees who bring their own devices (BYOD) to work are irked by the imposition of MDM restrictions. Mobile Device Manager Plus allows you to control only the apps on BYO-devices by containerizing them, leaving personal data untouched! https://ad.doubleclick.net/ddm/clk/304595813;131938128;j _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
