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

2018-02-28 Thread Hoa Le 
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");
@@ -417,6 +419,7 @@ void fill_testcase_data()
   SA_TIME_END);
fill_sec_attri(_attr_with_too_long_id,
   _long_section_id, SA_TIME_END);
+   fill_sec_attri(_buffer_sec, , SA_TIME_END);
 
strcpy(tcd.data1, "This is data1");
strcpy(tcd.data2, "This is data2");
@@ -491,6 +494,8 @@ void fill_testcase_data()
tcd.sec_invalid = 6;
 
fill_ckpt_name(, "none");
+   fill_ckpt_name(_buffer_ckpt,
+   "safCkpt=large_dataBuffer_ckpt,safApp=safCkptService");
 }
 
 void test_cpsv_cleanup(CPSV_CLEANUP_TC_TYPE tc)
@@ -7196,6 +7201,87 @@ final1:
test_validate(result, TEST_PASS);
 }
 
+void cpsv_it_overwrite_13()
+{
+   SaAisErrorT rc;
+   int result, result1;
+
+   SaSizeT large_buffer_size = 2560;
+   char *large_buffer;
+
+   printHead("To verify that overwrite writes into a section with large"
+   " dataBuffer");
+
+   result = test_ckptInitialize(CKPT_INIT_SUCCESS_T, TEST_CONFIG_MODE);
+   if (result != TEST_PASS)
+   goto final1;
+
+   rc = saCkptCheckpointOpen(tcd.ckptHandle, &(tcd.large_buffer_ckpt),
+   &(tcd.large_buffer_attrs), SA_CKPT_CHECKPOINT_CREATE |
+   SA_CKPT_CHECKPOINT_WRITE | SA_CKPT_CHECKPOINT_READ,
+   SA_TIME_ONE_SECOND, _buffer_hdl);
+   result = cpsv_test_result(rc, SA_AIS_OK,
+   "Created large_dataBuffer_ckpt with all flags and"
+   " large maxSectionSize", TEST_CONFIG_MODE);
+   if (result == TEST_PASS)
+   m_TEST_CPSV_PRINTF(" Checkpoint Handle: %llu\n",
+   tcd.large_buffer_hdl);
+   else
+   goto final2;
+
+   rc = saCkptSectionCreate(tcd.large_buffer_hdl, _buffer_sec,
+   , tcd.size);
+   result = cpsv_test_result(rc, SA_AIS_OK, "Created Section id 11",
+   TEST_CONFIG_MODE);
+   if (result != TEST_PASS)
+   goto final3;
+
+   large_buffer = (char *)malloc((large_buffer_size+1)*sizeof(char));
+   if (!large_buffer){
+   m_TEST_CPSV_PRINTF("\nOut of memory\n");
+   result = TEST_FAIL;
+   goto final3;
+   }
+   memset(large_buffer, 'a', large_buffer_size);
+   large_buffer[large_buffer_size] = '\0';
+   rc = saCkptSectionOverwrite(tcd.large_buffer_hdl, ,
+   large_buffer, large_buffer_size);
+   result = cpsv_test_result(rc, SA_AIS_OK,
+   "OverWrite in section 11 with large dataBuffer",
+   TEST_NONCONFIG_MODE);
+   if (rc == SA_AIS_OK)
+   m_TEST_CPSV_PRINTF(" DataSize: %llu\n", large_buffer_size);
+   if (result != TEST_PASS)
+   goto final4;
+
+   rc = saCkptCheckpointRead(tcd.large_buffer_hdl, _read,
+   tcd.nOfE, );
+   result = cpsv_test_result(rc, SA_AIS_OK,
+   "Read from section 11", TEST_CONFIG_MODE);
+   if (result != TEST_PASS)
+   goto final4;
+
+   if (strncmp(large_buffer, tcd.general_read.dataBuffer,
+   tcd.general_read.readSize) != 0)
+   result = TEST_FAIL;
+
+final4:
+   free(large_buffer);
+final3:
+   rc = saCkptCheckpointUnlink(tcd.ckptHandle, _buffer_ckpt);
+   result1 = cpsv_test_result(rc, SA_AIS_OK,
+   "Unlinked large_dataBuffer_ckpt", TEST_CONFIG_MODE);
+   if (result1 != TEST_PASS){
+   m_TEST_CPSV_PRINTF("\n Unlink failed ckpt not cleanedup\n");
+   result

[devel] [PATCH 0/1] Review Request for cpnd: Correct duration of cpnd_tmr_start in cpnd_proc_update_remote V2 [#2787]

2018-02-28 Thread Hoa Le 
Summary: cpnd: Correct duration of cpnd_tmr_start in cpnd_proc_update_remote 
[#2787]
Review request for Ticket(s): 2787
Peer Reviewer(s): Ravi Sekhar, Alex Jones
Pull request to: Ravi Sekhar, Alex Jones
Affected branch(es): develop, release
Development branch: ticket-2787
Base revision: 5629f554686a498f328e0c79fc946379cbcf6967
Personal repository: git://git.code.sf.net/u/xhoalee/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 27b8bea3f108acc3d793c83c8acfe6452d997481
Author: Hoa Le 
Date:   Thu, 1 Mar 2018 14:31:24 +0700

cpnd: Correct duration of cpnd_tmr_start in cpnd_proc_update_remote [#2787]

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.



Complete diffstat:
--
 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(-)


Testing Commands:
-
ckpttest 20 11


Testing, Expected Results:
--
Test case passes


Conditions of Submission:
-
ACK from reviewer


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

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

2018-02-28 Thread srinivas 
---
 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.
 
 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 any) is ignored.
-#export IMMSV_TRACE_FILENAME=osafimmnd
+#export IMMSV_TRACE_PATHNAME=osafimmnd
 
 # The directory where the imm.xml files and persistend backend files are
 # stored. Imm dump files may also be stored here or in a subdirectory.
diff --git a/src/imm/immpbed/immpbe.cc b/src/imm/immpbed/immpbe.cc
index 6e9b933..964086f 100644
--- a/src/imm/immpbed/immpbe.cc
+++ b/src/imm/immpbed/immpbe.cc
@@ -118,7 +118,7 @@ int main(int argc, char* argv[]) {
   const SaImmAdminOperationParamsT_2* params[] = {NULL};
   SaImmAdminOperationParamsT_2** retParams = NULL;
 
-  if ((logPath = getenv("IMMSV_TRACE_FILENAME"))) {
+  if ((logPath = getenv("IMMSV_TRACE_PATHNAME"))) {
 category_mask = 0x; /* TODO: set using -t flag ? */
   } else {
 logPath = defaultLog;
diff --git a/src/imm/tools/imm_dumper.cc b/src/imm/tools/imm_dumper.cc
index 5e5dd00..0365fc7 100644
--- a/src/imm/tools/imm_dumper.cc
+++ b/src/imm/tools/imm_dumper.cc
@@ -123,7 +123,7 @@ int main(int argc, char* argv[]) {
* osaf_extended_name_* before saImmOmInitialize and saImmOiInitialize */
   osaf_extended_name_init();
 
-  if ((logPath = getenv("IMMSV_TRACE_FILENAME"))) {
+  if ((logPath = getenv("IMMSV_TRACE_PATHNAME"))) {
 category_mask = 0x; /* TODO: set using -t flag ? */
   } else {
 logPath = defaultLog;
diff --git a/src/ntf/README b/src/ntf/README
index ce78b10..6dd5173 100644
--- a/src/ntf/README
+++ b/src/ntf/README
@@ -260,17 +260,20 @@ in ntfd.conf (see CONFIGURATION above) and restart the 
cluster.
 
 For fatal errors syslog is used.
 
-To enable traces in the NTF library, export the variable NTFSV_TRACE_FILENAME
+To enable traces in the NTF library, export the variable NTFSV_TRACE_PATHNAME
 with a valid filename before starting the application using the NTF library.
 Traces are always stored in $PKGLOGDIR directory and the directory component
 of the path name (if any) is ignored.
 
 For example:
 
-$ export NTFSV_TRACE_FILENAME=ntf.trace
+$ export NTFSV_TRACE_PATHNAME=ntf.trace
 $ ntfsend
 $ cat $pkglogdir/ntf.trace
 
+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/ntf/ntfd/ntfd.conf b/src/ntf/ntfd/ntfd.conf
index 07d90d5..56bb3d3 100644
--- a/src/ntf/ntfd/ntfd.conf
+++ b/src/ntf/ntfd/ntfd.conf
@@ -22,4 +22,4 @@ export NTFSV_ENV_HEALTHCHECK_KEY="Default"
 # The process will also route trace to the NTF trace-file as define here.
 # Traces are always stored in

[devel] [PATCH 0/1] Review Request for dtm : change trace config var name to _PATHNAME [#2792]

2018-02-28 Thread srinivas 
Summary: dtm: change trace config var name to _PATHNAME [#2792]
Review request for Ticket(s): 2792
Peer Reviewer(s): Anders, Vu Minh 
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop
Development branch: ticket-2792
Base revision: 5629f554686a498f328e0c79fc946379cbcf6967
Personal repository: git://git.code.sf.net/u/sri-mangipudy/review


Impacted area   Impact y/n

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


Comments (indicate scope for each "y" above):
-
change trace config var name to _PATHNAME 

revision b7296760a16ec66515a67aa1c810408a31b49448
Author: srinivas 
Date:   Thu, 1 Mar 2018 12:30:09 +0530

dtm: change trace config var name to _PATHNAME [#2792]



Complete diffstat:
--
 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(-)


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


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


Conditions of Submission:
-
*** HOW MANY DAYS BEFORE PUSHING, CONSENSUS ETC ***


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

Re: [devel] [PATCH 1/1] imm: return correct error code when working on more than 10000 objects [#2359]

2018-02-28 Thread Srinivas Mangipudy 
Thank you Vu, Zoran for your comments.

I will make code changes to return NCSCC_RC_NO_OBJECT from 
immsv_evt_enc_name_list.


Thank you
Srinivas


-Original Message-
From: Zoran Milinkovic [mailto:zoran.milinko...@ericsson.com] 
Sent: Tuesday, February 27, 2018 2:45 PM
To: Vu Minh Nguyen ; srinivas 

Cc: opensaf-devel@lists.sourceforge.net
Subject: RE: [PATCH 1/1] imm: return correct error code when working on more 
than 1 objects [#2359]

Hi,

I agree with Vu. You are mixing NCSCC errors with AIS errors.

Usually, when we introduce some limitation in the library, we must have the 
same limitation on the service side as well, and both checks return the same 
error code.
This is a bit tricky part. The check is on the service side in the decoding 
part which is more part of MDS than IMM.

If it's not possible to fix the service side and return ERR_INAVLID_PARAM or 
ERR_NO_RESOURCE, I would suggest to make an exception in this case and return 
ERR_INVALID_PARAM or ERR_NO_RESOURCE if the problem is caught in the library 
(so that applications does not need to restart) and return ERR_LIBRARY if the 
problem is caught on the service side (which means that some other library or 
earlier IMM library is used).
In both places (the service and the library sides) comments need to be added in 
the code to be visible that we have made an exception in this case.
So, on the service side, only comments need to be added since it already 
returns ERR_LIBRARY.

Thanks,
Zoran

-Original Message-
From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
Sent: den 27 februari 2018 02:27
To: 'srinivas' ; Zoran Milinkovic 

Cc: opensaf-devel@lists.sourceforge.net
Subject: RE: [PATCH 1/1] imm: return correct error code when working on more 
than 1 objects [#2359]

Hi Srinivas,

I see you added new error code type to the function ` immsv_evt_enc_name_list`.
I don't think that is a good idea to mix using 02 different returned error code 
types in one function.
(Actually,  SA_AIS_ERR_NO_RESOURCES(18) value is equal to
NCSCC_RC_NO_OBJECT(18))

And few minors are inline.

Regards, Vu

> -Original Message-
> From: srinivas [mailto:srinivas.mangip...@oracle.com]
> Sent: Tuesday, February 20, 2018 2:31 PM
> To: vu.m.ngu...@dektech.com.au; zoran.milinko...@ericsson.com
> Cc: opensaf-devel@lists.sourceforge.net; srinivas 
> 
> Subject: [PATCH 1/1] imm: return correct error code when working on 
> more than 1 objects [#2359]
> 
> ---
>  src/imm/agent/imma_proc.cc | 10 -- src/imm/common/immsv_evt.c 
> |  2 +-
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/src/imm/agent/imma_proc.cc b/src/imm/agent/imma_proc.cc 
> index 886b50c..af8fb58 100644
> --- a/src/imm/agent/imma_proc.cc
> +++ b/src/imm/agent/imma_proc.cc
> @@ -3543,8 +3543,14 @@ SaAisErrorT imma_evt_fake_evs(IMMA_CB *cb, 
> IMMSV_EVT *i_evt, IMMSV_EVT **o_evt,
>proc_rc = immsv_evt_enc(i_evt, );
> 
>if (proc_rc != NCSCC_RC_SUCCESS) {
> -TRACE_2("ERR_LIBRARY: Failed to pre-pack");
> -rc = SA_AIS_ERR_LIBRARY;
> +if (proc_rc != SA_AIS_ERR_NO_RESOURCES) {
> +  rc = SA_AIS_ERR_LIBRARY;
> +  TRACE_2("ERR_LIBRARY: Failed to pre-pack");
> +}
> +else {
[Vu] `else` statement should be on the same line with previous `{`.
> +  rc = SA_AIS_ERR_NO_RESOURCES;
> +  TRACE_2("ERR_NO_RESOURCES: Failed to pre-pack");
> +}
>  goto fail;
>}
[Vu] Can we simplify above logic by only adding below check after ` 
immsv_evt_enc`?
if (proc_rc == NCSCC_RC_NO_OBJECT) {
  TRACE_2("ERR_NO_RESOURCES: Failed to pre-pack");
  rc = SA_AIS_ERR_NO_RESOURCES;
  goto fail;
}

> 
> diff --git a/src/imm/common/immsv_evt.c b/src/imm/common/immsv_evt.c 
> index 88c5101..aef00d4 100644
> --- a/src/imm/common/immsv_evt.c
> +++ b/src/imm/common/immsv_evt.c
> @@ -775,7 +775,7 @@ static uint32_t immsv_evt_enc_name_list(NCS_UBAID 
> *o_ub, IMMSV_OBJ_NAME_LIST *p)
> 
>   if (objs >= IMMSV_MAX_OBJECTS) {
>   LOG_ER("TOO MANY Object Names line:%u", __LINE__);
> - return NCSCC_RC_OUT_OF_MEM;
> + return SA_AIS_ERR_NO_RESOURCES;
[Vu] I don' think It is a good idea to mix using 02 different returned error 
code types.

>   }
>   return NCSCC_RC_SUCCESS;
>  }
> --
> 2.7.4



--
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] amf: do not dereference null pointer [#2791]

2018-02-28 Thread Gary Lee 
Callers of ava_mds_send() assume *o_msg is not null, if
the return code is NCSCC_RC_SUCCESS.
---
 src/amf/agent/ava_mds.cc | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/amf/agent/ava_mds.cc b/src/amf/agent/ava_mds.cc
index 440885332..cd139365d 100644
--- a/src/amf/agent/ava_mds.cc
+++ b/src/amf/agent/ava_mds.cc
@@ -378,6 +378,10 @@ uint32_t ava_mds_send(AVA_CB *cb, AVSV_NDA_AVA_MSG *i_msg,
   /* retrieve the response */
   *o_msg = (AVSV_NDA_AVA_MSG *)mds_info.info.svc_send.info.sndrsp.o_rsp;
   mds_info.info.svc_send.info.sndrsp.o_rsp = 0;
+  if (*o_msg == nullptr) {
+LOG_ER("No response received");
+rc = NCSCC_RC_FAILURE;
+  }
 }
   } else
 /* just a 'normal' send */
-- 
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 amf: do not dereference null pointer [#2791]

2018-02-28 Thread Gary Lee 
Summary: amf: do not dereference null pointer [#2791]
Review request for Ticket(s): 2791
Peer Reviewer(s): AMF devs
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop
Development branch: ticket-2791
Base revision: 5629f554686a498f328e0c79fc946379cbcf6967
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 91c24d80f69c283d7107cd98fbcc6dffd3c7639e
Author: Gary Lee 
Date:   Thu, 1 Mar 2018 14:27:25 +1100

amf: do not dereference null pointer [#2791]

Callers of ava_mds_send() assume *o_msg is not null, if
the return code is NCSCC_RC_SUCCESS.



Complete diffstat:
--
 src/amf/agent/ava_mds.cc | 4 
 1 file changed, 4 insertions(+)


Testing Commands:
-
Run legacy tests

Testing, Expected Results:
--
Tests pass

Conditions of Submission:
-
*** HOW MANY DAYS BEFORE PUSHING, CONSENSUS ETC ***


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

Re: [devel] [PATCH 1/1] log: Fix cyclic crash when starting standby and OI is not active [#2711]

2018-02-28 Thread Lennart Lund 
Hi Vu

Yes, I am aware of that and was thinking about removing the isRtStream flag but 
decided to make the fix as simple as possible and keep all existing logic. 
However it is probably better to do as you suggest.

Thanks
Lennart

> -Original Message-
> From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> Sent: den 28 februari 2018 08:34
> To: Lennart Lund ; Canh Van Truong
> 
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: RE: [PATCH 1/1] log: Fix cyclic crash when starting standby and OI is
> not active [#2711]
> 
> Hi Lennart,
> 
> Other comment is, isRtStream field is used to distinguish that stream data
> belongs to configurable stream or runtime stream.
> 
> With this patch, you introduce STREAM_TYPE_APPLICATION_RT/_CFG, then I
> think
> that field may be unnecessary.
> 
> Regards, Vu
> 
> > -Original Message-
> > From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> > Sent: Wednesday, February 28, 2018 2:10 PM
> > To: 'Lennart Lund' ;
> > 'canh.v.tru...@dektech.com.au' 
> > Cc: 'opensaf-devel@lists.sourceforge.net'  > de...@lists.sourceforge.net>
> > Subject: RE: [PATCH 1/1] log: Fix cyclic crash when starting standby and
> OI is
> > not active [#2711]
> >
> > Hi Lennart,
> >
> > See my comments inline, with [Vu].
> >
> > Regards, Vu
> >
> > > -Original Message-
> > > From: Lennart Lund [mailto:lennart.l...@ericsson.com]
> > > Sent: Monday, February 26, 2018 8:40 PM
> > > To: vu.m.ngu...@dektech.com.au; canh.v.tru...@dektech.com.au
> > > Cc: opensaf-devel@lists.sourceforge.net; Lennart Lund
> > > 
> > > Subject: [PATCH 1/1] log: Fix cyclic crash when starting standby and OI
> is not
> > > active [#2711]
> > >
> > > Fix cyclic reboot caused by reading an IMM RT object when the OI is
> down
> > > ---
> > >  src/log/apitest/logtest.c  |  2 --
> > >  src/log/logd/lgs_config.cc |  2 +-
> > >  src/log/logd/lgs_evt.cc| 13 ++---
> > >  src/log/logd/lgs_fmt.h |  3 ++-
> > >  src/log/logd/lgs_imm.cc| 13 +++--
> > >  src/log/logd/lgs_mbcsv.cc  | 18 --
> > >  src/log/logd/lgs_mbcsv.h   |  3 ++-
> > >  src/log/logd/lgs_recov.cc  |  6 --
> > >  src/log/logd/lgs_stream.cc | 28 +++-
> > >  src/log/logd/lgs_stream.h  |  2 +-
> > >  src/osaf/immutil/immutil.h |  4 +++-
> > >  11 files changed, 41 insertions(+), 53 deletions(-)
> > >
> > > diff --git a/src/log/apitest/logtest.c b/src/log/apitest/logtest.c
> > > index afa1fcf57..f8fe135cb 100644
> > > --- a/src/log/apitest/logtest.c
> > > +++ b/src/log/apitest/logtest.c
> > > @@ -33,8 +33,6 @@
> > >  #include "base/osaf_extended_name.h"
> > >  #include 
> > >
> > > -#define LLDTEST
> > > -
> > >  SaNameT systemStreamName;
> > >  SaNameT alarmStreamName;
> > >  SaNameT globalConfig;
> > > diff --git a/src/log/logd/lgs_config.cc b/src/log/logd/lgs_config.cc
> > > index a70a2f6c6..4190e3048 100644
> > > --- a/src/log/logd/lgs_config.cc
> > > +++ b/src/log/logd/lgs_config.cc
> > > @@ -586,7 +586,7 @@ int lgs_cfg_verify_log_file_format(const char
> > > *log_file_format) {
> > >SaBoolT dummy;
> > >
> > >if (!lgs_is_valid_format_expression((const SaStringT)log_file_format,
> > > -  STREAM_TYPE_APPLICATION, ))
> {
> > > +  STREAM_TYPE_APPLICATION_RT,
> )) {
> > >  LOG_NO("logStreamFileFormat has invalid value = %s",
> log_file_format);
> > >  rc = -1;
> > >}
> > > diff --git a/src/log/logd/lgs_evt.cc b/src/log/logd/lgs_evt.cc
> > > index b8840b436..4b735875d 100644
> > > --- a/src/log/logd/lgs_evt.cc
> > > +++ b/src/log/logd/lgs_evt.cc
> > > @@ -877,7 +877,7 @@ SaAisErrorT
> > > create_new_app_stream(lgsv_stream_open_req_t *open_sync_param,
> > >
> > >/* Check the format string */
> > >if (!lgs_is_valid_format_expression(open_sync_param->logFileFmt,
> > > -  STREAM_TYPE_APPLICATION,
> > > +  STREAM_TYPE_APPLICATION_RT,
> > >)) {
> > >  TRACE("format expression failure");
> > >  rc = SA_AIS_ERR_INVALID_PARAM;
> > > @@ -947,16 +947,15 @@ SaAisErrorT
> > > create_new_app_stream(lgsv_stream_open_req_t *open_sync_param,
> > >open_sync_param->logFileName, open_sync_param-
> >logFilePathName,
> > >open_sync_param->maxLogFileSize, open_sync_param-
> > > >maxLogRecordSize,
> > >open_sync_param->logFileFullAction, open_sync_param-
> > > >maxFilesRotated,
> > > -  open_sync_param->logFileFmt, STREAM_TYPE_APPLICATION,
> > > twelveHourModeFlag,
> > > -  0,
> > > -  *o_stream);  // output
> > > +  open_sync_param->logFileFmt, STREAM_TYPE_APPLICATION_RT,
> > > +  twelveHourModeFlag, 0, *o_stream);  // output
> > >if (err == -1) {
> > >  log_stream_delete(o_stream);
> > >

Re: [devel] [PATCH 1/1] log: Fix cyclic crash when starting standby and OI is not active [#2711]

2018-02-28 Thread Lennart Lund 


> -Original Message-
> From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> Sent: den 28 februari 2018 08:10
> To: Lennart Lund ; Canh Van Truong
> 
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: RE: [PATCH 1/1] log: Fix cyclic crash when starting standby and OI is
> not active [#2711]
> 
> Hi Lennart,
> 
> See my comments inline, with [Vu].
> 
> Regards, Vu
> 
> > -Original Message-
> > From: Lennart Lund [mailto:lennart.l...@ericsson.com]
> > Sent: Monday, February 26, 2018 8:40 PM
> > To: vu.m.ngu...@dektech.com.au; canh.v.tru...@dektech.com.au
> > Cc: opensaf-devel@lists.sourceforge.net; Lennart Lund
> > 
> > Subject: [PATCH 1/1] log: Fix cyclic crash when starting standby and OI is
> not
> > active [#2711]
> >
> > Fix cyclic reboot caused by reading an IMM RT object when the OI is down
> > ---
> >  src/log/apitest/logtest.c  |  2 --
> >  src/log/logd/lgs_config.cc |  2 +-
> >  src/log/logd/lgs_evt.cc| 13 ++---
> >  src/log/logd/lgs_fmt.h |  3 ++-
> >  src/log/logd/lgs_imm.cc| 13 +++--
> >  src/log/logd/lgs_mbcsv.cc  | 18 --
> >  src/log/logd/lgs_mbcsv.h   |  3 ++-
> >  src/log/logd/lgs_recov.cc  |  6 --
> >  src/log/logd/lgs_stream.cc | 28 +++-
> >  src/log/logd/lgs_stream.h  |  2 +-
> >  src/osaf/immutil/immutil.h |  4 +++-
> >  11 files changed, 41 insertions(+), 53 deletions(-)
> >
> > diff --git a/src/log/apitest/logtest.c b/src/log/apitest/logtest.c
> > index afa1fcf57..f8fe135cb 100644
> > --- a/src/log/apitest/logtest.c
> > +++ b/src/log/apitest/logtest.c
> > @@ -33,8 +33,6 @@
> >  #include "base/osaf_extended_name.h"
> >  #include 
> >
> > -#define LLDTEST
> > -
> >  SaNameT systemStreamName;
> >  SaNameT alarmStreamName;
> >  SaNameT globalConfig;
> > diff --git a/src/log/logd/lgs_config.cc b/src/log/logd/lgs_config.cc
> > index a70a2f6c6..4190e3048 100644
> > --- a/src/log/logd/lgs_config.cc
> > +++ b/src/log/logd/lgs_config.cc
> > @@ -586,7 +586,7 @@ int lgs_cfg_verify_log_file_format(const char
> > *log_file_format) {
> >SaBoolT dummy;
> >
> >if (!lgs_is_valid_format_expression((const SaStringT)log_file_format,
> > -  STREAM_TYPE_APPLICATION, )) {
> > +  STREAM_TYPE_APPLICATION_RT,
> )) {
> >  LOG_NO("logStreamFileFormat has invalid value = %s",
> log_file_format);
> >  rc = -1;
> >}
> > diff --git a/src/log/logd/lgs_evt.cc b/src/log/logd/lgs_evt.cc
> > index b8840b436..4b735875d 100644
> > --- a/src/log/logd/lgs_evt.cc
> > +++ b/src/log/logd/lgs_evt.cc
> > @@ -877,7 +877,7 @@ SaAisErrorT
> > create_new_app_stream(lgsv_stream_open_req_t *open_sync_param,
> >
> >/* Check the format string */
> >if (!lgs_is_valid_format_expression(open_sync_param->logFileFmt,
> > -  STREAM_TYPE_APPLICATION,
> > +  STREAM_TYPE_APPLICATION_RT,
> >)) {
> >  TRACE("format expression failure");
> >  rc = SA_AIS_ERR_INVALID_PARAM;
> > @@ -947,16 +947,15 @@ SaAisErrorT
> > create_new_app_stream(lgsv_stream_open_req_t *open_sync_param,
> >open_sync_param->logFileName, open_sync_param-
> >logFilePathName,
> >open_sync_param->maxLogFileSize, open_sync_param-
> > >maxLogRecordSize,
> >open_sync_param->logFileFullAction, open_sync_param-
> > >maxFilesRotated,
> > -  open_sync_param->logFileFmt, STREAM_TYPE_APPLICATION,
> > twelveHourModeFlag,
> > -  0,
> > -  *o_stream);  // output
> > +  open_sync_param->logFileFmt, STREAM_TYPE_APPLICATION_RT,
> > +  twelveHourModeFlag, 0, *o_stream);  // output
> >if (err == -1) {
> >  log_stream_delete(o_stream);
> >  rc = SA_AIS_ERR_NO_MEMORY;
> >  goto done;
> >}
> >
> > -  rc = lgs_create_rt_appstream(*o_stream);
> > +  rc = lgs_create_appstream_rt_object(*o_stream);
> >if (rc != SA_AIS_OK) log_stream_delete(o_stream);
> >
> >  done:
> > @@ -1055,7 +1054,7 @@ static uint32_t proc_stream_open_msg(lgs_cb_t
> > *cb, lgsv_lgs_evt_t *evt) {
> >logStream = log_stream_get_by_name(name);
> >if (logStream != nullptr) {
> >  TRACE("existing stream - id %u", logStream->streamId);
> > -if (logStream->streamType == STREAM_TYPE_APPLICATION) {
> > +if (logStream->streamType == STREAM_TYPE_APPLICATION_RT) {
> >/* Verify the creation attributes for an existing appl. stream */
> >if (open_sync_param->lstr_open_flags & SA_LOG_STREAM_CREATE) {
> >  ais_rv = file_attribute_cmp(open_sync_param, logStream);
> > @@ -1218,7 +1217,7 @@ static uint32_t proc_stream_close_msg(lgs_cb_t
> > *cb, lgsv_lgs_evt_t *evt) {
> >
> >// This check is to avoid the client getting SA_AIS_BAD_OPERATION
> >// as there is no IMM OI implementer set.
> > -  if ((stream->streamType == STREAM_TYPE_APPLICATION) &&