[devel] [PATCH 1/1] amf: Validate env variable format set in comptype/comp objects [#2409]
Valid environment variable should have the format 'var=value'. This validation shall now be done by AMF during CCB operations on SaAmfCompType (CREATE) and SaAmfComp (CREATE/MODIFY) objects regarding the saAmfxxxCmdEnv attribute. --- src/amf/amfd/comp.cc | 59 +++- src/amf/amfd/comp.h | 2 ++ src/amf/amfd/comptype.cc | 27 +++--- 3 files changed, 84 insertions(+), 4 deletions(-) diff --git a/src/amf/amfd/comp.cc b/src/amf/amfd/comp.cc index 482322d..10fb1c3 100644 --- a/src/amf/amfd/comp.cc +++ b/src/amf/amfd/comp.cc @@ -1,7 +1,7 @@ /* -*- OpenSAF -*- * * (C) Copyright 2008 The OpenSAF Foundation - * (C) Copyright 2017 Ericsson AB - All Rights Reserved. + * (C) Copyright 2017, 2018 Ericsson AB. All rights reserved. * Copyright (C) 2017, Oracle and/or its affiliates. All rights reserved. * * This program is distributed in the hope that it will be useful, but @@ -328,6 +328,26 @@ done: TRACE_LEAVE(); } +/* + * Validate the component CmdEnv attribute + * + * @param const std::string + * + * @return bool + */ +bool is_cmd_env_valid(const std::string &cmd_env_var) { + /* following environment variable format is considered as invalid: + * - containing 'whitespace' + * - having none or more than one '=' + */ + if ((cmd_env_var.find_first_of(' ') != std::string::npos) || + (std::count(cmd_env_var.begin(), cmd_env_var.end(), '=') != 1)) { +return false; + } + + return true; +} + /** * Validate configuration attributes for an AMF Comp object * @param comp @@ -339,6 +359,7 @@ static int is_config_valid(const std::string &dn, CcbUtilOperationData_t *opdata) { SaAisErrorT rc; SaNameT aname; + unsigned int num_of_cmd_env; std::string::size_type pos; SaUint32T value; @@ -399,6 +420,27 @@ static int is_config_valid(const std::string &dn, return 0; } + if ((immutil_getAttrValuesNumber(const_cast("saAmfCompCmdEnv") + ,attributes, &num_of_cmd_env)) == SA_AIS_OK) + { +for (unsigned int i = 0; i < num_of_cmd_env; i++) { + std::string cmd_env = immutil_getStringAttr(attributes, + "saAmfCompCmdEnv", i); + + if (!is_cmd_env_valid(cmd_env)) { +report_ccb_validation_error(opdata, "Unknown environment variable" +" format '%s' for '%s'." +" Should be 'var=value'", +cmd_env.c_str(), dn.c_str()); +/* NOTE: We shall only fail the env variable format validation at CCB- + * CREATE operation, but not during initial config read, so as to avoid + * breaking systems with invalid env variables pre-existing in IMM */ +if (opdata != nullptr) + return 0; + } +} // for (...; i < num_of_cmd_env;...) + } + #if 0 if ((comp->comp_info.category == AVSV_COMP_TYPE_SA_AWARE) && (comp->comp_info.init_len == 0)) { LOG_ER("Sa Aware Component: instantiation command not configured"); @@ -1038,6 +1080,21 @@ static SaAisErrorT ccb_completed_modify_hdlr(CcbUtilOperationData_t *opdata) { opdata, "Modification of saAmfCompCmdEnv failed, nullptr arg"); goto done; } + for (unsigned index = 0; index < attribute->attrValuesNumber; index++) { +std::string mod_comp_env = *(static_cast(attribute-> + attrValues[index])); + +if (!is_cmd_env_valid(mod_comp_env)) { + report_ccb_validation_error(opdata, "Modification of saAmfCompCmdEnv" + " failed. Unknown environment variable" + " format '%s' for '%s'." + " Should be 'var=value'", + mod_comp_env.c_str(), + osaf_extended_name_borrow(&opdata-> +objectName)); + goto done; +} + } } else if (!strcmp(attribute->attrName, "saAmfCompInstantiateCmdArgv")) { if (value_is_deleted == true) continue; char *param_val = *((char **)value); diff --git a/src/amf/amfd/comp.h b/src/amf/amfd/comp.h index 1493d71..3544a3a 100644 --- a/src/amf/amfd/comp.h +++ b/src/amf/amfd/comp.h @@ -1,6 +1,7 @@ /* -*- OpenSAF -*- * * (C) Copyright 2008 The OpenSAF Foundation + * (C) Copyright 2017, 2018 Ericsson AB. All rights reserved. * * This program is distributed in the hope that it will be useful, but * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY @@ -295,4 +296,5 @@ extern SaAisErrorT check_comp_stability(const AVD_COMP *); extern AVD_CTCS_TYPE *get_ctcstype(const std::string &comptype_name, const std::string &cstyp
[devel] [PATCH 0/1] Review Request for amf: validate env variable format set in comptype and comp objects [#2409] V4
Summary: amf: Validate env variable format set in comptype/comp objects [#2409] Review request for Ticket(s): 2409 Peer Reviewer(s): AMF maintainers Pull request to: Gary Lee, Hans Nordeback Affected branch(es): develop, release Development branch: ticket-2409 Base revision: e61e96acac6428f53545ad9b6f4203f3032a51c3 Personal repository: git://git.code.sf.net/u/nguyenluu/review Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts n SAF servicesy OpenSAF servicesn Core libraries n Samples n Tests n Other n Comments (indicate scope for each "y" above): - revision de870ceadd4d59cacbacfad3cfed29b6e5fec615 Author: Nguyen Luu Date: Tue, 13 Feb 2018 10:51:49 +0700 amf: Validate env variable format set in comptype/comp objects [#2409] Valid environment variable should have the format 'var=value'. This validation shall now be done by AMF during CCB operations on SaAmfCompType (CREATE) and SaAmfComp (CREATE/MODIFY) objects regarding the saAmfxxxCmdEnv attribute. Complete diffstat: -- src/amf/amfd/comp.cc | 59 +++- src/amf/amfd/comp.h | 2 ++ src/amf/amfd/comptype.cc | 27 +++--- 3 files changed, 84 insertions(+), 4 deletions(-) Testing Commands: - 1) Create SaAmfCompType/SaAmfComp objects with invalid env variable format in attributes saAmfCtDefCmdEnv/saAmfCompCmdEnv (e.g 'var = value', 'var==value'). 2) Modify attribute saAmfCompCmdEnv of SaAmfComp object with invalid env variable format. Testing, Expected Results: -- AMFD will reject CREATE/MODIFY CCBs with invalid env variable format. Conditions of Submission: - Ack from an AMF maintainer. Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: --- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. -- 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 Opensaf-devel@lists.sourceforge.net https://lists.s
[devel] [PATCH 0/1] Review Request for fmd: prevent data races between MDS and main threads V2 [#2763]
Summary: fmd: prevent data races between MDS and main threads V2 [#2763] Review request for Ticket(s): 2763 Peer Reviewer(s): Anders Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** Affected branch(es): develop Development branch: ticket-2763 Base revision: e61e96acac6428f53545ad9b6f4203f3032a51c3 Personal repository: git://git.code.sf.net/u/userid-2226215/review Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts n SAF servicesn OpenSAF servicesy Core libraries n Samples n Tests n Other n Comments (indicate scope for each "y" above): - Changes from V1: * remove m_MMGR_ALLOC_FM_CB * remove m_MMGR_FREE_FM_CB * fm_cb is allocated with new * gfm_rcv_msg->info.node_info.node_name is not always null-terminated, so osaf_extended_name_borrow() cannot be used. Instead, construct a std::string directly from it. revision 6ef8c1bb1a2e491205a6a49cdacd9331cc7c15a4 Author: Gary Lee Date: Tue, 13 Feb 2018 14:53:24 +1100 fmd: prevent data races between MDS and main threads [#2763] Complete diffstat: -- src/fm/fmd/fm_cb.h| 89 ++- src/fm/fmd/fm_main.cc | 38 +++--- src/fm/fmd/fm_mds.cc | 35 +++- src/fm/fmd/fm_mds.h | 2 ++ src/fm/fmd/fm_mem.h | 8 - 5 files changed, 86 insertions(+), 86 deletions(-) Testing Commands: - Run regresion tests Testing, Expected Results: -- No error Conditions of Submission: - Ack from reviewer Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: --- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. -- 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 Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 1/1] fmd: prevent data races between MDS and main threads [#2763]
--- src/fm/fmd/fm_cb.h| 89 ++- src/fm/fmd/fm_main.cc | 38 +++--- src/fm/fmd/fm_mds.cc | 35 +++- src/fm/fmd/fm_mds.h | 2 ++ src/fm/fmd/fm_mem.h | 8 - 5 files changed, 86 insertions(+), 86 deletions(-) diff --git a/src/fm/fmd/fm_cb.h b/src/fm/fmd/fm_cb.h index ffb3d8478..cfa50d883 100644 --- a/src/fm/fmd/fm_cb.h +++ b/src/fm/fmd/fm_cb.h @@ -18,19 +18,20 @@ #ifndef FM_FMD_FM_CB_H_ #define FM_FMD_FM_CB_H_ -#include -#include #include #include +#include +#include +#include +#include +#include +#include "base/mutex.h" #include "base/ncssysf_ipc.h" #include "base/ncssysf_tmr.h" -#include "fm_amf.h" +#include "fm/fmd/fm_amf.h" #include "mds/mds_papi.h" #include "rde/agent/rda_papi.h" -#include -#include -#include extern uint32_t gl_fm_hdl; @@ -55,58 +56,60 @@ typedef struct fm_tmr { } FM_TMR; /* FM control block */ -typedef struct fm_cb { - uint32_t cb_hdl; - SYSF_MBX mbx; +struct FM_CB { + uint32_t cb_hdl{}; + SYSF_MBX mbx{}; /* FM AMF CB */ - FM_AMF_CB fm_amf_cb; - NODE_ID node_id; - SaNameT node_name; + FM_AMF_CB fm_amf_cb{}; + std::atomic node_id{}; + SaNameT node_name{}; - SaNameT peer_node_name; - NODE_ID peer_node_id; - MDS_DEST peer_adest; /* will be 0 if peer is not up */ + std::string peer_node_name{}; + std::atomic peer_node_id{}; + std::atomic peer_adest{}; /* will be 0 if peer is not up */ /* Holds own role. */ - PCS_RDA_ROLE role; + PCS_RDA_ROLE role{}; /* AMF HA state for FM */ - SaAmfHAStateT amf_state; + SaAmfHAStateT amf_state{}; /* MDS handles. */ - MDS_DEST adest; - MDS_HDL adest_hdl; - MDS_HDL adest_pwe1_hdl; + MDS_DEST adest{}; + MDS_HDL adest_hdl{}; + MDS_HDL adest_pwe1_hdl{}; /* Timers */ - FM_TMR promote_active_tmr; - FM_TMR activation_supervision_tmr; + FM_TMR promote_active_tmr{}; + FM_TMR activation_supervision_tmr{}; /* Time in terms of one hundredth of seconds (500 for 5 secs.) */ - uint32_t active_promote_tmr_val; - uint32_t activation_supervision_tmr_val; - bool fully_initialized; - bool csi_assigned; + uint32_t active_promote_tmr_val{}; + uint32_t activation_supervision_tmr_val{}; + bool fully_initialized{false}; + bool csi_assigned{false}; /* Variable to indicate OpenSAF control of TIPC transport */ - bool control_tipc; + bool control_tipc{true}; /* Booleans to mark service down events of critical Osaf Services */ - bool immd_down; - bool immnd_down; - bool amfnd_down; - bool amfd_down; - bool fm_down; - - bool peer_sc_up; - bool well_connected; - uint64_t cluster_size; - struct timespec last_well_connected; - struct timespec node_isolation_timeout; - SaClmHandleT clm_hdl; - bool use_remote_fencing; - SaNameT peer_clm_node_name; - bool peer_node_terminated; -} FM_CB; + bool immd_down{true}; + bool immnd_down{true}; + std::atomic amfnd_down{true}; + bool amfd_down{true}; + bool fm_down{false}; + + std::atomic peer_sc_up{false}; + bool well_connected{false}; + uint64_t cluster_size{}; + struct timespec last_well_connected{}; + struct timespec node_isolation_timeout{}; + SaClmHandleT clm_hdl{}; + bool use_remote_fencing{false}; + SaNameT peer_clm_node_name{}; + std::atomic peer_node_terminated{false}; + + base::Mutex mutex_{}; +}; extern const char *role_string[]; extern FM_CB *fm_cb; diff --git a/src/fm/fmd/fm_main.cc b/src/fm/fmd/fm_main.cc index 693bf9438..1244c2347 100644 --- a/src/fm/fmd/fm_main.cc +++ b/src/fm/fmd/fm_main.cc @@ -30,7 +30,7 @@ This file contains the main() routine for FM. #include "base/osaf_extended_name.h" #include "base/osaf_poll.h" #include "base/osaf_time.h" -#include "fm.h" +#include "fm/fmd/fm.h" #include "nid/agent/nid_api.h" #include "osaf/configmake.h" #include "osaf/consensus/consensus.h" @@ -107,7 +107,7 @@ void rda_cb(uint32_t cb_hdl, PCS_RDA_CB_INFO *cb_info, rc = ncs_ipc_send(&fm_cb->mbx, (NCS_IPC_MSG *)evt, NCS_IPC_PRIORITY_HIGH); if (rc != NCSCC_RC_SUCCESS) { -syslog(LOG_ERR, "IPC send failed %d", rc); +syslog(LOG_ERR, "IPC send failed %u", rc); free(evt); } @@ -144,17 +144,10 @@ int main(int argc, char *argv[]) { } /* Allocate memory for FM_CB. */ - fm_cb = m_MMGR_ALLOC_FM_CB; - if (NULL == fm_cb) { -syslog(LOG_ERR, "CB Allocation failed."); -goto fm_init_failed; - } + fm_cb = new FM_CB(); - memset(fm_cb, 0, sizeof(FM_CB)); fm_cb->fm_amf_cb.nid_started = nid_started; fm_cb->fm_amf_cb.amf_fd = -1; - fm_cb->fully_initialized = false; - fm_cb->csi_assigned = false; /* Variable to control whether FM will trigger failover immediately * upon recieving down event of critical services or will wait @@ -169,11 +162,6 @@ int main(int argc, char *argv[]) { */ fm_cb->control_tipc = true; /* Default behaviour */ - fm_cb->immd_down = true; - fm_cb->immnd_down = true; - fm_cb->amfnd_down = true; - fm_cb->amfd_down = true; - /* Creat
Re: [devel] [PATCH 1/1] dtm: updates to readme files and change of TRACE var in conf [#2776]
Ack with minor comments marked AndersW> below. regards, Anders Widell On 02/09/2018 12:36 PM, srinivas wrote: --- src/imm/README | 12 +++- src/imm/immloadd/imm_loader.cc | 2 +- src/imm/immnd/immnd.conf| 5 +++-- src/imm/immpbed/immpbe.cc | 2 +- src/imm/tools/imm_dumper.cc | 2 +- src/ntf/README | 10 ++ src/ntf/ntfd/ntfd.conf | 4 +++- src/ntf/ntfimcnd/ntfimcn_main.c | 2 +- 8 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/imm/README b/src/imm/README index d2f6b2f..ee5f8e8 100644 --- a/src/imm/README +++ b/src/imm/README @@ -3194,7 +3194,9 @@ To enable/disable immd/immnd trace in a running system, send signal USR2 to the immd/immnd process. Each signal toggles the trace. Trace default is disabled. -Traces are written to the file configured in immd.conf and immnd.conf +Traces are written to the file configured in immd.conf and immnd.conf. +Traces are always stored in $PKGLOGDIR directory and the directory component +of the path name (if any) is ignored. To enable traces from the very start, uncomment: @@ -3204,21 +3206,21 @@ in immd.conf/immnd.conf and restart the cluster. Errors, warnings and notice level messages are logged to the syslog. -To enable traces in the IMM library, export the variable IMMA_TRACE_PATHNAME +To enable traces in the IMM library, export the variable IMMA_TRACE_FILENAME with a valid pathname before starting the application using the IMM library. For example: -$ export IMMA_TRACE_PATHNAME=/tmp/imm.trace +$ export IMMA_TRACE_FILENAME=imm.trace $ ./immomtest -$ cat /tmp/imm.trace +$ cat $pkglogdir/imm.trace It is also possible to trace slave processes forked by the IMMND. This would be processes for loading, sync and dump/pbe. To enable such trace uncomment: - #export IMMSV_TRACE_PATHNAME=$pkglogdir/osafimmnd +#export IMMSV_TRACE_FILENAME=osafimmnd TEST diff --git a/src/imm/immloadd/imm_loader.cc b/src/imm/immloadd/imm_loader.cc index 4b6ad08..bfd02a7 100644 --- a/src/imm/immloadd/imm_loader.cc +++ b/src/imm/immloadd/imm_loader.cc @@ -2507,7 +2507,7 @@ int main(int argc, char *argv[]) { exit(1); } - if ((logPath = getenv("IMMSV_TRACE_PATHNAME"))) { + if ((logPath = getenv("IMMSV_TRACE_FILENAME"))) { category_mask = 0x; /* TODO: set using env variable ? */ } else { logPath = defaultLog; AndersW> Also remove PKGLOGDIR "/" from this line in the code (not visible in the patch): const char *defaultLog = PKGLOGDIR "/osafimmnd"; diff --git a/src/imm/immnd/immnd.conf b/src/imm/immnd/immnd.conf index 97af792..9172677 100644 --- a/src/imm/immnd/immnd.conf +++ b/src/imm/immnd/immnd.conf @@ -10,8 +10,9 @@ # Uncomment the next line to enable trace for the imm-loader/sync # and imm-pbe. When these are forked by the IMMND coordinator, # they attach as IMMA clients. These processes will also route trace to the -# IMMND trace-file as define here. -#export IMMSV_TRACE_PATHNAME=$pkglogdir/osafimmnd +# IMMND trace-file as define here. Traces are always stored in $PKGLOGDIR +# directory and the directory component of the path name (if any) is ignored. +#export IMMSV_TRACE_FILENAME=osafimmnd # The directory where the imm.xml files and persistend backend files are # stored. Imm dump files may also be stored here or in a subdirectory. diff --git a/src/imm/immpbed/immpbe.cc b/src/imm/immpbed/immpbe.cc index 964086f..6e9b933 100644 --- a/src/imm/immpbed/immpbe.cc +++ b/src/imm/immpbed/immpbe.cc @@ -118,7 +118,7 @@ int main(int argc, char* argv[]) { const SaImmAdminOperationParamsT_2* params[] = {NULL}; SaImmAdminOperationParamsT_2** retParams = NULL; - if ((logPath = getenv("IMMSV_TRACE_PATHNAME"))) { + if ((logPath = getenv("IMMSV_TRACE_FILENAME"))) { category_mask = 0x; /* TODO: set using -t flag ? */ } else { logPath = defaultLog; diff --git a/src/imm/tools/imm_dumper.cc b/src/imm/tools/imm_dumper.cc index 0365fc7..5e5dd00 100644 --- a/src/imm/tools/imm_dumper.cc +++ b/src/imm/tools/imm_dumper.cc @@ -123,7 +123,7 @@ int main(int argc, char* argv[]) { * osaf_extended_name_* before saImmOmInitialize and saImmOiInitialize */ osaf_extended_name_init(); - if ((logPath = getenv("IMMSV_TRACE_PATHNAME"))) { + if ((logPath = getenv("IMMSV_TRACE_FILENAME"))) { category_mask = 0x; /* TODO: set using -t flag ? */ } else { logPath = defaultLog; diff --git a/src/ntf/README b/src/ntf/README index b684d66..938766e 100644 --- a/src/ntf/README +++ b/src/ntf/README @@ -260,14 +260,16 @@ in ntfd.conf (see CONFIGURATION above) and restart the cluster. For fatal errors syslog is used. -To enable traces in the NTF library, export the variable NTFSV_TRACE_PATHNAME -with a valid pathname before starting the application using the NTF library. +To enable traces in the NTF library, export the variable NTFS
[devel] [PATCH 0/2] Review Request for dtm: Added following options --max-backups and --max-file-size to osaflog tool and in transportd [#2731]
Summary: dtm: configure trace file size and no of backups in transportd.conf [#2731] Review request for Ticket(s): 2731 Peer Reviewer(s):anders.wid...@ericsson.com Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** Affected branch(es): develop Development branch: ticket-2731 Base revision: a2ee56579bb33a05477b151c15d17f3b12025cf8 Personal repository: git://git.code.sf.net/u/syam-talluri/review Impacted area Impact y/n Docsy Build systemy RPM/packaging y Configuration files y Startup scripts n SAF servicesn OpenSAF servicesy Core libraries y Samples n Tests n Other n NOTE: Patch(es) contain lines longer than 80 characers Comments (indicate scope for each "y" above): - dtm: Added following options --max-backups and --max-file-size to osaflog tool and in implemented the functionality in the transportd. The messaging format between osaflog and transportd is modified by introducing a structure. revision b289abf08100596c382765c82c94763ac486731e Author: syam-talluri Date: Mon, 12 Feb 2018 17:29:26 +0530 dtm: Added following options --max-backups and --max-file-size to osaflog tool and in transportd [#2731] revision 0723340e8f4802e82655037e8d2743dddc684180 Author: syam-talluri Date: Mon, 12 Feb 2018 17:29:26 +0530 dtm: configure trace file size and no of backups in transportd.conf [#2731] Added Files: src/dtm/transport/transportd.conf Complete diffstat: -- opensaf.spec.in| 2 + src/dtm/Makefile.am| 3 + src/dtm/common/osaflog_protocol.h | 15 ++ src/dtm/tools/osaflog.cc | 262 ++--- src/dtm/transport/log_server.cc| 113 +++-- src/dtm/transport/log_server.h | 14 +- src/dtm/transport/log_writer.cc| 6 +- src/dtm/transport/log_writer.h | 4 +- src/dtm/transport/main.cc | 3 + src/dtm/transport/osaf-transport.in| 1 + src/dtm/transport/tests/log_writer_test.cc | 2 +- src/dtm/transport/transportd.conf | 13 ++ 12 files changed, 391 insertions(+), 47 deletions(-) Testing Commands: - Run osaflog command tool and tryied the --max-file-size=8 and --max--backups=9 Testing, Expected Results: -- *** PASTE COMMAND OUTPUTS / TEST RESULTS *** Conditions of Submission: - *** HOW MANY DAYS BEFORE PUSHING, CONSENSUS ETC *** Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: --- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.gitconfig file (
[devel] [PATCH 2/2] dtm: Added following options --max-backups and --max-file-size to osaflog tool and in transportd [#2731]
--- src/dtm/common/osaflog_protocol.h | 15 +++ src/dtm/tools/osaflog.cc | 262 ++ src/dtm/transport/log_server.cc | 60 - src/dtm/transport/log_server.h| 10 +- src/dtm/transport/main.cc | 5 +- src/dtm/transport/transportd.conf | 8 +- 6 files changed, 293 insertions(+), 67 deletions(-) diff --git a/src/dtm/common/osaflog_protocol.h b/src/dtm/common/osaflog_protocol.h index 61e9f6f..9723fd5 100644 --- a/src/dtm/common/osaflog_protocol.h +++ b/src/dtm/common/osaflog_protocol.h @@ -24,6 +24,21 @@ namespace Osaflog { +enum cmd { FLUSH, MAXBACKUPS, MAXFILESIZE}; +enum cmdreply { RFLUSH = 101, RMAXBACKUPS, RMAXFILESIZE, FAILURE}; +struct cmd_osaflog +{ +char marker[4]; +enum cmd m_cmd;// Command Enum +size_tm_value; // Value based on the command +}; + + +struct cmd_osaflog_resp +{ +enum cmdreply m_cmdreply;// Command Enum +}; + static constexpr const char* kServerSocketPath = PKGLOCALSTATEDIR "/osaf_log.sock"; diff --git a/src/dtm/tools/osaflog.cc b/src/dtm/tools/osaflog.cc index 3ce66f4..aefff81 100644 --- a/src/dtm/tools/osaflog.cc +++ b/src/dtm/tools/osaflog.cc @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -38,7 +39,9 @@ namespace { void PrintUsage(const char* program_name); -bool Flush(); +bool Flush(bool flush_done); +bool MaxTraceFileSize(size_t maxfilesize); +bool NoOfBackupFiles(size_t nooffiles); base::UnixServerSocket* CreateSocket(); uint64_t Random64Bits(uint64_t seed); bool PrettyPrint(const std::string& log_stream); @@ -53,20 +56,73 @@ char buf[65 * 1024]; } // namespace int main(int argc, char** argv) { - bool flush_option = false; - if (argc >= 2 && strcmp(argv[1], "--flush") == 0) { -flush_option = true; ---argc; -++argv; - } - if ((argc != 2) && (argc != 1 || flush_option == false)) { + struct option long_options[] = {{"max-file-size", required_argument, 0, 'm'}, + {"max-backups", required_argument, 0, 'b'}, + {"flush", no_argument, 0, 'f'}, + {"pretty-print", required_argument, 0, 'p'}, + {0, 0, 0, 0}}; + + size_t maxfilesize = 0; + size_t maxbackups = 0; + char *pplog = NULL; + int opt= 0; + + int long_index =0; + bool flush_result = true; + bool print_result = true; + bool maxfilesize_result = true; + bool noof_backup_result = true; + bool flush_set = false; + bool prettyprint_set = false; + + if (argc == 1) { PrintUsage(argv[0]); exit(EXIT_FAILURE); } - bool flush_result = Flush(); - bool print_result = true; - if (argc == 2) print_result = PrettyPrint(argv[1]); - exit((flush_result && print_result) ? EXIT_SUCCESS : EXIT_FAILURE); + + while ((opt = getopt_long(argc, argv,"m:b:p:f", + long_options, &long_index )) != -1) { +switch (opt){ + case 'p': + pplog = optarg; + flush_result = Flush(flush_set); + PrettyPrint(pplog); + prettyprint_set = true; + flush_set = true; + break; + case 'f': + flush_result = Flush(flush_set); + flush_set = true; + break; + case 'm': + maxfilesize = atoi(optarg); + maxfilesize_result = MaxTraceFileSize(maxfilesize); + break; + case 'b': + maxbackups = atoi(optarg); + noof_backup_result = NoOfBackupFiles(maxbackups); + break; + default: PrintUsage(argv[0]); + exit(EXIT_FAILURE); +} +} + +if(prettyprint_set != true) { + if (argc - optind == 1) { + flush_result = Flush(flush_set); + print_result = PrettyPrint(argv[optind]); + prettyprint_set = false; + }else if(argc - optind > 1) { + PrintUsage(argv[0]); + exit(EXIT_FAILURE); + } +} + + if(flush_result && print_result && maxfilesize_result && noof_backup_result) + exit(EXIT_SUCCESS); + + + exit(EXIT_FAILURE); } namespace { @@ -75,18 +131,95 @@ void PrintUsage(const char* program_name) { fprintf(stderr, "Usage: %s [OPTION] [LOGSTREAM]\n" "\n" - "Pretty-print the messages stored on disk for the specified\n" + "pretty-print the messages stored on disk for the specified\n" "LOGSTREAM. When a LOGSTREAM argument is specified, the option\n" "--flush is implied.\n" "\n" "Opions:\n" "\n" - " --flush Flush all buffered messages in the log server to disk\n" - " even when no LOGSTREAM is specified\n", + "--flush Flush all buffered messages in the log server to disk\n" +
[devel] [PATCH 1/2] dtm: configure trace file size and no of backups in transportd.conf [#2731]
--- opensaf.spec.in| 2 + src/dtm/Makefile.am| 3 + src/dtm/transport/log_server.cc| 95 -- src/dtm/transport/log_server.h | 10 +++- src/dtm/transport/log_writer.cc| 6 +- src/dtm/transport/log_writer.h | 4 +- src/dtm/transport/main.cc | 4 ++ src/dtm/transport/osaf-transport.in| 1 + src/dtm/transport/tests/log_writer_test.cc | 2 +- src/dtm/transport/transportd.conf | 13 10 files changed, 129 insertions(+), 11 deletions(-) create mode 100644 src/dtm/transport/transportd.conf diff --git a/opensaf.spec.in b/opensaf.spec.in index db4b5be..452d1c8 100644 --- a/opensaf.spec.in +++ b/opensaf.spec.in @@ -1397,6 +1397,7 @@ fi %config(noreplace) %{_pkgsysconfdir}/nodeinit.conf.controller %config(noreplace) %{_pkgsysconfdir}/nodeinit.conf.payload %config(noreplace) %{_pkgsysconfdir}/dtmd.conf +%config(noreplace) %{_pkgsysconfdir}/transportd.conf %{_pkglibdir}/osafrded %{_pkgclcclidir}/osaf-rded %{_pkglibdir}/osaffmd @@ -1423,6 +1424,7 @@ fi %dir %{_pkgsysconfdir} %config(noreplace) %{_pkgsysconfdir}/nodeinit.conf.payload %config(noreplace) %{_pkgsysconfdir}/dtmd.conf +%config(noreplace) %{_pkgsysconfdir}/transportd.conf %{_pkglibdir}/osafdtmd %{_pkglibdir}/osaftransportd %{_pkgclcclidir}/osaf-dtm diff --git a/src/dtm/Makefile.am b/src/dtm/Makefile.am index f3ba720..822249c 100644 --- a/src/dtm/Makefile.am +++ b/src/dtm/Makefile.am @@ -57,6 +57,9 @@ nodist_pkgclccli_SCRIPTS += \ dist_pkgsysconf_DATA += \ src/dtm/dtmnd/dtmd.conf +dist_pkgsysconf_DATA += \ + src/dtm/transport/transportd.conf + bin_osaftransportd_CXXFLAGS = $(AM_CXXFLAGS) bin_osaftransportd_CPPFLAGS = \ diff --git a/src/dtm/transport/log_server.cc b/src/dtm/transport/log_server.cc index 2d6c961..780feb1 100644 --- a/src/dtm/transport/log_server.cc +++ b/src/dtm/transport/log_server.cc @@ -18,21 +18,28 @@ #include "dtm/transport/log_server.h" #include +#include +#include #include "base/osaf_poll.h" #include "base/time.h" #include "dtm/common/osaflog_protocol.h" #include "osaf/configmake.h" +#define TRANSPORTD_CONFIG_FILE PKGSYSCONFDIR "/transportd.conf" + +size_t LogServer::no_of_backups = 9; +size_t LogServer::kmax_file_size = 5000 * 1024; + const Osaflog::ClientAddressConstantPrefix LogServer::address_header_{}; LogServer::LogServer(int term_fd) : term_fd_{term_fd}, log_socket_{Osaflog::kServerSocketPath, base::UnixSocket::kNonblocking}, log_streams_{}, - current_stream_{new LogStream{"mds.log", 1}}, + current_stream_{new LogStream{"mds.log", 1, LogServer::kmax_file_size}}, no_of_log_streams_{1} { log_streams_["mds.log"] = current_stream_; -} + } LogServer::~LogServer() { for (const auto& s : log_streams_) delete s.second; @@ -40,6 +47,12 @@ LogServer::~LogServer() { void LogServer::Run() { struct pollfd pfd[2] = {{term_fd_, POLLIN, 0}, {log_socket_.fd(), POLLIN, 0}}; + + /* Initialize a signal handler for loading new configuration from transportd.conf */ + if ((signal(SIGUSR2, usr2_sig_handler)) == SIG_ERR) { + syslog(LOG_ERR,"signal USR2 registration failed: %s", strerror(errno)); + } + do { for (int i = 0; i < 256; ++i) { char* buffer = current_stream_->current_buffer_position(); @@ -88,6 +101,12 @@ void LogServer::Run() { } while ((pfd[0].revents & POLLIN) == 0); } +void LogServer::usr2_sig_handler(int sig) { + syslog(LOG_ERR, "Recived the SIGUSR2 Signal"); + ReadConfig(TRANSPORTD_CONFIG_FILE); + signal(SIGUSR2, usr2_sig_handler); +} + LogServer::LogStream* LogServer::GetStream(const char* msg_id, size_t msg_id_size) { if (msg_id_size == current_stream_->log_name_size() && @@ -99,7 +118,8 @@ LogServer::LogStream* LogServer::GetStream(const char* msg_id, if (iter != log_streams_.end()) return iter->second; if (no_of_log_streams_ >= kMaxNoOfStreams) return nullptr; if (!ValidateLogName(msg_id, msg_id_size)) return nullptr; - LogStream* stream = new LogStream{log_name, 9}; + + LogStream* stream = new LogStream{log_name, LogServer::no_of_backups, LogServer::kmax_file_size}; auto result = log_streams_.insert( std::map::value_type{log_name, stream}); if (!result.second) osaf_abort(msg_id_size); @@ -107,6 +127,71 @@ LogServer::LogStream* LogServer::GetStream(const char* msg_id, return stream; } +bool LogServer::ReadConfig(const char *transport_config_file) { + FILE *transport_conf_file; + char line[256]; + size_t maxFileSize=0; + size_t noOfBackupFiles=0; + int i, n, comment_line, tag_len = 0; + + /* Open transportd.conf config file. */ + transport_conf_file = fopen(transport_config_file, "r"); + if (transport_conf_file == nullptr) { + +syslog(LOG_ERR,"Not able to read transportd.conf: %s", strerror(errno)); +return false; + } + + + /* Read fi
Re: [devel] Review Request for doc: update overview PR for split brain prevention with consensus service [#64]
Hi Ravi/Anders AndersW> This is slightly out of scope since there are many RAFT implementations, but I agree it could be a good idea to provide a sample configuration for etcd along with the sample etcd plugin. I will try to provide a sample plugin for an external etcd server, and maybe a sample plugin for another RAFT based key-value store. Thanks Gary -- 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 Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 0/3] Review Request for ntf: Checkpoint and cold sync reader information [#2757]
Hi Minh I have read the PR document and the README update and have no comments so you have my ACK Thanks Lennart > -Original Message- > From: Minh Hon Chau [mailto:minh.c...@dektech.com.au] > Sent: den 12 februari 2018 00:03 > To: Lennart Lund ; > srinivas.mangip...@oracle.com; Canh Van Truong > > Cc: opensaf-devel@lists.sourceforge.net > Subject: Re: [PATCH 0/3] Review Request for ntf: Checkpoint and cold sync > reader information [#2757] > > Hi, > > A friendly reminder, I'm going to push the PR doc update as there is no > comment by today. > > Thanks, > > Minh > > > On 29/01/18 15:01, Minh Hon Chau wrote: > > Hi all, > > > > If the patch look ok to you, I would like to push it tomorrow. > > > > I have updated the README and PR doc at the below links, if you have > > time please have a look. > > > > > https://sourceforge.net/p/opensaf/tickets/_discuss/thread/77619155/0a9b/ > attachment/2757_README.diff > > > > > > > https://sourceforge.net/p/opensaf/tickets/_discuss/thread/5671255e/16a4/ > attachment/OpenSAF_NTFSv_PR_2735.odt > > > > > > Thanks, > > > > Minh > > > > > > On 24/01/18 12:49, minh.c...@dektech.com.au wrote: > >> Hi Lennart, > >> > >> I tested the APIs between versions with/without the changes. I will send > >> out for review the README and PR change after the code review is > >> done. One > >> limitation is that both active and standby require the patches to work. > >> > >> Thanks, > >> Minh > >> > >>> Hi Minh > >>> > >>> Ack. I have not tested much > >>> > >>> Have you tested using the reader API while running old version on > >>> standby > >>> and new version on active and vice versa (upgrade case)? Limitations? > >>> PR documentation update? > >>> > >>> Thanks > >>> Lennart > >>> > -Original Message- > From: Minh Hon Chau > Sent: den 22 januari 2018 05:19 > To: Lennart Lund ; > srinivas.mangip...@oracle.com; Canh Van Truong > > Cc: opensaf-devel@lists.sourceforge.net; Minh Hon Chau > > Subject: [PATCH 0/3] Review Request for ntf: Checkpoint and cold sync > reader information [#2757] > > Summary: ntfd: Checkpoint reader to the standby when processes > reader > API requests [#2757] > Review request for Ticket(s): 2757 > Peer Reviewer(s): Lennart, Srinivas, Canh > Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** > Affected branch(es): develop > Development branch: ticket-2757 > Base revision: ee105cb3bf44eee4e8785e3de7d24f907641e2ab > Personal repository: git://git.code.sf.net/u/minh-chau/review > > > Impacted area Impact y/n > > Docs n > Build system n > RPM/packaging n > Configuration files n > Startup scripts n > SAF services y > OpenSAF services n > Core libraries n > Samples n > Tests n > Other n > > NOTE: Patch(es) contain lines longer than 80 characers > > Comments (indicate scope for each "y" above): > - > *** EXPLAIN/COMMENT THE PATCH SERIES HERE *** > > revision 74da3370accfa44a34a7abf9830ceaeae3ab5d4f > Author:Minh Chau > Date:Mon, 22 Jan 2018 15:08:59 +1100 > > ntftest: Add new test cases of suite 41 for cold sync and > checkpoint of > reader > APIs [#2757] > > > > revision ad38745b1c411bc52905725281c84c69e4147fef > Author:Minh Chau > Date:Mon, 22 Jan 2018 15:03:42 +1100 > > ntfd: Cold sync reader to the standby ntfd after rebooting the standby > controller [#2757] > > Assumpt that the reader information is updated to the standby ntfd via > checkpoint > upon reception of reader APIs requests. However, if the standby > controller > reboots > and comes up, the standby ntfd still has none of readers information > which is > available at the active ntfd. Now if a switchover happens, the new > active will > not > be able to process the reader APIs requests with existing reader > handles. > > This patch adds reader information as part of cold sync > > > > revision 47cf18850e6819c2db4642eb1e639aff5f0d8282 > Author:Minh Chau > Date:Mon, 22 Jan 2018 14:12:00 +1100 > > ntfd: Checkpoint reader to the standby when processes reader API > requests > [#2757] > > When active ntfd receives reader API requests: ReaderIntialize, > ReadNext, > ReadFinalize, active ntfd does not update the readers information > to the > standby. Thus, either switchover or failover happens, the client > can not > continue to use the reader APIs, because there
Re: [devel] [PATCH 0/1] Review Request for ntfd: Correct counting by in decode_reader_info [#2781]
Hi Minh, Ack from my end. Thank you Srinivas -Original Message- From: Minh Chau [mailto:minh.c...@dektech.com.au] Sent: Monday, February 12, 2018 8:38 AM To: canh.v.tru...@dektech.com.au Cc: Minh Chau ; opensaf-devel@lists.sourceforge.net Subject: [devel] [PATCH 0/1] Review Request for ntfd: Correct counting by in decode_reader_info [#2781] Summary: ntfd: Correct counting by in decode_reader_info [#2781] Review request for Ticket(s): 2781 Peer Reviewer(s): Canh Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** Affected branch(es): develop Development branch: ticket-2781 Base revision: 0005efbd2284a62847fe0aea4d1c8a2571bc Personal repository: git://git.code.sf.net/u/minh-chau/review Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts n SAF servicesy OpenSAF servicesn Core libraries n Samples n Tests n Other n Comments (indicate scope for each "y" above): - *** EXPLAIN/COMMENT THE PATCH SERIES HERE *** revision d09532c11ad56910c6de5c2503fe790806bd299b Author: Minh Chau Date: Mon, 12 Feb 2018 14:00:55 +1100 ntfd: Correct counting by in decode_reader_info [#2781] Complete diffstat: -- src/ntf/ntfd/ntfs_mbcsv.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) Testing Commands: - *** LIST THE COMMAND LINE TOOLS/STEPS TO TEST YOUR CHANGES *** Testing, Expected Results: -- *** PASTE COMMAND OUTPUTS / TEST RESULTS *** Conditions of Submission: - ack from reviewer Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: --- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! https://urldefense.proofpoint.com/v2/url?u=http-3A__sdm.link_slashdot&d=DwICAg&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=rU6x356sikQZSi7Ttc2DuiqAgbc0QIeANg72N5AllVc&m=oFI749HSS5__EaZ0L67sBbGQlUjBXF5iyNvW8DcLWew&s=j4fA1A3ARgmFxy-F6aNeqckYsZ-nz29cWXHIzIxq6jg&e= ___ Opensaf-devel mailing list Opensaf-devel@l