Re: [devel] [PATCH 1/1] mbc: fix mbcsv loop forever while it is being dispatch ALL [#2899]
Hi Canh,
ack, review only. One minor comment, instead of adding a similar function
mbcsv_check_if_peer_node_id_exist
reuse
mbcsv_search_and_return_peer
and add a new argument to this function (upper/lower/all part of
archword) instead. It is not that many place that
needs to be updated and code redundancy is avoided.
/Thanks HansN
On 07/30/2018 06:16 AM, Canh Van Truong wrote:
When processing "MBCSV_PEER_UP_MSG" msg in case dispatch all from user,
the msg may be too old. The msg may be come from the node that already rebooted.
This reason cause mbcsv loop forever in WHILE loop, because the active node
cannot
find the adest of peer msg.
The solution is that find entity in peer list and compare if the peer node id
that
already exist. If the peer node id exist in the entity (get node id from peer
anchor),
this mean that mbcsv already processed the "MBCSV_PEER_UP_MSG" for that peer
node.
---
src/mbc/mbcsv_peer.c | 41 -
1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/src/mbc/mbcsv_peer.c b/src/mbc/mbcsv_peer.c
index f863591c4..736c015c2 100644
--- a/src/mbc/mbcsv_peer.c
+++ b/src/mbc/mbcsv_peer.c
@@ -64,7 +64,7 @@ static const char *disc_trace[] = {"Peer UP msg", "Peer DOWN
msg",
* peer entity and returns peer pointer.
*
* Input:peer_list - MBCSv peer list.
-* anchor - Anchor value of the peer to be serched in the list.
+* anchor - Anchor value of the peer to be searched in the list.
*
* Returns: Peer instance pointer.
*
@@ -83,6 +83,36 @@ PEER_INST *mbcsv_search_and_return_peer(PEER_INST *peer_list,
return NULL;
}
+/**\
+* PROCEDURE: mbcsv_check_if_peer_node_id_exist
+*
+* Purpose: This function searches entire MBCA peer list for the peer entity
+* and checking if the peer nodeId is exist in peer entity
+*
+* Input:peer_list - MBCSv peer list.
+* anchor - Anchor value of the peer to be searched in the list.
+*
+* Returns: True/False
+*
+* Notes:
+*
+\**/
+bool mbcsv_check_if_peer_node_id_exist(PEER_INST *peer_list,
+ MBCSV_ANCHOR anchor)
+{
+ PEER_INST *peer;
+ uint32_t node_id;
+ uint32_t peer_node_id = anchor >> 32;
+
+ for (peer = peer_list; peer != NULL; peer = peer->next) {
+ node_id = peer->peer_anchor >> 32;
+ if (node_id == peer_node_id)
+ return true;
+ }
+
+ return false;
+}
+
/**\
* PROCEDURE: mbcsv_add_new_peer
*
@@ -733,6 +763,15 @@ uint32_t mbcsv_process_peer_up_info(MBCSV_EVT *msg,
CKPT_INST *ckpt,
}
}
+ // If the node id exist in any peer entity of the peer list, we already
+ // got peer up message for that peer node. So This peer up message
+ // is invalid and just ignore it. Fixed bug #2899
+ if (mbcsv_check_if_peer_node_id_exist(
+ ckpt->peer_list, msg->rcvr_peer_key.peer_anchor) == true) {
+ TRACE_LEAVE2("Peer up message is too old. Just ignore this");
+ return NCSCC_RC_SUCCESS;
+ }
+
if (0 != (mbx = mbcsv_get_mbx(msg->rcvr_peer_key.pwe_hdl,
msg->rcvr_peer_key.svc_id))) {
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1/1] amf: remove assignment for NPI component with enable DisableRestart [#2879]
Hi Hans,
I updated comment in V2.
B.R
/Thang
-Original Message-
From: Hans Nordeback
Sent: Tuesday, August 7, 2018 5:04 PM
To: Thang Nguyen ; gary@dektech.com.au;
minh.c...@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] amf: remove assignment for NPI component with enable
DisableRestart [#2879]
Hi Thang,
yes, you copied it from above, it looks as it is misspelled, thus -> this at
both comments?/Thanks HansN
On 08/07/2018 10:49 AM, Thang Nguyen wrote:
> Hi Hans,
>
> Thanks for your review.
> But that comment has already existed.
>
> B.R
> /Thang
>
> -Original Message-
> From: Hans Nordeback
> Sent: Tuesday, August 7, 2018 2:55 PM
> To: thang.nguyen ; gary@dektech.com.au;
> minh.c...@dektech.com.au
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1/1] amf: remove assignment for NPI component with enable
> DisableRestart [#2879]
>
> Hi Thang,
>
> ack, code review only. Very minor comment below. /Thanks HansN
>
>
> On 06/27/2018 07:48 PM, thang.nguyen wrote:
>> With NPI component configured with saAmfCtDefDisableRestart=1. Once
>> invoking restart admin op, amfnd does not remove the assignment and cause
>> the crash.
>>
>> Remove assignment before change the pres state to TERMINATION in clc.
>> ---
>>src/amf/amfnd/clc.cc | 9 +
>>1 file changed, 9 insertions(+)
>>
>> diff --git a/src/amf/amfnd/clc.cc b/src/amf/amfnd/clc.cc index
>> c8e60e6..df8356a 100644
>> --- a/src/amf/amfnd/clc.cc
>> +++ b/src/amf/amfnd/clc.cc
>> @@ -2217,6 +2217,15 @@ uint32_t avnd_comp_clc_inst_restart_hdler(AVND_CB
>> *cb, AVND_COMP *comp) {
>>/* invoke terminate callback */
>>rc = avnd_comp_cbk_send(cb, comp, AVSV_AMF_COMP_TERM, 0, 0);
>>else {
>> + if (m_AVND_COMP_IS_RESTART_DIS(comp) && (comp->csi_list.n_nodes > 0))
>> {
>> +/* A NPI component with DisableRestart=1. Restart admin op on su or
>> this
>> +comp itself, first perform reassignment for this component to other
>> SU
>> +then term it. At present assignment of whole SU will be gracefully
>> +reassigned instead of only this comp.
>> +*/
> [HansN] (and also the same comment in the if stmt before this else)
>
> /*Send amfd to gracefully remove assignments for this SU.*/
>
>> +/*Send amfd to gracefully remove assignments for thus SU.*/
>> +su_send_suRestart_recovery_msg(comp->su);
>> + }
>> rc =
>> avnd_comp_clc_cmd_execute(cb, comp,
>> AVND_COMP_CLC_CMD_TYPE_TERMINATE);
>> m_AVND_COMP_REG_PARAM_RESET(cb, comp);
>
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1/1] amf: remove assignment for NPI component with enable DisableRestart [#2879]
Hi Thang,
Ack, review only. I think you should keep V1, with the comments, my only
suggestion was to correct the misspelled "thus SU" to "this SU". /Thanks HansN
-Original Message-
From: thang.nguyen
Sent: den 8 augusti 2018 08:49
To: Hans Nordebäck ; Gary Lee
; Minh Hon Chau
Cc: opensaf-devel@lists.sourceforge.net; Thang Duc Nguyen
Subject: [PATCH 1/1] amf: remove assignment for NPI component with enable
DisableRestart [#2879]
With NPI component configured with saAmfCtDefDisableRestart=1. Once invoking
restart admin op, amfnd does not remove the assignment and cause the crash.
Remove assignment before change the pres state to TERMINATION in clc.
---
src/amf/amfnd/clc.cc | 4
1 file changed, 4 insertions(+)
diff --git a/src/amf/amfnd/clc.cc b/src/amf/amfnd/clc.cc index c8e60e6..126362b
100644
--- a/src/amf/amfnd/clc.cc
+++ b/src/amf/amfnd/clc.cc
@@ -2217,6 +2217,10 @@ uint32_t avnd_comp_clc_inst_restart_hdler(AVND_CB *cb,
AVND_COMP *comp) {
/* invoke terminate callback */
rc = avnd_comp_cbk_send(cb, comp, AVSV_AMF_COMP_TERM, 0, 0);
else {
+ /* For NPI component with DisableRestart=1 */
+ if (m_AVND_COMP_IS_RESTART_DIS(comp) && (comp->csi_list.n_nodes > 0)) {
+su_send_suRestart_recovery_msg(comp->su);
+ }
rc =
avnd_comp_clc_cmd_execute(cb, comp,
AVND_COMP_CLC_CMD_TYPE_TERMINATE);
m_AVND_COMP_REG_PARAM_RESET(cb, comp);
--
2.7.4
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 0/1] Review Request for amf: remove assignment for NPI component with enable DisableRestart V2 [#2879]
Summary: amf: remove assignment for NPI component with enable DisableRestart [#2879] Review request for Ticket(s): 2879 Peer Reviewer(s): Hans, Gary, Minh Pull request to: Gary, Minh Affected branch(es): develop Development branch: ticket-2879 Base revision: 35d2f00b16066ae16307c5ed5a68171edc817dbf Personal repository: git://git.code.sf.net/u/thangng/review 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): - revision 04696975b6b995ceb6e53b2719cb11244dec2c31 Author: thang.nguyen Date: Wed, 8 Aug 2018 13:33:30 +0700 amf: remove assignment for NPI component with enable DisableRestart [#2879] With NPI component configured with saAmfCtDefDisableRestart=1. Once invoking restart admin op, amfnd does not remove the assignment and cause the crash. Remove assignment before change the pres state to TERMINATION in clc. Complete diffstat: -- src/amf/amfnd/clc.cc | 4 1 file changed, 4 insertions(+) Testing Commands: - Described on the ticket Testing, Expected Results: -- N/A. Conditions of Submission: - N/A. Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 y y 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 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 ~/.gitconfig file (i.e. user.name, user.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. -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 1/1] amf: remove assignment for NPI component with enable DisableRestart [#2879]
With NPI component configured with saAmfCtDefDisableRestart=1. Once invoking
restart admin op, amfnd does not remove the assignment and cause the crash.
Remove assignment before change the pres state to TERMINATION in clc.
---
src/amf/amfnd/clc.cc | 4
1 file changed, 4 insertions(+)
diff --git a/src/amf/amfnd/clc.cc b/src/amf/amfnd/clc.cc
index c8e60e6..126362b 100644
--- a/src/amf/amfnd/clc.cc
+++ b/src/amf/amfnd/clc.cc
@@ -2217,6 +2217,10 @@ uint32_t avnd_comp_clc_inst_restart_hdler(AVND_CB *cb,
AVND_COMP *comp) {
/* invoke terminate callback */
rc = avnd_comp_cbk_send(cb, comp, AVSV_AMF_COMP_TERM, 0, 0);
else {
+ /* For NPI component with DisableRestart=1 */
+ if (m_AVND_COMP_IS_RESTART_DIS(comp) && (comp->csi_list.n_nodes > 0)) {
+su_send_suRestart_recovery_msg(comp->su);
+ }
rc =
avnd_comp_clc_cmd_execute(cb, comp,
AVND_COMP_CLC_CMD_TYPE_TERMINATE);
m_AVND_COMP_REG_PARAM_RESET(cb, comp);
--
2.7.4
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
