Hi Vu Looks good, Ack
Thanks Lennart > -----Original Message----- > From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au] > Sent: den 22 september 2016 09:17 > To: Lennart Lund <lennart.l...@ericsson.com>; mahesh.va...@oracle.com > Cc: opensaf-devel@lists.sourceforge.net > Subject: [PATCH 1 of 1] log: fix ER no stream exists in syslog [#2043] > > osaf/services/saf/logsv/lgs/lgs_amf.cc | 29 ++++++++------- > osaf/services/saf/logsv/lgs/lgs_config.cc | 10 ++--- > osaf/services/saf/logsv/lgs/lgs_evt.cc | 27 ++++++++------ > osaf/services/saf/logsv/lgs/lgs_imm.cc | 56 ++++++++++++++--------------- > - > osaf/services/saf/logsv/lgs/lgs_mbcsv.cc | 19 ++++------ > osaf/services/saf/logsv/lgs/lgs_stream.cc | 48 ++++++++++++++++++++++- > -- > osaf/services/saf/logsv/lgs/lgs_stream.h | 2 +- > 7 files changed, 110 insertions(+), 81 deletions(-) > > > The `number of streams` refers to total existing log streams in cluster. > And `stream_array` is the database holding all existing log streams. > When interating all log streams, logsv first started at the index `number of > streams`, > if getting NULL, logsv considered that case as `no stream`. This is absolutely > wrong. > > This patch provides other way to iterate all log streams. > > diff --git a/osaf/services/saf/logsv/lgs/lgs_amf.cc > b/osaf/services/saf/logsv/lgs/lgs_amf.cc > --- a/osaf/services/saf/logsv/lgs/lgs_amf.cc > +++ b/osaf/services/saf/logsv/lgs/lgs_amf.cc > @@ -26,13 +26,13 @@ > > static void close_all_files() { > log_stream_t *stream; > - int num = get_number_of_streams(); > - stream = log_stream_get_by_id(--num); > - while (stream != NULL) { > + SaBoolT endloop = SA_FALSE, jstart = SA_TRUE; > + > + // Iterate all existing log streams in cluster. > + while ((stream = iterate_all_streams(endloop, jstart)) && !endloop) { > + jstart = SA_FALSE; > if (log_stream_file_close(stream) != 0) > LOG_WA("Could not close file for stream %s", stream->name.c_str()); > - > - stream = log_stream_get_by_id(--num); > } > } > > @@ -52,7 +52,8 @@ static void close_all_files() { > static SaAisErrorT amf_active_state_handler(lgs_cb_t *cb, SaInvocationT > invocation) { > log_stream_t *stream; > SaAisErrorT error = SA_AIS_OK; > - int num; > + SaBoolT endloop = SA_FALSE, jstart = SA_TRUE; > + uint32_t count = 0; > > TRACE_ENTER2("HA ACTIVE request"); > > @@ -65,15 +66,15 @@ static SaAisErrorT amf_active_state_hand > conf_runtime_obj_create(cb->immOiHandle); > lgs_start_gcfg_applier(); > > - /* check existing streams */ > - num = get_number_of_streams(); > - stream = log_stream_get_by_id(--num); > - if (!stream) > + // Iterate all existing log streams in cluster. > + while ((stream = iterate_all_streams(endloop, jstart)) && !endloop) { > + jstart = SA_FALSE; > + *stream->p_fd = -1; /* First Initialize fd */ > + count++; > + } > + > + if (count == 0) > LOG_ER("No streams exist!"); > - while (stream != NULL) { > - *stream->p_fd = -1; /* First Initialize fd */ > - stream = log_stream_get_by_id(--num); > - } > > done: > /* Update role independent of stream processing */ > diff --git a/osaf/services/saf/logsv/lgs/lgs_config.cc > b/osaf/services/saf/logsv/lgs/lgs_config.cc > --- a/osaf/services/saf/logsv/lgs/lgs_config.cc > +++ b/osaf/services/saf/logsv/lgs/lgs_config.cc > @@ -458,7 +458,7 @@ int lgs_cfg_verify_root_dir(const std::s > int rc = 0; > log_stream_t *stream = NULL; > size_t n = root_str_in.size(); > - int num; > + SaBoolT endloop = SA_FALSE, jstart = SA_TRUE; > > if (n > PATH_MAX) { > LOG_NO("verify_root_dir Fail. Path > PATH_MAX"); > @@ -470,17 +470,15 @@ int lgs_cfg_verify_root_dir(const std::s > * Make sure that the path /rootPath/streamPath/<fileName><tail> > * must not be larger than PATH_MAX. > */ > - num = get_number_of_streams(); > - stream = log_stream_get_by_id(--num); > - while (stream != NULL) { > + // Iterate all existing log streams in cluster. > + while ((stream = iterate_all_streams(endloop, jstart)) && !endloop) { > + jstart = SA_FALSE; > if (lgs_is_valid_pathlength(stream->pathName, stream->fileName, > root_str_in) == false) { > TRACE("The rootPath is invalid (%s)", root_str_in.c_str()); > rc = -1; > goto done; > } > - > - stream = log_stream_get_by_id(--num); > } > > if (lgs_path_is_writeable_dir_h(root_str_in) == false) { > diff --git a/osaf/services/saf/logsv/lgs/lgs_evt.cc > b/osaf/services/saf/logsv/lgs/lgs_evt.cc > --- a/osaf/services/saf/logsv/lgs/lgs_evt.cc > +++ b/osaf/services/saf/logsv/lgs/lgs_evt.cc > @@ -532,14 +532,17 @@ static uint32_t proc_rda_cb_msg(lgsv_lgs > lgs_process_lga_down_list(); > > /* Check existing streams */ > - int num = get_number_of_streams(); > - stream = log_stream_get_by_id(--num); > - if (!stream) > + // Iterate all existing log streams in cluster. > + uint32_t count = 0; > + SaBoolT endloop = SA_FALSE, jstart = SA_TRUE; > + while ((stream = iterate_all_streams(endloop, jstart)) && !endloop) { > + jstart = SA_FALSE; > + *stream->p_fd = -1; /* Initialize fd */ > + count++; > + } > + > + if (count == 0) > LOG_ER("No streams exist!"); > - while (stream != NULL) { > - *stream->p_fd = -1; /* Initialize fd */ > - stream = log_stream_get_by_id(--num); > - } > } > > TRACE_LEAVE(); > @@ -800,7 +803,8 @@ SaAisErrorT create_new_app_stream(lgsv_s > SaBoolT twelveHourModeFlag; > SaUint32T logMaxLogrecsize_conf = 0; > SaConstStringT str_name; > - int num, err = 0; > + int err = 0; > + SaBoolT endloop = SA_FALSE, jstart = SA_TRUE; > const char *dnPrefix = "safLgStr="; > > TRACE_ENTER(); > @@ -862,16 +866,15 @@ SaAisErrorT create_new_app_stream(lgsv_s > } > > /* Verify that path and file are unique */ > - num = get_number_of_streams(); > - stream = log_stream_get_by_id(--num); > - while (stream != NULL) { > + // Iterate all existing log streams in cluster. > + while ((stream = iterate_all_streams(endloop, jstart)) && !endloop) { > + jstart = SA_FALSE; > if ((stream->fileName == open_sync_param->logFileName) && > (stream->pathName == open_sync_param->logFilePathName)) { > TRACE("pathname already exist"); > rc = SA_AIS_ERR_INVALID_PARAM; > goto done; > } > - stream = log_stream_get_by_id(--num); > } > > /* Verify that the name seems to be a DN */ > 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 > @@ -1022,7 +1022,6 @@ bool chk_filepath_stream_exist( > log_stream_t *i_stream = NULL; > std::string i_fileName; > std::string i_pathName; > - int num; > bool rc = false; > > TRACE_ENTER(); > @@ -1072,17 +1071,16 @@ bool chk_filepath_stream_exist( > > /* Check if any stream has given filename and path */ > TRACE("Check if any stream has given filename and path"); > - num = get_number_of_streams(); > - i_stream = log_stream_get_by_id(--num); > - while (i_stream != NULL) { > + // Iterate all existing log streams in cluster. > + SaBoolT endloop = SA_FALSE, jstart = SA_TRUE; > + while ((i_stream = iterate_all_streams(endloop, jstart)) && !endloop) { > + jstart = SA_FALSE; > TRACE("Check stream \"%s\"", i_stream->name.c_str()); > if ((i_stream->fileName == i_fileName) && > (i_stream->pathName == i_pathName)) { > rc = true; > break; > } > - > - i_stream = log_stream_get_by_id(--num); > } > > TRACE_LEAVE2("rc = %d", rc); > @@ -1832,15 +1830,14 @@ void logRootDirectory_filemove( > TRACE_ENTER(); > log_stream_t *stream; > std::string current_logfile; > - int num; > - > + SaBoolT endloop = SA_FALSE, jstart = SA_TRUE; > /* Close and rename files at current path > */ > - num = get_number_of_streams(); > - stream = log_stream_get_by_id(--num); > - while (stream != NULL) { > + // Iterate all existing log streams in cluster. > + while ((stream = iterate_all_streams(endloop, jstart)) && !endloop) { > + jstart = SA_FALSE; > + > TRACE("Handling file %s", stream->logFileCurrent.c_str()); > - > if (lgs_cb->ha_state == SA_AMF_HA_ACTIVE) { > current_logfile = stream->logFileCurrent; > } else { > @@ -1855,21 +1852,22 @@ void logRootDirectory_filemove( > LOG_ER("Old log files could not be renamed and closed for stream: %s", > stream->name.c_str()); > } > - stream = log_stream_get_by_id(--num); > } > > /* Create new files at new path > */ > - num = get_number_of_streams(); > - stream = log_stream_get_by_id(--num); > - while (stream != NULL) { > + char *current_time; > + endloop = SA_FALSE; > + jstart = SA_TRUE; > + 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", > stream->name.c_str()); > } > > /* Create the new log file based on updated configuration */ > - char *current_time = lgs_get_time(cur_time_in); > + current_time = lgs_get_time(cur_time_in); > stream->logFileCurrent = stream->fileName + "_" + current_time; > > if ((*stream->p_fd = log_file_open(new_logRootDirectory, > @@ -1882,8 +1880,8 @@ void logRootDirectory_filemove( > * Used if standby and configured for split file system > */ > stream->stb_logFileCurrent = stream->logFileCurrent; > - stream = log_stream_get_by_id(--num); > } > + > TRACE_LEAVE(); > } > > @@ -1898,7 +1896,6 @@ void logRootDirectory_filemove( > void logDataGroupname_fileown(const char *new_logDataGroupname) { > TRACE_ENTER(); > log_stream_t *stream; > - int num; > > if (new_logDataGroupname == NULL) { > LOG_ER("Data group is NULL"); > @@ -1910,13 +1907,14 @@ void logDataGroupname_fileown(const char > /* Not attribute values deletion > * Change ownership of log files to this new group > */ > - num = get_number_of_streams(); > - stream = log_stream_get_by_id(--num); > - while (stream != NULL) { > + // Iterate all existing log streams in cluster. > + SaBoolT endloop = SA_FALSE, jstart = SA_TRUE; > + while ((stream = iterate_all_streams(endloop, jstart)) && !endloop) { > + jstart = SA_FALSE; > lgs_own_log_files_h(stream, new_logDataGroupname); > - stream = log_stream_get_by_id(--num); > } > } > + > TRACE_LEAVE(); > } > > @@ -2708,10 +2706,10 @@ SaAisErrorT lgs_imm_init_configStreams(l > SaImmAttrValuesT_2 **attributes; > int wellknownStreamId = 0; > int appStreamId = 3; > - int streamId = 0, num; > + uint32_t streamId = 0; > SaNameT objectName; > const char *className = "SaLogStreamConfig"; > - > + SaBoolT endloop = SA_FALSE, jstart = SA_TRUE; > TRACE_ENTER(); > > om_rc = immutil_saImmOmInitialize(&omHandle, NULL, &immVersion); > @@ -2774,9 +2772,9 @@ SaAisErrorT lgs_imm_init_configStreams(l > osaf_abort(0); > } > > - num = get_number_of_streams(); > - stream = log_stream_get_by_id(--num); > - while (stream != NULL) { > + // Iterate all existing log streams in cluster. > + while ((stream = iterate_all_streams(endloop, jstart)) && !endloop) { > + jstart = SA_FALSE; > if (cb->scAbsenceAllowed != 0) { > int_rc = log_stream_open_file_restore(stream); > if (int_rc == -1) { > @@ -2802,8 +2800,6 @@ SaAisErrorT lgs_imm_init_configStreams(l > LOG_ER("immutil_update_one_rattr failed %s", saf_error(ais_rc)); > osaf_abort(0); > } > - > - stream = log_stream_get_by_id(--num); > } > > done: > 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 > @@ -264,7 +264,6 @@ done: > uint32_t lgs_mbcsv_change_HA_state(lgs_cb_t *cb, SaAmfHAStateT > ha_state) { > TRACE_ENTER(); > NCS_MBCSV_ARG mbcsv_arg; > - int num; > > memset(&mbcsv_arg, '\0', sizeof(NCS_MBCSV_ARG)); > > @@ -287,9 +286,10 @@ uint32_t lgs_mbcsv_change_HA_state(lgs_c > */ > log_stream_t *stream; > if (lgs_is_split_file_system()) { > - num = get_number_of_streams(); > - stream = log_stream_get_by_id(--num); > - while (stream != NULL) { /* Iterate over all streams */ > + // Iterate all existing log streams in cluster. > + SaBoolT endloop = SA_FALSE, jstart = SA_TRUE; > + while ((stream = iterate_all_streams(endloop, jstart)) && !endloop) { > + jstart = SA_FALSE; > if (ha_state == SA_AMF_HA_ACTIVE) { > stream->logFileCurrent = stream->stb_logFileCurrent; > stream->curFileSize = stream->stb_curFileSize; > @@ -300,8 +300,6 @@ uint32_t lgs_mbcsv_change_HA_state(lgs_c > stream->stb_curFileSize = stream->curFileSize; > *stream->p_fd = -1; /* Reopen files */ > } > - > - stream = log_stream_get_by_id(--num); > } > } > > @@ -612,7 +610,6 @@ static uint32_t edu_enc_streams(lgs_cb_t > uint32_t rc = NCSCC_RC_SUCCESS, num_rec = 0; > uint8_t *pheader = NULL; > lgsv_ckpt_header_t ckpt_hdr; > - int num; > > /* Prepare reg. structure to encode */ > ckpt_stream_rec = static_cast<lgs_ckpt_stream_open_t > *>(malloc(sizeof(lgs_ckpt_stream_open_t))); > @@ -630,10 +627,11 @@ static uint32_t edu_enc_streams(lgs_cb_t > } > ncs_enc_claim_space(uba, sizeof(lgsv_ckpt_header_t)); > > - num = get_number_of_streams(); > - log_stream_rec = log_stream_get_by_id(--num); > /* Walk through the reg list and encode record by record */ > - while (log_stream_rec != NULL) { > + // Iterate all existing log streams in cluster. > + SaBoolT endloop = SA_FALSE, jstart = SA_TRUE; > + while ((log_stream_rec = iterate_all_streams(endloop, jstart)) && > !endloop) { > + jstart = SA_FALSE; > lgs_ckpt_stream_open_set(log_stream_rec, ckpt_stream_rec); > rc = m_NCS_EDU_EXEC(&cb->edu_hdl, > edp_ed_open_stream_rec, uba, EDP_OP_TYPE_ENC, > ckpt_stream_rec, &ederror); > @@ -645,7 +643,6 @@ static uint32_t edu_enc_streams(lgs_cb_t > return rc; > } > ++num_rec; > - log_stream_rec = log_stream_get_by_id(--num); > } /* End while RegRec */ > > /* Encode RegHeader */ > 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 > @@ -37,9 +37,9 @@ > > static log_stream_t **stream_array; > /* We have at least the 3 well known streams. */ > -static unsigned int stream_array_size = 3; > +static uint32_t stream_array_size = 3; > /* Current number of streams */ > -static unsigned int numb_of_streams; > +static uint32_t numb_of_streams; > > static const uint32_t kInvalidId = static_cast<uint32_t> (-1); > > @@ -102,13 +102,47 @@ done: > } > > /** > - * Get current number of openning streams > + * Iterate all existing log streams in cluster (NOT thread safe). > + * This function should be called within main thread. > * > - * @param: none > - * @return current number of opening streams > + * @param jstart default is false. True if just starting the loop > + * @out end - true if reach the end > + * @return pointer to log stream > + * > + * Usage: > + * SaBoolT endloop = SA_FALSE, jstart = SA_TRUE; > + * while ((stream = iterate_all_streams(endloop, jstart)) && !endloop) { > + * jstart = SA_FALSE; > + * // Do somethings with stream > + * } > */ > -unsigned int get_number_of_streams() { > - return numb_of_streams; > +log_stream_t *iterate_all_streams(SaBoolT &end, SaBoolT jstart) { > + static uint32_t count = 0, stream_id = 0; > + log_stream_t *stream = nullptr; > + > + end = SA_FALSE; > + while (stream == nullptr) { > + // Begin the iteration, reset the data > + if (jstart == SA_TRUE) { > + count = 0; > + stream_id = 0; > + jstart = SA_FALSE; > + } > + > + // Reach the end of stream_array or get all existing streams. > + if (stream_id >= stream_array_size || count >= numb_of_streams) { > + end = SA_TRUE; > + break; > + } > + > + stream = log_stream_get_by_id(stream_id); > + stream_id++; > + > + // count only if stream_id exists > + if (stream != nullptr) count++; > + } > + > + return stream; > } > > /** > diff --git a/osaf/services/saf/logsv/lgs/lgs_stream.h > b/osaf/services/saf/logsv/lgs/lgs_stream.h > --- a/osaf/services/saf/logsv/lgs/lgs_stream.h > +++ b/osaf/services/saf/logsv/lgs/lgs_stream.h > @@ -130,7 +130,7 @@ extern void log_stream_print(log_stream_ > extern log_stream_t *log_stream_get_by_id(uint32_t id); > extern bool check_max_stream(); > void log_free_stream_resources(log_stream_t *stream); > -unsigned int get_number_of_streams(); > +log_stream_t *iterate_all_streams(SaBoolT &end, SaBoolT jstart); > extern log_stream_t *log_stream_get_by_name(const std::string &name); > > #endif ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
