Re: [devel] [PATCH 1/1] log: Delete the older file without closing current log file via admin operation [#3046]

[Vu Minh Nguyen]

Hi Canh,

Ack with minor comments.

Regards, Vu

> -Original Message-
> From: Canh Van Truong 
> Sent: Friday, June 14, 2019 10:21 PM
> To: lennart.l...@ericsson.com; vu.m.ngu...@dektech.com.au
> Cc: opensaf-devel@lists.sourceforge.net; Canh Van Truong
> 
> Subject: [PATCH 1/1] log: Delete the older file without closing current
log file
> via admin operation [#3046]
> 
> Improve the enhancement of ticket #3022, the user can rotate (delete)
older
> log
> files without closing current log file.
> 
> LOG add new parameter (numberOfFilesToRemove) to admin op command
> with id = 2.
>  - If parameter exists, LOGD remove number of older file base on parameter
> value
>  - If parameter is omitted, LOGD rotates log file normally. (closing
current log
> file, rotate log file if needed, create new log file)
> 
> This helps to avoid creating empty log file or small log file size
> ---
>  src/log/apitest/tet_LogOiOps.c | 272 +
> 
>  src/log/logd/lgs_filehdl.cc|   4 +-
>  src/log/logd/lgs_imm.cc| 104 
>  src/log/logd/lgs_stream.cc |  41 +--
>  src/log/logd/lgs_stream.h  |   3 +-
>  5 files changed, 305 insertions(+), 119 deletions(-)
> 
> diff --git a/src/log/apitest/tet_LogOiOps.c
b/src/log/apitest/tet_LogOiOps.c
> index 947e79664..25f53d3a2 100644
> --- a/src/log/apitest/tet_LogOiOps.c
> +++ b/src/log/apitest/tet_LogOiOps.c
> @@ -5521,95 +5521,201 @@ void admin_rotate_log_file(void)
>   }
>  }
> 
> +// Verify that log and cfg files are rotated when the number of log/cfg
files
> +// reach the "saLogStreamMaxFilesRotated" value (ticket #3045)
>  //
> -// Test case for verifying the admin operation rotates log file with
> -// parameter fails
> -// Steps:
> -// 1. Create configuration app stream
> -// 2. Verify command admin operation rotates log file with parameter
fails
> -// 3. Delete configuration app stream
> +// Step1: Create and delete the app stream with
> (saLogStreamMaxFilesRotated = 2)
> +// 4 times
> +// Step2: Verify that total is 4 cfg and log files (2 log files and 2 cfg
files)
> +void verRotatedLogCfgFile(void)
> +{
> + char command1[MAX_DATA], command2[MAX_DATA];
> + const char *object_dn =
> + "safLgStrCfg=verRotatedFile,safApp=safLogService";
> + const int max_file = 2;
> + char num_files_c[10];
> + uint32_t num_files = 0;
> +
> + // Command to create configuration application stream
> + sprintf(command1, "immcfg -c SaLogStreamConfig %s "
> +   "-a saLogStreamPathName=. "
> +   "-a saLogStreamFileName=verRotatedFile"
> +   " -a saLogStreamMaxFilesRotated=%d",
> +   object_dn, max_file);
> + // Command to delete configuration application stream
> + sprintf(command2, "immcfg -d %s", object_dn);
> +
> + // Create and delete the app stream 4 times.
> + // Log/cfg are created during creating stream.
> + for (int i = 0; i < max_file + 1; ++i) {
> + int rc = systemCall(command1);
> + if (rc == 0) {
> + osaf_nanosleep();
> + rc = systemCall(command2);
> + osaf_nanosleep();
> + }
> + if (rc != 0) {
> + rc_validate(rc, 0);
> + return;
> + }
> + }
> + // Command to count number of log/cfg files on disk
> + sprintf(command1, "find %s -type f -mmin -1 "
> +   "| egrep '%s.*\\.[log$\\|cfg$]' "
> +   "| wc -l | awk '{printf $1}'",
> +   log_root_path, "verRotatedFile");
> +
> + FILE *fp = popen(command1, "r");
> +
> + // Get number of cfg and log file
> + while (fgets(num_files_c, sizeof(num_files_c) - 1, fp) != NULL) {};
> + pclose(fp);
> +
> + // Verify cfg/log files are rotated by checking
> + // that there are totally 4 cfg and log files
> + num_files = atoi(num_files_c);
> + rc_validate(num_files, max_file * 2);
> +}
> +
> +// Verify that log file is rotated via admin operation with admin id = 2
> +// and no parameter
>  //
> -void admin_rotate_log_file_with_param(void)
> +// Step1: Create and delete the app stream with
> (saLogStreamMaxFilesRotated = 2)
> +// step2: Do admin operation 2 times to rotate log file.
> +//Two new log file are created
> +// step3: Verify log file is rotated via admin operation by checking that
> +//there are 2 log files on disk
> +void verRotatedLogCfgFile2(void)
>  {
> - int rc = 0;
> + int rc;
>   char command[MAX_DATA];
>   const char *object_dn =
> -
>   "safLgStrCfg=admin_rotate_file1,safApp=safLogService";
> + "safLgStrCfg=verRotatedFile2,safApp=safLogService";
> + char num_files_c[10];
> + uint32_t num_files = 0;
> 
> - // Create configuration application stream
> - sprintf(command, "immcfg -c SaLogStreamConfig 

Re: [devel] [PATCH 1/1] log: Delete the older file without closing current log file via admin operation [#3046]

[Lennart Lund]

Hi Canh,

Ack with minor comments. See [Lennart] below.

Note: It is very important to also update the "OpenSAF_LOGSv_PR.odt" document 
with the new admin operation

Thanks
Lennart


Från: Canh Van Truong 
Skickat: den 14 juni 2019 14:20
Till: Lennart Lund; Vu Minh Nguyen
Kopia: opensaf-devel@lists.sourceforge.net; Canh Van Truong
Ämne: [PATCH 1/1] log: Delete the older file without closing current log file 
via admin operation [#3046]

Improve the enhancement of ticket #3022, the user can rotate (delete) older log
files without closing current log file.

LOG add new parameter (numberOfFilesToRemove) to admin op command with id = 2.
 - If parameter exists, LOGD remove number of older file base on parameter value
 - If parameter is omitted, LOGD rotates log file normally. (closing current log
file, rotate log file if needed, create new log file)

This helps to avoid creating empty log file or small log file size
---
 src/log/apitest/tet_LogOiOps.c | 272 +
 src/log/logd/lgs_filehdl.cc|   4 +-
 src/log/logd/lgs_imm.cc| 104 
 src/log/logd/lgs_stream.cc |  41 +--
 src/log/logd/lgs_stream.h  |   3 +-
 5 files changed, 305 insertions(+), 119 deletions(-)

diff --git a/src/log/apitest/tet_LogOiOps.c b/src/log/apitest/tet_LogOiOps.c
index 947e79664..25f53d3a2 100644
--- a/src/log/apitest/tet_LogOiOps.c
+++ b/src/log/apitest/tet_LogOiOps.c
@@ -5521,95 +5521,201 @@ void admin_rotate_log_file(void)
 }
 }

+// Verify that log and cfg files are rotated when the number of log/cfg files
+// reach the "saLogStreamMaxFilesRotated" value (ticket #3045)
 //
-// Test case for verifying the admin operation rotates log file with
-// parameter fails
-// Steps:
-// 1. Create configuration app stream
-// 2. Verify command admin operation rotates log file with parameter fails
-// 3. Delete configuration app stream
+// Step1: Create and delete the app stream with (saLogStreamMaxFilesRotated = 
2)
+// 4 times
+// Step2: Verify that total is 4 cfg and log files (2 log files and 2 cfg 
files)
+void verRotatedLogCfgFile(void)
+{
+   char command1[MAX_DATA], command2[MAX_DATA];
+   const char *object_dn =
+   "safLgStrCfg=verRotatedFile,safApp=safLogService";
+   const int max_file = 2;
+   char num_files_c[10];
+   uint32_t num_files = 0;
+
+   // Command to create configuration application stream
+   sprintf(command1, "immcfg -c SaLogStreamConfig %s "
+ "-a saLogStreamPathName=. "
+ "-a saLogStreamFileName=verRotatedFile"
+ " -a saLogStreamMaxFilesRotated=%d",
+ object_dn, max_file);
+   // Command to delete configuration application stream
+   sprintf(command2, "immcfg -d %s", object_dn);
+
+   // Create and delete the app stream 4 times.
+   // Log/cfg are created during creating stream.
+   for (int i = 0; i < max_file + 1; ++i) {
+   int rc = systemCall(command1);
+   if (rc == 0) {
+   osaf_nanosleep();
+   rc = systemCall(command2);
+   osaf_nanosleep();
+   }
+   if (rc != 0) {
+   rc_validate(rc, 0);
+   return;
+   }
+   }
+   // Command to count number of log/cfg files on disk
+   sprintf(command1, "find %s -type f -mmin -1 "
+ "| egrep '%s.*\\.[log$\\|cfg$]' "
+ "| wc -l | awk '{printf $1}'",
+ log_root_path, "verRotatedFile");
+
+   FILE *fp = popen(command1, "r");
+
+   // Get number of cfg and log file
+   while (fgets(num_files_c, sizeof(num_files_c) - 1, fp) != NULL) {};
+   pclose(fp);
+
+   // Verify cfg/log files are rotated by checking
+   // that there are totally 4 cfg and log files
+   num_files = atoi(num_files_c);
+   rc_validate(num_files, max_file * 2);
+}
+
+// Verify that log file is rotated via admin operation with admin id = 2
+// and no parameter
 //
-void admin_rotate_log_file_with_param(void)
+// Step1: Create and delete the app stream with (saLogStreamMaxFilesRotated = 
2)
+// step2: Do admin operation 2 times to rotate log file.
+//Two new log file are created
+// step3: Verify log file is rotated via admin operation by checking that
+//there are 2 log files on disk
+void verRotatedLogCfgFile2(void)
 {
-   int rc = 0;
+   int rc;
 char command[MAX_DATA];
 const char *object_dn =
-   "safLgStrCfg=admin_rotate_file1,safApp=safLogService";
+   "safLgStrCfg=verRotatedFile2,safApp=safLogService";
+   char num_files_c[10];
+   uint32_t num_files = 0;

-   // Create configuration application stream
-   sprintf(command, "immcfg -c SaLogStreamConfig %s"
-

Re: [devel] [PATCH 1/1] log: Delete the older file without closing current log file via admin operation [#3046]

[Lennart Lund]

Hi Canh

I have written some new comments below.
I have also taken a quick look at the patch you attached. It seems to contain 
changes according to the discussion below and it seems to be correct. I have 
not tested but the test cases seems to cover what's needed to test.

A detail that I have seen is that many function headers (comment documenting 
function api) are placed in the wrong file. For globally used functions this 
information shall be placed in the file that contains the interface not in the 
source code file. In case of C or C++ code this is the .h file is the interface 
file.
For local functions (static) this documentation shall be placed with the source 
code.
However, this is not to be seen as a remark for this review so you don't have 
to take any actions to fix this.

Thanks
Lennart

From: Canh Van Truong 
Sent: den 14 juni 2019 06:56
To: Lennart Lund ; Vu Minh Nguyen 

Cc: opensaf-devel@lists.sourceforge.net
Subject: RE: [PATCH 1/1] log: Delete the older file without closing current log 
file via admin operation [#3046]

Thanks Lennart

Please see my reply comments and the updated patch in attachment

Thanks
Canh
From: Lennart Lund mailto:lennart.l...@ericsson.com>>
Sent: Thursday, June 13, 2019 11:02 PM
To: Canh Van Truong 
mailto:canh.v.tru...@dektech.com.au>>; Vu Minh 
Nguyen mailto:vu.m.ngu...@dektech.com.au>>
Cc: 
opensaf-devel@lists.sourceforge.net
Subject: Sv: [PATCH 1/1] log: Delete the older file without closing current log 
file via admin operation [#3046]

Hi Canh

Comments:

  *   The name of function lgs_admin_rotate_log_file() is misleading. If 
numberOfFilesToRemove is given no file rotation is done.
  *   Also the name of function rotate_if_needed() is misleading. This function 
can also be used to just remove the oldest file without rotation.
[Canh]  I agree that 2 function may be misleading when new feature is added. 
The function name should be changed name.

Both function have two unrelated functions which is not correct. This is why it 
will be hard to give them correct descriptive names.

A solution could be:

  *   Create a function for checking the numberOfFilesToRemove parameter.
If there is no parameter the function can return 0 otherwise the number of 
files to remove. In case of parameter error -1.
Could for example be named get_number_of_files_to_remove()
[Canh] Agree

  *   Keep the original  rotate_if_needed() function and create a new function 
for removing the oldest log files. The new function shall of course be placed 
in the same file as rotate_if_needed()
Name example: remove_oldest_log_files(files_to_remove)
[Canh] rotate_if_needed() just remove the older log/cfg files. I don't think we 
should create new function as remove_oldest_log_files(). Almost the code is the 
same.
We can rename the function rotate_if_needed()  as remove_oldest_log_files(int 
files_to_remove = 0). The argument of function can be optional.
If it files_to_remove = 0, the function remove the oldest file until there are 
"maxFilesRotated" - 1 files left on disk"

(Lennart) I have looked at the rotate_if_needed() function and can see that I 
was misled by the misleading name! This function seems to do exactly what I was 
thinking the remove_oldest_log_files(files_to_remove) should do so just 
renaming should be ok.
I am also ok with allowing 0 as a value for files_to_remove as you suggest. 
However, this is a border case. This function is doing two unrelated things 
which are removing the given number of files from the "rotation buffer" and 
removing files that for some reason exist outside of the "rotation buffer". Is 
there any reason to just do a cleanup? I assume you want to do a cleanup if 
needed also if the user requests a file rotation (using no parameter) and in 
this case call this function with files_to_remove = 0. Will this "cleaning" be 
done for any other reason?

  *   Now the lgs_admin_rotate_log_file() is no longer needed. Instead create a 
new function called something like lgs_admin_log_files() or place the logic 
directly in the callback function.

  *   The lgs_admin_log_files() only has to call 
get_number_of_files_to_remove() and if it returns 0 function rotate_if_needed() 
is called, if > 0 function remove_oldest_log_files() is called or if -1 is 
returned do some error handling (if needed). Note the 
get_number_of_files_to_remove() should handle the error reporting to the user 
(calling report_om_error())
Thanks
Lennart


Från: Canh Van Truong 
mailto:canh.v.tru...@dektech.com.au>>
Skickat: den 13 juni 2019 13:57
Till: Lennart Lund; Vu Minh Nguyen
Kopia: 
opensaf-devel@lists.sourceforge.net;
 Canh Van Truong
Ämne: [PATCH 1/1] log: Delete the older file without closing current log file 
via admin operation [#3046]

Improve the enhancement of ticket #3022, the user can rotate (delete) older log
files without closing current log file.

LOG 

Re: [devel] [PATCH 1/1] log: Delete the older file without closing current log file via admin operation [#3046]

[Lennart Lund]

Hi Canh

I think you can send the new version for review

Thanks
Lennart

From: Canh Van Truong 
Sent: den 14 juni 2019 13:31
To: Lennart Lund ; Vu Minh Nguyen 

Cc: opensaf-devel@lists.sourceforge.net
Subject: RE: [PATCH 1/1] log: Delete the older file without closing current log 
file via admin operation [#3046]

Hi Lennart

(Lennart) I have looked at the rotate_if_needed() function and can see that I 
was misled by the misleading name! This function seems to do exactly what I was 
thinking the remove_oldest_log_files(files_to_remove) should do so just 
renaming should be ok.
I am also ok with allowing 0 as a value for files_to_remove as you suggest. 
However, this is a border case. This function is doing two unrelated things 
which are removing the given number of files from the "rotation buffer" and 
removing files that for some reason exist outside of the "rotation buffer". Is 
there any reason to just do a cleanup? I assume you want to do a cleanup if 
needed also if the user requests a file rotation (using no parameter) and in 
this case call this function with files_to_remove = 0. Will this "cleaning" be 
done for any other reason?

Yep. This function just remove the oldest log files for 2 reason. One is that 
when the number of log files is greater than "rotation buffer", the oldest log 
files is removed until number of log file is "  - 1". 
Another is that remove the oldest log files that is requested from user.

In case the user requests a file rotation, it ( log_rotation_act () )will close 
the current log file first then call this function remove_oldest_log_files() 
with default argument = 0. Look at the function remove_oldest_log_files() also 
called in some places.

I just do it in one function to avoid the redundant code when there is too much 
the same code if it is in two functions.
If it is ok, I will send the new version again to community for review.

Thanks
Canh

From: Lennart Lund mailto:lennart.l...@ericsson.com>>
Sent: Friday, June 14, 2019 5:57 PM
To: Canh Van Truong 
mailto:canh.v.tru...@dektech.com.au>>; Vu Minh 
Nguyen mailto:vu.m.ngu...@dektech.com.au>>
Cc: 
opensaf-devel@lists.sourceforge.net
Subject: RE: [PATCH 1/1] log: Delete the older file without closing current log 
file via admin operation [#3046]

Hi Canh

I have written some new comments below.
I have also taken a quick look at the patch you attached. It seems to contain 
changes according to the discussion below and it seems to be correct. I have 
not tested but the test cases seems to cover what's needed to test.

A detail that I have seen is that many function headers (comment documenting 
function api) are placed in the wrong file. For globally used functions this 
information shall be placed in the file that contains the interface not in the 
source code file. In case of C or C++ code this is the .h file is the interface 
file.
For local functions (static) this documentation shall be placed with the source 
code.
However, this is not to be seen as a remark for this review so you don't have 
to take any actions to fix this.

Thanks
Lennart

From: Canh Van Truong 
mailto:canh.v.tru...@dektech.com.au>>
Sent: den 14 juni 2019 06:56
To: Lennart Lund mailto:lennart.l...@ericsson.com>>; 
Vu Minh Nguyen mailto:vu.m.ngu...@dektech.com.au>>
Cc: 
opensaf-devel@lists.sourceforge.net
Subject: RE: [PATCH 1/1] log: Delete the older file without closing current log 
file via admin operation [#3046]

Thanks Lennart

Please see my reply comments and the updated patch in attachment

Thanks
Canh
From: Lennart Lund mailto:lennart.l...@ericsson.com>>
Sent: Thursday, June 13, 2019 11:02 PM
To: Canh Van Truong 
mailto:canh.v.tru...@dektech.com.au>>; Vu Minh 
Nguyen mailto:vu.m.ngu...@dektech.com.au>>
Cc: 
opensaf-devel@lists.sourceforge.net
Subject: Sv: [PATCH 1/1] log: Delete the older file without closing current log 
file via admin operation [#3046]

Hi Canh

Comments:

  *   The name of function lgs_admin_rotate_log_file() is misleading. If 
numberOfFilesToRemove is given no file rotation is done.
  *   Also the name of function rotate_if_needed() is misleading. This function 
can also be used to just remove the oldest file without rotation.
[Canh]  I agree that 2 function may be misleading when new feature is added. 
The function name should be changed name.

Both function have two unrelated functions which is not correct. This is why it 
will be hard to give them correct descriptive names.

A solution could be:

  *   Create a function for checking the numberOfFilesToRemove parameter.
If there is no parameter the function can return 0 otherwise the number of 
files to remove. In case of parameter error -1.
Could for example be named get_number_of_files_to_remove()
[Canh] Agree

  *   Keep the original  rotate_if_needed() function and create a new function 
for removing the oldest log files. The new 

Re: [devel] [PATCH 1/1] log: Delete the older file without closing current log file via admin operation [#3046]

[Canh Van Truong]

Thanks Lennart

 

Please see my reply comments and the updated patch in attachment

 

Thanks

Canh

From: Lennart Lund  
Sent: Thursday, June 13, 2019 11:02 PM
To: Canh Van Truong ; Vu Minh Nguyen

Cc: opensaf-devel@lists.sourceforge.net
Subject: Sv: [PATCH 1/1] log: Delete the older file without closing current
log file via admin operation [#3046]

 

Hi Canh

 

Comments:

*   The name of function lgs_admin_rotate_log_file() is misleading. If
numberOfFilesToRemove is given no file rotation is done.
*   Also the name of function rotate_if_needed() is misleading. This
function can also be used to just remove the oldest file without rotation.

[Canh]  I agree that 2 function may be misleading when new feature is added.
The function name should be changed name.

 

Both function have two unrelated functions which is not correct. This is why
it will be hard to give them correct descriptive names.

 

A solution could be:

*   Create a function for checking the numberOfFilesToRemove parameter.
If there is no parameter the function can return 0 otherwise the number of
files to remove. In case of parameter error -1.
Could for example be named get_number_of_files_to_remove()

[Canh] Agree

*   Keep the original  rotate_if_needed() function and create a new
function for removing the oldest log files. The new function shall of course
be placed in the same file as rotate_if_needed()
Name example: remove_oldest_log_files(files_to_remove)

[Canh] rotate_if_needed() just remove the older log/cfg files. I don’t think
we should create new function as remove_oldest_log_files(). Almost the code
is the same.

We can rename the function rotate_if_needed()  as
remove_oldest_log_files(int files_to_remove = 0). The argument of function
can be optional.

If it files_to_remove = 0, the function remove the oldest file until there
are “maxFilesRotated" - 1 files left on disk”

 

*   Now the lgs_admin_rotate_log_file() is no longer needed. Instead
create a new function called something like lgs_admin_log_files() or place
the logic directly in the callback function.

*   The lgs_admin_log_files() only has to call
get_number_of_files_to_remove() and if it returns 0 function
rotate_if_needed() is called, if > 0 function remove_oldest_log_files() is
called or if -1 is returned do some error handling (if needed). Note the
get_number_of_files_to_remove() should handle the error reporting to the
user (calling report_om_error())

Thanks

Lennart

 

  _  

Från: Canh Van Truong mailto:canh.v.tru...@dektech.com.au> >
Skickat: den 13 juni 2019 13:57
Till: Lennart Lund; Vu Minh Nguyen
Kopia: opensaf-devel@lists.sourceforge.net
 ; Canh Van Truong
Ämne: [PATCH 1/1] log: Delete the older file without closing current log
file via admin operation [#3046] 

 

Improve the enhancement of ticket #3022, the user can rotate (delete) older
log
files without closing current log file.

LOG add new parameter (numberOfFilesToRemove) to admin op command with id =
2.
 - If parameter exists, LOGD remove number of older file base on parameter
value
 - If parameter is omitted, LOGD rotates log file normally. (closing current
log
file, rotate log file if needed, create new log file)

This helps to avoid creating empty log file or small log file size
---
 src/log/apitest/tet_LogOiOps.c | 272
+
 src/log/logd/lgs_filehdl.cc|   4 +-
 src/log/logd/lgs_imm.cc|  66 +++---
 src/log/logd/lgs_stream.cc |  31 +++--
 src/log/logd/lgs_stream.h  |   3 +-
 5 files changed, 265 insertions(+), 111 deletions(-)

diff --git a/src/log/apitest/tet_LogOiOps.c b/src/log/apitest/tet_LogOiOps.c
index 947e79664..25f53d3a2 100644
--- a/src/log/apitest/tet_LogOiOps.c
+++ b/src/log/apitest/tet_LogOiOps.c
@@ -5521,95 +5521,201 @@ void admin_rotate_log_file(void)
 }
 }
 
+// Verify that log and cfg files are rotated when the number of log/cfg
files
+// reach the "saLogStreamMaxFilesRotated" value (ticket #3045)
 //
-// Test case for verifying the admin operation rotates log file with
-// parameter fails
-// Steps:
-// 1. Create configuration app stream
-// 2. Verify command admin operation rotates log file with parameter fails
-// 3. Delete configuration app stream
+// Step1: Create and delete the app stream with (saLogStreamMaxFilesRotated
= 2)
+// 4 times
+// Step2: Verify that total is 4 cfg and log files (2 log files and 2 cfg
files)
+void verRotatedLogCfgFile(void)
+{
+   char command1[MAX_DATA], command2[MAX_DATA];
+   const char *object_dn =
+   "safLgStrCfg=verRotatedFile,safApp=safLogService";
+   const int max_file = 2;
+   char num_files_c[10];
+   uint32_t num_files = 0;
+
+   // Command to create configuration application stream
+   sprintf(command1, "immcfg -c SaLogStreamConfig %s "
+ "-a saLogStreamPathName=. "
+ "-a 

Re: [devel] [PATCH 1/1] log: Delete the older file without closing current log file via admin operation [#3046]

[Lennart Lund]

Hi Canh

Comments:

  *   The name of function lgs_admin_rotate_log_file() is misleading. If 
numberOfFilesToRemove is given no file rotation is done.
  *   Also the name of function rotate_if_needed() is misleading. This function 
can also be used to just remove the oldest file without rotation.

Both function have two unrelated functions which is not correct. This is why it 
will be hard to give them correct descriptive names.

A solution could be:

  *   Create a function for checking the numberOfFilesToRemove parameter.
If there is no parameter the function can return 0 otherwise the number of 
files to remove. In case of parameter error -1.
Could for example be named get_number_of_files_to_remove()
  *
Keep the original  rotate_if_needed() function and create a new function for 
removing the oldest log files. The new function shall of course be placed in 
the same file as rotate_if_needed()
Name example: remove_oldest_log_files(files_to_remove)
  *
Now the lgs_admin_rotate_log_file() is no longer needed. Instead create a new 
function called something like lgs_admin_log_files() or place the logic 
directly in the callback function.
  *
The lgs_admin_log_files() only has to call get_number_of_files_to_remove() and 
if it returns 0 function rotate_if_needed() is called, if > 0 function 
remove_oldest_log_files() is called or if -1 is returned do some error handling 
(if needed). Note the get_number_of_files_to_remove() should handle the error 
reporting to the user (calling report_om_error())

Thanks
Lennart


Från: Canh Van Truong 
Skickat: den 13 juni 2019 13:57
Till: Lennart Lund; Vu Minh Nguyen
Kopia: opensaf-devel@lists.sourceforge.net; Canh Van Truong
Ämne: [PATCH 1/1] log: Delete the older file without closing current log file 
via admin operation [#3046]

Improve the enhancement of ticket #3022, the user can rotate (delete) older log
files without closing current log file.

LOG add new parameter (numberOfFilesToRemove) to admin op command with id = 2.
 - If parameter exists, LOGD remove number of older file base on parameter value
 - If parameter is omitted, LOGD rotates log file normally. (closing current log
file, rotate log file if needed, create new log file)

This helps to avoid creating empty log file or small log file size
---
 src/log/apitest/tet_LogOiOps.c | 272 +
 src/log/logd/lgs_filehdl.cc|   4 +-
 src/log/logd/lgs_imm.cc|  66 +++---
 src/log/logd/lgs_stream.cc |  31 +++--
 src/log/logd/lgs_stream.h  |   3 +-
 5 files changed, 265 insertions(+), 111 deletions(-)

diff --git a/src/log/apitest/tet_LogOiOps.c b/src/log/apitest/tet_LogOiOps.c
index 947e79664..25f53d3a2 100644
--- a/src/log/apitest/tet_LogOiOps.c
+++ b/src/log/apitest/tet_LogOiOps.c
@@ -5521,95 +5521,201 @@ void admin_rotate_log_file(void)
 }
 }

+// Verify that log and cfg files are rotated when the number of log/cfg files
+// reach the "saLogStreamMaxFilesRotated" value (ticket #3045)
 //
-// Test case for verifying the admin operation rotates log file with
-// parameter fails
-// Steps:
-// 1. Create configuration app stream
-// 2. Verify command admin operation rotates log file with parameter fails
-// 3. Delete configuration app stream
+// Step1: Create and delete the app stream with (saLogStreamMaxFilesRotated = 
2)
+// 4 times
+// Step2: Verify that total is 4 cfg and log files (2 log files and 2 cfg 
files)
+void verRotatedLogCfgFile(void)
+{
+   char command1[MAX_DATA], command2[MAX_DATA];
+   const char *object_dn =
+   "safLgStrCfg=verRotatedFile,safApp=safLogService";
+   const int max_file = 2;
+   char num_files_c[10];
+   uint32_t num_files = 0;
+
+   // Command to create configuration application stream
+   sprintf(command1, "immcfg -c SaLogStreamConfig %s "
+ "-a saLogStreamPathName=. "
+ "-a saLogStreamFileName=verRotatedFile"
+ " -a saLogStreamMaxFilesRotated=%d",
+ object_dn, max_file);
+   // Command to delete configuration application stream
+   sprintf(command2, "immcfg -d %s", object_dn);
+
+   // Create and delete the app stream 4 times.
+   // Log/cfg are created during creating stream.
+   for (int i = 0; i < max_file + 1; ++i) {
+   int rc = systemCall(command1);
+   if (rc == 0) {
+   osaf_nanosleep();
+   rc = systemCall(command2);
+   osaf_nanosleep();
+   }
+   if (rc != 0) {
+   rc_validate(rc, 0);
+   return;
+   }
+   }
+   // Command to count number of log/cfg files on disk
+   sprintf(command1, "find %s -type f -mmin -1 "
+ "| egrep '%s.*\\.[log$\\|cfg$]' "
+ "| wc -l | awk '{printf $1}'",
+

[devel] [PATCH 1/1] log: Delete the older file without closing current log file via admin operation [#3046]

[Canh Van Truong]

Improve the enhancement of ticket #3022, the user can rotate (delete) older log
files without closing current log file.

LOG add new parameter (numberOfFilesToRemove) to admin op command with id = 2.
 - If parameter exists, LOGD remove number of older file base on parameter value
 - If parameter is omitted, LOGD rotates log file normally. (closing current log
file, rotate log file if needed, create new log file)

This helps to avoid creating empty log file or small log file size
---
 src/log/apitest/tet_LogOiOps.c | 272 +
 src/log/logd/lgs_filehdl.cc|   4 +-
 src/log/logd/lgs_imm.cc|  66 +++---
 src/log/logd/lgs_stream.cc |  31 +++--
 src/log/logd/lgs_stream.h  |   3 +-
 5 files changed, 265 insertions(+), 111 deletions(-)

diff --git a/src/log/apitest/tet_LogOiOps.c b/src/log/apitest/tet_LogOiOps.c
index 947e79664..25f53d3a2 100644
--- a/src/log/apitest/tet_LogOiOps.c
+++ b/src/log/apitest/tet_LogOiOps.c
@@ -5521,95 +5521,201 @@ void admin_rotate_log_file(void)
}
 }
 
+// Verify that log and cfg files are rotated when the number of log/cfg files
+// reach the "saLogStreamMaxFilesRotated" value (ticket #3045)
 //
-// Test case for verifying the admin operation rotates log file with
-// parameter fails
-// Steps:
-// 1. Create configuration app stream
-// 2. Verify command admin operation rotates log file with parameter fails
-// 3. Delete configuration app stream
+// Step1: Create and delete the app stream with (saLogStreamMaxFilesRotated = 
2)
+// 4 times
+// Step2: Verify that total is 4 cfg and log files (2 log files and 2 cfg 
files)
+void verRotatedLogCfgFile(void)
+{
+   char command1[MAX_DATA], command2[MAX_DATA];
+   const char *object_dn =
+   "safLgStrCfg=verRotatedFile,safApp=safLogService";
+   const int max_file = 2;
+   char num_files_c[10];
+   uint32_t num_files = 0;
+
+   // Command to create configuration application stream
+   sprintf(command1, "immcfg -c SaLogStreamConfig %s "
+ "-a saLogStreamPathName=. "
+ "-a saLogStreamFileName=verRotatedFile"
+ " -a saLogStreamMaxFilesRotated=%d",
+ object_dn, max_file);
+   // Command to delete configuration application stream
+   sprintf(command2, "immcfg -d %s", object_dn);
+
+   // Create and delete the app stream 4 times.
+   // Log/cfg are created during creating stream.
+   for (int i = 0; i < max_file + 1; ++i) {
+   int rc = systemCall(command1);
+   if (rc == 0) {
+   osaf_nanosleep();
+   rc = systemCall(command2);
+   osaf_nanosleep();
+   }
+   if (rc != 0) {
+   rc_validate(rc, 0);
+   return;
+   }
+   }
+   // Command to count number of log/cfg files on disk
+   sprintf(command1, "find %s -type f -mmin -1 "
+ "| egrep '%s.*\\.[log$\\|cfg$]' "
+ "| wc -l | awk '{printf $1}'",
+ log_root_path, "verRotatedFile");
+
+   FILE *fp = popen(command1, "r");
+
+   // Get number of cfg and log file
+   while (fgets(num_files_c, sizeof(num_files_c) - 1, fp) != NULL) {};
+   pclose(fp);
+
+   // Verify cfg/log files are rotated by checking
+   // that there are totally 4 cfg and log files
+   num_files = atoi(num_files_c);
+   rc_validate(num_files, max_file * 2);
+}
+
+// Verify that log file is rotated via admin operation with admin id = 2
+// and no parameter
 //
-void admin_rotate_log_file_with_param(void)
+// Step1: Create and delete the app stream with (saLogStreamMaxFilesRotated = 
2)
+// step2: Do admin operation 2 times to rotate log file.
+//Two new log file are created
+// step3: Verify log file is rotated via admin operation by checking that
+//there are 2 log files on disk
+void verRotatedLogCfgFile2(void)
 {
-   int rc = 0;
+   int rc;
char command[MAX_DATA];
const char *object_dn =
-   "safLgStrCfg=admin_rotate_file1,safApp=safLogService";
+   "safLgStrCfg=verRotatedFile2,safApp=safLogService";
+   char num_files_c[10];
+   uint32_t num_files = 0;
 
-   // Create configuration application stream
-   sprintf(command, "immcfg -c SaLogStreamConfig %s"
-   " -a saLogStreamPathName=. "
-   "-a saLogStreamFileName=admin_rotate_file1 ",
-   object_dn);
+   // Command to create configuration application stream
+   sprintf(command, "immcfg -c SaLogStreamConfig %s "
+" -a saLogStreamPathName=. "
+"-a saLogStreamFileName=verRotatedFile2"
+" -a saLogStreamMaxFilesRotated=2",
+