Hi Canh I think you can send the new version for review
Thanks Lennart From: Canh Van Truong <canh.v.tru...@dektech.com.au> Sent: den 14 juni 2019 13:31 To: Lennart Lund <lennart.l...@ericsson.com>; Vu Minh Nguyen <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 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 "<size of rotation buffer> - 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 <lennart.l...@ericsson.com<mailto:lennart.l...@ericsson.com>> Sent: Friday, June 14, 2019 5:57 PM To: Canh Van Truong <canh.v.tru...@dektech.com.au<mailto:canh.v.tru...@dektech.com.au>>; Vu Minh Nguyen <vu.m.ngu...@dektech.com.au<mailto:vu.m.ngu...@dektech.com.au>> Cc: opensaf-devel@lists.sourceforge.net<mailto: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 <canh.v.tru...@dektech.com.au<mailto:canh.v.tru...@dektech.com.au>> Sent: den 14 juni 2019 06:56 To: Lennart Lund <lennart.l...@ericsson.com<mailto:lennart.l...@ericsson.com>>; Vu Minh Nguyen <vu.m.ngu...@dektech.com.au<mailto:vu.m.ngu...@dektech.com.au>> Cc: opensaf-devel@lists.sourceforge.net<mailto: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 <lennart.l...@ericsson.com<mailto:lennart.l...@ericsson.com>> Sent: Thursday, June 13, 2019 11:02 PM To: Canh Van Truong <canh.v.tru...@dektech.com.au<mailto:canh.v.tru...@dektech.com.au>>; Vu Minh Nguyen <vu.m.ngu...@dektech.com.au<mailto:vu.m.ngu...@dektech.com.au>> Cc: opensaf-devel@lists.sourceforge.net<mailto: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 <canh.v.tru...@dektech.com.au<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<mailto: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(&kHundredMilliseconds); + rc = systemCall(command2); + osaf_nanosleep(&kOneSecond); + } + 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", + object_dn); + // Create the app stream. rc = systemCall(command); - if (rc == 0) { - // Admin operation opId = 2 with parameter - sprintf(command, "immadm -o 2 -p " - "saLogStreamSeverityFilter:SA_UINT32_T:7 %s >" - " /dev/null 2>&1 ", object_dn); - rc = system(command); - rc_validate(WEXITSTATUS(rc), 1); - - // Delete object - sprintf(command, "immcfg -d %s", object_dn); - systemCall(command); - } else { + if (rc != 0) { rc_validate(rc, 0); + return; } + + // Do admin operation to rotate log file without the parameter + // Two new log files are created. One oldest file is removed. + sprintf(command, "immadm -o 2 %s", object_dn); + for (int i = 0; i < 2; ++i) { + osaf_nanosleep(&kOneSecond); + rc = systemCall(command); + if (rc != 0) { + rc_validate(rc, 0); + return; + } + } + + // Command to count number of log files on disk + sprintf(command, "find %s -type f -mmin -1 " + "| egrep '%s.*\\.log$' " + "| wc -l | awk '{printf $1}'", + log_root_path, "verRotatedFile2"); + FILE *fp = popen(command, "r"); + // Get number of log files + while (fgets(num_files_c, sizeof(num_files_c) - 1, fp) != NULL) {}; + pclose(fp); + + // Command to delete configuration application stream + sprintf(command, "immcfg -d %s", object_dn); + // Close the application stream + rc = systemCall(command); + if (rc != 0) { + rc_validate(rc, 0); + return; + } + + // Verify log file is rotated by checking that + // there are totally 4 log files on disk + num_files = atoi(num_files_c); + rc_validate(num_files, 2); } -// Verify that log and cfg files are rotated when the number of log/cfg files -// reach the "saLogStreamMaxFilesRotated" value (ticket #3045) +// Verify that oldest log and cfg files are removed via admin operation with +// admin id = 2 and parameter exist // -// 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) +// Step1: Create and delete the app stream with (saLogStreamMaxFilesRotated = 4) +// (There are two log files exists during create stream 2 times) +// step2: Do admin operation to remove 1 oldest log files +// step3: Verify 1 oldest log file is removed by checking that +// there are 1 log files on disk +void verRotatedLogCfgFile3(void) { - char command1[MAX_DATA], command2[MAX_DATA]; - const char *object_dn = - "safLgStrCfg=verRotatedFile,safApp=safLogService"; - const uint32_t 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(&kHundredMilliseconds); - rc = systemCall(command2); - osaf_nanosleep(&kOneSecond); - } - 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$<file://.[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 total is 4 cfg and log files - num_files = atoi(num_files_c); - rc_validate(num_files, max_file * 2); + int rc; + char command1[MAX_DATA], command2[MAX_DATA]; + const char *object_dn = + "safLgStrCfg=verRotatedFile3,safApp=safLogService"; + 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=verRotatedFile3" + " -a saLogStreamMaxFilesRotated=4", + object_dn); + // Command to delete configuration application stream + sprintf(command2, "immcfg -d %s", object_dn); + + // Create and delete the app stream. + // 2 log are created during creating stream. + for (int i = 0; i < 2; ++i) { + rc = systemCall(command1); + if (rc == 0) { + osaf_nanosleep(&kHundredMilliseconds); + if (i == 1) break; // Keep opened stream at last turn + rc = systemCall(command2); + osaf_nanosleep(&kOneSecond); + } + if (rc != 0) { + rc_validate(rc, 0); + return; + } + } + + // Admin operation to remove 1 oldest log files + sprintf(command1, "immadm -o 2 -p numberOfFilesToRemove:SA_UINT32_T:1 " + "%s", object_dn); + rc = systemCall(command1); + 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_([0-9]{8}_[0-9]{6}\\.log$)\" " + "| wc -l | awk '{printf $1}'", + log_root_path, "verRotatedFile3"); + 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); + + // Close the application stream + rc = systemCall(command2); + if (rc != 0) { + rc_validate(rc, 0); + return; + } + + // Verify 1 oldest cfg/log files are removed + // by checking that there is 1 log files on disk + num_files = atoi(num_files_c); + rc_validate(num_files, 1); } __attribute__((constructor)) static void saOiOperations_constructor(void) @@ -5969,9 +6075,13 @@ __attribute__((constructor)) static void saOiOperations_constructor(void) 6, verLogFileRotate, "Verify that log file is not rotated if file size doesn't reach max file size (ticket1439)"); test_case_add(6, admin_rotate_log_file, - "Verify admin operation rotate log file is accepted"); - test_case_add(6, admin_rotate_log_file_with_param, - "Verify admin operation rotate log file with parameter fails"); - test_case_add(6, verRotatedLogCfgFile, - "Verify that both log and cfg files are rotated"); + "Verify admin operation rotate log file is accepted"); + test_case_add(6, verRotatedLogCfgFile, + "Verify that both log and cfg files are rotated"); + test_case_add(6, verRotatedLogCfgFile2, + "Verify log file is rotated via admin operation " + "without parameter"); + test_case_add(6, verRotatedLogCfgFile3, + "Verify oldest log files are removed via admin operation " + "with parameter"); } diff --git a/src/log/logd/lgs_filehdl.cc b/src/log/logd/lgs_filehdl.cc index b1afce3d5..e683f8b68 100644 --- a/src/log/logd/lgs_filehdl.cc +++ b/src/log/logd/lgs_filehdl.cc @@ -552,7 +552,7 @@ static int check_log_oldest(char *line, char *fname_prefix, *old_date = date; *old_time = time; return 1; - } else if ((date == *old_date) && (time < *old_time)) { + } else if ((date == *old_date) && (time <= *old_time)) { *old_date = date; *old_time = time; return 1; @@ -800,7 +800,7 @@ int get_number_of_cfg_files_hdl(void *indata, void *outdata, } } - if (old_ind == -1) goto done_log_free; + if (old_ind == -1) goto done_cfg_free; // If the oldest cfg file is older than the oldest log file, the output // is returned with oldest cfg file. Otherwise the output is returned empty diff --git a/src/log/logd/lgs_imm.cc b/src/log/logd/lgs_imm.cc index 49bc686b6..550959476 100644 --- a/src/log/logd/lgs_imm.cc +++ b/src/log/logd/lgs_imm.cc @@ -84,9 +84,11 @@ static std::vector<std::string> file_path_list; static void lgs_admin_change_filter( SaImmOiHandleT immOiHandle, SaInvocationT invocation, SaConstStringT objectName, const SaImmAdminOperationParamsT_2 *parameter); -static void lgs_admin_rotate_log_file(SaImmOiHandleT immOiHandle, - SaInvocationT invocation, - SaConstStringT objectName); +static void lgs_admin_rotate_log_file( + SaImmOiHandleT immOiHandle, + SaInvocationT invocation, + SaConstStringT objectName, + const SaImmAdminOperationParamsT_2 *parameter); static void report_oi_error(SaImmOiHandleT immOiHandle, SaImmOiCcbIdT ccbId, const char *format, ...) @@ -396,12 +398,7 @@ static void adminOperationCallback( } else if (opId == SA_LOG_ADMIN_CHANGE_FILTER) { lgs_admin_change_filter(immOiHandle, invocation, objName, param); } else if (opId == SA_LOG_ADMIN_ROTATE_FILE) { - if (param != nullptr) { - report_om_error(immOiHandle, invocation, - "Admin op rotate log file: parameters is not empty"); - } else { - lgs_admin_rotate_log_file(immOiHandle, invocation, objName); - } + lgs_admin_rotate_log_file(immOiHandle, invocation, objName, param); } else { report_om_error( immOiHandle, invocation, "Invalid operation ID"); } @@ -3408,9 +3405,11 @@ static void lgs_admin_change_filter( * @param invocation * @param objectName */ -static void lgs_admin_rotate_log_file(SaImmOiHandleT immOiHandle, - SaInvocationT invocation, - SaConstStringT objectName) { +static void lgs_admin_rotate_log_file( + SaImmOiHandleT immOiHandle, + SaInvocationT invocation, + SaConstStringT objectName, + const SaImmAdminOperationParamsT_2 *parameter) { TRACE_ENTER(); osafassert(objectName != nullptr); @@ -3424,13 +3423,42 @@ static void lgs_admin_rotate_log_file(SaImmOiHandleT immOiHandle, // Rotate log file. // Send the result "FAILED_OPERATION" to admin if the rotation fails SaAisErrorT result = SA_AIS_OK; - int ret = log_rotation_act(stream); - if (ret == 0) - // Checkpoint the stream to standby to rotate the log file - // in case the split file system - ckpt_stream_config(stream); - else - result = SA_AIS_ERR_FAILED_OPERATION; + + if (parameter == nullptr) { + if (log_rotation_act(stream) == 0) + // Checkpoint the stream to standby to rotate the log file + // in case the split file system + ckpt_stream_config(stream); + else + result = SA_AIS_ERR_FAILED_OPERATION; + } else { + if (strcmp(parameter->paramName, "numberOfFilesToRemove") != 0) { + report_om_error(immOiHandle, invocation, + "Admin op rotate log file, invalid param name. " + "Should be 'numberOfFilesToRemove'"); + return; + } + if (parameter->paramType != SA_IMM_ATTR_SAUINT32T) { + report_om_error(immOiHandle, invocation, + "Admin op rotate log file: invalid parameter type. " + "Should be 'SA_UINT32_T'"); + return; + } + + SaUint32T number_files_to_remove = + *(reinterpret_cast<SaUint32T *>(parameter->paramBuffer)); + + if (number_files_to_remove < 1 || + number_files_to_remove >= stream->maxFilesRotated) { + report_om_error(immOiHandle, invocation, + "Admin op rotate log file: Out of range " + "(min:1 - max:%d)", stream->maxFilesRotated - 1); + return; + } + + if (rotate_if_needed(stream, number_files_to_remove) == false) + result = SA_AIS_ERR_FAILED_OPERATION; + } result = immutil_saImmOiAdminOperationResult(immOiHandle, invocation, result); if (result == SA_AIS_ERR_BAD_HANDLE) { diff --git a/src/log/logd/lgs_stream.cc b/src/log/logd/lgs_stream.cc index 7709c3c81..bd499b958 100644 --- a/src/log/logd/lgs_stream.cc +++ b/src/log/logd/lgs_stream.cc @@ -269,35 +269,50 @@ static int delete_config_file(log_stream_t *stream) { } /** - * Remove oldest log file until there are 'maxFilesRotated' - 1 files left - * The oldest cfg files are also removed + * Remove oldest log and cfg files on disk. + * If the parameter @number_files_to_remove is zero, remove older log and cfg + * files until there are "maxFilesRotated" - 1 files left on disk. * * @param stream + * @param number_files_to_remove * @return true/false */ -static bool rotate_if_needed(log_stream_t *stream) { +bool rotate_if_needed(log_stream_t *stream, + int number_files_to_remove) { char oldest_log_file[PATH_MAX]; char oldest_cfg_file[PATH_MAX]; - const int max_files_rotated = static_cast<int>(stream->maxFilesRotated); + int max_files_rotated = static_cast<int>(stream->maxFilesRotated) - 1; - TRACE_ENTER(); + TRACE_ENTER2("number of files to remove: %d", number_files_to_remove); // Get number of log files and the oldest log file int log_file_cnt = get_number_of_log_files_h(stream, oldest_log_file); - while (log_file_cnt >= max_files_rotated) { + if (log_file_cnt == -1) return false; + + if (number_files_to_remove != 0) { + // If there are not enough number of log file to remove, return true + if (log_file_cnt <= number_files_to_remove) { + LOG_WA("No (%d) older log files to remove on disk", + number_files_to_remove); + return true; + } + max_files_rotated = log_file_cnt - number_files_to_remove; + } + + while (log_file_cnt > max_files_rotated) { TRACE("Delete oldest_log_file %s", oldest_log_file); if (file_unlink_h(oldest_log_file) == -1) { LOG_NO("Delete log file fail: %s - %s", oldest_log_file, strerror(errno)); return false; } log_file_cnt = get_number_of_log_files_h(stream, oldest_log_file); + if (log_file_cnt == -1) return false; } - if (log_file_cnt == -1) return false; // Housekeeping for cfg files int number_deleted_files = 0; int cfg_file_cnt = get_number_of_cfg_files_h(stream, oldest_cfg_file); - while (cfg_file_cnt >= max_files_rotated) { + while (cfg_file_cnt > max_files_rotated) { TRACE("Delete oldest_cfg_file %s", oldest_cfg_file); if (file_unlink_h(oldest_cfg_file) == -1) { LOG_NO("Delete cfg file fail: %s - %s", oldest_cfg_file, strerror(errno)); diff --git a/src/log/logd/lgs_stream.h b/src/log/logd/lgs_stream.h index 2aa6922f4..2bdfad79f 100644 --- a/src/log/logd/lgs_stream.h +++ b/src/log/logd/lgs_stream.h @@ -149,5 +149,6 @@ void lgs_ckpt_stream_open(log_stream_t *stream, uint32_t client_id); int log_rotation_act(log_stream_t *stream); int log_rotation_stb(log_stream_t *stream); - +bool rotate_if_needed(log_stream_t *stream, + int number_files_to_remove = 0); #endif // LOG_LOGD_LGS_STREAM_H_ -- 2.15.1 _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel