date:20170428

Re: [devel] [PATCH 1/1] amfd: make auto repair restriction configurable [#2435]

2017-04-28 Thread Gary Lee

Hi Nagu

I ‘simulated’ a downgrade by loading the new AMF class / object files on a 
legacy AMF cluster. amfd will ignore the object and allow deletion of it and 
its class definition.
For an upgrade, the object should be added in campInitAction.

Thanks
Gary

On 28/4/17, 8:48 pm, "Nagendra Kumar"  wrote:

Hi Gary,
Have you tested downgrade scenarios ? I assume you have tested upgrade 
scenario.

Thanks
-Nagu

> -Original Message-
> From: praveen malviya 
> Organization: Oracle Corporation
> Date: Wednesday, 26 April 2017 at 8:19 pm
> To: gary 
> Cc: 
> Subject: Re: [PATCH 1/1] amfd: make auto repair restriction configurable
> [#2435]
> 
> 
> 
> On 26-Apr-17 3:25 PM, Gary Lee wrote:
> > Hi Praveen
> >
> >  From talking with some SMF maintainers, some applications could be
> using saAmfSUMaintenanceCampaign even though AMF does not.
> How it is exposed to the application?
> I guess an SMF application can register for SMF callback
> SaSmfCampaignCallbackT only which has nothing to do with setting and
> unsetting in AMF. Also AMF does not send campaign name in 
notifications
> before 5.2.
> 
> Thanks,
> Praveen
> > So in terms of backwards compatibility, it is better to put this
> configuration in AMF instead.
> >
> > Thanks
> > Gary
> >
> > -Original Message-
> > From: praveen malviya 
> > Organization: Oracle Corporation
> > Date: Wednesday, 26 April 2017 at 7:47 pm
> > To: gary 
> > Cc: 
> > Subject: Re: [PATCH 1/1] amfd: make auto repair restriction 
configurable
> [#2435]
> >
> >  Hi Gary,
> >
> >  If I understand, before 5.2, while running campaign SMF used 
to set
> >  saAmfSUMaintenanceCampaign attribute in affected SUs using CCB
> >  operations. Since AMF feature "Restrictions to auto repair" 
was not
> >  implemented (implemented in #2144, 5.2), AMF was still taking 
actions
> if
> >  components faults while campaign is running and also su 
maintenance
> >  related notifications were not generated. With 2144 in 5.2 
release,
> SMF
> >  is still setting the saAmfSUMaintenanceCampaign. But if some 
faults
> >  happens now, AMF will be taking action and also it sends su
> maintenance
> >  related notification.
> >
> >  I guess before 5.2 release SMF was just setting and unsetting
> >  saAmfSUMaintenanceCampaign without any other use as #2144 was
> not
> >  implemented before 5.2?  If it is so, it means no application 
and even
> >  SMF itself does not track this attribute value before 5.2 
other than
> >  setting and unsetting? Based on this one solution could be:  
if SMF
> >  skips the step/command of setting saAmfSUMaintenanceCampaign
> based on a
> >  new attribute in class SaSmfCampaign. One object of this class 
is
> >  created for each campaign before starting the campaign.
> >
> >  Note:All Non-spec configuration attributes are named as 
"osafAmf*"
> in AMF.
> >
> >
> >  Thanks
> >  Praveen
> >
> >  On 21-Apr-17 3:21 PM, Gary Lee wrote:
> >  > This adds a configuration object for AMF at
> amfConfig=1,safApp=safAmfService.
> >  >
> >  > A configuration attribute 'amfRestrictAutoRepairEnable' is 
added.
> >  > This determines if 'suMaintenanceCampaign' should be ignored 
to
> maintain
> >  > legacy AMF behaviour. The default behaviour is not to 
support auto
> repair
> >  > restriction.
> >  >
> >  > To enable restriction:
> >  > immcfg -a amfRestrictAutoRepairEnable=1
> amfConfig=1,safApp=safAmfService
> >  >
> >  > To disable restriction:
> >  > immcfg -a amfRestrictAutoRepairEnable=0
> amfConfig=1,safApp=safAmfService
> >  > ---
> >  >   src/amf/Makefile.am|   3 +
> >  >   src/amf/amfd/comp.cc   |   2 +-
> >  >   src/amf/amfd/config.cc | 179
> +
> >  >   src/amf/amfd/config.h  |  21 +
> >  >   src/amf/amfd/imm.cc|  35 ++--
> >  >   src/amf/amfd/ndproc.cc |   4 +-
> >  >   src/amf/amfd/node.cc   |   4 +-

Re: [devel] [PATCH 1/1] amfd: make auto repair restriction configurable [#2435]

2017-04-28 Thread Nagendra Kumar

Hi Gary,
Have you tested downgrade scenarios ? I assume you have tested upgrade 
scenario.

Thanks
-Nagu

> -Original Message-
> From: praveen malviya 
> Organization: Oracle Corporation
> Date: Wednesday, 26 April 2017 at 8:19 pm
> To: gary 
> Cc: 
> Subject: Re: [PATCH 1/1] amfd: make auto repair restriction configurable
> [#2435]
> 
> 
> 
> On 26-Apr-17 3:25 PM, Gary Lee wrote:
> > Hi Praveen
> >
> >  From talking with some SMF maintainers, some applications could be
> using saAmfSUMaintenanceCampaign even though AMF does not.
> How it is exposed to the application?
> I guess an SMF application can register for SMF callback
> SaSmfCampaignCallbackT only which has nothing to do with setting and
> unsetting in AMF. Also AMF does not send campaign name in notifications
> before 5.2.
> 
> Thanks,
> Praveen
> > So in terms of backwards compatibility, it is better to put this
> configuration in AMF instead.
> >
> > Thanks
> > Gary
> >
> > -Original Message-
> > From: praveen malviya 
> > Organization: Oracle Corporation
> > Date: Wednesday, 26 April 2017 at 7:47 pm
> > To: gary 
> > Cc: 
> > Subject: Re: [PATCH 1/1] amfd: make auto repair restriction configurable
> [#2435]
> >
> >  Hi Gary,
> >
> >  If I understand, before 5.2, while running campaign SMF used to set
> >  saAmfSUMaintenanceCampaign attribute in affected SUs using CCB
> >  operations. Since AMF feature "Restrictions to auto repair" was not
> >  implemented (implemented in #2144, 5.2), AMF was still taking 
> actions
> if
> >  components faults while campaign is running and also su maintenance
> >  related notifications were not generated. With 2144 in 5.2 release,
> SMF
> >  is still setting the saAmfSUMaintenanceCampaign. But if some faults
> >  happens now, AMF will be taking action and also it sends su
> maintenance
> >  related notification.
> >
> >  I guess before 5.2 release SMF was just setting and unsetting
> >  saAmfSUMaintenanceCampaign without any other use as #2144 was
> not
> >  implemented before 5.2?  If it is so, it means no application and 
> even
> >  SMF itself does not track this attribute value before 5.2 other 
> than
> >  setting and unsetting? Based on this one solution could be:  if SMF
> >  skips the step/command of setting saAmfSUMaintenanceCampaign
> based on a
> >  new attribute in class SaSmfCampaign. One object of this class is
> >  created for each campaign before starting the campaign.
> >
> >  Note:All Non-spec configuration attributes are named as "osafAmf*"
> in AMF.
> >
> >
> >  Thanks
> >  Praveen
> >
> >  On 21-Apr-17 3:21 PM, Gary Lee wrote:
> >  > This adds a configuration object for AMF at
> amfConfig=1,safApp=safAmfService.
> >  >
> >  > A configuration attribute 'amfRestrictAutoRepairEnable' is added.
> >  > This determines if 'suMaintenanceCampaign' should be ignored to
> maintain
> >  > legacy AMF behaviour. The default behaviour is not to support 
> auto
> repair
> >  > restriction.
> >  >
> >  > To enable restriction:
> >  > immcfg -a amfRestrictAutoRepairEnable=1
> amfConfig=1,safApp=safAmfService
> >  >
> >  > To disable restriction:
> >  > immcfg -a amfRestrictAutoRepairEnable=0
> amfConfig=1,safApp=safAmfService
> >  > ---
> >  >   src/amf/Makefile.am|   3 +
> >  >   src/amf/amfd/comp.cc   |   2 +-
> >  >   src/amf/amfd/config.cc | 179
> +
> >  >   src/amf/amfd/config.h  |  21 +
> >  >   src/amf/amfd/imm.cc|  35 ++--
> >  >   src/amf/amfd/ndproc.cc |   4 +-
> >  >   src/amf/amfd/node.cc   |   4 +-
> >  >   src/amf/amfd/sgproc.cc |  14 ++--
> >  >   src/amf/amfd/su.cc |  37 +++--
> >  >   src/amf/amfd/su.h  |   3 +-
> >  >   src/amf/common/amf_defs.h  |   3 +
> >  >   src/amf/config/amf_classes.xml |  15 
> >  >   src/amf/config/amf_objects.xml |   7 ++
> >  >   13 files changed, 300 insertions(+), 27 deletions(-)
> >  >   create mode 100644 src/amf/amfd/config.cc
> >  >   create mode 100644 src/amf/amfd/config.h
> >  >
> >  > diff --git a/src/amf/Makefile.am b/src/amf/Makefile.am
> >  > index 8c175c2..1d6ca60 100644
> >  > --- a/src/amf/Makefile.am
> >  > +++ b/src/amf/Makefile.am
>

Re: [devel] [PATCH 1/1] smf: Attribute value handling in longDn applier is incorrect [#2442]

2017-04-28 Thread Rafael Odzakow

ACK, not tested.


On 04/27/2017 03:10 PM, Lennart Lund wrote:
> SMF has an IMM applier to track changes of attribute longDnsAllowed in 
> OpensafImm class.
> Fix that the applier does not find the value if the list of attributes to be 
> handled in the
> apply callback contains more than one attribute. Also if the found attribute 
> is not the serached
> one the cached attribute value is set to invalid
> ---
>   src/smf/smfd/SmfImmApplierHdl.cc | 28 +++-
>   1 file changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/src/smf/smfd/SmfImmApplierHdl.cc 
> b/src/smf/smfd/SmfImmApplierHdl.cc
> index d325ec4..693446b 100644
> --- a/src/smf/smfd/SmfImmApplierHdl.cc
> +++ b/src/smf/smfd/SmfImmApplierHdl.cc
> @@ -436,15 +436,12 @@ static void CcbApplyCallback(SaImmOiHandleT 
> immOiHandle, SaImmOiCcbIdT ccbId) {
>   
> objName = osaf_extended_name_borrow(>objectName);
> if (object_name_.compare(objName) != 0) {
> -TRACE("%s: Object \"%s\" not an OpensafConfig object", __FUNCTION__,
> -  objName);
> +LOG_NO("%s: Object \"%s\" wrong object", __FUNCTION__, objName);
>   goto done;
> }
>   
> -  /* Read value in opensafNetworkName
> -   * Note: This is implemented as a loop in case more attributes are
> -   *   added in the class in the future. For now the only
> -   *   attribute of interest here is opensafNetworkName
> +  /* Read value
> +   * Note: For now the only attribute of interest here is opensafNetworkName
>  */
> TRACE("%s: Read value in attributes", __FUNCTION__);
> attrMod = opdata->param.modify.attrMods[0];
> @@ -455,21 +452,26 @@ static void CcbApplyCallback(SaImmOiHandleT 
> immOiHandle, SaImmOiCcbIdT ccbId) {
>   if (attribute_name_.compare(attribute.attrName) != 0) {
> // Not found
> attrMod = opdata->param.modify.attrMods[i];
> +  attribute = attrMod->modAttr;
> continue;
>   }
>   
>   // Attribute found
>   value = static_cast(attribute.attrValues[0]);
> +TRACE("Attribute found: attrName '%s', value = %d",
> +  attribute.attrName, *value);
>   break;
> }
>   
> -  if (value == nullptr) {
> -TRACE("%s: Value is nullptr", __FUNCTION__);
> -SetAttributeValue("");
> -attribute_value_is_valid_ = false;
> -  } else {
> -SetAttributeValue(std::to_string(*value));
> -attribute_value_is_valid_ = true;
> +  if (attrMod != nullptr) {
> +if (value == nullptr) {
> +  TRACE("%s: Value is nullptr", __FUNCTION__);
> +  SetAttributeValue("");
> +  attribute_value_is_valid_ = false;
> +} else {
> +  SetAttributeValue(std::to_string(*value));
> +  attribute_value_is_valid_ = true;
> +}
> }
>   
>   done:


--
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 0/1] Review Request for amf: validate env variable format set in comptype and comp objects [#2409] V2

2017-04-28 Thread Nguyen Luu

Summary: amf: validate env variable format set in comptype/comp objects [#2409]
Review request for Ticket(s): 2409
Peer Reviewer(s): AMF devs 
Pull request to: AMF maintainers
Affected branch(es): develop, release
Development branch: ticket-2409
Base revision: ced8d99726a51b9306e53fb8fc1f80f70f715b96
Personal repository: git://git.code.sf.net/u/nguyenluu/review


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):
-
revision 94ced33d5ee94030122a5431a3a131ed705736a6
Author: Nguyen Luu 
Date:   Fri, 28 Apr 2017 17:03:05 +0700

amf: validate env variable format set in comptype/comp objects [#2409]

Valid environment variable should have the format 'var=value'.

AMF currently does not validate this format during CREATE CCBs
for comptype and comp objects (MODIFY allowed for comp after #2255)
related to saAmfxxxCmdEnv attribute.
Besides, the existing validation in avnd_comp_clc_cmd_execute()
is not good enough. This results in invalid env variables getting
printed with weird output.

Also, there is currently no check for duplicate env variables set
in both comptype and comp, so they get printed twice in trace.

This ticket fixes the above issues:
- Validate env variable format during comp[type] CREATE, MODIFY CCBs.
- Overwrite env variable set in comptype if it is also set in comp.



Complete diffstat:
--
 src/amf/amfd/comp.cc | 68 
 src/amf/amfd/comptype.cc | 36 ++---
 src/amf/amfnd/clc.cc |  6 -
 src/amf/amfnd/compdb.cc  | 43 +-
 4 files changed, 138 insertions(+), 15 deletions(-)


Testing Commands:
-
1) Create SaAmfCompType/SaAmfComp objects with invalid env variable format
in attributes saAmfCtDefCmdEnv/saAmfCompCmdEnv. (e.g 'var = value', 
'var==value')
2) Modify attribute saAmfCompCmdEnv of SaAmfComp object with invalid env
variable format.


Testing, Expected Results:
--
AMFD will reject CREATE/MODIFY CCBs with invalid env variable format.


Conditions of Submission:
-
Ack from reviewers of after 2 weeks.


Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  y  y
powerpc n  n
powerpc64   n  n


Reviewer Checklist:
---
[Submitters: make sure that your review doesn't trigger any checkmarks!]


Your checkin has not passed review because (see checked entries):

___ Your RR template is generally incomplete; it has too many blank entries
that need proper data filled in.

___ You have failed to nominate the proper persons for review and push.

___ Your patches do not have proper short+long header

___ You have grammar/spelling in your header that is unacceptable.

___ You have exceeded a sensible line length in your headers/comments/text.

___ You have failed to put in a proper Trac Ticket # into your commits.

___ You have incorrectly put/left internal data in your comments/files
(i.e. internal bug tracking tool IDs, product names etc)

___ You have not given any evidence of testing beyond basic build tests.
Demonstrate some level of runtime or other sanity testing.

___ You have ^M present in some of your files. These have to be removed.

___ You have needlessly changed whitespace or added whitespace crimes
like trailing spaces, or spaces before tabs.

___ You have mixed real technical changes with whitespace and other
cosmetic code cleanup changes. These have to be separate commits.

___ You need to refactor your submission into logical chunks; there is
too much content into a single commit.

___ You have extraneous garbage in your review (merge commits etc)

___ You have giant attachments which should never have been sent;
Instead you should place your content in a public tree to be pulled.

___ You have too many commits attached to an e-mail; resend as threaded
commits, or place in a public tree for a pull.

___ You have resent this content multiple times without a clear indication
of what has changed between each re-send.

___ You have failed to adequately and individually address all of the
comments and change requests that were proposed in the initial review.

___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc)

___ Your computer have a badly configured date and time; confusing the
the threaded patch review.

___ Your changes

[devel] [PATCH 1/1] amf: validate env variable format set in comptype/comp objects [#2409]

2017-04-28 Thread Nguyen Luu

Valid environment variable should have the format 'var=value'.

AMF currently does not validate this format during CREATE CCBs
for comptype and comp objects (MODIFY allowed for comp after #2255)
related to saAmfxxxCmdEnv attribute.
Besides, the existing validation in avnd_comp_clc_cmd_execute()
is not good enough. This results in invalid env variables getting
printed with weird output.

Also, there is currently no check for duplicate env variables set
in both comptype and comp, so they get printed twice in trace.

This ticket fixes the above issues:
- Validate env variable format during comp[type] CREATE, MODIFY CCBs.
- Overwrite env variable set in comptype if it is also set in comp.
---
 src/amf/amfd/comp.cc | 68 
 src/amf/amfd/comptype.cc | 36 ++---
 src/amf/amfnd/clc.cc |  6 -
 src/amf/amfnd/compdb.cc  | 43 +-
 4 files changed, 138 insertions(+), 15 deletions(-)

diff --git a/src/amf/amfd/comp.cc b/src/amf/amfd/comp.cc
index d4b51a6..c866891 100644
--- a/src/amf/amfd/comp.cc
+++ b/src/amf/amfd/comp.cc
@@ -339,6 +339,8 @@ static int is_config_valid(const std::string ,
CcbUtilOperationData_t *opdata) {
   SaAisErrorT rc;
   SaNameT aname;
+  unsigned int num_of_cmd_env;
+  std::string cmd_env;
   std::string::size_type pos;
   SaUint32T value;
 
@@ -399,6 +401,38 @@ static int is_config_valid(const std::string ,
 return 0;
   }
 
+  if 
((immutil_getAttrValuesNumber(const_cast("saAmfCompCmdEnv")
+   ,attributes, _of_cmd_env)) == SA_AIS_OK)
+  {
+for (unsigned int i = 0; i < num_of_cmd_env; i++) {
+  cmd_env = immutil_getStringAttr(attributes, "saAmfCompCmdEnv", i);
+
+  /* env variable format with 'whitespace' is considered as invalid */
+  if (cmd_env.find_first_of(' ') != std::string::npos) {
+report_ccb_validation_error(opdata, "Unknown enviroment variable 
format"
+" '%s' for '%s'. Should be 'var=value'",
+cmd_env.c_str(), dn.c_str());
+return false;
+  }
+
+  std::size_t equalPos = cmd_env.find_first_of('=');
+  unsigned int equal_sign = 0;
+
+  while (equalPos != std::string::npos) {
+equal_sign++;
+equalPos = cmd_env.find_first_of('=', equalPos + 1);
+  }
+  /* env variable format with none or more than one '='
+   * is considered as invalid */
+  if (equal_sign != 1) {
+report_ccb_validation_error(opdata, "Unknown enviroment variable 
format"
+" '%s' for '%s'. Should be 'var=value'",
+cmd_env.c_str(), dn.c_str());
+return false;
+  }
+}
+  }
+
 #if 0
 if ((comp->comp_info.category == AVSV_COMP_TYPE_SA_AWARE) && 
(comp->comp_info.init_len == 0)) {
 LOG_ER("Sa Aware Component: instantiation command not 
configured");
@@ -1035,6 +1069,40 @@ static SaAisErrorT 
ccb_completed_modify_hdlr(CcbUtilOperationData_t *opdata) {
 opdata, "Modification of saAmfCompCmdEnv failed, nullptr arg");
 goto done;
   }
+  for (unsigned index = 0; index < attribute->attrValuesNumber; index++) {
+std::string mod_comp_env = *(static_cast(attribute->
+  attrValues[index]));
+
+/* env variable format with 'whitespace' is considered as invalid */
+if (mod_comp_env.find_first_of(' ') != std::string::npos) {
+  report_ccb_validation_error(opdata, "Modification of saAmfCompCmdEnv"
+  " failed. Unknown enviroment variable"
+  " format '%s' for '%s'."
+  " Should be 'var=value'",
+  mod_comp_env.c_str(),
+  osaf_extended_name_borrow(>
+objectName));
+  goto done;
+}
+std::size_t equalPos = mod_comp_env.find_first_of('=');
+unsigned int equal_sign = 0;
+while (equalPos != std::string::npos) {
+  equal_sign++;
+  equalPos = mod_comp_env.find_first_of('=', equalPos + 1);
+}
+/* env variable format with none or more than one '='
+ * is considered as invalid */
+if (equal_sign != 1) {
+  report_ccb_validation_error(opdata, "Modification of saAmfCompCmdEnv"
+  " failed. Unknown enviroment variable"
+  " format '%s' for '%s'."
+  " Should be 'var=value'",
+  mod_comp_env.c_str(),
+  osaf_extended_name_borrow(>
+

Re: [devel] [PATCH 1/1] amfnd: Ignore second NCSMDS_DOWN [#2436]

2017-04-28 Thread minh chau

Hi AMF maintainers,

While waiting Mahesh checks whether another NCSMDS_DOWN(Vdest) should 
come 3 mins after headless, can we have a look at this patch?
I think we need it to make AMFND safe.

Thanks,
Minh

On 27/04/17 12:26, A V Mahesh wrote:
> Hi Minh chau,
>
> On 4/26/2017 5:43 PM, minh chau wrote:
>>
>> - Stop both SCs, amfnd receives 2 NCSMDS_DOWN, one is Adest, one is 
>> Vdest
>
> I don't seen unnatural events from MDS, as amfnd might have subsided 
> for them.
> Currently  transport (MDS) functionality doesn't provide event 
> differently for
> headless or non-headless and it is completely invisible to  MDS.
>
> I will go through this AMF case and will get back to you.
>
> -AVM
>
> On 4/26/2017 5:43 PM, minh chau wrote:
>> Hi Mahesh,
>>
>> The sequence is going like this:
>>
>> - Stop both SCs, amfnd receives 2 NCSMDS_DOWN, one is Adest, one is 
>> Vdest. I guess at this point MDS tells that both standby and active 
>> amfd are down?
>> 2017-04-26 21:13:52 PL-4 osafamfnd[413]: WA AMF director 
>> unexpectedly crashed
>>
>> - Leave cluster in headless about 3 mins, amfnd receives another 
>> NCSMDS_DOWN with Vdest, so MDS is telling no active amfd again?
>> syslog:
>> 2017-04-26 21:16:52 PL-4 osafamfnd[413]: WA AMF director 
>> unexpectedly crashed
>>
>> mds log:
>> <143>1 2017-04-26T21:16:52.873168+10:00 PL-4 osafamfnd 413 
>> mds.log [meta sequenceId="9881"] >> mds_mcm_await_active_tmr_expiry
>> <142>1 2017-04-26T21:16:52.873183+10:00 PL-4 osafamfnd 413 
>> mds.log [meta sequenceId="9882"] MCM:API: await_active_tmr expired 
>> for svc_id = AVND(13) Subscribed to svc_id = AVD(12) on VDEST id = 1
>> <143>1 2017-04-26T21:16:52.9453+10:00 PL-4 osafclmna 405 mds.log 
>> [meta sequenceId="938"] >> mds_mcm_await_active_tmr_expiry
>> <142>1 2017-04-26T21:16:52.945309+10:00 PL-4 osafclmna 405 
>> mds.log [meta sequenceId="939"] MCM:API: await_active_tmr expired for 
>> svc_id = CLMNA(36) Subscribed to svc_id = CLMS(34) on VDEST id = 16
>> <142>1 2017-04-26T21:16:52.945452+10:00 PL-4 osafsmfnd 454 
>> mds.log [meta sequenceId="620"] MCM:API: svc_down : 
>> await_active_tmr_expiry : svc_id = SMFND(31) on DEST id = 65535 got 
>> DOWN for svc_id = SMFD(30) on VDEST id = 15
>> <143>1 2017-04-26T21:16:52.945462+10:00 PL-4 osafsmfnd 454 
>> mds.log [meta sequenceId="621"] << mds_mcm_await_active_tmr_expiry
>> <143>1 2017-04-26T21:16:52.945938+10:00 PL-4 osafckptnd 432 
>> mds.log [meta sequenceId="1547"] >> mds_mcm_await_active_tmr_expiry
>> <142>1 2017-04-26T21:16:52.945947+10:00 PL-4 osafckptnd 432 
>> mds.log [meta sequenceId="1548"] MCM:API: await_active_tmr expired 
>> for svc_id = CPND(17) Subscribed to svc_id = CPD(16) on VDEST id = 9
>> <142>1 2017-04-26T21:16:52.946064+10:00 PL-4 osafckptnd 432 
>> mds.log [meta sequenceId="1558"] MCM:API: svc_down : 
>> await_active_tmr_expiry : svc_id = CPND(17) on DEST id = 65535 got 
>> DOWN for svc_id = CPD(16) on VDEST id = 9
>> <143>1 2017-04-26T21:16:52.946074+10:00 PL-4 osafckptnd 432 
>> mds.log [meta sequenceId="1559"] << mds_mcm_await_active_tmr_expiry
>> <143>1 2017-04-26T21:16:52.94611+10:00 PL-4 osafckptnd 432 
>> mds.log [meta sequenceId="1562"] >> mds_mcm_await_active_tmr_expiry
>> <142>1 2017-04-26T21:16:52.946118+10:00 PL-4 osafckptnd 432 
>> mds.log [meta sequenceId="1563"] MCM:API: await_active_tmr expired 
>> for svc_id = CLMA(35) Subscribed to svc_id = CLMS(34) on VDEST id = 16
>> <143>1 2017-04-26T21:16:52.955692+10:00 PL-4 osafimmnd 395 
>> mds.log [meta sequenceId="30048"] >> mds_mcm_await_active_tmr_expiry
>> <142>1 2017-04-26T21:16:52.955698+10:00 PL-4 osafimmnd 395 
>> mds.log [meta sequenceId="30049"] MCM:API: await_active_tmr expired 
>> for svc_id = CLMA(35) Subscribed to svc_id = CLMS(34) on VDEST id = 16
>> <142>1 2017-04-26T21:16:52.955765+10:00 PL-4 osafimmnd 395 
>> mds.log [meta sequenceId="30059"] MCM:API: svc_down : 
>> await_active_tmr_expiry : svc_id = CLMA(35) on DEST id = 65535 got 
>> DOWN for svc_id = CLMS(34) on VDEST id = 16
>> <143>1 2017-04-26T21:16:52.955775+10:00 PL-4 osafimmnd 395 
>> mds.log [meta sequenceId="30060"] << mds_mcm_await_active_tmr_expiry
>>
>> I guess the other node-director services also receive the 2nd 
>> NCSMDS_DOWN(Vdest), but those services have no problem because of 
>> service's logic (or likely ckptnd checks cb->is_cpd_up == true), so I 
>> thought it would be AMF problem, until I see the points from 
>> Suryanarayana. So the await_active_tmr is working as expected?
>>
>> thanks,
>> Minh
>>
>> On 26/04/17 17:11, A V Mahesh wrote:
>>> Hi Minh Chau,
>>>
>>> On 4/26/2017 12:05 PM, minh chau wrote:
 amfnd will receive another NCSMDS_DOWN
>>>
>>> you mean  amfnd is receiving  NCSMDS_DOWN for same amfd twice ?
>>> or  amfnd is receiving  NCSMDS_DOWN for both  active amfd & standby 
>>> amfd  ?
>>>
>>> -AVM
>>>
>>> On 4/26/2017 12:05 PM, minh chau wrote:

 @Suryanarayana: I think this fix makes AMFND a bit

[devel] [PATCH 0/1] Review Request for base: Make pid file safe to read by rename it from temporary created file [#2432]

2017-04-28 Thread Minh Chau

Summary: base: Make pid file safe to read by rename it from temporary created 
file [#2432]
Review request for Ticket(s): 2432
Peer Reviewer(s): AndersW, HansN, Ramesh
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop, release
Development branch: ticket-2432
Base revision: ced8d99726a51b9306e53fb8fc1f80f70f715b96
Personal repository: git://git.code.sf.net/u/minh-chau/review


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):
-
*** EXPLAIN/COMMENT THE PATCH SERIES HERE ***

revision 9ea4b18305677021af75f40b099203954c10f695
Author: Minh Chau 
Date:   Fri, 28 Apr 2017 19:22:42 +1000

base: Make pid file safe to read by rename it from temporary created file 
[#2432]

At startup, osaftransportd waits for osafdtmd.pid file creation
and then reads dtm pid. If osafdtmd.pid has not been completedly
created but osaftransportd still receives IN_CREATE, osaftransported
will fail to read pid of dtmd. That results in a node reboot with
a reason as "osafdtmd failed to start".

The patch implements an approach suggested by Anders Widell, which
creates a completed temporary pid file first, then renames it to
correct pid file name. Whenever osaftransportd is notified to read
dtmd's pid, the data in pid file should be always safe to read. In
addition to this, FileNotify needs to introduce IN_MOVED_TO event.



Complete diffstat:
--
 src/base/daemon.c   | 27 ++-
 src/base/file_notify.cc | 10 +-
 2 files changed, 27 insertions(+), 10 deletions(-)


Testing Commands:
-
TC1: Opensafd starts up as normal
TC2: Simulate reported problem by start osaftransportd before dtmd, and
add sleep(1) before dtmd calls fflush() in __create_pidfile


Testing, Expected Results:
--
Node starts successfully in both TCs
Without the patch, TC2 fails due to node reboot


Conditions of Submission:
-
ack from reviewers


Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  y  y
powerpc n  n
powerpc64   n  n


Reviewer Checklist:
---
[Submitters: make sure that your review doesn't trigger any checkmarks!]


Your checkin has not passed review because (see checked entries):

___ Your RR template is generally incomplete; it has too many blank entries
that need proper data filled in.

___ You have failed to nominate the proper persons for review and push.

___ Your patches do not have proper short+long header

___ You have grammar/spelling in your header that is unacceptable.

___ You have exceeded a sensible line length in your headers/comments/text.

___ You have failed to put in a proper Trac Ticket # into your commits.

___ You have incorrectly put/left internal data in your comments/files
(i.e. internal bug tracking tool IDs, product names etc)

___ You have not given any evidence of testing beyond basic build tests.
Demonstrate some level of runtime or other sanity testing.

___ You have ^M present in some of your files. These have to be removed.

___ You have needlessly changed whitespace or added whitespace crimes
like trailing spaces, or spaces before tabs.

___ You have mixed real technical changes with whitespace and other
cosmetic code cleanup changes. These have to be separate commits.

___ You need to refactor your submission into logical chunks; there is
too much content into a single commit.

___ You have extraneous garbage in your review (merge commits etc)

___ You have giant attachments which should never have been sent;
Instead you should place your content in a public tree to be pulled.

___ You have too many commits attached to an e-mail; resend as threaded
commits, or place in a public tree for a pull.

___ You have resent this content multiple times without a clear indication
of what has changed between each re-send.

___ You have failed to adequately and individually address all of the
comments and change requests that were proposed in the initial review.

___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc)

___ Your computer have a badly configured date and time; confusing the
the threaded patch review.

___ Your changes affect IPC mechanism, and you don't present any results
for in-service upgradability test.

___ Your changes affect user manual and documentation, your patch series

Re: [devel] [PATCH 1/1] log: fix checkpoint dest_names in open stream request [#2434]

2017-04-28 Thread Lennart Lund

Hi Canh

Ack

Thanks
Lennart

> -Original Message-
> From: Canh Van Truong [mailto:canh.v.tru...@dektech.com.au]
> Sent: den 25 april 2017 15:29
> To: Lennart Lund ; Vu Minh Nguyen
> ; mahesh.va...@oracle.com
> Cc: opensaf-devel@lists.sourceforge.net; Canh Van Truong
> 
> Subject: [PATCH 1/1] log: fix checkpoint dest_names in open stream request
> [#2434]
> 
> Handling of checkpoint for stream open is in serveral places. Handling of
> checkpoint
> is called in proc_stream_open_msg  forgot to add checkpoint of destination
> name.
> 
> Refactor so that handling of checkpoint data for stream open is done in one
> place
> and destination name was already added in checkpoint data at this place
> ---
>  src/log/logd/lgs_evt.cc| 69 
> ++
>  src/log/logd/lgs_imm.cc| 40 ++-
>  src/log/logd/lgs_mbcsv.cc  | 15 +-
>  src/log/logd/lgs_mbcsv.h   |  5 ++--
>  src/log/logd/lgs_stream.cc | 41 ++-
>  src/log/logd/lgs_stream.h  |  2 ++
>  6 files changed, 57 insertions(+), 115 deletions(-)
> 
> diff --git a/src/log/logd/lgs_evt.cc b/src/log/logd/lgs_evt.cc
> index 6972efe55..beb46e7a7 100644
> --- a/src/log/logd/lgs_evt.cc
> +++ b/src/log/logd/lgs_evt.cc
> @@ -821,72 +821,6 @@ snd_rsp:
>  }
> 
>  /**
> - * Stream open checkpointing
> - * @param cb
> - * @param logStream
> - * @param open_sync_param
> - * @return
> - */
> -static uint32_t lgs_ckpt_stream_open(lgs_cb_t *cb, log_stream_t
> *logStream,
> - lgsv_stream_open_req_t 
> *open_sync_param) {
> -  uint32_t async_rc = NCSCC_RC_SUCCESS;
> -  lgsv_ckpt_msg_v1_t ckpt_v1;
> -  lgsv_ckpt_msg_v2_t ckpt_v2;
> -  void *ckpt_ptr;
> -  lgsv_ckpt_header_t *header_ptr;
> -  lgs_ckpt_stream_open_t *ckpt_rec_open_ptr;
> -
> -  TRACE_ENTER();
> -
> -  if (lgs_is_peer_v2()) {
> -memset(_v2, 0, sizeof(ckpt_v2));
> -header_ptr = _v2.header;
> -ckpt_rec_open_ptr = _v2.ckpt_rec.stream_open;
> -ckpt_ptr = _v2;
> -  } else {
> -memset(_v1, 0, sizeof(ckpt_v1));
> -header_ptr = _v1.header;
> -ckpt_rec_open_ptr = _v1.ckpt_rec.stream_open;
> -ckpt_ptr = _v1;
> -  }
> -
> -  if (cb->ha_state == SA_AMF_HA_ACTIVE) {
> -header_ptr->ckpt_rec_type = LGS_CKPT_OPEN_STREAM;
> -header_ptr->num_ckpt_records = 1;
> -header_ptr->data_len = 1;
> -ckpt_rec_open_ptr->clientId = open_sync_param->client_id;
> -ckpt_rec_open_ptr->streamId = logStream->streamId;
> -
> -ckpt_rec_open_ptr->logFile =
> -const_cast(logStream->fileName.c_str());
> -ckpt_rec_open_ptr->logPath =
> -const_cast(logStream->pathName.c_str());
> -ckpt_rec_open_ptr->logFileCurrent =
> -const_cast(logStream->logFileCurrent.c_str());
> -ckpt_rec_open_ptr->fileFmt = logStream->logFileFormat;
> -ckpt_rec_open_ptr->logStreamName =
> -const_cast(logStream->name.c_str());
> -
> -ckpt_rec_open_ptr->maxFileSize = logStream->maxLogFileSize;
> -ckpt_rec_open_ptr->maxLogRecordSize = logStream-
> >fixedLogRecordSize;
> -ckpt_rec_open_ptr->logFileFullAction = logStream->logFullAction;
> -ckpt_rec_open_ptr->maxFilesRotated = logStream->maxFilesRotated;
> -ckpt_rec_open_ptr->creationTimeStamp = logStream-
> >creationTimeStamp;
> -ckpt_rec_open_ptr->numOpeners = logStream->numOpeners;
> -
> -ckpt_rec_open_ptr->streamType = logStream->streamType;
> -ckpt_rec_open_ptr->logRecordId = logStream->logRecordId;
> -
> -async_rc = lgs_ckpt_send_async(cb, ckpt_ptr, NCS_MBCSV_ACT_ADD);
> -if (async_rc == NCSCC_RC_SUCCESS) {
> -  TRACE_4("REG_REC ASYNC UPDATE SEND SUCCESS...");
> -}
> -  }
> -  TRACE_LEAVE2("async_rc = %d", async_rc);
> -  return async_rc;
> -}
> -
> -/**
>   * Create a new application stream
>   *
>   * @param open_sync_param[in] Parameters used to create the stream
> @@ -1226,8 +1160,9 @@ snd_rsp:
>rc = lgs_mds_msg_send(cb, , >fr_dest, >mds_ctxt,
>  MDS_SEND_PRIORITY_HIGH);
> 
> +  // Checkpoint the opened stream
>if (ais_rv == SA_AIS_OK) {
> -(void)lgs_ckpt_stream_open(cb, logStream, open_sync_param);
> +(void)lgs_ckpt_stream_open(logStream, open_sync_param->client_id);
>}
> 
>// These memories are allocated in MDS log open decode callback.
> diff --git a/src/log/logd/lgs_imm.cc b/src/log/logd/lgs_imm.cc
> index caf0cc92a..2eb3b7544 100644
> --- a/src/log/logd/lgs_imm.cc
> +++ b/src/log/logd/lgs_imm.cc
> @@ -318,43 +318,6 @@ static uint32_t ckpt_stream_config(log_stream_t
> *stream) {
>  }
> 
>  /**
> - * Pack and send an open stream checkpoint using mbcsv
> - * @param stream
> - * @return uint32
> - */
> -static uint32_t ckpt_stream_open(log_stream_t *stream) {
> -  uint32_t rc;
> -  lgsv_ckpt_msg_v1_t ckpt_v1;
> -  lgsv_ckpt_msg_v2_t ckpt_v2;
> -  void *ckpt_ptr;
> -  lgs_ckpt_stream_open_t *stream_open_ptr;
> -

Re: [devel] [PATCH 1/1] log: fix checkpoint dest_names in open stream request [#2434]

2017-04-28 Thread A V Mahesh

Hi Vu,

ACK. not tested.

-AVM


On 4/28/2017 1:27 PM, Vu Minh Nguyen wrote:
> Hi Mahesh,
>
> Regarding your suggestion, I think we should do it in a separated ticket
> that is re-organize headers/implementation files.
>
> How do you think?
>
> Regards, Vu
>
>> -Original Message-
>> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
>> Sent: Friday, April 28, 2017 4:56 AM
>> To: Canh Van Truong ;
>> lennart.l...@ericsson.com; vu.m.ngu...@dektech.com.au
>> Cc: opensaf-devel@lists.sourceforge.net
>> Subject: Re: [PATCH 1/1] log: fix checkpoint dest_names in open stream
>> request [#2434]
>>
>> Hi Canh,
>>
>> On 4/27/2017 5:14 PM, Canh Van Truong wrote:
>>> As current design, the configuration stream is just deleted (e.g. immcfg -d
>> )
>> I did see that , near future we my need to associated client when
>> deleting  the stream
>> the   that is way my comment was irrelevant of  clientId used or not for
>> stream_close , change this
>> `ckpt_stream_close(log_stream_t *stream, time_t closetime)`
>>
>> On 4/27/2017 5:14 PM, Canh Van Truong wrote:
>>> Handling of checkpoint for stream close is in several places. Maybe we
>> could refactor to handle in one place to make clean code.
>>
>> At lease for code readability , In this patch at lease we need to move
>> `ckpt_stream_close(log_stream_t *stream, time_t closetime)` same file
>> where lgs_ckpt_stream_open() located.
>>
>> -AVM
>>
>> On 4/27/2017 5:14 PM, Canh Van Truong wrote:
>>> Hi Mahesh,
>>>
>>> Handling of checkpoint for stream close is in several places. Maybe we
>> could refactor to handle in one place to make clean code.
>>> But it is not related to this ticket. We could refactor it in later ?
>>>
>>> Regards
>>> Canh
>>>
>>> -Original Message-
>>> From: Canh Van Truong [mailto:canh.v.tru...@dektech.com.au]
>>> Sent: Thursday, April 27, 2017 1:27 PM
>>> To: 'A V Mahesh' ;
>> 'lennart.l...@ericsson.com' ;
>> 'vu.m.ngu...@dektech.com.au' 
>>> Cc: 'opensaf-devel@lists.sourceforge.net' > de...@lists.sourceforge.net>
>>> Subject: RE: [PATCH 1/1] log: fix checkpoint dest_names in open stream
>> request [#2434]
>>> Hi Mahesh,
>>>
>>> As current design, the configuration stream is just deleted (e.g. immcfg -d
>> ) if the stream is opened only one time at creating cfg stream
>> (stream->numOpeners = 1). You can check at
>> "stream_ccb_completed_delete" function
>>>If stream->numOpeners > 1, the activity deleting configuration stream is
>> failed.
>>> It means that the configuration stream is just deleted when no client is
>> associated with this stream. So the client_id is assigned -1 when checkpoint
>> at stream close in deleting configuration stream.
>>> An invalid client_id is updated here for checkpoint data.
>>>
>>>
>>> In "proc_stream_close_msg", the valid client_id was updated for
>> checkpoint data when check-pointing.
>>> Thanks
>>> Canh
>>>
>>>
>>>
>>> -Original Message-
>>> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
>>> Sent: Thursday, April 27, 2017 10:35 AM
>>> To: Canh Van Truong ;
>> lennart.l...@ericsson.com; vu.m.ngu...@dektech.com.au
>>> Cc: opensaf-devel@lists.sourceforge.net
>>> Subject: Re: [PATCH 1/1] log: fix checkpoint dest_names in open stream
>> request [#2434]
>>> Hi Canh,
>>>
>>> On 4/27/2017 12:34 PM, Canh Van Truong wrote:
 For checkpoint stream open in proc_stream_open_msg, lgs need update a
 valid client_id for checkpoint data
>>> I was asking,   is For checkpoint stream close in proc_stream_close_msg,
>>> lgs need update a valid client_id for checkpoint data ?
>>> in `ckpt_stream_close`  client_id  is checkpoint  as  -1 now.
>>>
>>> irrelevant of  clientId used or not for stream_close , change this
>>> `ckpt_stream_close(log_stream_t *stream, time_t closetime)`
>>> function also to  ckpt_stream_close(log_stream_t *stream, time_t
>>> closetime, const uint32_t _id);
>>>
>>>
>> ==
>> ==
>>> static uint32_t ckpt_stream_close(log_stream_t *stream, time_t
>> closetime) {
>>>  uint32_t rc;
>>>  lgsv_ckpt_msg_v1_t ckpt_v1;
>>>  lgsv_ckpt_msg_v2_t ckpt_v2;
>>>  void *ckpt_ptr;
>>>
>>>  TRACE_ENTER();
>>>
>>>  if (lgs_is_peer_v2()) {
>>>memset(_v2, 0, sizeof(ckpt_v2));
>>>ckpt_v2.header.ckpt_rec_type = LGS_CKPT_CLOSE_STREAM;
>>>ckpt_v2.header.num_ckpt_records = 1;
>>>ckpt_v2.header.data_len = 1;
>>>
>>>/* No client. Logservice itself has opened stream */
>>>ckpt_v2.ckpt_rec.stream_close.clientId = -1;
>>>
>>>
>> ==
>> ==
>>> -AVM
>>>
>>> On 4/27/2017 12:34 PM, Canh Van Truong wrote:
 Hi Mahesh,

 For checkpoint stream open in proc_stream_open_msg, lgs need update a
 valid client_id for checkpoint data

 For

Re: [devel] [PATCH 1/1] log: generate hash only if having destination name set [#2438]

2017-04-28 Thread Lennart Lund

Hi Vu

ACK

Thanks
Lennart

> -Original Message-
> From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> Sent: den 26 april 2017 08:56
> To: Lennart Lund ; mahesh.va...@oracle.com;
> Canh Van Truong 
> Cc: opensaf-devel@lists.sourceforge.net; Vu Minh Nguyen
> 
> Subject: [PATCH 1/1] log: generate hash only if having destination name set
> [#2438]
> 
> rfc5424_msgid is only referred when streaming and
> having destination name set on that log stream.
> 
> It means it does meaningless job sometimes,
> that is do hash calculation even there is no destination name set.
> 
> This ticket will fix that.
> ---
>  src/log/logd/lgs_dest.cc  |  6 ++
>  src/log/logd/lgs_imm.cc   | 41 +++---
> ---
>  src/log/logd/lgs_main.cc  |  2 --
>  src/log/logd/lgs_mbcsv.cc | 21 +++--
>  4 files changed, 52 insertions(+), 18 deletions(-)
> 
> diff --git a/src/log/logd/lgs_dest.cc b/src/log/logd/lgs_dest.cc
> index ec8fcc6..9f8be27 100644
> --- a/src/log/logd/lgs_dest.cc
> +++ b/src/log/logd/lgs_dest.cc
> @@ -73,6 +73,12 @@ std::string DestinationHandler::GenerateMsgId(const
> std::string& dn,
>  msgid = ((isRtStream == true) ? std::string{sname + 'R'}
>: std::string{sname + 'C'});
>} else {
> +// Do `InitializeHashFunction()` once
> +static bool init_invoked = false;
> +if (init_invoked == false) {
> +  base::InitializeHashFunction();
> +  init_invoked = true;
> +}
>  msgid = base::Hash(dn);
>}
> 
> diff --git a/src/log/logd/lgs_imm.cc b/src/log/logd/lgs_imm.cc
> index caf0cc9..1b22c15 100644
> --- a/src/log/logd/lgs_imm.cc
> +++ b/src/log/logd/lgs_imm.cc
> @@ -2353,7 +2353,17 @@ static SaAisErrorT stream_create_and_configure1(
>char *value_str = *(reinterpret_cast(value));
>vstring.push_back(value_str);
>  }
> +
>  log_stream_add_dest_name(*stream, vstring);
> +if (vstring.empty() == false) {
> +  // Generate & cache `MSGID` to `rfc5424MsgId` which later
> +  // used in RFC5424 protocol
> +  (*stream)->rfc5424MsgId =
> +  DestinationHandler::Instance().GenerateMsgId(
> +  (*stream)->name, (*stream)->isRtStream);
> +  TRACE("%s: stream %s - msgid = %s", __func__, (*stream)-
> >name.c_str(),
> +(*stream)->rfc5424MsgId.c_str());
> +}
>}
>  }
>  i++;
> @@ -2390,11 +2400,6 @@ static SaAisErrorT stream_create_and_configure1(
>  osaf_abort(0);
>}
> 
> -  // Generate & cache `MSGID` to `rfc5424MsgId` which later
> -  // used in RFC5424 protocol
> -  (*stream)->rfc5424MsgId =
> DestinationHandler::Instance().GenerateMsgId(
> -  (*stream)->name, (*stream)->isRtStream);
> -
>  done:
>TRACE_LEAVE2("rc: %s", saf_error(rc));
>return rc;
> @@ -2519,7 +2524,20 @@ static void stream_ccb_apply_modify(const
> CcbUtilOperationData_t *opdata) {
>  char *value_str = *(reinterpret_cast(value));
>  vstring.push_back(value_str);
>}
> +
>apply_destination_names_change(stream, vstring, attrMod->modType);
> +  // Make sure generated msg is only called when
> +  // 1) Have destination set
> +  // 2) Not yet generated
> +  if (vstring.empty() == false && stream->rfc5424MsgId.empty() == true) {
> +// Generate & cache `MSGID` to `rfc5424MsgId` which later
> +// used in RFC5424 protocol
> +stream->rfc5424MsgId =
> +DestinationHandler::Instance().GenerateMsgId(
> +stream->name, stream->isRtStream);
> +TRACE("%s: stream %s - msgid = %s", __func__, stream->name.c_str(),
> +  stream->rfc5424MsgId.c_str());
> +  }
>  } else {
>LOG_ER("Error: Unknown attribute name");
>osafassert(0);
> @@ -2860,6 +2878,14 @@ static SaAisErrorT stream_create_and_configure(
>  vstring.push_back(value_str);
>}
>log_stream_add_dest_name(stream, vstring);
> +  if (vstring.empty() == false) {
> +// Generate & cache `MSGID` to `rfc5424MsgId` which later
> +// used in RFC5424 protocol
> +stream->rfc5424MsgId =
> DestinationHandler::Instance().GenerateMsgId(
> +dn, stream->isRtStream);
> +TRACE("%s: stream %s - msgid = %s", __func__, stream->name.c_str(),
> +  stream->rfc5424MsgId.c_str());
> +  }
>  } else if (!strcmp(attribute->attrName,
> "saLogStreamCreationTimestamp")) {
>if (attribute->attrValuesNumber != 0) {
>  /* Restore creation timestamp if exist
> @@ -2890,11 +2916,6 @@ static SaAisErrorT stream_create_and_configure(
>  }
>}
> 
> -  // Generate & cache `MSGID` to `rfc5424MsgId` which later
> -  // used in RFC5424 protocol
> -  stream->rfc5424MsgId =
> -  DestinationHandler::Instance().GenerateMsgId(dn, stream-
> >isRtStream);
> -
>  done:

Re: [devel] [PATCH 1/1] log: fix checkpoint dest_names in open stream request [#2434]

2017-04-28 Thread Vu Minh Nguyen

Hi Mahesh,

Regarding your suggestion, I think we should do it in a separated ticket 
that is re-organize headers/implementation files.

How do you think?

Regards, Vu

> -Original Message-
> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
> Sent: Friday, April 28, 2017 4:56 AM
> To: Canh Van Truong ;
> lennart.l...@ericsson.com; vu.m.ngu...@dektech.com.au
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1/1] log: fix checkpoint dest_names in open stream
> request [#2434]
> 
> Hi Canh,
> 
> On 4/27/2017 5:14 PM, Canh Van Truong wrote:
> > As current design, the configuration stream is just deleted (e.g. immcfg -d
> )
> I did see that , near future we my need to associated client when
> deleting  the stream
> the   that is way my comment was irrelevant of  clientId used or not for
> stream_close , change this
> `ckpt_stream_close(log_stream_t *stream, time_t closetime)`
> 
> On 4/27/2017 5:14 PM, Canh Van Truong wrote:
> > Handling of checkpoint for stream close is in several places. Maybe we
> could refactor to handle in one place to make clean code.
> 
> At lease for code readability , In this patch at lease we need to move
> `ckpt_stream_close(log_stream_t *stream, time_t closetime)` same file
> where lgs_ckpt_stream_open() located.
> 
> -AVM
> 
> On 4/27/2017 5:14 PM, Canh Van Truong wrote:
> > Hi Mahesh,
> >
> > Handling of checkpoint for stream close is in several places. Maybe we
> could refactor to handle in one place to make clean code.
> > But it is not related to this ticket. We could refactor it in later ?
> >
> > Regards
> > Canh
> >
> > -Original Message-
> > From: Canh Van Truong [mailto:canh.v.tru...@dektech.com.au]
> > Sent: Thursday, April 27, 2017 1:27 PM
> > To: 'A V Mahesh' ;
> 'lennart.l...@ericsson.com' ;
> 'vu.m.ngu...@dektech.com.au' 
> > Cc: 'opensaf-devel@lists.sourceforge.net'  de...@lists.sourceforge.net>
> > Subject: RE: [PATCH 1/1] log: fix checkpoint dest_names in open stream
> request [#2434]
> >
> > Hi Mahesh,
> >
> > As current design, the configuration stream is just deleted (e.g. immcfg -d
> ) if the stream is opened only one time at creating cfg stream
> (stream->numOpeners = 1). You can check at
> "stream_ccb_completed_delete" function
> >   If stream->numOpeners > 1, the activity deleting configuration stream is
> failed.
> >
> > It means that the configuration stream is just deleted when no client is
> associated with this stream. So the client_id is assigned -1 when checkpoint
> at stream close in deleting configuration stream.
> > An invalid client_id is updated here for checkpoint data.
> >
> >
> > In "proc_stream_close_msg", the valid client_id was updated for
> checkpoint data when check-pointing.
> >
> > Thanks
> > Canh
> >
> >
> >
> > -Original Message-
> > From: A V Mahesh [mailto:mahesh.va...@oracle.com]
> > Sent: Thursday, April 27, 2017 10:35 AM
> > To: Canh Van Truong ;
> lennart.l...@ericsson.com; vu.m.ngu...@dektech.com.au
> > Cc: opensaf-devel@lists.sourceforge.net
> > Subject: Re: [PATCH 1/1] log: fix checkpoint dest_names in open stream
> request [#2434]
> >
> > Hi Canh,
> >
> > On 4/27/2017 12:34 PM, Canh Van Truong wrote:
> >> For checkpoint stream open in proc_stream_open_msg, lgs need update a
> >> valid client_id for checkpoint data
> > I was asking,   is For checkpoint stream close in proc_stream_close_msg,
> > lgs need update a valid client_id for checkpoint data ?
> > in `ckpt_stream_close`  client_id  is checkpoint  as  -1 now.
> >
> > irrelevant of  clientId used or not for stream_close , change this
> > `ckpt_stream_close(log_stream_t *stream, time_t closetime)`
> > function also to  ckpt_stream_close(log_stream_t *stream, time_t
> > closetime, const uint32_t _id);
> >
> >
> ==
> ==
> > static uint32_t ckpt_stream_close(log_stream_t *stream, time_t
> closetime) {
> > uint32_t rc;
> > lgsv_ckpt_msg_v1_t ckpt_v1;
> > lgsv_ckpt_msg_v2_t ckpt_v2;
> > void *ckpt_ptr;
> >
> > TRACE_ENTER();
> >
> > if (lgs_is_peer_v2()) {
> >   memset(_v2, 0, sizeof(ckpt_v2));
> >   ckpt_v2.header.ckpt_rec_type = LGS_CKPT_CLOSE_STREAM;
> >   ckpt_v2.header.num_ckpt_records = 1;
> >   ckpt_v2.header.data_len = 1;
> >
> >   /* No client. Logservice itself has opened stream */
> >   ckpt_v2.ckpt_rec.stream_close.clientId = -1;
> >
> >
> ==
> ==
> >
> > -AVM
> >
> > On 4/27/2017 12:34 PM, Canh Van Truong wrote:
> >> Hi Mahesh,
> >>
> >> For checkpoint stream open in proc_stream_open_msg, lgs need update a
> >> valid client_id for checkpoint data
> >>
> >> For checkpoint stream open in create configuration stream (in
> >> stream_ccb_apply_create) and in encode the cold sync
> >>

Re: [devel] [PATCH 1/1] log: fix checkpoint dest_names in open stream request [#2434]

2017-04-28 Thread Vu Minh Nguyen

Hi Canh,

The function `lgs_ckpt_stream_open` has a check on node status. Only do
checkpoint if the current node status is ACTIVE. 
That check is removed from your patch. 

To make sure checkpoint should only be done on active node, I suggest adding
that check in the beginning of 
your newly function `lgs_ckpt_stream_open`, do nothing if the node status is
different from ACTIVE.

Regards, Vu

> -Original Message-
> From: Canh Van Truong [mailto:canh.v.tru...@dektech.com.au]
> Sent: Tuesday, April 25, 2017 3:29 PM
> To: lennart.l...@ericsson.com; vu.m.ngu...@dektech.com.au;
> mahesh.va...@oracle.com
> Cc: opensaf-devel@lists.sourceforge.net; Canh Van Truong
> 
> Subject: [PATCH 1/1] log: fix checkpoint dest_names in open stream request
> [#2434]
> 
> Handling of checkpoint for stream open is in serveral places. Handling of
> checkpoint
> is called in proc_stream_open_msg  forgot to add checkpoint of destination
> name.
> 
> Refactor so that handling of checkpoint data for stream open is done in
one
> place
> and destination name was already added in checkpoint data at this place
> ---
>  src/log/logd/lgs_evt.cc| 69
++
>  src/log/logd/lgs_imm.cc| 40 ++-
>  src/log/logd/lgs_mbcsv.cc  | 15 +-
>  src/log/logd/lgs_mbcsv.h   |  5 ++--
>  src/log/logd/lgs_stream.cc | 41 ++-
>  src/log/logd/lgs_stream.h  |  2 ++
>  6 files changed, 57 insertions(+), 115 deletions(-)
> 
> diff --git a/src/log/logd/lgs_evt.cc b/src/log/logd/lgs_evt.cc
> index 6972efe55..beb46e7a7 100644
> --- a/src/log/logd/lgs_evt.cc
> +++ b/src/log/logd/lgs_evt.cc
> @@ -821,72 +821,6 @@ snd_rsp:
>  }
> 
>  /**
> - * Stream open checkpointing
> - * @param cb
> - * @param logStream
> - * @param open_sync_param
> - * @return
> - */
> -static uint32_t lgs_ckpt_stream_open(lgs_cb_t *cb, log_stream_t
> *logStream,
> - lgsv_stream_open_req_t
*open_sync_param) {
> -  uint32_t async_rc = NCSCC_RC_SUCCESS;
> -  lgsv_ckpt_msg_v1_t ckpt_v1;
> -  lgsv_ckpt_msg_v2_t ckpt_v2;
> -  void *ckpt_ptr;
> -  lgsv_ckpt_header_t *header_ptr;
> -  lgs_ckpt_stream_open_t *ckpt_rec_open_ptr;
> -
> -  TRACE_ENTER();
> -
> -  if (lgs_is_peer_v2()) {
> -memset(_v2, 0, sizeof(ckpt_v2));
> -header_ptr = _v2.header;
> -ckpt_rec_open_ptr = _v2.ckpt_rec.stream_open;
> -ckpt_ptr = _v2;
> -  } else {
> -memset(_v1, 0, sizeof(ckpt_v1));
> -header_ptr = _v1.header;
> -ckpt_rec_open_ptr = _v1.ckpt_rec.stream_open;
> -ckpt_ptr = _v1;
> -  }
> -
> -  if (cb->ha_state == SA_AMF_HA_ACTIVE) {
> -header_ptr->ckpt_rec_type = LGS_CKPT_OPEN_STREAM;
> -header_ptr->num_ckpt_records = 1;
> -header_ptr->data_len = 1;
> -ckpt_rec_open_ptr->clientId = open_sync_param->client_id;
> -ckpt_rec_open_ptr->streamId = logStream->streamId;
> -
> -ckpt_rec_open_ptr->logFile =
> -const_cast(logStream->fileName.c_str());
> -ckpt_rec_open_ptr->logPath =
> -const_cast(logStream->pathName.c_str());
> -ckpt_rec_open_ptr->logFileCurrent =
> -const_cast(logStream->logFileCurrent.c_str());
> -ckpt_rec_open_ptr->fileFmt = logStream->logFileFormat;
> -ckpt_rec_open_ptr->logStreamName =
> -const_cast(logStream->name.c_str());
> -
> -ckpt_rec_open_ptr->maxFileSize = logStream->maxLogFileSize;
> -ckpt_rec_open_ptr->maxLogRecordSize = logStream-
> >fixedLogRecordSize;
> -ckpt_rec_open_ptr->logFileFullAction = logStream->logFullAction;
> -ckpt_rec_open_ptr->maxFilesRotated = logStream->maxFilesRotated;
> -ckpt_rec_open_ptr->creationTimeStamp = logStream-
> >creationTimeStamp;
> -ckpt_rec_open_ptr->numOpeners = logStream->numOpeners;
> -
> -ckpt_rec_open_ptr->streamType = logStream->streamType;
> -ckpt_rec_open_ptr->logRecordId = logStream->logRecordId;
> -
> -async_rc = lgs_ckpt_send_async(cb, ckpt_ptr, NCS_MBCSV_ACT_ADD);
> -if (async_rc == NCSCC_RC_SUCCESS) {
> -  TRACE_4("REG_REC ASYNC UPDATE SEND SUCCESS...");
> -}
> -  }
> -  TRACE_LEAVE2("async_rc = %d", async_rc);
> -  return async_rc;
> -}
> -
> -/**
>   * Create a new application stream
>   *
>   * @param open_sync_param[in] Parameters used to create the stream
> @@ -1226,8 +1160,9 @@ snd_rsp:
>rc = lgs_mds_msg_send(cb, , >fr_dest, >mds_ctxt,
>  MDS_SEND_PRIORITY_HIGH);
> 
> +  // Checkpoint the opened stream
>if (ais_rv == SA_AIS_OK) {
> -(void)lgs_ckpt_stream_open(cb, logStream, open_sync_param);
> +(void)lgs_ckpt_stream_open(logStream, open_sync_param->client_id);
>}
> 
>// These memories are allocated in MDS log open decode callback.
> diff --git a/src/log/logd/lgs_imm.cc b/src/log/logd/lgs_imm.cc
> index caf0cc92a..2eb3b7544 100644
> --- a/src/log/logd/lgs_imm.cc
> +++ b/src/log/logd/lgs_imm.cc
> @@ -318,43 +318,6 @@ static uint32_t ckpt_stream_config(log_stream_t
> *stream) {
>  }
> 
>

Re: [devel] [PATCH 1/1] base: Blocking send causes AMF health check time-out [#2278]

2017-04-28 Thread Anders Widell

Ack.

regards,

Anders Widell


On 04/27/2017 03:55 PM, Hans Nordeback wrote:
> ---
>   src/base/sysf_tmr.c | 20 +---
>   1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/src/base/sysf_tmr.c b/src/base/sysf_tmr.c
> index d4bd8de12..42679e103 100644
> --- a/src/base/sysf_tmr.c
> +++ b/src/base/sysf_tmr.c
> @@ -447,12 +447,14 @@ static uint32_t ncs_tmr_wait(void)
>   struct timespec ts;
>   struct pollfd set;
>   
> + m_NCS_LOCK(_tcb.safe.enter_lock, NCS_LOCK_WRITE);
>   if (clock_gettime(CLOCK_MONOTONIC, _start)) {
>   perror("clock_gettime with MONOTONIC Failed \n");
>   return NCSCC_RC_FAILURE;
>   }
>   
>   ts_current = ts_start;
> + m_NCS_UNLOCK(_tcb.safe.enter_lock, NCS_LOCK_WRITE);
>   
>   while (true) {
>   set.fd = m_GET_FD_FROM_SEL_OBJ(gl_tcb.sel_obj);
> @@ -781,12 +783,21 @@ tmr_t ncs_tmr_start(tmr_t tid,
> TMR_STATE_DESTROY); /* TmrSvc ignores 'old' one */
>   tmr = new_tmr;
>   }
> - scaled = (tmrDelay * 10 / NCS_MILLISECONDS_PER_TICK) + 1 +
> -  (get_time_elapsed_in_ticks(_start));
>   
>   /* Lock the enter wheel in the safe area */
>   m_NCS_LOCK(_tcb.safe.enter_lock, NCS_LOCK_WRITE);
>   
> + if (ts_start.tv_sec == 0 && ts_start.tv_nsec == 0) {
> + if (clock_gettime(CLOCK_MONOTONIC, _start)) {
> + syslog(LOG_ERR, "clock_gettime with MONOTONIC Failed \n");
> + m_NCS_UNLOCK(_tcb.safe.enter_lock, NCS_LOCK_WRITE);
> + return NULL;
> + }
> + }
> +
> + scaled = (tmrDelay * 10 / NCS_MILLISECONDS_PER_TICK) + 1 +
> +  (get_time_elapsed_in_ticks(_start));
> +
>   /* Do some up front initialization as if all will go well */
>   tmr->tmrCB = tmrCB;
>   tmr->tmrUarg = tmrUarg;
> @@ -989,9 +1000,12 @@ int64_t ncs_tmr_remaining(tmr_t tmrID, int64_t *p_tleft)
>NCS_LOCK_WRITE); /* critical region START */
>   return NCSCC_RC_FAILURE;
>   }
> +
> + ticks_elapsed = get_time_elapsed_in_ticks(_start);
> +
>   m_NCS_UNLOCK(_tcb.safe.enter_lock,
>NCS_LOCK_WRITE); /* critical region START */
> - ticks_elapsed = get_time_elapsed_in_ticks(_start);
> +
>   ticks_to_expiry = m_NCS_OS_NTOHLL_P(>key);
>   total_ticks_left = (ticks_to_expiry - ticks_elapsed);
>   


--
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/1] base: Blocking send causes AMF health check time-out [#2278]

2017-04-28 Thread ramesh betham

Ack with a comment.

Please add.. m_NCS_UNLOCK() as below mentioned.

+   m_NCS_LOCK(_tcb.safe.enter_lock, NCS_LOCK_WRITE);
if (clock_gettime(CLOCK_MONOTONIC, _start)) {
perror("clock_gettime with MONOTONIC Failed \n");
m_NCS_UNLOCK(_tcb.safe.enter_lock, NCS_LOCK_WRITE);
return NCSCC_RC_FAILURE;
}
  

Thanks,
Ramesh.

On 4/27/2017 7:25 PM, Hans Nordeback wrote:
> ---
>   src/base/sysf_tmr.c | 20 +---
>   1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/src/base/sysf_tmr.c b/src/base/sysf_tmr.c
> index d4bd8de12..42679e103 100644
> --- a/src/base/sysf_tmr.c
> +++ b/src/base/sysf_tmr.c
> @@ -447,12 +447,14 @@ static uint32_t ncs_tmr_wait(void)
>   struct timespec ts;
>   struct pollfd set;
>   
> + m_NCS_LOCK(_tcb.safe.enter_lock, NCS_LOCK_WRITE);
>   if (clock_gettime(CLOCK_MONOTONIC, _start)) {
>   perror("clock_gettime with MONOTONIC Failed \n");
>   return NCSCC_RC_FAILURE;
>   }
>   
>   ts_current = ts_start;
> + m_NCS_UNLOCK(_tcb.safe.enter_lock, NCS_LOCK_WRITE);
>   
>   while (true) {
>   set.fd = m_GET_FD_FROM_SEL_OBJ(gl_tcb.sel_obj);
> @@ -781,12 +783,21 @@ tmr_t ncs_tmr_start(tmr_t tid,
> TMR_STATE_DESTROY); /* TmrSvc ignores 'old' one */
>   tmr = new_tmr;
>   }
> - scaled = (tmrDelay * 10 / NCS_MILLISECONDS_PER_TICK) + 1 +
> -  (get_time_elapsed_in_ticks(_start));
>   
>   /* Lock the enter wheel in the safe area */
>   m_NCS_LOCK(_tcb.safe.enter_lock, NCS_LOCK_WRITE);
>   
> + if (ts_start.tv_sec == 0 && ts_start.tv_nsec == 0) {
> + if (clock_gettime(CLOCK_MONOTONIC, _start)) {
> + syslog(LOG_ERR, "clock_gettime with MONOTONIC Failed \n");
> + m_NCS_UNLOCK(_tcb.safe.enter_lock, NCS_LOCK_WRITE);
> + return NULL;
> + }
> + }
> +
> + scaled = (tmrDelay * 10 / NCS_MILLISECONDS_PER_TICK) + 1 +
> +  (get_time_elapsed_in_ticks(_start));
> +
>   /* Do some up front initialization as if all will go well */
>   tmr->tmrCB = tmrCB;
>   tmr->tmrUarg = tmrUarg;
> @@ -989,9 +1000,12 @@ int64_t ncs_tmr_remaining(tmr_t tmrID, int64_t *p_tleft)
>NCS_LOCK_WRITE); /* critical region START */
>   return NCSCC_RC_FAILURE;
>   }
> +
> + ticks_elapsed = get_time_elapsed_in_ticks(_start);
> +
>   m_NCS_UNLOCK(_tcb.safe.enter_lock,
>NCS_LOCK_WRITE); /* critical region START */
> - ticks_elapsed = get_time_elapsed_in_ticks(_start);
> +
>   ticks_to_expiry = m_NCS_OS_NTOHLL_P(>key);
>   total_ticks_left = (ticks_to_expiry - ticks_elapsed);
>   

--
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/1] amfd: make auto repair restriction configurable [#2435]

Re: [devel] [PATCH 1/1] amfd: make auto repair restriction configurable [#2435]

Re: [devel] [PATCH 1/1] smf: Attribute value handling in longDn applier is incorrect [#2442]

[devel] [PATCH 0/1] Review Request for amf: validate env variable format set in comptype and comp objects [#2409] V2

[devel] [PATCH 1/1] amf: validate env variable format set in comptype/comp objects [#2409]

Re: [devel] [PATCH 1/1] amfnd: Ignore second NCSMDS_DOWN [#2436]

[devel] [PATCH 0/1] Review Request for base: Make pid file safe to read by rename it from temporary created file [#2432]

Re: [devel] [PATCH 1/1] log: fix checkpoint dest_names in open stream request [#2434]

Re: [devel] [PATCH 1/1] log: fix checkpoint dest_names in open stream request [#2434]

Re: [devel] [PATCH 1/1] log: generate hash only if having destination name set [#2438]

Re: [devel] [PATCH 1/1] log: fix checkpoint dest_names in open stream request [#2434]

Re: [devel] [PATCH 1/1] log: fix checkpoint dest_names in open stream request [#2434]

Re: [devel] [PATCH 1/1] base: Blocking send causes AMF health check time-out [#2278]

Re: [devel] [PATCH 1/1] base: Blocking send causes AMF health check time-out [#2278]

14 matches

Site Navigation

Mail list logo

Footer information