[devel] [PATCH 1 of 1] amfnd: avoid sending SUSI resp during recovery [#1931]

[nagendra . k]

 osaf/services/saf/amf/amfnd/susm.cc |  4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)


During component failover recovery (NPI SU), Amfnd
attempt to send SUSI resp to Amfd though there is
no SUSI assignment is undergoing.
NPI SU terminating state is not a reflective of
SUSI assignment rather it can be during recovery
phases also.

diff --git a/osaf/services/saf/amf/amfnd/susm.cc 
b/osaf/services/saf/amf/amfnd/susm.cc
--- a/osaf/services/saf/amf/amfnd/susm.cc
+++ b/osaf/services/saf/amf/amfnd/susm.cc
@@ -1832,7 +1832,9 @@ uint32_t avnd_su_pres_st_chng_prc(AVND_C
rc = avnd_di_oper_send(cb, su, 
SA_AMF_COMPONENT_FAILOVER);
 
/* si assignment/removal failed.. inform AvD */
-   rc = avnd_di_susi_resp_send(cb, su, 
m_AVND_SU_IS_ALL_SI(su) ? 0 : si);
+   /* Send response to Amfd only when there is a pending 
assignment. */
+   if (m_AVND_SU_IS_ASSIGN_PEND(su))
+   rc = avnd_di_susi_resp_send(cb, su, 
m_AVND_SU_IS_ALL_SI(su) ? 0 : si);
}
 
/* instantiating -> term-failed */

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

[devel] [PATCH 0 of 1] Review Request for amfnd: avoid sending SUSI resp during recovery [#1931]

[nagendra . k]

Summary: amfnd: avoid sending SUSI resp during recovery [#1931]
Review request for Trac Ticket(s): #1931
Peer Reviewer(s): Amf Dev
Pull request to: <>
Affected branch(es): All
Development branch: Default


Impacted area   Impact y/n

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


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

changeset 5a658efc90c6367c4a37bfb3f9bd6db5d83f5321
Author: Nagendra Kumar
Date:   Thu, 04 Aug 2016 10:39:02 +0530

amfnd: avoid sending SUSI resp during recovery [#1931] During component
failover recovery (NPI SU), Amfnd attempt to send SUSI resp to Amfd 
though
there is no SUSI assignment is undergoing. NPI SU terminating state is 
not a
reflective of SUSI assignment rather it can be during recovery phases 
also.

P.S. : Please note that SU repair after component failover is failing for
non-headless case, but works fine for headless case.



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


Testing Commands:
-
As per ticket.

Testing, Expected Results:
--
Amfnd doesn't crash.

Conditions of Submission:
-
 <>


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


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


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

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

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

___ Your patches do not have proper short+long header

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

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

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

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

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

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

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

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

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

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

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

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

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

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

___ You have a misconfigured ~/.hgrc file (i.e. username, email etc)

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

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

___ Your changes affect user manual and documentation, your patch series
do not contain the patch that updates the Doxygen manual.


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

Re: [devel] [PATCH 3 of 4] log: update logsv to support long DN [#1315]

[Vu Minh Nguyen]

Hi Lennart,

Thanks for your comments. Please see my responses inline [Vu].

Regards, Vu

> -Original Message-
> From: Lennart Lund [mailto:lennart.l...@ericsson.com]
> Sent: Wednesday, August 3, 2016 8:57 PM
> To: Vu Minh Nguyen ;
> mahesh.va...@oracle.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: RE: [PATCH 3 of 4] log: update logsv to support long DN [#1315]
> 
> Hi Vu
> 
> Ack with comments and after static checks done
> 
> See comments inline [Lennart]
> 
> Also if not done already, run the static checks (cppcheck and cpplint) and
fix
> found problems for at least the new code.
[Vu] Done. Most are "Lines should be <= 80 characters long". Also fixing all
"Redundant blank line ...".
> 
> Thanks
> Lennart
> 
> > -Original Message-
> > From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> > Sent: den 1 juli 2016 12:42
> > To: mahesh.va...@oracle.com; Lennart Lund 
> > Cc: opensaf-devel@lists.sourceforge.net
> > Subject: [PATCH 3 of 4] log: update logsv to support long DN [#1315]
> >
> >  osaf/services/saf/logsv/lgs/Makefile.am |1 +
> >  osaf/services/saf/logsv/lgs/lgs.h   |4 +-
> >  osaf/services/saf/logsv/lgs/lgs_amf.cc  |   14 +-
> >  osaf/services/saf/logsv/lgs/lgs_config.cc   |   30 +-
> >  osaf/services/saf/logsv/lgs/lgs_evt.cc  |   77 --
> >  osaf/services/saf/logsv/lgs/lgs_filehdl.cc  |   12 +-
> >  osaf/services/saf/logsv/lgs/lgs_fmt.cc  |   41 ++-
> >  osaf/services/saf/logsv/lgs/lgs_imm.cc  |  256
++---
> >  osaf/services/saf/logsv/lgs/lgs_imm_gcfg.cc |   12 +-
> >  osaf/services/saf/logsv/lgs/lgs_main.cc |7 +-
> >  osaf/services/saf/logsv/lgs/lgs_mbcsv.cc|   74 +++--
> >  osaf/services/saf/logsv/lgs/lgs_mds.cc  |  296
---
> --
> >  osaf/services/saf/logsv/lgs/lgs_recov.cc|   44 +--
> >  osaf/services/saf/logsv/lgs/lgs_recov.h |8 +-
> >  osaf/services/saf/logsv/lgs/lgs_stream.cc   |  322
+
> --
> >  osaf/services/saf/logsv/lgs/lgs_stream.h|   18 +-
> >  osaf/services/saf/logsv/lgs/lgs_util.cc |   25 ++-
> >  osaf/services/saf/logsv/lgs/lgs_util.h  |2 +
> >  18 files changed, 630 insertions(+), 613 deletions(-)
> >
> >
> > Major change overview:
> > 1) Replace all internal SaNameT with C/C++ strings
> > 2) Remove NCS_PATRICIA_TREE used to hold stream DNs,
> >Using the existing database `stream_array` instead.
> > 3) Change a bit in `checkFieldSize()/lgs_fmt`,
> >check `numOfDigits` agains number of digits of the constant
> >SA_LOG_MAX_RECORD_SIZE
> >
> > diff --git a/osaf/services/saf/logsv/lgs/Makefile.am
> > b/osaf/services/saf/logsv/lgs/Makefile.am
> > --- a/osaf/services/saf/logsv/lgs/Makefile.am
> > +++ b/osaf/services/saf/logsv/lgs/Makefile.am
> > @@ -45,6 +45,7 @@ osaf_execbin_PROGRAMS = osaflogd
> >  osaflogd_CXXFLAGS = $(AM_CXXFLAGS)
> >
> >  osaflogd_CPPFLAGS = \
> > +   -DSA_EXTENDED_NAME_SOURCE \
> > $(AM_CPPFLAGS) \
> > -I$(top_srcdir)/osaf/libs/common/logsv/include \
> > -I$(top_srcdir)/osaf/libs/common/immsv/include
> > diff --git a/osaf/services/saf/logsv/lgs/lgs.h
> > b/osaf/services/saf/logsv/lgs/lgs.h
> > --- a/osaf/services/saf/logsv/lgs/lgs.h
> > +++ b/osaf/services/saf/logsv/lgs/lgs.h
> > @@ -123,11 +123,11 @@ extern  SaAisErrorT lgs_imm_init_configS
> >
> >  // Functions for recovery handling
> >  void lgs_cleanup_abandoned_streams();
> > -void lgs_delete_one_stream_object(char *name_str);
> > +void lgs_delete_one_stream_object(const std::string _str);
> >  void lgs_search_stream_objects();
> >  SaUint32T *lgs_get_scAbsenceAllowed_attr(SaUint32T *attr_val);
> >  int lgs_get_streamobj_attr(SaImmAttrValuesT_2 ***attrib_out,
> > -  char *object_name,
> > +  const std::string _name,
> >SaImmHandleT *immOmHandle);
> >  int lgs_free_streamobj_attr(SaImmHandleT immHandle);
> >
> > diff --git a/osaf/services/saf/logsv/lgs/lgs_amf.cc
> > b/osaf/services/saf/logsv/lgs/lgs_amf.cc
> > --- a/osaf/services/saf/logsv/lgs/lgs_amf.cc
> > +++ b/osaf/services/saf/logsv/lgs/lgs_amf.cc
> > @@ -27,13 +27,13 @@
> >  static void close_all_files()
> >  {
> > log_stream_t *stream;
> > -
> > -   stream = log_stream_getnext_by_name(NULL);
> > +   int num = get_number_of_streams();
> > +   stream = log_stream_get_by_id(--num);
> > while (stream != NULL) {
> > if (log_stream_file_close(stream) != 0)
> > -   LOG_WA("Could not close file for stream %s",
> > stream->name);
> > +   LOG_WA("Could not close file for stream %s",
> > stream->name.c_str());
> >
> > -   stream = log_stream_getnext_by_name(stream->name);
> > +   stream = log_stream_get_by_id(--num);
> > }
> >  }
> >
> > @@ -54,6 +54,7 @@ static SaAisErrorT amf_active_state_hand
> >  {
> > log_stream_t *stream;
> > SaAisErrorT error = SA_AIS_OK;
> > +   

Re: [devel] [PATCH 1 of 4] log: add one new option -f to saflogger tool [#1315]

[Vu Minh Nguyen]

Hi Lennart,

Please see my respond inline [Vu].

Regards, Vu

> -Original Message-
> From: Lennart Lund [mailto:lennart.l...@ericsson.com]
> Sent: Wednesday, August 3, 2016 5:10 PM
> To: Vu Minh Nguyen ;
> mahesh.va...@oracle.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: RE: [PATCH 1 of 4] log: add one new option -f to saflogger tool
> [#1315]
> 
> Hi Vu
> 
> For saflogger tool ACK with minor comments
> 
> See also comments inline [Lennart]
> 
> Thanks
> Lennart
> 
> > -Original Message-
> > From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> > Sent: den 1 juli 2016 12:42
> > To: mahesh.va...@oracle.com; Lennart Lund 
> > Cc: opensaf-devel@lists.sourceforge.net
> > Subject: [PATCH 1 of 4] log: add one new option -f to saflogger tool
> [#1315]
> >
> >  osaf/tools/saflog/saflogger/Makefile.am  |   1 +
> >  osaf/tools/saflog/saflogger/saf_logger.c |  90
> > ++-
> >  2 files changed, 75 insertions(+), 16 deletions(-)
> >
> >
> > With app stream, saflogger used app stream DN as logFileName.
> > With Long DN, the app stream DN could be longer than 255 characters in
> > length.
> >
> > This patch provides one new option -f .
> > This option is only applicable for app stream.
> >
> > diff --git a/osaf/tools/saflog/saflogger/Makefile.am
> > b/osaf/tools/saflog/saflogger/Makefile.am
> > --- a/osaf/tools/saflog/saflogger/Makefile.am
> > +++ b/osaf/tools/saflog/saflogger/Makefile.am
> > @@ -22,6 +22,7 @@ MAINTAINERCLEANFILES = Makefile.in
> >  bin_PROGRAMS = saflogger
> >
> >  saflogger_CPPFLAGS = \
> > +   -DSA_EXTENDED_NAME_SOURCE \
> > $(AM_CPPFLAGS) \
> > -I$(top_srcdir)/osaf/tools/saflog/include
> >
> > diff --git a/osaf/tools/saflog/saflogger/saf_logger.c
> > b/osaf/tools/saflog/saflogger/saf_logger.c
> > --- a/osaf/tools/saflog/saflogger/saf_logger.c
> > +++ b/osaf/tools/saflog/saflogger/saf_logger.c
> > @@ -37,8 +37,9 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> > +#include "osaf_extended_name.h"
> >
> > -#include 
> >  #include 
> [Lennart] Why removing this include? Many things is this file is used
directly
> in this file therefore this file should be included here.
> If this include is removed it will work anyway since the file is included
> indirectly via the osaf_extended_name.h file. This is unclear and creates
an
> unwanted dependency
[Vu] The header "saAis.h" contains core definitions. It must be included in
each service API header file (e.g "saLog.h")
I think the application no need to include that core header file, include
service API header file is enough.
> 
> >
> >  #include "saf_error.h"
> > @@ -83,7 +84,7 @@ static void usage(void)
> > printf("\t%s - write log record to log stream\n", progname);
> >
> > printf("\nSYNOPSIS\n");
> > -   printf("\t%s [options] [message ...]\n", progname);
> > +   printf("\t%s [options] [-f ] [message ...]\n", progname);
> >
> > printf("\nDESCRIPTION\n");
> > printf("\t%s is a SAF LOG client used to write a log record into a
> > specified log stream.\n", progname);
> > @@ -95,11 +96,16 @@ static void usage(void)
> > printf("\t-n or --notification   write to notification
stream\n");
> > printf("\t-y or --system write to system stream
(default)\n");
> > printf("\t-a NAME or --application=NAME  write to application
> stream
> > NAME (create it if not exist)\n");
> > +   printf("\t-f FILENAMEwrite log record to
FILENAME\n");
> > printf("\t-s SEV or --severity=SEV   use severity SEV, default
> > INFO\n");
> > printf("\t\tvalid severity names: emerg, alert, crit, error, warn,
> > notice, info\n");
> > +   printf("\nNOTES\n");
> > +   printf("\t1) -f is only applicable for app stream.\n");
> > +   printf("\t1)  length must not be longer than 255
> > characters.\n");
> >
> > printf("\nEXAMPLES\n");
> > printf("\tsaflogger -a safLgStrCfg=Test \"Hello world\"\n");
> > +   printf("\tsaflogger -a safLgStrCfg=Test -f testLogFile \"Hello
> > world\"\n");
> > printf("\tsaflogger -s crit \"I am going down\"\n\n");
> >  }
> >
> > @@ -230,7 +236,7 @@ static SaLogSeverityT get_severity(char
> >  int main(int argc, char *argv[])
> >  {
> > int c;
> > -   SaNameT logStreamName = {.length = 0 };
> > +   SaNameT logStreamName;
> > SaLogFileCreateAttributesT_2 *logFileCreateAttributes = NULL;
> > SaLogFileCreateAttributesT_2 appLogFileCreateAttributes;
> > SaLogStreamOpenFlagsT logStreamOpenFlags = 0;
> > @@ -253,6 +259,7 @@ int main(int argc, char *argv[])
> > SaLogStreamHandleT logStreamHandle;
> > SaSelectionObjectT selectionObject;
> > unsigned int wait_time;
> > +   bool is_appstream = false, f_opt = false;
> >
> > srandom(getpid());
> >
> > @@ -261,11 +268,23 @@ int main(int argc, char *argv[])
> > exit(EXIT_FAILURE);
> > }
> >
> > -   sprintf((char *)logSvcUsrName.value, "%s.%u@%s", "saflogger",
> 

[devel] [PATCH 0 of 1] Review Request for log: fix ER in syslog if changing saLogStreamFileName and other atrributes[#1887]

[Canh Van Truong]

Summary: log: fix ER in syslog if changing saLogStreamFileName and other 
atrributes[#1887]
Review request for Trac Ticket(s): #1887
Peer Reviewer(s): Vu, Mahesh, Lennart
Pull request to: Vu
Affected branch(es): 4.7.x, 5.0.x, default
Development branch: default


Impacted area   Impact y/n

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


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

changeset 17eedafb8a4d9df8fe145f48dc4b9278dd8a1214
Author: Canh Van Truong 
Date:   Thu, 04 Aug 2016 10:12:13 +0700

log: fix ER in syslog if changing saLogStreamFileName and other
atrributes[#1887]

When changing saLogStreamFileName with other attributes, the action 
"closing
current log file", "rename", "create new cfg and log file" are 
duplicated.
This cause the ER in syslog, because lgsv can not find log file when 
closing
secondly. Name of current log file was changed at first time.

This patch make lgsv just do above action only once.


Complete diffstat:
--
 osaf/services/saf/logsv/lgs/lgs_imm.cc |  52 
++--
 1 files changed, 26 insertions(+), 26 deletions(-)


Testing Commands:
-
Maunual test:
 1/ Change saLogStreamFileName together with other attribute
immcfg  safLgStrCfg=saLogAlarm,safApp=safLogService -a 
saLogStreamFixedLogRecordSize=150 -a saLogStreamFileName=alarmname
 2/ Check no ER in syslog (or messages file)
Check new cfg and log file in saflog folder


Testing, Expected Results:
--
 - Command when changing attributes in step 1 successfull 
 - No ER in syslog and new cfg, log file was created.

Conditions of Submission:
-
 <>


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


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


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

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

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

___ Your patches do not have proper short+long header

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

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

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

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

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

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

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

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

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

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

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

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

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

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

___ You have a misconfigured ~/.hgrc file (i.e. username, email etc)

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

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

___ Your changes affect user manual and documentation, your patch series
do not contain the patch that updates the Doxygen manual.


--
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net

Re: [devel] [PATCH 3 of 4] log: update logsv to support long DN [#1315]

[Lennart Lund]

Hi Vu

Ack with comments and after static checks done

See comments inline [Lennart]

Also if not done already, run the static checks (cppcheck and cpplint) and fix 
found problems for at least the new code.

Thanks
Lennart

> -Original Message-
> From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> Sent: den 1 juli 2016 12:42
> To: mahesh.va...@oracle.com; Lennart Lund 
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: [PATCH 3 of 4] log: update logsv to support long DN [#1315]
> 
>  osaf/services/saf/logsv/lgs/Makefile.am |1 +
>  osaf/services/saf/logsv/lgs/lgs.h   |4 +-
>  osaf/services/saf/logsv/lgs/lgs_amf.cc  |   14 +-
>  osaf/services/saf/logsv/lgs/lgs_config.cc   |   30 +-
>  osaf/services/saf/logsv/lgs/lgs_evt.cc  |   77 --
>  osaf/services/saf/logsv/lgs/lgs_filehdl.cc  |   12 +-
>  osaf/services/saf/logsv/lgs/lgs_fmt.cc  |   41 ++-
>  osaf/services/saf/logsv/lgs/lgs_imm.cc  |  256 ++---
>  osaf/services/saf/logsv/lgs/lgs_imm_gcfg.cc |   12 +-
>  osaf/services/saf/logsv/lgs/lgs_main.cc |7 +-
>  osaf/services/saf/logsv/lgs/lgs_mbcsv.cc|   74 +++--
>  osaf/services/saf/logsv/lgs/lgs_mds.cc  |  296 -
>  osaf/services/saf/logsv/lgs/lgs_recov.cc|   44 +--
>  osaf/services/saf/logsv/lgs/lgs_recov.h |8 +-
>  osaf/services/saf/logsv/lgs/lgs_stream.cc   |  322 
> +--
>  osaf/services/saf/logsv/lgs/lgs_stream.h|   18 +-
>  osaf/services/saf/logsv/lgs/lgs_util.cc |   25 ++-
>  osaf/services/saf/logsv/lgs/lgs_util.h  |2 +
>  18 files changed, 630 insertions(+), 613 deletions(-)
> 
> 
> Major change overview:
> 1) Replace all internal SaNameT with C/C++ strings
> 2) Remove NCS_PATRICIA_TREE used to hold stream DNs,
>Using the existing database `stream_array` instead.
> 3) Change a bit in `checkFieldSize()/lgs_fmt`,
>check `numOfDigits` agains number of digits of the constant
>SA_LOG_MAX_RECORD_SIZE
> 
> diff --git a/osaf/services/saf/logsv/lgs/Makefile.am
> b/osaf/services/saf/logsv/lgs/Makefile.am
> --- a/osaf/services/saf/logsv/lgs/Makefile.am
> +++ b/osaf/services/saf/logsv/lgs/Makefile.am
> @@ -45,6 +45,7 @@ osaf_execbin_PROGRAMS = osaflogd
>  osaflogd_CXXFLAGS = $(AM_CXXFLAGS)
> 
>  osaflogd_CPPFLAGS = \
> + -DSA_EXTENDED_NAME_SOURCE \
>   $(AM_CPPFLAGS) \
>   -I$(top_srcdir)/osaf/libs/common/logsv/include \
>   -I$(top_srcdir)/osaf/libs/common/immsv/include
> diff --git a/osaf/services/saf/logsv/lgs/lgs.h
> b/osaf/services/saf/logsv/lgs/lgs.h
> --- a/osaf/services/saf/logsv/lgs/lgs.h
> +++ b/osaf/services/saf/logsv/lgs/lgs.h
> @@ -123,11 +123,11 @@ extern  SaAisErrorT lgs_imm_init_configS
> 
>  // Functions for recovery handling
>  void lgs_cleanup_abandoned_streams();
> -void lgs_delete_one_stream_object(char *name_str);
> +void lgs_delete_one_stream_object(const std::string _str);
>  void lgs_search_stream_objects();
>  SaUint32T *lgs_get_scAbsenceAllowed_attr(SaUint32T *attr_val);
>  int lgs_get_streamobj_attr(SaImmAttrValuesT_2 ***attrib_out,
> -char *object_name,
> +const std::string _name,
>  SaImmHandleT *immOmHandle);
>  int lgs_free_streamobj_attr(SaImmHandleT immHandle);
> 
> diff --git a/osaf/services/saf/logsv/lgs/lgs_amf.cc
> b/osaf/services/saf/logsv/lgs/lgs_amf.cc
> --- a/osaf/services/saf/logsv/lgs/lgs_amf.cc
> +++ b/osaf/services/saf/logsv/lgs/lgs_amf.cc
> @@ -27,13 +27,13 @@
>  static void close_all_files()
>  {
>   log_stream_t *stream;
> -
> - stream = log_stream_getnext_by_name(NULL);
> + int num = get_number_of_streams();
> + stream = log_stream_get_by_id(--num);
>   while (stream != NULL) {
>   if (log_stream_file_close(stream) != 0)
> - LOG_WA("Could not close file for stream %s",
> stream->name);
> + LOG_WA("Could not close file for stream %s",
> stream->name.c_str());
> 
> - stream = log_stream_getnext_by_name(stream->name);
> + stream = log_stream_get_by_id(--num);
>   }
>  }
> 
> @@ -54,6 +54,7 @@ static SaAisErrorT amf_active_state_hand
>  {
>   log_stream_t *stream;
>   SaAisErrorT error = SA_AIS_OK;
> + int num;
> 
>   TRACE_ENTER2("HA ACTIVE request");
> 
> @@ -67,12 +68,13 @@ static SaAisErrorT amf_active_state_hand
>   lgs_start_gcfg_applier();
> 
>   /* check existing streams */
> - stream = log_stream_getnext_by_name(NULL);
> + num = get_number_of_streams();
> + stream = log_stream_get_by_id(--num);
>   if (!stream)
>   LOG_ER("No streams exist!");
>   while (stream != NULL) {
>   *stream->p_fd = -1; /* First Initialize fd */
> - stream = log_stream_getnext_by_name(stream->name);
> + stream = log_stream_get_by_id(--num);
>   }
> 
>  done:
> diff --git 

Re: [devel] [PATCH 2 of 4] log: update LOG agent to support Long DN & RDN [#1315]

[Lennart Lund]

Hi Vu,

For this patch ACK, no comments

Thanks
Lennart

> -Original Message-
> From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> Sent: den 1 juli 2016 12:42
> To: mahesh.va...@oracle.com; Lennart Lund 
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: [PATCH 2 of 4] log: update LOG agent to support Long DN & RDN
> [#1315]
> 
>  osaf/libs/agents/saf/lga/Makefile.am |1 +
>  osaf/libs/agents/saf/lga/lga.h   |5 +-
>  osaf/libs/agents/saf/lga/lga_api.c   |  110 ++
>  osaf/libs/agents/saf/lga/lga_mds.c   |  168 +
> -
>  osaf/libs/agents/saf/lga/lga_state.c |7 +-
>  osaf/libs/agents/saf/lga/lga_util.c  |   49 -
>  osaf/libs/saf/libSaLog/Makefile.am   |1 +
>  7 files changed, 206 insertions(+), 135 deletions(-)
> 
> 
> Replace all internal SaNameT by C string and make log agent support long DN,
> but keep backward compatible.
> 
> diff --git a/osaf/libs/agents/saf/lga/Makefile.am
> b/osaf/libs/agents/saf/lga/Makefile.am
> --- a/osaf/libs/agents/saf/lga/Makefile.am
> +++ b/osaf/libs/agents/saf/lga/Makefile.am
> @@ -26,6 +26,7 @@ noinst_HEADERS = \
>  noinst_LTLIBRARIES = liblga.la
> 
>  liblga_la_CPPFLAGS = \
> + -DSA_EXTENDED_NAME_SOURCE \
>   $(AM_CPPFLAGS) \
>   -I$(top_srcdir)/osaf/libs/common/logsv/include
> 
> diff --git a/osaf/libs/agents/saf/lga/lga.h b/osaf/libs/agents/saf/lga/lga.h
> --- a/osaf/libs/agents/saf/lga/lga.h
> +++ b/osaf/libs/agents/saf/lga/lga.h
> @@ -44,7 +44,7 @@
>  /* Log Stream Handle Definition */
>  typedef struct lga_log_stream_hdl_rec {
>   unsigned int log_stream_hdl;/* Log stream HDL from handle mgr
> */
> - SaNameT log_stream_name;/* log stream name mentioned
> during open log stream */
> + char *log_stream_name;  /* log stream name mentioned
> during open log stream */
>   unsigned int open_flags;/* log stream open flags as defined in
> AIS.02.01 */
>   unsigned int log_header_type;   /* log header type */
>   unsigned int lgs_log_stream_id; /* server reference for this
> log stream */
> @@ -141,7 +141,7 @@ extern lga_client_hdl_rec_t *lga_hdl_rec
>  extern lga_log_stream_hdl_rec_t
> *lga_log_stream_hdl_rec_add(lga_client_hdl_rec_t **hdl_rec,
>   uint32_t
> log_stream_id,
>   uint32_t
> log_stream_open_flags,
> - const SaNameT
> *logStreamName, uint32_t log_header_type);
> + const char
> *logStreamName, uint32_t log_header_type);
>  extern void lga_hdl_list_del(lga_client_hdl_rec_t **);
>  extern uint32_t lga_hdl_rec_del(lga_client_hdl_rec_t **,
> lga_client_hdl_rec_t *);
>  extern uint32_t lga_log_stream_hdl_rec_del(lga_log_stream_hdl_rec_t **,
> lga_log_stream_hdl_rec_t *);
> @@ -150,5 +150,6 @@ extern bool lga_validate_lga_client_hdl(
>  /* lga_util.c */
>  extern lga_client_hdl_rec_t *lga_find_hdl_rec_by_regid(lga_cb_t *lga_cb,
> uint32_t client_id);
>  extern void lga_msg_destroy(lgsv_msg_t *msg);
> +extern bool lga_is_extended_name_valid(const SaNameT* name);
> 
>  #endif   /* !LGA_H */
> diff --git a/osaf/libs/agents/saf/lga/lga_api.c
> b/osaf/libs/agents/saf/lga/lga_api.c
> --- a/osaf/libs/agents/saf/lga/lga_api.c
> +++ b/osaf/libs/agents/saf/lga/lga_api.c
> @@ -18,6 +18,7 @@
>  #include 
>  #include 
>  #include "lga.h"
> +#include "osaf_extended_name.h"
> 
>  #include "lga_state.h"
> 
> @@ -71,17 +72,17 @@ static bool is_lgs_state(lgs_state_t sta
>  }
> 
>  static void populate_open_params(lgsv_stream_open_req_t *open_param,
> -  const SaNameT *logStreamName,
> +  const char *logStreamName,
>lga_client_hdl_rec_t *hdl_rec,
>SaLogFileCreateAttributesT_2
> *logFileCreateAttributes,
>SaLogStreamOpenFlagsT
> logStreamOpenFlags)
>  {
>   TRACE_ENTER();
>   open_param->client_id = hdl_rec->lgs_client_id;
> - open_param->lstr_name = *logStreamName;
> + osaf_extended_name_lend(logStreamName, _param-
> >lstr_name);
> 
>   if (logFileCreateAttributes == NULL ||
> - is_well_know_stream((const char*)logStreamName->value)) {
> + is_well_know_stream(logStreamName)) {
>   open_param->logFileFmt = NULL;
>   open_param->logFileFmtLength = 0;
>   open_param->maxLogFileSize = 0;
> @@ -575,7 +576,7 @@ SaAisErrorT saLogFinalize(SaLogHandleT l
>   * @return
>   */
>  static SaAisErrorT validate_open_params(
> - const SaNameT *logStreamName,
> + const char *logStreamName,
>   const SaLogFileCreateAttributesT_2 *logFileCreateAttributes,
>   SaLogStreamOpenFlagsT logStreamOpenFlags,
>   SaLogStreamHandleT *logStreamHandle,
> @@ -589,38 +590,22 @@ static 

Re: [devel] [PATCH 1 of 1] cpsv: To update checkpoint user number for each node [#1669] V3

[Vo Minh Hoang]

Dear Mahesh,

I have just submit a V4 patch that try to eliminate the possible error in
communicating between old and new version.

My testing shows OK result but when I cannot reproduce the problem exactly,
I do not have high confident about it.

Would you please help me review and check the result of this patch.

Thank you and best regards,
Hoang

-Original Message-
From: A V Mahesh [mailto:mahesh.va...@oracle.com] 
Sent: Wednesday, July 27, 2016 4:53 PM
To: Vo Minh Hoang ; 'Nhat Pham'
; anders.wid...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1 of 1] cpsv: To update checkpoint user number for each
node [#1669] V3

Hi  Hoang,

I still able to reproduce the problem , some time it increments two time of
current readers ,

some time it is getting  decremented to less then zero ( variable are set
(0xfff6) )

Unfortunately I don't have any specific steps order , but this issue occurs
in cluster setup with  1new controller & 1 old controller  and  2 old
payloads

when tow  application opened & holded  on old payloads ( don't exist) , and
try to do fail-overs of controllers and then exit the applications on both
payloads,

you will end up with error.

I broad , I suggest you look at the new messages that are getting introduced
in this patch are prevented with version check


===

PL-3:~ # immlist safCkpt=checkpoint_test77
Name   Type Value(s)

safCkptSA_STRING_T 
safCkpt=checkpoint_test77
saCkptCheckpointUsedSize   SA_UINT64_T 110 (0x6e)
saCkptCheckpointSize   SA_UINT64_T 2097152 
(0x20)
saCkptCheckpointRetDurationSA_TIME_T 
9223372036854775807 (0x7fff, Sat Apr 12 05:17:16 2262)
saCkptCheckpointNumWriters SA_UINT32_T 
4294967286 (0xfff6)
saCkptCheckpointNumSectionsSA_UINT32_T  1 (0x1)
saCkptCheckpointNumReplicasSA_UINT32_T  2 (0x2)
saCkptCheckpointNumReaders SA_UINT32_T 
4294967286 (0xfff6)
saCkptCheckpointNumOpeners SA_UINT32_T  0 (0x0)
saCkptCheckpointNumCorruptSections SA_UINT32_T  0 (0x0)
saCkptCheckpointMaxSectionsSA_UINT32_T  1 (0x1)
saCkptCheckpointMaxSectionSize SA_UINT64_T 2097152 
(0x20)
saCkptCheckpointMaxSectionIdSize   SA_UINT64_T 256 (0x100)
*saCkptCheckpointCreationTimestamp  SA_TIME_T 
14696097540 (0x14651a48f19e8400, Wed Jul 27 14:25:54 2016)*
saCkptCheckpointCreationFlags  SA_UINT32_T  2 (0x2)
SaImmAttrImplementerName   SA_STRING_T 
safCheckPointService
SaImmAttrClassName SA_STRING_T 
SaCkptCheckpoint
SaImmAttrAdminOwnerNameSA_STRING_T 



SC-2:~ # immlist safCkpt=checkpoint_test77
Name   Type Value(s)

safCkptSA_STRING_T 
safCkpt=checkpoint_test77
saCkptCheckpointUsedSize   SA_UINT64_T 110 (0x6e)
saCkptCheckpointSize   SA_UINT64_T 2097152 
(0x20)
saCkptCheckpointRetDurationSA_TIME_T 
9223372036854775807 (0x7fff, Sat Apr 12 05:17:16 2262)
saCkptCheckpointNumWriters SA_UINT32_T  20 (0x14)
saCkptCheckpointNumSectionsSA_UINT32_T  1 (0x1)
saCkptCheckpointNumReplicasSA_UINT32_T  2 (0x2)
saCkptCheckpointNumReaders SA_UINT32_T  20 (0x14)
saCkptCheckpointNumOpeners SA_UINT32_T  20 (0x14)
saCkptCheckpointNumCorruptSections SA_UINT32_T  0 (0x0)
saCkptCheckpointMaxSectionsSA_UINT32_T  1 (0x1)
saCkptCheckpointMaxSectionSize SA_UINT64_T 2097152 
(0x20)
saCkptCheckpointMaxSectionIdSize   SA_UINT64_T 256 (0x100)
*saCkptCheckpointCreationTimestamp  SA_TIME_T 
14696106140 (0x14651b112d9d1c00, Wed Jul 27 14:40:14 2016)*
saCkptCheckpointCreationFlags  SA_UINT32_T  2 (0x2)
SaImmAttrImplementerName   SA_STRING_T 
safCheckPointService
SaImmAttrClassName SA_STRING_T 
SaCkptCheckpoint
SaImmAttrAdminOwnerNameSA_STRING_T 


===

-AVM

On 7/26/2016 8:41 

[devel] [PATCH 1 of 1] cpsv: To update checkpoint user number for each node [#1669] V4

[Hoang Vo]

 osaf/libs/common/cpsv/include/cpd_cb.h   |2 +
 osaf/libs/common/cpsv/include/cpd_proc.h |3 +
 osaf/libs/common/cpsv/include/cpd_red.h  |   13 ++
 osaf/libs/common/cpsv/include/cpsv_evt.h |8 +
 osaf/services/saf/cpsv/cpd/cpd_db.c  |   14 ++-
 osaf/services/saf/cpsv/cpd/cpd_evt.c |8 +
 osaf/services/saf/cpsv/cpd/cpd_mbcsv.c   |   96 ---
 osaf/services/saf/cpsv/cpd/cpd_proc.c|  148 +++
 osaf/services/saf/cpsv/cpd/cpd_red.c |   30 -
 osaf/services/saf/cpsv/cpd/cpd_sbevt.c   |   57 +--
 10 files changed, 344 insertions(+), 35 deletions(-)


Problem:
---
The saCkptCheckpointNumOpeners is not updated when a node which has a 
checkpoint client restarts.

Solution:

Currently CPD doesn't store number of user on each node. This patch updates CPD 
to update information
about users on each node for each checkpoint. When a node restarts, the CPD 
update the total number of
users for a checkpoint accordingly. This is reflected on 
saCkptCheckpointNumOpeners attribute correctly.

diff --git a/osaf/libs/common/cpsv/include/cpd_cb.h 
b/osaf/libs/common/cpsv/include/cpd_cb.h
--- a/osaf/libs/common/cpsv/include/cpd_cb.h
+++ b/osaf/libs/common/cpsv/include/cpd_cb.h
@@ -92,6 +92,8 @@ typedef struct cpd_ckpt_info_node {
uint32_t num_users;
uint32_t num_readers;
uint32_t num_writers;
+   uint32_t node_users_cnt;
+   CPD_NODE_USER_INFO *node_users;
 
/* for imm */
SaUint32T ckpt_used_size;
diff --git a/osaf/libs/common/cpsv/include/cpd_proc.h 
b/osaf/libs/common/cpsv/include/cpd_proc.h
--- a/osaf/libs/common/cpsv/include/cpd_proc.h
+++ b/osaf/libs/common/cpsv/include/cpd_proc.h
@@ -108,5 +108,8 @@ uint32_t cpd_mbcsv_enc_async_update(CPD_
 uint32_t cpd_mbcsv_close(CPD_CB *cb);
 bool cpd_is_noncollocated_replica_present_on_payload(CPD_CB *cb, 
CPD_CKPT_INFO_NODE *ckpt_node);
 uint32_t cpd_ckpt_reploc_imm_object_delete(CPD_CB *cb,  CPD_CKPT_REPLOC_INFO 
*ckpt_reploc_node ,bool is_unlink_set);
+void cpd_proc_increase_node_user_info(CPD_CKPT_INFO_NODE *ckpt_node, MDS_DEST 
cpnd_dest, SaCkptCheckpointOpenFlagsT open_flags);
+void cpd_proc_decrease_node_user_info(CPD_CKPT_INFO_NODE *ckpt_node, MDS_DEST 
cpnd_dest, SaCkptCheckpointOpenFlagsT open_flags);
+void cpd_proc_update_user_info_when_node_down(CPD_CB *cb, NODE_ID node_id);
 uint32_t cpd_proc_ckpt_update_post(CPD_CB *cb);
 #endif
diff --git a/osaf/libs/common/cpsv/include/cpd_red.h 
b/osaf/libs/common/cpsv/include/cpd_red.h
--- a/osaf/libs/common/cpsv/include/cpd_red.h
+++ b/osaf/libs/common/cpsv/include/cpd_red.h
@@ -64,6 +64,18 @@ typedef struct cpd_a2s_ckpt_usr_info {
 
 } CPD_A2S_CKPT_USR_INFO;
 
+typedef struct cpd_a2s_ckpt_usr_info_2 {
+   SaCkptCheckpointHandleT ckpt_id;
+   uint32_t num_user;
+   uint32_t num_writer;
+   uint32_t num_reader;
+   uint32_t num_sections;
+   uint32_t ckpt_on_scxb1;
+   uint32_t ckpt_on_scxb2;
+   uint32_t node_users_cnt;
+   CPD_NODE_USER_INFO *node_list;
+} CPD_A2S_CKPT_USR_INFO_2;
+
 typedef struct cpd_mbcsv_msg {
CPD_MBCSV_MSG_TYPE type;
union {
@@ -76,6 +88,7 @@ typedef struct cpd_mbcsv_msg {
CPD_A2S_CKPT_UNLINK ckpt_ulink;
CPD_A2S_CKPT_USR_INFO usr_info;
CPSV_CKPT_DEST_INFO dest_down;
+   CPD_A2S_CKPT_USR_INFO_2 usr_info_2;
} info;
 } CPD_MBCSV_MSG;
 
diff --git a/osaf/libs/common/cpsv/include/cpsv_evt.h 
b/osaf/libs/common/cpsv/include/cpsv_evt.h
--- a/osaf/libs/common/cpsv/include/cpsv_evt.h
+++ b/osaf/libs/common/cpsv/include/cpsv_evt.h
@@ -840,6 +840,14 @@ typedef struct cpd_tmr_info {
} info;
 } CPD_TMR_INFO;
 
+typedef struct cpd_node_user_info {
+   MDS_DEST dest;
+   uint32_t num_users;
+   uint32_t num_writers;
+   uint32_t num_readers;
+   struct cpd_node_user_info *next;
+} CPD_NODE_USER_INFO;
+
 /**
  CPD Event Data Structures
  
**/
diff --git a/osaf/services/saf/cpsv/cpd/cpd_db.c 
b/osaf/services/saf/cpsv/cpd/cpd_db.c
--- a/osaf/services/saf/cpsv/cpd/cpd_db.c
+++ b/osaf/services/saf/cpsv/cpd/cpd_db.c
@@ -137,6 +137,7 @@ uint32_t cpd_ckpt_node_delete(CPD_CB *cb
 {
uint32_t rc = NCSCC_RC_SUCCESS;
CPD_NODE_REF_INFO *nref_info, *next_info;
+   CPD_NODE_USER_INFO *node_user, *next_node_user;
 
TRACE_ENTER();
 
@@ -153,6 +154,13 @@ uint32_t cpd_ckpt_node_delete(CPD_CB *cb
nref_info = next_info;
}
 
+   node_user = ckpt_node->node_users;
+   while (node_user) {
+   next_node_user = node_user->next;
+   free(node_user);
+   node_user = next_node_user;
+   }
+
/* delete imm ckpt runtime object */
if ((cb->ha_state == SA_AMF_HA_ACTIVE) && (ckpt_node->is_unlink_set != 
true)) {
   

[devel] [PATCH 0 of 1] Review Request for cpsv: To update checkpoint user number for each node [#1669] V4

[Hoang Vo]

Summary: cpsv: To update checkpoint user number for each node [#1669] V4
Review request for Trac Ticket(s): 1669
Peer Reviewer(s): mahesh.va...@oracle.com; anders.wid...@ericsson.com
Pull request to: mahesh.va...@oracle.com; anders.wid...@ericsson.com
Affected branch(es): default
Development branch: default


Impacted area   Impact y/n

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


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

changeset a3c955aed8fc6edab76a536b97f66369d305fc89
Author: Hoang Vo 
Date:   Wed, 03 Aug 2016 17:20:08 +0700

cpsv: To update checkpoint user number for each node [#1669] V4

Problem:
--- The saCkptCheckpointNumOpeners is not updated when a node which 
has a
checkpoint client restarts.

Solution:
 Currently CPD doesn't store number of user on each node. This 
patch
updates CPD to update information about users on each node for each
checkpoint. When a node restarts, the CPD update the total number of 
users
for a checkpoint accordingly. This is reflected on
saCkptCheckpointNumOpeners attribute correctly.


Complete diffstat:
--
 osaf/libs/common/cpsv/include/cpd_cb.h   |2 +
 osaf/libs/common/cpsv/include/cpd_proc.h |3 ++
 osaf/libs/common/cpsv/include/cpd_red.h  |   13 +++
 osaf/libs/common/cpsv/include/cpsv_evt.h |8 +++
 osaf/services/saf/cpsv/cpd/cpd_db.c  |   14 ++-
 osaf/services/saf/cpsv/cpd/cpd_evt.c |8 +++
 osaf/services/saf/cpsv/cpd/cpd_mbcsv.c   |   96 
+++--
 osaf/services/saf/cpsv/cpd/cpd_proc.c|  148 

 osaf/services/saf/cpsv/cpd/cpd_red.c |   30 --
 osaf/services/saf/cpsv/cpd/cpd_sbevt.c   |   57 
+
 10 files changed, 344 insertions(+), 35 deletions(-)


Testing Commands:
-
Following old test steps
For in-service update:
Start SC-1 and PL-3 in old version
Start SC-2 and PL-4 in new version
Run ckpttest on each node

Testing, Expected Results:
--
All test cases passed

Conditions of Submission:
-
ACK from maintainer

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 

Re: [devel] [PATCH 1 of 1] log: Readme file for long DN support [#1315]

[Vu Minh Nguyen]

Hi Lennart,

Please see my responses in line, started with [Vu].
I will create a new README file for saflogger tool and put the info related
to new test cases to `~/tests/logsv/README`.

Updated files are attached. Please have a look. Thanks.

Regards, Vu

> -Original Message-
> From: Lennart Lund [mailto:lennart.l...@ericsson.com]
> Sent: Wednesday, August 3, 2016 2:50 PM
> To: Vu Minh Nguyen ;
> mahesh.va...@oracle.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: RE: [PATCH 1 of 1] log: Readme file for long DN support [#1315]
> 
> Hi Vu
> 
> In general this document should not be a document describing what to
> implement instead it should describe what is implemented and what in the
> log service that is affected by long DN and to some extent how it is
> implemented. See some comments/examples inline.
> To have information here about the saflogger tool I think is ok but maybe
it
> is a good idea to have a README file in the .../tools/saflog/ directory?
[Vu] Ok. I will create new README file for saflogger tool. The file content
is attached (README file)

> 
> Also some comments inline [Lennart]
> 
> Comments on the other patches will follow
> 
> Thanks
> Lennart
> 
> > -Original Message-
> > From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> > Sent: den 4 juli 2016 05:07
> > To: mahesh.va...@oracle.com; Lennart Lund 
> > Cc: opensaf-devel@lists.sourceforge.net
> > Subject: [PATCH 1 of 1] log: Readme file for long DN support [#1315]
> >
> >  osaf/services/saf/logsv/README_LONGDN |  138
> > ++
> >  1 files changed, 138 insertions(+), 0 deletions(-)
> >
> >
> > Show changes for long DN, including:
> > 1) saflogger tool
> > 2) log agent
> > 3) log services
> > 4) test suite
> >
> > diff --git a/osaf/services/saf/logsv/README_LONGDN
> > b/osaf/services/saf/logsv/README_LONGDN
> > new file mode 100644
> > --- /dev/null
> > +++ b/osaf/services/saf/logsv/README_LONGDN
> > @@ -0,0 +1,138 @@
> > +#
> > +#  -*- OpenSAF  -*-
> > +#
> > +# (C) Copyright 2016 The OpenSAF Foundation
> > +#
> > +# This program is distributed in the hope that it will be useful, but
> > +# WITHOUT ANY WARRANTY; without even the implied warranty of
> > MERCHANTABILITY
> > +# or FITNESS FOR A PARTICULAR PURPOSE. This file and program are
> > licensed
> > +# under the GNU Lesser General Public License Version 2.1, February
> 1999.
> > +# The complete license can be accessed from the following location:
> > +# http://opensource.org/licenses/lgpl-license.php
> > +# See the Copying file included with the OpenSAF distribution for full
> > +# licensing terms.
> > +#
> > +# Author(s): Ericsson AB
> > +#
> > +
> > +I. GENERAL INFORMATION (#1315)
> > +==
> > +
> > +The SaNameT type, which is used to pass distinguished stream DNs name
> in
> > saLogStreamOpen_2() API,
> > +or in `logSvcUsrName`, `notificationObject` or `notifyingObject` which
> > belong to `SaLogRecordT`
> > +data structure passed to `saLogWriteLogAsync()` API, contains a
fixed-size
> > buffer that limits
> > +the string length to a maximum of 255 bytes.
> > +
> > +#1315 ticket was raised for LOG to support long DN. The use cases could
> be:
> > +UC1) Write log records using the long DN in @Sl, @No, @Ng tokens using
> > Log API.
> > +UC2) Create any long DN name stream using Log API.
> > +UC3) Write record to long DN app stream using saflogger tool.
> [Lennart] A stream name is not a DN. Please rephrase.
[Vu] I rephrased it.

> > +
> > +The implementation was split into following parts:
> > +1) Update saflogger tool - add one more option `-f`
> > +2) Update LOG agent (API) to support Long DN
> > +3) Update LOG service to support Long DN
> > +4) Create test cases
> > +
> > +Besides, README file and OpenSAF_LOG_PR.odt have to be updated.
> [Lennart] Remove this sentence
[Vu] Done.
> 
> > +
> > +
> > +II) IMPLEMENTATION (#1315)
> > +==
> > +
> > +1) Update saflogger tool - Add One More Option `-f`
> [Lennart] This document shall contain information for existing
> implementation not what to be implemented. This means that frases like
> Update saflogger... or Add one more... etc. should not be used. This
> comment applies also to the rest of the document
> This header could be changed to something like:
> 1) saflogger tool, '-f' option
[Vu] Done.
> > +-
> > +`-f` option is only applicable for application stream. Means it is only
valid
> > +when comes together with option `-a`. If no `-f` is provided, default
> > behavior is maintained.
> [Lennart] Consider refrasing to:
> Means it is only valid with option `-a`.
[Vu] Done
> 
> > +
> > +Why need to add new option `-f` to saflogger tool?
> > +When long DN is supported, user is able to put long DN name after
option
> `-
> > a`, and saflogger tool will
> > +use that long DN for log file name of that app stream. In that 

Re: [devel] [PATCH 0 of 3] Review Request for lgsv: Log Service CLM integration [#1638] V2

[Anders Widell]

.cc and .h files are supposed to be in the same directory according to 
the style guide. You can specify the full path to the include file from 
the top of the source tree:

#include "osaf/services/saf/logsv/lgs/lgs_clm.h"

regards,
Anders Widell

On 08/03/2016 12:22 PM, A V Mahesh wrote:
> Hi Anders Widel,
>
> I cleared all the warnings, except following please suggest the how do 
> we adders them :
>
> 1 )  Reason for the followings warnings  because of lgs  *.h files are 
> not in a `include`  directory ,
>  can I ignore or do i move all the .h file to `include` directory ?
>
>
> ./lgs_clm.cc:18:  Include the directory when naming .h files 
> [build/include] [4]
> ./lgs_clm.cc:19:  Include the directory when naming .h files 
> [build/include] [4]
>
> -AVM
>
> On 8/2/2016 4:02 PM, Anders Widell wrote:
>> Hi!
>>
>> I ran "make cppcheck" and "make cpplint" on these new patches, and I 
>> still get the following warnings for the two new files lgs_clm.cc and 
>> lgs_clm.h:
>>
>> [osaf/services/saf/logsv/lgs/lgs_clm.cc:98] -> 
>> [osaf/services/saf/logsv/lgs/lgs_clm.cc:100]: (style) Variable 'rc' 
>> is reassigned a value before the old one has been used.
>> ./osaf/services/saf/logsv/lgs/lgs_clm.h:0:  No #ifndef header guard 
>> found, suggested CPP variable is: 
>> OSAF_SERVICES_SAF_LOGSV_LGS_LGS_CLM_H_  [build/header_guard] [5]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:18:  Include the directory 
>> when naming .h files  [build/include] [4]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:19:  Include the directory 
>> when naming .h files  [build/include] [4]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:31:  Lines should be <= 80 
>> characters long  [whitespace/line_length] [2]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:32:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:33:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:34:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:35:  { should almost always 
>> be at the end of the previous line  [whitespace/braces] [4]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:36:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:37:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:38:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:39:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:41:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:42:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:43:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:44:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:45:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:46:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:48:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:49:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:50:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:51:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:52:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:53:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:54:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:55:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:56:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:57:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:58:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:58:  Missing space before {  
>> [whitespace/braces] [5]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:59:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:60:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:61:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:62:  Tab found; better to 
>> use spaces  [whitespace/tab] [1]
>> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:63:  Tab 

Re: [devel] [PATCH 0 of 3] Review Request for lgsv: Log Service CLM integration [#1638] V2

[A V Mahesh]

Hi Anders Widel,

I cleared all the warnings, except following please suggest the how do 
we adders them :

1 )  Reason for the followings warnings  because of lgs  *.h files are 
not in a `include`  directory ,
  can I ignore or do i move all the .h file to `include` directory ?


./lgs_clm.cc:18:  Include the directory when naming .h files 
[build/include] [4]
./lgs_clm.cc:19:  Include the directory when naming .h files 
[build/include] [4]

-AVM

On 8/2/2016 4:02 PM, Anders Widell wrote:
> Hi!
>
> I ran "make cppcheck" and "make cpplint" on these new patches, and I 
> still get the following warnings for the two new files lgs_clm.cc and 
> lgs_clm.h:
>
> [osaf/services/saf/logsv/lgs/lgs_clm.cc:98] -> 
> [osaf/services/saf/logsv/lgs/lgs_clm.cc:100]: (style) Variable 'rc' is 
> reassigned a value before the old one has been used.
> ./osaf/services/saf/logsv/lgs/lgs_clm.h:0:  No #ifndef header guard 
> found, suggested CPP variable is: 
> OSAF_SERVICES_SAF_LOGSV_LGS_LGS_CLM_H_  [build/header_guard] [5]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:18:  Include the directory 
> when naming .h files  [build/include] [4]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:19:  Include the directory 
> when naming .h files  [build/include] [4]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:31:  Lines should be <= 80 
> characters long  [whitespace/line_length] [2]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:32:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:33:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:34:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:35:  { should almost always 
> be at the end of the previous line  [whitespace/braces] [4]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:36:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:37:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:38:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:39:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:41:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:42:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:43:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:44:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:45:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:46:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:48:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:49:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:50:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:51:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:52:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:53:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:54:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:55:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:56:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:57:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:58:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:58:  Missing space before {  
> [whitespace/braces] [5]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:59:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:60:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:61:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:62:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:63:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:64:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:65:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> ./osaf/services/saf/logsv/lgs/lgs_clm.cc:66:  Tab found; better to use 
> spaces  [whitespace/tab] [1]
> 

Re: [devel] [PATCH 1 of 4] log: add one new option -f to saflogger tool [#1315]

[Lennart Lund]

Hi Vu

For saflogger tool ACK with minor comments

See also comments inline [Lennart]

Thanks
Lennart

> -Original Message-
> From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> Sent: den 1 juli 2016 12:42
> To: mahesh.va...@oracle.com; Lennart Lund 
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: [PATCH 1 of 4] log: add one new option -f to saflogger tool [#1315]
> 
>  osaf/tools/saflog/saflogger/Makefile.am  |   1 +
>  osaf/tools/saflog/saflogger/saf_logger.c |  90
> ++-
>  2 files changed, 75 insertions(+), 16 deletions(-)
> 
> 
> With app stream, saflogger used app stream DN as logFileName.
> With Long DN, the app stream DN could be longer than 255 characters in
> length.
> 
> This patch provides one new option -f .
> This option is only applicable for app stream.
> 
> diff --git a/osaf/tools/saflog/saflogger/Makefile.am
> b/osaf/tools/saflog/saflogger/Makefile.am
> --- a/osaf/tools/saflog/saflogger/Makefile.am
> +++ b/osaf/tools/saflog/saflogger/Makefile.am
> @@ -22,6 +22,7 @@ MAINTAINERCLEANFILES = Makefile.in
>  bin_PROGRAMS = saflogger
> 
>  saflogger_CPPFLAGS = \
> + -DSA_EXTENDED_NAME_SOURCE \
>   $(AM_CPPFLAGS) \
>   -I$(top_srcdir)/osaf/tools/saflog/include
> 
> diff --git a/osaf/tools/saflog/saflogger/saf_logger.c
> b/osaf/tools/saflog/saflogger/saf_logger.c
> --- a/osaf/tools/saflog/saflogger/saf_logger.c
> +++ b/osaf/tools/saflog/saflogger/saf_logger.c
> @@ -37,8 +37,9 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include "osaf_extended_name.h"
> 
> -#include 
>  #include 
[Lennart] Why removing this include? Many things is this file is used directly 
in this file therefore this file should be included here.
If this include is removed it will work anyway since the file is included 
indirectly via the osaf_extended_name.h file. This is unclear and creates an 
unwanted dependency

> 
>  #include "saf_error.h"
> @@ -83,7 +84,7 @@ static void usage(void)
>   printf("\t%s - write log record to log stream\n", progname);
> 
>   printf("\nSYNOPSIS\n");
> - printf("\t%s [options] [message ...]\n", progname);
> + printf("\t%s [options] [-f ] [message ...]\n", progname);
> 
>   printf("\nDESCRIPTION\n");
>   printf("\t%s is a SAF LOG client used to write a log record into a
> specified log stream.\n", progname);
> @@ -95,11 +96,16 @@ static void usage(void)
>   printf("\t-n or --notification   write to notification 
> stream\n");
>   printf("\t-y or --system write to system stream 
> (default)\n");
>   printf("\t-a NAME or --application=NAME  write to application stream
> NAME (create it if not exist)\n");
> + printf("\t-f FILENAMEwrite log record to 
> FILENAME\n");
>   printf("\t-s SEV or --severity=SEV   use severity SEV, default
> INFO\n");
>   printf("\t\tvalid severity names: emerg, alert, crit, error, warn,
> notice, info\n");
> + printf("\nNOTES\n");
> + printf("\t1) -f is only applicable for app stream.\n");
> + printf("\t1)  length must not be longer than 255
> characters.\n");
> 
>   printf("\nEXAMPLES\n");
>   printf("\tsaflogger -a safLgStrCfg=Test \"Hello world\"\n");
> + printf("\tsaflogger -a safLgStrCfg=Test -f testLogFile \"Hello
> world\"\n");
>   printf("\tsaflogger -s crit \"I am going down\"\n\n");
>  }
> 
> @@ -230,7 +236,7 @@ static SaLogSeverityT get_severity(char
>  int main(int argc, char *argv[])
>  {
>   int c;
> - SaNameT logStreamName = {.length = 0 };
> + SaNameT logStreamName;
>   SaLogFileCreateAttributesT_2 *logFileCreateAttributes = NULL;
>   SaLogFileCreateAttributesT_2 appLogFileCreateAttributes;
>   SaLogStreamOpenFlagsT logStreamOpenFlags = 0;
> @@ -253,6 +259,7 @@ int main(int argc, char *argv[])
>   SaLogStreamHandleT logStreamHandle;
>   SaSelectionObjectT selectionObject;
>   unsigned int wait_time;
> + bool is_appstream = false, f_opt = false;
> 
>   srandom(getpid());
> 
> @@ -261,11 +268,23 @@ int main(int argc, char *argv[])
>   exit(EXIT_FAILURE);
>   }
> 
> - sprintf((char *)logSvcUsrName.value, "%s.%u@%s", "saflogger",
> getpid(), hostname);
> - logSvcUsrName.length = strlen((char *)logSvcUsrName.value);
> + if (setenv("SA_ENABLE_EXTENDED_NAMES", "1", 1) != 0) {
> + fprintf(stderr, "Failed to enable Extended SaNameT");
> + exit(EXIT_FAILURE);
> + }
> +
> + /**
> +  * osaf_extended_name_init() is added in case osaf_extended_*
> APIs
> +  * are used before saLogInitialize().
> +  */
> + osaf_extended_name_init();
> +
> + char svcUserName[kOsafMaxDnLength];
> + snprintf(svcUserName, sizeof(svcUserName), "%s.%u@%s",
> "saflogger", getpid(), hostname);
> + saAisNameLend(svcUserName, );
> 
>   /* Setup default values */
> - strcpy((char *)logStreamName.value, SA_LOG_STREAM_SYSTEM);
>   /* system 

Re: [devel] [PATCH 1 of 2] fm: Add support for self-fencing [#1859]

[Mathivanan Naickan Palanivelu]

It's an ack from me after fixing ander's comments. 
We can take up further comments as a next stab.

Mathi.


> -Original Message-
> From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
> Sent: Wednesday, August 03, 2016 1:18 PM
> To: Mathivanan Naickan Palanivelu; Praveen Malviya; Ramesh Babu Betham
> Cc: opensaf-devel@lists.sourceforge.net; Anders Widell
> Subject: RE: [PATCH 1 of 2] fm: Add support for self-fencing [#1859]
> 
> Hi, a gentle reminder. /Thanks HansN
> 
> -Original Message-
> From: Mathivanan Naickan Palanivelu [mailto:mathi.naic...@oracle.com]
> Sent: den 14 juli 2016 11:37
> To: Anders Widell ; Hans Nordebäck
> ; Praveen Malviya
> ; Ramesh Babu Betham
> 
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: RE: [PATCH 1 of 2] fm: Add support for self-fencing [#1859]
> 
> Iam testing this, shall revert by tomorrow.
> Thanks,
> Mathi.
> 
> > -Original Message-
> > From: Anders Widell [mailto:anders.wid...@ericsson.com]
> > Sent: Tuesday, July 12, 2016 7:08 PM
> > To: Hans Nordeback; Praveen Malviya; Mathivanan Naickan Palanivelu;
> > Ramesh Babu Betham
> > Cc: opensaf-devel@lists.sourceforge.net
> > Subject: Re: [PATCH 1 of 2] fm: Add support for self-fencing [#1859]
> >
> > One comment: in the prototype patch the feature was on by default, but
> > it ought to be off by default when we introduce this feature officially.
> >
> > / Anders Widell
> >
> > On 06/30/2016 10:32 AM, Anders Widell wrote:
> > > Hi!
> > >
> > > This patch is actually identical to the prototype code that I wrote
> > > and attached to the ticket, so I am not sure if I am supposed to
> > > also review it... anyways it is ack from from me for the first
> > > patch. :-)
> > >
> > > regards,
> > > Anders Widell
> > >
> > > On 06/23/2016 07:31 AM, Hans Nordeback wrote:
> > >> osaf/services/infrastructure/fm/fms/fm_cb.h   |  10 +
> > >>   osaf/services/infrastructure/fm/fms/fm_main.c |  16 +++-
> > >>   osaf/services/infrastructure/fm/fms/fm_mds.c  |  51
> > >> +++
> > >>   3 files changed, 75 insertions(+), 2 deletions(-)
> > >>
> > >>
> > >> In situations where remote fencing is not possible, this patch adds
> > >> support for self-fencing.
> > >>
> > >> diff --git a/osaf/services/infrastructure/fm/fms/fm_cb.h
> > >> b/osaf/services/infrastructure/fm/fms/fm_cb.h
> > >> --- a/osaf/services/infrastructure/fm/fms/fm_cb.h
> > >> +++ b/osaf/services/infrastructure/fm/fms/fm_cb.h
> > >> @@ -27,6 +27,10 @@
> > >>   #include "rda_papi.h"
> > >>   #include "fm_amf.h"
> > >>   +#include 
> > >> +#include 
> > >> +#include 
> > >> +
> > >>   uint32_t gl_fm_hdl;
> > >> typedef enum {
> > >> @@ -92,6 +96,12 @@ typedef struct fm_cb {
> > >>   bool amfnd_down;
> > >>   bool amfd_down;
> > >>   bool fm_down;
> > >> +
> > >> +bool peer_sc_up;
> > >> +bool well_connected;
> > >> +uint64_t cluster_size;
> > >> +struct timespec last_well_connected;
> > >> +struct timespec node_isolation_timeout;
> > >>   } FM_CB;
> > >> extern char *role_string[];
> > >> diff --git a/osaf/services/infrastructure/fm/fms/fm_main.c
> > >> b/osaf/services/infrastructure/fm/fms/fm_main.c
> > >> --- a/osaf/services/infrastructure/fm/fms/fm_main.c
> > >> +++ b/osaf/services/infrastructure/fm/fms/fm_main.c
> > >> @@ -30,7 +30,7 @@ This file contains the main() routine fo
> > >> #include 
> > >>   #include "fm.h"
> > >> -
> > >> +#include "osaf_time.h"
> > >> enum {
> > >>   FD_TERM = 0,
> > >> @@ -411,7 +411,19 @@ static uint32_t fm_get_args(FM_CB *fm_cb
> > >>   fm_cb->promote_active_tmr.type = FM_TMR_PROMOTE_ACTIVE;
> > >>   fm_cb->activation_supervision_tmr.type =
> > >> FM_TMR_ACTIVATION_SUPERVISION;
> > >>   -  TRACE_LEAVE();
> > >> +char* node_isolation_timeout =
> > >> getenv("FMS_NODE_ISOLATION_TIMEOUT");
> > >> +if (node_isolation_timeout != NULL) {
> > >> +osaf_millis_to_timespec(atoi(node_isolation_timeout),
> > >> +_cb->node_isolation_timeout);
> > >> +} else {
> > >> +fm_cb->node_isolation_timeout.tv_sec = 10;
> > >> +fm_cb->node_isolation_timeout.tv_nsec = 0;
> > >> +}
> > >> +TRACE("NODE_ISOLATION_TIMEOUT = %" PRId64 ".%09ld",
> > >> +  (int64_t) fm_cb->node_isolation_timeout.tv_sec,
> > >> +  fm_cb->node_isolation_timeout.tv_nsec);
> > >> +
> > >> +TRACE_LEAVE();
> > >>   return NCSCC_RC_SUCCESS;
> > >>   }
> > >>   diff --git a/osaf/services/infrastructure/fm/fms/fm_mds.c
> > >> b/osaf/services/infrastructure/fm/fms/fm_mds.c
> > >> --- a/osaf/services/infrastructure/fm/fms/fm_mds.c
> > >> +++ b/osaf/services/infrastructure/fm/fms/fm_mds.c
> > >> @@ -16,6 +16,8 @@
> > >>   */
> > >> #include "fm.h"
> > >> +#include "osaf_time.h"
> > >> +#include "ncssysf_def.h"
> > >> const MDS_CLIENT_MSG_FORMAT_VER
> > >> 

Re: [devel] [PATCH 1 of 1] rde: Change syslog priority from ER to WA when MDS send fails [#1907]

[ramesh betham]

Ack.

Thanks,
Ramesh.

On 7/8/2016 6:09 PM, Anders Widell wrote:
>   osaf/services/infrastructure/rde/rde_mds.cc |  2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
>
> RDE may fail to send MDS messages to its peer, e.g. when the peer node is 
> down.
> This should not be considered an error and therefore the syslog messages 
> should
> not have ER priority. The messages are now logged with WA priority.
>
> diff --git a/osaf/services/infrastructure/rde/rde_mds.cc 
> b/osaf/services/infrastructure/rde/rde_mds.cc
> --- a/osaf/services/infrastructure/rde/rde_mds.cc
> +++ b/osaf/services/infrastructure/rde/rde_mds.cc
> @@ -276,7 +276,7 @@ uint32_t rde_mds_send(struct rde_msg *ms
>   
>   rc = ncsmds_api();
>   if (NCSCC_RC_FAILURE == rc) {
> -  LOG_ER("Failed to send %s to %" PRIx64, rde_msg_name[msg->type],
> +  LOG_WA("Failed to send %s to %" PRIx64, rde_msg_name[msg->type],
>to_dest);
> base::Sleep(base::kOneHundredMilliseconds);
>   } else {


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

Re: [devel] [PATCH 2 of 3] lgs: director Cluster Membership (CLM) integration [#1638] V2

[Lennart Lund]

Hi Mahesh

See my new comments inline [Lennart]

Thanks
Lennart

> -Original Message-
> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
> Sent: den 3 augusti 2016 08:21
> To: Lennart Lund ; Vu Minh Nguyen
> ; Anders Widell
> 
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 2 of 3] lgs: director Cluster Membership (CLM)
> integration [#1638] V2
> 
> Hi Lennart,
> 
> I just incorporated possible comments based on the current code base,
> This My fist patch for LOG :)  , so  I just followed how existing
> functions/variables organized like.
> 
> I will considered all other inputs while doing  complete refracting code
> to C++  in a separate ticket soon.
> 
> Please see also inline response [AVM]
> 
> -AVM
> 
> On 8/2/2016 6:30 PM, Lennart Lund wrote:
> > Hi
> >
> > My comments for patch 2
> >
> > I have found some things that should be considered in order to get as clean
> code as possible:
> > 1.
> > Functions for handling CLM can be found in several files. It is better to
> collect all CLM handling in the lgs_clm file and just include function calls 
> in the
> rest of the code. Also all functions that shall be globally available should 
> have
> a prototype in the corresponding lgs_clm.h file
> [AVM]  I will considered all your inputs while doing  complete
> refracting code to C++  in a separate ticket soon.
[Lennart] I still think you should do this now. It will also make it easier to 
do the refactoring later

> > 2.
> > New global variables are added to the cb structure. Do not use global
> variables try at least to keep the scope of state variables, flags and other
> variables within the lgs_clm file. Implement setter and getter functions if
> needed or even better functions that are using the variables e.g. like the
> is_client_clm_member() function. This also makes it possible to make these
> functions thread safe (do not use the global cb lock mutex). which means
> > that this can be handled in one place instead of all over the code
> [AVM]  I moved the stuff that is possible based on current code base of
> LOG service , i will take care of your suggestions while doing complete
> refracting code to C++  in a separate ticket soon.
[Lennart] As mentioned in 1) I still think you should do this now

> > 3.
> > Take advantage of C++ and make simpler handling of lists. It's no longer
> needed to use patricia tree handling
> > Maybe even create a LgsClm class?
> [AVM] Same comment was give by Anders Widell as well.
> I will do all NCS_PATRICIA_TREE conversion to std::map of
> Log service  in one go in a separate ticket soon.
[Lennart] Why not now? There will be less and simpler code
> >
> > See also inline comments [Lennart]
> >
> > Thanks
> > Lennart
> >
> >> -Original Message-
> >> From: mahesh.va...@oracle.com [mailto:mahesh.va...@oracle.com]
> >> Sent: den 2 augusti 2016 10:18
> >> To: Vu Minh Nguyen ; Lennart Lund
> >> ; Anders Widell
> 
> >> Cc: opensaf-devel@lists.sourceforge.net
> >> Subject: [PATCH 2 of 3] lgs: director Cluster Membership (CLM)
> integration
> >> [#1638] V2
> >>
> >>   osaf/services/saf/logsv/lgs/Makefile.am |7 +-
> >>   osaf/services/saf/logsv/lgs/lgs_cb.h|   15 +++
> >>   osaf/services/saf/logsv/lgs/lgs_clm.cc  |  142
> >> +++
> >>   osaf/services/saf/logsv/lgs/lgs_clm.h   |   25 +
> >>   osaf/services/saf/logsv/lgs/lgs_evt.cc  |  143
> >> 
> >>   osaf/services/saf/logsv/lgs/lgs_evt.h   |2 +
> >>   osaf/services/saf/logsv/lgs/lgs_main.cc |   28 ++
> >>   osaf/services/saf/logsv/lgs/lgs_mds.cc  |   38 -
> >>   osaf/services/saf/logsv/lgs/lgs_util.cc |   83 ++
> >>   9 files changed, 480 insertions(+), 3 deletions(-)
> >>
> >>
> >> diff --git a/osaf/services/saf/logsv/lgs/Makefile.am
> >> b/osaf/services/saf/logsv/lgs/Makefile.am
> >> --- a/osaf/services/saf/logsv/lgs/Makefile.am
> >> +++ b/osaf/services/saf/logsv/lgs/Makefile.am
> >> @@ -37,7 +37,8 @@ noinst_HEADERS = \
> >>lgs_mbcsv_v3.h \
> >>lgs_mbcsv_v5.h \
> >>lgs_recov.h \
> >> -  lgs_imm_gcfg.h
> >> +  lgs_imm_gcfg.h \
> >> +  lgs_clm.h
> >>
> >>   osaf_execbindir = $(pkglibdir)
> >>   osaf_execbin_PROGRAMS = osaflogd
> >> @@ -67,7 +68,8 @@ osaflogd_SOURCES = \
> >>lgs_mbcsv_v3.cc \
> >>lgs_mbcsv_v5.cc \
> >>lgs_recov.cc \
> >> -  lgs_imm_gcfg.cc
> >> +  lgs_imm_gcfg.cc \
> >> +  lgs_clm.cc
> >>
> >>   osaflogd_LDADD = \
> >>$(top_builddir)/osaf/tools/safimm/src/libimmutil.la \
> >> @@ -75,4 +77,5 @@ osaflogd_LDADD = \
> >>$(top_builddir)/osaf/libs/saf/libSaAmf/libSaAmf.la \
> >>$(top_builddir)/osaf/libs/saf/libSaImm/libSaImmOi.la \
> >>$(top_builddir)/osaf/libs/saf/libSaImm/libSaImmOm.la \
> >> +  $(top_builddir)/osaf/libs/saf/libSaClm/libSaClm.la \
> >>

Re: [devel] [PATCH 1 of 1] log: Readme file for long DN support [#1315]

[Lennart Lund]

Hi Vu

In general this document should not be a document describing what to implement 
instead it should describe what is implemented and what in the log service that 
is affected by long DN and to some extent how it is implemented. See some 
comments/examples inline.
To have information here about the saflogger tool I think is ok but maybe it is 
a good idea to have a README file in the .../tools/saflog/ directory?

Also some comments inline [Lennart]

Comments on the other patches will follow

Thanks
Lennart

> -Original Message-
> From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> Sent: den 4 juli 2016 05:07
> To: mahesh.va...@oracle.com; Lennart Lund 
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: [PATCH 1 of 1] log: Readme file for long DN support [#1315]
> 
>  osaf/services/saf/logsv/README_LONGDN |  138
> ++
>  1 files changed, 138 insertions(+), 0 deletions(-)
> 
> 
> Show changes for long DN, including:
> 1) saflogger tool
> 2) log agent
> 3) log services
> 4) test suite
> 
> diff --git a/osaf/services/saf/logsv/README_LONGDN
> b/osaf/services/saf/logsv/README_LONGDN
> new file mode 100644
> --- /dev/null
> +++ b/osaf/services/saf/logsv/README_LONGDN
> @@ -0,0 +1,138 @@
> +#
> +#  -*- OpenSAF  -*-
> +#
> +# (C) Copyright 2016 The OpenSAF Foundation
> +#
> +# This program is distributed in the hope that it will be useful, but
> +# WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY
> +# or FITNESS FOR A PARTICULAR PURPOSE. This file and program are
> licensed
> +# under the GNU Lesser General Public License Version 2.1, February 1999.
> +# The complete license can be accessed from the following location:
> +# http://opensource.org/licenses/lgpl-license.php
> +# See the Copying file included with the OpenSAF distribution for full
> +# licensing terms.
> +#
> +# Author(s): Ericsson AB
> +#
> +
> +I. GENERAL INFORMATION (#1315)
> +==
> +
> +The SaNameT type, which is used to pass distinguished stream DNs name in
> saLogStreamOpen_2() API,
> +or in `logSvcUsrName`, `notificationObject` or `notifyingObject` which
> belong to `SaLogRecordT`
> +data structure passed to `saLogWriteLogAsync()` API, contains a fixed-size
> buffer that limits
> +the string length to a maximum of 255 bytes.
> +
> +#1315 ticket was raised for LOG to support long DN. The use cases could be:
> +UC1) Write log records using the long DN in @Sl, @No, @Ng tokens using
> Log API.
> +UC2) Create any long DN name stream using Log API.
> +UC3) Write record to long DN app stream using saflogger tool.
[Lennart] A stream name is not a DN. Please rephrase.
> +
> +The implementation was split into following parts:
> +1) Update saflogger tool - add one more option `-f`
> +2) Update LOG agent (API) to support Long DN
> +3) Update LOG service to support Long DN
> +4) Create test cases
> +
> +Besides, README file and OpenSAF_LOG_PR.odt have to be updated.
[Lennart] Remove this sentence

> +
> +
> +II) IMPLEMENTATION (#1315)
> +==
> +
> +1) Update saflogger tool - Add One More Option `-f`
[Lennart] This document shall contain information for existing implementation 
not what to be implemented. This means that frases like Update saflogger... or 
Add one more... etc. should not be used. This comment applies also to the rest 
of the document
This header could be changed to something like:
1) saflogger tool, '-f' option
> +-
> +`-f` option is only applicable for application stream. Means it is only valid
> +when comes together with option `-a`. If no `-f` is provided, default
> behavior is maintained.
[Lennart] Consider refrasing to:
Means it is only valid with option `-a`.

> +
> +Why need to add new option `-f` to saflogger tool?
> +When long DN is supported, user is able to put long DN name after option `-
> a`, and saflogger tool will
> +use that long DN for log file name of that app stream. In that case, the log
> file name will be over 256 characters in length.
> +As the result, logsv could get failed to create such long file name (most 
> Unix
> system supports file name length up to 255 characters).
> +
> +
> +2) Update LOG Agent (API) to Support Long DN
> +-
> +All internal SaNameT will be replaced by SaConstStringT and functions
> handling extended SaNameT are used.
> +
> +One point needs to be noticed is that, with extended SaNameT, data size
> could be up to 2048 bytes (not limited to 256 bytes any more).
> +As the result, several condition checking for max length size against 256
> bytes will be updated to 2048 bytes (kOsafMaxDnLength).
> +So, when supported long DN client (e.g: log client running on OpenSAF 5.1
> or newer versions) sends an record containing long DN
> +(e.g: `notificationObject` length > 256 bytes) to active Log service not
> support long DN (e.g: active log running 

Re: [devel] [PATCH 1 of 2] fm: Add support for self-fencing [#1859]

[Hans Nordebäck]

Hi, a gentle reminder. /Thanks HansN

-Original Message-
From: Mathivanan Naickan Palanivelu [mailto:mathi.naic...@oracle.com] 
Sent: den 14 juli 2016 11:37
To: Anders Widell ; Hans Nordebäck 
; Praveen Malviya ; 
Ramesh Babu Betham 
Cc: opensaf-devel@lists.sourceforge.net
Subject: RE: [PATCH 1 of 2] fm: Add support for self-fencing [#1859]

Iam testing this, shall revert by tomorrow.
Thanks,
Mathi.

> -Original Message-
> From: Anders Widell [mailto:anders.wid...@ericsson.com]
> Sent: Tuesday, July 12, 2016 7:08 PM
> To: Hans Nordeback; Praveen Malviya; Mathivanan Naickan Palanivelu; 
> Ramesh Babu Betham
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1 of 2] fm: Add support for self-fencing [#1859]
> 
> One comment: in the prototype patch the feature was on by default, but 
> it ought to be off by default when we introduce this feature officially.
> 
> / Anders Widell
> 
> On 06/30/2016 10:32 AM, Anders Widell wrote:
> > Hi!
> >
> > This patch is actually identical to the prototype code that I wrote 
> > and attached to the ticket, so I am not sure if I am supposed to 
> > also review it... anyways it is ack from from me for the first 
> > patch. :-)
> >
> > regards,
> > Anders Widell
> >
> > On 06/23/2016 07:31 AM, Hans Nordeback wrote:
> >> osaf/services/infrastructure/fm/fms/fm_cb.h   |  10 +
> >>   osaf/services/infrastructure/fm/fms/fm_main.c |  16 +++-
> >>   osaf/services/infrastructure/fm/fms/fm_mds.c  |  51
> >> +++
> >>   3 files changed, 75 insertions(+), 2 deletions(-)
> >>
> >>
> >> In situations where remote fencing is not possible, this patch adds 
> >> support for self-fencing.
> >>
> >> diff --git a/osaf/services/infrastructure/fm/fms/fm_cb.h
> >> b/osaf/services/infrastructure/fm/fms/fm_cb.h
> >> --- a/osaf/services/infrastructure/fm/fms/fm_cb.h
> >> +++ b/osaf/services/infrastructure/fm/fms/fm_cb.h
> >> @@ -27,6 +27,10 @@
> >>   #include "rda_papi.h"
> >>   #include "fm_amf.h"
> >>   +#include 
> >> +#include 
> >> +#include 
> >> +
> >>   uint32_t gl_fm_hdl;
> >> typedef enum {
> >> @@ -92,6 +96,12 @@ typedef struct fm_cb {
> >>   bool amfnd_down;
> >>   bool amfd_down;
> >>   bool fm_down;
> >> +
> >> +bool peer_sc_up;
> >> +bool well_connected;
> >> +uint64_t cluster_size;
> >> +struct timespec last_well_connected;
> >> +struct timespec node_isolation_timeout;
> >>   } FM_CB;
> >> extern char *role_string[];
> >> diff --git a/osaf/services/infrastructure/fm/fms/fm_main.c
> >> b/osaf/services/infrastructure/fm/fms/fm_main.c
> >> --- a/osaf/services/infrastructure/fm/fms/fm_main.c
> >> +++ b/osaf/services/infrastructure/fm/fms/fm_main.c
> >> @@ -30,7 +30,7 @@ This file contains the main() routine fo
> >> #include 
> >>   #include "fm.h"
> >> -
> >> +#include "osaf_time.h"
> >> enum {
> >>   FD_TERM = 0,
> >> @@ -411,7 +411,19 @@ static uint32_t fm_get_args(FM_CB *fm_cb
> >>   fm_cb->promote_active_tmr.type = FM_TMR_PROMOTE_ACTIVE;
> >>   fm_cb->activation_supervision_tmr.type = 
> >> FM_TMR_ACTIVATION_SUPERVISION;
> >>   -  TRACE_LEAVE();
> >> +char* node_isolation_timeout =
> >> getenv("FMS_NODE_ISOLATION_TIMEOUT");
> >> +if (node_isolation_timeout != NULL) {
> >> +osaf_millis_to_timespec(atoi(node_isolation_timeout),
> >> +_cb->node_isolation_timeout);
> >> +} else {
> >> +fm_cb->node_isolation_timeout.tv_sec = 10;
> >> +fm_cb->node_isolation_timeout.tv_nsec = 0;
> >> +}
> >> +TRACE("NODE_ISOLATION_TIMEOUT = %" PRId64 ".%09ld",
> >> +  (int64_t) fm_cb->node_isolation_timeout.tv_sec,
> >> +  fm_cb->node_isolation_timeout.tv_nsec);
> >> +
> >> +TRACE_LEAVE();
> >>   return NCSCC_RC_SUCCESS;
> >>   }
> >>   diff --git a/osaf/services/infrastructure/fm/fms/fm_mds.c
> >> b/osaf/services/infrastructure/fm/fms/fm_mds.c
> >> --- a/osaf/services/infrastructure/fm/fms/fm_mds.c
> >> +++ b/osaf/services/infrastructure/fm/fms/fm_mds.c
> >> @@ -16,6 +16,8 @@
> >>   */
> >> #include "fm.h"
> >> +#include "osaf_time.h"
> >> +#include "ncssysf_def.h"
> >> const MDS_CLIENT_MSG_FORMAT_VER 
> >> fm_fm_msg_fmt_map_table[FM_SUBPART_VER_MAX] = {
> FM_FM_MSG_FMT_VER_1 };
> >>   @@ -28,6 +30,8 @@ static uint32_t fm_encode(MDS_CALLBACK_E
> >>   static uint32_t fm_decode(MDS_CALLBACK_DEC_INFO *dec_info);
> >>   static uint32_t fm_fm_mds_enc(MDS_CALLBACK_ENC_INFO *enc_info);
> >>   static uint32_t fm_fm_mds_dec(MDS_CALLBACK_DEC_INFO *dec_info);
> >> +static void check_for_node_isolation(FM_CB *cb); static bool 
> >> +has_been_well_connected_recently(FM_CB *cb);
> >>   static uint32_t fm_mds_node_evt(FM_CB *cb, 
> >> MDS_CALLBACK_NODE_EVENT_INFO * node_evt);
> >>   static uint32_t fm_fill_mds_evt_post_fm_mbx(FM_CB *cb, FM_EVT 
> >> *fm_evt, NODE_ID node_id, FM_FSM_EVT_CODE 

Re: [devel] [PATCH 0 of 1] Review Request for rde: Change syslog priority from ER to WA when MDS send fails [#1907]

[Anders Widell]

I will push this tomorrow if there are no comments.

regards,

Anders Widell

On 08/02/2016 09:57 AM, Anders Widell wrote:
> Hi Ramesh!
>
> Have you had a chance to look at this patch yet?
>
> regards,
>
> Anders Widell
>
> On 07/08/2016 02:39 PM, Anders Widell wrote:
>> Summary: rde: Change syslog priority from ER to WA when MDS send fails 
>> [#1907]
>> Review request for Trac Ticket(s): 1907
>> Peer Reviewer(s): Ramesh
>> Pull request to:
>> Affected branch(es): opensaf-5.0.x, default(5.1)
>> Development branch: default
>>
>> 
>> Impacted area   Impact y/n
>> 
>>Docsn
>>Build systemn
>>RPM/packaging   n
>>Configuration files n
>>Startup scripts n
>>SAF servicesn
>>OpenSAF servicesy
>>Core libraries  n
>>Samples n
>>Tests   n
>>Other   n
>>
>>
>> Comments (indicate scope for each "y" above):
>> -
>>
>> changeset d6a6b95b4b5d707a66fb3ddf3549ed1833dd56fe
>> Author:  Anders Widell 
>> Date:Fri, 08 Jul 2016 10:17:27 +0200
>>
>>  rde: Change syslog priority from ER to WA when MDS send fails [#1907]
>>
>>  RDE may fail to send MDS messages to its peer, e.g. when the peer node 
>> is
>>  down. This should not be considered an error and therefore the syslog
>>  messages should not have ER priority. The messages are now logged with 
>> WA
>>  priority.
>>
>>
>> Complete diffstat:
>> --
>>osaf/services/infrastructure/rde/rde_mds.cc |  2 +-
>>1 files changed, 1 insertions(+), 1 deletions(-)
>>
>>
>> Testing Commands:
>> -
>> Run test cases for system controller failovers.
>>
>>
>> Testing, Expected Results:
>> --
>> The syslog message "Failed to send RDE_MSG_PEER_INFO_RESP(4) to ...", if 
>> seen,
>> should be logged with WARNING priority.
>>
>>
>> Conditions of Submission:
>> -
>> Ack from reviewer(s)
>>
>>
>> Arch  Built StartedLinux distro
>> ---
>> mipsn  n
>> mips64  n  n
>> x86 n  n
>> x86_64  y  y
>> powerpc n  n
>> powerpc64   n  n
>>
>>
>> Reviewer Checklist:
>> ---
>> [Submitters: make sure that your review doesn't trigger any checkmarks!]
>>
>>
>> Your checkin has not passed review because (see checked entries):
>>
>> ___ Your RR template is generally incomplete; it has too many blank entries
>>   that need proper data filled in.
>>
>> ___ You have failed to nominate the proper persons for review and push.
>>
>> ___ Your patches do not have proper short+long header
>>
>> ___ You have grammar/spelling in your header that is unacceptable.
>>
>> ___ You have exceeded a sensible line length in your headers/comments/text.
>>
>> ___ You have failed to put in a proper Trac Ticket # into your commits.
>>
>> ___ You have incorrectly put/left internal data in your comments/files
>>   (i.e. internal bug tracking tool IDs, product names etc)
>>
>> ___ You have not given any evidence of testing beyond basic build tests.
>>   Demonstrate some level of runtime or other sanity testing.
>>
>> ___ You have ^M present in some of your files. These have to be removed.
>>
>> ___ You have needlessly changed whitespace or added whitespace crimes
>>   like trailing spaces, or spaces before tabs.
>>
>> ___ You have mixed real technical changes with whitespace and other
>>   cosmetic code cleanup changes. These have to be separate commits.
>>
>> ___ You need to refactor your submission into logical chunks; there is
>>   too much content into a single commit.
>>
>> ___ You have extraneous garbage in your review (merge commits etc)
>>
>> ___ You have giant attachments which should never have been sent;
>>   Instead you should place your content in a public tree to be pulled.
>>
>> ___ You have too many commits attached to an e-mail; resend as threaded
>>   commits, or place in a public tree for a pull.
>>
>> ___ You have resent this content multiple times without a clear indication
>>   of what has changed between each re-send.
>>
>> ___ You have failed to adequately and individually address all of the
>>   comments and change requests that were proposed in the initial review.
>>
>> ___ You have a misconfigured ~/.hgrc file (i.e. username, email etc)
>>
>> ___ Your computer have a badly configured date and time; confusing the
>>   the threaded patch review.
>>
>> ___ Your changes affect IPC mechanism, and you don't present any results
>>   for in-service upgradability test.
>>
>> ___ Your changes affect user manual and documentation, your patch series
>>   do not contain the patch that updates the Doxygen manual.
>>

Re: [devel] [PATCH 1 of 5] amfd: replace SaNameT with string in include dir [#1642]

[praveen malviya]

On 02-Aug-16 3:39 AM, minh chau wrote:
> Hi Praveen,
>
> One comment with [Minh] in line.
>
> Thanks,
> Minh
>
> On 01/08/16 17:22, Gary Lee wrote:
>> Hi Praveen
>>
>> On 1/08/2016 4:29 PM, praveen malviya wrote:
>>> Hi Gary, Long,
>>>
>>> Some comments/observations:
>>> -In AMFD saAisNameBorrow() is used in logging and AMFND uses
>>> osaf_extended_name_borrow().
>>> For osaf_extended_name_borrow() note in osaf_extended_name.h says it
>>> is intended for mainly agent libraries. But middle-ware services
>>> always use core libs. At the same time saAisNameBorrow(), I think, is
>>> for application.
>>> any reason of using them this way and what is the recommended way?
>> I think I used both styles in amfd. I think we can change saAisNameXX
>> to osaf_extended_name_XX just before pushing, to make it consistent
>> with the rest of the OpenSAF services.
>>> -I think, one case may arrive from upgrade perspective.
>>> Suppose any application (say amf_demo app) is running without
>>> enabling long dn and a csi, with its RDn greater than 256, is added
>>> dynamically (long dn enabled in IMM). In this case AMFD will assign
>>> this csi to the running component. Component will not be able to read
>>> the CSI and may crash.
>>> This is related to invocation of CSI_SET callback but same will be
>>> valid for PG tracking also. There may be other cases also.
>>> Even truncation will not work in this case.
> [Minh] I think the agent patch that Gary submitted currently returns
> SA_AIS_ERR_NAME_TOO_LONG in saAmfDispatch() if long DN callback comes to
> legacy application (unadapted long DN app). The real callback won't be
> issued but application may crash if it exit() on non-SA_AIS_OK from
> Dispatch(). I guess you have seen this with #1553? Do you think it's
> good way if amf agent drops the long DN callback and also Dispatch()
> returns OK to legacy app, and print error in syslog?
Not observed in the context of #1553, but I remember about such a fix in 
NTF long dn changes.
Returning SA_AIS_OK in Dispatch() call saves application from crash, but 
the problem in AMF is still different as it will maintain a COMPCSI 
relationship without comp being actually assigned for that.

Can we think of a way where AMF will not allow this conf change by 
rejecting the CCB operation itself. For this AMF should know that this 
application supports Long dn or not. But this information needs to be 
carried from agent to AMFD.
Before going for such a way, I think, we can ask opinion from other AMF 
maintainers.

Thanks,
Praveen



>>> - While running some tests observed crashes in amfnd and amfd.
>>> I will update #1642 with bt information.
>> Minh will answer this bit.
>>
>> Thanks
>> Gary
>>
>

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

Re: [devel] [PATCH 2 of 3] lgs: director Cluster Membership (CLM) integration [#1638] V2

[A V Mahesh]

Hi Lennar,

I just incorporated possible comments based on the current code base,
This My fist patch for LOG :)  , so  I just followed how existing 
functions/variables organized like.

I will considered all other inputs while doing  complete refracting code 
to C++  in a separate ticket soon.

Please see also inline response [AVM]

-AVM

On 8/2/2016 6:30 PM, Lennart Lund wrote:
> Hi
>
> My comments for patch 2
>
> I have found some things that should be considered in order to get as clean 
> code as possible:
> 1.
> Functions for handling CLM can be found in several files. It is better to 
> collect all CLM handling in the lgs_clm file and just include function calls 
> in the rest of the code. Also all functions that shall be globally available 
> should have a prototype in the corresponding lgs_clm.h file
[AVM]  I will considered all your inputs while doing  complete 
refracting code to C++  in a separate ticket soon.
> 2.
> New global variables are added to the cb structure. Do not use global 
> variables try at least to keep the scope of state variables, flags and other 
> variables within the lgs_clm file. Implement setter and getter functions if 
> needed or even better functions that are using the variables e.g. like the 
> is_client_clm_member() function. This also makes it possible to make these 
> functions thread safe (do not use the global cb lock mutex). which means
> that this can be handled in one place instead of all over the code
[AVM]  I moved the stuff that is possible based on current code base of 
LOG service , i will take care of your suggestions while doing complete 
refracting code to C++  in a separate ticket soon.
> 3.
> Take advantage of C++ and make simpler handling of lists. It's no longer 
> needed to use patricia tree handling
> Maybe even create a LgsClm class?
[AVM] Same comment was give by Anders Widell as well.
I will do all NCS_PATRICIA_TREE conversion to std::map of 
Log service  in one go in a separate ticket soon.
>
> See also inline comments [Lennart]
>   
> Thanks
> Lennart
>
>> -Original Message-
>> From: mahesh.va...@oracle.com [mailto:mahesh.va...@oracle.com]
>> Sent: den 2 augusti 2016 10:18
>> To: Vu Minh Nguyen ; Lennart Lund
>> ; Anders Widell 
>> Cc: opensaf-devel@lists.sourceforge.net
>> Subject: [PATCH 2 of 3] lgs: director Cluster Membership (CLM) integration
>> [#1638] V2
>>
>>   osaf/services/saf/logsv/lgs/Makefile.am |7 +-
>>   osaf/services/saf/logsv/lgs/lgs_cb.h|   15 +++
>>   osaf/services/saf/logsv/lgs/lgs_clm.cc  |  142
>> +++
>>   osaf/services/saf/logsv/lgs/lgs_clm.h   |   25 +
>>   osaf/services/saf/logsv/lgs/lgs_evt.cc  |  143
>> 
>>   osaf/services/saf/logsv/lgs/lgs_evt.h   |2 +
>>   osaf/services/saf/logsv/lgs/lgs_main.cc |   28 ++
>>   osaf/services/saf/logsv/lgs/lgs_mds.cc  |   38 -
>>   osaf/services/saf/logsv/lgs/lgs_util.cc |   83 ++
>>   9 files changed, 480 insertions(+), 3 deletions(-)
>>
>>
>> diff --git a/osaf/services/saf/logsv/lgs/Makefile.am
>> b/osaf/services/saf/logsv/lgs/Makefile.am
>> --- a/osaf/services/saf/logsv/lgs/Makefile.am
>> +++ b/osaf/services/saf/logsv/lgs/Makefile.am
>> @@ -37,7 +37,8 @@ noinst_HEADERS = \
>>  lgs_mbcsv_v3.h \
>>  lgs_mbcsv_v5.h \
>>  lgs_recov.h \
>> -lgs_imm_gcfg.h
>> +lgs_imm_gcfg.h \
>> +lgs_clm.h
>>
>>   osaf_execbindir = $(pkglibdir)
>>   osaf_execbin_PROGRAMS = osaflogd
>> @@ -67,7 +68,8 @@ osaflogd_SOURCES = \
>>  lgs_mbcsv_v3.cc \
>>  lgs_mbcsv_v5.cc \
>>  lgs_recov.cc \
>> -lgs_imm_gcfg.cc
>> +lgs_imm_gcfg.cc \
>> +lgs_clm.cc
>>
>>   osaflogd_LDADD = \
>>  $(top_builddir)/osaf/tools/safimm/src/libimmutil.la \
>> @@ -75,4 +77,5 @@ osaflogd_LDADD = \
>>  $(top_builddir)/osaf/libs/saf/libSaAmf/libSaAmf.la \
>>  $(top_builddir)/osaf/libs/saf/libSaImm/libSaImmOi.la \
>>  $(top_builddir)/osaf/libs/saf/libSaImm/libSaImmOm.la \
>> +$(top_builddir)/osaf/libs/saf/libSaClm/libSaClm.la \
>>  $(top_builddir)/osaf/libs/agents/infrastructure/rda/librda.la
>> diff --git a/osaf/services/saf/logsv/lgs/lgs_cb.h
>> b/osaf/services/saf/logsv/lgs/lgs_cb.h
>> --- a/osaf/services/saf/logsv/lgs/lgs_cb.h
>> +++ b/osaf/services/saf/logsv/lgs/lgs_cb.h
>> @@ -21,6 +21,7 @@
>>   #include 
>>   #include 
>>   #include 
>> +#include 
>>   #include 
>>   #include 
>>
>> @@ -55,6 +56,11 @@ typedef struct lgs_stream_list {
>>   } lgs_stream_list_t;
>>
>>   typedef struct {
>> +NCS_PATRICIA_NODE patnode;
>> +NODE_ID node_id;
>> +} lgs_clm_node_t;
>> +
> [Lennart] Would it be possible to not add new patricia node handling? Since 
> the log service is compiled as C++ code C++ list tools could be used instead?
> lgs_clm_node_find
[AVM] Same comment was give by Anders Widell as well.
I will do all NCS_PATRICIA_TREE conversion to