Re: [devel] [PATCH 1/1] smf: smf: Upgrade nodes without using node group [#2592]

2017-10-17 Thread Rafael Odzakow 

Comments inline

Complex patch, consider separating logical changes and refactoring into 
two patches to make it easier.



On 09/28/2017 02:58 PM, Lennart Lund wrote:

Do not create a node group for lock operations on nodes if only one node
(deactivation/activation units). This is needed to make it possible to
upgrade old OpenSAF versions. On old versions AMF will crash and cause
cyclic node reboot
---
  src/smf/smfd/SmfUpgradeStep.cc | 517 ++---
  src/smf/smfd/SmfUpgradeStep.h  |  11 +-
  src/smf/smfd/SmfUtils.cc   |   8 +
  3 files changed, 345 insertions(+), 191 deletions(-)

diff --git a/src/smf/smfd/SmfUpgradeStep.cc b/src/smf/smfd/SmfUpgradeStep.cc
index d40fac4fc..892c32602 100644
--- a/src/smf/smfd/SmfUpgradeStep.cc
+++ b/src/smf/smfd/SmfUpgradeStep.cc
@@ -2793,23 +2793,11 @@ 
SmfAdminOperation::SmfAdminOperation(std::list *i_allUnits)
TRACE("%s m_NodeGroupAdminOwnerName '%s'", __FUNCTION__,
  m_admin_owner_name.c_str());
  
-  // Note:

-  // If any of the following steps fails m_creation_fail is set to true
-  // and an immediate return is done
-  // The public methods shall return fail if m_creation_fail is true
-
-  bool rc = initNodeGroupOm();
-  if (rc == false) {
-LOG_NO("%s: initNodeGroupOm Fail", __FUNCTION__);
-m_creation_fail = true;
-return;
-  }
-
-  rc = becomeAdminOwnerOfAmfClusterObj();
-  if (rc == false) {
-LOG_NO("%s: becomeAdminOwnerOfAmfClusterObj Fail", __FUNCTION__);
+  // Become admin owner using above created name
+  bool rc = initAdminOwner();
+  if (rc == false ) {
+LOG_NO("%s: initAdminOwner() Fail", __FUNCTION__);
  m_creation_fail = true;
-return;
}
  }
  
@@ -2840,16 +2828,9 @@ bool SmfAdminOperation::lock() {
  
createNodeAndSULockLists(SA_AMF_ADMIN_UNLOCKED);
  
-  rc = changeNodeGroupAdminState(SA_AMF_ADMIN_UNLOCKED, SA_AMF_ADMIN_LOCK);

-  if (rc == false) {
-LOG_NO("%s: changeNodeGroupAdminState() Fail %s", __FUNCTION__,
-   saf_error(m_errno));
-goto done;
-  }
-
-  rc = adminOperationSerialized(SA_AMF_ADMIN_LOCK, m_suList);
+  rc = changeAdminState(SA_AMF_ADMIN_UNLOCKED, SA_AMF_ADMIN_LOCK);
if (rc == false) {
-LOG_NO("%s: setAdminStateSUs() Fail %s", __FUNCTION__, saf_error(m_errno));
+LOG_NO("%s setAdminState() Fail", __FUNCTION__);
}
  
  done:

@@ -2880,17 +2861,9 @@ bool SmfAdminOperation::lock_in() {
  
createNodeAndSULockLists(SA_AMF_ADMIN_LOCKED);
  
-  rc = changeNodeGroupAdminState(SA_AMF_ADMIN_LOCKED,

- SA_AMF_ADMIN_LOCK_INSTANTIATION);
+  rc = changeAdminState(SA_AMF_ADMIN_LOCKED, SA_AMF_ADMIN_LOCK_INSTANTIATION);
if (rc == false) {
-LOG_NO("%s: changeNodeGroupAdminState() Fail %s", __FUNCTION__,
-   saf_error(m_errno));
-goto done;
-  }
-
-  rc = adminOperationSerialized(SA_AMF_ADMIN_LOCK_INSTANTIATION, m_suList);
-  if (rc == false) {
-LOG_NO("%s: setAdminStateSUs() Fail %s", __FUNCTION__, saf_error(m_errno));
+LOG_NO("%s setAdminState() Fail", __FUNCTION__);
}
  
  done:

@@ -2924,17 +2897,10 @@ bool SmfAdminOperation::unlock_in() {
// Note: This actually change state to SA_AMF_ADMIN_LOCKED
//   The admin operation is SA_AMF_ADMIN_UNLOCK_INSTANTIATION but
//   there is no SA_AMF_ADMIN_UNLOCK_INSTANTIATION state
-  rc = changeNodeGroupAdminState(SA_AMF_ADMIN_LOCKED_INSTANTIATION,
- SA_AMF_ADMIN_UNLOCK_INSTANTIATION);
+  rc = changeAdminState(SA_AMF_ADMIN_LOCKED_INSTANTIATION,
+ SA_AMF_ADMIN_UNLOCK_INSTANTIATION);
if (rc == false) {
-LOG_NO("%s: changeNodeGroupAdminState() Fail %s", __FUNCTION__,
-   saf_error(m_errno));
-goto done;
-  }
-
-  rc = adminOperationSerialized(SA_AMF_ADMIN_UNLOCK_INSTANTIATION, m_suList);
-  if (rc == false) {
-LOG_NO("%s: setAdminStateSUs() Fail %s", __FUNCTION__, saf_error(m_errno));
+LOG_NO("%s setAdminState() Fail", __FUNCTION__);
}
  
  done:

@@ -2965,16 +2931,9 @@ bool SmfAdminOperation::unlock() {
  
createNodeAndSUUnlockLists(SA_AMF_ADMIN_LOCKED);
  
-  rc = changeNodeGroupAdminState(SA_AMF_ADMIN_LOCKED, SA_AMF_ADMIN_UNLOCK);

-  if (rc == false) {
-LOG_NO("%s: changeNodeGroupAdminState() Fail %s", __FUNCTION__,
-   saf_error(m_errno));
-goto done;
-  }
-
-  rc = adminOperationSerialized(SA_AMF_ADMIN_UNLOCK, m_suList);
+  rc = changeAdminState(SA_AMF_ADMIN_LOCKED, SA_AMF_ADMIN_UNLOCK);
if (rc == false) {
-LOG_NO("%s: setAdminStateSUs() Fail %s", __FUNCTION__, saf_error(m_errno));
+LOG_NO("%s changeAdminState() Fail", __FUNCTION__);
}
  
  done:

@@ -3013,15 +2972,17 @@ done:
  /// SmfSetAdminState Private
  /// ---
  
-/// Get all needed IMM handles. Updates corresponding member variables

-/// Return false if fail
-///
-bool SmfAdminOperation::initNodeGroupOm() {
+// Initialize an OM (get an OM handle) and become admin owner with
+// admin

Re: [devel] [PATCH 1/1] smf: smf: Upgrade nodes without using node group [#2592]

2017-10-03 Thread Lennart Lund 
Sent again. Mail delivery system reports that this mail has failed to reach the 
community

> -Original Message-
> From: Lennart Lund
> Sent: den 28 september 2017 14:59
> To: Rafael Odzakow 
> Cc: opensaf-devel@lists.sourceforge.net; Lennart Lund
> 
> Subject: [PATCH 1/1] smf: smf: Upgrade nodes without using node group
> [#2592]
> 
> Do not create a node group for lock operations on nodes if only one node
> (deactivation/activation units). This is needed to make it possible to
> upgrade old OpenSAF versions. On old versions AMF will crash and cause
> cyclic node reboot
> ---
>  src/smf/smfd/SmfUpgradeStep.cc | 517 ++--
> -
>  src/smf/smfd/SmfUpgradeStep.h  |  11 +-
>  src/smf/smfd/SmfUtils.cc   |   8 +
>  3 files changed, 345 insertions(+), 191 deletions(-)
> 
> diff --git a/src/smf/smfd/SmfUpgradeStep.cc
> b/src/smf/smfd/SmfUpgradeStep.cc
> index d40fac4fc..892c32602 100644
> --- a/src/smf/smfd/SmfUpgradeStep.cc
> +++ b/src/smf/smfd/SmfUpgradeStep.cc
> @@ -2793,23 +2793,11 @@
> SmfAdminOperation::SmfAdminOperation(std::list
> *i_allUnits)
>TRACE("%s m_NodeGroupAdminOwnerName '%s'", __FUNCTION__,
>  m_admin_owner_name.c_str());
> 
> -  // Note:
> -  // If any of the following steps fails m_creation_fail is set to true
> -  // and an immediate return is done
> -  // The public methods shall return fail if m_creation_fail is true
> -
> -  bool rc = initNodeGroupOm();
> -  if (rc == false) {
> -LOG_NO("%s: initNodeGroupOm Fail", __FUNCTION__);
> -m_creation_fail = true;
> -return;
> -  }
> -
> -  rc = becomeAdminOwnerOfAmfClusterObj();
> -  if (rc == false) {
> -LOG_NO("%s: becomeAdminOwnerOfAmfClusterObj Fail",
> __FUNCTION__);
> +  // Become admin owner using above created name
> +  bool rc = initAdminOwner();
> +  if (rc == false ) {
> +LOG_NO("%s: initAdminOwner() Fail", __FUNCTION__);
>  m_creation_fail = true;
> -return;
>}
>  }
> 
> @@ -2840,16 +2828,9 @@ bool SmfAdminOperation::lock() {
> 
>createNodeAndSULockLists(SA_AMF_ADMIN_UNLOCKED);
> 
> -  rc = changeNodeGroupAdminState(SA_AMF_ADMIN_UNLOCKED,
> SA_AMF_ADMIN_LOCK);
> -  if (rc == false) {
> -LOG_NO("%s: changeNodeGroupAdminState() Fail %s", __FUNCTION__,
> -   saf_error(m_errno));
> -goto done;
> -  }
> -
> -  rc = adminOperationSerialized(SA_AMF_ADMIN_LOCK, m_suList);
> +  rc = changeAdminState(SA_AMF_ADMIN_UNLOCKED,
> SA_AMF_ADMIN_LOCK);
>if (rc == false) {
> -LOG_NO("%s: setAdminStateSUs() Fail %s", __FUNCTION__,
> saf_error(m_errno));
> +LOG_NO("%s setAdminState() Fail", __FUNCTION__);
>}
> 
>  done:
> @@ -2880,17 +2861,9 @@ bool SmfAdminOperation::lock_in() {
> 
>createNodeAndSULockLists(SA_AMF_ADMIN_LOCKED);
> 
> -  rc = changeNodeGroupAdminState(SA_AMF_ADMIN_LOCKED,
> - SA_AMF_ADMIN_LOCK_INSTANTIATION);
> +  rc = changeAdminState(SA_AMF_ADMIN_LOCKED,
> SA_AMF_ADMIN_LOCK_INSTANTIATION);
>if (rc == false) {
> -LOG_NO("%s: changeNodeGroupAdminState() Fail %s", __FUNCTION__,
> -   saf_error(m_errno));
> -goto done;
> -  }
> -
> -  rc = adminOperationSerialized(SA_AMF_ADMIN_LOCK_INSTANTIATION,
> m_suList);
> -  if (rc == false) {
> -LOG_NO("%s: setAdminStateSUs() Fail %s", __FUNCTION__,
> saf_error(m_errno));
> +LOG_NO("%s setAdminState() Fail", __FUNCTION__);
>}
> 
>  done:
> @@ -2924,17 +2897,10 @@ bool SmfAdminOperation::unlock_in() {
>// Note: This actually change state to SA_AMF_ADMIN_LOCKED
>//   The admin operation is SA_AMF_ADMIN_UNLOCK_INSTANTIATION
> but
>//   there is no SA_AMF_ADMIN_UNLOCK_INSTANTIATION state
> -  rc =
> changeNodeGroupAdminState(SA_AMF_ADMIN_LOCKED_INSTANTIATION,
> - SA_AMF_ADMIN_UNLOCK_INSTANTIATION);
> +  rc = changeAdminState(SA_AMF_ADMIN_LOCKED_INSTANTIATION,
> + SA_AMF_ADMIN_UNLOCK_INSTANTIATION);
>if (rc == false) {
> -LOG_NO("%s: changeNodeGroupAdminState() Fail %s", __FUNCTION__,
> -   saf_error(m_errno));
> -goto done;
> -  }
> -
> -  rc =
> adminOperationSerialized(SA_AMF_ADMIN_UNLOCK_INSTANTIATION,
> m_suList);
> -  if (rc == false) {
> -LOG_NO("%s: setAdminStateSUs() Fail %s", __FUNCTION__,
> saf_error(m_errno));
> +LOG_NO("%s setAdminState() Fail", __FUNCTION__);
>}
> 
>  done:
> @@ -2965,16 +2931,9 @@ bool SmfAdminOperation::unlock() {
> 
>createNodeAndSUUnlockLists(SA_AMF_ADMIN_LOCKED);
> 
> -  rc = changeNodeGroupAdminState(SA_AMF_ADMIN_LOCKED,
> SA_AMF_ADMIN_UNLOCK);
> -  if (rc == false) {
> -LOG_NO("%s: changeNodeGroupAdminState() Fail %s", __FUNCTION__,
> -   saf_error(m_errno));
> -goto done;
> -  }
> -
> -  rc = adminOperationSerialized(SA_AMF_ADMIN_UNLOCK, m_suList);
> +  rc = changeAdminState(SA_AMF_ADMIN_LOCKED,
> SA_AMF_ADMIN_UNLOCK);
>if (rc == false) {
> -LOG_NO("%s: setAdminStateSUs() Fail %s",

[devel] [PATCH 1/1] smf: smf: Upgrade nodes without using node group [#2592]

2017-09-28 Thread Lennart Lund 
Do not create a node group for lock operations on nodes if only one node
(deactivation/activation units). This is needed to make it possible to
upgrade old OpenSAF versions. On old versions AMF will crash and cause
cyclic node reboot
---
 src/smf/smfd/SmfUpgradeStep.cc | 517 ++---
 src/smf/smfd/SmfUpgradeStep.h  |  11 +-
 src/smf/smfd/SmfUtils.cc   |   8 +
 3 files changed, 345 insertions(+), 191 deletions(-)

diff --git a/src/smf/smfd/SmfUpgradeStep.cc b/src/smf/smfd/SmfUpgradeStep.cc
index d40fac4fc..892c32602 100644
--- a/src/smf/smfd/SmfUpgradeStep.cc
+++ b/src/smf/smfd/SmfUpgradeStep.cc
@@ -2793,23 +2793,11 @@ 
SmfAdminOperation::SmfAdminOperation(std::list *i_allUnits)
   TRACE("%s m_NodeGroupAdminOwnerName '%s'", __FUNCTION__,
 m_admin_owner_name.c_str());
 
-  // Note:
-  // If any of the following steps fails m_creation_fail is set to true
-  // and an immediate return is done
-  // The public methods shall return fail if m_creation_fail is true
-
-  bool rc = initNodeGroupOm();
-  if (rc == false) {
-LOG_NO("%s: initNodeGroupOm Fail", __FUNCTION__);
-m_creation_fail = true;
-return;
-  }
-
-  rc = becomeAdminOwnerOfAmfClusterObj();
-  if (rc == false) {
-LOG_NO("%s: becomeAdminOwnerOfAmfClusterObj Fail", __FUNCTION__);
+  // Become admin owner using above created name
+  bool rc = initAdminOwner();
+  if (rc == false ) {
+LOG_NO("%s: initAdminOwner() Fail", __FUNCTION__);
 m_creation_fail = true;
-return;
   }
 }
 
@@ -2840,16 +2828,9 @@ bool SmfAdminOperation::lock() {
 
   createNodeAndSULockLists(SA_AMF_ADMIN_UNLOCKED);
 
-  rc = changeNodeGroupAdminState(SA_AMF_ADMIN_UNLOCKED, SA_AMF_ADMIN_LOCK);
-  if (rc == false) {
-LOG_NO("%s: changeNodeGroupAdminState() Fail %s", __FUNCTION__,
-   saf_error(m_errno));
-goto done;
-  }
-
-  rc = adminOperationSerialized(SA_AMF_ADMIN_LOCK, m_suList);
+  rc = changeAdminState(SA_AMF_ADMIN_UNLOCKED, SA_AMF_ADMIN_LOCK);
   if (rc == false) {
-LOG_NO("%s: setAdminStateSUs() Fail %s", __FUNCTION__, saf_error(m_errno));
+LOG_NO("%s setAdminState() Fail", __FUNCTION__);
   }
 
 done:
@@ -2880,17 +2861,9 @@ bool SmfAdminOperation::lock_in() {
 
   createNodeAndSULockLists(SA_AMF_ADMIN_LOCKED);
 
-  rc = changeNodeGroupAdminState(SA_AMF_ADMIN_LOCKED,
- SA_AMF_ADMIN_LOCK_INSTANTIATION);
+  rc = changeAdminState(SA_AMF_ADMIN_LOCKED, SA_AMF_ADMIN_LOCK_INSTANTIATION);
   if (rc == false) {
-LOG_NO("%s: changeNodeGroupAdminState() Fail %s", __FUNCTION__,
-   saf_error(m_errno));
-goto done;
-  }
-
-  rc = adminOperationSerialized(SA_AMF_ADMIN_LOCK_INSTANTIATION, m_suList);
-  if (rc == false) {
-LOG_NO("%s: setAdminStateSUs() Fail %s", __FUNCTION__, saf_error(m_errno));
+LOG_NO("%s setAdminState() Fail", __FUNCTION__);
   }
 
 done:
@@ -2924,17 +2897,10 @@ bool SmfAdminOperation::unlock_in() {
   // Note: This actually change state to SA_AMF_ADMIN_LOCKED
   //   The admin operation is SA_AMF_ADMIN_UNLOCK_INSTANTIATION but
   //   there is no SA_AMF_ADMIN_UNLOCK_INSTANTIATION state
-  rc = changeNodeGroupAdminState(SA_AMF_ADMIN_LOCKED_INSTANTIATION,
- SA_AMF_ADMIN_UNLOCK_INSTANTIATION);
+  rc = changeAdminState(SA_AMF_ADMIN_LOCKED_INSTANTIATION,
+ SA_AMF_ADMIN_UNLOCK_INSTANTIATION);
   if (rc == false) {
-LOG_NO("%s: changeNodeGroupAdminState() Fail %s", __FUNCTION__,
-   saf_error(m_errno));
-goto done;
-  }
-
-  rc = adminOperationSerialized(SA_AMF_ADMIN_UNLOCK_INSTANTIATION, m_suList);
-  if (rc == false) {
-LOG_NO("%s: setAdminStateSUs() Fail %s", __FUNCTION__, saf_error(m_errno));
+LOG_NO("%s setAdminState() Fail", __FUNCTION__);
   }
 
 done:
@@ -2965,16 +2931,9 @@ bool SmfAdminOperation::unlock() {
 
   createNodeAndSUUnlockLists(SA_AMF_ADMIN_LOCKED);
 
-  rc = changeNodeGroupAdminState(SA_AMF_ADMIN_LOCKED, SA_AMF_ADMIN_UNLOCK);
-  if (rc == false) {
-LOG_NO("%s: changeNodeGroupAdminState() Fail %s", __FUNCTION__,
-   saf_error(m_errno));
-goto done;
-  }
-
-  rc = adminOperationSerialized(SA_AMF_ADMIN_UNLOCK, m_suList);
+  rc = changeAdminState(SA_AMF_ADMIN_LOCKED, SA_AMF_ADMIN_UNLOCK);
   if (rc == false) {
-LOG_NO("%s: setAdminStateSUs() Fail %s", __FUNCTION__, saf_error(m_errno));
+LOG_NO("%s changeAdminState() Fail", __FUNCTION__);
   }
 
 done:
@@ -3013,15 +2972,17 @@ done:
 /// SmfSetAdminState Private
 /// ---
 
-/// Get all needed IMM handles. Updates corresponding member variables
-/// Return false if fail
-///
-bool SmfAdminOperation::initNodeGroupOm() {
+// Initialize an OM (get an OM handle) and become admin owner with
+// admin owner name set to the SmfAdminOperation admin owner name
+// Note: Does not set admin ownership for any object
+//   Use becomeAdminOwnerOf(object) and releaseAdminOwnerOf(object)
+//
+//
+bool