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
