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://sourceforge.net/p/opensaf/wiki/Coding%20Rules/>, 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! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel


------------------------------------------------------------------------------
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

Reply via email to