Hi Nguyen, Reviewed the Patch. Ack from my side.
Thanks, Syam. -----Original Message----- From: Nguyen Luu [mailto:nguyen.tk....@dektech.com.au] Sent: Monday, April 9, 2018 7:56 AM To: Lennart Lund <lennart.l...@ericsson.com>; Vijay Roy <vijay....@oracle.com> Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [devel] [PATCH 1/1] smfd: Fix incorrect handling of SMFND NCSMDS_UP/DOWN events [#2821] Hi, I intend to push this patch by the end of today if there are no more comments. Thanks, Nguyen On 4/6/2018 10:54 AM, Nguyen Luu wrote: > Hi Lennart, > > Thank you for your comment. > > You suggested that the changed files be reformatted following Google > Coding Style (i.e space-indented); but according to OpenSAF Coding > Rules > <https://urldefense.proofpoint.com/v2/url?u=https-3A__sourceforge.net_p_opensaf_wiki_Coding-2520Rules_&d=DwIGaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=V-rHpog7bglMV_qtz_u-J_IeS6G1w2MSO-XRrmj38iQ&m=VlY7OygEZVrtnevHPu_qib7Y2P1nwqqwVvvp_IR_ZS4&s=RJtncFTnfz3-HwlTpk4sNF0rke1PxSzYqTiN-qU4Xow&e=>, > C code shall follow Linux kernel Coding Style (i.e tab-indented). > It seems that the old code line used mixed tabs and spaces, so that's > probably why you see my new code line as incorrectly indented. I used > 'tabs' actually. > > Can you check again and confirm the coding style? > > Thanks, > Nguyen > > On 4/5/2018 8:38 PM, Lennart Lund wrote: >> Hi Nguyen, >> >> Ack with comments. See below [Lennart] >> >> Thanks >> Lennart >> >>> -----Original Message----- >>> From: Nguyen Luu [mailto:nguyen.tk....@dektech.com.au] >>> Sent: den 28 mars 2018 10:08 >>> To: Lennart Lund <lennart.l...@ericsson.com>; vijay....@oracle.com >>> Cc: opensaf-devel@lists.sourceforge.net; Nguyen Tran Khoi Luu >>> <nguyen.tk....@dektech.com.au> >>> Subject: [PATCH 1/1] smfd: Fix incorrect handling of SMFND >>> NCSMDS_UP/DOWN events [#2821] >>> >>> Current handling of SMFND DOWN event does not take into account >>> failed SMFND UP event, which could eventually result in an inexact >>> view of the actual number of SMFND nodes in the cluster if, for >>> example, a node happened to be DOWN and UP twice, and the first UP >>> event somehow failed. >>> --- >>> src/smf/smfd/smfd_evt.c | 7 ++++--- >>> src/smf/smfd/smfd_smfnd.c | 9 +++++++++ >>> 2 files changed, 13 insertions(+), 3 deletions(-) >>> >>> diff --git a/src/smf/smfd/smfd_evt.c b/src/smf/smfd/smfd_evt.c index >>> 32f83fd..6f60c13 100644 >>> --- a/src/smf/smfd/smfd_evt.c >>> +++ b/src/smf/smfd/smfd_evt.c >>> @@ -1,6 +1,7 @@ >>> /* -*- OpenSAF -*- >>> * >>> * (C) Copyright 2008 The OpenSAF Foundation >>> + * Copyright (C) 2018 Ericsson AB. All Rights Reserved. >>> * >>> * This program is distributed in the hope that it will be useful, >>> but >>> * WITHOUT ANY WARRANTY; without even the implied warranty of >>> MERCHANTABILITY @@ -86,7 +87,7 @@ static void >>> proc_mds_info(smfd_cb_t *cb, SMFSV_EVT >>> *evt) >>> >>> if (mds_info->svc_id == NCSMDS_SVC_ID_SMFND) { >>> if (smfnd_up(mds_info->node_id, mds_info->dest, >>> - mds_info->rem_svc_pvt_ver) == >>> SA_AIS_OK) >> [Lennart] Format: The following line has incorrect indentation and is >> probably too long. It is Ok to reformat this file according to Google >> style guide >>> + mds_info- >>>> rem_svc_pvt_ver) == NCSCC_RC_SUCCESS) >>> cb->no_of_smfnd++; >>> else >>> LOG_WA("%s: SMFND UP failed", __FUNCTION__); @@ >>> -100,8 +101,8 @@ static void proc_mds_info(smfd_cb_t *cb, SMFSV_EVT >>> *evt) >>> } >>> >>> if (mds_info->svc_id == NCSMDS_SVC_ID_SMFND) { >>> - smfnd_down(mds_info->node_id); >>> - cb->no_of_smfnd--; >>> + if (smfnd_down(mds_info->node_id) == >>> NCSCC_RC_SUCCESS) >>> + cb->no_of_smfnd--; >>> } >>> break; >>> >>> diff --git a/src/smf/smfd/smfd_smfnd.c b/src/smf/smfd/smfd_smfnd.c >>> index c48adb2..5a64507 100644 >>> --- a/src/smf/smfd/smfd_smfnd.c >>> +++ b/src/smf/smfd/smfd_smfnd.c >>> @@ -1,6 +1,7 @@ >>> /* OpenSAF >>> * >>> * (C) Copyright 2008 The OpenSAF Foundation >>> + * Copyright (C) 2018 Ericsson AB. All Rights Reserved. >>> * >>> * This program is distributed in the hope that it will be useful, >>> but >>> * WITHOUT ANY WARRANTY; without even the implied warranty of >>> MERCHANTABILITY @@ -212,6 +213,14 @@ uint32_t >>> smfnd_down(SaClmNodeIdT i_node_id) >>> /* Update the node info */ >>> while (smfnd != NULL) { >>> if (smfnd->clmInfo.nodeId == i_node_id) { >>> + /* Check if the node state was already Down, >>> + * probably due to previous failed SMFND UP event >>> + */ >>> + if (smfnd->nd_state == ndDown) { >> [Lennart] Formatting: The following line is too long. It is Ok to >> reformat this file to follow Google style guide >>> + TRACE("SMFND node state was already >>> Down. No update needed"); >>> + pthread_mutex_unlock(&smfnd_list_lock); >>> + return NCSCC_RC_FAILURE; >>> + } >>> /* Store the nd state */ >>> TRACE( >>> "SMFND state updated for node [%s], id [%x]", >>> -- >>> 2.7.4 > > ---------------------------------------------------------------------- > -------- > > Check out the vibrant tech community on one of the world's most > engaging tech sites, Slashdot.org! > https://urldefense.proofpoint.com/v2/url?u=http-3A__sdm.link_slashdot&; > d=DwIGaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=V-rHpog7bglMV > _qtz_u-J_IeS6G1w2MSO-XRrmj38iQ&m=VlY7OygEZVrtnevHPu_qib7Y2P1nwqqwVvvp_ > IR_ZS4&s=PYS0Odx-C32dF6ALaumZgBXjAVMVkpIpzzIuCBfxLOY&e= > _______________________________________________ > Opensaf-devel mailing list > Opensaf-devel@lists.sourceforge.net > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.sourceforge > .net_lists_listinfo_opensaf-2Ddevel&d=DwIGaQ&c=RoP1YumCXCgaWHvlZYR8PZh > 8Bv7qIrMUB65eapI_JnE&r=V-rHpog7bglMV_qtz_u-J_IeS6G1w2MSO-XRrmj38iQ&m=V > lY7OygEZVrtnevHPu_qib7Y2P1nwqqwVvvp_IR_ZS4&s=IdNegm2tUNNrYdpDhTUuzwJ2w > WmdC6HKP6Q570GE4WY&e= ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! https://urldefense.proofpoint.com/v2/url?u=http-3A__sdm.link_slashdot&d=DwIGaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=V-rHpog7bglMV_qtz_u-J_IeS6G1w2MSO-XRrmj38iQ&m=VlY7OygEZVrtnevHPu_qib7Y2P1nwqqwVvvp_IR_ZS4&s=PYS0Odx-C32dF6ALaumZgBXjAVMVkpIpzzIuCBfxLOY&e= _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.sourceforge.net_lists_listinfo_opensaf-2Ddevel&d=DwIGaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=V-rHpog7bglMV_qtz_u-J_IeS6G1w2MSO-XRrmj38iQ&m=VlY7OygEZVrtnevHPu_qib7Y2P1nwqqwVvvp_IR_ZS4&s=IdNegm2tUNNrYdpDhTUuzwJ2wWmdC6HKP6Q570GE4WY&e= ------------------------------------------------------------------------------ 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
