Re: [devel] [PATCH 1/1] dtm: change trace config var name to _PATHNAME [#2792]

2018-03-06 Thread Vu Minh Nguyen
Hi Srinivas,

Looks good to me. Thanks!

Regards, Vu

> -Original Message-
> From: Srinivas Mangipudy [mailto:srinivas.mangip...@oracle.com]
> Sent: Monday, March 5, 2018 5:14 PM
> To: Vu Minh Nguyen ;
> anders.wid...@ericsson.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: RE: [PATCH 1/1] dtm: change trace config var name to _PATHNAME
> [#2792]
> 
> Hi Vu,
> 
> I updated OpenSAF_IMMsv_PR.odt and the document is at:
> https://sourceforge.net/p/opensaf/documentation/ci/default/tree/OpenSAF_I
> MMSv_PR.odt
> 
> Can you please review.
> 
> Thank you
> Srinivas
> 
> 
> 
> 
> -Original Message-
> From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> Sent: Thursday, March 1, 2018 1:30 PM
> To: srinivas ; anders.wid...@ericsson.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: RE: [PATCH 1/1] dtm: change trace config var name to _PATHNAME
> [#2792]
> 
> Ack with a  minor comment, with [Vu].
> 
> Regards, Vu
> 
> > -Original Message-
> > From: srinivas [mailto:srinivas.mangip...@oracle.com]
> > Sent: Thursday, March 1, 2018 2:31 PM
> > To: anders.wid...@ericsson.com; vu.m.ngu...@dektech.com.au
> > Cc: opensaf-devel@lists.sourceforge.net; srinivas
> > 
> > Subject: [PATCH 1/1] dtm: change trace config var name to _PATHNAME
> > [#2792]
> >
> > ---
> >  src/imm/README  | 9 ++---
> >  src/imm/immloadd/imm_loader.cc  | 2 +-
> >  src/imm/immnd/immnd.conf| 2 +-
> >  src/imm/immpbed/immpbe.cc   | 2 +-
> >  src/imm/tools/imm_dumper.cc | 2 +-
> >  src/ntf/README  | 7 +--
> >  src/ntf/ntfd/ntfd.conf  | 2 +-
> >  src/ntf/ntfimcnd/ntfimcn_main.c | 2 +-
> >  8 files changed, 17 insertions(+), 11 deletions(-)
> >
> > diff --git a/src/imm/README b/src/imm/README index ee5f8e8..750d811
> > 100644
> > --- a/src/imm/README
> > +++ b/src/imm/README
> > @@ -3206,12 +3206,12 @@ in immd.conf/immnd.conf and restart the
> cluster.
> >
> >  Errors, warnings and notice level messages are logged to the syslog.
> >
> > -To enable traces in the IMM library, export the variable
> > IMMA_TRACE_FILENAME
> > +To enable traces in the IMM library, export the variable
> > IMMA_TRACE_PATHNAME
> >  with a valid pathname before starting the application using the IMM
> library.
> [Vu] It may be good information for user to state here fixed location of
trace
> file as you already did for NTF README as below.
> +Traces are always stored in $PKGLOGDIR directory and the directory
> component
> +of the path name (if any) is ignored.
> 
> In OpenSAF_IMMsv_PR.odt
> (https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__sourceforge.net_p_opensaf_documentation_ci_default_tree_OpenSAF-
> 5FIMM=DwICAg=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE
> =rU6x356sikQZSi7Ttc2DuiqAgbc0QIeANg72N5AllVc=JgHty7D6Tv9zeykUc
> iK5bglWJyc6T9fA5EnNFw1Mm7Q=A7QbkwCI_4xr19yJ5_fLor10DmmuVruba
> AkDIvQF2yc=
> Sv_PR.odt),
> There is a statement regarding agent trace, I guess we should consider to
> update it too, chapter 3.6.2 Configuration:
> " Tracing of he IMMA library is possible by defining the environment
variable
> IMMA_TRACE_PATHNAME. The variable should have a legal file path and the
> trace will be appended to the file identified by that path"
> 
> >
> >  For example:
> >
> > -$ export IMMA_TRACE_FILENAME=imm.trace
> > +$ export IMMA_TRACE_PATHNAME=imm.trace
> >  $ ./immomtest
> >  $ cat $pkglogdir/imm.trace
> >
> > @@ -3220,8 +3220,11 @@ It is also possible to trace slave processes
> > forked by the IMMND.
> >  This would be processes for loading, sync and dump/pbe.
> >  To enable such trace uncomment:
> >
> > -#export IMMSV_TRACE_FILENAME=osafimmnd
> > +#export IMMSV_TRACE_PATHNAME=osafimmnd
> >
> > +It is recommended to use osaflog command as it takes care of flushing
> > +unwritten trace messages from memory to disk, as well as
> > +concatenating the pieces that may have resulted from log rotation of
the
> trace stream.
> >
> >  TEST
> >
> > diff --git a/src/imm/immloadd/imm_loader.cc
> > b/src/imm/immloadd/imm_loader.cc index 13fb417..de5a575 100644
> > --- a/src/imm/immloadd/imm_loader.cc
> > +++ b/src/imm/immloadd/imm_loader.cc
> > @@ -2507,7 +2507,7 @@ int main(int argc, char *argv[]) {
> >  exit(1);
> >}
> >
> > -  if ((logPath = getenv("IMMSV_TRACE_FILENAME"))) {
> > +  if ((logPath = getenv("IMMSV_TRACE_PATHNAME"))) {
> >  category_mask = 0x; /* TODO: set using env variable ? */
> >} else {
> >  logPath = defaultLog;
> > diff --git a/src/imm/immnd/immnd.conf b/src/imm/immnd/immnd.conf
> index
> > 9172677..b6a4823 100644
> > --- a/src/imm/immnd/immnd.conf
> > +++ b/src/imm/immnd/immnd.conf
> > @@ -12,7 +12,7 @@
> >  # they attach as IMMA clients. These processes will also route trace
> > to
> the
> >  # IMMND trace-file as define here. Traces are always stored in
> > $PKGLOGDIR  # directory and the directory component of the path name
> > (if 

Re: [devel] [PATCH 1/1] imm: not allow creating reserved IMM class names [#2771]

2018-03-06 Thread Hans Nordebäck

Hi Vu,

perhaps it is a bit easier to understand if you rename "len" to 
"no_of_elements" ? /Hans



On 03/07/2018 08:07 AM, Vu Minh Nguyen wrote:

Hi Hans,

Your suggestion is shorter and clean. But for me, it is a bit hard to 
understand that code.

cb->reserved_class_names = (char**)calloc(1, (len + 1) * sizeof(char*));

Looking at the above code, it says allocating an array with (len + 1) elements 
of char*. Easier to understand, I think.

Regards, Vu


-Original Message-
From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
Sent: Wednesday, March 7, 2018 1:31 PM
To: Zoran Milinkovic ; Vu Minh Nguyen
; ravisekhar.ko...@oracle.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [devel] [PATCH 1/1] imm: not allow creating reserved IMM class
names [#2771]

Hi,

regarding Zoran's comment below, one question, isn't this code
redundant, regarding divide/multiply:

size_t len = sizeof(default_reserved_names)/
sizeof(default_reserved_names[0]);
cb->reserved_class_names =
(char**)calloc(1, len * sizeof(char*) + 1);

instead:

cb->reserved_class_names =
(char**)calloc(1, sizeof(default_reserved_names) +
sizeof(char*));

?

/Hans

On 03/06/2018 04:35 PM, Zoran Milinkovic wrote:

Hi Vu,

There are few things that I have found

1. imma_om_api.cc does not need to be changed.

2.
There is a wrong calculation for a new allocated memory in

populate_reserved_class_names().

It is:
+   cb->reserved_class_names =
+   (char**)calloc(1, len * sizeof(char*) + 1);
... but it should be...
+   cb->reserved_class_names =
+   (char**)calloc(1, (len + 1) * sizeof(char*));

3.
Both is_regular_name() and is_valid_schema_name() can be written in

immnd_main.c file where they are used first. Also add function definitions to
the header immnd_init.h.

There is no need for a new files.
I would also prefix functions with "immnd_"

Ack from me with minor comments.
No need to send the patch for another review

Thanks,
Zoran

-Original Message-
From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
Sent: den 30 januari 2018 15:24
To: Zoran Milinkovic ;

ravisekhar.ko...@oracle.com

Cc: opensaf-devel@lists.sourceforge.net; Vu Minh Nguyen



Subject: [PATCH 1/1] imm: not allow creating reserved IMM class names

[#2771]

PBE will be restarted and will not be able to come up if user requests
creating IMM object class with same name of reserved ones.

This patch adds code to reject such request with

SA_AIS_ERR_INVALID_PARAM.

---
   src/imm/Makefile.am|  2 +
   src/imm/agent/imma_om_api.cc   |  1 -
   .../apitest/management/test_saImmOmClassCreate_2.c | 48 +++
   src/imm/immnd/ImmModel.cc  | 55 +---
   src/imm/immnd/immnd.conf   |  9 ++
   src/imm/immnd/immnd_cb.h   |  1 +
   src/imm/immnd/immnd_common.c   | 75 
   src/imm/immnd/immnd_common.h   | 32 +++
   src/imm/immnd/immnd_evt.c  | 17 
   src/imm/immnd/immnd_main.c | 99

+-

   10 files changed, 285 insertions(+), 54 deletions(-)
   create mode 100644 src/imm/immnd/immnd_common.c
   create mode 100644 src/imm/immnd/immnd_common.h

diff --git a/src/imm/Makefile.am b/src/imm/Makefile.am
index b7e4826..099efda 100644
--- a/src/imm/Makefile.am
+++ b/src/imm/Makefile.am
@@ -163,6 +163,7 @@ noinst_HEADERS += \
src/imm/immnd/immnd.h \
src/imm/immnd/immnd_cb.h \
src/imm/immnd/immnd_init.h \
+   src/imm/immnd/immnd_common.h \
src/imm/immpbed/immpbe.h \
src/imm/tools/imm_dumper.h

@@ -332,6 +333,7 @@ bin_osafimmnd_CPPFLAGS = \
$(AM_CPPFLAGS)

   bin_osafimmnd_SOURCES = \
+   src/imm/immnd/immnd_common.c \
src/imm/immnd/immnd_amf.c \
src/imm/immnd/immnd_db.c \
src/imm/immnd/immnd_evt.c \
diff --git a/src/imm/agent/imma_om_api.cc

b/src/imm/agent/imma_om_api.cc

index 7155799..a06f0ea 100644
--- a/src/imm/agent/imma_om_api.cc
+++ b/src/imm/agent/imma_om_api.cc
@@ -55,7 +55,6 @@ static bool immOmIsLoader = false;
   static const char *sysaClName = SA_IMM_ATTR_CLASS_NAME;
   static const char *sysaAdmName =

SA_IMM_ATTR_ADMIN_OWNER_NAME;

   static const char *sysaImplName = SA_IMM_ATTR_IMPLEMENTER_NAME;
-
   static int imma_om_resurrect(IMMA_CB *cb, IMMA_CLIENT_NODE

*cl_node,

bool *locked);
   static SaAisErrorT imma_finalizeCcb(SaImmCcbHandleT ccbHandle,
diff --git a/src/imm/apitest/management/test_saImmOmClassCreate_2.c

b/src/imm/apitest/management/test_saImmOmClassCreate_2.c

index 3ae4b0f..967b819 100644
--- 

Re: [devel] [PATCH 1/1] imm: not allow creating reserved IMM class names [#2771]

2018-03-06 Thread Vu Minh Nguyen
Hi Hans,

Your suggestion is shorter and clean. But for me, it is a bit hard to 
understand that code.

cb->reserved_class_names = (char**)calloc(1, (len + 1) * sizeof(char*));

Looking at the above code, it says allocating an array with (len + 1) elements 
of char*. Easier to understand, I think.

Regards, Vu

> -Original Message-
> From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
> Sent: Wednesday, March 7, 2018 1:31 PM
> To: Zoran Milinkovic ; Vu Minh Nguyen
> ; ravisekhar.ko...@oracle.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [devel] [PATCH 1/1] imm: not allow creating reserved IMM class
> names [#2771]
> 
> Hi,
> 
> regarding Zoran's comment below, one question, isn't this code
> redundant, regarding divide/multiply:
> 
> size_t len = sizeof(default_reserved_names)/
>   sizeof(default_reserved_names[0]);
>   cb->reserved_class_names =
>   (char**)calloc(1, len * sizeof(char*) + 1);
> 
> instead:
> 
> cb->reserved_class_names =
>   (char**)calloc(1, sizeof(default_reserved_names) +
> sizeof(char*));
> 
> ?
> 
> /Hans
> 
> On 03/06/2018 04:35 PM, Zoran Milinkovic wrote:
> > Hi Vu,
> >
> > There are few things that I have found
> >
> > 1. imma_om_api.cc does not need to be changed.
> >
> > 2.
> > There is a wrong calculation for a new allocated memory in
> populate_reserved_class_names().
> > It is:
> > +   cb->reserved_class_names =
> > +   (char**)calloc(1, len * sizeof(char*) + 1);
> > ... but it should be...
> > +   cb->reserved_class_names =
> > +   (char**)calloc(1, (len + 1) * sizeof(char*));
> >
> > 3.
> > Both is_regular_name() and is_valid_schema_name() can be written in
> immnd_main.c file where they are used first. Also add function definitions to
> the header immnd_init.h.
> > There is no need for a new files.
> > I would also prefix functions with "immnd_"
> >
> > Ack from me with minor comments.
> > No need to send the patch for another review
> >
> > Thanks,
> > Zoran
> >
> > -Original Message-
> > From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> > Sent: den 30 januari 2018 15:24
> > To: Zoran Milinkovic ;
> ravisekhar.ko...@oracle.com
> > Cc: opensaf-devel@lists.sourceforge.net; Vu Minh Nguyen
> 
> > Subject: [PATCH 1/1] imm: not allow creating reserved IMM class names
> [#2771]
> >
> > PBE will be restarted and will not be able to come up if user requests
> > creating IMM object class with same name of reserved ones.
> >
> > This patch adds code to reject such request with
> SA_AIS_ERR_INVALID_PARAM.
> > ---
> >   src/imm/Makefile.am|  2 +
> >   src/imm/agent/imma_om_api.cc   |  1 -
> >   .../apitest/management/test_saImmOmClassCreate_2.c | 48 +++
> >   src/imm/immnd/ImmModel.cc  | 55 +---
> >   src/imm/immnd/immnd.conf   |  9 ++
> >   src/imm/immnd/immnd_cb.h   |  1 +
> >   src/imm/immnd/immnd_common.c   | 75 
> >   src/imm/immnd/immnd_common.h   | 32 +++
> >   src/imm/immnd/immnd_evt.c  | 17 
> >   src/imm/immnd/immnd_main.c | 99
> +-
> >   10 files changed, 285 insertions(+), 54 deletions(-)
> >   create mode 100644 src/imm/immnd/immnd_common.c
> >   create mode 100644 src/imm/immnd/immnd_common.h
> >
> > diff --git a/src/imm/Makefile.am b/src/imm/Makefile.am
> > index b7e4826..099efda 100644
> > --- a/src/imm/Makefile.am
> > +++ b/src/imm/Makefile.am
> > @@ -163,6 +163,7 @@ noinst_HEADERS += \
> > src/imm/immnd/immnd.h \
> > src/imm/immnd/immnd_cb.h \
> > src/imm/immnd/immnd_init.h \
> > +   src/imm/immnd/immnd_common.h \
> > src/imm/immpbed/immpbe.h \
> > src/imm/tools/imm_dumper.h
> >
> > @@ -332,6 +333,7 @@ bin_osafimmnd_CPPFLAGS = \
> > $(AM_CPPFLAGS)
> >
> >   bin_osafimmnd_SOURCES = \
> > +   src/imm/immnd/immnd_common.c \
> > src/imm/immnd/immnd_amf.c \
> > src/imm/immnd/immnd_db.c \
> > src/imm/immnd/immnd_evt.c \
> > diff --git a/src/imm/agent/imma_om_api.cc
> b/src/imm/agent/imma_om_api.cc
> > index 7155799..a06f0ea 100644
> > --- a/src/imm/agent/imma_om_api.cc
> > +++ b/src/imm/agent/imma_om_api.cc
> > @@ -55,7 +55,6 @@ static bool immOmIsLoader = false;
> >   static const char *sysaClName = SA_IMM_ATTR_CLASS_NAME;
> >   static const char *sysaAdmName =
> SA_IMM_ATTR_ADMIN_OWNER_NAME;
> >   static const char *sysaImplName = SA_IMM_ATTR_IMPLEMENTER_NAME;
> > -
> >   static int imma_om_resurrect(IMMA_CB *cb, IMMA_CLIENT_NODE
> *cl_node,
> >bool *locked);
> >   static SaAisErrorT imma_finalizeCcb(SaImmCcbHandleT ccbHandle,
> > diff --git 

Re: [devel] [PATCH 1/1] imm: not allow creating reserved IMM class names [#2771]

2018-03-06 Thread Hans Nordebäck

Hi,

regarding Zoran's comment below, one question, isn't this code 
redundant, regarding divide/multiply:


size_t len = sizeof(default_reserved_names)/
sizeof(default_reserved_names[0]);
cb->reserved_class_names =
(char**)calloc(1, len * sizeof(char*) + 1);

instead:

cb->reserved_class_names =
(char**)calloc(1, sizeof(default_reserved_names) + 
sizeof(char*));

?

/Hans

On 03/06/2018 04:35 PM, Zoran Milinkovic wrote:

Hi Vu,

There are few things that I have found

1. imma_om_api.cc does not need to be changed.

2.
There is a wrong calculation for a new allocated memory in 
populate_reserved_class_names().
It is:
+   cb->reserved_class_names =
+   (char**)calloc(1, len * sizeof(char*) + 1);
... but it should be...
+   cb->reserved_class_names =
+   (char**)calloc(1, (len + 1) * sizeof(char*));

3.
Both is_regular_name() and is_valid_schema_name() can be written in 
immnd_main.c file where they are used first. Also add function definitions to 
the header immnd_init.h.
There is no need for a new files.
I would also prefix functions with "immnd_"

Ack from me with minor comments.
No need to send the patch for another review

Thanks,
Zoran

-Original Message-
From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
Sent: den 30 januari 2018 15:24
To: Zoran Milinkovic ; 
ravisekhar.ko...@oracle.com
Cc: opensaf-devel@lists.sourceforge.net; Vu Minh Nguyen 

Subject: [PATCH 1/1] imm: not allow creating reserved IMM class names [#2771]

PBE will be restarted and will not be able to come up if user requests
creating IMM object class with same name of reserved ones.

This patch adds code to reject such request with SA_AIS_ERR_INVALID_PARAM.
---
  src/imm/Makefile.am|  2 +
  src/imm/agent/imma_om_api.cc   |  1 -
  .../apitest/management/test_saImmOmClassCreate_2.c | 48 +++
  src/imm/immnd/ImmModel.cc  | 55 +---
  src/imm/immnd/immnd.conf   |  9 ++
  src/imm/immnd/immnd_cb.h   |  1 +
  src/imm/immnd/immnd_common.c   | 75 
  src/imm/immnd/immnd_common.h   | 32 +++
  src/imm/immnd/immnd_evt.c  | 17 
  src/imm/immnd/immnd_main.c | 99 +-
  10 files changed, 285 insertions(+), 54 deletions(-)
  create mode 100644 src/imm/immnd/immnd_common.c
  create mode 100644 src/imm/immnd/immnd_common.h

diff --git a/src/imm/Makefile.am b/src/imm/Makefile.am
index b7e4826..099efda 100644
--- a/src/imm/Makefile.am
+++ b/src/imm/Makefile.am
@@ -163,6 +163,7 @@ noinst_HEADERS += \
src/imm/immnd/immnd.h \
src/imm/immnd/immnd_cb.h \
src/imm/immnd/immnd_init.h \
+   src/imm/immnd/immnd_common.h \
src/imm/immpbed/immpbe.h \
src/imm/tools/imm_dumper.h
  
@@ -332,6 +333,7 @@ bin_osafimmnd_CPPFLAGS = \

$(AM_CPPFLAGS)
  
  bin_osafimmnd_SOURCES = \

+   src/imm/immnd/immnd_common.c \
src/imm/immnd/immnd_amf.c \
src/imm/immnd/immnd_db.c \
src/imm/immnd/immnd_evt.c \
diff --git a/src/imm/agent/imma_om_api.cc b/src/imm/agent/imma_om_api.cc
index 7155799..a06f0ea 100644
--- a/src/imm/agent/imma_om_api.cc
+++ b/src/imm/agent/imma_om_api.cc
@@ -55,7 +55,6 @@ static bool immOmIsLoader = false;
  static const char *sysaClName = SA_IMM_ATTR_CLASS_NAME;
  static const char *sysaAdmName = SA_IMM_ATTR_ADMIN_OWNER_NAME;
  static const char *sysaImplName = SA_IMM_ATTR_IMPLEMENTER_NAME;
-
  static int imma_om_resurrect(IMMA_CB *cb, IMMA_CLIENT_NODE *cl_node,
   bool *locked);
  static SaAisErrorT imma_finalizeCcb(SaImmCcbHandleT ccbHandle,
diff --git a/src/imm/apitest/management/test_saImmOmClassCreate_2.c 
b/src/imm/apitest/management/test_saImmOmClassCreate_2.c
index 3ae4b0f..967b819 100644
--- a/src/imm/apitest/management/test_saImmOmClassCreate_2.c
+++ b/src/imm/apitest/management/test_saImmOmClassCreate_2.c
@@ -426,6 +426,46 @@ void saImmOmClassCreate_2_19(void)
safassert(saImmOmFinalize(immOmHandle), SA_AIS_OK);
  }
  
+/*

+  Verify it is not allowed to create IMM object class with reserved name.
+  NOTE: As the list of reserved class names is read from the environment
+  variable IMMSV_RESERVED_CLASS_NAMES which is defined in immnd.conf file,
+  these 02 below test cases could fail if "objects" or "classes" name do
+  not exist in the list.
+ */
+void saImmOmClassCreate_with_reserved_name_01(void)
+{
+   const SaImmClassNameT className = (SaImmClassNameT) "objects";
+   SaImmAttrDefinitionT_2 attr1 = {"rdn", SA_IMM_ATTR_SANAMET,
+   SA_IMM_ATTR_CONFIG | SA_IMM_ATTR_RDN,
+   

Re: [devel] [PATCH 1/1] cpnd: Correct duration of cpnd_tmr_start in cpnd_proc_update_remote [#2787]

2018-03-06 Thread Hoa Le

Hi Alex,


Yes, the m_CPSV_CONVERT_SATIME_TEN_MILLI_SEC adjustment is used for 
SaTimeT timeout input. But in our case, the timeout duration is set by 
CPND_WAIT_TIME, this timeout is calculated based on the checkpoint 
buffer size. It is not big enough to be adjusted.
I checked all uses of cpnd_tmr_start() but the same issue doesn't appear 
in any other places.


There may be an issue in cpnd_evt_proc_nd2nd_ckpt_sect_exptmr_req() that 
the timeout duration is passed to cpnd_tmr_start() without applying the 
m_CPSV_CONVERT_SATIME_TEN_MILLI_SEC adjustment. But it seems to be out 
of scope for this ticket. I will check if an ticket for it is necessary.


Thank you.


--
Best regards,
Hoa Le

On 03/07/2018 03:34 AM, Alex Jones wrote:


Hi Hoa,


    Ack from me. I was able to reproduce it in UML by setting the 
value to 512.



    But, did you check the other uses of cpnd_tmr_start() to see if 
they have the issue?



    It looks like the correct reason to remove the 
m_CPSV_CONVERT_SATIME_TEN_MILLI_SEC adjustment for your case is 
because it is already set by CPND_WAIT_TIME above. Is that right?



    It looks like the call in cpnd_evt_proc_ckpt_sect_create() or 
cpnd_evt_proc_nd2nd_ckpt_sect_exptmr_req() might have the same 
problem. Can you check?



Alex


On 03/05/2018 09:02 PM, Hoa Le wrote:


NOTICE: This email was received from an EXTERNAL sender


Hi Alex,

This may relate to performance of the testing node.
In my testing environment, the test case failed with "SaSizeT 
large_buffer_size = 1000;" on one specific node, and on other 
nodes, the test case failed only when I set the large_buffer_size to 
2560.

Can you help re-test it with a larger buffer size (i.e. 5120) ?

Attached are traces and logs when running the test case without the 
change in ckptnd.


/<143>1 2018-03-06T08:53:56.72482+07:00 SC-1 osafckptnd 350 
osafckptnd [meta sequenceId="1029"] 350:ckpt/common/cpsv_evt.c:2839 
TR cpnd <<== [1] CPND_EVT_TIME_OUT(type=REPL_RSP_EXPI(3)) from CPD//
//<143>1 2018-03-06T08:53:56.724844+07:00 SC-1 osafckptnd 350 
osafckptnd [meta sequenceId="1030"] 350:ckpt/ckptnd/cpnd_evt.c:4679 
>> cpnd_evt_proc_timer_expiry //
//<143>1 2018-03-06T08:53:56.724867+07:00 SC-1 osafckptnd 350 
osafckptnd [meta sequenceId="1031"] 350:ckpt/ckptnd/cpnd_evt.c:4742 
TR   Before Calling m_NCS_TMR_DESTROY  tmr->ckpt_id 1  tmr->type 3//
//<143>1 2018-03-06T08:53:56.724889+07:00 SC-1 osafckptnd 350 
osafckptnd [meta sequenceId="1032"] 350:ckpt/ckptnd/cpnd_proc.c:1797 
>> cpnd_all_repl_rsp_expiry //
//<143>1 2018-03-06T08:53:56.72491+07:00 SC-1 osafckptnd 350 
osafckptnd [meta sequenceId="1033"] 350:ckpt/ckptnd/cpnd_mds.c:1150 
>> cpnd_mds_send_rsp //
//<143>1 2018-03-06T08:53:56.724932+07:00 SC-1 osafckptnd 350 
osafckptnd [meta sequenceId="1034"] 350:ckpt/common/cpsv_evt.c:2830 
TR cpnd ==>> CPA_EVT_ND2A_CKPT_DATA_RSP(err=5, type=OVWRITE(4)) to 
node 0x2010F/


Thanks.

--
Best regards,
Hoa Le
On 03/06/2018 07:33 AM, Jones, Alex wrote:


Hi Hoa,


  When I run your new test without the change in ckptnd, it passes. 
Shouldn't it fail?



Alex



*From:* Hoa Le 
*Sent:* Thursday, March 1, 2018 2:45:32 AM
*To:* alex.jo...@genband.com; ravisekhar.ko...@oracle.com
*Cc:* opensaf-devel@lists.sourceforge.net; Hoa Le
*Subject:* [PATCH 1/1] cpnd: Correct duration of cpnd_tmr_start in 
cpnd_proc_update_remote [#2787]


NOTICE: This email was received from an EXTERNAL sender


In cpnd_proc_update_remote() function, cpnd_tmr_start is invoked 
with the
timer duration parameter being adjusted by 
m_CPSV_CONVERT_SATIME_TEN_MILLI_SEC.
This duration is 0 in most cases, which will lead to 
SA_AIS_ERR_TIMEOUT error

of checkpoint write action if the checkpoint data is big enough.

This patch corrects the duration of cpnd_tmr_start in 
cpnd_proc_update_remote

and add a new test case (ckpttest 20 11) to verify the correction.
---
src/ckpt/apitest/test_cpa.c | 89 +++
src/ckpt/apitest/test_cpsv.h | 1 +
src/ckpt/apitest/test_cpsv_conf.h | 7 +--
src/ckpt/ckptnd/cpnd_proc.c | 4 +-
4 files changed, 95 insertions(+), 6 deletions(-)

diff --git a/src/ckpt/apitest/test_cpa.c b/src/ckpt/apitest/test_cpa.c
index 792eb27..0cc38a4 100644
--- a/src/ckpt/apitest/test_cpa.c
+++ b/src/ckpt/apitest/test_cpa.c
@@ -320,6 +320,8 @@ void fill_testcase_data()
fill_ckpt_attri(_app,
SA_CKPT_CHECKPOINT_COLLOCATED | SA_CKPT_WR_ALL_REPLICAS,
140, SA_TIME_END, 2, 85, 3);
+ fill_ckpt_attri(_buffer_attrs, SA_CKPT_WR_ALL_REPLICAS,
+ 4096, 100, 2, 5120, 3);

fill_ckpt_name(_replicas_ckpt,
"safCkpt=all_replicas_ckpt,safApp=safCkptService");
@@ 

[devel] [PATCH 1/1] osaf: use local etcd instance only [#2797]

2018-03-06 Thread Gary Lee
---
 src/osaf/consensus/plugins/etcd.plugin | 25 -
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/src/osaf/consensus/plugins/etcd.plugin 
b/src/osaf/consensus/plugins/etcd.plugin
index f8d3c7f25..586059b32 100644
--- a/src/osaf/consensus/plugins/etcd.plugin
+++ b/src/osaf/consensus/plugins/etcd.plugin
@@ -18,6 +18,7 @@
 
 readonly keyname="opensaf_consensus_lock"
 readonly directory="/opensaf/"
+readonly etcd_options="--no-sync"
 readonly etcd_timeout="5s"
 
 # get
@@ -30,7 +31,7 @@ readonly etcd_timeout="5s"
 get() {
   readonly key=$1
 
-  if value=$(etcdctl --timeout $etcd_timeout get "$directory$key" 2>&1)
+  if value=$(etcdctl $etcd_options --timeout $etcd_timeout get 
"$directory$key" 2>&1)
   then
 echo "$value"
 return 0
@@ -51,7 +52,8 @@ setkey() {
   readonly key=$1
   readonly value=$2
 
-  if etcdctl --timeout $etcd_timeout set "$directory$key" "$value" >/dev/null
+  if etcdctl $etcd_options --timeout $etcd_timeout set "$directory$key" \
+"$value" >/dev/null
   then
 return 0
   else
@@ -69,7 +71,8 @@ setkey() {
 erase() {
   readonly key=$1
 
-  if etcdctl --timeout $etcd_timeout rm "$directory$key" >/dev/null 2>&1
+  if etcdctl $etcd_options --timeout $etcd_timeout \
+rm "$directory$key" >/dev/null 2>&1
   then
 return 0
   else
@@ -90,7 +93,8 @@ lock() {
   readonly owner=$1
   readonly timeout=$2
 
-  if etcdctl --timeout $etcd_timeout mk "$directory$keyname" "$owner" \
+  if etcdctl $etcd_options --timeout $etcd_timeout \
+mk "$directory$keyname" "$owner" \
 --ttl "$timeout" >/dev/null 2>&1
   then
 return 0
@@ -101,7 +105,8 @@ lock() {
 # see if we already hold the lock
 if [ "$current_owner" = "$owner" ]; then
   # refresh TTL
-  if etcdctl --timeout $etcd_timeout set "$directory$keyname" "$owner" \
+  if etcdctl $etcd_options --timeout $etcd_timeout \
+set "$directory$keyname" "$owner" \
 --swap-with-value "$owner" --ttl "$timeout" >/dev/null 2>&1
   then
 return 0
@@ -145,14 +150,14 @@ unlock() {
 
   if [ "$forced" = false ]; then
 # unlock only succeeds if owner matches
-if etcdctl --timeout $etcd_timeout rm "$directory$keyname" \
+if etcdctl $etcd_options --timeout $etcd_timeout rm "$directory$keyname" \
   --with-value "$owner" >/dev/null 2>&1
 then
   return 0
 fi
 
 # failed! check we own the lock
-if current_owner=$(etcdctl --timeout $etcd_timeout get \
+if current_owner=$(etcdctl $etcd_options --timeout $etcd_timeout get \
   "$directory$keyname" 2>&1)
 then
   if [ "$owner" != "$current_owner" ]; then
@@ -163,7 +168,8 @@ unlock() {
 return 2
   fi
 
-  if etcdctl --timeout $etcd_timeout rm "$directory$keyname" >/dev/null 2>&1
+  if etcdctl $etcd_options --timeout $etcd_timeout \
+rm "$directory$keyname" >/dev/null 2>&1
   then
 return 0
   else
@@ -181,7 +187,8 @@ unlock() {
 watch() {
   readonly key=$1
 
-  if value=$(etcdctl --timeout $etcd_timeout watch "$directory$key" 2>&1)
+  if value=$(etcdctl $etcd_options --timeout $etcd_timeout \
+watch "$directory$key" 2>&1)
   then
 # if the key is removed, then "PrevNode.Value: " is returned
 echo "$value"
-- 
2.14.1


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


[devel] [PATCH 0/1] Review Request for osaf: use local etcd instance only [#2797]

2018-03-06 Thread Gary Lee
Summary: osaf: use local etcd instance only [#2797]
Review request for Ticket(s): 2797
Peer Reviewer(s): Anders 
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop
Development branch: ticket-2797
Base revision: 364cbdcd98fa56d26fad34ebc9603d4814a526c5
Personal repository: git://git.code.sf.net/u/userid-2226215/review


Impacted area   Impact y/n

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


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

revision 8ba1cf4a10e1756b50ce39beb717f496a2b36040
Author: Gary Lee 
Date:   Wed, 7 Mar 2018 15:11:56 +1100

osaf: use local etcd instance only [#2797]



Complete diffstat:
--
 src/osaf/consensus/plugins/etcd.plugin | 25 -
 1 file changed, 16 insertions(+), 9 deletions(-)


Testing Commands:
-
added --debug to the plugin as an etcdctl option

Testing, Expected Results:
--
ensure the request goes to the local etcd instance

Conditions of Submission:
-
ack, or in one week


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


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


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

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

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

___ Your patches do not have proper short+long header

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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


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


[devel] [PATCH 1/1] osaf: add example config for etcd [#2784]

2018-03-06 Thread Gary Lee
---
 src/osaf/consensus/plugins/etcd.readme | 32 
 1 file changed, 32 insertions(+)
 create mode 100644 src/osaf/consensus/plugins/etcd.readme

diff --git a/src/osaf/consensus/plugins/etcd.readme 
b/src/osaf/consensus/plugins/etcd.readme
new file mode 100644
index 0..bdfa3fa19
--- /dev/null
+++ b/src/osaf/consensus/plugins/etcd.readme
@@ -0,0 +1,32 @@
+Example etcd configuration
+==
+
+This document describes how to install and configure etcd on each
+node of an OpenSAF cluster. Note: it is also possible to run etcd outside
+the OpenSAF cluster, or on a subset of the cluster.
+
+etcd is generally available as a binary package in Linux distributions.
+
+For example, on Ubuntu:
+
+sudo apt-get install etcd
+
+Locate etcd.conf for your distribution. For example, it is located at
+/etc/default/etcd.conf on Ubuntu 17.10.
+
+The configuration below should help you get an initial etcd cluster running
+on a five node cluster.
+
+ETCD_LISTEN_PEER_URLS="http://0.0.0.0:2380;
+ETCD_LISTEN_CLIENT_URLS="http://localhost:2379;
+ETCD_INITIAL_ADVERTISE_PEER_URLS="http://0.0.0.0:2380;
+ETCD_INITIAL_CLUSTER="SC-1=http://x.x.x.x:2380,SC-2=http://x.x.x.x:2380,PL-3=http://x.x.x.x:2380,PL-4=http://x.x.x.x:2380,PL-5=http://x.x.x.x:2380;
+ETCD_INITIAL_CLUSTER_TOKEN="etcd-cluster"
+ETCD_ADVERTISE_CLIENT_URLS="http://localhost:2379;
+
+Replace x.x.x.x with appropriate IP addresses.
+
+A sample etcd v2 plugin is provided. It assumes etcd is running locally.
+
+If you have configured etcd to run elsewhere,
+please add the '--endpoints' option to etcdctl in the plugin.
-- 
2.14.1


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


[devel] [PATCH 0/1] Review Request for osaf: add example config for etcd [#2784]

2018-03-06 Thread Gary Lee
Summary: osaf: add example config for etcd [#2784]
Review request for Ticket(s): 2784
Peer Reviewer(s): Anders, Ravi 
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop
Development branch: ticket-2784
Base revision: 364cbdcd98fa56d26fad34ebc9603d4814a526c5
Personal repository: git://git.code.sf.net/u/userid-2226215/review


Impacted area   Impact y/n

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


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

revision eba49de4b62bb2274fd5bd97cf7395456fa587e3
Author: Gary Lee 
Date:   Wed, 7 Mar 2018 14:24:24 +1100

osaf: add example config for etcd [#2784]



Added Files:

 src/osaf/consensus/plugins/etcd.readme


Complete diffstat:
--
 src/osaf/consensus/plugins/etcd.readme | 32 
 1 file changed, 32 insertions(+)


Testing Commands:
-
*** LIST THE COMMAND LINE TOOLS/STEPS TO TEST YOUR CHANGES ***


Testing, Expected Results:
--
*** PASTE COMMAND OUTPUTS / TEST RESULTS ***


Conditions of Submission:
-
ack from any reviewer, or in one week

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 ~/.gitconfig file (i.e. user.name, user.email etc)

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

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

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


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


[devel] [PATCH 0/1] Review Request for msgd: during cold sync don't add tracking entries which already exist [#2793]

2018-03-06 Thread Alex Jones
Summary: msgd: during cold sync don't add tracking entries which already exist 
[#2793]
Review request for Ticket(s): 2793
Peer Reviewer(s): Srinivas
Pull request to: 
Affected branch(es): develop
Development branch: ticket-2793
Base revision: 5d0175a756c4d7fe47dc8b815725332ca7ca4291
Personal repository: git://git.code.sf.net/u/trguitar/review


Impacted area   Impact y/n

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


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

revision 916b838764c03891c5e35b18626d89aadbb5caf6
Author: Alex Jones 
Date:   Tue, 6 Mar 2018 18:48:49 -0500

msgd: during cold sync don't add tracking entries which already exist [#2793]

Opening of an existing msg q using saMsgQueueOpen (for q failover) may take a
long time.

When cold sync is done, sometimes two MDS cold sync requests are sent by the
standby, so the standby can receive 2 cold syncs. The standby code to process
the cold sync response blindly adds the tracking entries for message queue
groups. If two cold syncs are done, the tracking list can have duplicate
entries. When controllers are rebooted back and forth, this list can get large
(1000s of entries), and if another cluster node is rebooted and a q needs to
move from there, 1000s of duplicate tracking messages are sent by msgd, which
slows down the failover, and saMsgQueueOpen can take a long time.

Fix is to not blindly add tracking entries during cold sync, but only add them
if they are not already there.



Complete diffstat:
--
 src/msg/msgd/mqd_mbcsv.c | 17 +++--
 1 file changed, 3 insertions(+), 14 deletions(-)


Testing Commands:
-
1) create a msg q group
2) create 4 msg qs on different nodes and add them to the group
3) send some messages to the group (to enable tracking)
4) open another message q on a different node
5) reboot the controllers back and forth about 20 or 30 times
6) reboot the node with the message q from (4)
7) open the msg q on another node


Testing, Expected Results:
--
1) step 7 should not take seconds
2) there should not be 1000s of entries in syslog saying "unable to send
   "tracking message"

Conditions of Submission:
-
Mar 12 or ack from developer


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


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


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

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

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

___ Your patches do not have proper short+long header

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

[devel] [PATCH 1/1] msgd: during cold sync don't add tracking entries which already exist [#2793]

2018-03-06 Thread Alex Jones
Opening of an existing msg q using saMsgQueueOpen (for q failover) may take a
long time.

When cold sync is done, sometimes two MDS cold sync requests are sent by the
standby, so the standby can receive 2 cold syncs. The standby code to process
the cold sync response blindly adds the tracking entries for message queue
groups. If two cold syncs are done, the tracking list can have duplicate
entries. When controllers are rebooted back and forth, this list can get large
(1000s of entries), and if another cluster node is rebooted and a q needs to
move from there, 1000s of duplicate tracking messages are sent by msgd, which
slows down the failover, and saMsgQueueOpen can take a long time.

Fix is to not blindly add tracking entries during cold sync, but only add them
if they are not already there.
---
 src/msg/msgd/mqd_mbcsv.c | 17 +++--
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/src/msg/msgd/mqd_mbcsv.c b/src/msg/msgd/mqd_mbcsv.c
index b87b038d9..5b0de15c8 100644
--- a/src/msg/msgd/mqd_mbcsv.c
+++ b/src/msg/msgd/mqd_mbcsv.c
@@ -1057,7 +1057,6 @@ static uint32_t mqd_ckpt_encode_cold_sync_data(MQD_CB 
*pMqd,
MQD_OBJ_NODE *queue_record = 0;
MQD_OBJ_INFO queue_obj_info;
MQD_A2S_MSG cold_sync_data;
-   SaNameT queue_name;
SaNameT queue_index_name;
NCS_PATRICIA_NODE *q_node = 0;
NCS_LOCK *q_rec_lock = >mqd_cb_lock;
@@ -1075,7 +1074,6 @@ static uint32_t mqd_ckpt_encode_cold_sync_data(MQD_CB 
*pMqd,
}
memset(_obj_info, 0, sizeof(MQD_OBJ_INFO));
memset(_sync_data, 0, sizeof(MQD_A2S_MSG));
-   memset(_name, 0, sizeof(SaNameT));
memset(_index_name, 0, sizeof(SaNameT));
 
/*First reserve space to store the number of checkpoints that will be
@@ -1388,7 +1386,6 @@ static uint32_t mqd_a2s_make_record_from_coldsync(MQD_CB 
*pMqd,
 
uint32_t rc = NCSCC_RC_SUCCESS;
MQD_OBJ_NODE *q_obj_node = 0, *q_node = 0;
-   MQD_TRACK_OBJ *q_track_obj = 0;
uint32_t index = 0;
SaNameT record_qindex_name;
MQD_OBJECT_ELEM *pOelm = 0;
@@ -1458,17 +1455,9 @@ static uint32_t mqd_a2s_make_record_from_coldsync(MQD_CB 
*pMqd,
 
/* Filling the track info to the queue database */
for (index = 0; index < q_data_msg.track_cnt; index++) {
-   q_track_obj = m_MMGR_ALLOC_MQD_TRACK_OBJ;
-   if (q_track_obj == NULL) {
-   LOG_CR("%s:%u: ERR_MEMORY: Failed To Allocate Memory",
-  __FILE__, __LINE__);
-   rc = NCSCC_RC_FAILURE;
-   return NCSCC_RC_FAILURE;
-   }
-   memset(q_track_obj, 0, sizeof(MQD_TRACK_OBJ));
-   q_track_obj->dest = q_data_msg.track_info[index].dest;
-   q_track_obj->to_svc = q_data_msg.track_info[index].to_svc;
-   ncs_enqueue(_obj_node->oinfo.tlist, q_track_obj);
+   mqd_track_add(_obj_node->oinfo.tlist,
+   _data_msg.track_info[index].dest,
+   q_data_msg.track_info[index].to_svc);
}
if (new_record)
rc = mqd_db_node_add(pMqd, q_obj_node);
-- 
2.13.6


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


Re: [devel] [PATCH 1/1] cpnd: Correct duration of cpnd_tmr_start in cpnd_proc_update_remote [#2787]

2018-03-06 Thread Alex Jones


signature.asc
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel


Re: [devel] [PATCH 1/1] imm: improve immlist printout for multiple attribute values [#2753]

2018-03-06 Thread Zoran Milinkovic
Hi Vu,

Ack from me.

Thanks,
Zoran

-Original Message-
From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au] 
Sent: den 24 januari 2018 15:47
To: Zoran Milinkovic ; 
ravisekhar.ko...@oracle.com
Cc: opensaf-devel@lists.sourceforge.net; Vu Minh Nguyen 

Subject: [PATCH 1/1] imm: improve immlist printout for multiple attribute 
values [#2753]

Have one space separated among attribute values.
---
 src/imm/tools/imm_list.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/imm/tools/imm_list.c b/src/imm/tools/imm_list.c index 
83c0653..7ccd710 100644
--- a/src/imm/tools/imm_list.c
+++ b/src/imm/tools/imm_list.c
@@ -141,19 +141,19 @@ static void print_attr_value(SaImmValueTypeT 
attrValueType,  {
switch (attrValueType) {
case SA_IMM_ATTR_SAINT32T:
-   printf("%d (0x%x)", *((SaInt32T *)attrValue),
+   printf("%d (0x%x) ", *((SaInt32T *)attrValue),
   *((SaInt32T *)attrValue));
break;
case SA_IMM_ATTR_SAUINT32T:
-   printf("%u (0x%x)", *((SaUint32T *)attrValue),
+   printf("%u (0x%x) ", *((SaUint32T *)attrValue),
   *((SaUint32T *)attrValue));
break;
case SA_IMM_ATTR_SAINT64T:
-   printf("%lld (0x%llx)", *((SaInt64T *)attrValue),
+   printf("%lld (0x%llx) ", *((SaInt64T *)attrValue),
   *((SaInt64T *)attrValue));
break;
case SA_IMM_ATTR_SAUINT64T:
-   printf("%llu (0x%llx)", *((SaUint64T *)attrValue),
+   printf("%llu (0x%llx) ", *((SaUint64T *)attrValue),
   *((SaUint64T *)attrValue));
break;
case SA_IMM_ATTR_SATIMET: {
@@ -163,15 +163,15 @@ static void print_attr_value(SaImmValueTypeT 
attrValueType,
 
ctime_r(, buf);
buf[strlen(buf) - 1] = '\0'; /* Remove new line */
-   printf("%llu (0x%llx, %s)", *((SaTimeT *)attrValue),
+   printf("%llu (0x%llx, %s) ", *((SaTimeT *)attrValue),
   *((SaTimeT *)attrValue), buf);
break;
}
case SA_IMM_ATTR_SAFLOATT:
-   printf("%.8g", *((SaFloatT *)attrValue));
+   printf("%.8g ", *((SaFloatT *)attrValue));
break;
case SA_IMM_ATTR_SADOUBLET:
-   printf("%.17g", *((SaDoubleT *)attrValue));
+   printf("%.17g ", *((SaDoubleT *)attrValue));
break;
case SA_IMM_ATTR_SANAMET: {
SaNameT *myNameT = (SaNameT *)attrValue; @@ -194,12 +194,12 @@ 
static void print_attr_value(SaImmValueTypeT attrValueType,
printf("%x", (int)anyp->bufferAddr[i]);
}
}
-   printf(" size(%u)", (unsigned int)anyp->bufferSize);
+   printf(" size(%u) ", (unsigned int)anyp->bufferSize);
 
break;
}
default:
-   printf("Unknown");
+   printf("Unknown ");
break;
}
 }
--
1.9.1


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


Re: [devel] [PATCH 1/1] imm: not allow creating reserved IMM class names [#2771]

2018-03-06 Thread Zoran Milinkovic
Hi Vu,

There are few things that I have found

1. imma_om_api.cc does not need to be changed.

2.
There is a wrong calculation for a new allocated memory in 
populate_reserved_class_names().
It is:
+   cb->reserved_class_names =
+   (char**)calloc(1, len * sizeof(char*) + 1);
... but it should be...
+   cb->reserved_class_names =
+   (char**)calloc(1, (len + 1) * sizeof(char*));

3.
Both is_regular_name() and is_valid_schema_name() can be written in 
immnd_main.c file where they are used first. Also add function definitions to 
the header immnd_init.h.
There is no need for a new files.
I would also prefix functions with "immnd_"

Ack from me with minor comments.
No need to send the patch for another review

Thanks,
Zoran

-Original Message-
From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au] 
Sent: den 30 januari 2018 15:24
To: Zoran Milinkovic ; 
ravisekhar.ko...@oracle.com
Cc: opensaf-devel@lists.sourceforge.net; Vu Minh Nguyen 

Subject: [PATCH 1/1] imm: not allow creating reserved IMM class names [#2771]

PBE will be restarted and will not be able to come up if user requests
creating IMM object class with same name of reserved ones.

This patch adds code to reject such request with SA_AIS_ERR_INVALID_PARAM.
---
 src/imm/Makefile.am|  2 +
 src/imm/agent/imma_om_api.cc   |  1 -
 .../apitest/management/test_saImmOmClassCreate_2.c | 48 +++
 src/imm/immnd/ImmModel.cc  | 55 +---
 src/imm/immnd/immnd.conf   |  9 ++
 src/imm/immnd/immnd_cb.h   |  1 +
 src/imm/immnd/immnd_common.c   | 75 
 src/imm/immnd/immnd_common.h   | 32 +++
 src/imm/immnd/immnd_evt.c  | 17 
 src/imm/immnd/immnd_main.c | 99 +-
 10 files changed, 285 insertions(+), 54 deletions(-)
 create mode 100644 src/imm/immnd/immnd_common.c
 create mode 100644 src/imm/immnd/immnd_common.h

diff --git a/src/imm/Makefile.am b/src/imm/Makefile.am
index b7e4826..099efda 100644
--- a/src/imm/Makefile.am
+++ b/src/imm/Makefile.am
@@ -163,6 +163,7 @@ noinst_HEADERS += \
src/imm/immnd/immnd.h \
src/imm/immnd/immnd_cb.h \
src/imm/immnd/immnd_init.h \
+   src/imm/immnd/immnd_common.h \
src/imm/immpbed/immpbe.h \
src/imm/tools/imm_dumper.h
 
@@ -332,6 +333,7 @@ bin_osafimmnd_CPPFLAGS = \
$(AM_CPPFLAGS)
 
 bin_osafimmnd_SOURCES = \
+   src/imm/immnd/immnd_common.c \
src/imm/immnd/immnd_amf.c \
src/imm/immnd/immnd_db.c \
src/imm/immnd/immnd_evt.c \
diff --git a/src/imm/agent/imma_om_api.cc b/src/imm/agent/imma_om_api.cc
index 7155799..a06f0ea 100644
--- a/src/imm/agent/imma_om_api.cc
+++ b/src/imm/agent/imma_om_api.cc
@@ -55,7 +55,6 @@ static bool immOmIsLoader = false;
 static const char *sysaClName = SA_IMM_ATTR_CLASS_NAME;
 static const char *sysaAdmName = SA_IMM_ATTR_ADMIN_OWNER_NAME;
 static const char *sysaImplName = SA_IMM_ATTR_IMPLEMENTER_NAME;
-
 static int imma_om_resurrect(IMMA_CB *cb, IMMA_CLIENT_NODE *cl_node,
  bool *locked);
 static SaAisErrorT imma_finalizeCcb(SaImmCcbHandleT ccbHandle,
diff --git a/src/imm/apitest/management/test_saImmOmClassCreate_2.c 
b/src/imm/apitest/management/test_saImmOmClassCreate_2.c
index 3ae4b0f..967b819 100644
--- a/src/imm/apitest/management/test_saImmOmClassCreate_2.c
+++ b/src/imm/apitest/management/test_saImmOmClassCreate_2.c
@@ -426,6 +426,46 @@ void saImmOmClassCreate_2_19(void)
safassert(saImmOmFinalize(immOmHandle), SA_AIS_OK);
 }
 
+/*
+  Verify it is not allowed to create IMM object class with reserved name.
+  NOTE: As the list of reserved class names is read from the environment
+  variable IMMSV_RESERVED_CLASS_NAMES which is defined in immnd.conf file,
+  these 02 below test cases could fail if "objects" or "classes" name do
+  not exist in the list.
+ */
+void saImmOmClassCreate_with_reserved_name_01(void)
+{
+   const SaImmClassNameT className = (SaImmClassNameT) "objects";
+   SaImmAttrDefinitionT_2 attr1 = {"rdn", SA_IMM_ATTR_SANAMET,
+   SA_IMM_ATTR_CONFIG | SA_IMM_ATTR_RDN,
+   NULL};
+   const SaImmAttrDefinitionT_2 *attrDefinitions[] = {, NULL};
+
+   safassert(saImmOmInitialize(, , ),
+ SA_AIS_OK);
+   rc = saImmOmClassCreate_2(immOmHandle, className, SA_IMM_CLASS_CONFIG,
+ attrDefinitions);
+   test_validate(rc, SA_AIS_ERR_INVALID_PARAM);
+   safassert(saImmOmFinalize(immOmHandle), SA_AIS_OK);
+}
+
+void saImmOmClassCreate_with_reserved_name_02(void)
+{
+   const SaImmClassNameT className = (SaImmClassNameT) "classes";
+   

Re: [devel] [PATCH 0/1] Review Request for log: fix log agent may crash after recovery fails [#2670]

2018-03-06 Thread Lennart Lund
Hi Canh

+  // [Canh1] Following the ticket #1396, As I remember that you, Ander, Vu
+  // and me had discussed a lot of about ticket 1396 and we had final 
agreement
+  // for the solution of ticket 1396: the mds connection never shutdown 
after first client
+  // is initialized although this client(last client) is finalized after 
that.
+  // This mean that  when mds connection is initialized successfully,
+  // it will never be shutdown. So ncs_agents_shutdown() should not be 
called here
+  // if we finalize the last client, to keep the mds connection is alive

Yes, you are right.  So Ack.
But please look at the minor comments I had as well

Thanks
Lennart

> -Original Message-
> From: Canh Van Truong [mailto:canh.v.tru...@dektech.com.au]
> Sent: den 6 mars 2018 05:19
> To: Lennart Lund ; Vu Minh Nguyen
> 
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: RE: [PATCH 0/1] Review Request for log: fix log agent may crash
> after recovery fails [#2670]
> 
> Hi Lennart,
> 
> Please see my reply comment [Canh1] in the attachment.
> 
> Thanks
> Canh
> 
> -Original Message-
> From: Lennart Lund [mailto:lennart.l...@ericsson.com]
> Sent: Monday, March 5, 2018 7:40 PM
> To: Canh Van Truong ; Vu Minh Nguyen
> ; Lennart Lund
> 
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: RE: [PATCH 0/1] Review Request for log: fix log agent may crash
> after recovery fails [#2670]
> 
> Hi Canh,
> 
> I have added some new comments. See [Lennart1] in the attached .diff
> Note that this patch contains all comments so I shall be applied alone, not
> on top of the other comment patches
> 
> Thanks
> Lennart
> 
> > -Original Message-
> > From: Canh Van Truong [mailto:canh.v.tru...@dektech.com.au]
> > Sent: den 2 mars 2018 07:29
> > To: Lennart Lund ; Vu Minh Nguyen
> > 
> > Cc: opensaf-devel@lists.sourceforge.net
> > Subject: RE: [PATCH 0/1] Review Request for log: fix log agent may crash
> > after recovery fails [#2670]
> >
> > Hi Lennart,
> >
> > Please see my replied comment in attachment.
> >
> > Thanks
> > Canh
> >
> > -Original Message-
> > From: Lennart Lund [mailto:lennart.l...@ericsson.com]
> > Sent: Thursday, February 22, 2018 5:00 PM
> > To: Canh Van Truong ; Vu Minh Nguyen
> > 
> > Cc: opensaf-devel@lists.sourceforge.net; Canh Van Truong
> > ; Lennart Lund
> > 
> > Subject: RE: [PATCH 0/1] Review Request for log: fix log agent may crash
> > after recovery fails [#2670]
> >
> > Hi Canh
> >
> > I have done a review and have some comments. See attached diff
> >
> > Thanks
> > Lennart
> >
> > > -Original Message-
> > > From: Canh Van Truong [mailto:canh.v.tru...@dektech.com.au]
> > > Sent: den 9 november 2017 04:25
> > > To: Lennart Lund ; Vu Minh Nguyen
> > > 
> > > Cc: opensaf-devel@lists.sourceforge.net; Canh Van Truong
> > > 
> > > Subject: [PATCH 0/1] Review Request for log: fix log agent may crash
> after
> > > recovery fails [#2670]
> > >
> > > Summary: log: fix log agent may crash after recovery fails [#2670]
> > > Review request for Ticket(s): 2670
> > > Peer Reviewer(s): Lennart, Vu
> > > Pull request to: Vu
> > > Affected branch(es): develop, release
> > > Development branch: ticket-2670
> > > Base revision: ce78275348c06f5d69577744f0dab525e79443e7
> > > Personal repository: git://git.code.sf.net/u/canht32/review
> > >
> > > 
> > > Impacted area   Impact y/n
> > > 
> > >  Docsn
> > >  Build systemn
> > >  RPM/packaging   n
> > >  Configuration files n
> > >  Startup scripts n
> > >  SAF servicesy
> > >  OpenSAF servicesn
> > >  Core libraries  n
> > >  Samples n
> > >  Tests   n
> > >  Other   n
> > >
> > >
> > > Comments (indicate scope for each "y" above):
> > > -
> > > *** EXPLAIN/COMMENT THE PATCH SERIES HERE ***
> > >
> > > revision 980226974b1ab266b068eded487e6ce26b9645c0
> > > Author:   Canh Van Truong 
> > > Date: Thu, 9 Nov 2017 10:14:53 +0700
> > >
> > > log: fix log agent may crash after recovery fails [#2670]
> > >
> > > In log api, the client is deleted from the list in the agent after
> > recovery fails.
> > > there is no check if this client is used by other user.
> > >
> > > The patch fix to make sure that the deletion is just processed when it
> is
> > not
> > > being used.
> > >
> > >
> > >
> > > Complete diffstat:
> > > --
> > >