Hi ,
ACK with following minor comment. NOT tested.
On 12/3/2016 11:35 AM, Canh Van Truong wrote:
> /* first time open? */
> - if (stream->numOpeners == 0) {
> + if (stream->numOpeners == 0 || *stream->p_fd == -1) {
Some compilers will through error if we don't have `( )` for each condition
-AVM
On 12/3/2016 11:35 AM, Canh Van Truong wrote:
> osaf/services/saf/logsv/lgs/lgs_imm.cc | 14 +++++-----
> osaf/services/saf/logsv/lgs/lgs_mbcsv.cc | 2 +-
> osaf/services/saf/logsv/lgs/lgs_stream.cc | 38
> ++++++++++++++----------------
> 3 files changed, 26 insertions(+), 28 deletions(-)
>
>
> Issues:
> This is happen when changing IMM attribute in logs service.
> Following actions will be happen in IMM apply callback:
> 1. closing log file 2. Rename cfg/log file 3. create/open new cfg/log file
>
> In closing action(1), it need time to sync data from cache to disc device
> before
> closing file. The sync data takes long time and cause log service timeout.
> With closing action timeout, we do not know that closing file is successful
> or not.
>
> After closing action(1) timeout, apply callback is returned and new cfg/log
> file are
> not created.
> If closing action timeout but log file was closed successfully, 2 cases may
> be happen:
>
> a. Continue change this attribute with the same stream. Closing file action
> is happen again.
> One request is send to log file handle thread to sync data and closing file
> with the fd that
> could be closed successfull from previous closing file action. That causes
> bad discriptor error.
>
> b. Write log records. Because the new log file has not be created and log
> server still write the
> record to file with old fd that has be closed successfull from previous
> closing file action.
> This also causes bad file discriptor.
>
>
> Solution:
> In step 1 above, after closing log file fail, log service can contine to
> rename and open new file actions.
> Convert ER to WA and add some WA.
>
> diff --git a/osaf/services/saf/logsv/lgs/lgs_imm.cc
> b/osaf/services/saf/logsv/lgs/lgs_imm.cc
> --- a/osaf/services/saf/logsv/lgs/lgs_imm.cc
> +++ b/osaf/services/saf/logsv/lgs/lgs_imm.cc
> @@ -1849,7 +1849,7 @@ void logRootDirectory_filemove(
> if (log_stream_config_change(!LGS_STREAM_CREATE_FILES,
> old_logRootDirectory, stream,
> current_logfile,
> cur_time_in) != 0) {
> - LOG_ER("Old log files could not be renamed and closed for stream: %s",
> + LOG_WA("Old log files could not be renamed and closed for stream: %s",
> stream->name.c_str());
> }
> }
> @@ -1865,7 +1865,7 @@ void logRootDirectory_filemove(
> while ((stream = iterate_all_streams(endloop, jstart)) && !endloop) {
> jstart = SA_FALSE;
> if (lgs_create_config_file_h(new_logRootDirectory, stream) != 0) {
> - LOG_ER("New config file could not be created for stream: %s",
> + LOG_WA("New config file could not be created for stream: %s",
> stream->name.c_str());
> }
>
> @@ -1875,7 +1875,7 @@ void logRootDirectory_filemove(
>
> if ((*stream->p_fd = log_file_open(new_logRootDirectory,
> stream, stream->logFileCurrent,
> NULL)) == -1) {
> - LOG_ER("New log file could not be created for stream: %s",
> + LOG_WA("New log file could not be created for stream: %s",
> stream->name.c_str());
> }
>
> @@ -2325,13 +2325,13 @@ static void stream_ccb_apply_modify(cons
> if ((rc = log_stream_config_change(!LGS_STREAM_CREATE_FILES,
> root_path, stream,
> current_logfile_name, &cur_time))
> != 0) {
> - LOG_ER("log_stream_config_change failed: %d", rc);
> + LOG_WA("log_stream_config_change failed: %d", rc);
> }
>
> stream->fileName = fileName;
>
> if ((rc = lgs_create_config_file_h(root_path, stream)) != 0) {
> - LOG_ER("lgs_create_config_file_h failed: %d", rc);
> + LOG_WA("lgs_create_config_file_h failed: %d", rc);
> }
>
> char *current_time = lgs_get_time(&cur_time);
> @@ -2340,7 +2340,7 @@ static void stream_ccb_apply_modify(cons
> // Create the new log file based on updated configuration
> if ((*stream->p_fd = log_file_open(root_path,
> stream, stream->logFileCurrent,
> NULL)) == -1)
> - LOG_ER("New log file could not be created for stream: %s",
> + LOG_WA("New log file could not be created for stream: %s",
> stream->name.c_str());
> } else if (new_cfg_file_needed) {
> int rc;
> @@ -2348,7 +2348,7 @@ static void stream_ccb_apply_modify(cons
> root_path, stream,
> current_logfile_name, &cur_time))
> != 0) {
> - LOG_ER("log_stream_config_change failed: %d", rc);
> + LOG_WA("log_stream_config_change failed: %d", rc);
> }
> }
>
> diff --git a/osaf/services/saf/logsv/lgs/lgs_mbcsv.cc
> b/osaf/services/saf/logsv/lgs/lgs_mbcsv.cc
> --- a/osaf/services/saf/logsv/lgs/lgs_mbcsv.cc
> +++ b/osaf/services/saf/logsv/lgs/lgs_mbcsv.cc
> @@ -2128,7 +2128,7 @@ static uint32_t ckpt_proc_cfg_stream(lgs
> if ((rc = log_stream_config_change(LGS_STREAM_CREATE_FILES,
> root_path, stream,
> stream->stb_logFileCurrent,
> &closetime)) != 0) {
> - LOG_ER("log_stream_config_change failed: %d", rc);
> + LOG_WA("log_stream_config_change failed: %d", rc);
> }
>
> /* When modifying old files are closed and new are opened meaning that
> diff --git a/osaf/services/saf/logsv/lgs/lgs_stream.cc
> b/osaf/services/saf/logsv/lgs/lgs_stream.cc
> --- a/osaf/services/saf/logsv/lgs/lgs_stream.cc
> +++ b/osaf/services/saf/logsv/lgs/lgs_stream.cc
> @@ -729,11 +729,11 @@ int log_file_open(
> * @param stream
> */
> void log_stream_open_fileinit(log_stream_t *stream) {
> + osafassert(stream != NULL);
> TRACE_ENTER2("%s, numOpeners=%u", stream->name.c_str(),
> stream->numOpeners);
> - osafassert(stream != NULL);
>
> /* first time open? */
> - if (stream->numOpeners == 0) {
> + if (stream->numOpeners == 0 || *stream->p_fd == -1) {
> /* Create and save current log file name */
> stream->logFileCurrent = stream->fileName + "_" + lgs_get_time(NULL);
> log_initiate_stream_files(stream);
> @@ -1415,7 +1415,7 @@ int log_stream_config_change(bool create
> log_stream_t *stream,
> const std::string ¤t_logfile_name,
> time_t *cur_time_in) {
> - int rc;
> + int rc, ret = 0;
> int errno_ret;
> char *current_time = lgs_get_time(cur_time_in);
> std::string emptyStr = "";
> @@ -1433,28 +1433,31 @@ int log_stream_config_change(bool create
> /* close the existing log file, and only when there is a valid fd */
>
> if ((rc = fileclose_h(*stream->p_fd, &errno_ret)) == -1) {
> - LOG_NO("log_stream log file close FAILED: %s", strerror(errno_ret));
> - goto done;
> + LOG_WA("log_stream log file close FAILED: %s", strerror(errno_ret));
> + ret = -1;
> }
> *stream->p_fd = -1;
>
> rc = lgs_file_rename_h(root_path, stream->pathName,
> current_logfile_name,
> current_time, LGS_LOG_FILE_EXT, emptyStr);
> if (rc == -1) {
> - goto done;
> + LOG_WA("log file (%s) is renamed FAILED: %d",
> current_logfile_name.c_str(), rc);
> }
>
> rc = lgs_file_rename_h(root_path, stream->pathName, stream->fileName,
> current_time, LGS_LOG_FILE_CONFIG_EXT, emptyStr);
> if (rc == -1) {
> - goto done;
> + LOG_WA("cfg file (%s) is renamed FAILED: %d",
> stream->fileName.c_str(), rc);
> + ret = -1;
> }
> }
>
> /* Creating the new config file */
> if (create_files_f == LGS_STREAM_CREATE_FILES) {
> - if ((rc = lgs_create_config_file_h(root_path, stream)) != 0)
> - goto done;
> + if ((rc = lgs_create_config_file_h(root_path, stream)) != 0) {
> + LOG_WA("lgs_create_config_file_h (%s) failed: %d",
> stream->fileName.c_str(), rc);
> + ret = -1;
> + }
>
> stream->logFileCurrent = stream->fileName + "_" + current_time;
>
> @@ -1463,20 +1466,15 @@ int log_stream_config_change(bool create
> stream,
> stream->logFileCurrent,
> NULL);
> + if (*stream->p_fd == -1) {
> + LOG_WA("New log file could not be created for stream: %s",
> + stream->name.c_str());
> + ret = -1;
> + }
> }
>
> - /* Fix bug - this function makes return (-1) when create_files_f = false */
> - if (create_files_f == !LGS_STREAM_CREATE_FILES) {
> - rc = 0;
> - } else if (*stream->p_fd == -1) {
> - rc = -1;
> - } else {
> - rc = 0;
> - }
> -
> -done:
> TRACE_LEAVE();
> - return rc;
> + return ret;
> }
>
> /*
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
