Hi Canh I am not sure that this change is really valid. It is correct that applying the CCB may fail for two reasons regarding the log root directory parameter - The parameter may be invalid - The parameter could not be verified (we don't know if it is invalid) However in both cases the user must recreate the CCB and try again to apply if that is relevant.
It is maybe valid to change the oi error string See also my comments in the attached diff file Thanks Lennart > -----Original Message----- > From: Canh Van Truong [mailto:[email protected]] > Sent: den 28 september 2017 08:53 > To: Lennart Lund <[email protected]>; Vu Minh Nguyen > <[email protected]> > Cc: [email protected]; Canh Van Truong > <[email protected]> > Subject: [PATCH 1/1] log: fix to provide relevant info when lgsv is busy in > case > change root directory [#2600] > > In case change root directory, The new directory need to be verify if it is > writeable in ccb_completed_callback by log server. But log server is busy > sometime, > and log server does not provide info that user know to try again later. lgs > provides > "pathName: %s is NOT accepted" error message and this may not be > relevant. > > This fixing provides the error message "Log server is BUSY" instead > "pathName: %s is NOT accepted" > in case lgs is busy. > There is also updating test case logtest 5 2 > --- > src/log/apitest/tet_LogOiOps.c | 57 ++++++++++++++++++++++++++-------- > -------- > src/log/logd/lgs_config.cc | 9 ++++--- > src/log/logd/lgs_config.h | 4 +-- > src/log/logd/lgs_imm.cc | 14 ++++++++--- > 4 files changed, 54 insertions(+), 30 deletions(-) > > diff --git a/src/log/apitest/tet_LogOiOps.c b/src/log/apitest/tet_LogOiOps.c > index 7068b09d1..2352437e0 100644 > --- a/src/log/apitest/tet_LogOiOps.c > +++ b/src/log/apitest/tet_LogOiOps.c > @@ -22,6 +22,7 @@ > #include <saImmOm.h> > #include <limits.h> > #include <unistd.h> > +#include "base/osaf_time.h" > #include "base/saf_error.h" > > #include "logtest.h" > @@ -1626,6 +1627,11 @@ void saLogOi_502(void) > int rc = 0, tst_stat = 0; > char command[MAX_DATA]; > char tstdir[MAX_DATA]; > + char tail_command[MAX_DATA]; > + struct timespec timeout_time; > + const int kWaitTime = 10*1000; > + const char* error_string = "Log server is BUSY"; > + const char* tmp_file = "/tmp/logtest5_02.txt"; > > /* Path to test directory */ > sprintf(tstdir, "%s/xxtest", log_root_path); > @@ -1655,28 +1661,37 @@ void saLogOi_502(void) > goto done; > } > > - /* Change to xxtest */ > - sprintf( > - command, > - "immcfg -a logRootDirectory=%s > logConfig=1,safApp=safLogService", > - tstdir); > - rc = systemCall(command); > - if (rc != 0) { > - fprintf(stderr, "'%s' Fail rc=%d\n", command, rc); > - tst_stat = 1; > - goto done; > - } > > - /* Change back */ > - sprintf( > - command, > - "immcfg -a logRootDirectory=%s > logConfig=1,safApp=safLogService", > - log_root_path); > - rc = systemCall(command); > - if (rc != 0) { > - fprintf(stderr, "'%s' Fail rc=%d\n", command, rc); > - tst_stat = 1; > - goto done; > + // Change to root directory to xxtest > + sprintf(command, "immcfg -a logRootDirectory=%s " > + "logConfig=1,safApp=safLogService > %s 2>&1", > + tstdir, tmp_file); > + for (int i = 0; i < 2; i++) { > + osaf_set_millis_timeout(kWaitTime, &timeout_time); > + do { > + rc = system(command); > + if (rc != 0) { > + // Check if logsv is busy > + sprintf(tail_command, > + "tail -n 2 %s | grep -m 1 \"%s\"", > + tmp_file, error_string); > + int ret = system(tail_command); > + if (ret != 0) break; > + osaf_nanosleep(&kOneSecond); > + } > + } while (rc != 0 && !osaf_is_timeout(&timeout_time)); > + > + if (rc != 0) { > + fprintf(stderr, "'%s' Fail rc=%d\n", command, rc); > + tst_stat = 1; > + break; > + } > + > + // Change back > + sprintf(command, "immcfg -a logRootDirectory=%s > logConfig=1," > + "safApp=safLogService > %s 2>&1", > + log_root_path, tmp_file); > + osaf_nanosleep(&kOneSecond); > } > > done: > diff --git a/src/log/logd/lgs_config.cc b/src/log/logd/lgs_config.cc > index aec7204f6..c1549a9d9 100644 > --- a/src/log/logd/lgs_config.cc > +++ b/src/log/logd/lgs_config.cc > @@ -509,7 +509,7 @@ done: > * @param root_str_in[in] Root path to verify > * @return -1 on error > */ > -int lgs_cfg_verify_root_dir(const std::string &root_str_in) { > +int lgs_cfg_verify_root_dir(const std::string &root_str_in, int *errno_save) > { > int rc = 0; > log_stream_t *stream = nullptr; > size_t n = root_str_in.size(); > @@ -536,7 +536,7 @@ int lgs_cfg_verify_root_dir(const std::string > &root_str_in) { > } > } > > - if (lgs_path_is_writeable_dir_h(root_str_in) == false) { > + if (lgs_path_is_writeable_dir_h(root_str_in, errno_save) == false) { > LOG_NO("path_is_writeable_dir... Fail"); > rc = -1; > goto done; > @@ -1455,7 +1455,7 @@ const void *lgs_cfg_get(lgs_logconfGet_t param) { > * return: true = Path is valid > * false = Path is invalid > */ > -bool lgs_path_is_writeable_dir_h(const std::string &pathname) { > +bool lgs_path_is_writeable_dir_h(const std::string &pathname, int > *errno_save) { > bool is_writeable_dir = false; > > lgsf_apipar_t apipar; > @@ -1485,11 +1485,14 @@ bool lgs_path_is_writeable_dir_h(const > std::string &pathname) { > apipar.data_in = params_in_p; > apipar.data_out_size = 0; > apipar.data_out = nullptr; > + *errno_save = 0; > > api_rc = log_file_api(&apipar); > if (api_rc != LGSF_SUCESS) { > TRACE("%s - API error %s", __FUNCTION__, lgsf_retcode_str(api_rc)); > is_writeable_dir = false; > + if (api_rc == LGSF_BUSY) > + *errno_save = EBUSY; > } else { > if (apipar.hdl_ret_code_out == 0) > is_writeable_dir = false; > diff --git a/src/log/logd/lgs_config.h b/src/log/logd/lgs_config.h > index 6885271a4..0360a1dc9 100644 > --- a/src/log/logd/lgs_config.h > +++ b/src/log/logd/lgs_config.h > @@ -295,8 +295,8 @@ int lgs_cfg_update(const lgs_config_chg_t > *config_data); > * Parameter value validation functions. Validates parameters. > * For more information e.g. validation rules see lgs_conf.cc file > */ > -bool lgs_path_is_writeable_dir_h(const std::string &pathname); > -int lgs_cfg_verify_root_dir(const std::string &root_str_in); > +bool lgs_path_is_writeable_dir_h(const std::string &pathname, int > *errno_save); > +int lgs_cfg_verify_root_dir(const std::string &root_str_in, int *errno_save); > int lgs_cfg_verify_log_data_groupname(char *group_name); > int lgs_cfg_verify_log_file_format(const char *log_file_format); > int lgs_cfg_verify_max_logrecsize(uint32_t max_logrecsize_in); > diff --git a/src/log/logd/lgs_imm.cc b/src/log/logd/lgs_imm.cc > index 605024fb7..d61a12fba 100644 > --- a/src/log/logd/lgs_imm.cc > +++ b/src/log/logd/lgs_imm.cc > @@ -774,10 +774,16 @@ static SaAisErrorT config_ccb_completed_modify( > if (!strcmp(attribute->attrName, LOG_ROOT_DIRECTORY)) { > if (attribute->attrValuesNumber != 0) { > std::string pathName = *(static_cast<char **>(value)); > - if (lgs_cfg_verify_root_dir(pathName) != 0) { > - report_oi_error(immOiHandle, opdata->ccbId, > - "pathName: %s is NOT accepted", pathName.c_str()); > - ais_rc = SA_AIS_ERR_BAD_OPERATION; > + int errno_save = 0; > + if (lgs_cfg_verify_root_dir(pathName, &errno_save) != 0) { > + if (errno_save == EBUSY) { > + report_oi_error(immOiHandle, opdata->ccbId, "Log server is > BUSY"); > + ais_rc = SA_AIS_ERR_NO_RESOURCES; > + } else { > + report_oi_error(immOiHandle, opdata->ccbId, > + "pathName: %s is NOT accepted", > pathName.c_str()); > + ais_rc = SA_AIS_ERR_BAD_OPERATION; > + } > goto done; > } > TRACE("pathName: %s is accepted", pathName.c_str()); > -- > 2.13.0
log_2600_comments.diff
Description: log_2600_comments.diff
------------------------------------------------------------------------------ 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
