Re: [devel] [PATCH 1 of 1] mds: TCP performance improvements [#654]
Ok I will split and publish again with specific Commit Message. -AVM On 4/29/2014 12:19 PM, Hans Feldt wrote: > Hi, > > This seems like 3 different logical changes thus should be 3 different > patches. You should not change the MDS library in the same patch as the dtm > infrastructure service (unless the changes are very tightly coupled). > > What is the actual MDS/TCP performance improvement here and what change is > bringing it? > I think it would be the increased fragmentation limit but let's be explicit > about it in the commit message. > > Thanks, > Hans > >> -Original Message- >> From: mahesh.va...@oracle.com [mailto:mahesh.va...@oracle.com] >> Sent: den 29 april 2014 08:20 >> To: opensaf-devel@lists.sourceforge.net; Hans Feldt; mathi.naic...@oracle.com >> Cc: opensaf-devel@lists.sourceforge.net >> Subject: [PATCH 1 of 1] mds: TCP performance improvements [#654] >> >> osaf/libs/core/mds/include/mds_dt_tcp_disc.h | 1 - >> osaf/libs/core/mds/mds_dt_tcp.c| 20 +- >> osaf/libs/core/mds/mds_dt_trans.c | 12 +- >> osaf/services/infrastructure/dtms/config/dtmd.conf | 23 +++ >> osaf/services/infrastructure/dtms/dtm/dtm_intra.c | 13 +-- >> osaf/services/infrastructure/dtms/dtm/dtm_node_sockets.c | 44 >> ++--- >> osaf/services/infrastructure/dtms/dtm/dtm_read_config.c| 30 + >> osaf/services/infrastructure/dtms/include/dtm.h| 2 + >> osaf/services/infrastructure/dtms/include/dtm_cb.h | 3 + >> osaf/services/infrastructure/dtms/include/dtm_intra_disc.h | 2 - >> 10 files changed, 109 insertions(+), 41 deletions(-) >> >> >> This path is on top of Hans Feldt published on >> 4/29/2014 10:58 AM >> >> 1) Their is NO issue with MDS TCP in-service Upgrade ,The value of >> SO_RCVBUF & SO_SNDBUF of TCP sockets was set to 64000 (MDS_SND_RCV_SIZE) >> using setsockopt() ,the kernel doubles this value ,and this doubled >> value is returned by getsockopt(). So MDS_DIRECT_BUF_MAXSIZE size is >> used to send for for both >> previous & current versions of Opensaf nodes which are using MDS TCP as >> transport >> >> 2) Made DTM-TCP inter-node & intra-node socket SO_SNDBUF and >> SO_RCVBUF buffer in bytes configurable . The kernel doubles this value >> (to >> allow space for bookkeeping over-head) when it is set using >> setsockopt(),and >> this doubled value is returned by getsockopt(). The default value is >> set to DTM_SOCK_SND_RCV_BUF_SIZE=126976 >> >> 3) Made TCP_NODELAY configurable to specifi whether MDS TCP >> transport should follow the Nagle algorithm for deciding when to send >> data.By >> default, TCP will follow the Nagle algorithm is disable this behavior , >> can be >> configured to TRUE(1) or FALSE(0) >> >> diff --git a/osaf/libs/core/mds/include/mds_dt_tcp_disc.h >> b/osaf/libs/core/mds/include/mds_dt_tcp_disc.h >> --- a/osaf/libs/core/mds/include/mds_dt_tcp_disc.h >> +++ b/osaf/libs/core/mds/include/mds_dt_tcp_disc.h >> @@ -70,7 +70,6 @@ server_instance_upper + sub_ref_val + su >> #define MDS_SND_VERSION 1 >> #define MDS_RCV_IDENTIFIRE 0x56123456 >> #define MDS_RCV_VERSION 1 >> -#define DTM_INTRANODE_MSG_SIZE 1500 >> #define DTM_INTRANODE_UNSENT_MSG 200 >> >> #define MDS_MDTM_LOWER_INSTANCE 0x >> diff --git a/osaf/libs/core/mds/mds_dt_tcp.c >> b/osaf/libs/core/mds/mds_dt_tcp.c >> --- a/osaf/libs/core/mds/mds_dt_tcp.c >> +++ b/osaf/libs/core/mds/mds_dt_tcp.c >> @@ -38,7 +38,6 @@ >> #include >> >> #define MDS_MDTM_SUN_PATH 255 >> -#define MDS_SND_RCV_SIZE 64000 >> #define MDS_MDTM_CONNECT_PATH PKGLOCALSTATEDIR "/osaf_dtm_intra_server" >> >> #ifndef MDS_PORT_NUMBER >> @@ -53,6 +52,9 @@ >> /* Send_buffer_size + MDS_MDTM_DTM_PID_BUFFER_SIZE */ >> #define MDS_MDTM_DTM_PID_BUFFER_SIZE (2 + MDS_MDTM_DTM_PID_SIZE) >> >> +/* The default value is set by the rmem_default/wmem_default */ >> +#define MDS_SOCK_SND_RCV_BUF_SIZE 126976 >> + >> extern uint32_t mdtm_num_subscriptions; >> extern MDS_SUBTN_REF_VAL mdtm_handle; >> extern uint32_t mdtm_global_frag_num_tcp; >> @@ -79,11 +81,12 @@ uint32_t mdtm_process_recv_events_tcp(vo >> uint32_t mds_mdtm_init_tcp(NODE_ID nodeid, uint32_t *mds_tcp_ref) >> { >> uint32_t flags; >> -uint32_t size = MDS_SND_RCV_SIZE; >> +uint32_t size = MDS_SOCK_SND_RCV_BUF_SIZE; >> struct sockaddr_un server_addr_un, dhserver_addr_un; >> struct sockaddr_in server_addr_in; >> struct sockaddr_in6 server_addr_in6; >> uint8_t buffer[MDS_MDTM_DTM_PID_BUFFER_SIZE]; >> +char *ptr; >> >> mdtm_pid = getpid(); >> >> @@ -116,6 +119,19 @@ uint32_t mds_mdtm_init_tcp(NODE_ID nodei >> syslog(LOG_ERR, "MDS:MDTM:TCP ncs_patricia_tree_init failed >> MDTM_INIT\n"); >> return NCSCC_RC_FAILURE; >> } >> + >> +/* setting MDS_SOCK_SND_RCV_BUF_SIZE from environment variable if
Re: [devel] [PATCH 1 of 1] mds: TCP performance improvements [#654]
Another question, what would be the use case of disabling the Nagle algorithm? If you don't have one I am not sure we should do the change... Thanks, Hans > -Original Message- > From: A V Mahesh [mailto:mahesh.va...@oracle.com] > Sent: den 29 april 2014 09:06 > To: Hans Feldt > Cc: opensaf-devel@lists.sourceforge.net; mathi.naic...@oracle.com > Subject: Re: [PATCH 1 of 1] mds: TCP performance improvements [#654] > > > Ok I will split and publish again with specific Commit Message. > > -AVM > > > On 4/29/2014 12:19 PM, Hans Feldt wrote: > > Hi, > > > > This seems like 3 different logical changes thus should be 3 different > > patches. You should not change the MDS library in the same > patch as the dtm infrastructure service (unless the changes are very tightly > coupled). > > > > What is the actual MDS/TCP performance improvement here and what change is > > bringing it? > > I think it would be the increased fragmentation limit but let's be explicit > > about it in the commit message. > > > > Thanks, > > Hans > > > >> -Original Message- > >> From: mahesh.va...@oracle.com [mailto:mahesh.va...@oracle.com] > >> Sent: den 29 april 2014 08:20 > >> To: opensaf-devel@lists.sourceforge.net; Hans Feldt; > >> mathi.naic...@oracle.com > >> Cc: opensaf-devel@lists.sourceforge.net > >> Subject: [PATCH 1 of 1] mds: TCP performance improvements [#654] > >> > >> osaf/libs/core/mds/include/mds_dt_tcp_disc.h | 1 - > >> osaf/libs/core/mds/mds_dt_tcp.c| 20 +- > >> osaf/libs/core/mds/mds_dt_trans.c | 12 +- > >> osaf/services/infrastructure/dtms/config/dtmd.conf | 23 +++ > >> osaf/services/infrastructure/dtms/dtm/dtm_intra.c | 13 +-- > >> osaf/services/infrastructure/dtms/dtm/dtm_node_sockets.c | 44 > >> ++--- > >> osaf/services/infrastructure/dtms/dtm/dtm_read_config.c| 30 > >> + > >> osaf/services/infrastructure/dtms/include/dtm.h| 2 + > >> osaf/services/infrastructure/dtms/include/dtm_cb.h | 3 + > >> osaf/services/infrastructure/dtms/include/dtm_intra_disc.h | 2 - > >> 10 files changed, 109 insertions(+), 41 deletions(-) > >> > >> > >> This path is on top of Hans Feldt published on > >> 4/29/2014 10:58 AM > >> > >> 1) Their is NO issue with MDS TCP in-service Upgrade ,The value of > >> SO_RCVBUF & SO_SNDBUF of TCP sockets was set to 64000 > >> (MDS_SND_RCV_SIZE) > >> using setsockopt() ,the kernel doubles this value ,and this doubled > >> value is returned by getsockopt(). So MDS_DIRECT_BUF_MAXSIZE size is > >> used to send for for both > >> previous & current versions of Opensaf nodes which are using MDS TCP > >> as transport > >> > >> 2) Made DTM-TCP inter-node & intra-node socket SO_SNDBUF and > >> SO_RCVBUF buffer in bytes configurable . The kernel doubles this value > >> (to > >> allow space for bookkeeping over-head) when it is set using > >> setsockopt(),and > >> this doubled value is returned by getsockopt(). The default value is > >> set to DTM_SOCK_SND_RCV_BUF_SIZE=126976 > >> > >> 3) Made TCP_NODELAY configurable to specifi whether MDS TCP > >> transport should follow the Nagle algorithm for deciding when to send > >> data.By > >> default, TCP will follow the Nagle algorithm is disable this behavior > >> , can be > >> configured to TRUE(1) or FALSE(0) > >> > >> diff --git a/osaf/libs/core/mds/include/mds_dt_tcp_disc.h > >> b/osaf/libs/core/mds/include/mds_dt_tcp_disc.h > >> --- a/osaf/libs/core/mds/include/mds_dt_tcp_disc.h > >> +++ b/osaf/libs/core/mds/include/mds_dt_tcp_disc.h > >> @@ -70,7 +70,6 @@ server_instance_upper + sub_ref_val + su > >> #define MDS_SND_VERSION 1 > >> #define MDS_RCV_IDENTIFIRE 0x56123456 > >> #define MDS_RCV_VERSION 1 > >> -#define DTM_INTRANODE_MSG_SIZE 1500 > >> #define DTM_INTRANODE_UNSENT_MSG 200 > >> > >> #define MDS_MDTM_LOWER_INSTANCE 0x > >> diff --git a/osaf/libs/core/mds/mds_dt_tcp.c > >> b/osaf/libs/core/mds/mds_dt_tcp.c > >> --- a/osaf/libs/core/mds/mds_dt_tcp.c > >> +++ b/osaf/libs/core/mds/mds_dt_tcp.c > >> @@ -38,7 +38,6 @@ > >> #include > >> > >> #define MDS_MDTM_SUN_PATH 255 > >> -#define MDS_SND_RCV_SIZE 64000 > >> #define MDS_MDTM_CONNECT_PATH PKGLOCALSTATEDIR "/osaf_dtm_intra_server" > >> > >> #ifndef MDS_PORT_NUMBER > >> @@ -53,6 +52,9 @@ > >> /* Send_buffer_size + MDS_MDTM_DTM_PID_BUFFER_SIZE */ > >> #define MDS_MDTM_DTM_PID_BUFFER_SIZE (2 + MDS_MDTM_DTM_PID_SIZE) > >> > >> +/* The default value is set by the rmem_default/wmem_default */ > >> +#define MDS_SOCK_SND_RCV_BUF_SIZE 126976 > >> + > >> extern uint32_t mdtm_num_subscriptions; > >> extern MDS_SUBTN_REF_VAL mdtm_handle; > >> extern uint32_t mdtm_global_frag_num_tcp; > >> @@ -79,11 +81,12 @@ uint32_t mdtm_process_recv_events_tcp(vo > >> uint32_t mds_mdtm_init_tcp(NODE_ID nodeid, uint32_t *mds_tcp_ref) > >> { > >>
Re: [devel] [PATCH 5 of 5] amfd: add and use SU method no_assignments_of_hastate [#713]
Ack for all the series of patches with one comment on this one: Suggested name of +int AVD_SU::no_assignments_of_hastate(SaAmfHAStateT ha_state) { To +int AVD_SU:: hastate_assignments_count(SaAmfHAStateT ha_state) { Thanks -Nagu > -Original Message- > From: Hans Feldt [mailto:osafde...@gmail.com] > Sent: 22 April 2014 16:28 > To: Praveen Malviya; Nagendra Kumar; hans.nordeb...@ericsson.com > Cc: opensaf-devel@lists.sourceforge.net > Subject: [PATCH 5 of 5] amfd: add and use SU method > no_assignments_of_hastate [#713] > > osaf/services/saf/amf/amfd/include/su.h | 2 +- > osaf/services/saf/amf/amfd/sg_nway_fsm.cc | 2 +- > osaf/services/saf/amf/amfd/su.cc | 12 +--- > 3 files changed, 7 insertions(+), 9 deletions(-) > > > 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 > @@ -91,6 +91,7 @@ class AVD_SU { > struct avd_sutype *su_type; > AVD_SU *su_list_su_type_next; > > + int no_assignments_of_hastate(SaAmfHAStateT ha_state); > void add_comp(struct avd_comp_tag *comp); > void remove_comp(struct avd_comp_tag *comp); > void set_admin_state(SaAmfAdminStateT admin_state); > @@ -192,7 +193,6 @@ extern void avd_su_inc_curr_act_si(AVD_S > extern void avd_su_dec_curr_act_si(AVD_SU *su); > extern void avd_su_inc_curr_stdby_si(AVD_SU *su); > extern void avd_su_dec_curr_stdby_si(AVD_SU *su); > -extern uint32_t avd_su_get_current_no_of_assignments(AVD_SU *su, > SaAmfHAStateT ha_state); > extern AVD_SU *avd_su_get_or_create(const SaNameT *dn); > extern void su_nd_attribute_update(const AVD_SU *su, > AVSV_AMF_SU_ATTR_ID attrib_id); > > diff --git a/osaf/services/saf/amf/amfd/sg_nway_fsm.cc > b/osaf/services/saf/amf/amfd/sg_nway_fsm.cc > --- a/osaf/services/saf/amf/amfd/sg_nway_fsm.cc > +++ b/osaf/services/saf/amf/amfd/sg_nway_fsm.cc > @@ -1321,7 +1321,7 @@ AVD_SU *avd_sg_nway_get_su_std_equal(AVD > l_flag = true; > > /* Get the current no of Standby assignments on the su */ > - curr_su_stdby_cnt = > avd_su_get_current_no_of_assignments(curr_su, SA_AMF_HA_STANDBY); > + curr_su_stdby_cnt = curr_su- > >no_assignments_of_hastate(SA_AMF_HA_STANDBY); > > /* first try to select an SU which has no assignments */ > if ((curr_su->saAmfSuReadinessState == > SA_AMF_READINESS_IN_SERVICE) && > diff --git a/osaf/services/saf/amf/amfd/su.cc > b/osaf/services/saf/amf/amfd/su.cc > --- a/osaf/services/saf/amf/amfd/su.cc > +++ b/osaf/services/saf/amf/amfd/su.cc > @@ -105,18 +105,16 @@ AVD_SU *avd_su_get_or_create(const SaNam > /** > * @brief gets the current no of assignmnents on a SU for a particular > state > * > - * @param[in] su > * @param[in] ha_state > * > * @return returns current assignment cnt > */ > -uint32_t avd_su_get_current_no_of_assignments(AVD_SU *su, > SaAmfHAStateT ha_state) > -{ > - AVD_SU_SI_REL *curr_susi; > - uint32_t curr_assignment_cnt = 0; > +int AVD_SU::no_assignments_of_hastate(SaAmfHAStateT ha_state) { > + const AVD_SU_SI_REL *susi; > + int curr_assignment_cnt = 0; > > - for (curr_susi = su->list_of_susi;curr_susi != NULL;curr_susi = > curr_susi- > >su_next) { > - if (curr_susi->state == ha_state) > + for (susi = list_of_susi; susi != NULL; susi = susi->su_next) { > + if (susi->state == ha_state) > curr_assignment_cnt++; > } > -- "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE Instantly run your Selenium tests across 300+ browser/OS combos. Get unparalleled scalability from the best Selenium testing platform available. Simple to use. Nothing to install. Get started now for free." http://p.sf.net/sfu/SauceLabs ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1 of 1] amfnd: Cancel qscing timer on CSI removal [#583]
I don't find any use case to it, but Ack. Thanks -Nagu > -Original Message- > From: Gary Lee [mailto:gary@dektech.com.au] > Sent: 28 April 2014 09:25 > To: Nagendra Kumar; hans.fe...@ericsson.com; > hans.nordeb...@ericsson.com; Praveen Malviya > Cc: opensaf-devel@lists.sourceforge.net > Subject: [PATCH 1 of 1] amfnd: Cancel qscing timer on CSI removal [#583] > > osaf/services/saf/amf/amfnd/cbq.cc | 7 +++ > 1 files changed, 7 insertions(+), 0 deletions(-) > > > If a SI is in SHUTTING-down mode and a lock request arrives, the component is > required to call saAmfCSIQuiescingComplete() which shouldn't be needed. > > Therefore we should stop the qscing complete timer on CSI removal. > > diff --git a/osaf/services/saf/amf/amfnd/cbq.cc > b/osaf/services/saf/amf/amfnd/cbq.cc > --- a/osaf/services/saf/amf/amfnd/cbq.cc > +++ b/osaf/services/saf/amf/amfnd/cbq.cc > @@ -483,6 +483,13 @@ uint32_t avnd_evt_ava_resp_evh(AVND_CB * > if (NCSCC_RC_SUCCESS != rc) > goto done; > } > + > + /* cancel qscing_tmr in case this is this called > + during an admin shutdown etc.. */ > + if (m_AVND_TMR_IS_ACTIVE(comp->qscing_tmr)) { > + m_AVND_TMR_COMP_QSCING_CMPL_STOP(cb, > comp); > + } > + > break; > > case AVSV_AMF_PXIED_COMP_INST: -- "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE Instantly run your Selenium tests across 300+ browser/OS combos. Get unparalleled scalability from the best Selenium testing platform available. Simple to use. Nothing to install. Get started now for free." http://p.sf.net/sfu/SauceLabs ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 0 of 1] Review Request for amfd: infinite loop when processing CLM tracking validate #877
Summary: amfd: fix infinite loop when processing CLM node tracking validate Review request for Trac Ticket(s): #877 Peer Reviewer(s): Nagendra, Praveen, Hans F Pull request to: Affected branch(es): default Development branch: Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts n SAF servicesy OpenSAF servicesn Core libraries n Samples n Tests n Other n Comments (indicate scope for each "y" above): - changeset 9e1708d7a66517fcff52d6af094000feb1897143 Author: Alex Jones Date: Mon, 28 Apr 2014 15:56:46 -0400 amfd: fix infinite loop when processing CLM node tracking [#877] Logs from amfnd: Apr 25 21:37:13.521696 osafamfnd [4547] ER AMF director heart beat timeout, generating core for amfd clm_node_exit_validate loops through all the SUs for the passed in node, but never actually moves to the next SU, so the while test is always true. By moving on the the next SU in the loop, we can exit the while loop when there are no more SUs. Complete diffstat: -- osaf/services/saf/amf/amfd/clm.cc | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) Testing Commands: - Testing, Expected Results: -- Conditions of Submission: - Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: --- 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. -- "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE Instantly run your Selenium tests across 300+ browser/OS combos. Get unparalleled scalability from the best Selenium testing platform available. Simple to use. Nothing to install. Get started now for free." http://p.sf.net/sfu/SauceLabs ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 1 of 1] amfd: fix infinite loop when processing CLM node tracking [#877]
osaf/services/saf/amf/amfd/clm.cc | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) Logs from amfnd: Apr 25 21:37:13.521696 osafamfnd [4547] ER AMF director heart beat timeout, generating core for amfd clm_node_exit_validate loops through all the SUs for the passed in node, but never actually moves to the next SU, so the while test is always true. By moving on the the next SU in the loop, we can exit the while loop when there are no more SUs. diff --git a/osaf/services/saf/amf/amfd/clm.cc b/osaf/services/saf/amf/amfd/clm.cc --- a/osaf/services/saf/amf/amfd/clm.cc +++ b/osaf/services/saf/amf/amfd/clm.cc @@ -114,6 +114,7 @@ static void clm_node_exit_validate(AVD_A } susi = susi->su_next; } + su = su->avnd_list_su_next; } done: -- "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE Instantly run your Selenium tests across 300+ browser/OS combos. Get unparalleled scalability from the best Selenium testing platform available. Simple to use. Nothing to install. Get started now for free." http://p.sf.net/sfu/SauceLabs ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1 of 1] amfd: fix infinite loop when processing CLM node tracking [#877]
What is the test case, CLM node lock? /Hans > -Original Message- > From: Alex Jones [mailto:ajo...@genband.com] > Sent: den 29 april 2014 16:04 > To: Hans Feldt > Cc: opensaf-devel@lists.sourceforge.net > Subject: [PATCH 1 of 1] amfd: fix infinite loop when processing CLM node > tracking [#877] > > osaf/services/saf/amf/amfd/clm.cc | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > > Logs from amfnd: > Apr 25 21:37:13.521696 osafamfnd [4547] ER AMF director heart beat timeout, > generating core for amfd > > clm_node_exit_validate loops through all the SUs for the passed in node, but > never actually moves to the next SU, so the while test is always true. > > By moving on the the next SU in the loop, we can exit the while loop when > there > are no more SUs. > > diff --git a/osaf/services/saf/amf/amfd/clm.cc > b/osaf/services/saf/amf/amfd/clm.cc > --- a/osaf/services/saf/amf/amfd/clm.cc > +++ b/osaf/services/saf/amf/amfd/clm.cc > @@ -114,6 +114,7 @@ static void clm_node_exit_validate(AVD_A > } > susi = susi->su_next; > } > + su = su->avnd_list_su_next; > } > > done: -- "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE Instantly run your Selenium tests across 300+ browser/OS combos. Get unparalleled scalability from the best Selenium testing platform available. Simple to use. Nothing to install. Get started now for free." http://p.sf.net/sfu/SauceLabs ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1 of 1] amfd: fix infinite loop when processing CLM node tracking [#877]
Actually, it is a PLM Deactivation request, which CLM translates into NODE_LEFT. Alex On 04/29/2014 10:15 AM, Hans Feldt wrote: > What is the test case, CLM node lock? > /Hans > >> -Original Message- >> From: Alex Jones [mailto:ajo...@genband.com] >> Sent: den 29 april 2014 16:04 >> To: Hans Feldt >> Cc: opensaf-devel@lists.sourceforge.net >> Subject: [PATCH 1 of 1] amfd: fix infinite loop when processing CLM node >> tracking [#877] >> >> osaf/services/saf/amf/amfd/clm.cc | 1 + >> 1 files changed, 1 insertions(+), 0 deletions(-) >> >> >> Logs from amfnd: >> Apr 25 21:37:13.521696 osafamfnd [4547] ER AMF director heart beat timeout, >> generating core for amfd >> >> clm_node_exit_validate loops through all the SUs for the passed in node, but >> never actually moves to the next SU, so the while test is always true. >> >> By moving on the the next SU in the loop, we can exit the while loop when >> there >> are no more SUs. >> >> diff --git a/osaf/services/saf/amf/amfd/clm.cc >> b/osaf/services/saf/amf/amfd/clm.cc >> --- a/osaf/services/saf/amf/amfd/clm.cc >> +++ b/osaf/services/saf/amf/amfd/clm.cc >> @@ -114,6 +114,7 @@ static void clm_node_exit_validate(AVD_A >> } >> susi = susi->su_next; >> } >> +su = su->avnd_list_su_next; >> } >> >> done: -- "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE Instantly run your Selenium tests across 300+ browser/OS combos. Get unparalleled scalability from the best Selenium testing platform available. Simple to use. Nothing to install. Get started now for free." http://p.sf.net/sfu/SauceLabs ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [users] Websites and information
These things are decided by the Technical Leadership Council. About a year ago we decided to migrate from our own server to SourceForge. The old server is still up and running as an archive of old information, but it is no longer maintained. Eventually we wish to be able to shut it down, and you could help out with making this possible if you wish. We were thinking of downloading everything from the old server and store it as static web pages. So what you could do is to download all the old pages (wiki, tickets, mailing list archives) and create a tarball with static html pages. We can then upload the static html pages to http://opensaf.sourceforge.net. They have support for virtual hosts at SourceForge, so it would be possible to redirect the DNS to the new server. Unfortunately, we have no control of www.opensaf.org ourselves. They were supposed to have fixed the links by now, and I can see that they have indeed inserted a link to the new project page at SourceForge, but the old links also remain. / Anders Widell 2014-04-29 14:55, Guilherme Moro skrev: > Hi, > > The project was completely migrated to Sourceforge, is this a valid > assumption? > > if you go to http://www.opensaf.org/page/14944~220835/Projects > They are still referencing http://devel.opensaf.org/hg/opensaf as the > main check-in point > > http://devel.opensaf.org/ have some other information that seems > outdated as well > > So, for now we seem to have duplicated info scattered around the old > trac system and the new home of the project (sourceforge). And most of > the google search you do will point to outdated information. > > I know the tickets were migrated already, but we should start > deactivate the old systems gradually, and put everything in just one > place. > > I'm willing to help with that, so who can guide me through whatever > got decided regarding this migration, and more importantly, who > decides this stuff anyway? ;) > > Regards, > > Guilherme Moro > > -- > "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE > Instantly run your Selenium tests across 300+ browser/OS combos. Get > unparalleled scalability from the best Selenium testing platform available. > Simple to use. Nothing to install. Get started now for free." > http://p.sf.net/sfu/SauceLabs > ___ > Opensaf-users mailing list > opensaf-us...@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/opensaf-users > > -- "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE Instantly run your Selenium tests across 300+ browser/OS combos. Get unparalleled scalability from the best Selenium testing platform available. Simple to use. Nothing to install. Get started now for free." http://p.sf.net/sfu/SauceLabs ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 0 of 1] Review Request for imm: Allow admin-operations directly targeting an implementer/applier [#799]
Summary: imm: Allow admin-operations directly targeting an implementer/applier [#799] Review request for Trac Ticket(s): 799 Peer Reviewer(s): Neel; Zoran Pull request to: Affected branch(es): default(4.5) Development branch: Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts n SAF servicesy OpenSAF servicesn Core libraries n Samples n Tests n Other n Comments (indicate scope for each "y" above): - <> changeset f3bd590108f2bdb2bada136baf9b31bc8d963460 Author: Anders Bjornerstedt Date: Tue, 29 Apr 2014 16:17:10 +0200 imm: Allow admin-operations directly targeting an implementer/applier [#799] This enhancement (#799) extends the scope of the existing OpenSAF API for administrative-operations such that it supports the direct invocation of admin-operations on an implementer or applier. See the included patch of: osaf/services/saf/immsv/README for details. Complete diffstat: -- osaf/libs/agents/saf/imma/imma_oi_api.c | 6 osaf/services/saf/immsv/README | 84 +++- osaf/services/saf/immsv/immnd/ImmModel.cc| 14 +++ osaf/tools/safimm/immadm/imm_admin.c | 7 - tests/immsv/implementer/test_SaImmOiAdminOperation.c | 34 +++ 5 files changed, 131 insertions(+), 14 deletions(-) Testing Commands: - A new test case is included as 'immoitest 5 13'. Manual testing is also possible using the tool 'immapplier' to set up an OI or an applier; and the 'immadm' tool to invoke an admin operation. Set up an OI for example on SC1: immapplier -a MyOI OpensafImmTest The class name is only needed here tpo satisfy the immapplier tool. You could uswe any class that does not have an existing OI, or use *any* class and set up as an @pplier. Invoke an admin-op towards the OI. Use the OI-name (or applier name) plus explicitly set admin-owner also to be the same OI-name (or applier name): immadm -a MyOI -o 1 MyOI You can also try invoking bogus admin-operations directly on say the saAmfService: immadm -a safAmfService -o 4711 safAmfService AdminOwnerName == ImplementerName (safAmfService) - Could be direct admin-op on OI error - saImmOmAdminOperationInvoke_2 admin-op RETURNED: SA_AIS_ERR_INVALID_PARAM (7) error-string: Admin operation not supported for safAmfService (0) This is a good test of the robustness of the OIs. Some OIs are not so robust and crash only because they receive an unexpected admin-operation. Testing, Expected Results: -- Invoking an admin-opertion directly on OI or applier should result in the admin-op reaching that OI or applier. Conditions of Submission: - Ack from Neel (Ack from Zoran not mandatory) Arch Built StartedLinux distro --- mipsn 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 p
[devel] [PATCH 1 of 1] imm: Allow admin-operations directly targeting an implementer/applier [#799]
osaf/libs/agents/saf/imma/imma_oi_api.c | 6 - osaf/services/saf/immsv/README | 84 +++- osaf/services/saf/immsv/immnd/ImmModel.cc| 14 +++ osaf/tools/safimm/immadm/imm_admin.c | 7 +- tests/immsv/implementer/test_SaImmOiAdminOperation.c | 34 ++- 5 files changed, 131 insertions(+), 14 deletions(-) This enhancement (#799) extends the scope of the existing OpenSAF API for administrative-operations such that it supports the direct invocation of admin-operations on an implementer or applier. See the included patch of: osaf/services/saf/immsv/README for details. diff --git a/osaf/libs/agents/saf/imma/imma_oi_api.c b/osaf/libs/agents/saf/imma/imma_oi_api.c --- a/osaf/libs/agents/saf/imma/imma_oi_api.c +++ b/osaf/libs/agents/saf/imma/imma_oi_api.c @@ -901,12 +901,6 @@ static SaAisErrorT admin_op_result_commo TRACE_1("Reactive ressurect of handle %llx succeeded", immOiHandle); } - if (cl_node->isApplier) { - rc = SA_AIS_ERR_BAD_HANDLE; - TRACE_2("ERR_BAD_HANDLE: The SaImmOiHandleT is associated with an >>applier<< name"); - goto stale_handle; - } - if(isA2bCall && !(cl_node->isImmA2b)) { rc = SA_AIS_ERR_VERSION; TRACE_2("ERR_VERSION: saImmOmAdminOperationInvoke_o2 only supported for " diff --git a/osaf/services/saf/immsv/README b/osaf/services/saf/immsv/README --- a/osaf/services/saf/immsv/README +++ b/osaf/services/saf/immsv/README @@ -1758,16 +1758,96 @@ incremental validation. This can be achi it then replaying the same operations (that validated), followed by adding more operations. Replay is necessary because the OIs ccb-protocol can only cope with one invocation of validation (completed callback) for any given -ccb-id. Another possible us is in integrating IMM transaction commit +ccb-id. Another possible use is in integrating IMM transaction commit with the commit of some other transaction handling system. By validating without committing, the risk is significantly reduced that a final commit/apply will fail. The invocation of saImmOmCcbApply after a -successfull invocation of saImmOmCcbValidate can still fail/abort +successfull invocation of saImmOmCcbValidate can still fail/abort, but only due to resource problems (such as server crash or filesystem unavailability). This is a blocking syncronous call. + +Allow admin-operations directly targeting an implementer or applier (4.5) +== +https://sourceforge.net/p/opensaf/tickets/799/ + + +The existing IMM admin-operation API in OpenSAF: + +saImmOmAdminOperationInvoke_2() +saImmOmAdminOperationInvoke_o2() +saImmOmAdminOperationInvokeAsync_2() +SaImmOmAdminOperationInvokeCallbackT() +aImmOmAdminOperationInvokeCallbackT_o2() + +SaImmOiAdminOperationCallbackT_2() +saImmOiAdminOPerationResult() +saImmOiAdminOperationResult_o2() + +allows OM clients to invoke procedural requests directed at an imm object. +The OM client needs to have an admin-owner handle associated with an admin-owner +name that matches the admin-owner for the object. The actual target/receiver of +the admin-operation is really the implementer/OI registered for the object or +class. The object that anchors the request is effectively a parameter in the +request to the OI. + +This enhancement (#799) does not add any new API. It extends the allowed scope +for the existing admin-operation API to cover the case where the target is an OI +directly. Instead of supplying the DN of an object as the address/target, an +implementer-name is provided as the address/target. This allows the OM client to +invoke admin-operations directly and explicitly on an OI. The implementer-name +is used as a proxy for the object-name in the admin-operation API. + +Because the extended scope does not require the addition of any new API, access +to this extension is not based on the handle version used. But the enhancement +is introduced in OpenSAF release 4.5 and thus only available in that release or +later releases. + +The om-side admin-operation-invoke call takes an admin-owner-handle as argument. +The admin-owner handle needs to be an initialized handle and it must set the +admin-owner-name to be the *same* as the implementer-name. This may seem peculiar +and unnecessary. But it serves both as a confirmation that the OM client has not +made some mistake in using the new extended scope; and as a possible future +entry point for authorizing admin-operation access directly targeting the OI. + +The main benefit of this extension of scope, is that it allows a simple way for +applications to set up cluster scoped remote procedure call communication, where the +request arives FEVS-synchronously at the target. FEVS is the cluster synchronized +messaging mechanism used by the Immsv (see earlier i
Re: [devel] [PATCH 2 of 2] clm: add ETF file for CLMD [#848]
Comments inline: Thanks, Mathi. - hans.fe...@ericsson.com wrote: > See inline > > Thanks, > > Hans > > > > > > > > > From: Mathivanan Naickan Palanivelu [mailto:mathi.naic...@oracle.com] > Sent: den 25 april 2014 12:08 > To: Hans Feldt; Bertil Engelholm > Cc: opensaf-devel@lists.sourceforge.net > Subject: RE: [PATCH 2 of 2] clm: add ETF file for CLMD [#848] > > > > Nice start. > > > > Like Bertil commented, yes CLMNA should also be part of the same ETF > for the CLM ‘software’! > > [Hans] sure this was just an RFC, the same or another file? > I don't know which is better, i.e. ETF per middleware service or ETF per middleware component. I'm inclining towards ETF per middleware service! The SMF spec talks of "ETF w.r.t software bundles", not sure if it should be seen as ETF 'per' software bundle! To quote: "When a software vendor provides a new software bundle, an entity types file describ- ing the content of the software bundle shall be supplied with the bundle." ..."One entity types file may refer to multiple software bundles, each of which must be identified according to the XML schema" > And, > > > > What about the below, will they be part of some OpenSAF common ETF? > > +++ > > +-+--- > > +++ > > +++ > > + > > [Hans] I currently don’t see any need for that. > > The SMF spec/schema already defines which elements should be mandatory element, optional or alternative. We could use that as the guideline! > > And did you not think of the need for defining any appropriate item > from below? > > [Hans] Sorry I don’t get this list, what exactly is missing from this > ETF to generate the same imm.xml as today > While there is a rule/guideline from the SMF spec, it also depends on the scope for the tool that is going to use this ETF file. I'm not sure about the scope of this activity(patch request)!? You could weigh all your comments below w.r.t the guideline for ETF from the SMF spec. > > > - > > +++-- > > +-+ > > [Hans] I currently don’t see a need for the bundle part > > > > +- > > [Hans] already in there > > +- > > +- > > [Hans] could add that > > +-- > > [Hans] already in there > > +- > > [Hans] could add that > > +- > > +- > > +- > > +- > > +-- > > +- > > +- > > +- > > +-- > > +-- > > +-- > > +-- > > +-- > > +-- > > +-- > > +-- > > +-- > > [Hans] already in there > > +-- > > +-- > > + > > +-- > > +-- > > +- > > [Hans] yes the relevant ones are configured > > + > > > > > > Thanks, > > Mathi. > > > > > > >-Original Message- > > >From: Hans Feldt [ mailto:hans.fe...@ericsson.com ] > > >Sent: Thursday, April 24, 2014 7:10 PM > > >To: bertil.engelh...@ericsson.com ; Mathivanan Naickan Palanivelu > > >Cc: opensaf-devel@lists.sourceforge.net > > >Subject: [PATCH 2 of 2] clm: add ETF file for CLMD [#848] > > > > > > osaf/services/saf/clmsv/config/ETF-CLMD.xml | 35 > > >+ > > > osaf/services/saf/clmsv/config/Makefile.am | 2 + > > > 2 files changed, 37 insertions(+), 0 deletions(-) > > > > > > > > >Add an ETF file for CLMD. The contents corresponds to what we have in > the > > >IMM XML file. > > > > > >diff --git a/osaf/services/saf/clmsv/config/ETF-CLMD.xml > > >b/osaf/services/saf/clmsv/config/ETF-CLMD.xml > > >new file mode 100644 > > >--- /dev/null > > >+++ b/osaf/services/saf/clmsv/config/ETF-CLMD.xml > > >@@ -0,0 +1,35 @@ > > >+ > > >+http://www.w3.org/2001/XMLSchema- > > >instance " name="CLMD" provider="OSAF"> > > >+ > > >+ > >version="safVersion=1"> > > >+ > >version="safVersion=1"> > > >+ > > >+ > > >+ > > >+ > >key="safHealthcheckKey=Default"> > > >+ > >default="2400"/> > > >+ > >lowerBound="1800"/> > > >+ > > >+ > >lowerBound="100" default="100"/> > > >+ > > >+ > > >+ > > > xxCLCCLIDIRxx/osaf-clmd > > >+ instantiate > > >+ > > >+ > > >+ > > > xxCLCCLIDIRxx/osaf-clmd > > >+ cleanup > > >+ > > >+ > > >+ > > >+ true > > >+ > > > SA_AMF_NODE_FAILFAST > > >+ > > >+ > >lowerBound="400"/> > > >+ > > >+ > > >+ > > >+ > > >+ > >version="safVersion=1" /> > > >+ > > >+ > > >diff --git a/osaf/services/saf/clmsv/config/Makefile.am > > >b/osaf/services/saf/clmsv/config/Makefile.am > > >--- a/osaf/services/saf/clmsv/config/Makefile.am > > >+++ b/osaf/services/saf/clmsv/config/Makefile.am > > >@@ -38,6 +38,8 @@ dist_pkgimmxml_svc_DATA += \ > > > clmsv_pl_template.xml > > > endif > > > > > >+dist_pkgetfxml_svc_DATA = \ > > >+ ETF-CLMD.xml > > > endif > > > > > > install-data-hook: -- "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE Instantly run your Selenium tests across 300+ browser/OS combos. Get unparalleled scalability
[devel] [PATCH 7 of 7] amfd: add and use SU is_in_service [#713]
osaf/services/saf/amf/amfd/include/su.h | 9 + osaf/services/saf/amf/amfd/node.cc | 4 +--- osaf/services/saf/amf/amfd/sgproc.cc| 16 osaf/services/saf/amf/amfd/su.cc| 12 +++- 4 files changed, 17 insertions(+), 24 deletions(-) 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 @@ -116,6 +116,7 @@ class AVD_SU { void set_term_state(bool state); void set_su_switch(SaToggleState state); struct avd_avnd_tag *get_node_ptr(void); + bool is_in_service(void); private: void send_attribute_update(AVSV_AMF_SU_ATTR_ID attrib_id); @@ -133,14 +134,6 @@ typedef struct { } AVD_SUTCOMP_TYPE; -#define m_AVD_APP_SU_IS_INSVC(i_su,su_node_ptr) \ -((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)\ -) - /** * Get SUs from IMM and create internal objects * diff --git a/osaf/services/saf/amf/amfd/node.cc b/osaf/services/saf/amf/amfd/node.cc --- a/osaf/services/saf/amf/amfd/node.cc +++ b/osaf/services/saf/amf/amfd/node.cc @@ -877,9 +877,7 @@ void avd_node_admin_lock_unlock_shutdown su = node->list_of_su; while (su != NULL) { - su_node_ptr = su->get_node_ptr(); - - if (m_AVD_APP_SU_IS_INSVC(su, su_node_ptr) && + if ((su->is_in_service() == true) && ((su->saAmfSUPreInstantiable) ? (su->saAmfSUPresenceState == SA_AMF_PRESENCE_INSTANTIATED):true)) { /* Pres state check is to prevent assignment to SU in case node is instantiating diff --git a/osaf/services/saf/amf/amfd/sgproc.cc b/osaf/services/saf/amf/amfd/sgproc.cc --- a/osaf/services/saf/amf/amfd/sgproc.cc +++ b/osaf/services/saf/amf/amfd/sgproc.cc @@ -406,7 +406,6 @@ void avd_su_oper_state_evh(AVD_CL_CB *cb AVD_AVND *node; AVD_SU *su, *i_su; SaAmfReadinessStateT old_state; - AVD_AVND *su_node_ptr = NULL; bool node_reboot_req = true; TRACE_ENTER2("id:%u, node:%x, '%s' state:%u", n2d_msg->msg_info.n2d_opr_state.msg_id, @@ -444,8 +443,6 @@ void avd_su_oper_state_evh(AVD_CL_CB *cb goto done; } - su_node_ptr = su->get_node_ptr(); - if (n2d_msg->msg_info.n2d_opr_state.rec_rcvr.saf_amf == SA_AMF_NODE_SWITCHOVER) { saflog(LOG_NOTICE, amfSvcUsrName, "Node Switch-Over requested by '%s'", node->name.value); @@ -684,7 +681,7 @@ void avd_su_oper_state_evh(AVD_CL_CB *cb goto done; } - if (m_AVD_APP_SU_IS_INSVC(su, su_node_ptr)) { + if (su->is_in_service() == true) { su->set_readiness_state(SA_AMF_READINESS_IN_SERVICE); if ((cb->init_state == AVD_APP_STATE) && (old_state == SA_AMF_READINESS_OUT_OF_SERVICE)) { /* An application SU has become in service call SG FSM */ @@ -1269,7 +1266,6 @@ void avd_su_si_assign_evh(AVD_CL_CB *cb, void avd_sg_app_node_su_inst_func(AVD_CL_CB *cb, AVD_AVND *avnd) { AVD_SU *i_su; - AVD_AVND *su_node_ptr = NULL; TRACE_ENTER2("'%s'", avnd->name.value); @@ -1299,9 +1295,8 @@ void avd_sg_app_node_su_inst_func(AVD_CL } else { i_su->set_oper_state(SA_AMF_OPERATIONAL_ENABLED); - su_node_ptr = i_su->get_node_ptr(); - if (m_AVD_APP_SU_IS_INSVC(i_su, su_node_ptr)) { + if (i_su->is_in_service() == true) { i_su->set_readiness_state(SA_AMF_READINESS_IN_SERVICE); } } @@ -1381,9 +1376,8 @@ uint32_t avd_sg_app_su_inst_func(AVD_CL_ (su_node_ptr->saAmfNodeOperState == SA_AMF_OPERATIONAL_ENABLED) && (i_su->saAmfSUOperState == SA_AMF_OPERATIONAL_ENABLED) && (i_su->term_state == false)) { - su_node_ptr = i_su->get_node_ptr(); - if (m_AVD_APP_SU_IS_INSVC(i_su, su_node_ptr)) { + if (i_su->is_in_service() == true) { i_su->set_readiness_state(SA_AMF_READINESS_IN_SERVICE);
[devel] [PATCH 0 of 7] Review Request for AMF SU refactoring #713
Summary: AMF SU refactoring Review request for Trac Ticket(s): 713 Peer Reviewer(s): Hans, Praveen, Nags Pull request to: <> Affected branch(es): default Development branch: <> Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts n SAF servicesn OpenSAF servicesn Core libraries n Samples n Tests n Other n Comments (indicate scope for each "y" above): - changeset 5e2b7fc25c625513f60d72f8cb17afec0031fac4 Author: Hans Feldt Date: Wed, 30 Apr 2014 06:29:25 +0200 amfd: add and use SU assignment count methods [#713] changeset 1cecd1a4b444dde4961fc56d671944a66d637686 Author: Hans Feldt Date: Wed, 30 Apr 2014 06:29:26 +0200 amfd: add and use set_su_failover [#713] changeset 4cb397232bcb51f439c1fcad870d38498d8992e1 Author: Hans Feldt Date: Wed, 30 Apr 2014 06:29:26 +0200 amfd: add and use SU method set_term_state [#713] changeset 82163d3aa68df5e9d723cf90eb82ea6c3569dd74 Author: Hans Feldt Date: Wed, 30 Apr 2014 06:29:26 +0200 amfd: add SU constr/destr [#713] changeset 7c7e70b6dcacfd314b08648e9da7a419ea5f863f Author: Hans Feldt Date: Wed, 30 Apr 2014 06:29:27 +0200 amfd: add and use set_su_switch [#713] changeset 6693ca090a9a155933f33b2af105596eb0a2 Author: Hans Feldt Date: Wed, 30 Apr 2014 06:29:28 +0200 amfd: add and use SU get_node_ptr [#713] changeset 47c134295b3137fe5c36f91707cfdb17c3c1884c Author: Hans Feldt Date: Wed, 30 Apr 2014 06:29:29 +0200 amfd: add and use SU is_in_service [#713] Complete diffstat: -- osaf/services/saf/amf/amfd/clm.cc|2 +- osaf/services/saf/amf/amfd/comp.cc |7 ++-- osaf/services/saf/amf/amfd/include/su.h | 57 +++-- osaf/services/saf/amf/amfd/node.cc | 10 ++ osaf/services/saf/amf/amfd/role.cc |2 +- osaf/services/saf/amf/amfd/sg.cc | 12 +++- osaf/services/saf/amf/amfd/sg_2n_fsm.cc | 42 -- osaf/services/saf/amf/amfd/sg_nored_fsm.cc | 16 +- osaf/services/saf/amf/amfd/sg_npm_fsm.cc | 14 +- osaf/services/saf/amf/amfd/sg_nway_fsm.cc| 19 ++--- osaf/services/saf/amf/amfd/sg_nwayact_fsm.cc | 16 +- osaf/services/saf/amf/amfd/sgproc.cc | 24 - osaf/services/saf/amf/amfd/siass.cc | 29 +++- osaf/services/saf/amf/amfd/su.cc | 194 ++-- osaf/services/saf/amf/amfd/sutype.cc |5 +-- osaf/services/saf/amf/amfd/util.cc | 25 ++--- 16 files changed, 227 insertions(+), 247 deletions(-) Testing Commands: - Build and start Testing, Expected Results: -- <> Conditions of Submission: - <> Arch Built StartedLinux distro --- mipsn 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
[devel] [PATCH 2 of 7] amfd: add and use set_su_failover [#713]
osaf/services/saf/amf/amfd/comp.cc | 3 +-- osaf/services/saf/amf/amfd/include/su.h | 6 +- osaf/services/saf/amf/amfd/sg.cc| 3 +-- osaf/services/saf/amf/amfd/su.cc| 30 +- osaf/services/saf/amf/amfd/sutype.cc| 5 + 5 files changed, 25 insertions(+), 22 deletions(-) diff --git a/osaf/services/saf/amf/amfd/comp.cc b/osaf/services/saf/amf/amfd/comp.cc --- a/osaf/services/saf/amf/amfd/comp.cc +++ b/osaf/services/saf/amf/amfd/comp.cc @@ -1560,8 +1560,7 @@ static void comp_ccb_apply_delete_hdlr(s SA_IMM_ATTR_SAUINT32T, &comp->su->saAmfSUPreInstantiable); /* If SU becomes NPI then enable saAmfSUFailover flag Sec 3.11.1.3.2 AMF-B.04.01 spec */ if (!comp->su->saAmfSUPreInstantiable) { - comp->su->saAmfSUFailover = true; - su_nd_attribute_update(comp->su, saAmfSUFailOver_ID); + comp->su->set_su_failover(true); } } 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 @@ -38,6 +38,7 @@ /** * AMF director Service Unit representation. */ +//TODO: all attributes that have a setter should probably have an getter class AVD_SU { public: SaNameT name; @@ -91,6 +92,7 @@ class AVD_SU { struct avd_sutype *su_type; AVD_SU *su_list_su_type_next; + void set_su_failover(bool value); void dec_curr_stdby_si(void); void inc_curr_stdby_si(void); void inc_curr_act_si(void); @@ -108,6 +110,9 @@ class AVD_SU { void delete_all_susis(void); void set_all_susis_assigned_quiesced(void); void set_all_susis_assigned(void); + + private: + void send_attribute_update(AVSV_AMF_SU_ATTR_ID attrib_id); }; extern AmfDb *su_db; @@ -194,6 +199,5 @@ extern SaAisErrorT avd_sutcomptype_confi extern void avd_sutcomptype_constructor(void); extern AVD_SU *avd_su_get_or_create(const SaNameT *dn); -extern void su_nd_attribute_update(const AVD_SU *su, AVSV_AMF_SU_ATTR_ID attrib_id); #endif diff --git a/osaf/services/saf/amf/amfd/sg.cc b/osaf/services/saf/amf/amfd/sg.cc --- a/osaf/services/saf/amf/amfd/sg.cc +++ b/osaf/services/saf/amf/amfd/sg.cc @@ -1564,8 +1564,7 @@ void avd_sg_adjust_config(AVD_SG *sg) /* saAmfSUFailover must be true for a NPI SU sec 3.11.1.3.2 AMF-B.04.01 spec */ for (AVD_SU *su = sg->list_of_su; su != NULL; su = su->sg_list_su_next) { if (!su->saAmfSUFailover) { - su->saAmfSUFailover = true; - su_nd_attribute_update(su, saAmfSUFailOver_ID); + su->set_su_failover(true); } } diff --git a/osaf/services/saf/amf/amfd/su.cc b/osaf/services/saf/amf/amfd/su.cc --- a/osaf/services/saf/amf/amfd/su.cc +++ b/osaf/services/saf/amf/amfd/su.cc @@ -1427,19 +1427,19 @@ static void su_ccb_apply_modify_hdlr(str if (!strcmp(attr_mod->modAttr.attrName, "saAmfSUFailover")) { if (value_is_deleted) { - su->saAmfSUFailover = static_cast(su->su_type->saAmfSutDefSUFailover); + su->set_su_failover(su->su_type->saAmfSutDefSUFailover); su->saAmfSUFailover_configured = false; } else { - su->saAmfSUFailover = static_cast(*((SaUint32T *)attr_mod->modAttr.attrValues[0])); + bool value = + static_cast(*((SaUint32T *)attr_mod->modAttr.attrValues[0])); + su->set_su_failover(value); su->saAmfSUFailover_configured = true; } - TRACE("Modified saAmfSUFailover is '%u'", su->saAmfSUFailover); if (!su->saAmfSUPreInstantiable) { - su->saAmfSUFailover = true; + su->set_su_failover(true); su->saAmfSUFailover_configured = true; } - su_nd_attribute_update(su, saAmfSUFailOver_ID); } else if (!strcmp(attr_mod->modAttr.attrName, "saAmfSUMaintenanceCampaign")) { if (value_is_deleted) { su->saAmfSUMaintenanceCampaign.length = 0; @@ -1460,13 +1460,12 @@ static void su_ccb_apply_modify_hdlr(str su->su_type = sut; avd_sutype_add_su(su); if (su->saAmfSUPreInstantiable) { - su->saAmfSUFailover = static_cast(sut->saAmfSutDefSUFailove
[devel] [PATCH 3 of 7] amfd: add and use SU method set_term_state [#713]
osaf/services/saf/amf/amfd/clm.cc | 2 +- osaf/services/saf/amf/amfd/include/su.h | 6 +- osaf/services/saf/amf/amfd/node.cc | 2 +- osaf/services/saf/amf/amfd/sg.cc| 5 ++--- osaf/services/saf/amf/amfd/su.cc| 16 +++- 5 files changed, 16 insertions(+), 15 deletions(-) diff --git a/osaf/services/saf/amf/amfd/clm.cc b/osaf/services/saf/amf/amfd/clm.cc --- a/osaf/services/saf/amf/amfd/clm.cc +++ b/osaf/services/saf/amf/amfd/clm.cc @@ -59,7 +59,7 @@ static void clm_node_join_complete(AVD_A (sg_instantiated_su_count(su->sg_of_su) + su->sg_of_su->try_inst_counter)) { if (avd_snd_presence_msg(avd_cb, su, false) == NCSCC_RC_SUCCESS) { - m_AVD_SET_SU_TERM(avd_cb, su, false); + su->set_term_state(false); su->sg_of_su->try_inst_counter++; } else { LOG_ER("Internal error, could not send message to avnd"); 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 @@ -110,6 +110,7 @@ class AVD_SU { void delete_all_susis(void); void set_all_susis_assigned_quiesced(void); void set_all_susis_assigned(void); + void set_term_state(bool state); private: void send_attribute_update(AVSV_AMF_SU_ATTR_ID attrib_id); @@ -127,11 +128,6 @@ typedef struct { } AVD_SUTCOMP_TYPE; -#define m_AVD_SET_SU_TERM(cb,su,state) {\ -su->term_state = state;\ -m_AVSV_SEND_CKPT_UPDT_ASYNC_UPDT(cb, su, AVSV_CKPT_SU_TERM_STATE);\ -} - #define m_AVD_SET_SU_SWITCH(cb,su,state) {\ su->su_switch = state;\ m_AVSV_SEND_CKPT_UPDT_ASYNC_UPDT(cb, su, AVSV_CKPT_SU_SWITCH);\ diff --git a/osaf/services/saf/amf/amfd/node.cc b/osaf/services/saf/amf/amfd/node.cc --- a/osaf/services/saf/amf/amfd/node.cc +++ b/osaf/services/saf/amf/amfd/node.cc @@ -1055,7 +1055,7 @@ static void node_sus_termstate_set(AVD_A for (su = node->list_of_su; su; su = su->avnd_list_su_next) { if (su->saAmfSUPreInstantiable == true) - m_AVD_SET_SU_TERM(avd_cb, su, term_state); + su->set_term_state(term_state); } } diff --git a/osaf/services/saf/amf/amfd/sg.cc b/osaf/services/saf/amf/amfd/sg.cc --- a/osaf/services/saf/amf/amfd/sg.cc +++ b/osaf/services/saf/amf/amfd/sg.cc @@ -1037,7 +1037,7 @@ static uint32_t avd_sg_su_term_in_revers (su->saAmfSUPresenceState != SA_AMF_PRESENCE_TERMINATION_FAILED)) { if (avd_snd_presence_msg(avd_cb, su, true) == NCSCC_RC_SUCCESS) { - m_AVD_SET_SU_TERM(avd_cb, su, true); + su->set_term_state(true); } else { rc = NCSCC_RC_FAILURE; LOG_WA("Failed Termination '%s'", su->name.value); @@ -1098,8 +1098,7 @@ static void sg_app_sg_admin_unlock_inst( } /* set uncondionally of msg snd outcome */ - m_AVD_SET_SU_TERM(cb, su, false); - + su->set_term_state(false); } } } diff --git a/osaf/services/saf/amf/amfd/su.cc b/osaf/services/saf/amf/amfd/su.cc --- a/osaf/services/saf/amf/amfd/su.cc +++ b/osaf/services/saf/amf/amfd/su.cc @@ -1054,7 +1054,7 @@ static void su_admin_op_cb(SaImmOiHandle /* No need to terminate the SUs in Unins/Inst Failed/Term Failed state */ su->set_admin_state(SA_AMF_ADMIN_LOCKED_INSTANTIATION); avd_saImmOiAdminOperationResult(immoi_handle, invocation, SA_AIS_OK); - m_AVD_SET_SU_TERM(cb, su, true); + su->set_term_state(true); LOG_NO("'%s' presence state is '%u'", su_name->value, su->saAmfSUPresenceState); goto done; } @@ -1065,7 +1065,7 @@ static void su_admin_op_cb(SaImmOiHandle /* When the SU will terminate then prescence state change message will come and so store the callback parameters to send response later on. */ if (avd_snd_presence_msg(cb, su, true) == NCSCC_RC_SUCCESS) { - m_AVD_SET_SU_TERM(cb, su, true); + su->set_term_state(true); su->set_admin_state(SA_AMF_ADMIN_LOCKED_INSTANTIATION); su->pend_cbk.admin_oper = static_cast(op_id);
[devel] [PATCH 4 of 7] amfd: add SU constr/destr [#713]
osaf/services/saf/amf/amfd/include/su.h | 17 +-- osaf/services/saf/amf/amfd/su.cc| 74 +++- 2 files changed, 37 insertions(+), 54 deletions(-) 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 @@ -92,6 +92,9 @@ class AVD_SU { struct avd_sutype *su_type; AVD_SU *su_list_su_type_next; + AVD_SU() {}; + AVD_SU(const SaNameT *dn); + ~AVD_SU(); void set_su_failover(bool value); void dec_curr_stdby_si(void); void inc_curr_stdby_si(void); @@ -146,20 +149,6 @@ m_AVSV_SEND_CKPT_UPDT_ASYNC_UPDT(cb, su, else su_node_ptr = i_su->su_on_node; /** - * Allocate SU memory and initialize attributes to defaults - * @param dn - * - * @return AVD_SU* - */ -extern AVD_SU *avd_su_new(const SaNameT *dn); - -/** - * Free SU memory - * @param su - */ -extern void avd_su_delete(AVD_SU *su); - -/** * Get SUs from IMM and create internal objects * * @return SaAisErrorT diff --git a/osaf/services/saf/amf/amfd/su.cc b/osaf/services/saf/amf/amfd/su.cc --- a/osaf/services/saf/amf/amfd/su.cc +++ b/osaf/services/saf/amf/amfd/su.cc @@ -31,51 +31,46 @@ AmfDb *su_db = NULL; -AVD_SU *avd_su_new(const SaNameT *dn) -{ - SaNameT sg_name; - AVD_SU *su; - - su = new AVD_SU(); - - memcpy(su->name.value, dn->value, dn->length); - su->name.length = dn->length; - avsv_sanamet_init(dn, &sg_name, "safSg"); - su->saAmfSUFailover = false; - su->term_state = false; - su->su_switch = AVSV_SI_TOGGLE_STABLE; - su->saAmfSUPreInstantiable = static_cast(false); - /* saAmfSUOperState is set when the SU is added to model depending on -* node state. Initialized to invalid due to filtering in avd_su_oper_state_set. */ - su->saAmfSUOperState = static_cast(0); - su->saAmfSUPresenceState = SA_AMF_PRESENCE_UNINSTANTIATED; - su->saAmfSuReadinessState = SA_AMF_READINESS_OUT_OF_SERVICE; - su->su_is_external = false; - - return su; +AVD_SU::AVD_SU(const SaNameT *dn) { + memcpy(name.value, dn->value, sizeof(name.value)); + name.length = dn->length; + saAmfSUFailover = false; + term_state = false; + su_switch = AVSV_SI_TOGGLE_STABLE; + saAmfSUPreInstantiable = static_cast(false); + saAmfSUOperState = SA_AMF_OPERATIONAL_DISABLED; + saAmfSUPresenceState = SA_AMF_PRESENCE_UNINSTANTIATED; + saAmfSuReadinessState = SA_AMF_READINESS_OUT_OF_SERVICE; + su_is_external = false; + sg_of_su = NULL; + su_on_node = NULL; + list_of_susi = NULL; + list_of_comp = NULL; + sg_list_su_next = NULL; + avnd_list_su_next = NULL; + su_type = NULL; + su_list_su_type_next = NULL; + saAmfSUHostedByNode.length = 0; } /** * Delete the SU from the model. Check point with peer. Send delete order * to node director. Delete all contained components. - * - * @param i_su */ -void avd_su_delete(AVD_SU *su) -{ - TRACE_ENTER2("'%s'", su->name.value); +AVD_SU::~AVD_SU() { + TRACE_ENTER2("'%s'", name.value); /* All the components under this SU should have been deleted * by now, just do the sanity check to confirm it is done */ - osafassert(su->list_of_comp == NULL); + osafassert(list_of_comp == NULL); + osafassert(list_of_susi == NULL); - m_AVSV_SEND_CKPT_UPDT_ASYNC_RMV(avd_cb, su, AVSV_CKPT_AVD_SU_CONFIG); - avd_node_remove_su(su); - avd_sutype_remove_su(su); - su_db->erase(su); - avd_sg_remove_su(su); - delete su; + m_AVSV_SEND_CKPT_UPDT_ASYNC_RMV(avd_cb, this, AVSV_CKPT_AVD_SU_CONFIG); + avd_node_remove_su(this); + avd_sutype_remove_su(this); + su_db->erase(this); + avd_sg_remove_su(this); TRACE_LEAVE(); } @@ -93,8 +88,7 @@ AVD_SU *avd_su_get_or_create(const SaNam if (su == NULL) { TRACE("'%s' does not exist, creating it", dn->value); - su = avd_su_new(dn); - osafassert(su != NULL); + su = new AVD_SU(dn); unsigned int rc = su_db->insert(su); osafassert(rc == NCSCC_RC_SUCCESS); } @@ -420,8 +414,7 @@ static AVD_SU *su_create(const SaNameT * ** but needs to get configuration attributes initialized. */ if ((su = su_db->find(dn)) == NULL) { - if ((su = avd_su_new(dn)) == NULL) - goto done; + su = new AVD_SU(dn); } else TRACE("already created, refreshing config..."); @@ -1489,7 +1482,7 @@ static void su_ccb_apply_delete_hdlr(str TRACE_ENTER2("'%s'", su->name.value); if (avd_cb->avail_state_avd != SA_AMF_HA_ACTIVE) { - avd_su_delete(su); + de
[devel] [PATCH 6 of 7] amfd: add and use SU get_node_ptr [#713]
osaf/services/saf/amf/amfd/comp.cc | 4 ++-- osaf/services/saf/amf/amfd/include/su.h | 5 + osaf/services/saf/amf/amfd/node.cc | 6 +++--- osaf/services/saf/amf/amfd/sg.cc | 4 ++-- osaf/services/saf/amf/amfd/sg_2n_fsm.cc | 12 ++-- osaf/services/saf/amf/amfd/sg_nored_fsm.cc | 16 osaf/services/saf/amf/amfd/sg_npm_fsm.cc | 14 +++--- osaf/services/saf/amf/amfd/sg_nway_fsm.cc| 19 +-- osaf/services/saf/amf/amfd/sg_nwayact_fsm.cc | 16 osaf/services/saf/amf/amfd/sgproc.cc | 16 ++-- osaf/services/saf/amf/amfd/su.cc | 14 +++--- osaf/services/saf/amf/amfd/util.cc | 25 + 12 files changed, 72 insertions(+), 79 deletions(-) diff --git a/osaf/services/saf/amf/amfd/comp.cc b/osaf/services/saf/amf/amfd/comp.cc --- a/osaf/services/saf/amf/amfd/comp.cc +++ b/osaf/services/saf/amf/amfd/comp.cc @@ -1126,7 +1126,7 @@ static void comp_ccb_apply_modify_hdlr(s param.name = comp->comp_info.name; comp_type = avd_comptype_get(&comp->saAmfCompType); - m_AVD_GET_SU_NODE_PTR(avd_cb, comp->su, su_node_ptr); + su_node_ptr = comp->su->get_node_ptr(); if ((su_node_ptr->node_state == AVD_AVND_STATE_PRESENT) || (su_node_ptr->node_state == AVD_AVND_STATE_NO_CONFIG) || @@ -1567,7 +1567,7 @@ static void comp_ccb_apply_delete_hdlr(s /* send a message to the AVND deleting the * component. */ - m_AVD_GET_SU_NODE_PTR(avd_cb, comp->su, su_node_ptr); + su_node_ptr = comp->su->get_node_ptr(); if ((su_node_ptr->node_state == AVD_AVND_STATE_PRESENT) || (su_node_ptr->node_state == AVD_AVND_STATE_NO_CONFIG) || (su_node_ptr->node_state == AVD_AVND_STATE_NCS_INIT)) { 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 @@ -115,6 +115,7 @@ class AVD_SU { void set_all_susis_assigned(void); void set_term_state(bool state); void set_su_switch(SaToggleState state); + struct avd_avnd_tag *get_node_ptr(void); private: void send_attribute_update(AVSV_AMF_SU_ATTR_ID attrib_id); @@ -140,10 +141,6 @@ typedef struct { (i_su->saAmfSUOperState == SA_AMF_OPERATIONAL_ENABLED)\ ) -#define m_AVD_GET_SU_NODE_PTR(avd_cb,i_su,su_node_ptr) \ - if(true == i_su->su_is_external) su_node_ptr = avd_cb->ext_comp_info.local_avnd_node; \ - else su_node_ptr = i_su->su_on_node; - /** * Get SUs from IMM and create internal objects * diff --git a/osaf/services/saf/amf/amfd/node.cc b/osaf/services/saf/amf/amfd/node.cc --- a/osaf/services/saf/amf/amfd/node.cc +++ b/osaf/services/saf/amf/amfd/node.cc @@ -877,7 +877,7 @@ void avd_node_admin_lock_unlock_shutdown su = node->list_of_su; while (su != NULL) { - m_AVD_GET_SU_NODE_PTR(cb, su, su_node_ptr); + su_node_ptr = su->get_node_ptr(); if (m_AVD_APP_SU_IS_INSVC(su, su_node_ptr) && ((su->saAmfSUPreInstantiable) ? @@ -920,8 +920,8 @@ void avd_node_admin_lock_unlock_shutdown */ su_sg = su->sg_of_su->list_of_su; while (su_sg != NULL) { - m_AVD_GET_SU_NODE_PTR(cb, su, su_node_ptr); - m_AVD_GET_SU_NODE_PTR(cb, su_sg, su_sg_node_ptr); + su_node_ptr = su->get_node_ptr(); + su_sg_node_ptr = su_sg->get_node_ptr(); if ((su != su_sg) && (su_node_ptr == su_sg_node_ptr) && diff --git a/osaf/services/saf/amf/amfd/sg.cc b/osaf/services/saf/amf/amfd/sg.cc --- a/osaf/services/saf/amf/amfd/sg.cc +++ b/osaf/services/saf/amf/amfd/sg.cc @@ -752,7 +752,7 @@ static void sg_nd_attribute_update(AVD_S /* This value has to be updated on each SU on this SG */ su = sg->list_of_su; while (su) { - m_AVD_GET_SU_NODE_PTR(avd_cb, su, su_node_ptr); + su_node_ptr = su->get_node_ptr(); if ((su_node_ptr) && (su_node_ptr->node_state == AVD_AVND_STATE_PRESENT)) { param.name = su->name; @@ -1152,7 +1152,7 @@ static void sg_admin_op_cb(SaImmOiHandle /* Avoid multiple admin operations on other SUs belonging to the same SG. */ for (su = sg->list_of_su; su != NULL; su = su->sg_list_su_next) { - m_AVD_GET_SU_NODE_PTR(avd_cb, su, node); + node = su->get_node_ptr(); if (su->pend_cbk.invocation != 0) { report_admin_op_error(im
[devel] [PATCH 5 of 7] amfd: add and use set_su_switch [#713]
osaf/services/saf/amf/amfd/include/su.h | 6 +- osaf/services/saf/amf/amfd/role.cc | 2 +- osaf/services/saf/amf/amfd/sg_2n_fsm.cc | 30 -- osaf/services/saf/amf/amfd/su.cc| 6 ++ 4 files changed, 24 insertions(+), 20 deletions(-) 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 @@ -114,6 +114,7 @@ class AVD_SU { void set_all_susis_assigned_quiesced(void); void set_all_susis_assigned(void); void set_term_state(bool state); + void set_su_switch(SaToggleState state); private: void send_attribute_update(AVSV_AMF_SU_ATTR_ID attrib_id); @@ -131,11 +132,6 @@ typedef struct { } AVD_SUTCOMP_TYPE; -#define m_AVD_SET_SU_SWITCH(cb,su,state) {\ -su->su_switch = state;\ -m_AVSV_SEND_CKPT_UPDT_ASYNC_UPDT(cb, su, AVSV_CKPT_SU_SWITCH);\ -} - #define m_AVD_APP_SU_IS_INSVC(i_su,su_node_ptr) \ ((su_node_ptr->saAmfNodeAdminState == SA_AMF_ADMIN_UNLOCKED) && \ (su_node_ptr->saAmfNodeOperState == SA_AMF_OPERATIONAL_ENABLED) && \ diff --git a/osaf/services/saf/amf/amfd/role.cc b/osaf/services/saf/amf/amfd/role.cc --- a/osaf/services/saf/amf/amfd/role.cc +++ b/osaf/services/saf/amf/amfd/role.cc @@ -536,7 +536,7 @@ void avd_role_switch_ncs_su_evh(AVD_CL_C avd_sg_su_si_mod_snd(cb, i_su, SA_AMF_HA_ACTIVE); } else { avd_sg_su_oper_list_add(cb, i_su, false); - m_AVD_SET_SU_SWITCH(cb, i_su, AVSV_SI_TOGGLE_SWITCH); + i_su->set_su_switch(AVSV_SI_TOGGLE_SWITCH); m_AVD_SET_SG_FSM(cb, (i_su->sg_of_su), AVD_SG_FSM_SU_OPER); } } diff --git a/osaf/services/saf/amf/amfd/sg_2n_fsm.cc b/osaf/services/saf/amf/amfd/sg_2n_fsm.cc --- a/osaf/services/saf/amf/amfd/sg_2n_fsm.cc +++ b/osaf/services/saf/amf/amfd/sg_2n_fsm.cc @@ -866,7 +866,7 @@ SaAisErrorT avd_sg_2n_siswap_func(AVD_SI /* Add the SU to the operation list and change the SG state to SU_operation. */ avd_sg_su_oper_list_add(avd_cb, susi->su, false); m_AVD_SET_SG_FSM(avd_cb, susi->su->sg_of_su, AVD_SG_FSM_SU_OPER); - m_AVD_SET_SU_SWITCH(avd_cb, susi->su, AVSV_SI_TOGGLE_SWITCH); + susi->su->set_su_switch(AVSV_SI_TOGGLE_SWITCH); si->invocation = invocation; LOG_NO("%s Swap initiated", susi->si->name.value); @@ -917,7 +917,7 @@ static uint32_t avd_sg_2n_su_fault_su_op if (su->sg_of_su->su_oper_list.su == su) { su_ha_state = avd_su_state_determine(su); if (su_ha_state == SA_AMF_HA_QUIESCED) { - m_AVD_SET_SU_SWITCH(cb, su, AVSV_SI_TOGGLE_STABLE); + su->set_su_switch(AVSV_SI_TOGGLE_STABLE); } else if (su_ha_state == SA_AMF_HA_QUIESCING) { if (avd_sidep_si_dependency_exists_within_su(su)) { if (avd_sg_susi_mod_snd_honouring_si_dependency(su, SA_AMF_HA_QUIESCED) == NCSCC_RC_FAILURE) { @@ -999,7 +999,7 @@ static uint32_t avd_sg_2n_su_fault_su_op avd_su_role_failover(su, a_su); } - m_AVD_SET_SU_SWITCH(cb, a_su, AVSV_SI_TOGGLE_STABLE); + a_su->set_su_switch(AVSV_SI_TOGGLE_STABLE); } } /* if(su_ha_state == SA_AMF_HA_STANDBY) */ @@ -1039,8 +1039,8 @@ static uint32_t avd_sg_2n_su_fault_su_op if ((a_su->su_switch == AVSV_SI_TOGGLE_SWITCH) && (a_su->saAmfSuReadinessState == SA_AMF_READINESS_IN_SERVICE) && - (a_su_ha_state == SA_AMF_HA_QUIESCED)) { - m_AVD_SET_SU_SWITCH(cb, a_su, AVSV_SI_TOGGLE_STABLE); + (a_su_ha_state == SA_AMF_HA_QUIESCED)) { + a_su->set_su_switch(AVSV_SI_TOGGLE_STABLE); } else if (a_su_ha_state == SA_AMF_HA_QUIESCED) { /* the other SU has quiesced assignments meaning either it is * out of service or locked. So just send a remove request @@ -2127,8 +2127,9 @@ static uint32_t avd_sg_2n_susi_sucss_su_ } /* Finish the SI SWAP admin operation */ - m_AVD_SET_SU_SWITCH(cb, su->sg_of_su->su_oper_list.su, AVSV_SI_TOGGLE_STABLE); - avd_sg_su_oper_list_del(cb, su->sg_of_su->su_oper_list.su, false); + AVD_SU *su_at_head = su->sg_of_su->su_oper_list.su; + su_at_head->set_su_switch(AVSV_SI_TOGGLE_STABLE); + avd_sg_su_oper_list_del(cb, su_at_head, false); m_AVD_SET_SG_FSM(cb, su->
[devel] [PATCH 1 of 7] amfd: add and use SU assignment count methods [#713]
osaf/services/saf/amf/amfd/include/su.h | 8 +++--- osaf/services/saf/amf/amfd/siass.cc | 29 - osaf/services/saf/amf/amfd/su.cc| 44 3 files changed, 42 insertions(+), 39 deletions(-) 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 @@ -91,6 +91,10 @@ class AVD_SU { struct avd_sutype *su_type; AVD_SU *su_list_su_type_next; + void dec_curr_stdby_si(void); + void inc_curr_stdby_si(void); + void inc_curr_act_si(void); + void dec_curr_act_si(void); int hastate_assignments_count(SaAmfHAStateT ha_state); void add_comp(struct avd_comp_tag *comp); void remove_comp(struct avd_comp_tag *comp); @@ -189,10 +193,6 @@ extern SaAisErrorT avd_sutcomptype_confi */ extern void avd_sutcomptype_constructor(void); -extern void avd_su_inc_curr_act_si(AVD_SU *su); -extern void avd_su_dec_curr_act_si(AVD_SU *su); -extern void avd_su_inc_curr_stdby_si(AVD_SU *su); -extern void avd_su_dec_curr_stdby_si(AVD_SU *su); extern AVD_SU *avd_su_get_or_create(const SaNameT *dn); extern void su_nd_attribute_update(const AVD_SU *su, AVSV_AMF_SU_ATTR_ID attrib_id); diff --git a/osaf/services/saf/amf/amfd/siass.cc b/osaf/services/saf/amf/amfd/siass.cc --- a/osaf/services/saf/amf/amfd/siass.cc +++ b/osaf/services/saf/amf/amfd/siass.cc @@ -350,6 +350,7 @@ AVD_SU_SI_REL *avd_susi_find(AVD_CL_CB * uint32_t avd_susi_delete(AVD_CL_CB *cb, AVD_SU_SI_REL *susi, bool ckpt) { AVD_SU_SI_REL *p_su_si, *p_si_su, *i_su_si; + AVD_SU *su = susi->su; TRACE_ENTER2("%s %s", susi->su->name.value, susi->si->name.value); @@ -406,10 +407,10 @@ uint32_t avd_susi_delete(AVD_CL_CB *cb, only happens when active -> qsd and standby rebooted */ } else if ((susi->fsm == AVD_SU_SI_STATE_ASGND) || (susi->fsm == AVD_SU_SI_STATE_ASGN)){ if (SA_AMF_HA_STANDBY == susi->state) { - avd_su_dec_curr_stdby_si(susi->su); + su->dec_curr_stdby_si(); avd_si_dec_curr_stdby_ass(susi->si); } else if ((SA_AMF_HA_ACTIVE == susi->state) || (SA_AMF_HA_QUIESCING == susi->state)) { - avd_su_dec_curr_act_si(susi->su); + su->dec_curr_act_si(); avd_si_dec_curr_act_ass(susi->si); } } @@ -596,52 +597,54 @@ done: */ void avd_susi_update_assignment_counters(AVD_SU_SI_REL *susi, AVSV_SUSI_ACT action, SaAmfHAStateT current_ha_state, SaAmfHAStateT new_ha_state) { + AVD_SU *su = susi->su; + TRACE_ENTER2("SI:'%s', SU:'%s' action:%u current_ha_state:%u new_ha_state:%u", susi->si->name.value, susi->su->name.value, action, current_ha_state, new_ha_state); switch (action) { case AVSV_SUSI_ACT_ASGN: if (new_ha_state == SA_AMF_HA_ACTIVE) { - avd_su_inc_curr_act_si(susi->su); + su->inc_curr_act_si(); avd_si_inc_curr_act_ass(susi->si); } else if (new_ha_state == SA_AMF_HA_STANDBY) { - avd_su_inc_curr_stdby_si(susi->su); + su->inc_curr_stdby_si(); avd_si_inc_curr_stdby_ass(susi->si); } break; case AVSV_SUSI_ACT_MOD: if ((current_ha_state == SA_AMF_HA_STANDBY) && (new_ha_state == SA_AMF_HA_ACTIVE)) { /* standby to active */ - avd_su_inc_curr_act_si(susi->su); - avd_su_dec_curr_stdby_si(susi->su); + su->inc_curr_act_si(); + su->dec_curr_stdby_si(); avd_si_inc_curr_act_dec_std_ass(susi->si); } else if (((current_ha_state == SA_AMF_HA_ACTIVE) || (current_ha_state == SA_AMF_HA_QUIESCING)) && (new_ha_state == SA_AMF_HA_QUIESCED)) { /* active or quiescing to quiesced */ - avd_su_dec_curr_act_si(susi->su); + su->dec_curr_act_si(); avd_si_dec_curr_act_ass(susi->si); } else if ((current_ha_state == SA_AMF_HA_QUIESCED) && (new_ha_state == SA_AMF_HA_STANDBY)) { /* active or quiescinf to standby */ - avd_su_inc_curr_stdby_si(susi->su); + su->inc_curr_stdby_si(); avd_si_inc_curr_stdby_ass(susi->si); } else if (((current_ha_state == SA_AMF_HA_ACTIVE) || (current_ha_state == SA_AMF_HA_QUIESCING)) && (new_h
Re: [devel] [PATCH 1 of 1] AMF: support immediate effect when changing comp/hc-type attributes V1 [#819]
Comments: - the commit message says comp but its all about hctype, remve comp I guess? - strange indentation in new code, neither linux or google: - space between if and (, opening brace on same line etc See inline Thanks, Hans On 10 April 2014 13:25, Hans Nordeback wrote: > osaf/libs/common/amf/include/amf_defs.h |7 + > osaf/services/saf/amf/amfd/hlttype.cc | 70 +++- > osaf/services/saf/amf/amfnd/di.cc |3 + > osaf/services/saf/amf/amfnd/hcdb.cc | 161 > ++ > osaf/services/saf/amf/amfnd/include/avnd_hc.h |1 + > 5 files changed, 241 insertions(+), 1 deletions(-) > > > diff --git a/osaf/libs/common/amf/include/amf_defs.h > b/osaf/libs/common/amf/include/amf_defs.h > --- a/osaf/libs/common/amf/include/amf_defs.h > +++ b/osaf/libs/common/amf/include/amf_defs.h > @@ -254,6 +254,13 @@ typedef enum > saAmfHealthcheckMaxDuration_ID = 2, > } AVSV_AMF_HEALTHCHECK_ATTR_ID; > > +/* Attribute ID enum for the SaAmfHealthcheckType class */ > +typedef enum > +{ > + saAmfHctDefPeriod_ID = 1, > + saAmfHctDefMaxDuration_ID = 2, > +} AVSV_AMF_HEALTHCHECK_TYPE_ATTR_ID; > + > > #define AVSV_COMMON_SUB_ID_DEFAULT_VAL 1 > #define SA_AMF_PRESENCE_ORPHANED (SA_AMF_PRESENCE_TERMINATION_FAILED+1) > diff --git a/osaf/services/saf/amf/amfd/hlttype.cc > b/osaf/services/saf/amf/amfd/hlttype.cc > --- a/osaf/services/saf/amf/amfd/hlttype.cc > +++ b/osaf/services/saf/amf/amfd/hlttype.cc > @@ -15,12 +15,15 @@ > * > */ > > +#include > +#include > #include > > #include > #include > #include > #include > +#include "comp.h" > > /** > * Validates proposed change in comptype > @@ -56,6 +59,71 @@ done: > return rc; > } > > +static void ccb_apply_modify_hdlr(CcbUtilOperationData_t *opdata) opdata should be const > +{ > + const SaImmAttrModificationT_2 *attr_mod; > + int i; > + const AVD_COMP_TYPE *comp_type; > + SaNameT comp_type_dn; > + char *comp_type_name; > + > + TRACE_ENTER2("CCB ID %llu, '%s'", opdata->ccbId, > opdata->objectName.value); > + > + // input example: opdata.objectName.value, > safHealthcheckKey=AmfDemo,safVersion=1,safCompType=AmfDemo1 > + comp_type_name = strstr((char *)opdata->objectName.value, > "safVersion"); you should use avsv_sanamet_init() instead, it does the same thing. > + osafassert(comp_type_name); > + > + comp_type_dn.length = > + snprintf((char *)comp_type_dn.value, SA_MAX_NAME_LENGTH, > "%s", comp_type_name); > + > + if ((comp_type = avd_comptype_get(&comp_type_dn)) == NULL) { > + LOG_ER("Internal error: %s not found", comp_type_dn.value); > + return; > + } > + > + // Create a map of nodes where components "may" be using the given > SaAmfHealthcheckType. I don't understand why you use a map, feels like you get more code than needed. Try use a vector instead then the "name thing" will go away which anyway seems unused. > + // A msg will be sent to the related node regarding this change. If a > component has an > + // SaAmfHealthcheck record that overrides this SaAmfHealthcheckType > it will be handled by the amfnd. > + std::map node_map; > + AVD_COMP *comp = comp_type->list_of_comp; > + while (comp != NULL) { > + std::string name((const > char*)comp->su->su_on_node->name.value, comp->su->su_on_node->name.length); > + node_map.insert(std::make_pair(name, comp->su->su_on_node)); > + TRACE("comp name %s on node %s", comp->comp_info.name.value, > comp->su->su_on_node->name.value); > + comp = comp->comp_type_list_comp_next; > + } > + > + std::map::iterator it; > + for (it = node_map.begin(); it != node_map.end(); ++it) { > + i = 0; > + while ((attr_mod = opdata->param.modify.attrMods[i++]) != > NULL) { > + AVSV_PARAM_INFO param; > + const SaImmAttrValuesT_2 *attribute = > &attr_mod->modAttr; > + SaTimeT *param_val = (SaTimeT > *)attribute->attrValues[0]; > + > + memset(¶m, 0, sizeof(param)); > + param.class_id = AVSV_SA_AMF_HEALTH_CHECK_TYPE; > + param.act = AVSV_OBJ_OPR_MOD; > + param.name = opdata->objectName; > + param.value_len = sizeof(*param_val); > + memcpy(param.value, param_val, param.value_len); > + > + if (!strcmp(attribute->attrName, > "saAmfHctDefPeriod")) { > + TRACE("saAmfHctDefPeriod modified to '%llu' > for CompType '%s' on node '%s'", *param_val, > + opdata->objectName.value, > it->second->name.value); > + param.attr_id = saAmfHctDefPeriod_ID; > + } else if (!strcmp(a