[devel] [PATCH 1 of 1] amfd: avoid active SI failover during su or node failover recovery, N-WAY model[#1944]

2016-09-29 Thread praveen . malviya
 osaf/services/saf/amf/amfd/sg_nway_fsm.cc |  10 ++
 osaf/services/saf/amf/amfd/si.cc  |   8 ++--
 2 files changed, 16 insertions(+), 2 deletions(-)


AMF make SI active in two SUs when a component faults with su-failover recovery
in removal callback during lock operation on SU. In

As a part of lock operation when quiesced assignment gets successful for
both SI1 and SI2 in the locked SU1, AMFD makes SU2 active for both the SIs and
sends SU level delete to SU1. Comp1 in SU1 faults with su-failover recovery 
while handling
CSI Remove callback. AMFD makes SU3 active for both SIs as part of recovery. 
Since SU2
was already made active after successful quiesced assignments, AMFD should not 
perform
failover of SI1 and SI2 to SU3.

Patch avoids failover if SI is already active in some other healthy SU.

diff --git a/osaf/services/saf/amf/amfd/sg_nway_fsm.cc 
b/osaf/services/saf/amf/amfd/sg_nway_fsm.cc
--- a/osaf/services/saf/amf/amfd/sg_nway_fsm.cc
+++ b/osaf/services/saf/amf/amfd/sg_nway_fsm.cc
@@ -2999,6 +2999,16 @@ void avd_sg_nway_node_fail_stable(AVD_CL
if (curr_susi == susi)
continue;
 
+   /*
+  This node and its SUs are already marked OOS. 
+  If a valid active SISU exists for this SI on some other node 
and this SI 
+  is not having any sponsor then continue the for loop as for 
this 
+  SI (curr_susi->si) neither failover is required nor deletion 
of all 
+  assignments. This curr_susi will be deleted in this function.
+*/
+   if ((curr_susi->si->is_active() == true) && 
(curr_susi->si->spons_si_list == nullptr))
+   continue;
+   
if ((SA_AMF_HA_ACTIVE == curr_susi->state) ||
(SA_AMF_HA_QUIESCED == curr_susi->state) || 
(SA_AMF_HA_QUIESCING == curr_susi->state)) {
/* identify the most preferred standby su for this si */
diff --git a/osaf/services/saf/amf/amfd/si.cc b/osaf/services/saf/amf/amfd/si.cc
--- a/osaf/services/saf/amf/amfd/si.cc
+++ b/osaf/services/saf/amf/amfd/si.cc
@@ -1553,8 +1553,12 @@ void AVD_SI::update_alarm_state(bool ala
 bool AVD_SI::is_active() const
 {
for (AVD_SU_SI_REL *sisu = list_of_sisu; sisu != nullptr; sisu = 
sisu->si_next) {
-   if ((sisu->state == SA_AMF_HA_ACTIVE) && (sisu->fsm == 
AVD_SU_SI_STATE_ASGND) &&
-   (sisu->su->saAmfSuReadinessState == 
SA_AMF_READINESS_IN_SERVICE)) {
+   if ((sisu->state == SA_AMF_HA_ACTIVE) &&
+   ((sisu->fsm == AVD_SU_SI_STATE_ASGND) ||
+(sisu->fsm == AVD_SU_SI_STATE_ASGN) ||
+(sisu->fsm == AVD_SU_SI_STATE_MODIFY)) &&
+   (sisu->si->saAmfSIAdminState == SA_AMF_ADMIN_UNLOCKED) 
&&
+   (sisu->su->saAmfSuReadinessState == 
SA_AMF_READINESS_IN_SERVICE)) {
return true;
}
}

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


[devel] [PATCH 0 of 1] Review Request for amfd: avoid active SI failover during su or node failover recovery, N-WAY model[#1944]

2016-09-29 Thread praveen . malviya
Summary:  amfd: avoid active SI failover during su or node failover recovery, 
N-WAY model[#1944]
Review request for Trac Ticket(s): #1944 
Peer Reviewer(s): AMF devs 
Pull request to: <>
Affected branch(es): ALL 
Development branch: <>


Impacted area   Impact y/n

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


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

changeset 32afe4c02b285d1eac1ac70bae71fe0c3fd0eb60
Author: praveen.malv...@oracle.com
Date:   Fri, 30 Sep 2016 12:21:56 +0530

amfd: avoid active SI failover during su or node failover recovery, 
N-WAY
model[#1944]

AMF make SI active in two SUs when a component faults with su-failover
recovery in removal callback during lock operation on SU. In

As a part of lock operation when quiesced assignment gets successful for
both SI1 and SI2 in the locked SU1, AMFD makes SU2 active for both the 
SIs
and sends SU level delete to SU1. Comp1 in SU1 faults with su-failover
recovery while handling CSI Remove callback. AMFD makes SU3 active for 
both
SIs as part of recovery. Since SU2 was already made active after 
successful
quiesced assignments, AMFD should not perform failover of SI1 and SI2 to
SU3.

Patch avoids failover if SI is already active in some other healthy SU.


Complete diffstat:
--
 osaf/services/saf/amf/amfd/sg_nway_fsm.cc |  10 ++
 osaf/services/saf/amf/amfd/si.cc  |   8 ++--
 2 files changed, 16 insertions(+), 2 deletions(-)


Testing Commands:
-
Tested as per ticket description.
Also tested lock operation on Node group (it covers node).

Testing, Expected Results:
--
Two active SUs for same SI not observed.

Conditions of Submission:
-
Ack from any reviewer.

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 ~/.hgrc file (i.e. username, 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-deve

Re: [devel] [PATCH 0 of 1] Review Request for imm: Set validation abort error string when OI returns ERR_BAD_OPERATION or ERR_FAILED_OPERATION [#1650]

2016-09-29 Thread Neelakanta Reddy
Hi Hung,

Reviewed and tested the patch.
Ack.

/Neel.

On 2016/09/29 01:18 PM, Hung Nguyen wrote:
> Summary: imm: Set validation abort error string when OI returns 
> ERR_BAD_OPERATION or ERR_FAILED_OPERATION [#1650]
> Review request for Trac Ticket(s): 1650
> Peer Reviewer(s): Zoran, Neel
> Pull request to:
> Affected branch(es): 5.0, 5.1, 5.2
> Development branch: 5.2
>
> 
> Impacted area   Impact y/n
> 
>   Docsn
>   Build systemn
>   RPM/packaging   n
>   Configuration files n
>   Startup scripts n
>   SAF servicesy
>   OpenSAF servicesn
>   Core libraries  n
>   Samples n
>   Tests   n
>   Other   n
>
>
> Comments (indicate scope for each "y" above):
> -
>
>
> changeset c3d087ae2dcb8a6aa9b1e6fc79922ef8d70cfbe2
> Author:   Hung Nguyen 
> Date: Thu, 29 Sep 2016 11:15:50 +0700
>
>   imm: Set validation abort error string when OI returns 
> ERR_BAD_OPERATION or
>   ERR_FAILED_OPERATION [#1650]
>
>   Set validation abort error string when OI returns ERR_BAD_OPERATION or
>   ERR_FAILED_OPERATION. This is to prevent om clients from retrying the
>   operations that are already rejected by OI.
>
>
> Complete diffstat:
> --
>   osaf/services/saf/immsv/immnd/ImmModel.cc |  18 +++---
>   osaf/services/saf/immsv/immnd/immnd_evt.c |  82 
> ++
>   2 files changed, 37 insertions(+), 63 deletions(-)
>
>
> Testing Commands:
> -
>
>
>
> Testing, Expected Results:
> --
>
>
>
> Conditions of Submission:
> -
> Ack from reviewers.
>
>
> Arch  Built StartedLinux distro
> ---
> mipsn  n
> mips64  n  n
> x86 n  n
> x86_64  n  n
> 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 ~/.hgrc file (i.e. username, 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 0 of 1] Review Request for smf: Recreate IMM handles if bad handle when deleting node group [#2046]

2016-09-29 Thread Neelakanta Reddy
Hi Leenart,

Reviewed the patch.
Ack.

/Neel.

On 2016/09/29 06:15 PM, Lennart Lund wrote:
> Summary: smf: Recreate IMM handles if bad handle when deleting node group
> Review request for Trac Ticket(s): #2046
> Peer Reviewer(s): reddy.neelaka...@oracle.com, rafael.odza...@ericsson.com
> Pull request to: <>
> Affected branch(es): <>
> Development branch: <>
>
> 
> Impacted area   Impact y/n
> 
>   Docsn
>   Build systemn
>   RPM/packaging   n
>   Configuration files n
>   Startup scripts n
>   SAF servicesy
>   OpenSAF servicesn
>   Core libraries  n
>   Samples n
>   Tests   n
>   Other   n
>
>
> Comments (indicate scope for each "y" above):
> -
>   <>
>
> changeset 5092586880b8ee951502d59badb5d8c397b0916e
> Author:   Lennart Lund 
> Date: Thu, 29 Sep 2016 14:31:35 +0200
>
>   smf: Recreate IMM handles if bad handle when deleting node group [#2046]
>
>   Fix the deleteNodeGroup() method must be so that if the delete operation
>   fails with bad handles new handles are created so that the node group 
> can be
>   deleted
>
>
> Complete diffstat:
> --
>   osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc |  93 
> +
>   osaf/services/saf/smfsv/smfd/SmfUpgradeStep.hh |   3 +-
>   2 files changed, 67 insertions(+), 29 deletions(-)
>
>
> Testing Commands:
> -
>   <>
>
>
> Testing, Expected Results:
> --
>   <>
>
>
> Conditions of Submission:
> -
> Ack by reviewers
>
>
> Arch  Built StartedLinux distro
> ---
> mipsn  n
> mips64  n  n
> x86 n  n
> x86_64  n  n
> 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 ~/.hgrc file (i.e. username, 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 0 of 1] Review Request for smf: Delete node group if already exist when creating [#2049]

2016-09-29 Thread Neelakanta Reddy
Hi Leenart,

Reviewed the patch.
Ack.

/Neel.

On 2016/09/29 06:11 PM, Lennart Lund wrote:
> Summary: smf: Delete node group if already exist when creating
> Review request for Trac Ticket(s): #2049
> Peer Reviewer(s): reddy.neelaka...@oracle.com, rafael.odza...@ericsson.com
> Pull request to: <>
> Affected branch(es): <>
> Development branch: <>
>
> 
> Impacted area   Impact y/n
> 
>   Docsn
>   Build systemn
>   RPM/packaging   n
>   Configuration files n
>   Startup scripts n
>   SAF servicesy
>   OpenSAF servicesn
>   Core libraries  n
>   Samples n
>   Tests   n
>   Other   n
>
>
> Comments (indicate scope for each "y" above):
> -
>   <>
>
> changeset c680b5ad6c35b1ffcdfa9e563f1f32b3f3cc8e36
> Author:   Lennart Lund 
> Date: Thu, 29 Sep 2016 14:23:33 +0200
>
>   smf: Delete node group if already exist when creating [#2049]
>
>   If a node group for admin operation already exist when create is done 
> the
>   existing group shall be deleted so that the new group can be created
>
>
> Complete diffstat:
> --
>   osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc |  76 
> +---
>   1 files changed, 49 insertions(+), 27 deletions(-)
>
>
> Testing Commands:
> -
>   <>
>
>
> Testing, Expected Results:
> --
>   <>
>
>
> Conditions of Submission:
> -
> Ack from reviewers
>
>
> Arch  Built StartedLinux distro
> ---
> mipsn  n
> mips64  n  n
> x86 n  n
> x86_64  n  n
> 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 ~/.hgrc file (i.e. username, 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 of 1] clm: add support for cluster reboot V3 [#2053]

2016-09-29 Thread Anders Widell
One idea to prevent fast nodes from rebooting and joining the old 
cluster before all old nodes have shut down, is to set a flag in the CLM 
server that prevents any node from joining during the time the cluster 
reset is in progress (i.e. until the active CLM server has shut down).

/ Anders Widell


On 09/29/2016 03:47 PM, Hans Nordebäck wrote:
> Hi Anders, thanks for the comments, I have updated the patch accordingly.
> Mathi have you had any time to look at the patch?
> /BR HansN
>
> -Original Message-
> From: Anders Widell
> Sent: den 29 september 2016 14:29
> To: Hans Nordebäck ; mathi.naic...@oracle.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1 of 1] clm: add support for cluster reboot V3 [#2053]
>
> Ack with comments.
>
> One comment is that we ought to export this new admin op in the saClm.h file 
> (or actually a new file separate file, like we have done when extending the 
> SAF API in other services). I think we should also select a new number (i.e. 
> 4) for this, even though it is targeted for a different object. So add e.g. 
> the following line:
>
> SA_CLM_ADMIN_CLUSTER_RESET = 4
>
> In the future, we should consider how this feature interact with the enhanced 
> cluster management feature that is on the roadmap. For now I think it is good 
> enough, but in the future I would like to have a two-phase shutdown to avoid 
> the possibility that some of the nodes reboot very quickly and potentially 
> could come up before some other slow node has shut down.
>
> See further code comments inline below, marked AndersW>
>
> regards,
>
> Anders Widell
>
> On 09/28/2016 01:55 PM, Hans Nordeback wrote:
>>osaf/libs/common/clmsv/include/clmsv_msg.h   |   6 +++
>>osaf/libs/core/common/include/osaf_utility.h |   5 +++
>>osaf/libs/core/common/osaf_utility.c |  22 +
>>osaf/services/saf/clmsv/clms/clms.h  |   3 +-
>>osaf/services/saf/clmsv/clms/clms_imm.c  |  18 ++
>>osaf/services/saf/clmsv/clms/clms_mds.c  |  46 
>> +++-
>>osaf/services/saf/clmsv/clms/clms_util.c |  12 +++
>>osaf/services/saf/clmsv/nodeagent/main.c |  12 +++
>>scripts/opensaf_reboot   |  22 ++---
>>9 files changed, 139 insertions(+), 7 deletions(-)
>>
>>
>> Admin command to request cluster reboot:
>> immadm -o 1 safCluster=myClmCluster
>>
>> diff --git a/osaf/libs/common/clmsv/include/clmsv_msg.h
>> b/osaf/libs/common/clmsv/include/clmsv_msg.h
>> --- a/osaf/libs/common/clmsv/include/clmsv_msg.h
>> +++ b/osaf/libs/common/clmsv/include/clmsv_msg.h
>> @@ -23,6 +23,7 @@ typedef enum clms_msg_type {
>>  CLMSV_CLMS_TO_CLMA_CBK_MSG,
>>  CLMSV_CLMS_TO_CLMA_API_RESP_MSG,
>>  CLMSV_CLMS_TO_CLMA_IS_MEMBER_MSG,
>> +  CLMSV_CLMS_TO_CLMNA_REBOOT_MSG,
>>  CLMSV_MSG_MAX
>>} CLMSV_MSG_TYPE;
>>
>> @@ -174,6 +175,10 @@ typedef struct clmsv_is_member_info_t {
>>  SaUint32T client_id;
>>}CLMSV_IS_MEMBER_INFO;
>>
>> +typedef struct clmsv_reboot_info_t {
>> +  SaClmNodeIdT node_id;
>> +} CLMSV_REBOOT_INFO;
>> +
>>/* Top Level CLMSv MDS message structure for use between CLMS-> CLMA && 
>> CLMA -> CLMS */
>>typedef struct clmsv_msg_t {
>>  struct clmsv_msg_t *next;   /* Mailbox processing */
>> @@ -183,6 +188,7 @@ typedef struct clmsv_msg_t {
>>CLMSV_CBK_INFO cbk_info;/* Callback Messages from CLMS to CLA 
>> */
>>CLMSV_API_RESP_INFO api_resp_info;  /* Response Messages from 
>> CLMS to CLA */
>>CLMSV_IS_MEMBER_INFO is_member_info;/*Is node member or not 
>> Message from CLMS to CLA*/
>> +CLMSV_REBOOT_INFO reboot_info;  /* Reboot request from CLMS to CLMNA */
>>  } info;
>>} CLMSV_MSG;
>>
>> diff --git a/osaf/libs/core/common/include/osaf_utility.h
>> b/osaf/libs/core/common/include/osaf_utility.h
>> --- a/osaf/libs/core/common/include/osaf_utility.h
>> +++ b/osaf/libs/core/common/include/osaf_utility.h
>> @@ -24,6 +24,8 @@
>>#ifndef OPENSAF_CORE_OSAF_UTILITY_H_
>>#define OPENSAF_CORE_OSAF_UTILITY_H_
>>
>> +#define USE_SAFE_REBOOT 1
> AndersW> Maybe instead use:
>
> enum {
> kUseSafeReboot = 1
> };
>
>> +
>>#include 
>>
>>#ifdef  __cplusplus
>> @@ -68,6 +70,9 @@ extern void osaf_abort(long i_cause)
>>#endif
>>nothrow, noreturn));
>>
>> +extern void osaf_safe_reboot()
>> +__attribute__ ((nothrow));
>> +
> AndersW> Since this is C code, a function taking no parameter should be
> declared "void osaf_safe_reboot(void)"
>>static inline void osaf_mutex_lock_ordie(pthread_mutex_t* io_mutex) {
>>  int result = pthread_mutex_lock(io_mutex);
>>  if (result != 0) osaf_abort(result); diff --git
>> a/osaf/libs/core/common/osaf_utility.c
>> b/osaf/libs/core/common/osaf_utility.c
>> --- a/osaf/libs/core/common/osaf_utility.c
>> +++ b/osaf/libs/core/common/osaf_utility.c
>> @@ -16,9 +16,12 @@
>> */
>>
>>#include "osaf_u

Re: [devel] [PATCH 1 of 1] smf: Recreate IMM handles if bad handle when deleting node group [#2046]

2016-09-29 Thread Rafael Odzakow
ACK


On 09/29/2016 02:45 PM, Lennart Lund wrote:
>   osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc |  93 
> ++---
>   osaf/services/saf/smfsv/smfd/SmfUpgradeStep.hh |   3 +-
>   2 files changed, 67 insertions(+), 29 deletions(-)
>
>
> Fix the deleteNodeGroup() method must be so that if the delete operation fails
> with bad handles new handles are created so that the node group can be deleted
>
> diff --git a/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc 
> b/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc
> --- a/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc
> +++ b/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc
> @@ -2823,7 +2823,7 @@ SmfAdminOperation::SmfAdminOperation(std
>   // and an immediate return is done
>   // The public methods shall return fail if m_creation_fail is true
>   
> - bool rc = getAllImmHandles();
> + bool rc = initAllImmHandles();
>   if (rc == false) {
>   LOG_NO("%s: getAllImmHandles Fail", __FUNCTION__);
>   m_creation_fail = true;
> @@ -2860,11 +2860,7 @@ SmfAdminOperation::SmfAdminOperation(std
>   
>   SmfAdminOperation::~SmfAdminOperation()
>   {
> - // This frees all IMM handles that's based on the omHandle and
> - // give up admin ownership of Amf Cluster object
> - if (m_omHandle != 0) {
> - (void)immutil_saImmOmFinalize(m_omHandle);
> - }
> +finalizeAllImmHandles();
>   }
>   
>   ///
> @@ -3079,7 +3075,7 @@ bool SmfAdminOperation::restart()
>   /// Get all needed IMM handles. Updates corresponding member variables
>   /// Return false if fail
>   ///
> -bool SmfAdminOperation::getAllImmHandles()
> +bool SmfAdminOperation::initAllImmHandles()
>   {
>   SaAisErrorT ais_rc = SA_AIS_ERR_TRY_AGAIN;
>   int timeout_try_cnt = 6;
> @@ -3156,6 +3152,15 @@ bool SmfAdminOperation::getAllImmHandles
>   return rc;
>   }
>   
> +// Free all IMM handles that's based on the omHandle and
> +// give up admin ownership of Amf Cluster object
> +void SmfAdminOperation::finalizeAllImmHandles() {
> + if (m_omHandle != 0) {
> + (void)immutil_saImmOmFinalize(m_omHandle);
> + }
> +
> +}
> +
>   /// Check if the AIS return is considered a campaign error
>   /// Do not Fail if any of the following AIS codes
>   ///
> @@ -3552,6 +3557,8 @@ bool SmfAdminOperation::createNodeGroup(
>   // A node group with the same name already exist
>   // May happen if a previous delete after usage has
>   // failed
> +LOG_NO("%s: saImmOmCcbObjectCreate_2 Fail %s",
> +   __FUNCTION__, saf_error(m_errno));
>   bool rc = deleteNodeGroup();
>   if (rc == false) {
>   LOG_NO("%s: deleteNodeGroup() Fail",
> @@ -3591,8 +3598,7 @@ bool SmfAdminOperation::createNodeGroup(
>   ///
>   bool SmfAdminOperation::deleteNodeGroup()
>   {
> - bool rc = true;
> - m_errno = SA_AIS_OK;
> + SaAisErrorT ais_rc = SA_AIS_OK;
>   
>   TRACE_ENTER();
>   std::string nodeGroupName_s =
> @@ -3602,25 +3608,53 @@ bool SmfAdminOperation::deleteNodeGroup(
>   &nodeGroupName);
>   
>   TRACE("\t Deleting nodeGroup '%s'", nodeGroupName_s.c_str());
> -
> - m_errno = immutil_saImmOmCcbObjectDelete(m_ccbHandle,
> - &nodeGroupName);
> - if (m_errno != SA_AIS_OK) {
> - LOG_NO("%s: saImmOmCcbObjectDelete '%s' Fail %s",
> -  __FUNCTION__, nodeGroupName_s.c_str(),
> -  saf_error(m_errno));
> - rc = false;
> - } else {
> - m_errno = saImmOmCcbApply(m_ccbHandle);
> - if (m_errno != SA_AIS_OK) {
> - LOG_NO("%s: saImmOmCcbApply() Fail '%s'",
> - __FUNCTION__, saf_error(m_errno));
> - rc = false;
> - }
> - }
> -
> - TRACE_LEAVE();
> - return rc;
> + bool method_rc = false;
> +const uint32_t MAX_NO_RETRIES = 2;
> +uint32_t retry_cnt = 0;
> +while (++retry_cnt <= MAX_NO_RETRIES) {
> +// Handles including ccb handle may have been invalidated by
> +// IMM resulting in SA_AIS_ERR_BAD_HANDLE response on the
> +// delete object request.
> +// If that's the case: Try to create new handles and try 
> again
> +ais_rc = immutil_saImmOmCcbObjectDelete(m_ccbHandle,
> +&nodeGroupName);
> +TRACE("%s: immutil_saImmOmCcbObjectDelete %s",
> +__FUNCTION__, saf_error(m_errno));
> +
> +if (ais_rc == SA_AIS_ERR_BAD_HANDLE) {
> +LOG_NO("%s: saImmOmCcbObjectDelete Fail %s",
> +   __FUNCTION__, saf_error(m_errno));
> +finalizeAllImmHandles();
> +bool rc = initAllIm

Re: [devel] [PATCH 0 of 1] Review Request for smf: Delete node group if already exist when creating [#2049]

2016-09-29 Thread Rafael Odzakow
ACK


On 09/29/2016 02:41 PM, Lennart Lund wrote:
> Summary: smf: Delete node group if already exist when creating
> Review request for Trac Ticket(s): #2049
> Peer Reviewer(s): reddy.neelaka...@oracle.com, rafael.odza...@ericsson.com
> Pull request to: <>
> Affected branch(es): <>
> Development branch: <>
>
> 
> Impacted area   Impact y/n
> 
>   Docsn
>   Build systemn
>   RPM/packaging   n
>   Configuration files n
>   Startup scripts n
>   SAF servicesy
>   OpenSAF servicesn
>   Core libraries  n
>   Samples n
>   Tests   n
>   Other   n
>
>
> Comments (indicate scope for each "y" above):
> -
>   <>
>
> changeset c680b5ad6c35b1ffcdfa9e563f1f32b3f3cc8e36
> Author:   Lennart Lund 
> Date: Thu, 29 Sep 2016 14:23:33 +0200
>
>   smf: Delete node group if already exist when creating [#2049]
>
>   If a node group for admin operation already exist when create is done 
> the
>   existing group shall be deleted so that the new group can be created
>
>
> Complete diffstat:
> --
>   osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc |  76 
> +---
>   1 files changed, 49 insertions(+), 27 deletions(-)
>
>
> Testing Commands:
> -
>   <>
>
>
> Testing, Expected Results:
> --
>   <>
>
>
> Conditions of Submission:
> -
> Ack from reviewers
>
>
> Arch  Built StartedLinux distro
> ---
> mipsn  n
> mips64  n  n
> x86 n  n
> x86_64  n  n
> 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 ~/.hgrc file (i.e. username, 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 of 1] dtm: Convert transport monitor script to a daemon [#2035]

2016-09-29 Thread Hans Nordebäck
One very minor comment, instead of the busy wait we may check if e.g. inotify 
can be used for daemons and implement a "wrapper" for easier use?
/BR HansN

-Original Message-
From: Anders Widell [mailto:anders.wid...@ericsson.com] 
Sent: den 22 september 2016 13:17
To: mahesh.va...@oracle.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: [devel] [PATCH 1 of 1] dtm: Convert transport monitor script to a 
daemon [#2035]

 configure.ac|  
  3 +-
 opensaf.spec.in |  
  4 +-
 osaf/services/infrastructure/dtms/Makefile.am   |  
  3 +-
 osaf/services/infrastructure/dtms/scripts/Makefile.am   |  
  3 +-
 osaf/services/infrastructure/dtms/scripts/osaf-transport-monitor.in |  
 82 ---
 osaf/services/infrastructure/dtms/scripts/osaf-transport.in |  
 45 ++-
 osaf/services/infrastructure/dtms/transport/Makefile.am |  
 41 +++
 osaf/services/infrastructure/dtms/transport/main.cc |  
 52 
 osaf/services/infrastructure/dtms/transport/tests/Makefile.am   |  
 45 
 osaf/services/infrastructure/dtms/transport/tests/mock_logtrace.cc  |  
 34 +++
 osaf/services/infrastructure/dtms/transport/tests/mock_logtrace.h   |  
 23 ++
 osaf/services/infrastructure/dtms/transport/tests/mock_osaf_poll.cc |  
 26 ++
 osaf/services/infrastructure/dtms/transport/tests/mock_osaf_poll.h  |  
 38 +++
 osaf/services/infrastructure/dtms/transport/tests/transport_monitor_test.cc |  
109 ++
 osaf/services/infrastructure/dtms/transport/transport_monitor.cc|  
 95 
 osaf/services/infrastructure/dtms/transport/transport_monitor.h |  
 92 
 osaf/services/infrastructure/nid/scripts/opensafd.in|  
  6 +-
 17 files changed, 593 insertions(+), 108 deletions(-)


Convert the osaf-transport-monitor shell script into a daemon called 
osaftransportd. The functionality of this new daemon is (should be) exactly the 
same as the functionality of the shell script that it replaces.

diff --git a/configure.ac b/configure.ac
--- a/configure.ac
+++ b/configure.ac
@@ -784,10 +784,11 @@ AC_CONFIG_FILES([
 osaf/services/infrastructure/Makefile
 osaf/services/infrastructure/dtms/Makefile
 osaf/services/infrastructure/dtms/dtm/Makefile
+osaf/services/infrastructure/dtms/transport/Makefile
+osaf/services/infrastructure/dtms/transport/tests/Makefile
 osaf/services/infrastructure/dtms/scripts/Makefile
 osaf/services/infrastructure/dtms/scripts/osaf-dtm
 osaf/services/infrastructure/dtms/scripts/osaf-transport
-osaf/services/infrastructure/dtms/scripts/osaf-transport-monitor
 osaf/services/infrastructure/dtms/config/Makefile
 osaf/services/infrastructure/dtms/include/Makefile
 osaf/services/infrastructure/fm/Makefile
diff --git a/opensaf.spec.in b/opensaf.spec.in
--- a/opensaf.spec.in
+++ b/opensaf.spec.in
@@ -1384,9 +1384,9 @@ fi
 %{_pkglibdir}/osaffmd
 %{_pkgclcclidir}/osaf-fmd
 %{_pkglibdir}/osafdtmd
+%{_pkglibdir}/osaftransportd
 %{_pkgclcclidir}/osaf-dtm
 %{_pkgclcclidir}/osaf-transport
-%{_pkgclcclidir}/osaf-transport-monitor
 %{_bindir}/rdegetrole
 %if %is_immxml
 %{_pkgimmxml_svcdir}/common_pl_template.xml
@@ -1406,9 +1406,9 @@ fi
 %config(noreplace) %{_pkgsysconfdir}/nodeinit.conf.payload
 %config(noreplace) %{_pkgsysconfdir}/dtmd.conf  %{_pkglibdir}/osafdtmd
+%{_pkglibdir}/osaftransportd
 %{_pkgclcclidir}/osaf-dtm
 %{_pkgclcclidir}/osaf-transport
-%{_pkgclcclidir}/osaf-transport-monitor
 
 %files libs
 %defattr(-,root,root)
diff --git a/osaf/services/infrastructure/dtms/Makefile.am 
b/osaf/services/infrastructure/dtms/Makefile.am
--- a/osaf/services/infrastructure/dtms/Makefile.am
+++ b/osaf/services/infrastructure/dtms/Makefile.am
@@ -18,5 +18,4 @@ include $(top_srcdir)/Makefile.common
 
 MAINTAINERCLEANFILES = Makefile.in
 
-SUBDIRS =  config  dtm  include  scripts
-
+SUBDIRS =  config  dtm  transport include  scripts
diff --git a/osaf/services/infrastructure/dtms/scripts/Makefile.am 
b/osaf/services/infrastructure/dtms/scripts/Makefile.am
--- a/osaf/services/infrastructure/dtms/scripts/Makefile.am
+++ b/osaf/services/infrastructure/dtms/scripts/Makefile.am
@@ -20,5 +20,4 @@ MAINTAINERCLEANFILES = Makefile.in
 
 nodist_pkgclccli_SCRIPTS = \
$(top_builddir)/osaf/services/infrastructure/dtms/scripts/osaf-dtm \
-   
$(top_builddir)/osaf/services/infrastructure/dtms/scripts/osaf-transport \
-
$(top_builddir)/osaf/services/infrastructure/dtms/scripts/osaf-transport-monitor
+   
+$(top_builddir)/osaf/services/infrastructure/dtms/scripts/osaf-transpor
+t
diff --git 
a/osaf/services/infrastructure/dtms/scripts/osaf-transport-monitor.in 
b/osaf/services/infrastructure/dtms/scripts/os

Re: [devel] [PATCH 1 of 1] clm: add support for cluster reboot V3 [#2053]

2016-09-29 Thread Hans Nordebäck
Hi Anders, thanks for the comments, I have updated the patch accordingly.
Mathi have you had any time to look at the patch?
/BR HansN

-Original Message-
From: Anders Widell 
Sent: den 29 september 2016 14:29
To: Hans Nordebäck ; mathi.naic...@oracle.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1 of 1] clm: add support for cluster reboot V3 [#2053]

Ack with comments.

One comment is that we ought to export this new admin op in the saClm.h file 
(or actually a new file separate file, like we have done when extending the SAF 
API in other services). I think we should also select a new number (i.e. 4) for 
this, even though it is targeted for a different object. So add e.g. the 
following line:

SA_CLM_ADMIN_CLUSTER_RESET = 4

In the future, we should consider how this feature interact with the enhanced 
cluster management feature that is on the roadmap. For now I think it is good 
enough, but in the future I would like to have a two-phase shutdown to avoid 
the possibility that some of the nodes reboot very quickly and potentially 
could come up before some other slow node has shut down.

See further code comments inline below, marked AndersW>

regards,

Anders Widell

On 09/28/2016 01:55 PM, Hans Nordeback wrote:
>   osaf/libs/common/clmsv/include/clmsv_msg.h   |   6 +++
>   osaf/libs/core/common/include/osaf_utility.h |   5 +++
>   osaf/libs/core/common/osaf_utility.c |  22 +
>   osaf/services/saf/clmsv/clms/clms.h  |   3 +-
>   osaf/services/saf/clmsv/clms/clms_imm.c  |  18 ++
>   osaf/services/saf/clmsv/clms/clms_mds.c  |  46 
> +++-
>   osaf/services/saf/clmsv/clms/clms_util.c |  12 +++
>   osaf/services/saf/clmsv/nodeagent/main.c |  12 +++
>   scripts/opensaf_reboot   |  22 ++---
>   9 files changed, 139 insertions(+), 7 deletions(-)
>
>
> Admin command to request cluster reboot:
> immadm -o 1 safCluster=myClmCluster
>
> diff --git a/osaf/libs/common/clmsv/include/clmsv_msg.h 
> b/osaf/libs/common/clmsv/include/clmsv_msg.h
> --- a/osaf/libs/common/clmsv/include/clmsv_msg.h
> +++ b/osaf/libs/common/clmsv/include/clmsv_msg.h
> @@ -23,6 +23,7 @@ typedef enum clms_msg_type {
> CLMSV_CLMS_TO_CLMA_CBK_MSG,
> CLMSV_CLMS_TO_CLMA_API_RESP_MSG,
> CLMSV_CLMS_TO_CLMA_IS_MEMBER_MSG,
> +  CLMSV_CLMS_TO_CLMNA_REBOOT_MSG,
> CLMSV_MSG_MAX
>   } CLMSV_MSG_TYPE;
>   
> @@ -174,6 +175,10 @@ typedef struct clmsv_is_member_info_t {
> SaUint32T client_id;
>   }CLMSV_IS_MEMBER_INFO;
>   
> +typedef struct clmsv_reboot_info_t {
> +  SaClmNodeIdT node_id;
> +} CLMSV_REBOOT_INFO;
> +
>   /* Top Level CLMSv MDS message structure for use between CLMS-> CLMA && 
> CLMA -> CLMS */
>   typedef struct clmsv_msg_t {
> struct clmsv_msg_t *next;   /* Mailbox processing */
> @@ -183,6 +188,7 @@ typedef struct clmsv_msg_t {
>   CLMSV_CBK_INFO cbk_info;/* Callback Messages from CLMS to CLA */
>   CLMSV_API_RESP_INFO api_resp_info;  /* Response Messages from CLMS 
> to CLA */
>   CLMSV_IS_MEMBER_INFO is_member_info;/*Is node member or not Message 
> from CLMS to CLA*/
> +CLMSV_REBOOT_INFO reboot_info;   /* Reboot request from CLMS to CLMNA */
> } info;
>   } CLMSV_MSG;
>   
> diff --git a/osaf/libs/core/common/include/osaf_utility.h 
> b/osaf/libs/core/common/include/osaf_utility.h
> --- a/osaf/libs/core/common/include/osaf_utility.h
> +++ b/osaf/libs/core/common/include/osaf_utility.h
> @@ -24,6 +24,8 @@
>   #ifndef OPENSAF_CORE_OSAF_UTILITY_H_
>   #define OPENSAF_CORE_OSAF_UTILITY_H_
>   
> +#define USE_SAFE_REBOOT 1
AndersW> Maybe instead use:

enum {
   kUseSafeReboot = 1
};

> +
>   #include 
>   
>   #ifdef  __cplusplus
> @@ -68,6 +70,9 @@ extern void osaf_abort(long i_cause)
>   #endif
>   nothrow, noreturn));
>   
> +extern void osaf_safe_reboot()
> +__attribute__ ((nothrow));
> +
AndersW> Since this is C code, a function taking no parameter should be
declared "void osaf_safe_reboot(void)"
>   static inline void osaf_mutex_lock_ordie(pthread_mutex_t* io_mutex) {
> int result = pthread_mutex_lock(io_mutex);
> if (result != 0) osaf_abort(result); diff --git 
> a/osaf/libs/core/common/osaf_utility.c 
> b/osaf/libs/core/common/osaf_utility.c
> --- a/osaf/libs/core/common/osaf_utility.c
> +++ b/osaf/libs/core/common/osaf_utility.c
> @@ -16,9 +16,12 @@
>*/
>   
>   #include "osaf_utility.h"
> +#include "ncssysf_def.h"
> +#include "configmake.h"
AndersW> Include the project's header files after the system header
files (except for "osaf_utility.h" of course, which should be at the top).
>   #include 
>   #include 
>   #include 
> +#include 
>   
>   void osaf_abort(long i_cause)
>   {
> @@ -26,3 +29,22 @@ void osaf_abort(long i_cause)
>   i_cause, __builtin_return_address(0), errno);
>   abort();
>   }
> +
> +void osaf_safe_reboot()
> +{
> + char str[256];
> +
> + snprintf(str, sizeof(str), PKG

Re: [devel] [PATCH 1 of 1] smf: balanced upgrade software bundle missmatch [#2080]

2016-09-29 Thread Lennart Lund
ACK

Thanks
Lennart

> -Original Message-
> From: Rafael Odzakow
> Sent: den 29 september 2016 10:26
> To: reddy.neelaka...@oracle.com; Lennart Lund
> 
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: [PATCH 1 of 1] smf: balanced upgrade software bundle missmatch
> [#2080]
> 
>  osaf/services/saf/smfsv/smfd/SmfExecControl.cc |  9 +++--
>  1 files changed, 3 insertions(+), 6 deletions(-)
> 
> 
> When merging steps for the BALANCED_MODE execution there is a
> missmatch in how
> the software bundles are added to the new step. The problem will show
> when
> nodes have different software installed and not all of those nodes are
> included
> in the balanced list.
> 
> diff --git a/osaf/services/saf/smfsv/smfd/SmfExecControl.cc
> b/osaf/services/saf/smfsv/smfd/SmfExecControl.cc
> --- a/osaf/services/saf/smfsv/smfd/SmfExecControl.cc
> +++ b/osaf/services/saf/smfsv/smfd/SmfExecControl.cc
> @@ -103,13 +103,10 @@ bool createStepForBalancedProc(SmfUpgrad
>SmfCampaign* camp = SmfCampaignThread::instance()->campaign();
>SmfUpgradeCampaign* ucamp = camp->getUpgradeCampaign();
> 
> -  std::vector steps;
> -  for (auto step : getStepsMatchingBalancedGroup(procedure, ucamp)) {
> -// copy steps and callbacks
> +  auto steps = getStepsMatchingBalancedGroup(procedure, ucamp);
> +  for (auto step : steps) {
>  SmfUpgradeProcedure* oproc = step->getProcedure();
> -steps.insert(steps.end(),
> - oproc->getProcSteps().begin(),
> - oproc->getProcSteps().end());
> +// copy callbacks to the new procedure
>  procedure->getCallbackList(oproc->getUpgradeMethod());
>}
>if (!steps.empty()) {

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


[devel] [PATCH 1 of 1] smf: Recreate IMM handles if bad handle when deleting node group [#2046]

2016-09-29 Thread Lennart Lund
 osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc |  93 ++---
 osaf/services/saf/smfsv/smfd/SmfUpgradeStep.hh |   3 +-
 2 files changed, 67 insertions(+), 29 deletions(-)


Fix the deleteNodeGroup() method must be so that if the delete operation fails
with bad handles new handles are created so that the node group can be deleted

diff --git a/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc 
b/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc
--- a/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc
+++ b/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc
@@ -2823,7 +2823,7 @@ SmfAdminOperation::SmfAdminOperation(std
// and an immediate return is done
// The public methods shall return fail if m_creation_fail is true
 
-   bool rc = getAllImmHandles();
+   bool rc = initAllImmHandles();
if (rc == false) {
LOG_NO("%s: getAllImmHandles Fail", __FUNCTION__);
m_creation_fail = true;
@@ -2860,11 +2860,7 @@ SmfAdminOperation::SmfAdminOperation(std
 
 SmfAdminOperation::~SmfAdminOperation()
 {
-   // This frees all IMM handles that's based on the omHandle and
-   // give up admin ownership of Amf Cluster object
-   if (m_omHandle != 0) {
-   (void)immutil_saImmOmFinalize(m_omHandle);
-   }
+finalizeAllImmHandles();
 }
 
 ///
@@ -3079,7 +3075,7 @@ bool SmfAdminOperation::restart()
 /// Get all needed IMM handles. Updates corresponding member variables
 /// Return false if fail
 ///
-bool SmfAdminOperation::getAllImmHandles()
+bool SmfAdminOperation::initAllImmHandles()
 {
SaAisErrorT ais_rc = SA_AIS_ERR_TRY_AGAIN;
int timeout_try_cnt = 6;
@@ -3156,6 +3152,15 @@ bool SmfAdminOperation::getAllImmHandles
return rc;
 }
 
+// Free all IMM handles that's based on the omHandle and
+// give up admin ownership of Amf Cluster object
+void SmfAdminOperation::finalizeAllImmHandles() {
+   if (m_omHandle != 0) {
+   (void)immutil_saImmOmFinalize(m_omHandle);
+   }
+
+}
+
 /// Check if the AIS return is considered a campaign error
 /// Do not Fail if any of the following AIS codes
 ///
@@ -3552,6 +3557,8 @@ bool SmfAdminOperation::createNodeGroup(
 // A node group with the same name already exist
 // May happen if a previous delete after usage has
 // failed
+LOG_NO("%s: saImmOmCcbObjectCreate_2 Fail %s",
+   __FUNCTION__, saf_error(m_errno));
 bool rc = deleteNodeGroup();
 if (rc == false) {
 LOG_NO("%s: deleteNodeGroup() Fail",
@@ -3591,8 +3598,7 @@ bool SmfAdminOperation::createNodeGroup(
 ///
 bool SmfAdminOperation::deleteNodeGroup()
 {
-   bool rc = true;
-   m_errno = SA_AIS_OK;
+   SaAisErrorT ais_rc = SA_AIS_OK;
 
TRACE_ENTER();
std::string nodeGroupName_s =
@@ -3602,25 +3608,53 @@ bool SmfAdminOperation::deleteNodeGroup(
&nodeGroupName);
 
TRACE("\t Deleting nodeGroup '%s'", nodeGroupName_s.c_str());
-
-   m_errno = immutil_saImmOmCcbObjectDelete(m_ccbHandle,
-   &nodeGroupName);
-   if (m_errno != SA_AIS_OK) {
-   LOG_NO("%s: saImmOmCcbObjectDelete '%s' Fail %s",
-__FUNCTION__, nodeGroupName_s.c_str(),
-saf_error(m_errno));
-   rc = false;
-   } else {
-   m_errno = saImmOmCcbApply(m_ccbHandle);
-   if (m_errno != SA_AIS_OK) {
-   LOG_NO("%s: saImmOmCcbApply() Fail '%s'",
-   __FUNCTION__, saf_error(m_errno));
-   rc = false;
-   }
-   }
-
-   TRACE_LEAVE();
-   return rc;
+   bool method_rc = false;
+const uint32_t MAX_NO_RETRIES = 2;
+uint32_t retry_cnt = 0;
+while (++retry_cnt <= MAX_NO_RETRIES) {
+// Handles including ccb handle may have been invalidated by
+// IMM resulting in SA_AIS_ERR_BAD_HANDLE response on the
+// delete object request.
+// If that's the case: Try to create new handles and try again
+ais_rc = immutil_saImmOmCcbObjectDelete(m_ccbHandle,
+&nodeGroupName);
+TRACE("%s: immutil_saImmOmCcbObjectDelete %s",
+__FUNCTION__, saf_error(m_errno));
+
+if (ais_rc == SA_AIS_ERR_BAD_HANDLE) {
+LOG_NO("%s: saImmOmCcbObjectDelete Fail %s",
+   __FUNCTION__, saf_error(m_errno));
+finalizeAllImmHandles();
+bool rc = initAllImmHandles();
+if (rc == false) {
+LOG_NO("%s: getAllImmHandles() Fail",
+   __FUNCTION__);
+method_rc = false;
+

[devel] [PATCH 0 of 1] Review Request for smf: Recreate IMM handles if bad handle when deleting node group [#2046]

2016-09-29 Thread Lennart Lund
Summary: smf: Recreate IMM handles if bad handle when deleting node group
Review request for Trac Ticket(s): #2046
Peer Reviewer(s): reddy.neelaka...@oracle.com, rafael.odza...@ericsson.com
Pull request to: <>
Affected branch(es): <>
Development branch: <>


Impacted area   Impact y/n

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


Comments (indicate scope for each "y" above):
-
 <>

changeset 5092586880b8ee951502d59badb5d8c397b0916e
Author: Lennart Lund 
Date:   Thu, 29 Sep 2016 14:31:35 +0200

smf: Recreate IMM handles if bad handle when deleting node group [#2046]

Fix the deleteNodeGroup() method must be so that if the delete operation
fails with bad handles new handles are created so that the node group 
can be
deleted


Complete diffstat:
--
 osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc |  93 
+
 osaf/services/saf/smfsv/smfd/SmfUpgradeStep.hh |   3 +-
 2 files changed, 67 insertions(+), 29 deletions(-)


Testing Commands:
-
 <>


Testing, Expected Results:
--
 <>


Conditions of Submission:
-
Ack by reviewers


Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  n  n
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 ~/.hgrc file (i.e. username, 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 of 1] smf: Delete node group if already exist when creating [#2049]

2016-09-29 Thread Lennart Lund
 osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc |  76 -
 1 files changed, 49 insertions(+), 27 deletions(-)


If a node group for admin operation already exist when create is done the
existing group shall be deleted so that the new group can be created

diff --git a/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc 
b/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc
--- a/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc
+++ b/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc
@@ -57,15 +57,13 @@
 #include "smfd.h"
 #include "osaf_time.h"
 
-#if 0 /*LLDTEST*/
-#include "SmfExecControlHdl.h"
-#endif
-
 /* 
  *   DEFINITIONS
  * 
  */
 
+extern struct ImmutilWrapperProfile immutilWrapperProfile;
+
 /* 
  *   TYPE DEFINITIONS
  * 
@@ -81,8 +79,6 @@
  * 
  */
 
-#define LLDPROTO1 /* The prototype code */
-
 
//
 // Class SmfUpgradeStep
 // Purpose:
@@ -3450,7 +3446,6 @@ bool SmfAdminOperation::setNodeGroupPare
 ///
 bool SmfAdminOperation::createNodeGroup(SaAmfAdminStateT i_fromState)
 {
-   bool rc = true;
m_errno = SA_AIS_OK;
 
TRACE_ENTER();
@@ -3537,31 +3532,58 @@ bool SmfAdminOperation::createNodeGroup(
 
// 
// Create the node group
-   m_errno = immutil_saImmOmCcbObjectCreate_2(
-   m_ccbHandle,
-   className,
-   &nodeGroupParentDn,
-   attrValues);
-
-   if (m_errno != SA_AIS_OK) {
-   LOG_NO("%s: saImmOmCcbObjectCreate_2() '%s' Fail %s",
-   __FUNCTION__, nGnodeName, saf_error(m_errno));
-   rc = false;
-   } else {
-   m_errno = saImmOmCcbApply(m_ccbHandle);
-   if (m_errno != SA_AIS_OK) {
-   LOG_NO("%s: saImmOmCcbApply() Fail '%s'",
-   __FUNCTION__, saf_error(m_errno));
-   rc = false;
-   }
-   }
+   bool method_rc = false;
+const uint32_t MAX_NO_RETRIES = 2;
+uint32_t retry_cnt = 0;
+while (++retry_cnt <= MAX_NO_RETRIES) {
+// Creating the node group object will fail if a previously
+// created node group was for some reason not deleted
+// If that's the case (SA_AIS_ERR_EXIST) try to delete the
+// node group and try again.
+m_errno = immutil_saImmOmCcbObjectCreate_2(
+m_ccbHandle,
+className,
+&nodeGroupParentDn,
+attrValues);
+TRACE("%s: immutil_saImmOmCcbObjectCreate_2 %s",
+__FUNCTION__, saf_error(m_errno));
+
+if (m_errno == SA_AIS_ERR_EXIST) {
+// A node group with the same name already exist
+// May happen if a previous delete after usage has
+// failed
+bool rc = deleteNodeGroup();
+if (rc == false) {
+LOG_NO("%s: deleteNodeGroup() Fail",
+   __FUNCTION__);
+method_rc = false;
+break;
+}
+continue;
+} else if (m_errno != SA_AIS_OK) {
+LOG_NO("%s: saImmOmCcbObjectCreate_2() '%s' Fail %s",
+__FUNCTION__, nGnodeName, saf_error(m_errno));
+method_rc = false;
+break;
+} else {
+m_errno = saImmOmCcbApply(m_ccbHandle);
+if (m_errno != SA_AIS_OK) {
+LOG_NO("%s: saImmOmCcbApply() Fail '%s'",
+__FUNCTION__, saf_error(m_errno));
+method_rc = false;
+} else {
+method_rc = true;
+}
+break;
+}
+}
 
if (nodeName != NULL)
free(nodeName);
if (nodeNameList != NULL)
free(nodeNameList);
-   TRACE_LEAVE();
-   return rc;
+   TRACE_LEAVE2("rc %s", method_rc? "Ok":"Fail");
+   return method_rc;
 }
 
 /// Delete the SmfSetAdminState instance specific node group

--
___
Opensaf-devel mailing list
O

[devel] [PATCH 0 of 1] Review Request for smf: Delete node group if already exist when creating [#2049]

2016-09-29 Thread Lennart Lund
Summary: smf: Delete node group if already exist when creating
Review request for Trac Ticket(s): #2049
Peer Reviewer(s): reddy.neelaka...@oracle.com, rafael.odza...@ericsson.com
Pull request to: <>
Affected branch(es): <>
Development branch: <>


Impacted area   Impact y/n

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


Comments (indicate scope for each "y" above):
-
 <>

changeset c680b5ad6c35b1ffcdfa9e563f1f32b3f3cc8e36
Author: Lennart Lund 
Date:   Thu, 29 Sep 2016 14:23:33 +0200

smf: Delete node group if already exist when creating [#2049]

If a node group for admin operation already exist when create is done 
the
existing group shall be deleted so that the new group can be created


Complete diffstat:
--
 osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc |  76 
+---
 1 files changed, 49 insertions(+), 27 deletions(-)


Testing Commands:
-
 <>


Testing, Expected Results:
--
 <>


Conditions of Submission:
-
Ack from reviewers


Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  n  n
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 ~/.hgrc file (i.e. username, 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 of 1] clm: add support for cluster reboot V3 [#2053]

2016-09-29 Thread Anders Widell
Ack with comments.

One comment is that we ought to export this new admin op in the saClm.h 
file (or actually a new file separate file, like we have done when 
extending the SAF API in other services). I think we should also select 
a new number (i.e. 4) for this, even though it is targeted for a 
different object. So add e.g. the following line:

SA_CLM_ADMIN_CLUSTER_RESET = 4

In the future, we should consider how this feature interact with the 
enhanced cluster management feature that is on the roadmap. For now I 
think it is good enough, but in the future I would like to have a 
two-phase shutdown to avoid the possibility that some of the nodes 
reboot very quickly and potentially could come up before some other slow 
node has shut down.

See further code comments inline below, marked AndersW>

regards,

Anders Widell

On 09/28/2016 01:55 PM, Hans Nordeback wrote:
>   osaf/libs/common/clmsv/include/clmsv_msg.h   |   6 +++
>   osaf/libs/core/common/include/osaf_utility.h |   5 +++
>   osaf/libs/core/common/osaf_utility.c |  22 +
>   osaf/services/saf/clmsv/clms/clms.h  |   3 +-
>   osaf/services/saf/clmsv/clms/clms_imm.c  |  18 ++
>   osaf/services/saf/clmsv/clms/clms_mds.c  |  46 
> +++-
>   osaf/services/saf/clmsv/clms/clms_util.c |  12 +++
>   osaf/services/saf/clmsv/nodeagent/main.c |  12 +++
>   scripts/opensaf_reboot   |  22 ++---
>   9 files changed, 139 insertions(+), 7 deletions(-)
>
>
> Admin command to request cluster reboot:
> immadm -o 1 safCluster=myClmCluster
>
> diff --git a/osaf/libs/common/clmsv/include/clmsv_msg.h 
> b/osaf/libs/common/clmsv/include/clmsv_msg.h
> --- a/osaf/libs/common/clmsv/include/clmsv_msg.h
> +++ b/osaf/libs/common/clmsv/include/clmsv_msg.h
> @@ -23,6 +23,7 @@ typedef enum clms_msg_type {
> CLMSV_CLMS_TO_CLMA_CBK_MSG,
> CLMSV_CLMS_TO_CLMA_API_RESP_MSG,
> CLMSV_CLMS_TO_CLMA_IS_MEMBER_MSG,
> +  CLMSV_CLMS_TO_CLMNA_REBOOT_MSG,
> CLMSV_MSG_MAX
>   } CLMSV_MSG_TYPE;
>   
> @@ -174,6 +175,10 @@ typedef struct clmsv_is_member_info_t {
> SaUint32T client_id;
>   }CLMSV_IS_MEMBER_INFO;
>   
> +typedef struct clmsv_reboot_info_t {
> +  SaClmNodeIdT node_id;
> +} CLMSV_REBOOT_INFO;
> +
>   /* Top Level CLMSv MDS message structure for use between CLMS-> CLMA && 
> CLMA -> CLMS */
>   typedef struct clmsv_msg_t {
> struct clmsv_msg_t *next;   /* Mailbox processing */
> @@ -183,6 +188,7 @@ typedef struct clmsv_msg_t {
>   CLMSV_CBK_INFO cbk_info;/* Callback Messages from CLMS to CLA */
>   CLMSV_API_RESP_INFO api_resp_info;  /* Response Messages from CLMS 
> to CLA */
>   CLMSV_IS_MEMBER_INFO is_member_info;/*Is node member or not Message 
> from CLMS to CLA*/
> +CLMSV_REBOOT_INFO reboot_info;   /* Reboot request from CLMS to CLMNA */
> } info;
>   } CLMSV_MSG;
>   
> diff --git a/osaf/libs/core/common/include/osaf_utility.h 
> b/osaf/libs/core/common/include/osaf_utility.h
> --- a/osaf/libs/core/common/include/osaf_utility.h
> +++ b/osaf/libs/core/common/include/osaf_utility.h
> @@ -24,6 +24,8 @@
>   #ifndef OPENSAF_CORE_OSAF_UTILITY_H_
>   #define OPENSAF_CORE_OSAF_UTILITY_H_
>   
> +#define USE_SAFE_REBOOT 1
AndersW> Maybe instead use:

enum {
   kUseSafeReboot = 1
};

> +
>   #include 
>   
>   #ifdef  __cplusplus
> @@ -68,6 +70,9 @@ extern void osaf_abort(long i_cause)
>   #endif
>   nothrow, noreturn));
>   
> +extern void osaf_safe_reboot()
> +__attribute__ ((nothrow));
> +
AndersW> Since this is C code, a function taking no parameter should be 
declared "void osaf_safe_reboot(void)"
>   static inline void osaf_mutex_lock_ordie(pthread_mutex_t* io_mutex) {
> int result = pthread_mutex_lock(io_mutex);
> if (result != 0) osaf_abort(result);
> diff --git a/osaf/libs/core/common/osaf_utility.c 
> b/osaf/libs/core/common/osaf_utility.c
> --- a/osaf/libs/core/common/osaf_utility.c
> +++ b/osaf/libs/core/common/osaf_utility.c
> @@ -16,9 +16,12 @@
>*/
>   
>   #include "osaf_utility.h"
> +#include "ncssysf_def.h"
> +#include "configmake.h"
AndersW> Include the project's header files after the system header 
files (except for "osaf_utility.h" of course, which should be at the top).
>   #include 
>   #include 
>   #include 
> +#include 
>   
>   void osaf_abort(long i_cause)
>   {
> @@ -26,3 +29,22 @@ void osaf_abort(long i_cause)
>   i_cause, __builtin_return_address(0), errno);
>   abort();
>   }
> +
> +void osaf_safe_reboot()
> +{
> + char str[256];
> +
> + snprintf(str, sizeof(str), PKGLIBDIR "/opensaf_reboot %u %s %u", 0, 
> "not_used", USE_SAFE_REBOOT);
> + syslog(LOG_NOTICE, "Reboot ordered using command: %s", str);
> +
> + int rc = system(str);
> + if (rc < 0) {
AndersW> To strictly follow the man page for system(), we should check 
"if (rc == -1)" here, instead of "if (rc < 0)"
> + syslog(LOG_CRIT, "Node reboot failure: exit co

Re: [devel] [PATCH 0 of 1] Review Request for smf:retry of Admin operation for TIMEOUT in si-swap to be avoided [#2069]

2016-09-29 Thread Rafael Odzakow
ACK


On 09/28/2016 08:49 AM, reddy.neelaka...@oracle.com wrote:
> Summary: smf:retry of Admin operation for TIMEOUT in si-swap to be avoided 
> [#2069]
> Review request for Trac Ticket(s): 2069
> Peer Reviewer(s): Lennart, Rafel
> Affected branch(es): 5.0.x,5.1.x, default
> Development branch: default
>
> 
> Impacted area   Impact y/n
> 
>   Docsn
>   Build systemn
>   RPM/packaging   n
>   Configuration files n
>   Startup scripts n
>   SAF servicesy
>   OpenSAF servicesn
>   Core libraries  n
>   Samples n
>   Tests   n
>   Other   n
>
>
> Comments (indicate scope for each "y" above):
> -
>
> changeset a2bf2350146e8ad02496a5df5157466dd025760d
> Author:   Neelakanta Reddy
> Date: Wed, 28 Sep 2016 12:15:34 +0530
>
>   smf:retry of Admin operation for TIMEOUT in si-swap to be avoided 
> [#2069]
>
>   For the si-swap operation if TIMEOUT occurs the and the node is active 
> the
>   campaign will fail. If the node is standby SmfCampaignThread::instance()
>   will be terminated at Quiesced state, and nothing will be done.
>
>
> Complete diffstat:
> --
>   osaf/services/saf/smfsv/smfd/SmfUpgradeProcedure.cc |  11 ++-
>   1 files changed, 6 insertions(+), 5 deletions(-)
>
>
> Testing Commands:
> -
> when the camapign is ongoing, simulate a TIMEOUT for si-swap admin operation
>
> Testing, Expected Results:
> --
> with this patch, retry of si-swap is not supported.
>
> 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 ~/.hgrc file (i.e. username, 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 of 1] smf: Avoid flooding of syslog when the upgrade involves node reboot [#2058]

2016-09-29 Thread Lennart Lund
ACK

Thanks
Lennart

> -Original Message-
> From: reddy.neelaka...@oracle.com [mailto:reddy.neelaka...@oracle.com]
> Sent: den 29 september 2016 10:48
> To: Lennart Lund ; Rafael Odzakow
> 
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: [PATCH 1 of 1] smf: Avoid flooding of syslog when the upgrade
> involves node reboot [#2058]
> 
>  osaf/services/saf/smfsv/smfd/SmfUtils.cc |  2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> 
> If the upgrade involves node reboot, and the rebooted node joins late, then
> the sylog will be flodded with NO messages
> 
> diff --git a/osaf/services/saf/smfsv/smfd/SmfUtils.cc
> b/osaf/services/saf/smfsv/smfd/SmfUtils.cc
> --- a/osaf/services/saf/smfsv/smfd/SmfUtils.cc
> +++ b/osaf/services/saf/smfsv/smfd/SmfUtils.cc
> @@ -176,7 +176,7 @@ getNodeDestination(const std::string & i
>   }
>  }
>  free(nodeName);
> - LOG_NO("%s: className '%s'",__FUNCTION__, className);
> + TRACE("%s: className '%s'",__FUNCTION__, className);
>  } else {
>  LOG_NO("Unknown class name %s", className);
>  return false;

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


Re: [devel] [PATCH 1 of 1] smf:retry of Admin operation for TIMEOUT in si-swap to be avoided [#2069]

2016-09-29 Thread Lennart Lund
ACK

Thanks
Lennart

> -Original Message-
> From: reddy.neelaka...@oracle.com [mailto:reddy.neelaka...@oracle.com]
> Sent: den 28 september 2016 08:49
> To: Lennart Lund ; Rafael Odzakow
> 
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: [PATCH 1 of 1] smf:retry of Admin operation for TIMEOUT in si-swap
> to be avoided [#2069]
> 
>  osaf/services/saf/smfsv/smfd/SmfUpgradeProcedure.cc |  11 ++-
>  1 files changed, 6 insertions(+), 5 deletions(-)
> 
> 
> For the si-swap operation if TIMEOUT occurs the and the node is active the
> campaign will fail.
> If the node is standby SmfCampaignThread::instance() will be terminated at
> Quiesced state, and nothing will be done.
> 
> diff --git a/osaf/services/saf/smfsv/smfd/SmfUpgradeProcedure.cc
> b/osaf/services/saf/smfsv/smfd/SmfUpgradeProcedure.cc
> --- a/osaf/services/saf/smfsv/smfd/SmfUpgradeProcedure.cc
> +++ b/osaf/services/saf/smfsv/smfd/SmfUpgradeProcedure.cc
> @@ -4247,7 +4247,6 @@ SmfSwapThread::main(void)
>   int rc = admOp.execute(0);
>   while ((rc == SA_AIS_ERR_TRY_AGAIN) ||
>(rc == SA_AIS_ERR_BUSY) ||
> -  (rc == SA_AIS_ERR_TIMEOUT) ||
>(rc == SA_AIS_ERR_FAILED_OPERATION)) {
> 
>  if (retryCnt > max_swap_retry) {
> @@ -4255,12 +4254,11 @@ SmfSwapThread::main(void)
>  goto exit_error;
>  }
> 
> -if ((rc == SA_AIS_ERR_TIMEOUT) ||
> -(rc == SA_AIS_ERR_FAILED_OPERATION)) {
> -//A timeout or failed operation occur. It is 
> undefined if the
> operation was successful or not.
> +if (rc == SA_AIS_ERR_FAILED_OPERATION) {
> +//A failed operation occur. It is undefined if the 
> operation was
> successful or not.
>  //We wait for maximum two minutes to see if the 
> campaign
> thread is terminated (which it is in a successful swap)
>  //If not terminated, retry the SWAP operation.
> -LOG_NO("SA_AMF_ADMIN_SI_SWAP return
> SA_AIS_ERR_TIMEOUT or SA_AIS_ERR_FAILED_OPERATION [%d]. Wait for
> SmfCampaignThread to die, if not retry", rc);
> +LOG_NO("SA_AMF_ADMIN_SI_SWAP return
> SA_AIS_ERR_FAILED_OPERATION [%d]. Wait for SmfCampaignThread to die,
> if not retry", rc);
>  termCnt = 0;
>  while (SmfCampaignThread::instance() != NULL) {
>  if(termCnt >= 60) { //Wait for max 2 minutes 
> (60 * 2 sec)
> @@ -4271,6 +4269,8 @@ SmfSwapThread::main(void)
>  osaf_nanosleep(&sleepTime);
>  termCnt++;
>  }
> + goto exit_error;
> +
>  } else { //SA_AIS_ERR_TRY_AGAIN or SA_AIS_ERR_BUSY
>  LOG_NO("SA_AMF_ADMIN_SI_SWAP return
> SA_AIS_ERR_TRY_AGAIN or SA_AIS_ERR_BUSY [%d], wait 2 seconds and
> retry", rc);
>  struct timespec sleepTime = { 2, 0 };
> @@ -4284,6 +4284,7 @@ SmfSwapThread::main(void)
>  if (rc != SA_AIS_OK) {
>  //SA_AIS_ERR_LIBRARY, SA_AIS_ERR_BAD,_HANDLE
> SA_AIS_ERR_INIT, SA_AIS_ERR,_INVALID_PARAM,
> SA_AIS_ERR_NO_MEMORY
>  //SA_AIS_ERR_NO_RESOURCES, SA_AIS_ERR_BAD_OPERATION,
> SA_AIS_ERR_NOT_EXIST, SA_AIS_ERR_EXIST, SA_AIS_ERR_UNAVAILABLE
> + //SA_AIS_ERR_TIMEOUT
>  LOG_NO("Admin op SA_AMF_ADMIN_SI_SWAP fail [rc = %d]", rc);
>  goto exit_error;
>  }

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


Re: [devel] [PATCH 0 of 1] Review Request for smf: Avoid flooding of syslog when the upgrade involves node reboot [#2058]

2016-09-29 Thread Rafael Odzakow
ACK


On 09/29/2016 10:47 AM, reddy.neelaka...@oracle.com wrote:
> Summary: smf: Avoid flooding of syslog when the upgrade involves node reboot 
> [#2058]
> Review request for Trac Ticket(s): 2058
> Peer Reviewer(s): Rafel, Leenart
> Affected branch(es): 5.0.x, 5.1.x, default
> Development branch: default
>
> 
> Impacted area   Impact y/n
> 
>   Docsn
>   Build systemn
>   RPM/packaging   n
>   Configuration files n
>   Startup scripts n
>   SAF servicesn
>   OpenSAF servicesn
>   Core libraries  n
>   Samples n
>   Tests   n
>   Other   n
>
>
> Comments (indicate scope for each "y" above):
> -
>
> changeset 8f7d6e822b28030929e1d6e906b603b5ccd1b758
> Author:   Neelakanta Reddy
> Date: Thu, 29 Sep 2016 14:13:48 +0530
>
>   smf: Avoid flooding of syslog when the upgrade involves node reboot 
> [#2058]
>   If the upgrade involves node reboot, and the rebooted node joins late, 
> then
>   the sylog will be flodded with NO messages
>
>
> Complete diffstat:
> --
>   osaf/services/saf/smfsv/smfd/SmfUtils.cc |  2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
>
> Testing Commands:
> -
>
>
> Testing, Expected Results:
> --
> If node, joints late after restart at the time of upgrade, then syslog should 
> not be flooded with messages.
>
> 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 ~/.hgrc file (i.e. username, 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 of 1] cpsv: remove longDnsAllowed checking each checkpoint creating time [#2068] V3

2016-09-29 Thread Anders Widell
Ack, not tested.

/ Anders Widell


On 09/29/2016 10:15 AM, Hoang Vo wrote:
>   osaf/libs/common/cpsv/include/cpnd_evt.h.old |   0
>   osaf/libs/common/cpsv/include/cpnd_init.h|   1 -
>   osaf/services/saf/cpsv/cpd/cpd_db.c  |   3 +++
>   osaf/services/saf/cpsv/cpd/cpd_evt.c |   4 +++-
>   osaf/services/saf/cpsv/cpnd/cpnd_evt.c   |  12 
>   osaf/services/saf/cpsv/cpnd/cpnd_proc.c  |  25 -
>   6 files changed, 6 insertions(+), 39 deletions(-)
>
>
> Problem:
> Statistically the check point create time for SC and PL (sync and async) has
> degradation more than 30% after bring in patch 8004
>
> Solution:
> Remove unnecessary checking that cost time. imm will take the role of checking
>
> diff --git a/osaf/libs/common/cpsv/include/cpnd_evt.h.old 
> b/osaf/libs/common/cpsv/include/cpnd_evt.h.old
> deleted file mode 100644
> diff --git a/osaf/libs/common/cpsv/include/cpnd_init.h 
> b/osaf/libs/common/cpsv/include/cpnd_init.h
> --- a/osaf/libs/common/cpsv/include/cpnd_init.h
> +++ b/osaf/libs/common/cpsv/include/cpnd_init.h
> @@ -130,7 +130,6 @@ uint32_t cpnd_all_repl_rsp_expiry(CPND_C
>   uint32_t cpnd_open_active_sync_expiry(CPND_CB *cb, CPND_TMR_INFO *tmr_info);
>   void cpnd_proc_free_read_data(CPSV_EVT *evt);
>   SaUint32T cpnd_get_scAbsenceAllowed_attr();
> -SaUint32T cpnd_get_longDnsAllowed_attr();
>   /* End cpnd_proc.c */
>   
>   /* File : ---  cpnd_amf.c */
> diff --git a/osaf/services/saf/cpsv/cpd/cpd_db.c 
> b/osaf/services/saf/cpsv/cpd/cpd_db.c
> --- a/osaf/services/saf/cpsv/cpd/cpd_db.c
> +++ b/osaf/services/saf/cpsv/cpd/cpd_db.c
> @@ -106,6 +106,9 @@ uint32_t cpd_ckpt_node_add(NCS_PATRICIA_
>   err = create_runtime_ckpt_object(ckpt_node, immOiHandle);
>   if (err != SA_AIS_OK) {
>   LOG_ER("create runtime ckpt object failed with error: 
> %u",err);
> + if (err == SA_AIS_ERR_INVALID_PARAM) {
> + return NCSCC_RC_INVALID_INPUT;
> + }
>   return NCSCC_RC_FAILURE;
>   }
>   }
> diff --git a/osaf/services/saf/cpsv/cpd/cpd_evt.c 
> b/osaf/services/saf/cpsv/cpd/cpd_evt.c
> --- a/osaf/services/saf/cpsv/cpd/cpd_evt.c
> +++ b/osaf/services/saf/cpsv/cpd/cpd_evt.c
> @@ -238,9 +238,11 @@ static uint32_t cpd_evt_proc_ckpt_create
>   rc = SA_AIS_ERR_NO_MEMORY;
>   goto send_rsp;
>   } else if (proc_rc != NCSCC_RC_SUCCESS) {
> -
>   TRACE_4("cpd ckpt create failure ckpt name,dest :  %s, 
> %"PRIu64, ckpt_name, sinfo->dest);
>   rc = SA_AIS_ERR_LIBRARY;
> + if (proc_rc == NCSCC_RC_INVALID_INPUT) {
> + rc = SA_AIS_ERR_INVALID_PARAM;
> + }
>   goto send_rsp;
>   }
>   
> diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_evt.c 
> b/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
> --- a/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
> +++ b/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
> @@ -605,12 +605,6 @@ static uint32_t cpnd_evt_proc_ckpt_open(
>   TRACE_ENTER();
>   memset(&send_evt, '\0', sizeof(CPSV_EVT));
>   
> - if ((cpnd_get_longDnsAllowed_attr() == 0) && 
> osaf_is_an_extended_name(&evt->info.openReq.ckpt_name)) {
> - LOG_ER("cpnd - longDnsAllowed == false - NOT supporting 
> extended name");
> - send_evt.info.cpa.info.openRsp.error = SA_AIS_ERR_INVALID_PARAM;
> - goto agent_rsp;
> - }
> -
>   if (!cpnd_is_cpd_up(cb)) {
>   send_evt.info.cpa.info.openRsp.error = SA_AIS_ERR_TRY_AGAIN;
>   goto agent_rsp;
> @@ -1137,12 +1131,6 @@ static uint32_t cpnd_evt_proc_ckpt_unlin
>   TRACE_ENTER();
>   memset(&send_evt, '\0', sizeof(CPSV_EVT));
>   
> - if ((cpnd_get_longDnsAllowed_attr() == 0) && 
> osaf_is_an_extended_name(&evt->info.ulinkReq.ckpt_name)) {
> - LOG_ER("cpnd - longDnsAllowed == false - NOT supporting 
> extended name");
> - send_evt.info.cpa.info.ulinkRsp.error = 
> SA_AIS_ERR_INVALID_PARAM;
> - goto agent_rsp;
> - }
> -
>   if (!cpnd_is_cpd_up(cb)) {
>   send_evt.info.cpa.info.ulinkRsp.error = SA_AIS_ERR_TRY_AGAIN;
>   goto agent_rsp;
> diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_proc.c 
> b/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
> --- a/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
> +++ b/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
> @@ -2735,31 +2735,6 @@ SaUint32T cpnd_get_scAbsenceAllowed_attr
>   }
>   
>   
> /
> - * Name  : cpnd_get_longDnsAllowed_attr()
> - *
> - * Description   : This function gets scAbsenceAllowed attribute
> - *
> - * Arguments : -
> - *
> - * Return Values : scAbsenceAllowed attribute (0 = not allowed)
> - 
> */
> -SaUint32T cpnd_get_longDnsAllowe

[devel] [PATCH 0 of 1] Review Request for amfnd: honour max num of retries for comp instantiation during repair[#2059].

2016-09-29 Thread praveen . malviya
Summary: amfnd: honour max num of retries for comp instantiation during 
repair[#2059]. 
Review request for Trac Ticket(s): #2059 
Peer Reviewer(s): AMF devs 
Pull request to: <>
Affected branch(es): ALL 
Development branch: <>


Impacted area   Impact y/n

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


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

changeset 2b5e4adfe681aa97a9d97e101c0f3b35f2d5864b
Author: praveen.malv...@oracle.com
Date:   Thu, 29 Sep 2016 15:34:49 +0530

amfnd: honour max num of retries for comp instantiation during
repair[#2059].

SU stuck in TERMINATING state when comp fails to instantiate as part of
repair. Repair was performed after comp-failover recovery.

Comp faults with comp-failover recovery. After removal of assignments,
AMFNFD tries to repair component by trying to instantiate it. Since
instantiate scripit is not present, comp instantiation fails and AMFND
cleans up it succesfully. After successful cleanup AMFND niether tries 
to
instantiate comp again not it marks comp and SU to INST_FAILED state. 
AMFND
should mark the comp and SU to INST_FAILED state after finishing
instantiation of comp MAX_TRY times. If the value is not configured for 
the
comp then AMFND should honour the default value of
saAmfNumMaxInstantiateWithoutDelay=2.

Patch honours saAmfNumMaxInstantiateWithoutDelay and if comp 
instantiation
fails after these many retries then both comp and SU moves to 
INST_FAILED
state.


Complete diffstat:
--
 osaf/services/saf/amf/amfnd/clc.cc |  8 
 1 files changed, 8 insertions(+), 0 deletions(-)


Testing Commands:
-
Tested as per description.

Testing, Expected Results:
--
Both comp and SU moves to INST_FAILED state after max num of retries.

Conditions of Submission:
-
Ack from any reviewer.

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 ~/.hgrc file (i.e. username, 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.


-

[devel] [PATCH 1 of 1] amfnd: honour max num of retries for comp instantiation during repair[#2059]

2016-09-29 Thread praveen . malviya
 osaf/services/saf/amf/amfnd/clc.cc |  8 
 1 files changed, 8 insertions(+), 0 deletions(-)


SU stuck in TERMINATING state when comp fails to instantiate as part of repair.
Repair was performed after comp-failover recovery.

Comp faults with comp-failover recovery. After removal of assignments, AMFNFD
tries to repair component by trying to instantiate it. Since instantiate scripit
is not present, comp instantiation fails and AMFND cleans up it succesfully. 
After
successful cleanup AMFND niether tries to instantiate comp again not it marks 
comp
and SU to INST_FAILED state. AMFND should mark the comp and SU to INST_FAILED 
state
after finishing instantiation of comp MAX_TRY times. If the value is not 
configured
for the comp then AMFND should honour the default value of 
saAmfNumMaxInstantiateWithoutDelay=2.

Patch honours saAmfNumMaxInstantiateWithoutDelay and if comp instantiation 
fails after
these many retries then both comp and SU moves to INST_FAILED state.

diff --git a/osaf/services/saf/amf/amfnd/clc.cc 
b/osaf/services/saf/amf/amfnd/clc.cc
--- a/osaf/services/saf/amf/amfnd/clc.cc
+++ b/osaf/services/saf/amf/amfnd/clc.cc
@@ -1785,6 +1785,14 @@ static bool is_failed_comp_eligible_for_
 /*A failed component is eligible for instantiation 
in the context of comp-restart recovery.*/ 
return true;
+} else {//SU is failed.
+/*As a part of repair comp instantiation failed
+  and it was cleaned up again.*/
+if (m_AVND_COMP_IS_FAILED(comp) && !comp->csi_list.n_nodes &&
+(!m_AVND_SU_IS_ADMN_TERM(comp->su)) &&
+(avnd_cb->oper_state == SA_AMF_OPERATIONAL_ENABLED) && 
+(!m_AVND_IS_SHUTTING_DOWN(avnd_cb)) && 
!sufailover_in_progress(comp->su))
+   return true;
 }
   }
 

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


[devel] [PATCH 0 of 1] Review Request for imm: Set validation abort error string when OI returns ERR_BAD_OPERATION or ERR_FAILED_OPERATION [#1650]

2016-09-29 Thread Hung Nguyen
Summary: imm: Set validation abort error string when OI returns 
ERR_BAD_OPERATION or ERR_FAILED_OPERATION [#1650]
Review request for Trac Ticket(s): 1650
Peer Reviewer(s): Zoran, Neel
Pull request to:
Affected branch(es): 5.0, 5.1, 5.2
Development branch: 5.2


Impacted area   Impact y/n

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


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


changeset 97603e4263e470fae5ebbbe3a2a064b32033159b
Author: Hung Nguyen 
Date:   Thu, 29 Sep 2016 16:22:03 +0700

imm: Set validation abort error string when OI returns 
ERR_BAD_OPERATION or
ERR_FAILED_OPERATION [#1650]

Set validation abort error string when OI returns ERR_BAD_OPERATION or
ERR_FAILED_OPERATION. This is to prevent om clients from retrying the
operations that are already rejected by OI.


Complete diffstat:
--
 osaf/services/saf/immsv/immnd/ImmModel.cc |  23 ---
 osaf/services/saf/immsv/immnd/immnd_evt.c |  74 
+-
 2 files changed, 37 insertions(+), 60 deletions(-)


Testing Commands:
-



Testing, Expected Results:
--



Conditions of Submission:
-
Ack from reviewers.


Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  n  n
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 ~/.hgrc file (i.e. username, 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 of 1] imm: Set validation abort error string when OI returns ERR_BAD_OPERATION or ERR_FAILED_OPERATION [#1650]

2016-09-29 Thread Hung Nguyen
 osaf/services/saf/immsv/immnd/ImmModel.cc |  23 -
 osaf/services/saf/immsv/immnd/immnd_evt.c |  74 +++---
 2 files changed, 37 insertions(+), 60 deletions(-)


Set validation abort error string when OI returns ERR_BAD_OPERATION or 
ERR_FAILED_OPERATION.
This is to prevent om clients from retrying the operations that are already 
rejected by OI.

diff --git a/osaf/services/saf/immsv/immnd/ImmModel.cc 
b/osaf/services/saf/immsv/immnd/ImmModel.cc
--- a/osaf/services/saf/immsv/immnd/ImmModel.cc
+++ b/osaf/services/saf/immsv/immnd/ImmModel.cc
@@ -10497,6 +10497,11 @@ ImmModel::deleteObject(ObjectMap::iterat
 
 void
 ImmModel::setCcbErrorString(CcbInfo *ccb, const char *errorString, va_list vl) 
{
+/* Only set error strings on node where the OM client resides */
+if (!ccb->mOriginatingConn && !(ccb->mAugCcbParent && 
ccb->mAugCcbParent->mOriginatingConn)) {
+return;
+}
+
 int errLen = strlen(errorString) + 1;
 char *fmtError = (char *)malloc(errLen);
 int len;
@@ -10872,7 +10877,11 @@ ImmModel::ccbObjDelContinuation(immsv_oi
 "implementer returned error, Ccb aborted with error: %u",
  rsp->result);
 ccb->mVeto = SA_AIS_ERR_FAILED_OPERATION;
-setCcbErrorString(ccb, IMM_VALIDATION_ABORT "Implementer 
returned error: ", rsp->result);
+if (rsp->result == SA_AIS_ERR_BAD_OPERATION || rsp->result == 
SA_AIS_ERR_FAILED_OPERATION) {
+setCcbErrorString(ccb, IMM_VALIDATION_ABORT "Implementer 
returned error: %u", rsp->result);
+} else {
+setCcbErrorString(ccb, IMM_RESOURCE_ABORT "Implementer 
returned error: %u", rsp->result);
+}
 //TODO: This is perhaps more drastic than the specification
 //demands. We are here aborting the entire Ccb, whereas the 
spec
 //seems to allow for a non-ok returnvalue from implementer 
@@ -11150,7 +11159,11 @@ ImmModel::ccbObjCreateContinuation(SaUin
 "implementer returned error, Ccb aborted with error: %u",
 error);
 ccb->mVeto = SA_AIS_ERR_FAILED_OPERATION;
-setCcbErrorString(ccb, IMM_VALIDATION_ABORT "Implementer returned 
error: %u", error);
+if (error == SA_AIS_ERR_BAD_OPERATION || error == 
SA_AIS_ERR_FAILED_OPERATION) {
+setCcbErrorString(ccb, IMM_VALIDATION_ABORT "Implementer returned 
error: %u", error);
+} else {
+setCcbErrorString(ccb, IMM_RESOURCE_ABORT "Implementer returned 
error: %u", error);
+}
 //TODO: This is perhaps more drastic than the specification demands.
 //We are here aborting the entire Ccb, whereas the spec seems to allow
 //for a non-ok returnvalue from implementer (in this callback) to
@@ -11240,7 +11253,11 @@ ImmModel::ccbObjModifyContinuation(SaUin
 LOG_IN("ImmModel::ccbObjModifyContinuation: "
 "implementer returned error, Ccb aborted with error: %u", error);
 ccb->mVeto = SA_AIS_ERR_FAILED_OPERATION;
-setCcbErrorString(ccb, IMM_VALIDATION_ABORT "Implementer returned 
error: %u", error);
+if (error == SA_AIS_ERR_BAD_OPERATION || error == 
SA_AIS_ERR_FAILED_OPERATION) {
+setCcbErrorString(ccb, IMM_VALIDATION_ABORT "Implementer returned 
error: %u", error);
+} else {
+setCcbErrorString(ccb, IMM_RESOURCE_ABORT "Implementer returned 
error: %u", error);
+}
 //TODO: This is perhaps more drastic than the specification demands.
 //We are here aborting the entire Ccb, whereas the spec seems to allow
 //for a non-ok returnvalue from implementer (in this callback) to
diff --git a/osaf/services/saf/immsv/immnd/immnd_evt.c 
b/osaf/services/saf/immsv/immnd/immnd_evt.c
--- a/osaf/services/saf/immsv/immnd/immnd_evt.c
+++ b/osaf/services/saf/immsv/immnd/immnd_evt.c
@@ -3742,41 +3742,21 @@ static void immnd_evt_proc_ccb_obj_modif
send_evt.type = IMMSV_EVT_TYPE_IMMA;
send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR;
IMMSV_ATTR_NAME_LIST strList;
-   IMMSV_ATTR_NAME_LIST errStrList = { { 0 }, NULL };
-   
+   IMMSV_ATTR_NAME_LIST* errStrList = 
immModel_ccbGrabErrStrings(cb, evt->info.ccbUpcallRsp.ccbId);
+
if (evt->info.ccbUpcallRsp.result != SA_AIS_OK) {
-   if(evt->info.ccbUpcallRsp.result != 
SA_AIS_ERR_FAILED_OPERATION) {
-   char buf[2];
-   int size;
-
-   /* Create error string */
-   size = snprintf(buf, 1,
-   IMM_RESOURCE_ABORT "Upcall 
failed with error code: %u",
-   evt->info.ccbUpcallRsp.result);
-   osafassert(size >= 0);
-   

[devel] [PATCH 1 of 1] smf: Avoid flooding of syslog when the upgrade involves node reboot [#2058]

2016-09-29 Thread reddy . neelakanta
 osaf/services/saf/smfsv/smfd/SmfUtils.cc |  2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


If the upgrade involves node reboot, and the rebooted node joins late, then the 
sylog will be flodded with NO messages

diff --git a/osaf/services/saf/smfsv/smfd/SmfUtils.cc 
b/osaf/services/saf/smfsv/smfd/SmfUtils.cc
--- a/osaf/services/saf/smfsv/smfd/SmfUtils.cc
+++ b/osaf/services/saf/smfsv/smfd/SmfUtils.cc
@@ -176,7 +176,7 @@ getNodeDestination(const std::string & i
}
 }
 free(nodeName);
-   LOG_NO("%s: className '%s'",__FUNCTION__, className);
+   TRACE("%s: className '%s'",__FUNCTION__, className);
 } else {
 LOG_NO("Unknown class name %s", className);
 return false;

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


[devel] [PATCH 0 of 1] Review Request for smf: Avoid flooding of syslog when the upgrade involves node reboot [#2058]

2016-09-29 Thread reddy . neelakanta
Summary: smf: Avoid flooding of syslog when the upgrade involves node reboot 
[#2058]
Review request for Trac Ticket(s): 2058
Peer Reviewer(s): Rafel, Leenart
Affected branch(es): 5.0.x, 5.1.x, default
Development branch: default


Impacted area   Impact y/n

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


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

changeset 8f7d6e822b28030929e1d6e906b603b5ccd1b758
Author: Neelakanta Reddy
Date:   Thu, 29 Sep 2016 14:13:48 +0530

smf: Avoid flooding of syslog when the upgrade involves node reboot 
[#2058]
If the upgrade involves node reboot, and the rebooted node joins late, 
then
the sylog will be flodded with NO messages


Complete diffstat:
--
 osaf/services/saf/smfsv/smfd/SmfUtils.cc |  2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


Testing Commands:
-


Testing, Expected Results:
--
If node, joints late after restart at the time of upgrade, then syslog should 
not be flooded with messages.

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 ~/.hgrc file (i.e. username, 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 0 of 1] Review Request for imm: Set validation abort error string when OI returns ERR_BAD_OPERATION or ERR_FAILED_OPERATION [#1650]

2016-09-29 Thread Hung Nguyen
Hi,

Please ignore this patch, I will send new one for review.

BR,

Hung Nguyen - DEK Technologies



From: Hung Nguyen hung.d.ngu...@dektech.com.au
Sent: Thursday, September 29, 2016 2:48PM
To: Zoran Milinkovic, Neelakanta Reddy
 zoran.milinko...@ericsson.com, reddy.neelaka...@oracle.com
Cc: Opensaf-devel
 opensaf-devel@lists.sourceforge.net
Subject: [devel] [PATCH 0 of 1] Review Request for imm: Set validation abort 
error string when OI returns ERR_BAD_OPERATION or ERR_FAILED_OPERATION [#1650]


Summary: imm: Set validation abort error string when OI returns 
ERR_BAD_OPERATION or ERR_FAILED_OPERATION [#1650]
Review request for Trac Ticket(s): 1650
Peer Reviewer(s): Zoran, Neel
Pull request to:
Affected branch(es): 5.0, 5.1, 5.2
Development branch: 5.2


Impacted area   Impact y/n

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


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


changeset c3d087ae2dcb8a6aa9b1e6fc79922ef8d70cfbe2
Author: Hung Nguyen 
Date:   Thu, 29 Sep 2016 11:15:50 +0700

imm: Set validation abort error string when OI returns 
ERR_BAD_OPERATION or
ERR_FAILED_OPERATION [#1650]

Set validation abort error string when OI returns ERR_BAD_OPERATION or
ERR_FAILED_OPERATION. This is to prevent om clients from retrying the
operations that are already rejected by OI.


Complete diffstat:
--
  osaf/services/saf/immsv/immnd/ImmModel.cc |  18 +++---
  osaf/services/saf/immsv/immnd/immnd_evt.c |  82 
++
  2 files changed, 37 insertions(+), 63 deletions(-)


Testing Commands:
-



Testing, Expected Results:
--



Conditions of Submission:
-
Ack from reviewers.


Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  n  n
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 ~/.hgrc file (i.e. username, 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.


--
___

[devel] [PATCH 1 of 1] amfd: assert in avd_compcsi_cleanup_imm_object [#2081]

2016-09-29 Thread Hans Nordeback
 osaf/services/saf/amf/amfd/csi.cc |  4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)


Causes cyclic reboot of SC-1 during resilience testing

diff --git a/osaf/services/saf/amf/amfd/csi.cc 
b/osaf/services/saf/amf/amfd/csi.cc
--- a/osaf/services/saf/amf/amfd/csi.cc
+++ b/osaf/services/saf/amf/amfd/csi.cc
@@ -1585,8 +1585,10 @@ void avd_compcsi_cleanup_imm_object(AVD_
SaNameT comp_name;
avsv_sanamet_init_from_association_dn(&dn, &comp_name, 
"safComp", csi->name.c_str());
AVD_COMP *comp = comp_db->find(Amf::to_string(&comp_name));
+   if (comp == nullptr) {
+   LOG_WA("Component %s not found in comp_db", 
osaf_extended_name_borrow(&comp_name));
+   }
osaf_extended_name_free(&comp_name);
-   osafassert(comp);
 
susi = avd_susi_find(avd_cb, comp->su->name, si->name);
if (susi == nullptr || (susi->fsm == AVD_SU_SI_STATE_ABSENT)) {

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


[devel] [PATCH 0 of 1] Review Request for amfd: assert in avd_compcsi_cleanup_imm_object [#2081]

2016-09-29 Thread Hans Nordeback
Summary: amfd: assert in avd_compcsi_cleanup_imm_object 
Review request for Trac Ticket(s): #2081
Peer Reviewer(s): Minh, Praveen, Nagu
Pull request to: 
Affected branch(es): default
Development branch: default


Impacted area   Impact y/n

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


Comments (indicate scope for each "y" above):
-
 <>

changeset 57035b7b03e5948709a3134d210139a66e0bafb9
Author: Hans Nordeback 
Date:   Thu, 29 Sep 2016 10:26:26 +0200

amfd: assert in avd_compcsi_cleanup_imm_object [#2081]

Causes cyclic reboot of SC-1 during resilience testing


Complete diffstat:
--
 osaf/services/saf/amf/amfd/csi.cc |  4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)


Testing Commands:
-
 <>


Testing, Expected Results:
--
 <>


Conditions of Submission:
-
 <>


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 ~/.hgrc file (i.e. username, 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 of 1] amfd: assert in avd_compcsi_cleanup_imm_object [#2081]

2016-09-29 Thread Hans Nordeback
 osaf/services/saf/amf/amfd/csi.cc |  4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)


Causes cyclic reboot of SC-1 during resilience testing

diff --git a/osaf/services/saf/amf/amfd/csi.cc 
b/osaf/services/saf/amf/amfd/csi.cc
--- a/osaf/services/saf/amf/amfd/csi.cc
+++ b/osaf/services/saf/amf/amfd/csi.cc
@@ -1585,8 +1585,10 @@ void avd_compcsi_cleanup_imm_object(AVD_
SaNameT comp_name;
avsv_sanamet_init_from_association_dn(&dn, &comp_name, 
"safComp", csi->name.c_str());
AVD_COMP *comp = comp_db->find(Amf::to_string(&comp_name));
+   if (comp == nullptr) {
+   LOG_WA("Component %s not found in comp_db", 
osaf_extended_name_borrow(&comp_name));
+   }
osaf_extended_name_free(&comp_name);
-   osafassert(comp);
 
susi = avd_susi_find(avd_cb, comp->su->name, si->name);
if (susi == nullptr || (susi->fsm == AVD_SU_SI_STATE_ABSENT)) {

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


[devel] [PATCH 0 of 1] Review Request for #2081 amfd: assert in avd_compcsi_cleanup_imm_object

2016-09-29 Thread Hans Nordeback
Summary: amfd: assert in avd_compcsi_cleanup_imm_object
Review request for Trac Ticket(s): #2081
Peer Reviewer(s): Minh, Praveen, Nagu
Pull request to: 
Affected branch(es): default
Development branch: default


Impacted area   Impact y/n

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


Comments (indicate scope for each "y" above):
-
 <>

changeset 57035b7b03e5948709a3134d210139a66e0bafb9
Author: Hans Nordeback 
Date:   Thu, 29 Sep 2016 10:26:26 +0200

amfd: assert in avd_compcsi_cleanup_imm_object [#2081]

Causes cyclic reboot of SC-1 during resilience testing


Complete diffstat:
--
 osaf/services/saf/amf/amfd/csi.cc |  4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)


Testing Commands:
-
 <>


Testing, Expected Results:
--
 <>


Conditions of Submission:
-
 <>


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 ~/.hgrc file (i.e. username, 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 of 1] Review Request for smf: balanced upgrade software bundle missmatch [#2080]

2016-09-29 Thread Rafael Odzakow
Summary: smf: balanced upgrade software bundle missmatch [#2080]
Review request for Trac Ticket(s): #2080
Peer Reviewer(s): lennart, reddy
Pull request to: <>
Affected branch(es): 5.1.x
Development branch: <>


Impacted area   Impact y/n

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


Comments (indicate scope for each "y" above):
-
 <>

changeset 12f0f4b6bc10533a310a879ad68e490a288c3bb8
Author: Rafael Odzakow 
Date:   Thu, 29 Sep 2016 10:08:42 +0200

smf: balanced upgrade software bundle missmatch [#2080]

When merging steps for the BALANCED_MODE execution there is a missmatch 
in
how the software bundles are added to the new step. The problem will 
show
when nodes have different software installed and not all of those nodes 
are
included in the balanced list.


Complete diffstat:
--
 osaf/services/saf/smfsv/smfd/SmfExecControl.cc |  9 +++--
 1 files changed, 3 insertions(+), 6 deletions(-)


Testing Commands:
-
 <>


Testing, Expected Results:
--
 <>


Conditions of Submission:
-
 <>


Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  n  n
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 ~/.hgrc file (i.e. username, 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 of 1] smf: balanced upgrade software bundle missmatch [#2080]

2016-09-29 Thread Rafael Odzakow
 osaf/services/saf/smfsv/smfd/SmfExecControl.cc |  9 +++--
 1 files changed, 3 insertions(+), 6 deletions(-)


When merging steps for the BALANCED_MODE execution there is a missmatch in how
the software bundles are added to the new step. The problem will show when
nodes have different software installed and not all of those nodes are included
in the balanced list.

diff --git a/osaf/services/saf/smfsv/smfd/SmfExecControl.cc 
b/osaf/services/saf/smfsv/smfd/SmfExecControl.cc
--- a/osaf/services/saf/smfsv/smfd/SmfExecControl.cc
+++ b/osaf/services/saf/smfsv/smfd/SmfExecControl.cc
@@ -103,13 +103,10 @@ bool createStepForBalancedProc(SmfUpgrad
   SmfCampaign* camp = SmfCampaignThread::instance()->campaign();
   SmfUpgradeCampaign* ucamp = camp->getUpgradeCampaign();
 
-  std::vector steps;
-  for (auto step : getStepsMatchingBalancedGroup(procedure, ucamp)) {
-// copy steps and callbacks
+  auto steps = getStepsMatchingBalancedGroup(procedure, ucamp);
+  for (auto step : steps) {
 SmfUpgradeProcedure* oproc = step->getProcedure();
-steps.insert(steps.end(),
- oproc->getProcSteps().begin(),
- oproc->getProcSteps().end());
+// copy callbacks to the new procedure
 procedure->getCallbackList(oproc->getUpgradeMethod());
   }
   if (!steps.empty()) {

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


[devel] [PATCH 1 of 1] cpsv: remove longDnsAllowed checking each checkpoint creating time [#2068] V3

2016-09-29 Thread Hoang Vo
 osaf/libs/common/cpsv/include/cpnd_evt.h.old |   0 
 osaf/libs/common/cpsv/include/cpnd_init.h|   1 -
 osaf/services/saf/cpsv/cpd/cpd_db.c  |   3 +++
 osaf/services/saf/cpsv/cpd/cpd_evt.c |   4 +++-
 osaf/services/saf/cpsv/cpnd/cpnd_evt.c   |  12 
 osaf/services/saf/cpsv/cpnd/cpnd_proc.c  |  25 -
 6 files changed, 6 insertions(+), 39 deletions(-)


Problem:
Statistically the check point create time for SC and PL (sync and async) has
degradation more than 30% after bring in patch 8004

Solution:
Remove unnecessary checking that cost time. imm will take the role of checking

diff --git a/osaf/libs/common/cpsv/include/cpnd_evt.h.old 
b/osaf/libs/common/cpsv/include/cpnd_evt.h.old
deleted file mode 100644
diff --git a/osaf/libs/common/cpsv/include/cpnd_init.h 
b/osaf/libs/common/cpsv/include/cpnd_init.h
--- a/osaf/libs/common/cpsv/include/cpnd_init.h
+++ b/osaf/libs/common/cpsv/include/cpnd_init.h
@@ -130,7 +130,6 @@ uint32_t cpnd_all_repl_rsp_expiry(CPND_C
 uint32_t cpnd_open_active_sync_expiry(CPND_CB *cb, CPND_TMR_INFO *tmr_info);
 void cpnd_proc_free_read_data(CPSV_EVT *evt);
 SaUint32T cpnd_get_scAbsenceAllowed_attr();
-SaUint32T cpnd_get_longDnsAllowed_attr();
 /* End cpnd_proc.c */
 
 /* File : ---  cpnd_amf.c */
diff --git a/osaf/services/saf/cpsv/cpd/cpd_db.c 
b/osaf/services/saf/cpsv/cpd/cpd_db.c
--- a/osaf/services/saf/cpsv/cpd/cpd_db.c
+++ b/osaf/services/saf/cpsv/cpd/cpd_db.c
@@ -106,6 +106,9 @@ uint32_t cpd_ckpt_node_add(NCS_PATRICIA_
err = create_runtime_ckpt_object(ckpt_node, immOiHandle);
if (err != SA_AIS_OK) {
LOG_ER("create runtime ckpt object failed with error: 
%u",err);
+   if (err == SA_AIS_ERR_INVALID_PARAM) {
+   return NCSCC_RC_INVALID_INPUT;
+   }
return NCSCC_RC_FAILURE;
}
}
diff --git a/osaf/services/saf/cpsv/cpd/cpd_evt.c 
b/osaf/services/saf/cpsv/cpd/cpd_evt.c
--- a/osaf/services/saf/cpsv/cpd/cpd_evt.c
+++ b/osaf/services/saf/cpsv/cpd/cpd_evt.c
@@ -238,9 +238,11 @@ static uint32_t cpd_evt_proc_ckpt_create
rc = SA_AIS_ERR_NO_MEMORY;
goto send_rsp;
} else if (proc_rc != NCSCC_RC_SUCCESS) {
-
TRACE_4("cpd ckpt create failure ckpt name,dest :  %s, 
%"PRIu64, ckpt_name, sinfo->dest);
rc = SA_AIS_ERR_LIBRARY;
+   if (proc_rc == NCSCC_RC_INVALID_INPUT) {
+   rc = SA_AIS_ERR_INVALID_PARAM;
+   }
goto send_rsp;
}
 
diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_evt.c 
b/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
--- a/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
+++ b/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
@@ -605,12 +605,6 @@ static uint32_t cpnd_evt_proc_ckpt_open(
TRACE_ENTER();
memset(&send_evt, '\0', sizeof(CPSV_EVT));
 
-   if ((cpnd_get_longDnsAllowed_attr() == 0) && 
osaf_is_an_extended_name(&evt->info.openReq.ckpt_name)) {
-   LOG_ER("cpnd - longDnsAllowed == false - NOT supporting 
extended name");
-   send_evt.info.cpa.info.openRsp.error = SA_AIS_ERR_INVALID_PARAM;
-   goto agent_rsp;
-   }
-
if (!cpnd_is_cpd_up(cb)) {
send_evt.info.cpa.info.openRsp.error = SA_AIS_ERR_TRY_AGAIN;
goto agent_rsp;
@@ -1137,12 +1131,6 @@ static uint32_t cpnd_evt_proc_ckpt_unlin
TRACE_ENTER();
memset(&send_evt, '\0', sizeof(CPSV_EVT));
 
-   if ((cpnd_get_longDnsAllowed_attr() == 0) && 
osaf_is_an_extended_name(&evt->info.ulinkReq.ckpt_name)) {
-   LOG_ER("cpnd - longDnsAllowed == false - NOT supporting 
extended name");
-   send_evt.info.cpa.info.ulinkRsp.error = 
SA_AIS_ERR_INVALID_PARAM;
-   goto agent_rsp;
-   }
-
if (!cpnd_is_cpd_up(cb)) {
send_evt.info.cpa.info.ulinkRsp.error = SA_AIS_ERR_TRY_AGAIN;
goto agent_rsp;
diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_proc.c 
b/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
--- a/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
+++ b/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
@@ -2735,31 +2735,6 @@ SaUint32T cpnd_get_scAbsenceAllowed_attr
 }
 
 
/
- * Name  : cpnd_get_longDnsAllowed_attr()
- *
- * Description   : This function gets scAbsenceAllowed attribute
- *
- * Arguments : -
- * 
- * Return Values : scAbsenceAllowed attribute (0 = not allowed)
- 
*/
-SaUint32T cpnd_get_longDnsAllowed_attr()
-{
-   SaUint32T rc_attr_val = 0;
-   char *attribute_names[] = {
-   "longDnsAllowed",
-   NULL
-   };
-
-   TRACE_ENTER();
-
-   rc_attr_val = cpnd_get_imm_attr(attribute_names);
-
-   TRA

[devel] [PATCH 0 of 1] Review Request for cpsv: remove longDnsAllowed checking each checkpoint creating time [#2068] V3

2016-09-29 Thread Hoang Vo
Summary: cpsv: remove longDnsAllowed checking each checkpoint creating time 
[#2068] V3
Review request for Trac Ticket(s): 2068
Peer Reviewer(s): mahesh.va...@oracle.com; anders.wid...@ericsson.com
Pull request to: mahesh.va...@oracle.com; anders.wid...@ericsson.com
Affected branch(es): default
Development branch: default


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

changeset e7e4d893fd7e11ca0c14376c9ab4c5914c8256f9
Author: Hoang Vo 
Date:   Thu, 29 Sep 2016 15:05:49 +0700

cpsv: remove longDnsAllowed checking each checkpoint creating time 
[#2068]
V3

Problem: Statistically the check point create time for SC and PL (sync 
and
async) has degradation more than 30% after bring in patch 8004

Solution: Remove unnecessary checking that cost time. imm will take the 
role
of checking


Complete diffstat:
--
 osaf/libs/common/cpsv/include/cpnd_evt.h.old |   0 
 osaf/libs/common/cpsv/include/cpnd_init.h|   1 -
 osaf/services/saf/cpsv/cpd/cpd_db.c  |   3 +++
 osaf/services/saf/cpsv/cpd/cpd_evt.c |   4 +++-
 osaf/services/saf/cpsv/cpnd/cpnd_evt.c   |  12 
 osaf/services/saf/cpsv/cpnd/cpnd_proc.c  |  25 -
 6 files changed, 6 insertions(+), 39 deletions(-)


Testing Commands:
-


Testing, Expected Results:
--


Conditions of Submission:
-


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 ~/.hgrc file (i.e. username, 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 of 1] cpsv: remove longDnsAllowed checking each checkpoint creating time [#2068] V2

2016-09-29 Thread Anders Widell
Hi!

Please find my comments inline below, marked AndersW>.

regards,

Anders Widell


On 09/29/2016 08:25 AM, Hoang Vo wrote:
>   osaf/libs/common/cpsv/include/cpnd_evt.h.old |   0
>   osaf/libs/common/cpsv/include/cpnd_init.h|   1 -
>   osaf/services/saf/cpsv/cpd/cpd_db.c  |   3 +++
>   osaf/services/saf/cpsv/cpd/cpd_evt.c |   4 +++-
>   osaf/services/saf/cpsv/cpnd/cpnd_evt.c   |  12 
>   osaf/services/saf/cpsv/cpnd/cpnd_proc.c  |  25 -
>   6 files changed, 6 insertions(+), 39 deletions(-)
>
>
> Problem:
> Statistically the check point create time for SC and PL (sync and async) has
> degradation more than 30% after bring in patch 8004
>
> Solution:
> Remove unnecessary checking that cost time. imm will take the role of checking
>
> diff --git a/osaf/libs/common/cpsv/include/cpnd_evt.h.old 
> b/osaf/libs/common/cpsv/include/cpnd_evt.h.old
> deleted file mode 100644
> diff --git a/osaf/libs/common/cpsv/include/cpnd_init.h 
> b/osaf/libs/common/cpsv/include/cpnd_init.h
> --- a/osaf/libs/common/cpsv/include/cpnd_init.h
> +++ b/osaf/libs/common/cpsv/include/cpnd_init.h
> @@ -130,7 +130,6 @@ uint32_t cpnd_all_repl_rsp_expiry(CPND_C
>   uint32_t cpnd_open_active_sync_expiry(CPND_CB *cb, CPND_TMR_INFO *tmr_info);
>   void cpnd_proc_free_read_data(CPSV_EVT *evt);
>   SaUint32T cpnd_get_scAbsenceAllowed_attr();
> -SaUint32T cpnd_get_longDnsAllowed_attr();
>   /* End cpnd_proc.c */
>   
>   /* File : ---  cpnd_amf.c */
> diff --git a/osaf/services/saf/cpsv/cpd/cpd_db.c 
> b/osaf/services/saf/cpsv/cpd/cpd_db.c
> --- a/osaf/services/saf/cpsv/cpd/cpd_db.c
> +++ b/osaf/services/saf/cpsv/cpd/cpd_db.c
> @@ -106,6 +106,9 @@ uint32_t cpd_ckpt_node_add(NCS_PATRICIA_
>   err = create_runtime_ckpt_object(ckpt_node, immOiHandle);
>   if (err != SA_AIS_OK) {
>   LOG_ER("create runtime ckpt object failed with error: 
> %u",err);
> + if (err == SA_AIS_ERR_INVALID_PARAM) {
> + return NCSCC_RC_FAILURE|NCSCC_RC_INVALID_INPUT;
AndersW> Style issue: please insert spaces around binary operators (the 
vertical bar above).
AndersW> As far as I know this would become the first place in OpenSAF 
where we return a combination of several NCSCC_RC_* error codes at the 
same time. Can you change the code above so that you only return 
NCSS_RC_INVALID_INPUT?
> + }
>   return NCSCC_RC_FAILURE;
>   }
>   }
> diff --git a/osaf/services/saf/cpsv/cpd/cpd_evt.c 
> b/osaf/services/saf/cpsv/cpd/cpd_evt.c
> --- a/osaf/services/saf/cpsv/cpd/cpd_evt.c
> +++ b/osaf/services/saf/cpsv/cpd/cpd_evt.c
> @@ -238,9 +238,11 @@ static uint32_t cpd_evt_proc_ckpt_create
>   rc = SA_AIS_ERR_NO_MEMORY;
>   goto send_rsp;
>   } else if (proc_rc != NCSCC_RC_SUCCESS) {
> -
>   TRACE_4("cpd ckpt create failure ckpt name,dest :  %s, 
> %"PRIu64, ckpt_name, sinfo->dest);
>   rc = SA_AIS_ERR_LIBRARY;
> + if (proc_rc&NCSCC_RC_INVALID_INPUT) {
Anders> Essentially the same comments as above. Change the line above to 
"if (proc_rc == NCSCC_RC_INVALID_INPUT) {"
> + rc = SA_AIS_ERR_INVALID_PARAM;
> + }
>   goto send_rsp;
>   }
>   
> diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_evt.c 
> b/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
> --- a/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
> +++ b/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
> @@ -605,12 +605,6 @@ static uint32_t cpnd_evt_proc_ckpt_open(
>   TRACE_ENTER();
>   memset(&send_evt, '\0', sizeof(CPSV_EVT));
>   
> - if ((cpnd_get_longDnsAllowed_attr() == 0) && 
> osaf_is_an_extended_name(&evt->info.openReq.ckpt_name)) {
> - LOG_ER("cpnd - longDnsAllowed == false - NOT supporting 
> extended name");
> - send_evt.info.cpa.info.openRsp.error = SA_AIS_ERR_INVALID_PARAM;
> - goto agent_rsp;
> - }
> -
>   if (!cpnd_is_cpd_up(cb)) {
>   send_evt.info.cpa.info.openRsp.error = SA_AIS_ERR_TRY_AGAIN;
>   goto agent_rsp;
> @@ -1137,12 +1131,6 @@ static uint32_t cpnd_evt_proc_ckpt_unlin
>   TRACE_ENTER();
>   memset(&send_evt, '\0', sizeof(CPSV_EVT));
>   
> - if ((cpnd_get_longDnsAllowed_attr() == 0) && 
> osaf_is_an_extended_name(&evt->info.ulinkReq.ckpt_name)) {
> - LOG_ER("cpnd - longDnsAllowed == false - NOT supporting 
> extended name");
> - send_evt.info.cpa.info.ulinkRsp.error = 
> SA_AIS_ERR_INVALID_PARAM;
> - goto agent_rsp;
> - }
> -
>   if (!cpnd_is_cpd_up(cb)) {
>   send_evt.info.cpa.info.ulinkRsp.error = SA_AIS_ERR_TRY_AGAIN;
>   goto agent_rsp;
> diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_proc.c 
> b/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
> --- a/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
> +++ b/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
> @@ -2735,31 +2735,6 @@ SaU

[devel] [PATCH 0 of 1] Review Request for imm: Set validation abort error string when OI returns ERR_BAD_OPERATION or ERR_FAILED_OPERATION [#1650]

2016-09-29 Thread Hung Nguyen
Summary: imm: Set validation abort error string when OI returns 
ERR_BAD_OPERATION or ERR_FAILED_OPERATION [#1650]
Review request for Trac Ticket(s): 1650
Peer Reviewer(s): Zoran, Neel
Pull request to:
Affected branch(es): 5.0, 5.1, 5.2
Development branch: 5.2


Impacted area   Impact y/n

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


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


changeset c3d087ae2dcb8a6aa9b1e6fc79922ef8d70cfbe2
Author: Hung Nguyen 
Date:   Thu, 29 Sep 2016 11:15:50 +0700

imm: Set validation abort error string when OI returns 
ERR_BAD_OPERATION or
ERR_FAILED_OPERATION [#1650]

Set validation abort error string when OI returns ERR_BAD_OPERATION or
ERR_FAILED_OPERATION. This is to prevent om clients from retrying the
operations that are already rejected by OI.


Complete diffstat:
--
 osaf/services/saf/immsv/immnd/ImmModel.cc |  18 +++---
 osaf/services/saf/immsv/immnd/immnd_evt.c |  82 
++
 2 files changed, 37 insertions(+), 63 deletions(-)


Testing Commands:
-



Testing, Expected Results:
--



Conditions of Submission:
-
Ack from reviewers.


Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  n  n
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 ~/.hgrc file (i.e. username, 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 of 1] imm: Set validation abort error string when OI returns ERR_BAD_OPERATION or ERR_FAILED_OPERATION [#1650]

2016-09-29 Thread Hung Nguyen
 osaf/services/saf/immsv/immnd/ImmModel.cc |  18 +-
 osaf/services/saf/immsv/immnd/immnd_evt.c |  82 --
 2 files changed, 37 insertions(+), 63 deletions(-)


Set validation abort error string when OI returns ERR_BAD_OPERATION or 
ERR_FAILED_OPERATION.
This is to prevent om clients from retrying the operations that are already 
rejected by OI.

diff --git a/osaf/services/saf/immsv/immnd/ImmModel.cc 
b/osaf/services/saf/immsv/immnd/ImmModel.cc
--- a/osaf/services/saf/immsv/immnd/ImmModel.cc
+++ b/osaf/services/saf/immsv/immnd/ImmModel.cc
@@ -10872,7 +10872,11 @@ ImmModel::ccbObjDelContinuation(immsv_oi
 "implementer returned error, Ccb aborted with error: %u",
  rsp->result);
 ccb->mVeto = SA_AIS_ERR_FAILED_OPERATION;
-setCcbErrorString(ccb, IMM_VALIDATION_ABORT "Implementer 
returned error: ", rsp->result);
+if (rsp->result == SA_AIS_ERR_BAD_OPERATION || rsp->result == 
SA_AIS_ERR_FAILED_OPERATION) {
+setCcbErrorString(ccb, IMM_VALIDATION_ABORT "Implementer 
returned error: %u", rsp->result);
+} else {
+setCcbErrorString(ccb, IMM_RESOURCE_ABORT "Implementer 
returned error: %u", rsp->result);
+}
 //TODO: This is perhaps more drastic than the specification
 //demands. We are here aborting the entire Ccb, whereas the 
spec
 //seems to allow for a non-ok returnvalue from implementer 
@@ -11150,7 +11154,11 @@ ImmModel::ccbObjCreateContinuation(SaUin
 "implementer returned error, Ccb aborted with error: %u",
 error);
 ccb->mVeto = SA_AIS_ERR_FAILED_OPERATION;
-setCcbErrorString(ccb, IMM_VALIDATION_ABORT "Implementer returned 
error: %u", error);
+if (error == SA_AIS_ERR_BAD_OPERATION || error == 
SA_AIS_ERR_FAILED_OPERATION) {
+setCcbErrorString(ccb, IMM_VALIDATION_ABORT "Implementer returned 
error: %u", error);
+} else {
+setCcbErrorString(ccb, IMM_RESOURCE_ABORT "Implementer returned 
error: %u", error);
+}
 //TODO: This is perhaps more drastic than the specification demands.
 //We are here aborting the entire Ccb, whereas the spec seems to allow
 //for a non-ok returnvalue from implementer (in this callback) to
@@ -11240,7 +11248,11 @@ ImmModel::ccbObjModifyContinuation(SaUin
 LOG_IN("ImmModel::ccbObjModifyContinuation: "
 "implementer returned error, Ccb aborted with error: %u", error);
 ccb->mVeto = SA_AIS_ERR_FAILED_OPERATION;
-setCcbErrorString(ccb, IMM_VALIDATION_ABORT "Implementer returned 
error: %u", error);
+if (error == SA_AIS_ERR_BAD_OPERATION || error == 
SA_AIS_ERR_FAILED_OPERATION) {
+setCcbErrorString(ccb, IMM_VALIDATION_ABORT "Implementer returned 
error: %u", error);
+} else {
+setCcbErrorString(ccb, IMM_RESOURCE_ABORT "Implementer returned 
error: %u", error);
+}
 //TODO: This is perhaps more drastic than the specification demands.
 //We are here aborting the entire Ccb, whereas the spec seems to allow
 //for a non-ok returnvalue from implementer (in this callback) to
diff --git a/osaf/services/saf/immsv/immnd/immnd_evt.c 
b/osaf/services/saf/immsv/immnd/immnd_evt.c
--- a/osaf/services/saf/immsv/immnd/immnd_evt.c
+++ b/osaf/services/saf/immsv/immnd/immnd_evt.c
@@ -3741,42 +3741,23 @@ static void immnd_evt_proc_ccb_obj_modif
memset(&send_evt, '\0', sizeof(IMMSV_EVT));
send_evt.type = IMMSV_EVT_TYPE_IMMA;
send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR;
-   IMMSV_ATTR_NAME_LIST strList;
-   IMMSV_ATTR_NAME_LIST errStrList = { { 0 }, NULL };
-   
+
+   IMMSV_ATTR_NAME_LIST* errStrList = 
immModel_ccbGrabErrStrings(cb, evt->info.ccbUpcallRsp.ccbId);
if (evt->info.ccbUpcallRsp.result != SA_AIS_OK) {
-   if(evt->info.ccbUpcallRsp.result != 
SA_AIS_ERR_FAILED_OPERATION) {
-   char buf[2];
-   int size;
-
-   /* Create error string */
-   size = snprintf(buf, 1,
-   IMM_RESOURCE_ABORT "Upcall 
failed with error code: %u",
-   evt->info.ccbUpcallRsp.result);
-   osafassert(size >= 0);
-   errStrList.name.size = ++size;
-   errStrList.name.buf = (char *)malloc(size);
-   errStrList.next = NULL;
-   size = snprintf(errStrList.name.buf, 
errStrList.name.size,
-   IMM_RESOURCE_ABORT "Upcall 
failed with error code: %u",
-   

Re: [devel] [PATCH 1 of 1] MDS: TIPC change logging level at TIPC_ERRINFO for all but TIPC_ERR_OVERLOAD [#2079]

2016-09-29 Thread Anders Widell
Ack.

regards,

Anders Widell


On 09/28/2016 01:37 PM, Hans Nordeback wrote:
>   osaf/libs/core/mds/mds_dt_tipc.c |  3 +--
>   1 files changed, 1 insertions(+), 2 deletions(-)
>
>
> diff --git a/osaf/libs/core/mds/mds_dt_tipc.c 
> b/osaf/libs/core/mds/mds_dt_tipc.c
> --- a/osaf/libs/core/mds/mds_dt_tipc.c
> +++ b/osaf/libs/core/mds/mds_dt_tipc.c
> @@ -605,8 +605,7 @@ ssize_t recvfrom_connectionless (int sd,
>   m_MDS_LOG_CRITICAL("MDTM: undelivered 
> message condition ancillary data: TIPC_ERR_OVERLOAD");
>   } else {
>   /* TIPC_ERRINFO - TIPC error code 
> associated with a returned data message or a connection termination message  
> so abort */
> - LOG_CR("MDTM: undelivered message 
> condition ancillary data: TIPC_ERRINFO abort err : %d", anc_data[0]);
> - m_MDS_LOG_CRITICAL("MDTM: undelivered 
> message condition ancillary data: TIPC_ERRINFO abort err : %d", anc_data[0]);
> + m_MDS_LOG_DBG("MDTM: undelivered 
> message condition ancillary data: TIPC_ERRINFO abort err : %d", anc_data[0]);
>   }
>   } else if (anc->cmsg_type == TIPC_RETDATA) {
>   /* If we set TIPC_DEST_DROPPABLE off message 
> (configure TIPC to return rejected messages to the sender )


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


Re: [devel] [PATCH 1 of 1] cpsv: remove longDnsAllowed checking each checkpoint creating time [#2068] V2

2016-09-29 Thread A V Mahesh
ACK not tested

-AVM


On 9/29/2016 12:27 PM, Vo Minh Hoang wrote:
> Dear Mahesh,
>
> Thank you very much for your comment.
>
> The function osaf_is_an_extended_name() just check input SaNameT is longDN
> or not.
> So it do not have any use here.
> It is mostly used for handling encode/decode part.
>
> Best regards,
> Hoang
>
> -Original Message-
> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
> Sent: Thursday, September 29, 2016 1:44 PM
> To: Hoang Vo ; anders.wid...@ericsson.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [devel] [PATCH 1 of 1] cpsv: remove longDnsAllowed checking
> each checkpoint creating time [#2068] V2
>
> Hi Hoang,
>
> It looks  osaf_is_an_extended_name() checks longDnsAllowed other services
> are checking that way, so jut keep the code as I suggested in V1 patch.
>
> -AVM
>
> On 9/29/2016 12:08 PM, A V Mahesh wrote:
>> ACK not tested
>>
>> -AVM
>>
>> On 9/29/2016 11:55 AM, Hoang Vo wrote:
>>> osaf/libs/common/cpsv/include/cpnd_evt.h.old |   0
>>> osaf/libs/common/cpsv/include/cpnd_init.h|   1 -
>>> osaf/services/saf/cpsv/cpd/cpd_db.c  |   3 +++
>>> osaf/services/saf/cpsv/cpd/cpd_evt.c |   4 +++-
>>> osaf/services/saf/cpsv/cpnd/cpnd_evt.c   |  12 
>>> osaf/services/saf/cpsv/cpnd/cpnd_proc.c  |  25
> -
>>> 6 files changed, 6 insertions(+), 39 deletions(-)
>>>
>>>
>>> Problem:
>>> Statistically the check point create time for SC and PL (sync and
>>> async) has degradation more than 30% after bring in patch 8004
>>>
>>> Solution:
>>> Remove unnecessary checking that cost time. imm will take the role of
>>> checking
>>>
>>> diff --git a/osaf/libs/common/cpsv/include/cpnd_evt.h.old
>>> b/osaf/libs/common/cpsv/include/cpnd_evt.h.old
>>> deleted file mode 100644
>>> diff --git a/osaf/libs/common/cpsv/include/cpnd_init.h
>>> b/osaf/libs/common/cpsv/include/cpnd_init.h
>>> --- a/osaf/libs/common/cpsv/include/cpnd_init.h
>>> +++ b/osaf/libs/common/cpsv/include/cpnd_init.h
>>> @@ -130,7 +130,6 @@ uint32_t cpnd_all_repl_rsp_expiry(CPND_C
>>> uint32_t cpnd_open_active_sync_expiry(CPND_CB *cb, CPND_TMR_INFO
> *tmr_info);
>>> void cpnd_proc_free_read_data(CPSV_EVT *evt);
>>> SaUint32T cpnd_get_scAbsenceAllowed_attr(); -SaUint32T
>>> cpnd_get_longDnsAllowed_attr();
>>> /* End cpnd_proc.c */
>>> 
>>> /* File : ---  cpnd_amf.c */
>>> diff --git a/osaf/services/saf/cpsv/cpd/cpd_db.c
>>> b/osaf/services/saf/cpsv/cpd/cpd_db.c
>>> --- a/osaf/services/saf/cpsv/cpd/cpd_db.c
>>> +++ b/osaf/services/saf/cpsv/cpd/cpd_db.c
>>> @@ -106,6 +106,9 @@ uint32_t cpd_ckpt_node_add(NCS_PATRICIA_
>>> err = create_runtime_ckpt_object(ckpt_node, 
>>> immOiHandle);
>>> if (err != SA_AIS_OK) {
>>> LOG_ER("create runtime ckpt object failed with
> error: %u",err);
>>> +   if (err == SA_AIS_ERR_INVALID_PARAM) {
>>> +   return
> NCSCC_RC_FAILURE|NCSCC_RC_INVALID_INPUT;
>>> +   }
>>> return NCSCC_RC_FAILURE;
>>> }
>>> }
>>> diff --git a/osaf/services/saf/cpsv/cpd/cpd_evt.c
>>> b/osaf/services/saf/cpsv/cpd/cpd_evt.c
>>> --- a/osaf/services/saf/cpsv/cpd/cpd_evt.c
>>> +++ b/osaf/services/saf/cpsv/cpd/cpd_evt.c
>>> @@ -238,9 +238,11 @@ static uint32_t cpd_evt_proc_ckpt_create
>>> rc = SA_AIS_ERR_NO_MEMORY;
>>> goto send_rsp;
>>> } else if (proc_rc != NCSCC_RC_SUCCESS) {
>>> -
>>> TRACE_4("cpd ckpt create failure ckpt name,dest :  %s,
> %"PRIu64, ckpt_name, sinfo->dest);
>>> rc = SA_AIS_ERR_LIBRARY;
>>> +   if (proc_rc&NCSCC_RC_INVALID_INPUT) {
>>> +   rc = SA_AIS_ERR_INVALID_PARAM;
>>> +   }
>>> goto send_rsp;
>>> }
>>> 
>>> diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
>>> b/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
>>> --- a/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
>>> +++ b/osaf/services/saf/cpsv/cpnd/cpnd_evt.c
>>> @@ -605,12 +605,6 @@ static uint32_t cpnd_evt_proc_ckpt_open(
>>> TRACE_ENTER();
>>> memset(&send_evt, '\0', sizeof(CPSV_EVT));
>>> 
>>> -   if ((cpnd_get_longDnsAllowed_attr() == 0) &&
> osaf_is_an_extended_name(&evt->info.openReq.ckpt_name)) {
>>> -   LOG_ER("cpnd - longDnsAllowed == false - NOT supporting
> extended name");
>>> -   send_evt.info.cpa.info.openRsp.error =
> SA_AIS_ERR_INVALID_PARAM;
>>> -   goto agent_rsp;
>>> -   }
>>> -
>>> if (!cpnd_is_cpd_up(cb)) {
>>> send_evt.info.cpa.info.openRsp.error = 
>>> SA_AIS_ERR_TRY_AGAIN;
>>> goto agent_rsp;
>>> @@ -1137,12 +1131,6 @@ static uint32_t cpnd_evt_proc_ckpt_unlin
>>> TRACE_ENTER();
>>> memset(&send_evt, '\0', sizeof(CPSV_EVT));
>>> 
>>> -   if ((cpnd