Hi Mahesh Ack with comments
I have not found any big issues but cppcheck reports some problems. I have tested with parent changeset 8611:15ea36eab483 '2017-02-23 13:44:28,233 INFO - *** List of cppcheck errors added by patch ***' '2017-02-23 13:44:28,233 INFO - *** [src/log/logd/lgs_filehdl.cc:577]: (style) Variable 'len' is assigned a value that is never used.' '2017-02-23 13:44:28,233 INFO - *** [src/log/logd/lgs_stream.cc:1337]: (style) Unsigned variable 'id' can't be negative so it is unnecessary to test it.' '2017-02-23 13:44:28,234 INFO - *** [src/log/logd/lgs_stream.cc:1350]: (style) The scope of the variable 'tmp' can be reduced.' '2017-02-23 13:44:28,234 INFO - *** [src/log/logd/lgs_stream.cc:1447]: (style) The function 'log_stream_id_print' is never used.' '2017-02-23 13:44:28,234 INFO - *** [src/log/logd/lgs_stream.cc:408]: (style) The function 'log_free_stream_resources' is never used.' '2017-02-23 13:44:28,234 INFO - *** [src/log/logd/lgs_stream.cc:416]: (warning) Assignment of function parameter has no effect outside the function. Did you forget dereferencing it?' '2017-02-23 13:44:28,235 INFO - *** [src/log/logd/lgs_stream.cc:533]: (portability) Non reentrant function 'strtok' called. For threadsafe applications it is recommended to use the reentrant replacement function 'strtok_r'.' '2017-02-23 13:44:28,235 INFO - *** [src/log/logd/lgs_stream.cc:852]: (style) Obsolescent function 'usleep' called. It is recommended to use 'nanosleep' or 'setitimer' instead.' '2017-02-23 13:44:28,235 INFO - *** [src/log/logd/lgs_stream.cc:869]: (style) Obsolescent function 'usleep' called. It is recommended to use 'nanosleep' or 'setitimer' instead.' Also, would it be possible to add new test case for this to logtest? Thanks Lennart > -----Original Message----- > From: [email protected] [mailto:[email protected]] > Sent: den 6 februari 2017 07:42 > To: Canh Van Truong <[email protected]>; Lennart Lund > <[email protected]>; Vu Minh Nguyen > <[email protected]> > Cc: [email protected] > Subject: [PATCH 1 of 1] logd: implement cfg files rotation [#2239] > > src/log/logd/lgs_file.cc | 4 + > src/log/logd/lgs_file.h | 1 + > src/log/logd/lgs_filehdl.cc | 182 > ++++++++++++++++++++++++++++++++++++++++++- > src/log/logd/lgs_filehdl.h | 2 + > src/log/logd/lgs_stream.cc | 88 +++++++++++++++++++- > 5 files changed, 262 insertions(+), 15 deletions(-) > > > Requirement : > > Current implementation of LOGsv do rotation of log files as defined, but not > for cfg files. > So, if we play with log streams application in a long time a large numbers of > cfg files are generated , > which is causing system space issue. > > Implementation : > > SAI-AIS-LOG-A.02.01 specification not defied the approach on .cfg ROTATE > policy, > so it is implementation specific , this enhancement taken approach as CFG > file can also be ROTATED concurrently > whenever the log files with the same <closetime> suffix is being ROTATED. > > This enhancement ensures Until the old log files is not deleted and the old > .cfg file is still available since it contains information about how > to read the old log files, in other words when the last old log file has been > rotated out also the old .cfg > file will be removed (but not before that). > > Notes : README will be update if LOG developers thinks it is required > > diff --git a/src/log/logd/lgs_file.cc b/src/log/logd/lgs_file.cc > --- a/src/log/logd/lgs_file.cc > +++ b/src/log/logd/lgs_file.cc > @@ -152,6 +152,10 @@ static void *file_hndl_thread(void *nopa > hndl_rc = get_number_of_log_files_hdl(lgs_com_data.indata_ptr, > lgs_com_data.outdata_ptr, > lgs_com_data.outdata_size); > break; > + case LGSF_GET_NUM_CFGFILES: > + hndl_rc = get_number_of_cfg_files_hdl(lgs_com_data.indata_ptr, > + lgs_com_data.outdata_ptr, > lgs_com_data.outdata_size); > + break; > case LGSF_MAKELOGDIR: > hndl_rc = make_log_dir_hdl(lgs_com_data.indata_ptr, > lgs_com_data.outdata_ptr, > lgs_com_data.outdata_size); > diff --git a/src/log/logd/lgs_file.h b/src/log/logd/lgs_file.h > --- a/src/log/logd/lgs_file.h > +++ b/src/log/logd/lgs_file.h > @@ -44,6 +44,7 @@ typedef enum { > LGSF_CHECKDIR, > LGSF_OWN_LOGFILES, > LGSF_GET_FILE_PAR, > + LGSF_GET_NUM_CFGFILES, > LGSF_NOREQ > }lgsf_treq_t; > > diff --git a/src/log/logd/lgs_filehdl.cc b/src/log/logd/lgs_filehdl.cc > --- a/src/log/logd/lgs_filehdl.cc > +++ b/src/log/logd/lgs_filehdl.cc > @@ -531,7 +531,7 @@ int delete_file_hdl(void *indata, void * > > /********************************************************** > ******************** > * Utility functions for get_number_of_log_files_hdl > */ > -static int check_oldest(char *line, char *fname_prefix, int > fname_prefix_size, int *old_date, int *old_time) { > +static int check_log_oldest(char *line, char *fname_prefix, int > fname_prefix_size, int *old_date, int *old_time) { > int date, time, c, d; > date = time = c = d = 0; > int len = 0; > @@ -558,15 +558,50 @@ static int check_oldest(char *line, char > return 1; > } > } else { > - TRACE_3("no match"); > + TRACE_3("no log_oldest match"); > } > return 0; > } > > -/* Filter function used by scandir. */ > +/********************************************************* > ********************* > + * Utility functions for get_number_of_cfg_files_hdl > + */ > +static int check_cfg_oldest(char *line, char *fname_prefix, int > fname_prefix_size, int *old_date, int *old_time) { > + int date, time; > + date = time = 0; > + int len = 0; > + std::string name_format; > + char time_stamps[] = "_%d_%d.cfg"; > + > + len = strlen(time_stamps); > + len += fname_prefix_size; > + > + name_format = std::string(fname_prefix); > + TRACE_3("fname: %s", name_format.c_str()); > + > + name_format = name_format + time_stamps; > + if (sscanf(line, name_format.c_str(), &date, &time) >= 1) { > + TRACE_3("%s: line: arg 1: %d 2: %d ok", __FUNCTION__, > + date, time); > + if (date < *old_date || *old_date == -1) { > + *old_date = date; > + *old_time = time; > + return 1; > + } else if ((date == *old_date) && (time < *old_time)) { > + *old_date = date; > + *old_time = time; > + return 1; > + } > + } else { > + TRACE_3("no cfg_oldest match"); > + } > + return 0; > +} > + > +/* Log Filter function used by scandir. */ > static std::string file_prefix; > > -static int filter_func(const struct dirent *finfo) { > +static int log_filter_func(const struct dirent *finfo) { > int ret; > int filenameLen = strlen(finfo->d_name) - strlen(".log"); > > @@ -576,6 +611,17 @@ static int filter_func(const struct dire > return !ret; > } > > +/* Cfg Filter function used by scandir. */ > +static int cfg_filter_func(const struct dirent *finfo) { > + int ret; > + int filenameLen = strlen(finfo->d_name) - strlen(".cfg"); > + > + ret = strncmp(file_prefix.c_str(), finfo->d_name, file_prefix.size()) > + || strcmp(finfo->d_name + filenameLen, ".cfg"); > + > + return !ret; > +} > + > /** > * Return number of log files in a dir and the name of the oldest file. > * @param indata, see gnolfh_in_t > @@ -602,7 +648,7 @@ int get_number_of_log_files_hdl(void *in > path = std::string(params_in->logsv_root_dir) + "/" + params_in- > >pathName; > > osaf_mutex_unlock_ordie(&lgs_ftcom_mutex); /* UNLOCK critical section > */ > - files = n = scandir(path.c_str(), &namelist, filter_func, alphasort); > + files = n = scandir(path.c_str(), &namelist, log_filter_func, alphasort); > osaf_mutex_lock_ordie(&lgs_ftcom_mutex); /* LOCK after critical section > */ > > if (n == -1 && errno == ENOENT) { > @@ -623,7 +669,7 @@ int get_number_of_log_files_hdl(void *in > > while (n--) { > TRACE_3("%s", namelist[n]->d_name); > - if (check_oldest(namelist[n]->d_name, params_in->file_name, > + if (check_log_oldest(namelist[n]->d_name, params_in->file_name, > strlen(params_in->file_name), &old_date, &old_time)) { > old_ind = n; > } else { > @@ -657,6 +703,128 @@ done_exit: > } > > /** > + * Return number of cfg files in a dir and the name of the oldest file. > + * @param indata, see gnolfh_in_t > + * @param outdata, char *oldest_file > + * @param max_outsize, Max size for oldest_file string > + * > + * @return int, number of cfgfiles or -1 if error > + */ > +int get_number_of_cfg_files_hdl(void *indata, void *outdata, size_t > max_outsize) { > + struct dirent **cfg_namelist; > + struct dirent **log_namelist; > + int n, cfg_old_date = -1, cfg_old_time = -1, old_ind = -1, cfg_files = 0, > i, > failed = 0; > + int log_old_date = -1, log_old_time = -1 , log_files = 0; > + std::string path; > + gnolfh_in_t *params_in; > + char *oldest_file; > + int rc = 0; > + > + TRACE_ENTER(); > + > + params_in = static_cast<gnolfh_in_t *>(indata); > + oldest_file = static_cast<char *>(outdata); > + /* Initialize the filter */ > + file_prefix = params_in->file_name; > + path = std::string(params_in->logsv_root_dir) + "/" + params_in- > >pathName; > + > + osaf_mutex_unlock_ordie(&lgs_ftcom_mutex); /* UNLOCK critical section > */ > + log_files = n = scandir(path.c_str(), &log_namelist, log_filter_func, > alphasort); > + osaf_mutex_lock_ordie(&lgs_ftcom_mutex); /* LOCK after critical section > */ > + > + if (n == -1 && errno == ENOENT) { > + rc = 0; > + goto done_exit; > + } > + > + if (n < 0) { > + LOG_WA("scandir:%s - %s", strerror(errno), path.c_str()); > + rc = -1; > + goto done_exit; > + } > + > + if (n == 0) { > + rc = log_files; > + goto done_exit; > + } > + > + while (n--) { > + TRACE_3("%s", log_namelist[n]->d_name); > + if (check_log_oldest(log_namelist[n]->d_name, params_in->file_name, > + strlen(params_in->file_name), &log_old_date, > &log_old_time)) { > + old_ind = n; > + } else { > + failed++; /* wrong format */ > + } > + } > + > + if (old_ind != -1) { > + n = 0; > + old_ind = -1; > + failed = 0; > + osaf_mutex_unlock_ordie(&lgs_ftcom_mutex); /* UNLOCK critical > section */ > + cfg_files = n = scandir(path.c_str(), &cfg_namelist, cfg_filter_func, > alphasort); > + osaf_mutex_lock_ordie(&lgs_ftcom_mutex); /* LOCK after critical > section */ > + > + if (n == -1 && errno == ENOENT) { > + rc = 0; > + goto done_log_free; > + } > + > + if (n < 0) { > + LOG_WA("scandir:%s - %s", strerror(errno), path.c_str()); > + rc = -1; > + goto done_log_free; > + } > + > + if (n == 0) { > + rc = cfg_files; > + goto done_log_free; > + } > + > + while (n--) { > + if (check_cfg_oldest(cfg_namelist[n]->d_name, params_in->file_name, > + strlen(params_in->file_name), &cfg_old_date, > &cfg_old_time)) { > + old_ind = n; > + } else { > + failed++; /* wrong format */ > + } > + } > + > + if ((old_ind != -1) && (cfg_old_date == log_old_date) && (cfg_old_time > <= log_old_time)) { > + TRACE_1(" (cfg_old_date:%d == log_old_date:%d) && (cfg_old_time:%d > <= log_old_time:%d )", cfg_old_date, log_old_date, cfg_old_time, > log_old_time); > + TRACE_1("oldest: %s", cfg_namelist[old_ind]->d_name); > + n = snprintf(oldest_file, max_outsize, "%s/%s", > + path.c_str(), cfg_namelist[old_ind]->d_name); > + > + if (n < 0 || static_cast<uint32_t>(n) >= max_outsize) { > + LOG_WA("oldest_file > max_outsize"); > + rc = -1; > + goto done_cfg_free; > + } else { > + rc = (cfg_files - failed); > + } > + } > + } > + > +done_cfg_free: > + /* Free scandir allocated memory */ > + for (i = 0; i < cfg_files; i++) > + free(cfg_namelist[i]); > + free(cfg_namelist); > + > +done_log_free: > + /* Free scandir allocated memory */ > + for (i = 0; i < log_files; i++) > + free(log_namelist[i]); > + free(log_namelist); > + > +done_exit: > + TRACE_LEAVE(); > + return rc; > +} > + > +/** > * Change the ownership of all log files to a new group. > * The input directory will be scanned and any file suffixed by ".log" > * and prefixed by input file_name will be marked as log files. > @@ -681,7 +849,7 @@ int own_log_files_by_group_hdl(void *ind > file_prefix = params_in->file_name; > > osaf_mutex_unlock_ordie(&lgs_ftcom_mutex); /* UNLOCK critical section > */ > - files = n = scandir(params_in->dir_path, &namelist, filter_func, > alphasort); > + files = n = scandir(params_in->dir_path, &namelist, log_filter_func, > alphasort); > > if (n == -1 && errno == ENOENT) { > rc = 0; > diff --git a/src/log/logd/lgs_filehdl.h b/src/log/logd/lgs_filehdl.h > --- a/src/log/logd/lgs_filehdl.h > +++ b/src/log/logd/lgs_filehdl.h > @@ -73,6 +73,7 @@ typedef struct { > > /* > * get_number_of_log_files_hdl(..) > + * get_number_of_cfg_files_hdl(..) > */ > typedef struct { > /* File name prefix (name part before time stamps) */ > @@ -185,6 +186,7 @@ int fileopen_hdl(void *indata, void *out > int fileclose_hdl(void *indata, void *outdata, size_t max_outsize); > int delete_file_hdl(void *indata, void *outdata, size_t max_outsize); > int get_number_of_log_files_hdl(void *indata, void *outdata, size_t > max_outsize); > +int get_number_of_cfg_files_hdl(void *indata, void *outdata, size_t > max_outsize); > int own_log_files_by_group_hdl(void *indata, void *outdata, size_t > max_outsize); > int lgs_get_file_params_hdl(void *indata, void *outdata, size_t > max_outsize); > > diff --git a/src/log/logd/lgs_stream.cc b/src/log/logd/lgs_stream.cc > --- a/src/log/logd/lgs_stream.cc > +++ b/src/log/logd/lgs_stream.cc > @@ -47,6 +47,7 @@ static int lgs_stream_array_insert(log_s > static int lgs_stream_array_insert_new(log_stream_t *stream, uint32_t > *id); > static int lgs_stream_array_remove(int id); > static int get_number_of_log_files_h(log_stream_t *logStream, char > *oldest_file); > +static int get_number_of_cfg_files_h(log_stream_t *logStream, char > *oldest_file); > > /** > * Open/create a file for append in non blocking mode. > @@ -264,32 +265,53 @@ static int delete_config_file(log_stream > * @return -1 on error > */ > static int rotate_if_needed(log_stream_t *stream) { > - char oldest_file[PATH_MAX]; > + char oldest_log_file[PATH_MAX]; > + char oldest_cfg_file[PATH_MAX]; > int rc = 0; > - int file_cnt; > + int log_file_cnt, cfg_file_cnt; > + bool oldest_cfg = false; > > TRACE_ENTER(); > > - /* Rotate out files from previous lifes */ > - if ((file_cnt = get_number_of_log_files_h(stream, oldest_file)) == -1) { > + /* Rotate out log files from previous lifes */ > + if ((log_file_cnt = get_number_of_log_files_h(stream, oldest_log_file)) == > -1) { > rc = -1; > goto done; > } > > + /* Rotate out cfg files from previous lifes */ > + if (!((cfg_file_cnt = get_number_of_cfg_files_h(stream, oldest_cfg_file)) > == -1)) { > + oldest_cfg = true; > + } > + > + TRACE("delete oldest_cfg_file: %s oldest_log_file %s", oldest_cfg_file, > oldest_log_file); > + > /* > ** Remove until we have one less than allowed, we are just about to > ** create a new one again. > */ > - while (file_cnt >= static_cast<int>(stream->maxFilesRotated)) { > - if ((rc = file_unlink_h(oldest_file)) == -1) { > - LOG_NO("Could not delete: %s - %s", oldest_file, strerror(errno)); > + while (log_file_cnt >= static_cast<int>(stream->maxFilesRotated)) { > + if ((rc = file_unlink_h(oldest_log_file)) == -1) { > + LOG_NO("Could not log delete: %s - %s", oldest_log_file, > strerror(errno)); > goto done; > } > > - if ((file_cnt = get_number_of_log_files_h(stream, oldest_file)) == -1) { > + if (oldest_cfg == true) { > + oldest_cfg = false; > + if ((rc = file_unlink_h(oldest_cfg_file)) == -1) { > + LOG_NO("Could not cfg delete: %s - %s", oldest_cfg_file, > strerror(errno)); > + goto done; > + } > + } > + > + if ((log_file_cnt = get_number_of_log_files_h(stream, oldest_log_file)) > == -1) { > rc = -1; > goto done; > } > + > + if (!((cfg_file_cnt = get_number_of_cfg_files_h(stream, oldest_cfg_file)) > == -1)) { > + oldest_cfg = true; > + } > } > > done: > @@ -938,6 +960,56 @@ done: > TRACE_LEAVE(); > return rc; > } > +/** > + * Return number of cfg files in a dir and the name of the oldest file. > + * @param logStream[in] > + * @param oldest_file[out] > + * > + * @return -1 if error else number of log files > + */ > +static int get_number_of_cfg_files_h(log_stream_t *logStream, char > *oldest_file) { > + lgsf_apipar_t apipar; > + lgsf_retcode_t api_rc; > + gnolfh_in_t parameters_in; > + size_t path_len; > + int rc = 0; > + > + TRACE_ENTER(); > + > + std::string logsv_root_dir = static_cast<const char *>( > + lgs_cfg_get(LGS_IMM_LOG_ROOT_DIRECTORY)); > + > + parameters_in.file_name = const_cast<char *>(logStream- > >fileName.c_str()); > + parameters_in.logsv_root_dir = const_cast<char > *>(logsv_root_dir.c_str()); > + parameters_in.pathName = const_cast<char *>(logStream- > >pathName.c_str()); > + > + path_len = strlen(parameters_in.file_name) + > + strlen(parameters_in.logsv_root_dir) + > + strlen(parameters_in.pathName); > + if (path_len > PATH_MAX) { > + LOG_WA("Path to cfg files > PATH_MAX"); > + rc = -1; > + goto done; > + } > + > + /* Fill in API structure */ > + apipar.req_code_in = LGSF_GET_NUM_CFGFILES; > + apipar.data_in_size = sizeof(gnolfh_in_t); > + apipar.data_in = ¶meters_in; > + apipar.data_out_size = PATH_MAX; > + apipar.data_out = oldest_file; > + api_rc = log_file_api(&apipar); > + if (api_rc != LGSF_SUCESS) { > + TRACE("%s - API error %s",__FUNCTION__,lgsf_retcode_str(api_rc)); > + rc = -1; > + } else { > + rc = apipar.hdl_ret_code_out; > + } > + > +done: > + TRACE_LEAVE(); > + return rc; > +} > > /** > * Handle log file rotation on standby node. ------------------------------------------------------------------------------ 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
