Hi Canh

Ack

Thanks
Lennart

> -----Original Message-----
> From: Canh Van Truong [mailto:[email protected]]
> Sent: den 3 december 2016 07:05
> To: Vu Minh Nguyen <[email protected]>; Lennart Lund
> <[email protected]>; [email protected]
> Cc: [email protected]
> Subject: [PATCH 1 of 1] log: fix bad file discriptor error when changing imm
> attributes[#2215]
> 
>  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 &current_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;
>  }
> 
>  /*

------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to