Hi Canh, Ack with minor comments.
Regards, Vu > -----Original Message----- > From: Canh Van Truong <canh.v.tru...@dektech.com.au> > Sent: Monday, June 3, 2019 6:18 PM > To: lennart.l...@ericsson.com; vu.m.ngu...@dektech.com.au > Cc: opensaf-devel@lists.sourceforge.net; Canh Van Truong > <canh.v.tru...@dektech.com.au> > Subject: [PATCH 1/1] log: Delete the older file without closing current log file > [#3046] > > Improve the enhancement of ticket #3022, the user can rotate (delete) older > log > files without closing current log file. > > LOG add new parameter(optional) to admin op command with id = 2. > - If parameter exists, LOGD rotates log file without closing current log file > - 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/logd/lgs_filehdl.cc | 11 ++++++-- > src/log/logd/lgs_imm.cc | 69 ++++++++++++++++++++++++++++++++------ > ------- > src/log/logd/lgs_stream.cc | 13 +++++---- > src/log/logd/lgs_stream.h | 1 + > 4 files changed, 66 insertions(+), 28 deletions(-) > > diff --git a/src/log/logd/lgs_filehdl.cc b/src/log/logd/lgs_filehdl.cc > index be2b95e1f..ad0e5b5c8 100644 > --- a/src/log/logd/lgs_filehdl.cc > +++ b/src/log/logd/lgs_filehdl.cc > @@ -695,7 +695,7 @@ done_free: > free(namelist); > > done_exit: > - TRACE_LEAVE(); > + TRACE_LEAVE2("rc = %d", rc); > return rc; > } > > @@ -790,7 +790,12 @@ int get_number_of_cfg_files_hdl(void *indata, void > *outdata, > &cfg_old_time)) { > old_ind = n; > } else { > - failed++; /* wrong format */ > + const std::string current_cfg_name = > + std::string(params_in->file_name) + ".cfg"; > + if (strncmp(cfg_namelist[n]->d_name, current_cfg_name.c_str(), > + current_cfg_name.size()) != 0) { > + failed++; // wrong format > + } > } > } > > @@ -828,7 +833,7 @@ done_log_free: > } > > done_exit: > - TRACE_LEAVE(); > + TRACE_LEAVE2("rc = %d", rc); > return rc; > } > > diff --git a/src/log/logd/lgs_imm.cc b/src/log/logd/lgs_imm.cc > index 49bc686b6..612004cad 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"); > } > @@ -3403,14 +3400,20 @@ static void lgs_admin_change_filter( > > /** > * Handle admin operation that rotates the log file > + * - If parameter exists, LOGD rotate log file without closing current log file > + * - If parameter is omitted, LOGD rotate log file normally. > + * (closing current log file, rotate log file if needed, create new log file) > * > * @param immOiHandle > * @param invocation > * @param objectName > + * @param 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) { > TRACE_ENTER(); > > osafassert(objectName != nullptr); > @@ -3424,13 +3427,41 @@ 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) { > + int ret = log_rotation_act(stream); [Vu] can remove one else by this way. If (ret != 0) { Report_om_error(); Return; } ckpt_stream_config(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; > + } else { > + if (strcmp(parameter->paramName, "saLogStreamMaxFiles") != 0) { > + report_om_error(immOiHandle, invocation, > + "Admin op rotate log file, invalid param name. " > + "Should be 'saLogStreamMaxFiles'"); > + return; > + } else if (parameter->paramType != SA_IMM_ATTR_SAUINT32T) { [Vu] else here is not necessary. Use if (parameter->paramType != SA_IMM_ATTR_SAUINT32T) { .... return; } > + report_om_error(immOiHandle, invocation, > + "Admin op rotate log file: invalid parameter type. " > + "Should be 'SA_UINT64_T'"); > + return; > + } else { [Vu] else here is not necessary too. > + SaUint32T max_files_rotated = > + *(reinterpret_cast<SaUint32T *>(parameter->paramBuffer)) + 1; > + > + if (max_files_rotated < 2 || max_files_rotated > 128) { > + report_om_error(immOiHandle, invocation, > + "Admin op rotate log file: Out of range " > + "(min:1 - max:127)"); [Vu] min = 2, max = 128? > + return; > + } > + > + if (max_files_rotated <= stream->maxFilesRotated) > + if (rotate_if_needed(stream, max_files_rotated) == -1) > + 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 d31ae170b..76cdd52f8 100644 > --- a/src/log/logd/lgs_stream.cc > +++ b/src/log/logd/lgs_stream.cc > @@ -272,9 +272,10 @@ static int delete_config_file(log_stream_t *stream) > { > * Remove oldest log file until there are 'maxFilesRotated' - 1 files left > * > * @param stream > + * @param max_files_rotated > * @return -1 on error > */ > -static int rotate_if_needed(log_stream_t *stream) { > +int rotate_if_needed(log_stream_t *stream, int max_files_rotated) { > char oldest_log_file[PATH_MAX]; > char oldest_cfg_file[PATH_MAX]; > int rc = 0; > @@ -303,7 +304,7 @@ static int rotate_if_needed(log_stream_t *stream) { > ** Remove until we have one less than allowed, we are just about to > ** create a new one again. > */ > - while (log_file_cnt >= static_cast<int>(stream->maxFilesRotated)) { > + while (log_file_cnt >= max_files_rotated) { > if ((rc = file_unlink_h(oldest_log_file)) == -1) { > LOG_NO("Could not log delete: %s - %s", oldest_log_file, strerror(errno)); > goto done; > @@ -362,7 +363,7 @@ void log_initiate_stream_files(log_stream_t *stream) > { > (void)delete_config_file(stream); > > /* Remove files from a previous life if needed */ > - if (rotate_if_needed(stream) == -1) { > + if (rotate_if_needed(stream, stream->maxFilesRotated) == -1) { > TRACE("%s - rotate_if_needed() FAIL", __FUNCTION__); > goto done; > } > @@ -1111,7 +1112,7 @@ int log_rotation_stb(log_stream_t *stream) { > } > > // Remove oldest file if needed > - if (rotate_if_needed(stream) == -1) { > + if (rotate_if_needed(stream, stream->maxFilesRotated) == -1) { > TRACE("Old file removal failed"); > } > // Save new name for current log file and open it > @@ -1180,7 +1181,7 @@ int log_rotation_act(log_stream_t *stream) { > // Reset file size for current log file > stream->curFileSize = 0; > // Remove oldest file if needed > - if (rotate_if_needed(stream) == -1) > + if (rotate_if_needed(stream, stream->maxFilesRotated) == -1) > TRACE("Old file removal failed"); > > // Create a new file name that includes "open time stamp" and open the file > @@ -1534,7 +1535,7 @@ int log_stream_config_change(bool create_files_f, > const std::string &root_path, > } > } > > - rotate_if_needed(stream); > + rotate_if_needed(stream, stream->maxFilesRotated); > > /* Reset file size for new log file */ > stream->curFileSize = 0; > diff --git a/src/log/logd/lgs_stream.h b/src/log/logd/lgs_stream.h > index 2aa6922f4..0681fdfeb 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); > +int rotate_if_needed(log_stream_t *stream, int max_files_rotated); > > #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