[devel] [PATCH 0 of 1] Review Request for amfd: Remove asserts from validation routines [#849]
Summary: amfd: Remove asserts from validation routines [#849] Review request for Trac Ticket(s): 849 Peer Reviewer(s): Hans F, Hans N, Nagendra, Praveen Pull request to: Affected branch(es): all Development branch: default 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): - changeset b1b63571db541aea1c11ea47fdaef92ce5ac9b08 Author: Gary Lee gary@dektech.com.au Date: Thu, 08 May 2014 15:52:36 +1000 amfd: Remove asserts from validation routines [#849] When an unknown attribute in encountered in various callbacks, sometimes an assert is called. In other cases, the operation is rejected. This may create forward compatibility issues if new attributes are added in the future. In this patch, the asserts are replaced with a warning and the corresponding attribute change will be permitted but ignored. Complete diffstat: -- osaf/services/saf/amf/amfd/app.cc| 18 +++--- osaf/services/saf/amf/amfd/comp.cc | 5 +++-- osaf/services/saf/amf/amfd/compcstype.cc | 5 +++-- osaf/services/saf/amf/amfd/hlt.cc| 10 ++ osaf/services/saf/amf/amfd/sg.cc | 9 + osaf/services/saf/amf/amfd/si.cc | 5 +++-- osaf/services/saf/amf/amfd/su.cc | 5 +++-- 7 files changed, 34 insertions(+), 23 deletions(-) Testing Commands: - LIST THE COMMAND LINE TOOLS/STEPS TO TEST YOUR CHANGES 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 ~/.hgrc file (i.e. username, 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. -- Is your legacy SCM system holding you back? Join Perforce May 7 to find out: #149; 3 signs your SCM is hindering your productivity #149; Requirements for releasing software faster #149; Expert tips
[devel] [PATCH 1 of 1] amfd: Remove asserts from validation routines [#849]
osaf/services/saf/amf/amfd/app.cc| 18 +++--- osaf/services/saf/amf/amfd/comp.cc | 5 +++-- osaf/services/saf/amf/amfd/compcstype.cc | 5 +++-- osaf/services/saf/amf/amfd/hlt.cc| 10 ++ osaf/services/saf/amf/amfd/sg.cc | 9 + osaf/services/saf/amf/amfd/si.cc | 5 +++-- osaf/services/saf/amf/amfd/su.cc | 5 +++-- 7 files changed, 34 insertions(+), 23 deletions(-) When an unknown attribute in encountered in various callbacks, sometimes an assert is called. In other cases, the operation is rejected. This may create forward compatibility issues if new attributes are added in the future. In this patch, the asserts are replaced with a warning and the corresponding attribute change will be permitted but ignored. diff --git a/osaf/services/saf/amf/amfd/app.cc b/osaf/services/saf/amf/amfd/app.cc --- a/osaf/services/saf/amf/amfd/app.cc +++ b/osaf/services/saf/amf/amfd/app.cc @@ -243,12 +243,15 @@ static SaAisErrorT app_ccb_completed_cb( SaNameT dn = *((SaNameT*)attribute-attrValues[0]); if (NULL == avd_apptype_get(dn)) { report_ccb_validation_error(opdata, saAmfAppType '%s' not found, dn.value); + rc = SA_AIS_ERR_BAD_OPERATION; goto done; } rc = SA_AIS_OK; break; - } else - osafassert(0); + } else { + LOG_WA(Ignoring unknown attribute '%s', attribute-attrName); + rc = SA_AIS_OK; + } } break; case CCBUTIL_DELETE: @@ -294,10 +297,10 @@ static void app_ccb_apply_cb(CcbUtilOper app-saAmfAppType = *((SaNameT*)attribute-attrValues[0]); app-app_type = avd_apptype_get(app-saAmfAppType); avd_apptype_add_app(app); - break; } - else - osafassert(0); + else { + TRACE(Ignoring unknown attribute '%s', attribute-attrName); + } } break; } @@ -396,8 +399,9 @@ static SaAisErrorT app_rt_attr_cb(SaImmO if (!strcmp(attributeName, saAmfApplicationCurrNumSGs)) { avd_saImmOiRtObjectUpdate_sync(objectName, attributeName, SA_IMM_ATTR_SAUINT32T, app-saAmfApplicationCurrNumSGs); - } else - osafassert(0); + } else { + LOG_WA(Ignoring unknown attribute '%s', attributeName); + } } return SA_AIS_OK; diff --git a/osaf/services/saf/amf/amfd/comp.cc b/osaf/services/saf/amf/amfd/comp.cc --- a/osaf/services/saf/amf/amfd/comp.cc +++ b/osaf/services/saf/amf/amfd/comp.cc @@ -830,8 +830,9 @@ static SaAisErrorT comp_rt_attr_cb(SaImm /* TODO */ } else if (!strcmp(saAmfCompCurrProxiedNames, attributeName)) { /* TODO */ - } else - osafassert(0); + } else { + LOG_WA(Ignoring unknown attribute '%s', attributeName); + } } return SA_AIS_OK; diff --git a/osaf/services/saf/amf/amfd/compcstype.cc b/osaf/services/saf/amf/amfd/compcstype.cc --- a/osaf/services/saf/amf/amfd/compcstype.cc +++ b/osaf/services/saf/amf/amfd/compcstype.cc @@ -435,8 +435,9 @@ static SaAisErrorT compcstype_rt_attr_ca SA_IMM_ATTR_SAUINT32T, cst-saAmfCompNumCurrStandbyCSIs); } else if (!strcmp(saAmfCompAssignedCsi, attributeName)) { /* TODO */ - } else - osafassert(0); + } else { + LOG_WA(Ignoring unknown attribute '%s', attributeName); + } } return SA_AIS_OK; diff --git a/osaf/services/saf/amf/amfd/hlt.cc b/osaf/services/saf/amf/amfd/hlt.cc --- a/osaf/services/saf/amf/amfd/hlt.cc +++ b/osaf/services/saf/amf/amfd/hlt.cc @@ -48,8 +48,9 @@ static SaAisErrorT ccb_completed_modify_ *value, opdata-objectName.value); return SA_AIS_ERR_BAD_OPERATION; } - } else - osafassert(0); + } else { + LOG_WA(Ignoring unknown attribute '%s', attribute-attrName); + } } return SA_AIS_OK; @@ -144,8 +145,9 @@ static
Re: [devel] [PATCH 0 of 1] Review Request for imm: Allow admin-operations directly targeting an implementer/applier [#799]
Will push this tomorrow if there are no objections. /AndersBj -Original Message- From: Anders Bjornerstedt [mailto:anders.bjornerst...@ericsson.com] Sent: den 29 april 2014 16:37 To: reddy.neelaka...@oracle.com; Zoran Milinkovic Cc: opensaf-devel@lists.sourceforge.net Subject: [devel] [PATCH 0 of 1] Review Request for imm: Allow admin-operations directly targeting an implementer/applier [#799] Summary: imm: Allow admin-operations directly targeting an implementer/applier [#799] Review request for Trac Ticket(s): 799 Peer Reviewer(s): Neel; Zoran Pull request to: Affected branch(es): default(4.5) Development branch: 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 changeset f3bd590108f2bdb2bada136baf9b31bc8d963460 Author: Anders Bjornerstedt anders.bjornerst...@ericsson.com Date: Tue, 29 Apr 2014 16:17:10 +0200 imm: Allow admin-operations directly targeting an implementer/applier [#799] This enhancement (#799) extends the scope of the existing OpenSAF API for administrative-operations such that it supports the direct invocation of admin-operations on an implementer or applier. See the included patch of: osaf/services/saf/immsv/README for details. Complete diffstat: -- osaf/libs/agents/saf/imma/imma_oi_api.c | 6 osaf/services/saf/immsv/README | 84 +++- osaf/services/saf/immsv/immnd/ImmModel.cc| 14 +++ osaf/tools/safimm/immadm/imm_admin.c | 7 - tests/immsv/implementer/test_SaImmOiAdminOperation.c | 34 +++ 5 files changed, 131 insertions(+), 14 deletions(-) Testing Commands: - A new test case is included as 'immoitest 5 13'. Manual testing is also possible using the tool 'immapplier' to set up an OI or an applier; and the 'immadm' tool to invoke an admin operation. Set up an OI for example on SC1: immapplier -a MyOI OpensafImmTest The class name is only needed here tpo satisfy the immapplier tool. You could uswe any class that does not have an existing OI, or use *any* class and set up as an @pplier. Invoke an admin-op towards the OI. Use the OI-name (or applier name) plus explicitly set admin-owner also to be the same OI-name (or applier name): immadm -a MyOI -o 1 MyOI You can also try invoking bogus admin-operations directly on say the saAmfService: immadm -a safAmfService -o 4711 safAmfService AdminOwnerName == ImplementerName (safAmfService) - Could be direct admin-op on OI error - saImmOmAdminOperationInvoke_2 admin-op RETURNED: SA_AIS_ERR_INVALID_PARAM (7) error-string: Admin operation not supported for safAmfService (0) This is a good test of the robustness of the OIs. Some OIs are not so robust and crash only because they receive an unexpected admin-operation. Testing, Expected Results: -- Invoking an admin-opertion directly on OI or applier should result in the admin-op reaching that OI or applier. Conditions of Submission: - Ack from Neel (Ack from Zoran not mandatory) Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 n n 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
[devel] [PATCH 1 of 3] imm: add support for configurable OI callback timeout [#16]
osaf/libs/common/immsv/immsv_evt.c | 56 ++- osaf/libs/common/immsv/include/immsv_evt.h | 4 + osaf/libs/common/immsv/include/immsv_evt_model.h | 1 + osaf/services/saf/immsv/immd/immd_evt.c | 9 ++- osaf/services/saf/immsv/immnd/ImmModel.cc| 89 ++- osaf/services/saf/immsv/immnd/ImmModel.hh| 6 +- osaf/services/saf/immsv/immnd/immnd_evt.c| 38 - osaf/services/saf/immsv/immnd/immnd_init.h | 8 +- 8 files changed, 175 insertions(+), 36 deletions(-) When an implementer is set, the configurable timeout is sent to IMM service. The configurable timeout is used for calculating timeouts for OI callbacks and waiting on search replies from RTA update callback. diff --git a/osaf/libs/common/immsv/immsv_evt.c b/osaf/libs/common/immsv/immsv_evt.c --- a/osaf/libs/common/immsv/immsv_evt.c +++ b/osaf/libs/common/immsv/immsv_evt.c @@ -64,6 +64,7 @@ static const char *immd_evt_names[] = { IMMD_EVT_ND2D_FEVS_REQ_2, IMMD_EVT_ND2D_LOADING_COMPLETED, IMMD_EVT_ND2D_2PBE_PRELOAD, + IMMD_EVT_ND2D_IMPLSET_REQ_2, undefined (high) }; @@ -173,6 +174,8 @@ static const char *immnd_evt_names[] = { IMMND_EVT_A2ND_CL_TIMEOUT, IMMND_EVT_A2ND_ACCESSOR_GET, IMMND_EVT_A2ND_CCB_VALIDATE, /* saImmOmCcbValidate */ + IMMND_EVT_A2ND_OI_IMPL_SET_2, /* saImmOiImplementerSet */ + IMMND_EVT_D2ND_IMPLSET_RSP_2, /* Implementer set reply from D with impl id */ undefined (high) }; @@ -1511,7 +1514,8 @@ static uint32_t immsv_evt_enc_sublevels( (i_evt-info.immd.type == IMMD_EVT_ND2D_FEVS_REQ_2)) { IMMSV_OCTET_STRING *os = (i_evt-info.immd.info.fevsReq.msg); immsv_evt_enc_inline_string(o_ub, os); - } else if (i_evt-info.immd.type == IMMD_EVT_ND2D_IMPLSET_REQ) { + } else if ((i_evt-info.immd.type == IMMD_EVT_ND2D_IMPLSET_REQ) || + (i_evt-info.immd.type == IMMD_EVT_ND2D_IMPLSET_REQ_2)) { IMMSV_OCTET_STRING *os = (i_evt-info.immd.info.impl_set.r.impl_name); if(!immsv_evt_enc_inline_text(__LINE__, o_ub, os)) { return NCSCC_RC_OUT_OF_MEM; @@ -1564,7 +1568,9 @@ static uint32_t immsv_evt_enc_sublevels( IMMSV_OCTET_STRING *os = (i_evt-info.immnd.info.fevsReq.msg); immsv_evt_enc_inline_string(o_ub, os); } else if ((i_evt-info.immnd.type == IMMND_EVT_A2ND_OI_IMPL_SET) || + (i_evt-info.immnd.type == IMMND_EVT_A2ND_OI_IMPL_SET_2) || (i_evt-info.immnd.type == IMMND_EVT_D2ND_IMPLSET_RSP) || + (i_evt-info.immnd.type == IMMND_EVT_D2ND_IMPLSET_RSP_2) || (i_evt-info.immnd.type == IMMND_EVT_A2ND_OI_CL_IMPL_SET) || (i_evt-info.immnd.type == IMMND_EVT_A2ND_OI_CL_IMPL_REL) || (i_evt-info.immnd.type == IMMND_EVT_A2ND_OI_OBJ_IMPL_SET) || @@ -2141,7 +2147,8 @@ static uint32_t immsv_evt_dec_sublevels( (o_evt-info.immd.type == IMMD_EVT_ND2D_FEVS_REQ_2)) { IMMSV_OCTET_STRING *os = (o_evt-info.immd.info.fevsReq.msg); immsv_evt_dec_inline_string(i_ub, os); - } else if (o_evt-info.immd.type == IMMD_EVT_ND2D_IMPLSET_REQ) { + } else if ((o_evt-info.immd.type == IMMD_EVT_ND2D_IMPLSET_REQ) || + (o_evt-info.immd.type == IMMD_EVT_ND2D_IMPLSET_REQ_2)) { IMMSV_OCTET_STRING *os = (o_evt-info.immd.info.impl_set.r.impl_name); immsv_evt_dec_inline_string(i_ub, os); } else if (o_evt-info.immd.type == IMMD_EVT_ND2D_OI_OBJ_MODIFY) { @@ -2177,7 +2184,9 @@ static uint32_t immsv_evt_dec_sublevels( immsv_evt_dec_inline_string(i_ub, os); } else if ((o_evt-info.immnd.type == IMMND_EVT_A2ND_OI_IMPL_SET) || + (o_evt-info.immnd.type == IMMND_EVT_A2ND_OI_IMPL_SET_2) || (o_evt-info.immnd.type == IMMND_EVT_D2ND_IMPLSET_RSP) || + (o_evt-info.immnd.type == IMMND_EVT_D2ND_IMPLSET_RSP_2) || (o_evt-info.immnd.type == IMMND_EVT_A2ND_OI_CL_IMPL_SET) || (o_evt-info.immnd.type == IMMND_EVT_A2ND_OI_CL_IMPL_REL) || (o_evt-info.immnd.type == IMMND_EVT_A2ND_OI_OBJ_IMPL_SET) || @@ -2949,6 +2958,7 @@ static uint32_t immsv_evt_enc_toplevel(I break; case IMMD_EVT_ND2D_IMPLSET_REQ: /*OiImplementerSet */ + case IMMD_EVT_ND2D_IMPLSET_REQ_2: /*OiImplementerSet */ IMMSV_RSRV_SPACE_ASSERT(p8, o_ub, 8);
[devel] [PATCH 3 of 3] immtests: add test cases for OI callback timeout [#16]
tests/immsv/implementer/test_cleanup.c |1 + tests/immsv/implementer/test_saImmOiImplementerSet.c | 287 +++ 2 files changed, 288 insertions(+), 0 deletions(-) Test cases cover OI callback timeout and timeout on searching for an object with RTA diff --git a/tests/immsv/implementer/test_cleanup.c b/tests/immsv/implementer/test_cleanup.c --- a/tests/immsv/implementer/test_cleanup.c +++ b/tests/immsv/implementer/test_cleanup.c @@ -31,6 +31,7 @@ static char *objects[] = { 123456789012345678901234567890123456789012345678901234567890123,123456789012345678901234567890123456789012345678901234567890123,123456789012345678901234567890123456789012345678901234567890123,rdn=root, Test,rdn=root, rdn=root, + obj=1, NULL }; diff --git a/tests/immsv/implementer/test_saImmOiImplementerSet.c b/tests/immsv/implementer/test_saImmOiImplementerSet.c --- a/tests/immsv/implementer/test_saImmOiImplementerSet.c +++ b/tests/immsv/implementer/test_saImmOiImplementerSet.c @@ -16,6 +16,10 @@ */ #include immtest.h +#include unistd.h +#include pthread.h +#include poll.h +#include stdlib.h void saImmOiImplementerSet_01(void) { @@ -70,6 +74,284 @@ void saImmOiImplementerSet_05(void) safassert(saImmOiFinalize(immOiHandle), SA_AIS_OK); } +int saImmOiImplementerSet_callback_wait = 15; +SaImmClassNameT saImmOiImplementerSet_className = NULL; + +static SaAisErrorT saImmOiImplementerSet_ModifyCallback(SaImmOiHandleT immOiHandle, SaImmOiCcbIdT ccbId, + const SaNameT *objectName, const SaImmAttrModificationT_2 **attrMods) { + + sleep(saImmOiImplementerSet_callback_wait); + return SA_AIS_OK; +} + +static SaAisErrorT saImmOiImplementerSet_RtAttrUpdateCallbackT(SaImmOiHandleT immOiHandle, const SaNameT *objectName, const SaImmAttrNameT *attributeNames) { + SaUint32T attrVal = (SaUint32T)random(); + SaImmAttrValueT attrValues[1] = { attrVal }; + SaImmAttrModificationT_2 attrMod = { SA_IMM_ATTR_VALUES_REPLACE, { attr1, SA_IMM_ATTR_SAUINT32T, 1, attrValues } }; + const SaImmAttrModificationT_2 *attrMods[2] = { attrMod, NULL }; + + sleep(saImmOiImplementerSet_callback_wait); + + saImmOiRtObjectUpdate_2(immOiHandle, objectName, attrMods); + + return SA_AIS_OK; +} + +void *saImmOiImplementerSet_modify_thread(void *arg) { + int *ready = (int *)arg; + SaImmOiHandleT immOiHandle; + SaSelectionObjectT selObj; + SaImmOiCallbacksT_2 configImmOiCallbacks = { NULL , NULL , NULL , NULL , NULL , NULL , saImmOiImplementerSet_ModifyCallback , NULL }; + SaImmOiCallbacksT_2 rtImmOiCallbacks = { NULL , NULL , NULL , NULL , NULL , NULL , NULL , saImmOiImplementerSet_RtAttrUpdateCallbackT }; + SaNameT rdn = { 5, obj=1 }; + struct pollfd pfd; + int rc = 1; + int config = 1; + + assert(saImmOiImplementerSet_className != NULL); + + if(!strcmp(saImmOiImplementerSet_className, runtimeClassName)) { + SaImmAttrValueT attrVal[1] = { rdn }; + SaImmAttrValuesT_2 rdnVal = { rdn, SA_IMM_ATTR_SANAMET, 1, attrVal }; + const SaImmAttrValuesT_2 *attrValues[2] = { rdnVal, NULL }; + + safassert(saImmOiInitialize_2(immOiHandle, rtImmOiCallbacks, immVersion), SA_AIS_OK); + safassert(saImmOiSelectionObjectGet(immOiHandle, selObj), SA_AIS_OK); + safassert(saImmOiImplementerSet(immOiHandle, (SaImmOiImplementerNameT)__FUNCTION__), SA_AIS_OK); + safassert(saImmOiRtObjectCreate_2(immOiHandle, runtimeClassName, NULL, attrValues), SA_AIS_OK); + config = 0; + } else { + safassert(saImmOiInitialize_2(immOiHandle, configImmOiCallbacks, immVersion), SA_AIS_OK); + safassert(saImmOiSelectionObjectGet(immOiHandle, selObj), SA_AIS_OK); + safassert(saImmOiImplementerSet(immOiHandle, (SaImmOiImplementerNameT)__FUNCTION__), SA_AIS_OK); + safassert(saImmOiClassImplementerSet(immOiHandle, saImmOiImplementerSet_className), SA_AIS_OK); + } + + *ready = 1; + + while((!rc || rc == 1) *ready) { + pfd.fd = selObj; + pfd.events = POLLIN; + rc = poll(pfd, 1, 100); + + safassert(saImmOiDispatch(immOiHandle, SA_DISPATCH_ONE), SA_AIS_OK); + } + + if(!config) { + safassert(saImmOiRtObjectDelete(immOiHandle, rdn), SA_AIS_OK); + } else { + safassert(saImmOiClassImplementerRelease(immOiHandle, saImmOiImplementerSet_className), SA_AIS_OK); + } + + safassert(saImmOiImplementerClear(immOiHandle), SA_AIS_OK); + safassert(saImmOiFinalize(immOiHandle), SA_AIS_OK); + + *ready = 1; + + return NULL; +} + +void saImmOiImplementerSet_06(void) +{ + SaImmHandleT immHandle; + SaImmAdminOwnerHandleT ownerHandle; +
[devel] [PATCH 2 of 3] imm: use IMMA_OI_CALLBACK_TIMEOUT for setting OI callback timeout [#16]
osaf/libs/agents/saf/imma/imma_cb.h | 1 + osaf/libs/agents/saf/imma/imma_def.h| 1 + osaf/libs/agents/saf/imma/imma_oi_api.c | 23 ++- 3 files changed, 24 insertions(+), 1 deletions(-) IMMA uses IMMA_OI_CALLBACK_TIMEOUT for sending OI timeout to IMM service using implementer set operations. diff --git a/osaf/libs/agents/saf/imma/imma_cb.h b/osaf/libs/agents/saf/imma/imma_cb.h --- a/osaf/libs/agents/saf/imma/imma_cb.h +++ b/osaf/libs/agents/saf/imma/imma_cb.h @@ -71,6 +71,7 @@ typedef struct imma_client_node { * enviroment variable IMMA_MAX_OPEN_SEARCHES_PER_HANDLE */ uint32_t maxSearchHandles; uint32_t searchHandleSize; /* Number of open search handles */ + uint32_t oiTimeout; /* Timeout for OI callback. If the value is 0, the default timeout (6s) will be used */ } IMMA_CLIENT_NODE; /* Node to store adminOwner info */ diff --git a/osaf/libs/agents/saf/imma/imma_def.h b/osaf/libs/agents/saf/imma/imma_def.h --- a/osaf/libs/agents/saf/imma/imma_def.h +++ b/osaf/libs/agents/saf/imma/imma_def.h @@ -25,6 +25,7 @@ #define IMMA_MINOR_VERSION 0x0e #define IMMSV_WAIT_TIME 1000 /* Default MDS wait time in 10ms units =10 sec*/ +#define IMMSV_OI_CALLBACK_WAIT_TIME 6 /* Default wait time for OI callback to reply in seconds = 6 sec*/ diff --git a/osaf/libs/agents/saf/imma/imma_oi_api.c b/osaf/libs/agents/saf/imma/imma_oi_api.c --- a/osaf/libs/agents/saf/imma/imma_oi_api.c +++ b/osaf/libs/agents/saf/imma/imma_oi_api.c @@ -139,6 +139,21 @@ SaAisErrorT saImmOiInitialize_2(SaImmOiH cl_node-syncr_timeout = IMMSV_WAIT_TIME; /* Default */ } + if((timeout_env_value = getenv(IMMA_OI_CALLBACK_TIMEOUT))!=NULL) { + char *endp = NULL; + cl_node-oiTimeout = strtol(timeout_env_value, endp, 10); + if(!endp || *endp) { + TRACE_2(Failed to parse IMMA_OI_CALLBACK_TIMEOUT. + OI timeout will be set to the default value: %u, + IMMSV_OI_CALLBACK_WAIT_TIME); + cl_node-oiTimeout = IMMSV_OI_CALLBACK_WAIT_TIME; + } else { + TRACE_2(IMMA library OI timeout set to:%u, cl_node-oiTimeout); + } + } else { + cl_node-oiTimeout = IMMSV_OI_CALLBACK_WAIT_TIME; + } + /* Take the CB Lock */ if (m_NCS_LOCK(cb-cb_lock, NCS_LOCK_WRITE) != NCSCC_RC_SUCCESS) { TRACE_4(ERR_LIBRARY: LOCK failed); @@ -1276,11 +1291,17 @@ SaAisErrorT saImmOiImplementerSet(SaImmO /* Populate Send the Open Event to IMMND */ memset(evt, 0, sizeof(IMMSV_EVT)); evt.type = IMMSV_EVT_TYPE_IMMND; - evt.info.immnd.type = IMMND_EVT_A2ND_OI_IMPL_SET; evt.info.immnd.info.implSet.client_hdl = immOiHandle; evt.info.immnd.info.implSet.impl_name.size = nameLen; evt.info.immnd.info.implSet.impl_name.buf = implementerName; + if(cl_node-isImmA2d cl_node-oiTimeout != IMMSV_OI_CALLBACK_WAIT_TIME) { + evt.info.immnd.info.implSet.oi_timeout = cl_node-oiTimeout; + evt.info.immnd.type = IMMND_EVT_A2ND_OI_IMPL_SET_2; + } else { + evt.info.immnd.type = IMMND_EVT_A2ND_OI_IMPL_SET; + } + /* Unlock before MDS Send */ m_NCS_UNLOCK(cb-cb_lock, NCS_LOCK_WRITE); locked = false; -- Is your legacy SCM system holding you back? Join Perforce May 7 to find out: #149; 3 signs your SCM is hindering your productivity #149; Requirements for releasing software faster #149; Expert tips and advice for migrating your SCM now http://p.sf.net/sfu/perforce ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] Default value of saAmfSGAutoRepair in opensaf models
Should we change value for saAmfSGAutoRepair (in osaf/services/saf/amf/config/amf_objects.xml) to have the value True since we now actually support this? Thanks, Hans -- Is your legacy SCM system holding you back? Join Perforce May 7 to find out: #149; 3 signs your SCM is hindering your productivity #149; Requirements for releasing software faster #149; Expert tips and advice for migrating your SCM now http://p.sf.net/sfu/perforce ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] AMF: remove support for MDS_CALLBACK_COPY?
According to MDS documentation COPY support is needed when sending messages to the same MDS core which I interpret as the same linux process. Since AMF does not do that (correct me if I am wrong) this code can/should be removed. I added an assert in the COPY callback and nothing happens... Comments? Thanks, Hans -- Is your legacy SCM system holding you back? Join Perforce May 7 to find out: #149; 3 signs your SCM is hindering your productivity #149; Requirements for releasing software faster #149; Expert tips and advice for migrating your SCM now http://p.sf.net/sfu/perforce ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 1 of 1] osaf: Fix compilation errors when building with GCC 4.9.0 [#883]
osaf/libs/core/common/ncs_main_pub.c | 2 +- osaf/libs/core/common/ncs_sprr.c | 2 +- osaf/libs/core/include/ncssysf_def.h | 4 ++-- osaf/libs/core/leap/hj_dec.c | 4 ++-- osaf/libs/core/leap/hj_enc.c | 8 osaf/libs/core/leap/hj_hdl.c | 18 +- osaf/libs/core/leap/hj_ubaid.c | 6 +++--- osaf/libs/core/leap/sysf_ipc.c | 6 +++--- osaf/libs/core/leap/sysf_mem.c | 4 ++-- osaf/libs/core/leap/sysf_tmr.c | 2 +- osaf/services/infrastructure/fm/fms/fm_mds.c | 2 +- osaf/services/saf/clmsv/clms/clms_mbcsv.c| 2 +- osaf/services/saf/cpsv/cpd/cpd_amf.c | 4 ++-- osaf/services/saf/cpsv/cpd/cpd_mds.c | 2 +- osaf/services/saf/glsv/gld/gld_evt.c | 2 +- osaf/services/saf/logsv/lgs/lgs_mbcsv.c | 2 +- osaf/services/saf/ntfsv/ntfs/ntfs_mbcsv.c| 2 +- 17 files changed, 36 insertions(+), 36 deletions(-) OpenSAF did not build successfully with GCC 4.9.0, due to a new warning: In file included from ncs_sprr.c:37:0: ncs_sprr.c: In function 'ncs_splr_api': ../../../../osaf/libs/core/include/ncssysf_def.h:105:54: error: right-hand operand of comma expression has no effect [-Werror=unused-value] #define m_LEAP_DBG_SINK(r) (TRACE(IN LEAP_DBG_SINK), r) ^ ncs_sprr.c:46:58: note: in expansion of macro 'm_LEAP_DBG_SINK' #define m_NCS_SPRR_DBG_SINK(x,y) printf(SPRR:%s\n, y),m_LEAP_DBG_SINK(x) ^ ncs_sprr.c:237:9: note: in expansion of macro 'm_NCS_SPRR_DBG_SINK' rc = m_NCS_SPRR_DBG_SINK(NCSCC_RC_DUPLICATE_ENTRY, SPLR duplication attempted); The warning actually pointed out a rather tricky bug in ncs_sprr.c, that is not obvious the first time you look at the code. The bug is that the comma operator is used within a C preprocessor macro, without surrounding parentheses. When this macro is used in an assignment statement, the code does not do what you would expect, since the comma operator has lower precedence than the assignment operator in the C language. By adding parentheses around the macro definition, this bug is solved. diff --git a/osaf/libs/core/common/ncs_main_pub.c b/osaf/libs/core/common/ncs_main_pub.c --- a/osaf/libs/core/common/ncs_main_pub.c +++ b/osaf/libs/core/common/ncs_main_pub.c @@ -759,7 +759,7 @@ void ncs_get_sys_params_arg(NCS_SYS_PARA if (m_NCS_GET_PHYINFO_FROM_NODE_ID(sys_params-node_id, sys_params-shelf_id, sys_params-slot_id, sub_slot_id) != NCSCC_RC_SUCCESS) { - m_LEAP_DBG_SINK(NCSCC_RC_FAILURE); + m_LEAP_DBG_SINK_VOID; return; } diff --git a/osaf/libs/core/common/ncs_sprr.c b/osaf/libs/core/common/ncs_sprr.c --- a/osaf/libs/core/common/ncs_sprr.c +++ b/osaf/libs/core/common/ncs_sprr.c @@ -43,7 +43,7 @@ #ifdef NDEBUG #define m_NCS_SPRR_DBG_SINK(x,y) (x) #else -#define m_NCS_SPRR_DBG_SINK(x,y) printf(SPRR:%s\n, y),m_LEAP_DBG_SINK(x) +#define m_NCS_SPRR_DBG_SINK(x,y) (printf(SPRR:%s\n, y),m_LEAP_DBG_SINK(x)) #endif #define m_NCSSPRR_TRACE_ARG2(x,y) diff --git a/osaf/libs/core/include/ncssysf_def.h b/osaf/libs/core/include/ncssysf_def.h --- a/osaf/libs/core/include/ncssysf_def.h +++ b/osaf/libs/core/include/ncssysf_def.h @@ -91,8 +91,8 @@ void opensaf_reboot(unsigned node_id, co ** ** / -#define m_KEY_CHK_FMT(k,f) { if (k.fmat != f) m_LEAP_DBG_SINK(0);} -#define m_KEY_CHK_LEN(l){ if (l SYSF_MAX_KEY_LEN) m_LEAP_DBG_SINK(0); } +#define m_KEY_CHK_FMT(k,f) { if (k.fmat != f) m_LEAP_DBG_SINK_VOID;} +#define m_KEY_CHK_LEN(l){ if (l SYSF_MAX_KEY_LEN) m_LEAP_DBG_SINK_VOID; } #define m_KEY_CHK_SLEN(s) { uint32_t l = m_NCS_STRLEN(s); m_KEY_CHK_LEN(l); } /* diff --git a/osaf/libs/core/leap/hj_dec.c b/osaf/libs/core/leap/hj_dec.c --- a/osaf/libs/core/leap/hj_dec.c +++ b/osaf/libs/core/leap/hj_dec.c @@ -68,7 +68,7 @@ USRBUF *ncs_decode_n_octets(USRBUF *u, u **/ if ((s = m_MMGR_DATA_AT_START(u, count, (char *)os)) != (char *)os) { if (s == 0) { - m_LEAP_DBG_SINK(0); + m_LEAP_DBG_SINK_VOID; return (USRBUF *)0; } memcpy(os, s, (size_t)count); @@ -84,7 +84,7 @@ USRBUF *ncs_decode_n_octets(USRBUF *u, u uint8_t *ncs_flatten_n_octets(USRBUF *u, uint8_t *os, uint32_t count) { if (u == BNULL) { - m_LEAP_DBG_SINK(0); + m_LEAP_DBG_SINK_VOID; return NULL; } diff --git a/osaf/libs/core/leap/hj_enc.c b/osaf/libs/core/leap/hj_enc.c --- a/osaf/libs/core/leap/hj_enc.c +++ b/osaf/libs/core/leap/hj_enc.c @@ -171,7 +171,7
Re: [devel] [PATCH 1 of 1] osaf: Fix compilation errors when building with GCC 4.9.0 [#883]
Yes that should ALSO be done. :-) There are probably several more places where we have printf. This patch just fixes the missing parentheses. I could fix the printf too, while touching that line. / Anders Widell On 05/08/2014 03:33 PM, Hans Feldt wrote: Why not just replace with trace? /Hans -Original Message- From: Anders Widell [mailto:anders.wid...@ericsson.com] Sent: den 8 maj 2014 15:04 To: ramesh.bet...@oracle.com Cc: opensaf-devel@lists.sourceforge.net Subject: [devel] [PATCH 1 of 1] osaf: Fix compilation errors when building with GCC 4.9.0 [#883] osaf/libs/core/common/ncs_main_pub.c | 2 +- osaf/libs/core/common/ncs_sprr.c | 2 +- osaf/libs/core/include/ncssysf_def.h | 4 ++-- osaf/libs/core/leap/hj_dec.c | 4 ++-- osaf/libs/core/leap/hj_enc.c | 8 osaf/libs/core/leap/hj_hdl.c | 18 +- osaf/libs/core/leap/hj_ubaid.c | 6 +++--- osaf/libs/core/leap/sysf_ipc.c | 6 +++--- osaf/libs/core/leap/sysf_mem.c | 4 ++-- osaf/libs/core/leap/sysf_tmr.c | 2 +- osaf/services/infrastructure/fm/fms/fm_mds.c | 2 +- osaf/services/saf/clmsv/clms/clms_mbcsv.c| 2 +- osaf/services/saf/cpsv/cpd/cpd_amf.c | 4 ++-- osaf/services/saf/cpsv/cpd/cpd_mds.c | 2 +- osaf/services/saf/glsv/gld/gld_evt.c | 2 +- osaf/services/saf/logsv/lgs/lgs_mbcsv.c | 2 +- osaf/services/saf/ntfsv/ntfs/ntfs_mbcsv.c| 2 +- 17 files changed, 36 insertions(+), 36 deletions(-) OpenSAF did not build successfully with GCC 4.9.0, due to a new warning: In file included from ncs_sprr.c:37:0: ncs_sprr.c: In function 'ncs_splr_api': ../../../../osaf/libs/core/include/ncssysf_def.h:105:54: error: right-hand operand of comma expression has no effect [- Werror=unused-value] #define m_LEAP_DBG_SINK(r) (TRACE(IN LEAP_DBG_SINK), r) ^ ncs_sprr.c:46:58: note: in expansion of macro 'm_LEAP_DBG_SINK' #define m_NCS_SPRR_DBG_SINK(x,y) printf(SPRR:%s\n, y),m_LEAP_DBG_SINK(x) ^ ncs_sprr.c:237:9: note: in expansion of macro 'm_NCS_SPRR_DBG_SINK' rc = m_NCS_SPRR_DBG_SINK(NCSCC_RC_DUPLICATE_ENTRY, SPLR duplication attempted); The warning actually pointed out a rather tricky bug in ncs_sprr.c, that is not obvious the first time you look at the code. The bug is that the comma operator is used within a C preprocessor macro, without surrounding parentheses. When this macro is used in an assignment statement, the code does not do what you would expect, since the comma operator has lower precedence than the assignment operator in the C language. By adding parentheses around the macro definition, this bug is solved. diff --git a/osaf/libs/core/common/ncs_main_pub.c b/osaf/libs/core/common/ncs_main_pub.c --- a/osaf/libs/core/common/ncs_main_pub.c +++ b/osaf/libs/core/common/ncs_main_pub.c @@ -759,7 +759,7 @@ void ncs_get_sys_params_arg(NCS_SYS_PARA if (m_NCS_GET_PHYINFO_FROM_NODE_ID(sys_params-node_id, sys_params-shelf_id, sys_params-slot_id, sub_slot_id) != NCSCC_RC_SUCCESS) { -m_LEAP_DBG_SINK(NCSCC_RC_FAILURE); +m_LEAP_DBG_SINK_VOID; return; } diff --git a/osaf/libs/core/common/ncs_sprr.c b/osaf/libs/core/common/ncs_sprr.c --- a/osaf/libs/core/common/ncs_sprr.c +++ b/osaf/libs/core/common/ncs_sprr.c @@ -43,7 +43,7 @@ #ifdef NDEBUG #define m_NCS_SPRR_DBG_SINK(x,y) (x) #else -#define m_NCS_SPRR_DBG_SINK(x,y) printf(SPRR:%s\n, y),m_LEAP_DBG_SINK(x) +#define m_NCS_SPRR_DBG_SINK(x,y) (printf(SPRR:%s\n, y),m_LEAP_DBG_SINK(x)) #endif #define m_NCSSPRR_TRACE_ARG2(x,y) diff --git a/osaf/libs/core/include/ncssysf_def.h b/osaf/libs/core/include/ncssysf_def.h --- a/osaf/libs/core/include/ncssysf_def.h +++ b/osaf/libs/core/include/ncssysf_def.h @@ -91,8 +91,8 @@ void opensaf_reboot(unsigned node_id, co ** ** / -#define m_KEY_CHK_FMT(k,f) { if (k.fmat != f) m_LEAP_DBG_SINK(0);} -#define m_KEY_CHK_LEN(l){ if (l SYSF_MAX_KEY_LEN) m_LEAP_DBG_SINK(0); } +#define m_KEY_CHK_FMT(k,f) { if (k.fmat != f) m_LEAP_DBG_SINK_VOID;} +#define m_KEY_CHK_LEN(l){ if (l SYSF_MAX_KEY_LEN) m_LEAP_DBG_SINK_VOID; } #define m_KEY_CHK_SLEN(s) { uint32_t l = m_NCS_STRLEN(s); m_KEY_CHK_LEN(l); } /* diff --git a/osaf/libs/core/leap/hj_dec.c b/osaf/libs/core/leap/hj_dec.c --- a/osaf/libs/core/leap/hj_dec.c +++ b/osaf/libs/core/leap/hj_dec.c @@ -68,7 +68,7 @@ USRBUF *ncs_decode_n_octets(USRBUF *u, u **/ if ((s = m_MMGR_DATA_AT_START(u,
Re: [devel] [PATCH 0 of 3] Review Request for imm: add support for configurable OI callback timeout [#16]
Ack on these patches with the following comments: --- 1) In the imma client you have added: +#define IMMSV_OI_CALLBACK_WAIT_TIME 6 /* Default wait time for OI callback to reply in seconds = 6 sec*/ This default timeout is set in the client if the client has not explicitly defined IMMA_OI_CALLBACK_TIMEOUT. But this is redundant since the default is defined in the server (even before this fix). Another thing is that we want to be able in principle to change the default by changing it in ONE place. The natural place is the original place in the server, possibly at some point replaced by the configuration attribute in the existing SAF imm object mentionend in the ticket. The config attribute would then only control the default value and qould not override explicit client settings. WE dont want different clients getting different defaults depending on when they where compiled. IF the client has not explicitly defined IMMA_OI_CALLBACK_TIMEOUT then the implementer set should use the value 0 for the timeout in the message to the server and let the server do the defaulting according to todays (pre #16 fix) behavior. That is the behavior you still get when sending IMMND_EVT_A2ND_OI_IMPL_SET instead of IMMND_EVT_A2ND_OI_IMPL_SET_2. --- 2) I will update the immsv/README with a description of the new IMMA_OI_CALLBACK_TIMEOUT provided as an additional patch to this ticket. Tested with the new immoitest which is slooower since it is testing the oi-timeout setting mechanism. /AndersBj -Original Message- From: Zoran Milinkovic [mailto:zoran.milinko...@ericsson.com] Sent: den 8 maj 2014 13:46 To: reddy.neelaka...@oracle.com Cc: opensaf-devel@lists.sourceforge.net Subject: [devel] [PATCH 0 of 3] Review Request for imm: add support for configurable OI callback timeout [#16] Summary: imm: add support for configurable OI callback timeout [#16] Review request for Trac Ticket(s): 16 Peer Reviewer(s): Neelakanta, Anders Pull request to: Zoran Affected branch(es): default(4.5) Development branch: default(4.5) 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): - changeset da62b083125842a8e6b1437fdc6224b3f1e6720e Author: Zoran Milinkovic zoran.milinko...@ericsson.com Date: Thu, 08 May 2014 13:40:11 +0200 imm: add support for configurable OI callback timeout [#16] When an implementer is set, the configurable timeout is sent to IMM service. The configurable timeout is used for calculating timeouts for OI callbacks and waiting on search replies from RTA update callback. changeset bcd50a218ba95178ea86735ff322b722bdf577f7 Author: Zoran Milinkovic zoran.milinko...@ericsson.com Date: Thu, 08 May 2014 13:27:07 +0200 imm: use IMMA_OI_CALLBACK_TIMEOUT for setting OI callback timeout [#16] IMMA uses IMMA_OI_CALLBACK_TIMEOUT for sending OI timeout to IMM service using implementer set operations. changeset d11cb26e44f7554d6b22476d874086044d22d624 Author: Zoran Milinkovic zoran.milinko...@ericsson.com Date: Thu, 08 May 2014 13:41:57 +0200 immtests: add test cases for OI callback timeout [#16] Test cases cover OI callback timeout and timeout on searching for an object with RTA Complete diffstat: -- osaf/libs/agents/saf/imma/imma_cb.h |1 + osaf/libs/agents/saf/imma/imma_def.h |1 + osaf/libs/agents/saf/imma/imma_oi_api.c | 23 +++- osaf/libs/common/immsv/immsv_evt.c | 56 - osaf/libs/common/immsv/include/immsv_evt.h |4 ++ osaf/libs/common/immsv/include/immsv_evt_model.h |1 + osaf/services/saf/immsv/immd/immd_evt.c |9 - osaf/services/saf/immsv/immnd/ImmModel.cc| 89 +++-- osaf/services/saf/immsv/immnd/ImmModel.hh|6 ++- osaf/services/saf/immsv/immnd/immnd_evt.c| 38 --- osaf/services/saf/immsv/immnd/immnd_init.h |8 ++- tests/immsv/implementer/test_cleanup.c |1 + tests/immsv/implementer/test_saImmOiImplementerSet.c | 287 ++ 13 files changed, 487 insertions(+), 37 deletions(-) Testing Commands: - immoitest Testing, Expected Results: -- immoitest
[devel] [PATCH 1 of 1] imm: update immsv/README for IMMA_OI_CALLBACK_TIMEOUT [#16]
osaf/services/saf/immsv/README | 22 ++ 1 files changed, 22 insertions(+), 0 deletions(-) See osaf/services/saf/immsv/README for details on IMMA_OI_CALLBACK_TIMEOUT. diff --git a/osaf/services/saf/immsv/README b/osaf/services/saf/immsv/README --- a/osaf/services/saf/immsv/README +++ b/osaf/services/saf/immsv/README @@ -1848,6 +1848,28 @@ be resolved by the timeout of the synchr of progress on some task, not a permanently hung set of processes. +Support for configurable OI callback timeout (4.5) += +https://sourceforge.net/p/opensaf/tickets/16/ + +A new environment variable IMMA_OI_CALLBACK_TIMEOUT is recognized by the OI library. +In the same way as for the existing IMMA_SYNCR_TIMEOUT, this environment variable +is sampled by the SaImmOiInitialize function for initializing an oi-handle. + +The OI-handle will have that timeout associated with it for the rest of its lifetime, +for the server side monitoring of callbacks generated over the handle. + +The unit used for the value is seconds. + +Different OI handles can have different callback timeouts, by changing the value of +IMMA_OI_CALLBACK_TIMEOUT prior to each saImmOiIntitialize. + +If the environment variable is not defined, then the default OI timeout defined in +the server will be used by the server when monitoring callbacks to this OI. +Currently the default OI callback timeout is defined to be 6 seconds. It is not +recommended to set an OI callback timeout to a value higher than 10 seconds, since +that is the default om/oi-client side timeout for synchronous downcalls. + DEPENDENCIES -- Is your legacy SCM system holding you back? Join Perforce May 7 to find out: #149; 3 signs your SCM is hindering your productivity #149; Requirements for releasing software faster #149; Expert tips and advice for migrating your SCM now http://p.sf.net/sfu/perforce ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel