Comments inline... On 05/06/2014 09:08 AM, Hans Feldt wrote: > First I don't think we should bring in cluster and application state just > like this. It is not relevant for this case and we anyway don't support such > ops.
[Alex] It is relevant in that this macro is a test for SU in-service. And, according to 3.2.1.4 of the AMF spec (B.04.01) cluster and application admin state need to be checked. I understand that we don't support admin operations for these two, but having these checks in here doesn't hurt because they are always initialized to UNLOCKED right now. If you feel strongly that these should not be included, I can remove them, but you know that when someone adds support for admin operations for cluster and application, that they will forget to update this function. :) > > Second this macro is out of control and should be changed into a (inline) > function instead. > Haven't I already done that in my refactoring series? [Alex] In the latest default branch this is a macro. Do you want me to make this an inline function, or should I let you change all the macros to inline functions first? > > I don't see in the analysis what the problem really is. Why the 7 seconds? [Alex] The problem is that the SU is being assigned an SI, even though the SU is not instantiated, yet. One of the components in the SU takes 7 seconds to instantiate. We can't be assigning an SI to an SU which is in INSTANTIATING state. Otherwise, no CSIs get assigned here because the component instantiates after the SI assignment. This is because the macro is only following B.01.01 for determining in-service. This is a bug in the B.01.01 spec, which was addressed in B.04.01. In B.04.01, section 3.2.1.4, in-service must also check cluster admin state, application admin state, SU operational state, and SU presence state. > > Thanks, > Hans > >> -----Original Message----- >> From: Alex Jones [mailto:ajo...@genband.com] >> Sent: den 2 maj 2014 21:27 >> To: Hans Feldt; praveen.malv...@oracle.com; nagendr...@oracle.com >> Cc: opensaf-devel@lists.sourceforge.net >> Subject: [PATCH 1 of 1] amfd: fix SU in-service macro [#493] >> >> osaf/services/saf/amf/amfd/include/su.h | 13 +++++++++---- >> 1 files changed, 9 insertions(+), 4 deletions(-) >> >> >> May 2 18:56:32 linux osafamfnd[12420]: NO Assigned >> 'safSi=Dataplane-Np1-SI-1,safApp=DataplaneApp' STANDBY to >> 'safSu=Dataplane-SU1,safSg=Dataplane-Np1,safApp=DataplaneApp' >> May 2 18:56:39 linux osafamfnd[12420]: NO >> 'safSu=Dataplane-SU1,safSg=Dataplane-Np1,safApp=DataplaneApp' Presence State >> INSTANTIATING => INSTANTIATED >> >> You can see that amfnd has assigned the SI to the SU, but the SU doesn't >> transition to INSTANTIATED until 7 seconds later. The m_AVD_APP_SU_IS_INSVC >> macro is using tests from B.01.01, so it doesn't take into account SU >> presence >> state as specified in B.04.01. >> >> The solution is to bring this macro up to date with B.04.01 by adding tests >> for >> cluster admin state, application admin state, node operational state, and SU >> presence state. >> >> diff --git a/osaf/services/saf/amf/amfd/include/su.h >> b/osaf/services/saf/amf/amfd/include/su.h >> --- a/osaf/services/saf/amf/amfd/include/su.h >> +++ b/osaf/services/saf/amf/amfd/include/su.h >> @@ -33,6 +33,8 @@ >> #include <amf_defs.h> >> #include <msg.h> >> #include <comp.h> >> +#include <app.h> >> +#include <cluster.h> >> #include "include/db_template.h" >> >> /** >> @@ -134,11 +136,14 @@ m_AVSV_SEND_CKPT_UPDT_ASYNC_UPDT(cb, su, >> } >> >> #define m_AVD_APP_SU_IS_INSVC(i_su,su_node_ptr) \ >> -((su_node_ptr->saAmfNodeAdminState == SA_AMF_ADMIN_UNLOCKED) && \ >> +((su_node_ptr->cluster->saAmfClusterAdminState == SA_AMF_ADMIN_UNLOCKED) && >> \ >> +(i_su->sg_of_su->app->saAmfApplicationAdminState == SA_AMF_ADMIN_UNLOCKED) >> && \ >> +(i_su->saAmfSUAdminState == SA_AMF_ADMIN_UNLOCKED) &&\ >> +(i_su->sg_of_su->saAmfSGAdminState == SA_AMF_ADMIN_UNLOCKED) &&\ >> +(su_node_ptr->saAmfNodeAdminState == SA_AMF_ADMIN_UNLOCKED) && \ >> (su_node_ptr->saAmfNodeOperState == SA_AMF_OPERATIONAL_ENABLED) && \ >> -(i_su->sg_of_su->saAmfSGAdminState == SA_AMF_ADMIN_UNLOCKED) &&\ >> -(i_su->saAmfSUAdminState == SA_AMF_ADMIN_UNLOCKED) &&\ >> -(i_su->saAmfSUOperState == SA_AMF_OPERATIONAL_ENABLED)\ >> +(i_su->saAmfSUOperState == SA_AMF_OPERATIONAL_ENABLED) && \ >> +(i_su->saAmfSUPresenceState == SA_AMF_PRESENCE_INSTANTIATED || >> i_su->saAmfSUPresenceState == >> SA_AMF_PRESENCE_RESTARTING) \ >> ) >> >> #define m_AVD_GET_SU_NODE_PTR(avd_cb,i_su,su_node_ptr) \ ------------------------------------------------------------------------------ Is your legacy SCM system holding you back? Join Perforce May 7 to find out: • 3 signs your SCM is hindering your productivity • Requirements for releasing software faster • Expert tips and advice for migrating your SCM now http://p.sf.net/sfu/perforce _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
