[devel] [PATCH 1 of 1] imm: Dont allow standby IMMD to send fevs if active IMMD is still up [#2029]

2016-09-19 Thread Hung Nguyen 
 osaf/services/saf/immsv/immd/immd_amf.c  |   1 +
 osaf/services/saf/immsv/immd/immd_evt.c  |   2 +-
 osaf/services/saf/immsv/immd/immd_proc.c |  49 ++-
 osaf/services/saf/immsv/immd/immd_proc.h |   2 +-
 4 files changed, 25 insertions(+), 29 deletions(-)


During node shutdown, if active IMMD is down before IMMND,
the IMMD will fail to broadcast discard node message to other IMMNDs.

In this case, standby IMMD will help broadcast the discard messages.
But it should only do that when the active IMMD is really down (immd_remote_up 
is false)
or when it becomes active (invoking immd_pending_discards in rda/amf callbacks).

This is to prevent the active IMMD and standby IMMD from sending the discard 
messages with different number
which cause fevs message loss.

diff --git a/osaf/services/saf/immsv/immd/immd_amf.c 
b/osaf/services/saf/immsv/immd/immd_amf.c
--- a/osaf/services/saf/immsv/immd/immd_amf.c
+++ b/osaf/services/saf/immsv/immd/immd_amf.c
@@ -265,6 +265,7 @@ static void immd_saf_csi_set_cb(SaInvoca
immd_proc_elect_coord(cb, true);
}
immd_db_purge_fevs(cb);
+   immd_pending_discards(cb);
}
}
 
diff --git a/osaf/services/saf/immsv/immd/immd_evt.c 
b/osaf/services/saf/immsv/immd/immd_evt.c
--- a/osaf/services/saf/immsv/immd/immd_evt.c
+++ b/osaf/services/saf/immsv/immd/immd_evt.c
@@ -2582,7 +2582,7 @@ static uint32_t immd_evt_proc_rda_callba
immd_proc_elect_coord(cb, true);
}
immd_db_purge_fevs(cb);
-   immd_pending_payload_discards(cb); /*Ensure node down for 
payloads.*/
+   immd_pending_discards(cb);
}
 done:
TRACE_LEAVE();
diff --git a/osaf/services/saf/immsv/immd/immd_proc.c 
b/osaf/services/saf/immsv/immd/immd_proc.c
--- a/osaf/services/saf/immsv/immd/immd_proc.c
+++ b/osaf/services/saf/immsv/immd/immd_proc.c
@@ -615,7 +615,6 @@ uint32_t immd_process_immnd_down(IMMD_CB
NCS_UBAID uba;
char *tmpData = NULL;
bool coord_exists = true; /* Assumption at this point.*/
-   bool possible_fo = false;
TRACE_ENTER();
 
TRACE_5("immd_process_immnd_down pid:%u on-active:%u "
@@ -648,29 +647,26 @@ uint32_t immd_process_immnd_down(IMMD_CB
   "detected at standby immd!! %x. "
   "Possible failover",
   
immd_get_slot_and_subslot_id_from_node_id(immnd_info->immnd_key), 
cb->immd_self_id);
-   possible_fo = true;
+
if (immnd_info->isCoord && immnd_info->syncStarted) {
immd_proc_abort_sync(cb, immnd_info);
}
+
+   if (coord_exists) {
+   immd_proc_rebroadcast_fevs(cb, 2);
+   }
}
}
 
-   if (active || possible_fo || !cb->immd_remote_up) {
+   if (active || !cb->immd_remote_up) {
/* 
 ** HAFE - Let IMMND subscribe for IMMND up/down events instead?
 ** ABT - Not for now. IMMND up/down are only subscribed by 
IMMD.
-** This is the cleanest solution with respect to FEVS and 
works fine
-** for all cases except IMMND down at the active controller. 
-** That case has to be handled in a special way. Currently we 
do it
-** by having both the standby and active IMMD broadcast the 
same 
-** DISCARD_NODE message. IMMNDs have to cope with the possibly 
-** redundant message, which they do in fevs_receive discarding
-** any duplicate (same sequence no).
+** Active IMMD will inform other IMMNDs about the down IMMND.
+** If the active is down, the standby (has not become active 
yet)
+** will do the job.
 */
if (coord_exists) {
-   if (possible_fo) {
-   immd_proc_rebroadcast_fevs(cb, 2);
-   }
 
TRACE_5("Notify all remaining IMMNDs of the departed 
IMMND");
memset(_evt, 0, sizeof(IMMSV_EVT));
@@ -700,14 +696,13 @@ uint32_t immd_process_immnd_down(IMMD_CB
 
free(tmpData);
}
-   } else if(!(immnd_info->isOnController)) {
-   /* Standby NOT immediately sending redundant D2ND_DISCARD_NODE 
in this case.
-  But will record any payload down event in case the active SC 
is included
-  in a burst of node downs. See ticket #563. The active IMMD 
may be going
-  down together with many payload nodes, such that the active 
IMMD never has
-  time to generate the discard node message for all payloads. 
This

[devel] [PATCH 0 of 1] Review Request for imm: Dont allow standby IMMD to send fevs if active IMMD is still up [#2029]

2016-09-19 Thread Hung Nguyen 
Summary: imm: Dont allow standby IMMD to send fevs if active IMMD is still up 
[#2029]
Review request for Trac Ticket(s): 2029
Peer Reviewer(s): Zoran, Neel
Pull request to:
Affected branch(es): 4.7, 5.0, 5.1
Development branch: 5.1


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 248768e6b8eb58fb9e72c93b5f2a190fab437d48
Author: Hung Nguyen 
Date:   Tue, 20 Sep 2016 10:57:50 +0700

imm: Dont allow standby IMMD to send fevs if active IMMD is still up 
[#2029]

During node shutdown, if active IMMD is down before IMMND, the IMMD will
fail to broadcast discard node message to other IMMNDs.

In this case, standby IMMD will help broadcast the discard messages. 
But it
should only do that when the active IMMD is really down (immd_remote_up 
is
false) or when it becomes active (invoking immd_pending_discards in 
rda/amf
callbacks).

This is to prevent the active IMMD and standby IMMD from sending the 
discard
messages with different number which cause fevs message loss.


Complete diffstat:
--
 osaf/services/saf/immsv/immd/immd_amf.c  |   1 +
 osaf/services/saf/immsv/immd/immd_evt.c  |   2 +-
 osaf/services/saf/immsv/immd/immd_proc.c |  49 
++---
 osaf/services/saf/immsv/immd/immd_proc.h |   2 +-
 4 files changed, 25 insertions(+), 29 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] log: fix ER syslog when running logtest 5 2 [#1864]

2016-09-19 Thread A V Mahesh 
ACK,

Not tested.

-AVM


On 9/16/2016 2:30 PM, Canh Van Truong wrote:
>   tests/logsv/tet_LogOiOps.c |  11 +++
>   1 files changed, 11 insertions(+), 0 deletions(-)
>
>
> Not wait for apply callback in logtest 5 2 done before removing test directory
> (in this test case, it changes 'logRootDirectory' attribute, then there are 
> action of renames files)
>
> The patch adds a check loop to make sure all apply callback (rename log 
> files) done
> before removing the log test directory.
>
> diff --git a/tests/logsv/tet_LogOiOps.c b/tests/logsv/tet_LogOiOps.c
> --- a/tests/logsv/tet_LogOiOps.c
> +++ b/tests/logsv/tet_LogOiOps.c
> @@ -969,6 +969,7 @@ void saLogOi_52(void)
>   void saLogOi_48(void)
>   {
>   int rc = 0, tst_stat = 0;
> + unsigned int count = 100;
>   char command[256];
>   char tstdir[256];
>   
> @@ -1011,6 +1012,16 @@ void saLogOi_48(void)
>   }
>   
>   done_remove:
> + /* Check if all files have been renamed in 'tstdir' before remove them*/
> + for (int i = 0; i < count; i++) {
> + usleep(100*1000);
> + /* Find if any files in 'tstdir' have not been rename (append 
> time_stamp). */
> + sprintf(command, "find %s -type f | egrep -v 
> \".*_[0-9]{8}_[0-9]{6}.*\"  1> /dev/null",
> + tstdir);
> + rc = system(command);
> + if (WEXITSTATUS(rc) != 0)
> + break;
> + }
>   /* Remove xxtest no longer used */
>   sprintf(command, "rm -rf %s/", tstdir);
>   rc = tet_system(command);


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

Re: [devel] [PATCH 1 of 1] log: fix logtest fails when run after immomtest [#2028]

2016-09-19 Thread A V Mahesh 
ACK,

Not tested.

-AVM


On 9/13/2016 4:50 PM, Vu Minh Nguyen wrote:
>   tests/logsv/tet_log_longDN.c |  131 
> +++---
>   1 files changed, 73 insertions(+), 58 deletions(-)
>
>
> Only test case #4 and #5 of suite #13 require long DN enabled in IMM.
> If the long DN is enabled on running system, no need to enable it.
>
> diff --git a/tests/logsv/tet_log_longDN.c b/tests/logsv/tet_log_longDN.c
> --- a/tests/logsv/tet_log_longDN.c
> +++ b/tests/logsv/tet_log_longDN.c
> @@ -60,6 +60,7 @@ static char v_saLogStreamLogFileFormat[1
>   static uint32_t v_saLogStreamFixedLogRecordSize = 200;
>   static char v_saLogStreamFileName[256] = {0};
>   static uint32_t v_longDnsAllowed = 0;
> +static bool g_setLongDnsAllowed = false;
>   
>   typedef enum {
>   E_ALARM,
> @@ -103,6 +104,55 @@ SaConstStringT notificationObjDf = "Noti
>   static void logWriteLogCallbackT(SaInvocationT invocation, SaAisErrorT 
> error);
>   static SaLogCallbacksT logCallbacksLd = { 0, 0, logWriteLogCallbackT };
>   
> +
> +//>
> +// Enable long DN in IMM it is not set on current system
> +//
> +// NOTE: Need long DN enabled in case of creating long DN in IMM.
> +// means, test case #4 and #5 require this.
> +//<
> +static int enableLongDN(void)
> +{
> + int rc;
> +
> + // No need to enable if long DN is set on current system
> + if (v_longDnsAllowed != 0) return 0;
> +
> + /* Enable long DN in IMM */
> + rc = system("immcfg -o safImmService -a longDnsAllowed=1 
> opensafImm=opensafImm,safApp=safImmService");
> + if (WEXITSTATUS(rc) != 0) {
> + fprintf(stderr, "Failed to enable long DN \n");
> + return 1;
> + }
> +
> + g_setLongDnsAllowed = true;
> + return 0;
> +}
> +
> +//>
> +// Restore the longDnsAllowed
> +//<
> +static void disableLongDN(void)
> +{
> + int rc;
> + char command[MAX_DATA] = {0};
> +
> + // No need to enable if long DN is set on current system
> + if (g_setLongDnsAllowed == false) return;
> +
> + sprintf(command, "immcfg -o safImmService -a longDnsAllowed=%d 
> opensafImm=opensafImm,safApp=safImmService",
> + v_logMaxLogrecsize);
> +
> + /* Restore back to previous value */
> + rc = system(command);
> + if (WEXITSTATUS(rc) != 0) {
> + fprintf(stderr, "Failed to restore longDnsAllowed \n");
> + return;
> + }
> +
> + g_setLongDnsAllowed = 0;
> +}
> +
>   //>
>   // Following attributes are backup before performing testing
>   // logMaxLogrecsize;
> @@ -115,12 +165,6 @@ static int backupData(stream_type_t type
>   {
>   int rc;
>   
> - saAisNameLend(s_opensafImm, _opensafImm);
> - rc = get_attr_value(_opensafImm, "longDnsAllowed", 
> _longDnsAllowed);
> - if (rc == -1) {
> - /* Failed, use default one */
> - fprintf(stderr, "Failed to get attribute longDnsAllowed value 
> from IMM\n");
> - }
>   rc = get_attr_value(, "logMaxLogrecsize", 
> _logMaxLogrecsize);
>   if (rc == -1) {
>   /* Failed, use default one */
> @@ -229,12 +273,6 @@ static int setUpTestEnv(stream_type_t ty
>   int rc;
>   char command[MAX_DATA];
>   
> - /* Enable long DN feature */
> - rc = system("immcfg -m -a longDnsAllowed=1 
> opensafImm=opensafImm,safApp=safImmService");
> - if (WEXITSTATUS(rc) != 0) {
> - fprintf(stderr, "Failed to enable long DN \n");
> - return -1;
> - }
>   sprintf(command, "immcfg -a logMaxLogrecsize=%d "
>   "logConfig=1,safApp=safLogService 2> /dev/null", 
> SA_LOG_MAX_RECORD_SIZE);
>   rc = system(command);
> @@ -285,12 +323,6 @@ void restoreData(stream_type_t type)
>   int rc;
>   char command[MAX_DATA];
>   
> - sprintf(command, "immcfg -a longDnsAllowed=%d %s", v_longDnsAllowed, 
> s_opensafImm);
> - rc = system(command);
> - if (WEXITSTATUS(rc) != 0) {
> - fprintf(stderr, "Failed to perform cmd = %s\n", command);
> - }
> -
>   sprintf(command, "immcfg -a logMaxLogrecsize=%d "
>   "logConfig=1,safApp=safLogService 2> /dev/null", 
> v_logMaxLogrecsize);
>   rc = system(command);
> @@ -824,6 +856,20 @@ void longDN_AppStream(void)
>   int rc;
>   SaAisErrorT ais;
>   
> + saAisNameLend(s_opensafImm, _opensafImm);
> + rc = get_attr_value(_opensafImm, "longDnsAllowed", 
> _longDnsAllowed);
> + if (rc == -1) {
> + /* Failed, use default one */
> + fprintf(stderr, "Failed to get attribute longDnsAllowed value 
> from IMM\n");
> + }
> +
> + rc = enableLongDN();
> + if (rc != 0) {
> + fprintf(stderr, "failed to enable long DN in IMM\n");
> + rc_validate(WEXITSTATUS(rc), 0);
> + return;
> + }
> +
>   rc = backupData(E_APPLI);
>   if (rc != 0) {
>   fprintf(stderr, "Backup data failed\n");
> @@ -873,6 +919,7 @@ void longDN_AppStream(void)
>   done_init:
>

Re: [devel] [PATCH 1 of 1] amfnd: avoid sending multiple node-switchover recovery request for same fault[#1935]

2016-09-19 Thread minh chau 
Hi Praveen,

Ack for code review, just one minor comment as below:
Should we reset the @failed_su = nullptr in case of NODE_FAILOVER as well?
uint32_t avnd_comp_clc_fsm_run(AVND_CB *cb, AVND_COMP *comp, 
AVND_COMP_CLC_PRES_FSM_EV ev)
{
...
 LOG_NO("Informing director of node fail-over");
 rc = avnd_di_oper_send(cb, cb->failed_su, 
SA_AMF_NODE_FAILOVER);
 osafassert(NCSCC_RC_SUCCESS == rc);
...
}

Thanks,
Minh
On 02/09/16 15:34, praveen.malv...@oracle.com wrote:
>   osaf/services/saf/amf/amfnd/susm.cc |  3 ++-
>   1 files changed, 2 insertions(+), 1 deletions(-)
>
>
> In the reported problem, AMFND sends multiple mode-switchover recovery request
> for same fault when nodeautorepair is disabled. Sometimes, amfnd is successful
> in sending the request but it may crash also.
>
> In the reported problem, AMFND reports node-switchover recovery to AMFD. AMFD
> removes assignments gracefully and does not reboot the node as nodeautorepair
> is disabled. After this user deletes the faulty SU after perofrming lock and 
> lock-in
> admin operations. Same SU is added again and instantiated. AMFND while 
> instantiating
> the SU, sends recovery request to AMFD again for the same fault.
>
> Patch resets failed_su in avnd_cb so that recovery request is not sent
> for same fault again.
>
> diff --git a/osaf/services/saf/amf/amfnd/susm.cc 
> b/osaf/services/saf/amf/amfnd/susm.cc
> --- a/osaf/services/saf/amf/amfnd/susm.cc
> +++ b/osaf/services/saf/amf/amfnd/susm.cc
> @@ -1413,6 +1413,7 @@ static void perform_pending_nodeswitchov
>   if (su->sufailover == false) {
>   uint32_t rc = avnd_di_oper_send(avnd_cb, su, 
> SA_AMF_NODE_SWITCHOVER);
>   osafassert(NCSCC_RC_SUCCESS == rc);
> + avnd_cb->failed_su = nullptr;
>   }
>   }
>   
> @@ -1478,7 +1479,7 @@ uint32_t avnd_su_pres_fsm_run(AVND_CB *c
>   goto done;
>   } else if ((cb->term_state == 
> AVND_TERM_STATE_NODE_SWITCHOVER_STARTED) &&
>   (cb->oper_state == SA_AMF_OPERATIONAL_DISABLED) &&
> - (su->sufailover == false)) {
> + (su->sufailover == false) && (cb->failed_su != 
> nullptr)) {
>   perform_pending_nodeswitchover();
>   }
>   
>


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

Re: [devel] [PATCH 1 of 1] amf: fix activeCompName in csiStateDescriptor used in CSI SET cbk [#2021]

2016-09-19 Thread Hans Nordebäck 
Ack, code review only./Thanks HansN

-Original Message-
From: praveen.malv...@oracle.com [mailto:praveen.malv...@oracle.com] 
Sent: den 15 september 2016 11:42
To: Hans Nordebäck ; nagendr...@oracle.com; Gary 
Lee ; Minh Hon Chau 
Cc: opensaf-devel@lists.sourceforge.net
Subject: [PATCH 1 of 1] amf: fix activeCompName in csiStateDescriptor used in 
CSI SET cbk [#2021]

 osaf/libs/agents/saf/amfa/ava_hdl.cc |  28 +++-
 osaf/libs/common/amf/n2avamsg.c  |  14 ++
 osaf/services/saf/amf/amfd/util.cc   |   2 +-
 osaf/services/saf/amf/amfnd/comp.cc  |   1 -
 4 files changed, 26 insertions(+), 19 deletions(-)


In the reported problem, active compname is improperly as \'sa\'
in CSI SET callback for HA state.

After setting activeCompName in standby descriptor, AMFND is wrongly calling
osaf_extended_name_clear() for active descriptor. Since 
SaAmfCSIStateDescriptorT is a union, it clears the filled value.
Along with the above problems there are other minor problems like:
-AMFND is copying activecompName using osaf_extended_name_alloc() in agent 
message,  only when it is an extended name.
 osaf_extended_name_alloc() should be done irrespective of short or long dn.
-Agent should perform validation check on dn based on HA state of the CSI SET 
callback,  as activecompname is not populated for all the HA states.
-When above problems are fixed, one more problem pops up:
 a standby component gets its own name in activeCompName in CSI set callback 
for standby.
 There is a minor regression at AMFD.

Patch fixes all these problems.

diff --git a/osaf/libs/agents/saf/amfa/ava_hdl.cc 
b/osaf/libs/agents/saf/amfa/ava_hdl.cc
--- a/osaf/libs/agents/saf/amfa/ava_hdl.cc
+++ b/osaf/libs/agents/saf/amfa/ava_hdl.cc
@@ -624,21 +624,31 @@ uint32_t ava_hdl_cbk_rec_prc(AVSV_AMF_CB
AVSV_AMF_CSI_SET_PARAM *csi_set = >param.csi_set;
 
if (!ava_sanamet_is_valid(_set->csi_desc.csiName) ||
-   !ava_sanamet_is_valid(_set->comp_name) ||
-   
!ava_sanamet_is_valid(_set->csi_desc.csiStateDescriptor.activeDescriptor.activeCompName)
 ||
-   
!ava_sanamet_is_valid(_set->csi_desc.csiStateDescriptor.standbyDescriptor.activeCompName))
 {
+   !ava_sanamet_is_valid(_set->comp_name)) {
rc = SA_AIS_ERR_NAME_TOO_LONG;
}
+   bool actv_or_stdby = true;
+   if ((csi_set->ha == SA_AMF_HA_ACTIVE) && 
+   
(csi_set->csi_desc.csiStateDescriptor.activeDescriptor.transitionDescriptor != 
SA_AMF_CSI_NEW_ASSIGN)) {
+   actv_or_stdby = 
ava_sanamet_is_valid(_set->csi_desc.csiStateDescriptor.activeDescriptor.activeCompName);
+   } else if (csi_set->ha == SA_AMF_HA_STANDBY) {
+   actv_or_stdby = 
ava_sanamet_is_valid(_set->csi_desc.csiStateDescriptor.standbyDescriptor.activeCompName);
+   }
+   if (!actv_or_stdby)
+   rc = SA_AIS_ERR_NAME_TOO_LONG;
 
if (rc == SA_AIS_OK && reg_cbk->saAmfCSISetCallback) {
TRACE("CSISet: CSIName = %s, CSIFlags = %d, HA 
state = %d",

osaf_extended_name_borrow(_set->csi_desc.csiName),csi_set->csi_desc.csiFlags,csi_set->ha);
-   TRACE("CSISet: Active Transition Descriptor = 
%u, Active Component Name = %s",
-   
csi_set->csi_desc.csiStateDescriptor.activeDescriptor.transitionDescriptor,
-   
osaf_extended_name_borrow(_set->csi_desc.csiStateDescriptor.activeDescriptor.activeCompName));
-   TRACE("CSISet: ActiveCompName = %s, StandbyRank 
= %u",
-   
osaf_extended_name_borrow(_set->csi_desc.csiStateDescriptor.standbyDescriptor.activeCompName),
-   
csi_set->csi_desc.csiStateDescriptor.standbyDescriptor.standbyRank);
+   if ((csi_set->ha == SA_AMF_HA_ACTIVE) && 
+   
(csi_set->csi_desc.csiStateDescriptor.activeDescriptor.transitionDescriptor != 
SA_AMF_CSI_NEW_ASSIGN))
+   TRACE("CSISet: Active Transition 
Descriptor = %u, Active Component Name = %s",
+   
csi_set->csi_desc.csiStateDescriptor.activeDescriptor.transitionDescriptor,
+   
osaf_extended_name_borrow(_set->csi_desc.csiStateDescriptor.activeDescriptor.activeCompName));
+   if (csi_set->ha == SA_AMF_HA_STANDBY)
+

Re: [devel] Review request for updates to SMF PR for 5.1

2016-09-19 Thread Neelakanta Reddy 
Hi Rafel,

Reviewed the PR doc.
Ack, with following comments:

1.  The newly added attributes, has to be mentioned in the section 6.2.2
2.  "From OpenSAF 5.1 " has to be added for the section 5.2

Regards,
Neel.

On 2016/09/15 06:06 PM, Rafael Odzakow wrote:
>


--
___
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 log: fix ER no stream exists in syslog [#2043]

2016-09-19 Thread Vu Minh Nguyen 
Summary: log: fix ER no stream exists in syslog [#2043]
Review request for Trac Ticket(s): #2043
Peer Reviewer(s): Lennart, Mahesh
Pull request to: <>
Affected branch(es): 5.1, 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 438902ac647e8cdbac71569f3e733d621ee067e8
Author: Vu Minh Nguyen 
Date:   Mon, 19 Sep 2016 14:15:58 +0700

log: fix ER no stream exists in syslog [#2043]

The `number of streams` refers to total existing log streams in 
cluster. And
`stream_array` is the database holding all existing log streams. When
interating all log streams, logsv first started at the index `number of
streams`, if getting NULL, logsv considered that case as `no stream`. 
This
is absolutely wrong.

This patch provides other way to iterate all log streams.


Complete diffstat:
--
 osaf/services/saf/logsv/lgs/lgs_amf.cc|  27 +++
 osaf/services/saf/logsv/lgs/lgs_config.cc |  13 +
 osaf/services/saf/logsv/lgs/lgs_evt.cc|  28 
 osaf/services/saf/logsv/lgs/lgs_imm.cc|  70 
++
 osaf/services/saf/logsv/lgs/lgs_mbcsv.cc  |  27 +++
 osaf/services/saf/logsv/lgs/lgs_stream.cc |   6 +++---
 6 files changed, 120 insertions(+), 51 deletions(-)


Testing Commands:
-
 Create 02 streams, and then delete it in following order:
 1) create cfg stream A
 2) create cfg stream B
 3) delete stream A
 4) reboot active node


Testing, Expected Results:
--
 No see "no stream exists" in syslog


Conditions of Submission:
-
 Get acks from peer 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] log: fix ER no stream exists in syslog [#2043]

2016-09-19 Thread Vu Minh Nguyen 
 osaf/services/saf/logsv/lgs/lgs_amf.cc|  27 ---
 osaf/services/saf/logsv/lgs/lgs_config.cc |  13 +++-
 osaf/services/saf/logsv/lgs/lgs_evt.cc|  28 ---
 osaf/services/saf/logsv/lgs/lgs_imm.cc|  70 ++
 osaf/services/saf/logsv/lgs/lgs_mbcsv.cc  |  27 ---
 osaf/services/saf/logsv/lgs/lgs_stream.cc |   6 +-
 6 files changed, 120 insertions(+), 51 deletions(-)


The `number of streams` refers to total existing log streams in cluster.
And `stream_array` is the database holding all existing log streams.
When interating all log streams, logsv first started at the index `number of 
streams`,
if getting NULL, logsv considered that case as `no stream`. This is absolutely 
wrong.

This patch provides other way to iterate all log streams.

diff --git a/osaf/services/saf/logsv/lgs/lgs_amf.cc 
b/osaf/services/saf/logsv/lgs/lgs_amf.cc
--- a/osaf/services/saf/logsv/lgs/lgs_amf.cc
+++ b/osaf/services/saf/logsv/lgs/lgs_amf.cc
@@ -26,13 +26,18 @@
 
 static void close_all_files() {
   log_stream_t *stream;
-  int num = get_number_of_streams();
-  stream = log_stream_get_by_id(--num);
-  while (stream != NULL) {
+  uint32_t count = 0, stream_id = 0;
+  uint32_t num = get_number_of_streams();
+  stream = log_stream_get_by_id(stream_id);
+  while (count < num) {
+if (stream == nullptr) goto next;
+
+count++;
 if (log_stream_file_close(stream) != 0)
   LOG_WA("Could not close file for stream %s", stream->name.c_str());
 
-stream = log_stream_get_by_id(--num);
+ next:
+stream = log_stream_get_by_id(++stream_id);
   }
 }
 
@@ -52,7 +57,8 @@ static void close_all_files() {
 static SaAisErrorT amf_active_state_handler(lgs_cb_t *cb, SaInvocationT 
invocation) {
   log_stream_t *stream;
   SaAisErrorT error = SA_AIS_OK;
-  int num;
+  uint32_t num;
+  uint32_t count = 0, stream_id = 0;
 
   TRACE_ENTER2("HA ACTIVE request");
 
@@ -67,12 +73,17 @@ static SaAisErrorT amf_active_state_hand
 
   /* check existing streams */
   num = get_number_of_streams();
-  stream = log_stream_get_by_id(--num);
+  stream = log_stream_get_by_id(stream_id);
   if (!stream)
 LOG_ER("No streams exist!");
-  while (stream != NULL) {
+  while (count < num) {
+if (stream == nullptr) goto next;
+
+count++;
 *stream->p_fd = -1; /* First Initialize fd */
-stream = log_stream_get_by_id(--num);
+
+ next:
+stream = log_stream_get_by_id(++stream_id);
   }
 
 done:
diff --git a/osaf/services/saf/logsv/lgs/lgs_config.cc 
b/osaf/services/saf/logsv/lgs/lgs_config.cc
--- a/osaf/services/saf/logsv/lgs/lgs_config.cc
+++ b/osaf/services/saf/logsv/lgs/lgs_config.cc
@@ -458,7 +458,8 @@ int lgs_cfg_verify_root_dir(const std::s
   int rc = 0;
   log_stream_t *stream = NULL;
   size_t n = root_str_in.size();
-  int num;
+  uint32_t num;
+  uint32_t count = 0, stream_id = 0;
 
   if (n > PATH_MAX) {
 LOG_NO("verify_root_dir Fail. Path > PATH_MAX");
@@ -471,8 +472,11 @@ int lgs_cfg_verify_root_dir(const std::s
* must not be larger than PATH_MAX.
*/
   num = get_number_of_streams();
-  stream = log_stream_get_by_id(--num);
-  while (stream != NULL) {
+  stream = log_stream_get_by_id(stream_id);
+  while (count < num) {
+if (stream == nullptr) goto next;
+
+count++;
 if (lgs_is_valid_pathlength(stream->pathName, stream->fileName,
 root_str_in) == false) {
   TRACE("The rootPath is invalid (%s)", root_str_in.c_str());
@@ -480,7 +484,8 @@ int lgs_cfg_verify_root_dir(const std::s
   goto done;
 }
 
-stream = log_stream_get_by_id(--num);
+ next:
+stream = log_stream_get_by_id(++stream_id);
   }
 
   if (lgs_path_is_writeable_dir_h(root_str_in) == false) {
diff --git a/osaf/services/saf/logsv/lgs/lgs_evt.cc 
b/osaf/services/saf/logsv/lgs/lgs_evt.cc
--- a/osaf/services/saf/logsv/lgs/lgs_evt.cc
+++ b/osaf/services/saf/logsv/lgs/lgs_evt.cc
@@ -532,13 +532,19 @@ static uint32_t proc_rda_cb_msg(lgsv_lgs
 lgs_process_lga_down_list();
 
 /* Check existing streams */
-int num = get_number_of_streams();
-stream = log_stream_get_by_id(--num);
+uint32_t num = get_number_of_streams();
+uint32_t count = 0, stream_id = 0;
+stream = log_stream_get_by_id(stream_id);
 if (!stream)
   LOG_ER("No streams exist!");
-while (stream != NULL) {
+while (count < num) {
+  if (stream == nullptr) goto next;
+
+  count++;
   *stream->p_fd = -1; /* Initialize fd */
-  stream = log_stream_get_by_id(--num);
+
+   next:
+  stream = log_stream_get_by_id(++stream_id);
 }
   }
 
@@ -800,7 +806,8 @@ SaAisErrorT create_new_app_stream(lgsv_s
   SaBoolT twelveHourModeFlag;
   SaUint32T logMaxLogrecsize_conf = 0;
   SaConstStringT str_name;
-  int num, err = 0;
+  int err = 0;
+  uint32_t count = 0, stream_id = 0, num;
   const char *dnPrefix = "safLgStr=";
 
   TRACE_ENTER();
@@ -863,15 +870,20 @@ SaAisErrorT create_new_app_stream(lgsv_s
 
   /* Verify that path and file are

[devel] [PATCH 0 of 1] Review Request for amfd: do not send duplicate removal of assignment, N-Way model [#2022]

2016-09-19 Thread praveen . malviya 
Summary: amfd: do not send duplicate removal of assignment, N-Way model [#2022] 
Review request for Trac Ticket(s): #2022 
Peer Reviewer(s): AMF devs 
Pull request to: <>
Affected branch(es): 5.1 and default. 
Development branch: <>


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):
-
Issue may be applicable to other branches also.

changeset ff931e3b691b9b7cbf4f07843d307417ef2998b0
Author: praveen.malv...@oracle.com
Date:   Mon, 19 Sep 2016 12:21:13 +0530

amfd: do not send duplicate removal of assignment, N-Way model [#2022]

AMFD asserted during lock operation on NG when its nodes are deplyed 
with
N-WAY model application.

When lock on NG operation is issued, AMFD sends quiesced state to 
active SIs
in all the three SUs. Responses for SU2 and SU3 comes and AMFD sends SU
level deletion to them. SU2 contains standby assignment of SI1 which is
still getting quiesced in SU1. Now SU1 faults with su-failover recovery 
when
SU2 is still not responded for deletion of assignments. AMFD tries to
perform recovery of SU1 and tries to fail-over SI1 of SU1 but could not 
find
any valid standby in any other SU. Since failover is not possible, AMFD
tries to delete all standby assignments of SI1. AMFD tries to send 
deletion
for SI1 to SU2 for which SU level deletion is already sent and it 
asserts.

Patch avoids sending duplicate deletion of assignments.


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


Testing Commands:
-
Tested as per ticket description.

Testing, Expected Results:
--
No AMFD assertion.

Conditions of Submission:
-
Ack from anybody before 5.1RC2.

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 0 of 1] Review Request for cpa: provide support for full range of timeout for CkptSynchronize API [#2044]

2016-09-19 Thread mahesh . valla 
Summary:cpa: provide support for full range of timeout for CkptSynchronize API 
[#2044] 
Review request for Trac Ticket(s): #2044
Peer Reviewer(s): Hoang
Pull request to: <>
Affected branch(es): 4.7 to 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 e9fd9dd168bc232daf8b46f768ba8a2aba2d41d0
Author: A V Mahesh 
Date:   Mon, 19 Sep 2016 11:12:35 +0530

cpa: provide support for full range of timeout for CkptSynchronize API
[#2044]

saCkptCheckpointSynchronize() API functions that take an explicit
timeout parameter are currently expect the user to always set this 
parameter
to MIN timeout value.

Now this patch implements support for the full range of timeout values
i.e. [0, SA_TIME_MAX] , since the underlying POSIX functions do support
them.


Complete diffstat:
--
 osaf/libs/agents/saf/cpa/cpa_api.c |  13 ++---
 1 files changed, 6 insertions(+), 7 deletions(-)


Testing Commands:
-
Pass => Zero timeout parameter to CkptSynchronize() API

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] cpa: provide support for full range of timeout for CkptSynchronize API [#2044]

2016-09-19 Thread mahesh . valla 
 osaf/libs/agents/saf/cpa/cpa_api.c |  13 ++---
 1 files changed, 6 insertions(+), 7 deletions(-)


saCkptCheckpointSynchronize() API  functions that take an explicit timeout 
parameter are currently
expect the user to always set this parameter to MIN timeout value.

Now this patch  implements support for the  full range of timeout values i.e. 
[0, SA_TIME_MAX] ,
since the underlying POSIX functions do support them.

diff --git a/osaf/libs/agents/saf/cpa/cpa_api.c 
b/osaf/libs/agents/saf/cpa/cpa_api.c
--- a/osaf/libs/agents/saf/cpa/cpa_api.c
+++ b/osaf/libs/agents/saf/cpa/cpa_api.c
@@ -4114,7 +4114,12 @@ SaAisErrorT saCkptCheckpointSynchronize(
SA_AIS_ERR_INVALID_PARAM, checkpointHandle);
rc =SA_AIS_ERR_INVALID_PARAM;
goto done;
-   }
+   } else if (timeout > (SA_TIME_ONE_MILLISECOND * 
MDS_MAX_TIMEOUT_MILLISECOND)) {
+   TRACE_4("Cpa CkptSynchronize: timeout > MDS_MAX_TIMEOUT setting 
to MDS max timeout value:%lld,ckptHandle:%llx",
+   (SA_TIME_ONE_MILLISECOND * 
MDS_MAX_TIMEOUT_MILLISECOND), ckptHandle);
+   timeout = (SA_TIME_ONE_MILLISECOND * 
MDS_MAX_TIMEOUT_MILLISECOND);
+   }
+
 
/* retrieve CPA CB */
m_CPA_RETRIEVE_CB(cb);
@@ -4202,12 +4207,6 @@ SaAisErrorT saCkptCheckpointSynchronize(
/* Convert the time from saTimeT to millisecs */
timeout = m_CPSV_CONVERT_SATIME_TEN_MILLI_SEC(timeout);
 
-   if (timeout < CPSV_WAIT_TIME) {
-   rc = SA_AIS_ERR_TIMEOUT;
-   TRACE_4("cpa CkptSynchronize Api failed with return 
value:%d,ckptHandle:%llx ", rc, checkpointHandle);
-   goto fail1;
-   }
-
m_NCS_UNLOCK(>cb_lock, NCS_LOCK_WRITE);
 
proc_rc = cpa_mds_msg_sync_send(cb->cpa_mds_hdl, 
&(gc_node->active_mds_dest), , _evt, timeout);

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