date:20191023

Re: [devel] [PATCH 1/1] smf: Improve SmfAdminStateHandler() Return false if Fail [#3104]

2019-10-23 Thread Tran Thuan

Hi Phuc,

ACK from me.

Best Regards,
ThuanTr

-Original Message-
From: phuc.h.chau  
Sent: Thursday, October 24, 2019 10:28 AM
To: lennart.l...@ericsson.com; thuan.t...@dektech.com.au;
thang.d.ngu...@dektech.com.au; minh.c...@dektech.com.au;
gary@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net; phuc.h.chau

Subject: [PATCH 1/1] smf: Improve SmfAdminStateHandler() Return false if
Fail [#3104]

SW upgrade testing, if found that if a service unit is in
INSTANTIATION_FAILED,
one_step upgrade will not continue with the software installation.
---
 src/smf/smfd/SmfAdminState.cc | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)
 mode change 100644 => 100755 src/smf/smfd/SmfAdminState.cc

diff --git a/src/smf/smfd/SmfAdminState.cc b/src/smf/smfd/SmfAdminState.cc
old mode 100644
new mode 100755
index 076f9f0..4730215
--- a/src/smf/smfd/SmfAdminState.cc
+++ b/src/smf/smfd/SmfAdminState.cc
@@ -858,7 +858,7 @@ bool SmfAdminStateHandler::deleteNodeGroup() {
 bool SmfAdminStateHandler::nodeGroupAdminOperation(
 SaAmfAdminOperationIdT adminOp) {
 
-  bool method_rc = true;
+  bool method_rc = false;
 
   TRACE_ENTER();
 
@@ -920,20 +920,17 @@ bool SmfAdminStateHandler::nodeGroupAdminOperation(
   } else if (imm_rc != SA_AIS_OK) {
 LOG_NO("%s adminOpTimeout Fail %s", __FUNCTION__,
saf_error(imm_rc));
 errno_ = imm_rc;
-method_rc = false;
   } else {
 LOG_NO("%s adminOpTimeout Fail %s", __FUNCTION__,
saf_error(oi_rc));
 errno_ = oi_rc;
-method_rc = false;
   }
 }
   } else {
 LOG_NO("%s: becomeAdminOwnerOf(%s) Fail", __FUNCTION__,
nodeGroupName_s.c_str());
-method_rc = false;
   }
 
-  if (method_rc == true) {
+  if (admset_rc == true) {
 TRACE("%s Admin operation is done. Release ownership if nodegroup",
   __FUNCTION__);
 if (releaseAdminOwnerOf(nodeGroupName_s) == false) {
-- 
2.7.4




___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

[devel] [PATCH 1/1] base: add serial number arithmetic (RFC1982) [#3074]

2019-10-23 Thread thuan.tran

- Adapt MDS with this SNA implementation.
---
 src/base/Makefile.am |   6 +-
 src/base/sna.h   | 126 +++
 src/base/tests/sna_test.cc   | 117 
 src/mds/mds_tipc_fctrl_intf.cc   |   3 +-
 src/mds/mds_tipc_fctrl_portid.cc |  45 ++-
 src/mds/mds_tipc_fctrl_portid.h  |  79 +++
 6 files changed, 280 insertions(+), 96 deletions(-)
 create mode 100644 src/base/sna.h
 create mode 100644 src/base/tests/sna_test.cc

diff --git a/src/base/Makefile.am b/src/base/Makefile.am
index 025fb86a2..5082175cf 100644
--- a/src/base/Makefile.am
+++ b/src/base/Makefile.am
@@ -173,7 +173,8 @@ noinst_HEADERS += \
src/base/unix_client_socket.h \
src/base/unix_server_socket.h \
src/base/unix_socket.h \
-   src/base/usrbuf.h
+   src/base/usrbuf.h \
+   src/base/sna.h
 
 TESTS += bin/testleap bin/libbase_test bin/core_common_test
 
@@ -237,7 +238,8 @@ bin_libbase_test_SOURCES = \
src/base/tests/time_compare_test.cc \
src/base/tests/time_convert_test.cc \
src/base/tests/time_subtract_test.cc \
-   src/base/tests/unix_socket_test.cc
+   src/base/tests/unix_socket_test.cc \
+   src/base/tests/sna_test.cc
 
 bin_libbase_test_LDADD = \
$(GTEST_DIR)/lib/libgtest.la \
diff --git a/src/base/sna.h b/src/base/sna.h
new file mode 100644
index 0..b231fb134
--- /dev/null
+++ b/src/base/sna.h
@@ -0,0 +1,126 @@
+/*  -*- OpenSAF  -*-
+ *
+ * Copyright Ericsson AB 2019 - 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
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed
+ * under the GNU Lesser General Public License Version 2.1, February 1999.
+ * The complete license can be accessed from the following location:
+ * http://opensource.org/licenses/lgpl-license.php
+ * See the Copying file included with the OpenSAF distribution for full
+ * licensing terms.
+ *
+ * Reference: Serial Number Arithmetic from RFC1982
+ *
+ */
+
+#ifndef BASE_SNA_H_
+#define BASE_SNA_H_
+
+#include 
+#include 
+
+#define MAX_16BITS 65536  // 2^16
+#define MAX_32BITS 4294967296  // 2^32
+
+template 
+class _sna {
+ private:
+  T i;
+  uint64_t max() {
+if (typeid(T) == typeid(uint64_t)) {
+  return MAX_32BITS;
+} else if (typeid(T) == typeid(uint32_t)) {
+  return MAX_16BITS;
+} else {
+  printf("Type:%s\n", typeid(T).name());
+  throw std::out_of_range("Invalid type");
+}
+  }
+
+ public:
+  _sna(): i(0) {}
+  _sna(const _sna &t) {
+i = t.i;
+  }
+  explicit _sna(const uint64_t &n) {
+if ((n < 0) || (n > (max()-1)))
+  throw std::out_of_range("SNA assign with invalid value");
+i = n;
+  }
+  _sna& operator=(const _sna &t) {
+// check for self-assignment
+if (&t == this)
+  return *this;
+i = t.i;
+return *this;
+  }
+  T v() const {
+return i;
+  }
+  _sna& operator+=(const uint64_t& n) {
+if ((n < 0) || (n > (max()/2 - 1)))
+  throw std::out_of_range("SNA received invalid addition value");
+i = (i + n) % max();
+return *this;
+  }
+  friend _sna operator+(_sna m, const uint64_t& n) {
+m += n;
+return m;
+  }
+  // prefix ++
+  _sna& operator++() {
+*this += 1;
+return *this;
+  }
+  // postfix ++
+  _sna operator++(int) {
+_sna tmp(*this);
+operator++();
+return tmp;
+  }
+  bool operator==(const _sna& rhs) {
+return i == rhs.i;
+  }
+  bool operator==(const uint32_t val) {
+return i == val;
+  }
+  bool operator!=(const _sna& rhs) {
+return i != rhs.i;
+  }
+  bool operator<(const _sna& rhs) {
+return (i < rhs.i && rhs.i - i < max()/2) || \
+   (i > rhs.i && i - rhs.i > max()/2);
+  }
+  bool operator>=(const _sna& rhs) {
+return !(*this < rhs);
+  }
+  bool operator>(const _sna& rhs) {
+return (i < rhs.i && rhs.i - i > max()/2) || \
+   (i > rhs.i && i - rhs.i < max()/2);
+  }
+  bool operator<=(const _sna& rhs) {
+return !(*this > rhs);
+  }
+  int64_t operator-(const _sna& rhs) {
+if (*this >= rhs) {
+  if (i >= rhs.v()) {
+return i - rhs.v();
+  } else {
+return (i + max()) - rhs.v();
+  }
+} else {
+  if (i < rhs.v()) {
+return i - rhs.v();
+  } else {
+return i - (rhs.v() + max());
+  }
+}
+  }
+};
+
+typedef _sna sna16_t;
+typedef _sna sna32_t;
+
+#endif  // BASE_SNA_H_
diff --git a/src/base/tests/sna_test.cc b/src/base/tests/sna_test.cc
new file mode 100644
index 0..4b7eb83e3
--- /dev/null
+++ b/src/base/tests/sna_test.cc
@@ -0,0 +1,117 @@
+/*  -*- OpenSAF  -*-
+ *
+ * Copyright Ericsson AB 2019 - 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
+ * or FITNE

[devel] [PATCH 0/1] Review Request for base: add serial number arithmetic (RFC1982) [#3074] V2

2019-10-23 Thread thuan.tran

Summary: base: add serial number arithmetic (RFC1982) [#3074]
Review request for Ticket(s): 3074
Peer Reviewer(s): Minh, Gary
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop
Development branch: ticket-3074
Base revision: 9b1c588a4f244cb34a304b03c7568e04fa6cd8a7
Personal repository: git://git.code.sf.net/u/thuantr/review


Impacted area   Impact y/n

 Docsn
 Build systemn
 RPM/packaging   n
 Configuration files n
 Startup scripts n
 SAF servicesn
 OpenSAF servicesy
 Core libraries  n
 Samples n
 Tests   n
 Other   n


Comments (indicate scope for each "y" above):
-
N/A

revision 7530732fef7a3191c82d503d700479130e0596bf
Author: thuan.tran 
Date:   Wed, 23 Oct 2019 10:13:56 +0700

base: add serial number arithmetic (RFC1982) [#3074]

- Adapt MDS with this SNA implementation.



Added Files:

 src/base/sna.h
 src/base/tests/sna_test.cc


Complete diffstat:
--
 src/base/Makefile.am |   6 +-
 src/base/sna.h   | 126 +++
 src/base/tests/sna_test.cc   | 117 
 src/mds/mds_tipc_fctrl_intf.cc   |   3 +-
 src/mds/mds_tipc_fctrl_portid.cc |  45 +++---
 src/mds/mds_tipc_fctrl_portid.h  |  79 +++-
 6 files changed, 280 insertions(+), 96 deletions(-)


Testing Commands:
-
N/A

Testing, Expected Results:
--
N/A

Conditions of Submission:
-
ACK by reviewers

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.



___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

[devel] [PATCH 0/1] Review Request for smf: Improve SmfAdminStateHandler() Return false if Fail [#3104] V2

2019-10-23 Thread phuc.h.chau

Summary: smf: Improve SmfAdminStateHandler() Return false if Fail [#3104]
Review request for Ticket(s): 3104
Peer Reviewer(s): Lennart Lund, Thuan Tran, Thang Nguyen
Pull request to: Minh Chau, Gary
Affected branch(es): develop
Development branch: ticket-3104
Base revision: 9b1c588a4f244cb34a304b03c7568e04fa6cd8a7
Personal repository: git://git.code.sf.net/u/zchxphc/review


Impacted area   Impact y/n

 Docsn
 Build systemn
 RPM/packaging   n
 Configuration files n
 Startup scripts n
 SAF servicesn
 OpenSAF servicesy
 Core libraries  n
 Samples n
 Tests   n
 Other   n


Comments (indicate scope for each "y" above):
-
*** EXPLAIN/COMMENT THE PATCH SERIES HERE ***

revision 39cfd872474ad831e326b24159a8c2799942a355
Author: phuc.h.chau 
Date:   Wed, 23 Oct 2019 17:58:37 +0700

smf: Improve SmfAdminStateHandler() Return false if Fail [#3104]

SW upgrade testing, if found that if a service unit is in INSTANTIATION_FAILED,
one_step upgrade will not continue with the software installation.



Complete diffstat:
--
 src/smf/smfd/SmfAdminState.cc | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)


Testing Commands:
-
N/A

Testing, Expected Results:
--
N/A

Conditions of Submission:
-
ACK from reviewers

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.



___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

[devel] [PATCH 1/1] smf: Improve SmfAdminStateHandler() Return false if Fail [#3104]

2019-10-23 Thread phuc.h.chau

SW upgrade testing, if found that if a service unit is in INSTANTIATION_FAILED,
one_step upgrade will not continue with the software installation.
---
 src/smf/smfd/SmfAdminState.cc | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)
 mode change 100644 => 100755 src/smf/smfd/SmfAdminState.cc

diff --git a/src/smf/smfd/SmfAdminState.cc b/src/smf/smfd/SmfAdminState.cc
old mode 100644
new mode 100755
index 076f9f0..4730215
--- a/src/smf/smfd/SmfAdminState.cc
+++ b/src/smf/smfd/SmfAdminState.cc
@@ -858,7 +858,7 @@ bool SmfAdminStateHandler::deleteNodeGroup() {
 bool SmfAdminStateHandler::nodeGroupAdminOperation(
 SaAmfAdminOperationIdT adminOp) {
 
-  bool method_rc = true;
+  bool method_rc = false;
 
   TRACE_ENTER();
 
@@ -920,20 +920,17 @@ bool SmfAdminStateHandler::nodeGroupAdminOperation(
   } else if (imm_rc != SA_AIS_OK) {
 LOG_NO("%s adminOpTimeout Fail %s", __FUNCTION__, saf_error(imm_rc));
 errno_ = imm_rc;
-method_rc = false;
   } else {
 LOG_NO("%s adminOpTimeout Fail %s", __FUNCTION__, saf_error(oi_rc));
 errno_ = oi_rc;
-method_rc = false;
   }
 }
   } else {
 LOG_NO("%s: becomeAdminOwnerOf(%s) Fail", __FUNCTION__,
nodeGroupName_s.c_str());
-method_rc = false;
   }
 
-  if (method_rc == true) {
+  if (admset_rc == true) {
 TRACE("%s Admin operation is done. Release ownership if nodegroup",
   __FUNCTION__);
 if (releaseAdminOwnerOf(nodeGroupName_s) == false) {
-- 
2.7.4



___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Re: [devel] [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to send response message [#3102]

2019-10-23 Thread Tran Thuan

Hi Minh,

Please see my responses inline.

In general, I am trying to implement new version to reuse exist database.

Best Regards,

ThuanTr

From: Minh Hon Chau  
Sent: Wednesday, October 23, 2019 12:23 PM
To: Tran Thuan ; 'Nguyen Minh Vu' 
; hans.nordeb...@ericsson.com; 
gary@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to 
send response message [#3102]

Hi Thuan,

Please see comment inline

Thanks

Minh

On 23/10/19 3:32 pm, Tran Thuan wrote:

Hi Minh,

Thanks for comments. See my response inline.

Btw, I am preparing to send out new patch, I think I found an issue in current 
patch.

Best Regards,

ThuanTr

-Original Message-
From: Minh Hon Chau   

Sent: Wednesday, October 23, 2019 5:52 AM
To: Tran Thuan   ; 
'Nguyen Minh Vu'   
; hans.nordeb...@ericsson.com 
 ; gary@dektech.com.au 

Cc: opensaf-devel@lists.sourceforge.net 

Subject: Re: [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to 
send response message [#3102]

Hi Thuan,

I wonder the patch would work in the same reproduced steps if the both 

adests have subscribed each other more than 2 services. The svc_cnt will 

be greater than 1 until it is the last service down event. I think 

that's why mds has the database @subtn_results, in which each item is an 

adest associated with a service id separately.

[Thuan] We can understand that adest still alive, then go with origin flow 
(wait 1.5s).

But can a process send SNDRSP then mds unregister? I think it cannot, because 
it’s in SNDRSP (blocking)

[M] Not unregister, it can be unsubscribe. Or do you mean a process can not 
send two SNDRSP at the same time on 2 different subscribed services? 
[T] I think process cannot do anything relate to MDS API because it is under 
send SNDRSP (blocking).

The scenario of this ticket happens for the process terminated/crash.

[M] Yes my doubt is in the context of this ticket - terminated/crash - you 
would get 2 service down event I think

[T] Yes, will get 2 service down events.

[M] I don't think adding a new database at the global scope for this specific 
case is a good idea, if we can reuse the existing database. Can you try to use 
MDS_SUBSCRIPTION_INFO, add a flag or something similar to indicate which case 
mds should wait 1.5 sec. It would isolate the bug fix in the scope of this 
problem.

[T] I don’t want to mess up current database and its logic. But anyway, I am 
trying to reuse that database. Please wait for next version.

The problem originally resides at the services code e.g ntf, imm... 

where the threads structure between mds receiving thread and main thread 

cause a race condition. Thus the service sends a message with a death 

adest which is removed from mds database, that confuses mds and hit 1.5 

secs wait time.

If I read the code correctly, the 1.5 wait time is for another case, it 

gives another chance to wait 1.5 when the subscription result is empty 

in @subtn_results because the service up has not arrived yet.

[Thuan] Yes, it will give a chance if adest not yet UP any.

My patch still keep that chance as origin code.

But I think I need reduce timeout for adest down timer, I am verifying this 
change.

mds  subscribe >

mds  sends message A x

mds wait 1.5 sec

mds <--- service up 

mds  sends message A >

So the 1.5 sec time is for early phase of waiting service up.

} else if (sub_info->tmr_flag != true) {

if ((MDS_SENDTYPE_RSP == req->i_sendtype) ||

(MDS_SENDTYPE_RRSP == req->i_sendtype)) {

time_wait = true;

m_MDS_LOG_INFO(

"MDS_SND_RCV:Disc queue: Subscr exists no timer 

running: Waiting for some time\n");

-> I think at this line, it means: the SUBSCRIPTION_TMR has timeout, and 

mds is sending RSP/RRSP which means mds should have received the 

*request* message (?), so mds wants to wait for another 1.5 second for 

service_up to create the subscription result in database.

The problem in this ticket hit 1.5 because the service down has already 

come and mds removed the subscription result item, now the ntf, imm... 

sends msg with a death adest, and mds now it thinks it is waiting for a 

service up to come as at the early phase, so it waits. Both two 

scenarios can be distinguished themselves to avoid to wait 1.5 secs for 

the latter case I think.

Thanks

Minh

On 22/10/19 9:50 pm, Tran Thuan wrote:

> Hi Vu,

> 

> Thanks for additional comments.

> I reply your concerns inline below.

> 

> Best Regards,

> ThuanTr

> 

> -Original Message-

> From: Nguyen Minh Vu <  
> vu.m.ngu

Re: [devel] [PATCH 1/1] smf: Improve SmfAdminStateHandler() Return false if Fail [#3104]

2019-10-23 Thread Chau Hoang Phuc

Hi,

There is a issue with this change, I need consider more.
Please ignore this review for now.

Best Regards,
Phuc chau

-Original Message-
From: phuc.h.chau [mailto:phuc.h.c...@dektech.com.au] 
Sent: Wednesday, October 23, 2019 03:28 PM
To: lennart.l...@ericsson.com; thuan.t...@dektech.com.au;
thang.d.ngu...@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net; phuc.h.chau

Subject: [PATCH 1/1] smf: Improve SmfAdminStateHandler() Return false if
Fail [#3104]

SW upgrade testing, if found that if a service unit is in
INSTANTIATION_FAILED, one_step upgrade will not continue with the software
installation.
---
 src/smf/smfd/SmfAdminState.cc | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)  mode change 100644 =>
100755 src/smf/smfd/SmfAdminState.cc

diff --git a/src/smf/smfd/SmfAdminState.cc b/src/smf/smfd/SmfAdminState.cc
old mode 100644 new mode 100755 index 076f9f0..4730215
--- a/src/smf/smfd/SmfAdminState.cc
+++ b/src/smf/smfd/SmfAdminState.cc
@@ -858,7 +858,7 @@ bool SmfAdminStateHandler::deleteNodeGroup() {  bool
SmfAdminStateHandler::nodeGroupAdminOperation(
 SaAmfAdminOperationIdT adminOp) {
 
-  bool method_rc = true;
+  bool method_rc = false;
 
   TRACE_ENTER();
 
@@ -920,20 +920,17 @@ bool SmfAdminStateHandler::nodeGroupAdminOperation(
   } else if (imm_rc != SA_AIS_OK) {
 LOG_NO("%s adminOpTimeout Fail %s", __FUNCTION__,
saf_error(imm_rc));
 errno_ = imm_rc;
-method_rc = false;
   } else {
 LOG_NO("%s adminOpTimeout Fail %s", __FUNCTION__,
saf_error(oi_rc));
 errno_ = oi_rc;
-method_rc = false;
   }
 }
   } else {
 LOG_NO("%s: becomeAdminOwnerOf(%s) Fail", __FUNCTION__,
nodeGroupName_s.c_str());
-method_rc = false;
   }
 
-  if (method_rc == true) {
+  if (admset_rc == true) {
 TRACE("%s Admin operation is done. Release ownership if nodegroup",
   __FUNCTION__);
 if (releaseAdminOwnerOf(nodeGroupName_s) == false) {
--
2.7.4




___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

[devel] [PATCH 1/1] smf: Improve SmfAdminStateHandler() Return false if Fail [#3104]

2019-10-23 Thread phuc.h.chau

SW upgrade testing, if found that if a service unit is in INSTANTIATION_FAILED,
one_step upgrade will not continue with the software installation.
---
 src/smf/smfd/SmfAdminState.cc | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)
 mode change 100644 => 100755 src/smf/smfd/SmfAdminState.cc

diff --git a/src/smf/smfd/SmfAdminState.cc b/src/smf/smfd/SmfAdminState.cc
old mode 100644
new mode 100755
index 076f9f0..4730215
--- a/src/smf/smfd/SmfAdminState.cc
+++ b/src/smf/smfd/SmfAdminState.cc
@@ -858,7 +858,7 @@ bool SmfAdminStateHandler::deleteNodeGroup() {
 bool SmfAdminStateHandler::nodeGroupAdminOperation(
 SaAmfAdminOperationIdT adminOp) {
 
-  bool method_rc = true;
+  bool method_rc = false;
 
   TRACE_ENTER();
 
@@ -920,20 +920,17 @@ bool SmfAdminStateHandler::nodeGroupAdminOperation(
   } else if (imm_rc != SA_AIS_OK) {
 LOG_NO("%s adminOpTimeout Fail %s", __FUNCTION__, saf_error(imm_rc));
 errno_ = imm_rc;
-method_rc = false;
   } else {
 LOG_NO("%s adminOpTimeout Fail %s", __FUNCTION__, saf_error(oi_rc));
 errno_ = oi_rc;
-method_rc = false;
   }
 }
   } else {
 LOG_NO("%s: becomeAdminOwnerOf(%s) Fail", __FUNCTION__,
nodeGroupName_s.c_str());
-method_rc = false;
   }
 
-  if (method_rc == true) {
+  if (admset_rc == true) {
 TRACE("%s Admin operation is done. Release ownership if nodegroup",
   __FUNCTION__);
 if (releaseAdminOwnerOf(nodeGroupName_s) == false) {
-- 
2.7.4



___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

[devel] [PATCH 0/1] Review Request for smf: Improve SmfAdminStateHandler() Return false if Fail [#3104] V2

2019-10-23 Thread phuc.h.chau

Summary: smf: Improve SmfAdminStateHandler() Return false if Fail [#3104]
Review request for Ticket(s): 3104
Peer Reviewer(s): Lennart Lund, Thuan Tran, Thang Nguyen
Pull request to: Minh Chau, Gary
Affected branch(es): develop
Development branch: ticket-3104
Base revision: 9b1c588a4f244cb34a304b03c7568e04fa6cd8a7
Personal repository: git://git.code.sf.net/u/zchxphc/review


Impacted area   Impact y/n

 Docsn
 Build systemn
 RPM/packaging   n
 Configuration files n
 Startup scripts n
 SAF servicesn
 OpenSAF servicesy
 Core libraries  n
 Samples n
 Tests   n
 Other   n


Comments (indicate scope for each "y" above):
-
*** EXPLAIN/COMMENT THE PATCH SERIES HERE ***

revision 4e299fa1d909a40f62d987c6c07d7e61575cff95
Author: phuc.h.chau 
Date:   Wed, 23 Oct 2019 13:36:58 +0700

smf: Improve SmfAdminStateHandler() Return false if Fail [#3104]

SW upgrade testing, if found that if a service unit is in INSTANTIATION_FAILED,
one_step upgrade will not continue with the software installation.



Complete diffstat:
--
 src/smf/smfd/SmfAdminState.cc | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)


Testing Commands:
-
N/A

Testing, Expected Results:
--
N/A

Conditions of Submission:
-
ACK from reviewers

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.



___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Re: [devel] [PATCH 1/1] smf: Improve SmfAdminStateHandler() Return false if Fail [#3104]

[devel] [PATCH 1/1] base: add serial number arithmetic (RFC1982) [#3074]

[devel] [PATCH 0/1] Review Request for base: add serial number arithmetic (RFC1982) [#3074] V2

[devel] [PATCH 0/1] Review Request for smf: Improve SmfAdminStateHandler() Return false if Fail [#3104] V2

[devel] [PATCH 1/1] smf: Improve SmfAdminStateHandler() Return false if Fail [#3104]

Re: [devel] [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to send response message [#3102]

Re: [devel] [PATCH 1/1] smf: Improve SmfAdminStateHandler() Return false if Fail [#3104]

[devel] [PATCH 1/1] smf: Improve SmfAdminStateHandler() Return false if Fail [#3104]

[devel] [PATCH 0/1] Review Request for smf: Improve SmfAdminStateHandler() Return false if Fail [#3104] V2

9 matches

Site Navigation

Mail list logo

Footer information