date:20161012

[devel] [PATCH 1 of 1] amfnd: queue PG track action msgs [#2115]

2016-10-12 Thread Gary Lee

 osaf/services/saf/amf/amfnd/di.cc |  8 ++--
 1 files changed, 2 insertions(+), 6 deletions(-)


After SC absence, active amfd will reject messages from 'veteran' amfnds until
its local amfnd has started. During this period, if a PG track action msg
is sent and rejected by amfd, it will cause the sending amfnd to lose sync
with amfd. So we should also queue this message to be re-sent.

diff --git a/osaf/services/saf/amf/amfnd/di.cc 
b/osaf/services/saf/amf/amfnd/di.cc
--- a/osaf/services/saf/amf/amfnd/di.cc
+++ b/osaf/services/saf/amf/amfnd/di.cc
@@ -1033,12 +1033,8 @@ uint32_t avnd_di_msg_send(AVND_CB *cb, A
if (!msg)
goto done;
 
-   /* Verify Ack nack and PG track action msgs are not buffered */
-   if (m_AVSV_N2D_MSG_IS_PG_TRACK_ACT(msg->info.avd)) {
-   /*send the response to AvD */
-   rc = avnd_mds_send(cb, msg, >avd_dest, 0);
-   goto done;
-   } else if (m_AVSV_N2D_MSG_IS_VER_ACK_NACK(msg->info.avd)) {
+   /* Verify Ack nack msgs are not buffered */
+   if (m_AVSV_N2D_MSG_IS_VER_ACK_NACK(msg->info.avd)) {
/*send the response to active AvD (In case MDS has not updated 
its
   tables by this time) */
TRACE_1("%s, Active AVD Adest: %" PRIu64, __FUNCTION__, 
cb->active_avd_adest);

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

[devel] [PATCH 1 of 1] smf: balanced upgrade, missing removal of exectrl copy [#2114]

2016-10-12 Thread Rafael Odzakow

 osaf/services/saf/smfsv/smfd/SmfExecControlHdl.cc  |  69 ++---
 osaf/services/saf/smfsv/smfd/SmfExecControlHdl.h   |  13 +--
 osaf/services/saf/smfsv/smfd/SmfUpgradeCampaign.cc |   1 +
 3 files changed, 51 insertions(+), 32 deletions(-)


diff --git a/osaf/services/saf/smfsv/smfd/SmfExecControlHdl.cc 
b/osaf/services/saf/smfsv/smfd/SmfExecControlHdl.cc
--- a/osaf/services/saf/smfsv/smfd/SmfExecControlHdl.cc
+++ b/osaf/services/saf/smfsv/smfd/SmfExecControlHdl.cc
@@ -41,7 +41,10 @@ SmfExecControlObjHandler::SmfExecControl
   m_exec_ctrl_name_ad(0),
   m_procExecMode_ad(0),
   m_numberOfSingleSteps_ad(0),
-  m_nodesForSingleStep_ad(0)
+  m_nodesForSingleStep_ad(0),
+  m_omHandle(0),
+  m_ownerHandle(0),
+  m_ccbHandle(0)
 {
   p_immutil_object = new SmfImmUtils; // Deleted by uninstall and in destructor
 }
@@ -57,8 +60,10 @@ SmfExecControlObjHandler::~SmfExecContro
   // overwritten the next time a copy has to be created
   //
   TRACE_ENTER();
-  if (p_immutil_object != 0)
+  if (p_immutil_object != NULL) {
 delete p_immutil_object;
+p_immutil_object = NULL;
+  }
   TRACE_LEAVE();
 }
 
@@ -94,8 +99,10 @@ bool SmfExecControlObjHandler::install()
 
 void SmfExecControlObjHandler::uninstall() {
   TRACE_ENTER();
-  if (p_immutil_object != 0)
+  if (p_immutil_object != NULL) {
 delete p_immutil_object;
+p_immutil_object = NULL;
+  }
   removeExecControlObjectCopy();
   TRACE_LEAVE();
 }
@@ -311,6 +318,20 @@ void SmfExecControlObjHandler::removeExe
 
   TRACE("Deleting object '%s'", c_openSafSmfExecControl_copy);
 
+  if (createImmOmHandles() == false) {
+TRACE("createCcbHandle Fail");
+  }
+
+  const SaNameT *names[2];
+  names[0] = _name;
+  names[1] = NULL;
+  ais_rc = immutil_saImmOmAdminOwnerSet(m_ownerHandle, names, SA_IMM_ONE);
+
+  if (ais_rc != SA_AIS_OK) {
+LOG_NO("%s - saImmOmAdminOwnerSet FAILED: %s",
+   __FUNCTION__, saf_error(ais_rc));
+  }
+
   ais_rc = immutil_saImmOmCcbObjectDelete(m_ccbHandle,
   _name);
   if (ais_rc != SA_AIS_OK) {
@@ -325,6 +346,8 @@ void SmfExecControlObjHandler::removeExe
   }
   }
 
+  finalizeImmOmHandles();
+
   TRACE_LEAVE();
 }
 
@@ -344,10 +367,8 @@ bool SmfExecControlObjHandler::copyExecC
   // 
   // Create handles needed
   //
-  rc = createAllImmHandles();
-  if (rc == false) {
-LOG_NO("%s: createAllImmHandles Fail", __FUNCTION__);
-return rc;
+  if (createImmOmHandles() == false) {
+TRACE("createCcbHandle Fail");
   }
 
   // 
@@ -398,6 +419,7 @@ bool SmfExecControlObjHandler::copyExecC
 }
   }
 
+  finalizeImmOmHandles();
   TRACE_LEAVE();
   return rc;
 }
@@ -428,7 +450,7 @@ void SmfExecControlObjHandler::saveAttri
  * 
  * @return false on Fail
  */
-bool SmfExecControlObjHandler::createAllImmHandles() {
+bool SmfExecControlObjHandler::createImmOmHandles() {
   SaAisErrorT ais_rc = SA_AIS_ERR_TRY_AGAIN;
   int timeout_try_cnt = 6;
   bool rc = true;
@@ -448,23 +470,6 @@ bool SmfExecControlObjHandler::createAll
 rc = false;
   }
 
-  // Accessors handle
-  if (rc == true) {
-timeout_try_cnt = 6;
-while (timeout_try_cnt > 0) {
-  ais_rc = immutil_saImmOmAccessorInitialize(m_omHandle,
- _accessorHandle);
-  if (ais_rc != SA_AIS_ERR_TIMEOUT)
-break;
-  timeout_try_cnt--;
-}
-if (ais_rc != SA_AIS_OK) {
-  LOG_NO("%s: saImmOmAccessorInitialize Fail %s",
- __FUNCTION__, saf_error(ais_rc));
-  rc = false;
-}
-  }
-
   // Admin owner handle
   if (rc == true) {
 timeout_try_cnt = 6;
@@ -504,3 +509,17 @@ bool SmfExecControlObjHandler::createAll
   TRACE_LEAVE();
   return rc;
 }
+
+void SmfExecControlObjHandler::finalizeImmOmHandles() {
+  if (m_omHandle != 0) {
+SaAisErrorT ais_rc = immutil_saImmOmFinalize(m_omHandle);
+if (ais_rc != SA_AIS_OK) {
+  LOG_NO("%s: immutil_saImmOmFinalize Fail %s",
+ __FUNCTION__, saf_error(ais_rc));
+}
+  }
+
+  m_omHandle = 0;
+  m_ownerHandle = 0;
+  m_ccbHandle = 0;
+}
diff --git a/osaf/services/saf/smfsv/smfd/SmfExecControlHdl.h 
b/osaf/services/saf/smfsv/smfd/SmfExecControlHdl.h
--- a/osaf/services/saf/smfsv/smfd/SmfExecControlHdl.h
+++ b/osaf/services/saf/smfsv/smfd/SmfExecControlHdl.h
@@ -85,7 +85,8 @@ class SmfExecControlObjHandler {
   bool readOpenSafSmfConfig();
   bool copyExecControlObject();
   void removeExecControlObjectCopy();
-  bool createAllImmHandles();
+  bool createImmOmHandles();
+  void finalizeImmOmHandles();
   void saveAttributeDescriptors();
 
 
@@ -104,7 +105,7 @@ class SmfExecControlObjHandler {
 
   
   // For OpenSafSmfExecControl object copy
-  const char* c_openSafSmfExecControl_copy = "openSafSmfExecControl_copy";
+  const char* c_openSafSmfExecControl_copy = 
"openSafSmfExecControl=SmfHdlCopy";
   SaImmAttrValuesT_2 **m_attributes;
   SaImmAttrValuesT_2 *m_exec_ctrl_name_ad;
   SaImmAttrValuesT_2

[devel] [PATCH 0 of 1] Review Request for smf: balanced upgrade, missing removal of exectrl copy [#2114]

2016-10-12 Thread Rafael Odzakow

Summary: smf: balanced upgrade, missing removal of exectrl copy
Review request for Trac Ticket(s): #2114
Peer Reviewer(s): lennart
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 servicesn
 OpenSAF servicesy
 Core libraries  n
 Samples n
 Tests   n
 Other   n


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

changeset ae5e92b72a913374d4ce4fd0795ade2724a8a483
Author: Rafael Odzakow 
Date:   Wed, 12 Oct 2016 13:03:30 +0200

smf: balanced upgrade, missing removal of exectrl copy [#2114]

When doing several bisu upgrades it was noticed that a IMM copy of 
execControl
object was not removed after a upgrade. Looking into the code there is 
a bug
which would case SMF to never remove this execControl copy.


Complete diffstat:
--
 osaf/services/saf/smfsv/smfd/SmfExecControlHdl.cc  |  69 
-
 osaf/services/saf/smfsv/smfd/SmfExecControlHdl.h   |  13 ++---
 osaf/services/saf/smfsv/smfd/SmfUpgradeCampaign.cc |   1 +
 3 files changed, 51 insertions(+), 32 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.


--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Re: [devel] [PATCH 1 of 1] smf: reading of IMM longdn attribute at the camapign admin operation [#2087]

2016-10-12 Thread Anders Widell

Hi!

Build fails on the opensaf-5.0.x branch after ticket [#2087] was pushed:

   CXX  osafsmfd-SmfCampaignInit.o
SmfCampaignInit.cc:230:2: error: ‘upActiter’ does not name a type
   upActiter = m_campInitAction.begin();
   ^
SmfCampaignInit.cc:231:2: error: expected unqualified-id before ‘while’
   while (upActiter != m_campInitAction.end()) {
   ^
SmfCampaignInit.cc: In member function ‘bool SmfCampaignInit::execute()’:
SmfCampaignInit.cc:228:5: error: control reaches end of non-void 
function [-Werror=return-type]
  }
  ^
cc1plus: all warnings being treated as errors
make[7]: *** [osafsmfd-SmfCampaignInit.o] Error 1

regards,

Anders Widell


On 10/10/2016 12:15 PM, Lennart Lund wrote:
> Ack
>
> Thanks
> Lennart
>
>> -Original Message-
>> From: reddy.neelaka...@oracle.com [mailto:reddy.neelaka...@oracle.com]
>> Sent: den 5 oktober 2016 14:32
>> To: Lennart Lund ; Rafael Odzakow
>> 
>> Cc: opensaf-devel@lists.sourceforge.net
>> Subject: [PATCH 1 of 1] smf: reading of IMM longdn attribute at the camapign
>> admin operation [#2087]
>>
>>   osaf/services/saf/smfsv/smfd/SmfCampaignInit.cc  |  14 +-
>>   osaf/services/saf/smfsv/smfd/smfd_campaign_oi.cc |  13 -
>>   2 files changed, 13 insertions(+), 14 deletions(-)
>>
>>
>> Reading of IMM longdn atribute is changed from camapaign init to oi admin
>> operation.
>> with this each admin-operation of smf will check if londn flag is update in 
>> smf
>>
>> diff --git a/osaf/services/saf/smfsv/smfd/SmfCampaignInit.cc
>> b/osaf/services/saf/smfsv/smfd/SmfCampaignInit.cc
>> --- a/osaf/services/saf/smfsv/smfd/SmfCampaignInit.cc
>> +++ b/osaf/services/saf/smfsv/smfd/SmfCampaignInit.cc
>> @@ -225,22 +225,10 @@ SmfCampaignInit::execute()
>>   return false;
>>   }
>>
>> -TRACE("1. Read_IMM_long_DN_config_and_set_control_block()");
>> -if
>> (!immUtil.read_IMM_long_DN_config_and_set_control_block(smfd_cb)) {
>> -LOG_ER("SmfCampaignInit: reading long DN config from IMM
>> FAILED");
>> -TRACE_LEAVE();
>> -return false;
>> -}
>>  std::list < SmfUpgradeAction * >::iterator upActiter;
>>  upActiter = m_campInitAction.begin();
>>  while (upActiter != m_campInitAction.end()) {
>> -TRACE("2. %s:
>> read_IMM_long_DN_config_and_set_control_block()",__FUNCTION__);
>> -if
>> (!immUtil.read_IMM_long_DN_config_and_set_control_block(smfd_cb)) {
>> -LOG_ER("SmfCampaignInit: reading long DN config
>> from IMM FAILED");
>> -TRACE_LEAVE();
>> -return false;
>> -}
>> -SaAisErrorT rc = (*upActiter)-
>>> execute(SmfCampaignThread::instance()->getImmHandle(),
>> +SaAisErrorT rc = (*upActiter)-
>>> execute(SmfCampaignThread::instance()->getImmHandle(),
>>  );
>>  if (rc != SA_AIS_OK) {
>>  LOG_ER("SmfCampaignInit init action %d failed,
>> rc=%s", (*upActiter)->getId(), saf_error(rc));
>> diff --git a/osaf/services/saf/smfsv/smfd/smfd_campaign_oi.cc
>> b/osaf/services/saf/smfsv/smfd/smfd_campaign_oi.cc
>> --- a/osaf/services/saf/smfsv/smfd/smfd_campaign_oi.cc
>> +++ b/osaf/services/saf/smfsv/smfd/smfd_campaign_oi.cc
>> @@ -72,6 +72,7 @@ static void saImmOiAdminOperationCallbac
>>const
>> SaImmAdminOperationParamsT_2 ** params)
>>   {
>>  SaAisErrorT rc = SA_AIS_OK;
>> +SmfImmUtils immutil;
>>
>>  TRACE_ENTER();
>>
>> @@ -83,6 +84,16 @@ static void saImmOiAdminOperationCallbac
>>  goto done;
>>  }
>>
>> +   //Read IMM configuration for long DNs and set cb data structure
>> +   //There is chance that long DN is configured in IMM not in SMF config
>> object
>> +TRACE("2. %s:
>> read_IMM_long_DN_config_and_set_control_block()",__FUNCTION__);
>> +if
>> (!immutil.read_IMM_long_DN_config_and_set_control_block(smfd_cb)) {
>> +LOG_ER("read_IMM_long_DN_config_and_set_control_block
>> FAIL");
>> +
>>  (void)immutil_saImmOiAdminOperationResult(immOiHandle,
>> invocation, SA_AIS_ERR_INVALID_PARAM);
>> +goto done;
>> +}
>> +
>> +
>>  /* Call admin operation and return result */
>>  rc = campaign->adminOperation(opId, params);
>>
>> @@ -898,7 +909,7 @@ uint32_t read_config_and_set_control_blo
>>
>>  //Read IMM configuration for long DNs and set cb data structure
>>   //The long DN info is configured in IMM not in SMF config object
>> -TRACE("3. %s:
>> read_IMM_long_DN_config_and_set_control_block()",__FUNCTION__);
>> +TRACE("1. %s:
>> read_IMM_long_DN_config_and_set_control_block()",__FUNCTION__);
>>  if (!immutil.read_IMM_long_DN_config_and_set_control_block(cb))
>> {
>>
>>  LOG_ER("read_IMM_long_DN_config_and_set_control_block
>>

Re: [devel] [PATCH 1 of 1] imm: Return error string when object is being created in another CCB [#2111]

2016-10-12 Thread Zoran Milinkovic

Hi Hung,

Reviewed the patch.
Ack from me.

Thanks,
Zoran

-Original Message-
From: Hung Nguyen [mailto:hung.d.ngu...@dektech.com.au] 
Sent: den 12 oktober 2016 05:02
To: Zoran Milinkovic ; 
reddy.neelaka...@oracle.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: [PATCH 1 of 1] imm: Return error string when object is being created 
in another CCB [#2111]

 osaf/services/saf/immsv/immnd/ImmModel.cc |  1 +
 1 files changed, 1 insertions(+), 0 deletions(-)


Return error string when object is being created in another CCB.

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
@@ -8079,6 +8079,7 @@ SaAisErrorT ImmModel::ccbObjectCreate(Im
 TRACE_7("ERR_EXIST: object '%s' is already registered "
 "for creation in a ccb, but not applied yet",
 objectName.c_str());
+setCcbErrorString(ccb, "IMM: ERR_EXIST: object is already 
registered for creation in a ccb");
 }
 } else {
 TRACE_7("ERR_EXIST: object '%s' exists", objectName.c_str());

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Re: [devel] [PATCH 1 of 1] smfd: handle failed si-swap of middleware [#1605]

2016-10-12 Thread Rafael Odzakow

ACK


On 10/10/2016 01:46 PM, Neelakanta Reddy wrote:
> Hi Alex,
>
> Reviewed and tested the patch.
> Ack.
>
> /Neel.
>
> On 2016/10/07 11:10 PM, Alex Jones wrote:
>> osaf/services/saf/smfsv/smfd/SmfStepState.cc|  13 
>>   osaf/services/saf/smfsv/smfd/SmfUpgradeProcedure.cc |  68 
>> +---
>>   osaf/services/saf/smfsv/smfd/SmfUpgradeProcedure.hh |   6 +
>>   3 files changed, 76 insertions(+), 11 deletions(-)
>>
>>
>> Sep 27 00:34:14 q50-s1 osafsmfd[6667]: NO SA_AMF_ADMIN_SI_SWAP [rc=1] 
>> successfully initiated
>> Sep 27 00:34:15 q50-s1 osafimmnd[6571]: NO ERR_BAD_OPERATION: 
>> Mismatch on administrative owner '' != 'SMFSERVICE'
>> Sep 27 00:34:17 q50-s1 osafsmfd[6667]: NO Fail to invoke admin 
>> operation, rc=SA_AIS_ERR_BAD_OPERATION (20). 
>> dn=[safSi=SC-2N,safApp=OpenSAF], opId=[7]
>> Sep 27 00:34:17 q50-s1 osafsmfd[6667]: NO Admin op 
>> SA_AMF_ADMIN_SI_SWAP fail [rc = 20]
>> Sep 27 00:34:17 q50-s1 osafsmfd[6667]: NO CAMP: Procedure 
>> safSmfProc=RollingUpgrade returned FAILED
>> Sep 27 00:36:14 q50-s1 osafsmfd[6667]: NO Campaign thread does not 
>> disappear within 120 seconds after SA_AMF_ADMIN_SI_SWAP, the 
>> operation was assumed failed.
>> Sep 27 00:36:14 q50-s1 kernel: [14934029.531187] osafsmfd[32024]: 
>> segfault at 4 ip 004425b6 sp 7f67f7ffe1c0 error 4 in 
>> osafsmfd[40+9a000]
>> Sep 27 00:36:14 q50-s1 osafamfnd[6649]: NO 
>> 'safComp=SMF,safSu=SC-1,safSg=2N,safApp=OpenSAF' faulted due to 
>> 'avaDown' : Recovery is 'nodeFailfast'
>> Sep 27 00:36:14 q50-s1 osafamfnd[6649]: ER 
>> safComp=SMF,safSu=SC-1,safSg=2N,safApp=OpenSAF Faulted due to:avaDown 
>> Recovery is:nodeFailfast
>>
>> There are a few problems here. One is that the SmfSwapThread is 
>> pointing to a
>> deleted procedure when the original active controller is reassigned 
>> active. The
>> second problem is that a new SmfSwapThread is created when the 
>> original active
>> controller is reassigned active, so now there are two running. The 
>> first thread
>> tries to use its proc pointer (which has been deleted when the 
>> original active
>> goes to quiesced) and causes the segfault.
>>
>> The proposed solution is a little different from that proposed in the 
>> ticket
>> description. This solution proposes to use the existence of the 
>> SmfSwapThread as
>> a test. When the original active controller is reassigned active 
>> because the
>> si-swap failed, it will still remove the RestartIndicator as it does 
>> now. But,
>> if the SmfSwapThread is still running, it will not create a new one, 
>> but update
>> it with the recreated procedure pointer, and let it handle the 
>> si-swap timeout.
>> Then it will report the error. I believe this solution is backwards 
>> compatible
>> because no IMM changes are made like the ones proposed in the ticket.
>>
>> diff --git a/osaf/services/saf/smfsv/smfd/SmfStepState.cc 
>> b/osaf/services/saf/smfsv/smfd/SmfStepState.cc
>> --- a/osaf/services/saf/smfsv/smfd/SmfStepState.cc
>> +++ b/osaf/services/saf/smfsv/smfd/SmfStepState.cc
>> @@ -424,6 +424,19 @@ SmfStepStateUndone::execute(SmfUpgradeSt
>>   {
>>   TRACE_ENTER();
>>   +if (i_step->calculateStepType() != SA_AIS_OK) {
>> +LOG_ER("SmfStepStateUndone: Failed to calculate step 
>> type");
>> +changeState(i_step, SmfStepStateFailed::instance());
>> +TRACE_LEAVE();
>> +return SMF_STEP_FAILED;
>> +}
>> +
>> +if (i_step->getSwitchOver() == true) {
>> +TRACE("Switch over is needed in this step");
>> +TRACE_LEAVE();
>> +return SMF_STEP_SWITCHOVER;
>> +}
>> +
>>   i_step->setRetryCount(0); /* Reset the retry counter */
>>   changeState(i_step, SmfStepStateExecuting::instance());
>>   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
>> @@ -482,12 +482,17 @@ SmfUpgradeProcedure::switchOver()
>>   osafassert(0);
>>   }
>>   -TRACE("SmfUpgradeProcedure::switchOver: Create the restart 
>> indicator");
>> - 
>> SmfCampaignThread::instance()->campaign()->getUpgradeCampaign()->createSmfRestartIndicator();
>> -
>> -SmfSwapThread *swapThread = new SmfSwapThread(this);
>> -TRACE("SmfUpgradeProcedure::switchOver, Starting SI_SWAP thread");
>> -swapThread->start();
>> +if (!SmfSwapThread::running()) {
>> +TRACE("SmfUpgradeProcedure::switchOver: Create the restart 
>> indicator");
>> + 
>> SmfCampaignThread::instance()->campaign()->getUpgradeCampaign()->createSmfRestartIndicator();
>> +
>> +SmfSwapThread *swapThread = new SmfSwapThread(this);
>> +TRACE("SmfUpgradeProcedure::switchOver, Starting SI_SWAP 
>> thread");
>> +swapThread->start();
>> +} else {
>> +

Re: [devel] [PATCH 1 of 1] immtool: Don't finalize admo if it hasn't been initialized [#2107]

2016-10-12 Thread Neelakanta Reddy

Hi Hung,

Reviewed and tested the patch.
Ack.

/Neel.

On 2016/10/11 09:27 AM, Hung Nguyen wrote:
>   osaf/tools/safimm/immcfg/imm_cfg.c |  2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
>
> Don't finalize admo if it hasn't been initialized.
>
> diff --git a/osaf/tools/safimm/immcfg/imm_cfg.c 
> b/osaf/tools/safimm/immcfg/imm_cfg.c
> --- a/osaf/tools/safimm/immcfg/imm_cfg.c
> +++ b/osaf/tools/safimm/immcfg/imm_cfg.c
> @@ -1703,7 +1703,7 @@ static int imm_operation(int argc, char
>   }
>   }
>   
> - if(useAdminOwner) {
> + if(useAdminOwner && adminOwnerName) {
>   error = immutil_saImmOmAdminOwnerFinalize(ownerHandle);
>   if (SA_AIS_OK != error) {
>   fprintf(stderr, "error - 
> saImmOmAdminOwnerFinalize FAILED: %s\n", saf_error(error));


--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Re: [devel] [PATCH 1 of 1] log: fix saLogStreamOpen_2 returns SA_AIS_ERR_BAD_OPERATION during si-swap [#2093]

2016-10-12 Thread A V Mahesh

Ok ACK , not tested.

-AVM


On 10/12/2016 12:42 PM, Vu Minh Nguyen wrote:
> Hi Mahesh,
>
> I added one comment in "PATCH 0 of 1", telling reason why not using ha
> state.
> Here it is.
> //>
>   Previous fix is not correct as ha state in cb is not changed
>   when entering QUIESED state. So, basing on ha_state variable
>   is not correct. Using is_quiesced_set flag instead.
> //<
>
> Regards, Vu
>
>> -Original Message-
>> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
>> Sent: Wednesday, October 12, 2016 2:10 PM
>> To: Vu Minh Nguyen ;
>> lennart.l...@ericsson.com
>> Cc: opensaf-devel@lists.sourceforge.net
>> Subject: Re: [PATCH 1 of 1] log: fix saLogStreamOpen_2 returns
>> SA_AIS_ERR_BAD_OPERATION during si-swap [#2093]
>>
>> Hi Vu,
>>
>> Can we use  (cb->ha_state != SA_AMF_HA_ACTIVE) instead of
>> `cb->is_quiesced_set == true`
>> as if this is a agent response, so it will have more readability .
>>
>> -AVM
>>
>> On 10/11/2016 10:36 AM, Vu Minh Nguyen wrote:
>>>osaf/services/saf/logsv/lgs/lgs_evt.cc |   12 +++-
>>>tests/logsv/tet_saLogStreamOpen_2.c|  101
>> +
>>>2 files changed, 111 insertions(+), 2 deletions(-)
>>>
>>>
>>> During si-swap, old active logd will enter QUEISED HA state, then
>> STANDBY.
>>> In QUEISED state, IMM OI implementer will be cleared.
>>> If opening a new application stream or closing an app stream while
>>> the HA state is in QUEISED, log client will get BAD_OPERATION error.
>>>
>>> In such transition state, logsv should return TRY_AGAIN to clients.
>>> Once the transition is completely done, above requests will be served.
>>>
>>> 02 test cases are created to verify the fix, test case 53 & 54 of suite
> #2.
>>> 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
>>> @@ -1021,7 +1021,11 @@ static uint32_t proc_stream_open_msg(lgs
>>>}
>>>  } else {
>>>/* Stream does not exist */
>>> -if (cb->immOiHandle == 0) {
>>> +
>>> +// Client should try again when role changes is in transition
> (queued).
>>> +// This check is to avoid the client getting SA_AIS_BAD_OPERATION
>>> +// as there is no IMM OI implementer set.
>>> +if (cb->immOiHandle == 0 || cb->is_quiesced_set == true) {
>>>  TRACE("IMM service unavailable, open stream failed");
>>>  ais_rv = SA_AIS_ERR_TRY_AGAIN;
>>>  goto snd_rsp;
>>> @@ -1153,7 +1157,11 @@ static uint32_t proc_stream_close_msg(lg
>>>goto snd_rsp;
>>>  }
>>>
>>> -  if ((stream->streamType == STREAM_TYPE_APPLICATION) && (cb-
>>> immOiHandle == 0)) {
>>> +  // Client should try again when role changes is in transition
> (queued).
>>> +  // This check is to avoid the client getting SA_AIS_BAD_OPERATION
>>> +  // as there is no IMM OI implementer set.
>>> +  if ((stream->streamType == STREAM_TYPE_APPLICATION) &&
>>> +  (cb->immOiHandle == 0 || cb->is_quiesced_set == true)) {
>>>TRACE("IMM service unavailable, close stream failed");
>>>ais_rc = SA_AIS_ERR_TRY_AGAIN;
>>>goto snd_rsp;
>>> diff --git a/tests/logsv/tet_saLogStreamOpen_2.c
>> b/tests/logsv/tet_saLogStreamOpen_2.c
>>> --- a/tests/logsv/tet_saLogStreamOpen_2.c
>>> +++ b/tests/logsv/tet_saLogStreamOpen_2.c
>>> @@ -795,6 +795,105 @@ void saLogMultiThreadMultiInit(void)
>>> test_validate(rc, SA_AIS_OK);
>>>}
>>>
>>> +//
>>> +// This test to verify Open call gets BAD_OPERATION or not during
> si-swap
>>> +//
>>> +void saLogStreamOpen_BadOp(void)
>>> +{
>>> +   SaAisErrorT rc;
>>> +   char command[1000];
>>> +   int loop;
>>> +
>>> +
>>> +   rc = saLogInitialize(, , );
>>> +   if (rc != SA_AIS_OK) {
>>> +   fprintf(stderr, "Failed at saLogInitialize: %d \n",
> (int)rc);
>>> +   test_validate(rc, SA_AIS_OK);
>>> +   return;
>>> +   }
>>> +
>>> +   // Perform si-swap in background
>>> +   sprintf(command, "amf-adm si-swap safSi=SC-2N,safApp=OpenSAF
>> &");
>>> +   loop = system(command);
>>> +
>>> +   // To make Open probably processed righ after logsv HA is set to
>> QUEISED state.
>>> +   // This test is created to reproduce the issue reported in ticket
> #2093
>>> +   usleep(100*1000);
>>> +
>>> +   loop = 100;
>>> +tryagain:
>>> +   rc = saLogStreamOpen_2(logHandle, ,
>> ,
>>> +  SA_LOG_STREAM_CREATE, SA_TIME_ONE_SECOND,
>> );
>>> +   if (rc == SA_AIS_ERR_TRY_AGAIN && loop--) {
>>> +   usleep(100*1000);
>>> +   goto tryagain;
>>> +   }
>>> +
>>> +   test_validate(rc, SA_AIS_OK);
>>> +
>>> +   rc = saLogFinalize(logHandle);
>>> +   if (rc != SA_AIS_OK) {
>>> +   fprintf(stderr, "Failed to call salogFinalize: %d \n", (int)
> rc);
>>> +   }
>>> +}
>>> +
>>> +//
>>> +// This test to verify Close call gets BAD_OPERATION in syslog or not
>> during si-swap
>>> +//
>>> +void saLogStreamClose_BadOp(void)
>>> +{

Re: [devel] [PATCH 1 of 1] log: fix failure to create directory when changing logRootDirectory [#2054]

2016-10-12 Thread A V Mahesh

Hi Vu,

If possible  use  global `lgs_conf.logRootDirectory`  after the call 
`lgs_rootpathconf_set(new_logRootDirectory)`

instead of   passed  `new_logRootDirectory` parameter.

-AVM

On 10/12/2016 12:39 PM, Vu Minh Nguyen wrote:
> Hi Mahesh,
>
> See my comment inline.
>
> Regards, Vu
>
>> -Original Message-
>> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
>> Sent: Wednesday, October 12, 2016 1:52 PM
>> To: Lennart Lund ; Vu Minh Nguyen
>> 
>> Cc: opensaf-devel@lists.sourceforge.net
>> Subject: Re: [PATCH 1 of 1] log: fix failure to create directory when
> changing
>> logRootDirectory [#2054]
>>
>> Hi VU,
>>
>> ACK with following :
>>
>> Not an issue but It is good to use lgs_conf.logRootDirectory insted of
>> new_logRootDirectory
> [Vu] Not sure if I got your point here. ` new_logRootDirectory` is an
> parameter name.
> Using `lgs_conf.logRootDirectory` is not correct as the code line `+
> lgs_rootpathconf_set(new_logRootDirectory);` intents
> to update the global `lgs_conf.logRootDirectory` to `new_logRootDirectory`.
>
>> for the remaining part of logRootDirectory_filemove()  , because
>> mutex_unlocked for  lgs_conf.logRootDirectory
>>
>> -AVM
>>
>> On 10/10/2016 4:48 PM, Lennart Lund wrote:
>>> Hi Vu
>>>
>>> Ack with comment. See below
>>>
>>> /***
>>>* Support older check-point protocols prior to version 5
>>>*/
>>> /**
>>>* Used for updating logRootDirectory on standby when check-pointed
>>>*
>>>* Set the logRootDirectory parameter in the lgs_conf struct
>>>* Used for holding data from config object
>>>*
>>>* @param root_path_str
>>>*/
>>> void lgs_rootpathconf_set(const std::string _path_str) {
>>>
>>> [Lennart]
>>> The comments and maybe the place of this function seems incorrect since
>> this function is not only used on standby server
>>> Thanks
>>> Lennart
>>>
>>>
 -Original Message-
 From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
 Sent: den 21 september 2016 10:08
 To: Lennart Lund ;
>> mahesh.va...@oracle.com
 Cc: opensaf-devel@lists.sourceforge.net
 Subject: [PATCH 1 of 1] log: fix failure to create directory when
> changing
 logRootDirectory [#2054]

osaf/services/saf/logsv/lgs/lgs_imm.cc |   3 +
tests/logsv/tet_LogOiOps.c |  82
 ++
2 files changed, 85 insertions(+), 0 deletions(-)


 When changing `logRootDirectory`, the new directory was not updated to
 global `lgs_conf.logRootDirectory`, therefore all refering to new
> directory
 got the old value.

 This patch adds code to make sure new directory updated.
 And one test case #03 of suite #5 are added to verify this case.

 diff --git a/osaf/services/saf/logsv/lgs/lgs_imm.cc
 b/osaf/services/saf/logsv/lgs/lgs_imm.cc
 --- a/osaf/services/saf/logsv/lgs/lgs_imm.cc
 +++ b/osaf/services/saf/logsv/lgs/lgs_imm.cc
 @@ -1858,6 +1858,9 @@ void logRootDirectory_filemove(
stream = log_stream_get_by_id(--num);
  }

 +  // Change logrootDirectory to new_logRootDirectory
 +  lgs_rootpathconf_set(new_logRootDirectory);
 +
  /* Create new files at new path
   */
  num = get_number_of_streams();
 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
 @@ -1023,6 +1023,87 @@ done:
}

/**
 + * CCB Object Modify, root directory. Path exist. OK
 + * Result shall be OK
 + */
 +void change_root_path(void)
 +{
 +  int rc = 0, tst_stat = 0;
 +  char command[256];
 +  char tstdir[256];
 +
 +  /* Path to test directory */
 +  sprintf(tstdir, "%s/croot", log_root_path);
 +
 +  // Remove if the test folder is exist
 +  sprintf(command, "rm -rf %s/", tstdir);
 +  rc = tet_system(command);
 +
 +  /* Create test directory */
 +  sprintf(command, "mkdir -p %s", tstdir);
 +  rc = tet_system(command);
 +  if (rc != 0) {
 +  fprintf(stderr, "'%s' Fail rc=%d\n", command, rc);
 +  tst_stat = 1;
 +  goto done;
 +  }
 +
 +  /* Make sure it can be accessed by server */
 +  sprintf(command, "chmod ugo+w,ugo+r %s", tstdir);
 +  rc = tet_system(command);
 +  if (rc != 0) {
 +  fprintf(stderr, "'%s' Fail rc=%d\n", command, rc);
 +  tst_stat = 1;
 +  goto done;
 +  }
 +
 +  sprintf(command, "immcfg -c SaLogStreamConfig
 safLgStrCfg=testRoot "
 +  "-a saLogStreamPathName=./testRoot -a
 saLogStreamFileName=testRoot");
 +  rc = tet_system(command);
 +  if (rc != 0) {
 +  fprintf(stderr, "'%s' Fail rc=%d\n", command, rc);
 +  tst_stat = 1;
 +  goto done;
 +

Re: [devel] [PATCH 1 of 1] log: fix saLogStreamOpen_2 returns SA_AIS_ERR_BAD_OPERATION during si-swap [#2093]

2016-10-12 Thread Vu Minh Nguyen

Hi Mahesh,

I added one comment in "PATCH 0 of 1", telling reason why not using ha
state.
Here it is.
//>
 Previous fix is not correct as ha state in cb is not changed
 when entering QUIESED state. So, basing on ha_state variable
 is not correct. Using is_quiesced_set flag instead.
//<

Regards, Vu

> -Original Message-
> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
> Sent: Wednesday, October 12, 2016 2:10 PM
> To: Vu Minh Nguyen ;
> lennart.l...@ericsson.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1 of 1] log: fix saLogStreamOpen_2 returns
> SA_AIS_ERR_BAD_OPERATION during si-swap [#2093]
> 
> Hi Vu,
> 
> Can we use  (cb->ha_state != SA_AMF_HA_ACTIVE) instead of
> `cb->is_quiesced_set == true`
> as if this is a agent response, so it will have more readability .
> 
> -AVM
> 
> On 10/11/2016 10:36 AM, Vu Minh Nguyen wrote:
> >   osaf/services/saf/logsv/lgs/lgs_evt.cc |   12 +++-
> >   tests/logsv/tet_saLogStreamOpen_2.c|  101
> +
> >   2 files changed, 111 insertions(+), 2 deletions(-)
> >
> >
> > During si-swap, old active logd will enter QUEISED HA state, then
> STANDBY.
> > In QUEISED state, IMM OI implementer will be cleared.
> > If opening a new application stream or closing an app stream while
> > the HA state is in QUEISED, log client will get BAD_OPERATION error.
> >
> > In such transition state, logsv should return TRY_AGAIN to clients.
> > Once the transition is completely done, above requests will be served.
> >
> > 02 test cases are created to verify the fix, test case 53 & 54 of suite
#2.
> >
> > 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
> > @@ -1021,7 +1021,11 @@ static uint32_t proc_stream_open_msg(lgs
> >   }
> > } else {
> >   /* Stream does not exist */
> > -if (cb->immOiHandle == 0) {
> > +
> > +// Client should try again when role changes is in transition
(queued).
> > +// This check is to avoid the client getting SA_AIS_BAD_OPERATION
> > +// as there is no IMM OI implementer set.
> > +if (cb->immOiHandle == 0 || cb->is_quiesced_set == true) {
> > TRACE("IMM service unavailable, open stream failed");
> > ais_rv = SA_AIS_ERR_TRY_AGAIN;
> > goto snd_rsp;
> > @@ -1153,7 +1157,11 @@ static uint32_t proc_stream_close_msg(lg
> >   goto snd_rsp;
> > }
> >
> > -  if ((stream->streamType == STREAM_TYPE_APPLICATION) && (cb-
> >immOiHandle == 0)) {
> > +  // Client should try again when role changes is in transition
(queued).
> > +  // This check is to avoid the client getting SA_AIS_BAD_OPERATION
> > +  // as there is no IMM OI implementer set.
> > +  if ((stream->streamType == STREAM_TYPE_APPLICATION) &&
> > +  (cb->immOiHandle == 0 || cb->is_quiesced_set == true)) {
> >   TRACE("IMM service unavailable, close stream failed");
> >   ais_rc = SA_AIS_ERR_TRY_AGAIN;
> >   goto snd_rsp;
> > diff --git a/tests/logsv/tet_saLogStreamOpen_2.c
> b/tests/logsv/tet_saLogStreamOpen_2.c
> > --- a/tests/logsv/tet_saLogStreamOpen_2.c
> > +++ b/tests/logsv/tet_saLogStreamOpen_2.c
> > @@ -795,6 +795,105 @@ void saLogMultiThreadMultiInit(void)
> > test_validate(rc, SA_AIS_OK);
> >   }
> >
> > +//
> > +// This test to verify Open call gets BAD_OPERATION or not during
si-swap
> > +//
> > +void saLogStreamOpen_BadOp(void)
> > +{
> > +   SaAisErrorT rc;
> > +   char command[1000];
> > +   int loop;
> > +
> > +
> > +   rc = saLogInitialize(, , );
> > +   if (rc != SA_AIS_OK) {
> > +   fprintf(stderr, "Failed at saLogInitialize: %d \n",
(int)rc);
> > +   test_validate(rc, SA_AIS_OK);
> > +   return;
> > +   }
> > +
> > +   // Perform si-swap in background
> > +   sprintf(command, "amf-adm si-swap safSi=SC-2N,safApp=OpenSAF
> &");
> > +   loop = system(command);
> > +
> > +   // To make Open probably processed righ after logsv HA is set to
> QUEISED state.
> > +   // This test is created to reproduce the issue reported in ticket
#2093
> > +   usleep(100*1000);
> > +
> > +   loop = 100;
> > +tryagain:
> > +   rc = saLogStreamOpen_2(logHandle, ,
> ,
> > +  SA_LOG_STREAM_CREATE, SA_TIME_ONE_SECOND,
> );
> > +   if (rc == SA_AIS_ERR_TRY_AGAIN && loop--) {
> > +   usleep(100*1000);
> > +   goto tryagain;
> > +   }
> > +
> > +   test_validate(rc, SA_AIS_OK);
> > +
> > +   rc = saLogFinalize(logHandle);
> > +   if (rc != SA_AIS_OK) {
> > +   fprintf(stderr, "Failed to call salogFinalize: %d \n", (int)
rc);
> > +   }
> > +}
> > +
> > +//
> > +// This test to verify Close call gets BAD_OPERATION in syslog or not
> during si-swap
> > +//
> > +void saLogStreamClose_BadOp(void)
> > +{
> > +   SaAisErrorT rc;
> > +   char command[1000];
> > +   int loop;
> > +
> > +   rc = saLogInitialize(, , );
> > +   if (rc != SA_AIS_OK) {
> > +

Re: [devel] [PATCH 1 of 1] log: fix saLogStreamOpen_2 returns SA_AIS_ERR_BAD_OPERATION during si-swap [#2093]

2016-10-12 Thread A V Mahesh

Hi Vu,

Can we use  (cb->ha_state != SA_AMF_HA_ACTIVE) instead of 
`cb->is_quiesced_set == true`
as if this is a agent response, so it will have more readability .

-AVM

On 10/11/2016 10:36 AM, Vu Minh Nguyen wrote:
>   osaf/services/saf/logsv/lgs/lgs_evt.cc |   12 +++-
>   tests/logsv/tet_saLogStreamOpen_2.c|  101 
> +
>   2 files changed, 111 insertions(+), 2 deletions(-)
>
>
> During si-swap, old active logd will enter QUEISED HA state, then STANDBY.
> In QUEISED state, IMM OI implementer will be cleared.
> If opening a new application stream or closing an app stream while
> the HA state is in QUEISED, log client will get BAD_OPERATION error.
>
> In such transition state, logsv should return TRY_AGAIN to clients.
> Once the transition is completely done, above requests will be served.
>
> 02 test cases are created to verify the fix, test case 53 & 54 of suite #2.
>
> 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
> @@ -1021,7 +1021,11 @@ static uint32_t proc_stream_open_msg(lgs
>   }
> } else {
>   /* Stream does not exist */
> -if (cb->immOiHandle == 0) {
> +
> +// Client should try again when role changes is in transition (queued).
> +// This check is to avoid the client getting SA_AIS_BAD_OPERATION
> +// as there is no IMM OI implementer set.
> +if (cb->immOiHandle == 0 || cb->is_quiesced_set == true) {
> TRACE("IMM service unavailable, open stream failed");
> ais_rv = SA_AIS_ERR_TRY_AGAIN;
> goto snd_rsp;
> @@ -1153,7 +1157,11 @@ static uint32_t proc_stream_close_msg(lg
>   goto snd_rsp;
> }
>   
> -  if ((stream->streamType == STREAM_TYPE_APPLICATION) && (cb->immOiHandle == 
> 0)) {
> +  // Client should try again when role changes is in transition (queued).
> +  // This check is to avoid the client getting SA_AIS_BAD_OPERATION
> +  // as there is no IMM OI implementer set.
> +  if ((stream->streamType == STREAM_TYPE_APPLICATION) &&
> +  (cb->immOiHandle == 0 || cb->is_quiesced_set == true)) {
>   TRACE("IMM service unavailable, close stream failed");
>   ais_rc = SA_AIS_ERR_TRY_AGAIN;
>   goto snd_rsp;
> diff --git a/tests/logsv/tet_saLogStreamOpen_2.c 
> b/tests/logsv/tet_saLogStreamOpen_2.c
> --- a/tests/logsv/tet_saLogStreamOpen_2.c
> +++ b/tests/logsv/tet_saLogStreamOpen_2.c
> @@ -795,6 +795,105 @@ void saLogMultiThreadMultiInit(void)
>   test_validate(rc, SA_AIS_OK);
>   }
>   
> +//
> +// This test to verify Open call gets BAD_OPERATION or not during si-swap
> +//
> +void saLogStreamOpen_BadOp(void)
> +{
> + SaAisErrorT rc;
> + char command[1000];
> + int loop;
> +
> +
> + rc = saLogInitialize(, , );
> + if (rc != SA_AIS_OK) {
> + fprintf(stderr, "Failed at saLogInitialize: %d \n", (int)rc);
> + test_validate(rc, SA_AIS_OK);
> + return;
> + }
> +
> + // Perform si-swap in background
> + sprintf(command, "amf-adm si-swap safSi=SC-2N,safApp=OpenSAF &");
> + loop = system(command);
> +
> + // To make Open probably processed righ after logsv HA is set to 
> QUEISED state.
> + // This test is created to reproduce the issue reported in ticket #2093
> + usleep(100*1000);
> +
> + loop = 100;
> +tryagain:
> + rc = saLogStreamOpen_2(logHandle, , 
> ,
> +SA_LOG_STREAM_CREATE, SA_TIME_ONE_SECOND, 
> );
> + if (rc == SA_AIS_ERR_TRY_AGAIN && loop--) {
> + usleep(100*1000);
> + goto tryagain;
> + }
> +
> + test_validate(rc, SA_AIS_OK);
> +
> + rc = saLogFinalize(logHandle);
> + if (rc != SA_AIS_OK) {
> + fprintf(stderr, "Failed to call salogFinalize: %d \n", (int) 
> rc);
> + }
> +}
> +
> +//
> +// This test to verify Close call gets BAD_OPERATION in syslog or not during 
> si-swap
> +//
> +void saLogStreamClose_BadOp(void)
> +{
> + SaAisErrorT rc;
> + char command[1000];
> + int loop;
> +
> + rc = saLogInitialize(, , );
> + if (rc != SA_AIS_OK) {
> + fprintf(stderr, "Failed at saLogInitialize: %d \n", (int)rc);
> + test_validate(rc, SA_AIS_OK);
> + return;
> + }
> +
> +
> + loop = 100;
> +tryagain:
> + rc = saLogStreamOpen_2(logHandle, , 
> ,
> +SA_LOG_STREAM_CREATE, SA_TIME_ONE_SECOND, 
> );
> + if (rc == SA_AIS_ERR_TRY_AGAIN && loop--) {
> + usleep(100*1000);
> + goto tryagain;
> + }
> +
> + if (rc != SA_AIS_OK) {
> + fprintf(stderr, "Failed to open stream (%d)\n", rc);
> + test_validate(rc, SA_AIS_OK);
> + goto done;
> + }
> +
> + // Perform si-swap in background
> + sprintf(command, "amf-adm si-swap safSi=SC-2N,safApp=OpenSAF &");
> + loop = system(command);
> +
> + // To make

Re: [devel] [PATCH 1 of 1] log: fix failure to create directory when changing logRootDirectory [#2054]

2016-10-12 Thread Vu Minh Nguyen

Hi Mahesh,

See my comment inline.

Regards, Vu

> -Original Message-
> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
> Sent: Wednesday, October 12, 2016 1:52 PM
> To: Lennart Lund ; Vu Minh Nguyen
> 
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1 of 1] log: fix failure to create directory when
changing
> logRootDirectory [#2054]
> 
> Hi VU,
> 
> ACK with following :
> 
> Not an issue but It is good to use lgs_conf.logRootDirectory insted of
> new_logRootDirectory
[Vu] Not sure if I got your point here. ` new_logRootDirectory` is an
parameter name.
Using `lgs_conf.logRootDirectory` is not correct as the code line `+
lgs_rootpathconf_set(new_logRootDirectory);` intents
to update the global `lgs_conf.logRootDirectory` to `new_logRootDirectory`.

> 
> for the remaining part of logRootDirectory_filemove()  , because
> mutex_unlocked for  lgs_conf.logRootDirectory
> 
> -AVM
> 
> On 10/10/2016 4:48 PM, Lennart Lund wrote:
> > Hi Vu
> >
> > Ack with comment. See below
> >
> > /***
> >   * Support older check-point protocols prior to version 5
> >   */
> > /**
> >   * Used for updating logRootDirectory on standby when check-pointed
> >   *
> >   * Set the logRootDirectory parameter in the lgs_conf struct
> >   * Used for holding data from config object
> >   *
> >   * @param root_path_str
> >   */
> > void lgs_rootpathconf_set(const std::string _path_str) {
> >
> > [Lennart]
> > The comments and maybe the place of this function seems incorrect since
> this function is not only used on standby server
> >
> > Thanks
> > Lennart
> >
> >
> >> -Original Message-
> >> From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> >> Sent: den 21 september 2016 10:08
> >> To: Lennart Lund ;
> mahesh.va...@oracle.com
> >> Cc: opensaf-devel@lists.sourceforge.net
> >> Subject: [PATCH 1 of 1] log: fix failure to create directory when
changing
> >> logRootDirectory [#2054]
> >>
> >>   osaf/services/saf/logsv/lgs/lgs_imm.cc |   3 +
> >>   tests/logsv/tet_LogOiOps.c |  82
> >> ++
> >>   2 files changed, 85 insertions(+), 0 deletions(-)
> >>
> >>
> >> When changing `logRootDirectory`, the new directory was not updated to
> >> global `lgs_conf.logRootDirectory`, therefore all refering to new
directory
> >> got the old value.
> >>
> >> This patch adds code to make sure new directory updated.
> >> And one test case #03 of suite #5 are added to verify this case.
> >>
> >> diff --git a/osaf/services/saf/logsv/lgs/lgs_imm.cc
> >> b/osaf/services/saf/logsv/lgs/lgs_imm.cc
> >> --- a/osaf/services/saf/logsv/lgs/lgs_imm.cc
> >> +++ b/osaf/services/saf/logsv/lgs/lgs_imm.cc
> >> @@ -1858,6 +1858,9 @@ void logRootDirectory_filemove(
> >>   stream = log_stream_get_by_id(--num);
> >> }
> >>
> >> +  // Change logrootDirectory to new_logRootDirectory
> >> +  lgs_rootpathconf_set(new_logRootDirectory);
> >> +
> >> /* Create new files at new path
> >>  */
> >> num = get_number_of_streams();
> >> 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
> >> @@ -1023,6 +1023,87 @@ done:
> >>   }
> >>
> >>   /**
> >> + * CCB Object Modify, root directory. Path exist. OK
> >> + * Result shall be OK
> >> + */
> >> +void change_root_path(void)
> >> +{
> >> +  int rc = 0, tst_stat = 0;
> >> +  char command[256];
> >> +  char tstdir[256];
> >> +
> >> +  /* Path to test directory */
> >> +  sprintf(tstdir, "%s/croot", log_root_path);
> >> +
> >> +  // Remove if the test folder is exist
> >> +  sprintf(command, "rm -rf %s/", tstdir);
> >> +  rc = tet_system(command);
> >> +
> >> +  /* Create test directory */
> >> +  sprintf(command, "mkdir -p %s", tstdir);
> >> +  rc = tet_system(command);
> >> +  if (rc != 0) {
> >> +  fprintf(stderr, "'%s' Fail rc=%d\n", command, rc);
> >> +  tst_stat = 1;
> >> +  goto done;
> >> +  }
> >> +
> >> +  /* Make sure it can be accessed by server */
> >> +  sprintf(command, "chmod ugo+w,ugo+r %s", tstdir);
> >> +  rc = tet_system(command);
> >> +  if (rc != 0) {
> >> +  fprintf(stderr, "'%s' Fail rc=%d\n", command, rc);
> >> +  tst_stat = 1;
> >> +  goto done;
> >> +  }
> >> +
> >> +  sprintf(command, "immcfg -c SaLogStreamConfig
> >> safLgStrCfg=testRoot "
> >> +  "-a saLogStreamPathName=./testRoot -a
> >> saLogStreamFileName=testRoot");
> >> +  rc = tet_system(command);
> >> +  if (rc != 0) {
> >> +  fprintf(stderr, "'%s' Fail rc=%d\n", command, rc);
> >> +  tst_stat = 1;
> >> +  goto done;
> >> +  }
> >> +
> >> +  /* Change to xxtest */
> >> +  sprintf(command, "immcfg -a logRootDirectory=%s
> >> logConfig=1,safApp=safLogService", tstdir);
> >> +  rc = tet_system(command);
> >> +  if (rc != 0) {
> >> +  fprintf(stderr, "'%s' Fail rc=%d\n", command, rc);
> >> +

Re: [devel] [PATCH 1 of 1] log: write_log_record_hdl get bad file descriptor [#2028]

2016-10-12 Thread A V Mahesh

Ok,

ACK.

-AVM


On 10/12/2016 12:27 PM, Vu Minh Nguyen wrote:
> Hi Mahesh,
>
> In README file, there is a note on this. Refer to "Note on memory handling
> for writting log record"
>
> And caller here means the one that call the function `log_stream_write_h`,
> not log client.
>
> Regards, Vu
>
>> -Original Message-
>> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
>> Sent: Wednesday, October 12, 2016 1:47 PM
>> To: Vu Minh Nguyen ;
>> lennart.l...@ericsson.com
>> Cc: opensaf-devel@lists.sourceforge.net
>> Subject: Re: [PATCH 1 of 1] log: write_log_record_hdl get bad file
> descriptor
>> [#2028]
>>
>> Hi Vu,
>>
>> `Return (-1) to inform that it is caller's responsibility to free the
>> allocated mem` is this existing practices for application ?
>> or new , if new do we need to document it .
>>
>> -AVM
>>
>> On 10/12/2016 10:14 AM, A V Mahesh wrote:
>>> ACK
>>>
>>> -AVM
>>>
>>>
>>> On 9/13/2016 4:19 PM, Vu Minh Nguyen wrote:
 osaf/services/saf/logsv/lgs/lgs_filehdl.cc |  16 +++-
osaf/services/saf/logsv/lgs/lgs_stream.cc  |   6 ++
2 files changed, 17 insertions(+), 5 deletions(-)


 logsv did pass the WRITE request to file handle thread even
 the file descriptor was invalid. Also, when closing file,
 the file handle thread did not set it to invalid.

 This patch fixes these things.

 diff --git a/osaf/services/saf/logsv/lgs/lgs_filehdl.cc
 b/osaf/services/saf/logsv/lgs/lgs_filehdl.cc
 --- a/osaf/services/saf/logsv/lgs/lgs_filehdl.cc
 +++ b/osaf/services/saf/logsv/lgs/lgs_filehdl.cc
 @@ -467,16 +467,15 @@ open_retry:
 */
int fileclose_hdl(void *indata, void *outdata, size_t max_outsize) {
  int rc = 0;
 -  int fd;
 +  int *fd = static_cast(indata);
-  fd = *static_cast(indata);
 -  TRACE_ENTER2("fd=%d", fd);
 +  TRACE_ENTER2("fd=%d", *fd);
osaf_mutex_unlock_ordie(_ftcom_mutex); /* UNLOCK critical
 section */
  /* Flush and synchronize the file before closing to guaranty that
 the file
   * is not written to after it's closed
   */
 -  if ((rc = fdatasync(fd)) == -1) {
 +  if ((rc = fdatasync(*fd)) == -1) {
if ((errno == EROFS) || (errno == EINVAL)) {
  TRACE("Synchronization is not supported for this file");
} else {
 @@ -485,9 +484,16 @@ int fileclose_hdl(void *indata, void *ou
  }
/* Close the file */
 -  rc = close(fd);
 +  rc = close(*fd);
  if (rc == -1) {
LOG_ER("fileclose() %s",strerror(errno));
 +  } else {
 +// When file system is busy, operations on files will take time.
 +// In that case, file handle thread will get timeout and the
 `requester`
 +// will put the `fd` into one link list to do retry next time.
 +// But if closing file succesfully, let the `requester` knows and
 +// no need to send `close file request` again.
 +*fd = -1;
  }
osaf_mutex_lock_ordie(_ftcom_mutex); /* LOCK after critical
 section */
 diff --git a/osaf/services/saf/logsv/lgs/lgs_stream.cc
 b/osaf/services/saf/logsv/lgs/lgs_stream.cc
 --- a/osaf/services/saf/logsv/lgs/lgs_stream.cc
 +++ b/osaf/services/saf/logsv/lgs/lgs_stream.cc
 @@ -1122,6 +1122,12 @@ int log_stream_write_h(log_stream_t *str
if (*stream->p_fd == -1) {
  TRACE("%s - Initiating stream files \"%s\" Failed",
 __FUNCTION__,
stream->name.c_str());
 +  // Seems file system is busy - can not create requrested files.
 +  // Let inform the log client TRY_AGAIN.
 +  //
 +  // Return (-1) to inform that it is caller's responsibility
 +  // to free the allocated memmories.
 +  return -1;
} else {
  TRACE("%s - stream files initiated", __FUNCTION__);
}
>


--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Re: [devel] [PATCH 1 of 1] log: write_log_record_hdl get bad file descriptor [#2028]

2016-10-12 Thread Vu Minh Nguyen

Hi Mahesh,

In README file, there is a note on this. Refer to "Note on memory handling
for writting log record"

And caller here means the one that call the function `log_stream_write_h`,
not log client.

Regards, Vu

> -Original Message-
> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
> Sent: Wednesday, October 12, 2016 1:47 PM
> To: Vu Minh Nguyen ;
> lennart.l...@ericsson.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1 of 1] log: write_log_record_hdl get bad file
descriptor
> [#2028]
> 
> Hi Vu,
> 
> `Return (-1) to inform that it is caller's responsibility to free the
> allocated mem` is this existing practices for application ?
> or new , if new do we need to document it .
> 
> -AVM
> 
> On 10/12/2016 10:14 AM, A V Mahesh wrote:
> > ACK
> >
> > -AVM
> >
> >
> > On 9/13/2016 4:19 PM, Vu Minh Nguyen wrote:
> >> osaf/services/saf/logsv/lgs/lgs_filehdl.cc |  16 +++-
> >>   osaf/services/saf/logsv/lgs/lgs_stream.cc  |   6 ++
> >>   2 files changed, 17 insertions(+), 5 deletions(-)
> >>
> >>
> >> logsv did pass the WRITE request to file handle thread even
> >> the file descriptor was invalid. Also, when closing file,
> >> the file handle thread did not set it to invalid.
> >>
> >> This patch fixes these things.
> >>
> >> diff --git a/osaf/services/saf/logsv/lgs/lgs_filehdl.cc
> >> b/osaf/services/saf/logsv/lgs/lgs_filehdl.cc
> >> --- a/osaf/services/saf/logsv/lgs/lgs_filehdl.cc
> >> +++ b/osaf/services/saf/logsv/lgs/lgs_filehdl.cc
> >> @@ -467,16 +467,15 @@ open_retry:
> >>*/
> >>   int fileclose_hdl(void *indata, void *outdata, size_t max_outsize) {
> >> int rc = 0;
> >> -  int fd;
> >> +  int *fd = static_cast(indata);
> >>   -  fd = *static_cast(indata);
> >> -  TRACE_ENTER2("fd=%d", fd);
> >> +  TRACE_ENTER2("fd=%d", *fd);
> >>   osaf_mutex_unlock_ordie(_ftcom_mutex); /* UNLOCK critical
> >> section */
> >> /* Flush and synchronize the file before closing to guaranty that
> >> the file
> >>  * is not written to after it's closed
> >>  */
> >> -  if ((rc = fdatasync(fd)) == -1) {
> >> +  if ((rc = fdatasync(*fd)) == -1) {
> >>   if ((errno == EROFS) || (errno == EINVAL)) {
> >> TRACE("Synchronization is not supported for this file");
> >>   } else {
> >> @@ -485,9 +484,16 @@ int fileclose_hdl(void *indata, void *ou
> >> }
> >>   /* Close the file */
> >> -  rc = close(fd);
> >> +  rc = close(*fd);
> >> if (rc == -1) {
> >>   LOG_ER("fileclose() %s",strerror(errno));
> >> +  } else {
> >> +// When file system is busy, operations on files will take time.
> >> +// In that case, file handle thread will get timeout and the
> >> `requester`
> >> +// will put the `fd` into one link list to do retry next time.
> >> +// But if closing file succesfully, let the `requester` knows and
> >> +// no need to send `close file request` again.
> >> +*fd = -1;
> >> }
> >>   osaf_mutex_lock_ordie(_ftcom_mutex); /* LOCK after critical
> >> section */
> >> diff --git a/osaf/services/saf/logsv/lgs/lgs_stream.cc
> >> b/osaf/services/saf/logsv/lgs/lgs_stream.cc
> >> --- a/osaf/services/saf/logsv/lgs/lgs_stream.cc
> >> +++ b/osaf/services/saf/logsv/lgs/lgs_stream.cc
> >> @@ -1122,6 +1122,12 @@ int log_stream_write_h(log_stream_t *str
> >>   if (*stream->p_fd == -1) {
> >> TRACE("%s - Initiating stream files \"%s\" Failed",
> >> __FUNCTION__,
> >>   stream->name.c_str());
> >> +  // Seems file system is busy - can not create requrested files.
> >> +  // Let inform the log client TRY_AGAIN.
> >> +  //
> >> +  // Return (-1) to inform that it is caller's responsibility
> >> +  // to free the allocated memmories.
> >> +  return -1;
> >>   } else {
> >> TRACE("%s - stream files initiated", __FUNCTION__);
> >>   }
> >



--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Re: [devel] [PATCH 1 of 1] log: fix failure to create directory when changing logRootDirectory [#2054]

2016-10-12 Thread A V Mahesh

Hi VU,

ACK with following :

Not an issue but It is good to use lgs_conf.logRootDirectory insted of 
new_logRootDirectory

for the remaining part of logRootDirectory_filemove()  , because 
mutex_unlocked for  lgs_conf.logRootDirectory

-AVM

On 10/10/2016 4:48 PM, Lennart Lund wrote:
> Hi Vu
>
> Ack with comment. See below
>
> /***
>   * Support older check-point protocols prior to version 5
>   */
> /**
>   * Used for updating logRootDirectory on standby when check-pointed
>   *
>   * Set the logRootDirectory parameter in the lgs_conf struct
>   * Used for holding data from config object
>   *
>   * @param root_path_str
>   */
> void lgs_rootpathconf_set(const std::string _path_str) {
>
> [Lennart]
> The comments and maybe the place of this function seems incorrect since this 
> function is not only used on standby server
>
> Thanks
> Lennart
>
>
>> -Original Message-
>> From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
>> Sent: den 21 september 2016 10:08
>> To: Lennart Lund ; mahesh.va...@oracle.com
>> Cc: opensaf-devel@lists.sourceforge.net
>> Subject: [PATCH 1 of 1] log: fix failure to create directory when changing
>> logRootDirectory [#2054]
>>
>>   osaf/services/saf/logsv/lgs/lgs_imm.cc |   3 +
>>   tests/logsv/tet_LogOiOps.c |  82
>> ++
>>   2 files changed, 85 insertions(+), 0 deletions(-)
>>
>>
>> When changing `logRootDirectory`, the new directory was not updated to
>> global `lgs_conf.logRootDirectory`, therefore all refering to new directory
>> got the old value.
>>
>> This patch adds code to make sure new directory updated.
>> And one test case #03 of suite #5 are added to verify this case.
>>
>> diff --git a/osaf/services/saf/logsv/lgs/lgs_imm.cc
>> b/osaf/services/saf/logsv/lgs/lgs_imm.cc
>> --- a/osaf/services/saf/logsv/lgs/lgs_imm.cc
>> +++ b/osaf/services/saf/logsv/lgs/lgs_imm.cc
>> @@ -1858,6 +1858,9 @@ void logRootDirectory_filemove(
>>   stream = log_stream_get_by_id(--num);
>> }
>>
>> +  // Change logrootDirectory to new_logRootDirectory
>> +  lgs_rootpathconf_set(new_logRootDirectory);
>> +
>> /* Create new files at new path
>>  */
>> num = get_number_of_streams();
>> 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
>> @@ -1023,6 +1023,87 @@ done:
>>   }
>>
>>   /**
>> + * CCB Object Modify, root directory. Path exist. OK
>> + * Result shall be OK
>> + */
>> +void change_root_path(void)
>> +{
>> +int rc = 0, tst_stat = 0;
>> +char command[256];
>> +char tstdir[256];
>> +
>> +/* Path to test directory */
>> +sprintf(tstdir, "%s/croot", log_root_path);
>> +
>> +// Remove if the test folder is exist
>> +sprintf(command, "rm -rf %s/", tstdir);
>> +rc = tet_system(command);
>> +
>> +/* Create test directory */
>> +sprintf(command, "mkdir -p %s", tstdir);
>> +rc = tet_system(command);
>> +if (rc != 0) {
>> +fprintf(stderr, "'%s' Fail rc=%d\n", command, rc);
>> +tst_stat = 1;
>> +goto done;
>> +}
>> +
>> +/* Make sure it can be accessed by server */
>> +sprintf(command, "chmod ugo+w,ugo+r %s", tstdir);
>> +rc = tet_system(command);
>> +if (rc != 0) {
>> +fprintf(stderr, "'%s' Fail rc=%d\n", command, rc);
>> +tst_stat = 1;
>> +goto done;
>> +}
>> +
>> +sprintf(command, "immcfg -c SaLogStreamConfig
>> safLgStrCfg=testRoot "
>> +"-a saLogStreamPathName=./testRoot -a
>> saLogStreamFileName=testRoot");
>> +rc = tet_system(command);
>> +if (rc != 0) {
>> +fprintf(stderr, "'%s' Fail rc=%d\n", command, rc);
>> +tst_stat = 1;
>> +goto done;
>> +}
>> +
>> +/* Change to xxtest */
>> +sprintf(command, "immcfg -a logRootDirectory=%s
>> logConfig=1,safApp=safLogService", tstdir);
>> +rc = tet_system(command);
>> +if (rc != 0) {
>> +fprintf(stderr, "'%s' Fail rc=%d\n", command, rc);
>> +tst_stat = 1;
>> +goto free;
>> +}
>> +
>> +// Verify if the directory and subdirectly are created successfully
>> +usleep(100*1000); // to make sure logsv done processing of
>> directories creation
>> +sprintf(command, "ls %s/testRoot 1>/dev/null", tstdir);
>> +rc = tet_system(command);
>> +if (rc != 0) {
>> +fprintf(stderr, "'%s' Fail rc=%d\n", command, rc);
>> +tst_stat = 1;
>> +}
>> +
>> +/* Change back */
>> +sprintf(command, "immcfg -a logRootDirectory=%s
>> logConfig=1,safApp=safLogService", log_root_path);
>> +rc = tet_system(command);
>> +if (rc != 0) {
>> +fprintf(stderr, "'%s' Fail to restore rc=%d\n", command, rc);
>> +}
>> +
>> +free:
>> +// Delete test app stream
>> +sprintf(command, "immcfg -d safLgStrCfg=testRoot");;
>> +rc = tet_system(command);
>> +if

Re: [devel] [PATCH 1 of 1] log: write_log_record_hdl get bad file descriptor [#2028]

2016-10-12 Thread A V Mahesh

Hi Vu,

`Return (-1) to inform that it is caller's responsibility to free the 
allocated mem` is this existing practices for application ?
or new , if new do we need to document it .

-AVM

On 10/12/2016 10:14 AM, A V Mahesh wrote:
> ACK
>
> -AVM
>
>
> On 9/13/2016 4:19 PM, Vu Minh Nguyen wrote:
>> osaf/services/saf/logsv/lgs/lgs_filehdl.cc |  16 +++-
>>   osaf/services/saf/logsv/lgs/lgs_stream.cc  |   6 ++
>>   2 files changed, 17 insertions(+), 5 deletions(-)
>>
>>
>> logsv did pass the WRITE request to file handle thread even
>> the file descriptor was invalid. Also, when closing file,
>> the file handle thread did not set it to invalid.
>>
>> This patch fixes these things.
>>
>> diff --git a/osaf/services/saf/logsv/lgs/lgs_filehdl.cc 
>> b/osaf/services/saf/logsv/lgs/lgs_filehdl.cc
>> --- a/osaf/services/saf/logsv/lgs/lgs_filehdl.cc
>> +++ b/osaf/services/saf/logsv/lgs/lgs_filehdl.cc
>> @@ -467,16 +467,15 @@ open_retry:
>>*/
>>   int fileclose_hdl(void *indata, void *outdata, size_t max_outsize) {
>> int rc = 0;
>> -  int fd;
>> +  int *fd = static_cast(indata);
>>   -  fd = *static_cast(indata);
>> -  TRACE_ENTER2("fd=%d", fd);
>> +  TRACE_ENTER2("fd=%d", *fd);
>>   osaf_mutex_unlock_ordie(_ftcom_mutex); /* UNLOCK critical 
>> section */
>> /* Flush and synchronize the file before closing to guaranty that 
>> the file
>>  * is not written to after it's closed
>>  */
>> -  if ((rc = fdatasync(fd)) == -1) {
>> +  if ((rc = fdatasync(*fd)) == -1) {
>>   if ((errno == EROFS) || (errno == EINVAL)) {
>> TRACE("Synchronization is not supported for this file");
>>   } else {
>> @@ -485,9 +484,16 @@ int fileclose_hdl(void *indata, void *ou
>> }
>>   /* Close the file */
>> -  rc = close(fd);
>> +  rc = close(*fd);
>> if (rc == -1) {
>>   LOG_ER("fileclose() %s",strerror(errno));
>> +  } else {
>> +// When file system is busy, operations on files will take time.
>> +// In that case, file handle thread will get timeout and the 
>> `requester`
>> +// will put the `fd` into one link list to do retry next time.
>> +// But if closing file succesfully, let the `requester` knows and
>> +// no need to send `close file request` again.
>> +*fd = -1;
>> }
>>   osaf_mutex_lock_ordie(_ftcom_mutex); /* LOCK after critical 
>> section */
>> diff --git a/osaf/services/saf/logsv/lgs/lgs_stream.cc 
>> b/osaf/services/saf/logsv/lgs/lgs_stream.cc
>> --- a/osaf/services/saf/logsv/lgs/lgs_stream.cc
>> +++ b/osaf/services/saf/logsv/lgs/lgs_stream.cc
>> @@ -1122,6 +1122,12 @@ int log_stream_write_h(log_stream_t *str
>>   if (*stream->p_fd == -1) {
>> TRACE("%s - Initiating stream files \"%s\" Failed", 
>> __FUNCTION__,
>>   stream->name.c_str());
>> +  // Seems file system is busy - can not create requrested files.
>> +  // Let inform the log client TRY_AGAIN.
>> +  //
>> +  // Return (-1) to inform that it is caller's responsibility
>> +  // to free the allocated memmories.
>> +  return -1;
>>   } else {
>> TRACE("%s - stream files initiated", __FUNCTION__);
>>   }
>


--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

[devel] [PATCH 1 of 1] amfnd: queue PG track action msgs [#2115]

[devel] [PATCH 1 of 1] smf: balanced upgrade, missing removal of exectrl copy [#2114]

[devel] [PATCH 0 of 1] Review Request for smf: balanced upgrade, missing removal of exectrl copy [#2114]

Re: [devel] [PATCH 1 of 1] smf: reading of IMM longdn attribute at the camapign admin operation [#2087]

Re: [devel] [PATCH 1 of 1] imm: Return error string when object is being created in another CCB [#2111]

Re: [devel] [PATCH 1 of 1] smfd: handle failed si-swap of middleware [#1605]

Re: [devel] [PATCH 1 of 1] immtool: Don't finalize admo if it hasn't been initialized [#2107]

Re: [devel] [PATCH 1 of 1] log: fix saLogStreamOpen_2 returns SA_AIS_ERR_BAD_OPERATION during si-swap [#2093]

Re: [devel] [PATCH 1 of 1] log: fix failure to create directory when changing logRootDirectory [#2054]

Re: [devel] [PATCH 1 of 1] log: fix saLogStreamOpen_2 returns SA_AIS_ERR_BAD_OPERATION during si-swap [#2093]

Re: [devel] [PATCH 1 of 1] log: fix saLogStreamOpen_2 returns SA_AIS_ERR_BAD_OPERATION during si-swap [#2093]

Re: [devel] [PATCH 1 of 1] log: fix failure to create directory when changing logRootDirectory [#2054]

Re: [devel] [PATCH 1 of 1] log: write_log_record_hdl get bad file descriptor [#2028]

Re: [devel] [PATCH 1 of 1] log: write_log_record_hdl get bad file descriptor [#2028]

Re: [devel] [PATCH 1 of 1] log: fix failure to create directory when changing logRootDirectory [#2054]

Re: [devel] [PATCH 1 of 1] log: write_log_record_hdl get bad file descriptor [#2028]

16 matches

Site Navigation

Mail list logo

Footer information