Hi Nguyen, My comment was just a minor comment. Just handle it based on what you think is best.
Thanks Lennart > -----Original Message----- > From: Syam Prasad Talluri [mailto:syam.tall...@oracle.com] > Sent: den 9 april 2018 11:28 > To: Nguyen Tran Khoi Luu <nguyen.tk....@dektech.com.au>; 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 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=RJtnc > FTnfz3-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=IdNegm2tUNNrYdpD > hTUuzwJ2w > > 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=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIr > MUB65eapI_JnE&r=V-rHpog7bglMV_qtz_u-J_IeS6G1w2MSO- > XRrmj38iQ&m=VlY7OygEZVrtnevHPu_qib7Y2P1nwqqwVvvp_IR_ZS4&s=PYS0 > Odx-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_Jn > E&r=V-rHpog7bglMV_qtz_u-J_IeS6G1w2MSO- > XRrmj38iQ&m=VlY7OygEZVrtnevHPu_qib7Y2P1nwqqwVvvp_IR_ZS4&s=IdNe > gm2tUNNrYdpDhTUuzwJ2wWmdC6HKP6Q570GE4WY&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
