will share a patch /Hans On 7 May 2014 07:47, Hans Feldt <hans.fe...@ericsson.com> wrote: > The way you have changed the macro has made useable only for pre-instantiable > SUs since the table you point to is only valid for pre-inst SUs. > > We already have some tech debt with this macro: > // TODO(nagu) remove saAmfSUPreInstantiable check and > move into m_AVD_APP_SU_IS_INSVC > > So maybe it is best that to clean this up properly now? > > Proposal: > * change the macro to a function > * make it aware of SU type (pre/non-pre instantiable) > * change use of the macro where checks for INSTANTIATED has been added. > > What do you think? > > Thanks, > Hans > > >> -----Original Message----- >> From: Alex Jones [mailto:ajo...@genband.com] >> Sent: den 6 maj 2014 18:07 >> To: Hans Feldt; praveen.malv...@oracle.com; nagendr...@oracle.com >> Cc: opensaf-devel@lists.sourceforge.net >> Subject: Re: [PATCH 1 of 1] amfd: fix SU in-service macro [#493] >> >> 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. > [Hans] I tried todo the same thing before and got >> >> 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
------------------------------------------------------------------------------ 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
