Hi Canh,
Ack with minor comments.
Regards, Vu
> -----Original Message-----
> From: Canh Van Truong <canh.v.tru...@dektech.com.au>
> Sent: Tuesday, April 16, 2019 3:27 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: Log enhance to rotate log file via admin
operation
> [#3022]
[Vu] might change the slogan to 'log: rotate log file ...'?
>
> In some cases the Users want to rotate log file at the time althouth
> the file size hasn't been reached MAX FILE SIZE. Log service provides the
> new feature that user can rotate log file via admin operation.
> ---
> src/ais/include/saLog.h | 3 +-
> src/log/apitest/tet_LogOiOps.c | 76 ++++++++++++
> src/log/logd/lgs_imm.cc | 258
++++++++++++++++++++++++---------------
> --
> src/log/logd/lgs_mbcsv.cc | 51 ++++++--
> src/log/logd/lgs_stream.cc | 153 ++++++++++++------------
> src/log/logd/lgs_stream.h | 3 +
> 6 files changed, 342 insertions(+), 202 deletions(-)
>
> diff --git a/src/ais/include/saLog.h b/src/ais/include/saLog.h
> index 313c8d287..c28750600 100644
> --- a/src/ais/include/saLog.h
> +++ b/src/ais/include/saLog.h
> @@ -196,7 +196,8 @@ extern "C" {
>
> /* Admin operation IDs */
> typedef enum {
> - SA_LOG_ADMIN_CHANGE_FILTER = 1
> + SA_LOG_ADMIN_CHANGE_FILTER = 1,
> + SA_LOG_ADMIN_ROTATE_FILE = 2
> } saLogAdminOperationIdT;
>
> /*
> diff --git a/src/log/apitest/tet_LogOiOps.c
b/src/log/apitest/tet_LogOiOps.c
> index e74cd5331..ea96d86ce 100644
> --- a/src/log/apitest/tet_LogOiOps.c
> +++ b/src/log/apitest/tet_LogOiOps.c
> @@ -5479,6 +5479,78 @@ done:
> &v_saLogStreamFixedLogRecordSize,
> SA_IMM_ATTR_SAUINT32T);
> }
>
> +//
> +// Test case for verifying the admin operation rotate log file for
> +// configuration application is ok
> +// Steps:
> +// 1. Create configuration app stream
> +// 2. Verify command admin operation rotate log file is ok
> +// 3. Delete configuration app stream
> +//
> +void admin_rotate_log_file(void)
> +{
> + int rc = 0;
> + char command[MAX_DATA];
> + const char *object_dn =
> +
> "safLgStrCfg=admin_rotate_file,safApp=safLogService";
> +
> + // Create configuration application stream
> + sprintf(command, "immcfg -c SaLogStreamConfig %s "
> + "-a saLogStreamPathName=. "
> + "-a saLogStreamFileName=admin_rotate_file ",
> object_dn);
> + rc = systemCall(command);
> + osaf_nanosleep(&kOneSecond);
> + if (rc == 0) {
> + // Admin operation rotate log file
> + sprintf(command, "immadm -o 2 %s", object_dn);
> + rc = systemCall(command);
> + rc_validate(rc, 0);
> +
> + // Delete object
> + sprintf(command, "immcfg -d %s", object_dn);
> + systemCall(command);
> + } else {
> + rc_validate(rc, 0);
> + }
> +}
> +
> +//
> +// Test case for verifying the admin operation rotate log file for
> +// configuration application is ok
> +// Steps:
> +// 1. Create configuration app stream
> +// 2. Verify command admin operation rotate log file with param fails
> +// 3. Delete configuration app stream
> +//
> +void admin_rotate_log_file_with_param(void)
> +{
> + int rc = 0;
> + char command[MAX_DATA];
> + const char *object_dn =
> +
> "safLgStrCfg=admin_rotate_file1,safApp=safLogService";
> +
> + // Create configuration application stream
> + sprintf(command, "immcfg -c SaLogStreamConfig %s"
> + " -a saLogStreamPathName=. "
> + "-a saLogStreamFileName=admin_rotate_file1 ",
> + object_dn);
> + 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 {
> + rc_validate(rc, 0);
> + }
> +}
> +
> __attribute__((constructor)) static void saOiOperations_constructor(void)
> {
> /* Stream objects */
> @@ -5835,4 +5907,8 @@ __attribute__((constructor)) static void
> saOiOperations_constructor(void)
> test_case_add(
> 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 ok");
> + test_case_add(6, admin_rotate_log_file_with_param,
> + "Verify admin operation rotate log file with parameter
> fails");
> }
> diff --git a/src/log/logd/lgs_imm.cc b/src/log/logd/lgs_imm.cc
> index 45c69f9db..c010142f3 100644
> --- a/src/log/logd/lgs_imm.cc
> +++ b/src/log/logd/lgs_imm.cc
> @@ -81,6 +81,12 @@ static std::vector<std::string> file_path_list;
> /* FUNCTIONS
> * ---------
> */
> +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 report_oi_error(SaImmOiHandleT immOiHandle, SaImmOiCcbIdT
> ccbId,
> const char *format, ...)
> @@ -127,14 +133,18 @@ static void report_om_error(SaImmOiHandleT
> immOiHandle,
>
> va_list ap;
> va_start(ap, format);
> - (void)vsnprintf(ao_err_string, 256, format, ap);
> + (void)vsnprintf(ao_err_string, sizeof(ao_err_string), format, ap);
> va_end(ap);
>
> TRACE("%s", ao_err_string);
> - if (saImmOiAdminOperationResult_o2(immOiHandle, invocation,
> - SA_AIS_ERR_INVALID_PARAM,
> - ao_err_params) ==
SA_AIS_ERR_BAD_HANDLE) {
> + SaAisErrorT rc = saImmOiAdminOperationResult_o2(immOiHandle,
> invocation,
> +
SA_AIS_ERR_INVALID_PARAM,
> + ao_err_params);
> + if (rc == SA_AIS_ERR_BAD_HANDLE) {
> lgsOiCreateBackground();
> + } else if (rc != SA_AIS_OK) {
> + LOG_ER("immutil_saImmOiAdminOperationResult failed %s",
> saf_error(rc));
> + osaf_abort(0);
> }
> }
>
> @@ -376,122 +386,26 @@ static void adminOperationCallback(
> SaImmOiHandleT immOiHandle, SaInvocationT invocation,
> const SaNameT *objectName, SaImmAdminOperationIdT opId,
> const SaImmAdminOperationParamsT_2 **params) {
> - SaUint32T severityFilter;
> const SaImmAdminOperationParamsT_2 *param = params[0];
> - log_stream_t *stream;
> - SaAisErrorT ais_rc = SA_AIS_OK;
> SaConstStringT objName = osaf_extended_name_borrow(objectName);
>
> TRACE_ENTER2("%s", objName);
>
> if (lgs_cb->ha_state != SA_AMF_HA_ACTIVE) {
> LOG_ER("admin op callback in applier");
> - goto done;
> - }
> -
> - if ((stream = log_stream_get_by_name(objName)) == NULL) {
> - report_om_error(immOiHandle, invocation, "Stream %s not found",
> objName);
> - goto done;
> - }
> -
> - if (opId == SA_LOG_ADMIN_CHANGE_FILTER) {
> - /* Only allowed to update runtime objects (application streams) */
> - if (stream->streamType != STREAM_TYPE_APPLICATION_RT) {
> - report_om_error(immOiHandle, invocation,
> - "Admin op change filter for non app stream");
> - goto done;
> - }
> -
> - /* Not allow perform adm op on configurable app streams */
> - if (stream->isRtStream != SA_TRUE) {
> - ais_rc = immutil_saImmOiAdminOperationResult(immOiHandle,
> invocation,
> -
SA_AIS_ERR_NOT_SUPPORTED);
> - // TODO(Lennart) May be relevant to recover OI if BAD HANDLE
> - if (ais_rc != SA_AIS_OK) {
> - LOG_ER("immutil_saImmOiAdminOperationResult failed %s",
> - saf_error(ais_rc));
> - osaf_abort(0);
> - }
> -
> - goto done;
> - }
> -
> - if (param == NULL) {
> - /* No parameters given in admin op */
> - report_om_error(immOiHandle, invocation,
> - "Admin op change filter: parameters are missing");
> - goto done;
> - }
> -
> - if (strcmp(param->paramName, "saLogStreamSeverityFilter") != 0) {
> - report_om_error(immOiHandle, invocation,
> - "Admin op change filter, invalid param name");
> - goto done;
> - }
> -
> - if (param->paramType != SA_IMM_ATTR_SAUINT32T) {
> - report_om_error(immOiHandle, invocation,
> - "Admin op change filter: invalid parameter type");
> - goto done;
> - }
> -
> - severityFilter = *((SaUint32T *)param->paramBuffer);
> -
> - if (severityFilter > 0x7f) { /* not a level, a bitmap */
> - report_om_error(immOiHandle, invocation,
> - "Admin op change filter: invalid severity");
> - goto done;
> - }
> -
> - if (severityFilter == stream->severityFilter) {
> - ais_rc = immutil_saImmOiAdminOperationResult(immOiHandle,
> invocation,
> - SA_AIS_ERR_NO_OP);
> - // TODO(Lennart) May be relevant to recover OI if BAD HANDLE
> - if (ais_rc != SA_AIS_OK) {
> - LOG_ER("immutil_saImmOiAdminOperationResult failed %s",
> - saf_error(ais_rc));
> - osaf_abort(0);
> - }
> -
> - goto done;
> - }
> -
> - TRACE("Changing severity for stream %s to %u", stream->name.c_str(),
> - severityFilter);
> - stream->severityFilter = severityFilter;
> -
> - ais_rc = immutil_update_one_rattr(
> - immOiHandle, objName,
> - const_cast<SaImmAttrNameT>("saLogStreamSeverityFilter"),
> - SA_IMM_ATTR_SAUINT32T, &stream->severityFilter);
> - if (ais_rc != SA_AIS_OK) {
> - LOG_ER("%s: immutil_update_one_rattr failed %s", __FUNCTION__,
> - saf_error(ais_rc));
> - osaf_abort(0);
> - }
> -
> - ais_rc =
> - immutil_saImmOiAdminOperationResult(immOiHandle, invocation,
> SA_AIS_OK);
> - // TODO(Lennart) May be relevant to recover OI if BAD HANDLE
> - if (ais_rc != SA_AIS_OK) {
> - LOG_ER("immutil_saImmOiAdminOperationResult failed %s",
> - saf_error(ais_rc));
> - osaf_abort(0);
> + } 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);
> }
> -
> - /* Send changed severity filter to clients */
> - lgs_send_severity_filter_to_clients(stream->streamId,
severityFilter);
> -
> - /* Checkpoint to standby LOG server */
> - ckpt_stream_config(stream);
> } else {
> - report_om_error(
> - immOiHandle, invocation,
> - "Invalid operation ID, should be %d (one) for change filter",
> - SA_LOG_ADMIN_CHANGE_FILTER);
> + report_om_error( immOiHandle, invocation, "Invalid operation ID");
> }
>
> -done:
> TRACE_LEAVE();
> }
>
> @@ -3401,3 +3315,129 @@ done:
> TRACE_LEAVE();
> return rc_attr_val;
> }
> +
> +/**
> + * Handle admin operation that changes the severity filter
> + *
> + * @param immOiHandle
> + * @param invocation
> + * @param objectName
> + * @param stream
> + * @param parameter
> + */
> +static void lgs_admin_change_filter(
> + SaImmOiHandleT immOiHandle, SaInvocationT invocation,
> + SaConstStringT objectName, const SaImmAdminOperationParamsT_2
> *parameter) {
> + SaAisErrorT rc = SA_AIS_OK;
> + TRACE_ENTER();
> +
> + osafassert(objectName != nullptr);
> + log_stream_t *stream = log_stream_get_by_name(objectName);
> +
> + if (parameter == nullptr) {
> + // No parameters given in admin op
> + report_om_error(immOiHandle, invocation,
> + "Admin op change filter: parameters are missing");
> + return;
> + }
> +
> + SaUint32T severityFilter =
> + *(reinterpret_cast<SaUint32T *>(parameter->paramBuffer));
> +
> + if (stream == nullptr) {
> + report_om_error(immOiHandle, invocation, "Stream %s not found",
> objectName);
> + } else if (stream->streamType != STREAM_TYPE_APPLICATION_RT) {
> + // Only allowed to update runtime objects (application streams)
> + report_om_error(immOiHandle, invocation,
> + "Admin op change filter for non runtime app stream");
> + } else if (strcmp(parameter->paramName, "saLogStreamSeverityFilter") !=
> 0) {
> + report_om_error(immOiHandle, invocation,
> + "Admin op change filter, invalid param name");
> + } else if (parameter->paramType != SA_IMM_ATTR_SAUINT32T) {
> + report_om_error(immOiHandle, invocation,
> + "Admin op change filter: invalid parameter type");
> + } else if (severityFilter > 0x7f) {
> + report_om_error(immOiHandle, invocation,
> + "Admin op change filter: invalid severity");
> + } else if (severityFilter == stream->severityFilter) {
> + rc = immutil_saImmOiAdminOperationResult(immOiHandle, invocation,
> + SA_AIS_ERR_NO_OP);
> + if (rc == SA_AIS_ERR_BAD_HANDLE) {
> + lgsOiCreateBackground();
> + } else if (rc != SA_AIS_OK) {
> + LOG_ER("immutil_saImmOiAdminOperationResult failed %s",
> saf_error(rc));
> + osaf_abort(0);
> + }
> + } else {
> + TRACE("Changing severity for stream %s to %u",
> + stream->name.c_str(), severityFilter);
> + stream->severityFilter = severityFilter;
> +
> + rc = immutil_update_one_rattr(immOiHandle, objectName,
> +
const_cast<SaImmAttrNameT>("saLogStreamSeverityFilter"),
> + SA_IMM_ATTR_SAUINT32T, &stream->severityFilter);
> + if (rc == SA_AIS_ERR_BAD_HANDLE) {
> + lgsOiCreateBackground();
> + } else if (rc != SA_AIS_OK) {
> + LOG_ER("%s: update_one_rattr failed %s", __FUNCTION__,
> saf_error(rc));
> + osaf_abort(0);
> + }
> +
> + rc = immutil_saImmOiAdminOperationResult(immOiHandle, invocation,
> + SA_AIS_OK);
> + if (rc == SA_AIS_ERR_BAD_HANDLE) {
> + lgsOiCreateBackground();
> + } else if (rc != SA_AIS_OK) {
> + LOG_ER("immutil_saImmOiAdminOperationResult failed %s",
> saf_error(rc));
> + osaf_abort(0);
> + }
> +
> + // Send changed severity filter to all clients that subscribed the
callback
> + lgs_send_severity_filter_to_clients(stream->streamId,
severityFilter);
> +
> + // Checkpoint to standby LOG server
> + ckpt_stream_config(stream);
> + }
> + TRACE_LEAVE();
> +}
> +
> +/**
> + * Handle admin operation that rotates the log file
> + *
> + * @param immOiHandle
> + * @param invocation
> + * @param objectName
> + */
> +static void lgs_admin_rotate_log_file(SaImmOiHandleT immOiHandle,
> + SaInvocationT invocation,
> + SaConstStringT objectName) {
> + SaAisErrorT rc = SA_AIS_OK;
> + TRACE_ENTER();
> +
> + osafassert(objectName != nullptr);
> + log_stream_t *stream = log_stream_get_by_name(objectName);
> +
> + if (stream == nullptr) {
> + report_om_error(immOiHandle, invocation, "Stream %s not found",
> objectName);
[Vu] if we do 'return here', can remove the else below.
> + } else {
> + // 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;
> +
> + rc = immutil_saImmOiAdminOperationResult(immOiHandle, invocation,
> result);
[Vu] re-use the 'result' variable here instead of defining a new one.
result = immutil_saImmOiAdminOperationResult(immOiHandle, invocation,
result);
if (result == SA_AIS_ERR_BAD_HANDLE)
> + if (rc == SA_AIS_ERR_BAD_HANDLE) {
> + lgsOiCreateBackground();
> + } else if (rc != SA_AIS_OK) {
> + LOG_ER("immutil_saImmOiAdminOperationResult failed %s",
> saf_error(rc));
> + osaf_abort(0);
> + }
> + }
> + TRACE_LEAVE();
> +}
> diff --git a/src/log/logd/lgs_mbcsv.cc b/src/log/logd/lgs_mbcsv.cc
> index 8f66fb926..cd3d70009 100644
> --- a/src/log/logd/lgs_mbcsv.cc
> +++ b/src/log/logd/lgs_mbcsv.cc
> @@ -2316,6 +2316,7 @@ static uint32_t ckpt_proc_cfg_stream(lgs_cb_t
> *cb, void *data) {
> char *dest_names = nullptr;
> SaUint32T severityFilter;
> char *logFileCurrent;
> + bool new_cfg_file_needed = false;
>
> TRACE_ENTER();
>
> @@ -2370,9 +2371,16 @@ static uint32_t ckpt_proc_cfg_stream(lgs_cb_t
> *cb, void *data) {
> goto done;
> }
>
> + if ((stream->maxLogFileSize != maxLogFileSize) ||
> + (stream->fixedLogRecordSize != fixedLogRecordSize) ||
> + (stream->logFullAction != logFullAction) ||
> + (stream->maxFilesRotated != maxFilesRotated) ||
> + (strcmp(stream->logFileFormat, logFileFormat) != 0) ||
> + (stream->fileName != fileName))
> + new_cfg_file_needed = true;
[Vu] should assign the whole condition in if to `new_cfg_file_needed` as:
new_cfg_file_needed = (stream->maxLogFileSize != maxLogFileSize) ||
(stream->fixedLogRecordSize != fixedLogRecordSize) ||
(stream->logFullAction != logFullAction) ||
(stream->maxFilesRotated != maxFilesRotated) ||
(strcmp(stream->logFileFormat, logFileFormat) != 0) ||
(stream->fileName != fileName);
> +
> TRACE("config stream %s, id: %u", stream->name.c_str(), stream-
> >streamId);
> stream->act_last_close_timestamp = closetime; /* Not used if ver 1 */
> - stream->fileName = fileName;
> stream->maxLogFileSize = maxLogFileSize;
> stream->fixedLogRecordSize = fixedLogRecordSize;
> stream->logFullAction = logFullAction;
> @@ -2408,22 +2416,43 @@ static uint32_t ckpt_proc_cfg_stream(lgs_cb_t
> *cb, void *data) {
> }
> }
>
> - /* If split file mode, update standby files */
> + // If split file system mode, update standby files
> if (lgs_is_split_file_system()) {
> - int rc = 0;
> std::string root_path =
> static_cast<const char
> *>(lgs_cfg_get(LGS_IMM_LOG_ROOT_DIRECTORY));
> - if ((rc = log_stream_config_change(LGS_STREAM_CREATE_FILES,
> root_path,
> - stream,
stream->stb_logFileCurrent,
> - &closetime)) != 0) {
> - LOG_WA("log_stream_config_change failed: %d", rc);
> + if (new_cfg_file_needed) {
> + int rc = 0;
> + // Change config file if need
> + if ((rc = log_stream_config_change(!LGS_STREAM_CREATE_FILES,
> root_path,
> + stream,
stream->stb_logFileCurrent,
> + &closetime)) != 0)
> + LOG_WA("log_stream_config_change failed: %d", rc);
> +
> + stream->fileName = fileName;
> +
> + if ((rc = lgs_create_config_file_h(root_path, stream)) != 0)
> + LOG_WA("lgs_create_config_file_h failed: %d", rc);
> +
> + stream->stb_logFileCurrent = stream->logFileCurrent;
> +
> + // Create the new log file based on updated configuration
> + *stream->p_fd = log_file_open(root_path, stream,
> + stream->stb_logFileCurrent, NULL);
> + if (*stream->p_fd == -1)
> + LOG_WA("New log file could not be created for stream: %s",
> + stream->name.c_str());
> + } else if (stream->stb_prev_actlogFileCurrent != logFileCurrent) {
> + // If there is no changed attributes that needs to change cfg file
> + // but the current log file has already changed in ACTIVE, the log
file
> + // has already rotated in ACTIVE. Should also rotate in STANDBY
> + if (log_rotation_stb(stream) != 0)
> + LOG_WA("Rotate log file fails");
> }
>
> - /* When modifying old files are closed and new are opened meaning
that
> - * we have a new standby current log-file
> - */
> stream->stb_prev_actlogFileCurrent = stream->logFileCurrent;
> - stream->stb_logFileCurrent = stream->logFileCurrent;
> + } else {
> + stream->fileName = fileName;
> + stream->logFileCurrent = logFileCurrent;
> }
>
> done:
> diff --git a/src/log/logd/lgs_stream.cc b/src/log/logd/lgs_stream.cc
> index 28344c8cd..d31ae170b 100644
> --- a/src/log/logd/lgs_stream.cc
> +++ b/src/log/logd/lgs_stream.cc
> @@ -1043,10 +1043,9 @@ done:
> * This handler shall be used on standby only
> *
> * @param stream
> - * @param count
> - * @return
> + * @return -1 on error
> */
> -static int log_rotation_stb(log_stream_t *stream, size_t count) {
> +int log_rotation_stb(log_stream_t *stream) {
> int rc = 0;
> int errno_save;
> int errno_ret;
> @@ -1058,8 +1057,6 @@ static int log_rotation_stb(log_stream_t *stream,
> size_t count) {
>
> TRACE_ENTER();
>
> - stream->stb_curFileSize += count;
> -
> /* If active node has rotated files there is a new "logFileCurrent"
> * (check pointed). Standby always follows active rotation.
> *
> @@ -1068,85 +1065,83 @@ static int log_rotation_stb(log_stream_t
> *stream, size_t count) {
> * as described above.
> */
>
> - /* XXX Should be handled...??
> - * If local rotate has been done within one second before active is
rotating
> + /* If local rotate has been done within one second before active is
rotating
> * the file name in stb_logFileCurrent and logFileCurrent may be the
same!
> * This means that a rotation on active when standby is running is not
> * guaranteed. This situation though is very unlikely.
> */
> if (stream->logFileCurrent != stream->stb_prev_actlogFileCurrent) {
> TRACE("Active has rotated (follow active)");
> - /* Active has rotated */
> + // Active has rotated
> stream->stb_prev_actlogFileCurrent = stream->logFileCurrent;
> - /* Use close time from active for renaming of closed file
> - */
> -
> + // Use close time from active for renaming of closed file
> current_time_str = lgs_get_time(&stream->act_last_close_timestamp);
> do_rotate = true;
> - /* Use same name as active for new current log file */
> + // Use same name as active for new current log file
> new_current_log_filename = stream->logFileCurrent;
> } else if (stream->stb_curFileSize > stream->maxLogFileSize) {
> TRACE("Standby has rotated (filesize)");
> - /* Standby current log file has reached max limit */
> - /* Use internal time since active has not yet rotated */
> + // Standby current log file has reached max limit */
> + // Use internal time since active has not yet rotated
> current_time_str = lgs_get_time(NULL);
> do_rotate = true;
> - /* Create new name for current log file based on local time */
> + // Create new name for current log file based on local time
> new_current_log_filename = stream->fileName + "_" + current_time_str;
> }
>
> if (do_rotate) {
> - /* Time to use as "close time stamp" and "open time stamp" for new
> - * log file as created by active.
> - */
> -
> - /* Close current log file */
> - rc = fileclose_h(*stream->p_fd, &errno_ret);
> - *stream->p_fd = -1;
> - if (rc == -1) {
> - LOG_NO("close FAILED: %s", strerror(errno_ret));
> - goto done;
> + // Close current log file
> + if (*stream->p_fd != -1) {
> + rc = fileclose_h(*stream->p_fd, &errno_ret);
> + *stream->p_fd = -1;
> + if (rc == -1) {
> + LOG_NO("Close log file failed: %s", strerror(errno_ret));
> + return rc;
> + }
> }
>
> std::string emptyStr = "";
> - /* Rename file to give it the "close timestamp" */
> + // Rename file to give it the "close timestamp"
> rc = lgs_file_rename_h(root_path, stream->pathName,
> stream->stb_logFileCurrent, current_time_str,
> LGS_LOG_FILE_EXT, &emptyStr);
> - if (rc == -1) goto done;
> + if (rc == -1) {
> + LOG_NO("Rename log file failed");
> + return rc;
> + }
>
> - /* Remove oldest file if needed */
> + // Remove oldest file if needed
> if (rotate_if_needed(stream) == -1) {
> TRACE("Old file removal failed");
> }
> -
> - /* Save new name for current log file and open it */
> + // Save new name for current log file and open it
> stream->stb_logFileCurrent = new_current_log_filename;
> + // Reset file size
> + stream->stb_curFileSize = 0;
> +
> if ((*stream->p_fd = log_file_open(
> root_path, stream, stream->stb_logFileCurrent, &errno_save))
==
> -1) {
> LOG_IN("Could not open '%s' - %s",
stream->stb_logFileCurrent.c_str(),
> strerror(errno_save));
> rc = -1;
> - goto done;
> }
> -
> - stream->stb_curFileSize = 0;
> }
>
> -done:
> TRACE_LEAVE();
> return rc;
> }
>
> /**
> - * Handle log file rotation. Do rotation if needed
> + * Handle log file rotation in ACTIVE node
> + *
> + * Step1: Close the log file and create a new one.
> + * Step2: If number of files > max number of files, deleting the oldest
> *
> * @param stream
> - * @param count
> * @return -1 on error
> */
> -static int log_rotation_act(log_stream_t *stream, size_t count) {
> +int log_rotation_act(log_stream_t *stream) {
> int rc;
> int errno_save;
> int errno_ret;
> @@ -1155,58 +1150,49 @@ static int log_rotation_act(log_stream_t
> *stream, size_t count) {
> static_cast<const char
> *>(lgs_cfg_get(LGS_IMM_LOG_ROOT_DIRECTORY));
> std::string emptyStr = "";
>
> - /* If file size > max file size:
> - * - Close the log file and create a new.
> - * - If number of files > max number of files delete the oldest
> - */
> - rc = 0;
> - stream->curFileSize += count;
> -
> - if ((stream->curFileSize) > stream->maxLogFileSize) {
> - osaf_clock_gettime(CLOCK_REALTIME, &closetime_tspec);
> - time_t closetime = closetime_tspec.tv_sec;
> - char *current_time = lgs_get_time(&closetime);
> + TRACE_ENTER();
> + osaf_clock_gettime(CLOCK_REALTIME, &closetime_tspec);
> + time_t closetime = closetime_tspec.tv_sec;
> + char *current_time = lgs_get_time(&closetime);
>
> - /* Close current log file */
> + // Close current log file
> + if (*stream->p_fd != -1) {
> rc = fileclose_h(*stream->p_fd, &errno_ret);
> *stream->p_fd = -1;
> if (rc == -1) {
> - LOG_NO("close FAILED: %s", strerror(errno_ret));
> - goto done;
> + LOG_NO("Close log file failed: %s", strerror(errno_ret));
> + return rc;
> }
> + }
>
> - /* Rename file to give it the "close timestamp" */
> - rc = lgs_file_rename_h(root_path, stream->pathName, stream-
> >logFileCurrent,
> - current_time, LGS_LOG_FILE_EXT, &emptyStr);
> - if (rc == -1) goto done;
> -
> - /* Save time when logFileCurrent was closed */
> - stream->act_last_close_timestamp = closetime;
> -
> - /* Invalidate logFileCurrent for this stream */
> - stream->logFileCurrent[0] = 0;
> -
> - /* Reset file size for current log file */
> - stream->curFileSize = 0;
> + // Rename file to give it the "close timestamp"
> + rc = lgs_file_rename_h(root_path, stream->pathName, stream-
> >logFileCurrent,
> + current_time, LGS_LOG_FILE_EXT, &emptyStr);
> + if (rc == -1) {
> + LOG_NO("Rename log file failed");
> + return rc;
> + }
>
> - /* Remove oldest file if needed */
> - if (rotate_if_needed(stream) == -1) {
> - TRACE("Old file removal failed");
> - }
> + // Save time when logFileCurrent was closed
> + stream->act_last_close_timestamp = closetime;
> + // Invalidate logFileCurrent for this stream
> + stream->logFileCurrent[0] = 0;
> + // Reset file size for current log file
> + stream->curFileSize = 0;
> + // Remove oldest file if needed
> + if (rotate_if_needed(stream) == -1)
> + TRACE("Old file removal failed");
>
> - /* Create a new file name that includes "open time stamp" and open
the
> file
> - */
> - stream->logFileCurrent = stream->fileName + "_" + current_time;
> - if ((*stream->p_fd = log_file_open(
> - root_path, stream, stream->logFileCurrent, &errno_save)) ==
-1) {
> - LOG_IN("Could not open '%s' - %s", stream->logFileCurrent.c_str(),
> - strerror(errno_save));
> - rc = -1;
> - goto done;
> - }
> + // Create a new file name that includes "open time stamp" and open the
> file
> + stream->logFileCurrent = stream->fileName + "_" + current_time;
> + if ((*stream->p_fd = log_file_open(
> + root_path, stream, stream->logFileCurrent, &errno_save)) == -1) {
> + LOG_IN("Could not open '%s' - %s", stream->logFileCurrent.c_str(),
> + strerror(errno_save));
> + rc = -1;
> }
>
> -done:
> + TRACE_LEAVE();
> return rc;
> }
>
> @@ -1328,9 +1314,12 @@ int log_stream_write_h(log_stream_t *stream,
> const char *buf, size_t count) {
> * for split file system.
> */
> if (lgs_cb->ha_state == SA_AMF_HA_ACTIVE) {
> - rc = log_rotation_act(stream, count);
> + stream->curFileSize += count;
> + if (stream->curFileSize > stream->maxLogFileSize)
> + rc = log_rotation_act(stream);
> } else {
> - rc = log_rotation_stb(stream, count);
> + stream->stb_curFileSize += count;
> + rc = log_rotation_stb(stream);
> }
>
> done:
> @@ -1545,6 +1534,8 @@ int log_stream_config_change(bool create_files_f,
> const std::string &root_path,
> }
> }
>
> + rotate_if_needed(stream);
> +
> /* 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 ad8c7412f..2aa6922f4 100644
> --- a/src/log/logd/lgs_stream.h
> +++ b/src/log/logd/lgs_stream.h
> @@ -147,4 +147,7 @@ void log_stream_form_dest_names(log_stream_t
> *stream);
>
> 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);
> +
> #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
