[devel] [PATCH 1 of 1] log: Make more configuration attributes possible to change in runtime [#1288]
osaf/services/saf/logsv/README | 21 +
osaf/services/saf/logsv/lgs/lgs_config.c | 50 +-
osaf/services/saf/logsv/lgs/lgs_config.h |1 +
osaf/services/saf/logsv/lgs/lgs_file.c |1 +
osaf/services/saf/logsv/lgs/lgs_imm.c| 224 +---
tests/logsv/logtest.c|2 +-
tests/logsv/tet_LogOiOps.c | 714 +-
7 files changed, 867 insertions(+), 146 deletions(-)
More log service configuration attributes can be changed in runtime
including logMaxLogrecsize
diff --git a/osaf/services/saf/logsv/README b/osaf/services/saf/logsv/README
--- a/osaf/services/saf/logsv/README
+++ b/osaf/services/saf/logsv/README
@@ -212,6 +212,8 @@ The following attributes can be changed
Can be changed to an existing path.
- logStreamFileFormat
+ - logMaxLogrecsize
+ - logFileIoTimeout
- logStreamSystemHighLimit
logStreamSystemLowLimit
@@ -500,6 +502,8 @@ in the same way as checkpointing is done
Configuration changes are now checkpointed using a variable length buffer
containing a cfgupd list.
+With this change, when attributes are check pointed,
+the list is always updated with all check pointed values.
CONTRIBUTORS/MAINTAINERS
@@ -711,3 +715,20 @@ of second from log timestamp (@Ck, @Nk).
- @Nk: for alarm and notification log stream.
Preceding and trailing zeros (0) if needed should exist.
+
+
+2. Log record size could be up to 32Kb (#1288)
+--
+With this ticket, logMaxLogrecsize and logFileIoTimeout attributes
+could be possible to change in runtime.
+
+logMaxLogrecsize attribute value could be up to 32768 (32Kb).
+
+logFileIoTimeout, this attribute defines the time period for
+which the main thread has to wait for the file IO operation to complete.
+The value is in milliseconds. In default, the main thread waits up to 500ms
+for file handling thread complete file IO operation.
+
+If user suffers the timeout when sending a big record size (e.g: 32Kb),
+He can adjust the waiting time by updating logFileIoTimeout attribute value up
to 5000ms.
+
diff --git a/osaf/services/saf/logsv/lgs/lgs_config.c
b/osaf/services/saf/logsv/lgs/lgs_config.c
--- a/osaf/services/saf/logsv/lgs/lgs_config.c
+++ b/osaf/services/saf/logsv/lgs/lgs_config.c
@@ -120,24 +120,22 @@ static struct {
SaUint32T logStreamSystemLowLimit;
SaUint32T logStreamAppHighLimit;
SaUint32T logStreamAppLowLimit;
-
- /* Not runtime configurable */
SaUint32T logMaxLogrecsize;
SaUint32T logMaxApplicationStreams;
SaUint32T logFileIoTimeout;
SaUint32T logFileSysConfig;
} lgs_conf_def = {
- PKGLOGDIR, /*logRootDirectory*/
- "", /*logDataGroupname*/
- DEFAULT_APP_SYS_FORMAT_EXP, /* logStreamFileFormat */
- 1024, /*logMaxLogrecsize*/
- 0, /*logStreamSystemHighLimit*/
- 0, /*logStreamSystemLowLimit*/
- 0, /*logStreamAppHighLimit*/
- 0, /*logStreamAppLowLimit*/
- 64, /*logMaxApplicationStreams*/
- 500,/*logFileIoTimeout*/
- 1, /*logFileSysConfig*/
+ .logRootDirectory = PKGLOGDIR, /*logRootDirectory*/
+ .logDataGroupname = "", /*logDataGroupname*/
+ .logStreamFileFormat = DEFAULT_APP_SYS_FORMAT_EXP, /*
logStreamFileFormat */
+ .logStreamSystemHighLimit = 0, /*logStreamSystemHighLimit*/
+ .logStreamSystemLowLimit = 0, /*logStreamSystemLowLimit*/
+ .logStreamAppHighLimit = 0, /*logStreamAppHighLimit*/
+ .logStreamAppLowLimit = 0, /*logStreamAppLowLimit*/
+ .logMaxLogrecsize = 1024, /*logMaxLogrecsize*/
+ .logMaxApplicationStreams = 64, /*logMaxApplicationStreams*/
+ .logFileIoTimeout = 500,/*logFileIoTimeout*/
+ .logFileSysConfig = 1, /*logFileSysConfig*/
};
static lgs_conf_t lgs_conf = {
@@ -544,9 +542,8 @@ int lgs_cfg_verify_log_file_format(const
/**
* Verify logMaxLogrecsize; Max size of a log record including header
* Rules:
- * - Because of an incorrect declaration in the server (uint16_t) that shall be
- * uint32_t the max value cannot be set > 2^15-1 (32767)
- * - Must be > 256
+ * - Must be >= 256
+ * - Must not larger than 65535 (UINT16_MAX)
*
* @param max_logrecsize_in[in]
* @return -1 on error
@@ -554,8 +551,7 @@ int lgs_cfg_verify_log_file_format(const
int lgs_cfg_verify_max_logrecsize(uint32_t max_logrecsize_in)
{
int rc = 0;
-
- if ((max_logrecsize_in > 32767) || (max_logrecsize_in < 256)) {
+ if ((max_logrecsize_in > 65535) || (max_logrecsize_in < 256)) {
LOG_NO("verify_max_logrecsize Fail");
rc = -1;
}
@@ -609,14 +605,24 @@ int lgs_cfg_verify_max_application_strea
/**
* Verify logFileIoTimeout
- * Rules: No rules defined will always return 0 (ok)
- *
+ * Rule
[devel] [PATCH 0 of 1] Review Request for log: Make more configuration attributes possible to change in runtime [#1288]
Summary: log: Make more configuration attributes possible to change in runtime Review request for Trac Ticket(s): #1288 Peer Reviewer(s): mathi.naic...@oracle.com Pull request to: <> Affected branch(es): devel 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): - NOTE! This patch must be applied on top of patch for #593 until #593 is acked and pushed! changeset 82593c4d2e7e06c2d1bd097f6e7eba1fb16e9e49 Author: Vu Minh Nguyen Date: Tue, 25 Aug 2015 10:36:40 +0200 log: Make more configuration attributes possible to change in runtime [#1288] More log service configuration attributes can be changed in runtime including logMaxLogrecsize Complete diffstat: -- osaf/services/saf/logsv/README | 21 +++ osaf/services/saf/logsv/lgs/lgs_config.c | 50 --- osaf/services/saf/logsv/lgs/lgs_config.h |1 + osaf/services/saf/logsv/lgs/lgs_file.c |1 + osaf/services/saf/logsv/lgs/lgs_imm.c| 224 +++- tests/logsv/logtest.c|2 +- tests/logsv/tet_LogOiOps.c | 714 + 7 files changed, 867 insertions(+), 146 deletions(-) Testing Commands: - # logtest New test cases has been added for testing runtime changable configuration attributes. Also see README for more info about which attributes that has become possible to change in runtime Testing, Expected Results: -- All tests shall PASS Conditions of Submission: - <> 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 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. -- ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] Release schedule for OpenSAF 4.7
Hi all! The code freeze for OpenSAF 4.7 is approaching. We are now aiming to tag OpenSAF 4.7.FC on October 1. This means that there is around five weeks left for implementing enhancements that are to be included in OpenSAF 4.7. When 4.7.FC has been tagged, we will only accept bug fixes on the 4.7 branch. regards, Anders Widell -- ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 1 of 1] amfd: Standby controller reboots if adding additional SI in N+M model [#1457]
osaf/services/saf/amf/amfd/si.cc | 89 +++-
1 files changed, 88 insertions(+), 1 deletions(-)
As N+M SU(s) are unlocked and having assignment, if dynamically adding an SI
(without specified saAmfSIAdminState) that causes the standby controller reboots
If additional SI is added without specified saAmfSIAdminState, the admin state
will be set as UNLOCKED as default. And if the configuration has enough
capability
to establish new assignment for additional SI, the active amfd will checkpoint
the current active assignment with standby amfd in the flow of CCB. Sometimes,
this checkpoint comes to standby amfd before standby amfd creates additional SI
by CCB apply. Therfore, standby amfd can't find the SI in checkpoint, then node
reboots by a false assert.
In AMF PR, 7.1.3 Add an SI in an SG without locking SG, the additonal SI should
be in admin LOCKED state while it's being added. The patch adds validation for
admin state of additional SI.
diff --git a/osaf/services/saf/amf/amfd/si.cc b/osaf/services/saf/amf/amfd/si.cc
--- a/osaf/services/saf/amf/amfd/si.cc
+++ b/osaf/services/saf/amf/amfd/si.cc
@@ -538,6 +538,89 @@ static int is_config_valid(const SaNameT
return 1;
}
+/**
+ * Validates if a SG's admin state is valid for creating a SI
+ * Should only be called if the SI admin state is UNLOCKED
+ * @param si_dn
+ * @param attributes
+ * @param opdata
+ * @return true if so
+ */
+static bool sg_admin_state_is_valid_for_si_create(const SaNameT *si_dn,
+const SaImmAttrValuesT_2 **attributes,
+const CcbUtilOperationData_t *opdata)
+{
+ SaNameT sg_name = {0};
+
+ if (immutil_getAttr(const_cast("saAmfSIProtectedbySG"),
+ attributes, 0, &sg_name) != SA_AIS_OK) {
+ TRACE("saAmfSIProtectedbySG is not specified for '%s'",
si_dn->value);
+ return false;
+ }
+
+ const AVD_SG *sg = sg_db->find(Amf::to_string(&sg_name));
+ if (sg != NULL) {
+ // SG had been created
+ // create SI is accepted under LOCKED SG, LOCKED-IN SG is fine
either
+ if (sg->saAmfSGAdminState != SA_AMF_ADMIN_LOCKED ||
+ sg->saAmfSGAdminState !=
SA_AMF_ADMIN_LOCKED_INSTANTIATION) {
+ TRACE("'%s' created UNLOCKED but '%s' is not locked",
+ si_dn->value, sg_name.value);
+ return false;
+ }
+ } else {
+ // SG does not exist in current model
+ // the check of sg existence should be already done in
is_config_valid
+ osafassert(ccbutil_getCcbOpDataByDN(opdata->ccbId, &sg_name));
+ }
+
+ return true;
+}
+
+/**
+ * Validation performed when a SI is dynamically created with a CCB.
+ * @param dn
+ * @param attributes
+ * @param opdata
+ *
+ * @return bool
+ */
+static bool is_ccb_create_config_valid(const SaNameT *dn,
+ const SaImmAttrValuesT_2 **attributes,
+ const CcbUtilOperationData_t *opdata)
+{
+ SaAmfAdminStateT admstate;
+ SaAisErrorT rc;
+
+ osafassert(opdata != NULL); // must be called in CCB context
+
+ rc = immutil_getAttr("saAmfSIAdminState", attributes, 0, &admstate);
+ // Default value is UNLOCKED if created without a value
+ if (rc != SA_AIS_OK)
+ admstate = SA_AMF_ADMIN_UNLOCKED;
+
+ // locked state is fine
+ if (admstate == SA_AMF_ADMIN_LOCKED)
+ return true;
+
+ if (admstate != SA_AMF_ADMIN_UNLOCKED) {
+ report_ccb_validation_error(opdata,
+ "'%s' created with invalid saAmfSIAdminState (%u)",
+ dn->value, admstate);
+ return false;
+ }
+
+ if (sg_admin_state_is_valid_for_si_create(dn, attributes, opdata))
+ return true;
+
+ amflog(SA_LOG_SEV_NOTICE, "CCB %d creation of '%s' failed",
+ opdata->ccbId, dn->value);
+ report_ccb_validation_error(opdata,
+ "SG is not configured properly to allow creation of UNLOCKED
SI");
+
+ return false;
+}
+
static AVD_SI *si_create(SaNameT *si_name, const SaImmAttrValuesT_2
**attributes)
{
unsigned int i;
@@ -1002,8 +1085,12 @@ static SaAisErrorT si_ccb_completed_cb(C
switch (opdata->operationType) {
case CCBUTIL_CREATE:
-if (is_config_valid(&opdata->objectName,
opdata->param.create.attrValues, opdata))
+if (is_config_valid(&opdata->objectName,
+ opdata->param.create.attrValues, opdata) &&
+ is_ccb_create_config_valid(&opdata->objectName,
+ opdata->param.create.attrValues,
opdata)) {
rc = SA_AIS_OK;
+}
break;
case CCBUTIL_MODIFY:
[devel] [PATCH 0 of 1] Review Request for Request for amfd: Standby controller reboots if adding additional SI in N+M model [#1457]
Summary: amfd: Standby controller reboots if adding additional SI in N+M model [#1457] Review request for Trac Ticket(s): 1457 Peer Reviewer(s): Hans N, Nagu, Praveen Pull request to: Affected branch(es): 4.5, 4.6, default 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 1736c73e54ff1d3dd9cec016887dde5ea4289df9 Author: Minh Hon Chau Date: Wed, 26 Aug 2015 14:45:43 +1000 amfd: Standby controller reboots if adding additional SI in N+M model [#1457] As N+M SU(s) are unlocked and having assignment, if dynamically adding an SI (without specified saAmfSIAdminState) that causes the standby controller reboots If additional SI is added without specified saAmfSIAdminState, the admin state will be set as UNLOCKED as default. And if the configuration has enough capability to establish new assignment for additional SI, the active amfd will checkpoint the current active assignment with standby amfd in the flow of CCB. Sometimes, this checkpoint comes to standby amfd before standby amfd creates additional SI by CCB apply. Therfore, standby amfd can't find the SI in checkpoint, then node reboots by a false assert. In AMF PR, 7.1.3 Add an SI in an SG without locking SG, the additonal SI should be in admin LOCKED state while it's being added. The patch adds validation for admin state of additional SI. Complete diffstat: -- osaf/services/saf/amf/amfd/si.cc | 89 +++- 1 files changed, 88 insertions(+), 1 deletions(-) Testing Commands: - - TC1: Repeat the test described in ticket #1457 - TC2: Repeat the same test with slight change in add_SIc.xml, which specifies SaAmfSIAdminState as 2 Testing, Expected Results: -- - TC1: CCB is rejected - TC2: CCB succeeds, then unlocking SI succeeds Conditions of Submission: - ack from reviewers 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 tes
[devel] [PATCH 1 of 1] amfa: return NOT_SUPPORTED for PM_ABNORMAL_END [#181]
osaf/libs/agents/saf/amfa/ava_api.c | 28 ++--
1 files changed, 22 insertions(+), 6 deletions(-)
Amf is returning SA_AIS_ERR_INVALID_PARAM for SA_AMF_PM_ABNORMAL_END and its
combination with other SaAmfPmErrorsT flags. This is incorrect behaviour
because SA_AMF_PM_ABNORMAL_END and its combination with other SaAmfPmErrorsT
flags are valid input to APIs saAmfPmStart and saAmfPmStop.
Since these combinations feature is not supported in Amf as of now,
so, Amf should return SA_AIS_ERR_NOT_SUPPORTED for such
combinations.
The fix provides the same.
diff --git a/osaf/libs/agents/saf/amfa/ava_api.c
b/osaf/libs/agents/saf/amfa/ava_api.c
--- a/osaf/libs/agents/saf/amfa/ava_api.c
+++ b/osaf/libs/agents/saf/amfa/ava_api.c
@@ -953,9 +953,17 @@ SaAisErrorT saAmfPmStart(SaAmfHandleT hd
/* input validation of pmError */
if (pmErr != SA_AMF_PM_NON_ZERO_EXIT && pmErr != SA_AMF_PM_ZERO_EXIT &&
- pmErr != (SA_AMF_PM_NON_ZERO_EXIT | SA_AMF_PM_ZERO_EXIT)) {
- TRACE_LEAVE2("Incorrect argument specified for SaAmfPmErrorsT
");
- return SA_AIS_ERR_INVALID_PARAM;
+ pmErr != (SA_AMF_PM_NON_ZERO_EXIT |
SA_AMF_PM_ZERO_EXIT)) {
+ if ((pmErr == SA_AMF_PM_ABNORMAL_END) ||
+ (pmErr == (SA_AMF_PM_NON_ZERO_EXIT |
SA_AMF_PM_ABNORMAL_END)) ||
+ (pmErr == (SA_AMF_PM_ZERO_EXIT |
SA_AMF_PM_ABNORMAL_END)) ||
+ (pmErr == (SA_AMF_PM_ZERO_EXIT |
SA_AMF_PM_NON_ZERO_EXIT | SA_AMF_PM_ABNORMAL_END))) {
+ TRACE_LEAVE2("Unsupported argument specified for
SaAmfPmErrorsT ");
+ return SA_AIS_ERR_NOT_SUPPORTED;
+ } else {
+ TRACE_LEAVE2("Incorrect argument specified for
SaAmfPmErrorsT ");
+ return SA_AIS_ERR_INVALID_PARAM;
+ }
}
/* input validation of Recomended recovery */
@@ -1064,9 +1072,17 @@ SaAisErrorT saAmfPmStop(SaAmfHandleT hdl
/* input validation of pmError */
if (pmErr != SA_AMF_PM_NON_ZERO_EXIT && pmErr != SA_AMF_PM_ZERO_EXIT &&
- pmErr != (SA_AMF_PM_NON_ZERO_EXIT | SA_AMF_PM_ZERO_EXIT)) {
- TRACE_LEAVE2("Incorrect argument specified for SaAmfPmErrorsT");
- return SA_AIS_ERR_INVALID_PARAM;
+ pmErr != (SA_AMF_PM_NON_ZERO_EXIT |
SA_AMF_PM_ZERO_EXIT)) {
+ if ((pmErr == SA_AMF_PM_ABNORMAL_END) ||
+ (pmErr == (SA_AMF_PM_NON_ZERO_EXIT |
SA_AMF_PM_ABNORMAL_END)) ||
+ (pmErr == (SA_AMF_PM_ZERO_EXIT |
SA_AMF_PM_ABNORMAL_END)) ||
+ (pmErr == (SA_AMF_PM_ZERO_EXIT |
SA_AMF_PM_NON_ZERO_EXIT | SA_AMF_PM_ABNORMAL_END))) {
+ TRACE_LEAVE2("Unsupported argument specified for
SaAmfPmErrorsT ");
+ return SA_AIS_ERR_NOT_SUPPORTED;
+ } else {
+ TRACE_LEAVE2("Incorrect argument specified for
SaAmfPmErrorsT");
+ return SA_AIS_ERR_INVALID_PARAM;
+ }
}
/* input validation of Process ID */
--
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 0 of 1] Review Request for amfa: return NOT_SUPPORTED for PM_ABNORMAL_END [#181]
Summary: amfa: return NOT_SUPPORTED for PM_ABNORMAL_END [#181] Review request for Trac Ticket(s): #181 Peer Reviewer(s): Praveen, Hans N Pull request to: Affected branch(es): Default 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 b7d495faec01256b24916a1b7df5ffc0b28a Author: Nagendra Kumar Date: Wed, 26 Aug 2015 10:40:00 +0530 amfa: return NOT_SUPPORTED for PM_ABNORMAL_END [#181] Amf is returning SA_AIS_ERR_INVALID_PARAM for SA_AMF_PM_ABNORMAL_END and its combination with other SaAmfPmErrorsT flags. This is incorrect behaviour because SA_AMF_PM_ABNORMAL_END and its combination with other SaAmfPmErrorsT flags are valid input to APIs saAmfPmStart and saAmfPmStop. Since these combinations feature is not supported in Amf as of now, so, Amf should return SA_AIS_ERR_NOT_SUPPORTED for such combinations. The fix provides the same. Complete diffstat: -- osaf/libs/agents/saf/amfa/ava_api.c | 28 ++-- 1 files changed, 22 insertions(+), 6 deletions(-) Testing Commands: - Start amf demo appl with saAmfPmStart and saAmfPmStop. 1. Pass pmErr as 1, 2, 3 in saAmfPmStart(Combination of SA_AMF_PM_NON_ZERO_EXIT, SA_AMF_PM_ZERO_EXIT). 2. Pass pmErr as 4, 5, 6, 7 in saAmfPmStart(Combination of SA_AMF_PM_NON_ZERO_EXIT, SA_AMF_PM_ZERO_EXIT and SA_AMF_PM_ABNORMAL_END). 3. Pass pmErr as 8, 10, 16 in saAmfPmStart(Some random values). 4. Perform #1, #2, #3 test cases for saAmfPmStart_3(It needs some modification in demo code for using B4.1 API). 5. Perform #1, #2, #3 test cases for saAmfPmStop. Testing, Expected Results: -- 1. Return value is 1(SA_AIS_OK). 2. Return value is 19(SA_AIS_ERR_NOT_SUPPORTED). 3. Return value is 7(SA_AIS_ERR_INVALID_PARAM). 3. Same result as #1, #2, #3. 4. Same result as #1, #2, #3. Conditions of Submission: - Ack from reviewers. 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 ser
Re: [devel] [PATCH 1 of 1] amfd: Convert AVD_AVND list_of_ncs_su and list_of_ncs_su to std::vector [#1142]
ack (not tested) - two minor comments below.
On 21/08/15 23:22, Hans Nordeback wrote:
> osaf/services/saf/amf/amfd/ckpt_dec.cc |2 +-
> osaf/services/saf/amf/amfd/clm.cc| 12 +-
> osaf/services/saf/amf/amfd/include/node.h| 27 +---
> osaf/services/saf/amf/amfd/include/su.h |1 -
> osaf/services/saf/amf/amfd/main.cc |3 +-
> osaf/services/saf/amf/amfd/ndfsm.cc | 15 +-
> osaf/services/saf/amf/amfd/ndproc.cc |7 +-
> osaf/services/saf/amf/amfd/node.cc | 161
> +-
> osaf/services/saf/amf/amfd/nodegroup.cc | 31 ++---
> osaf/services/saf/amf/amfd/nodeswbundle.cc |5 +-
> osaf/services/saf/amf/amfd/role.cc |6 +-
> osaf/services/saf/amf/amfd/sg_2n_fsm.cc | 21 +--
> osaf/services/saf/amf/amfd/sg_nored_fsm.cc | 27 +---
> osaf/services/saf/amf/amfd/sg_npm_fsm.cc | 28 +---
> osaf/services/saf/amf/amfd/sg_nway_fsm.cc| 39 ++
> osaf/services/saf/amf/amfd/sg_nwayact_fsm.cc | 25 +--
> osaf/services/saf/amf/amfd/sgproc.cc | 56 ++--
> osaf/services/saf/amf/amfd/su.cc |5 +-
> osaf/services/saf/amf/amfd/util.cc |4 +-
> 19 files changed, 152 insertions(+), 323 deletions(-)
>
>
> diff --git a/osaf/services/saf/amf/amfd/ckpt_dec.cc
> b/osaf/services/saf/amf/amfd/ckpt_dec.cc
> --- a/osaf/services/saf/amf/amfd/ckpt_dec.cc
> +++ b/osaf/services/saf/amf/amfd/ckpt_dec.cc
> @@ -3043,7 +3043,7 @@ static uint32_t dec_ng_admin_state(AVD_C
> AVD_AVND *node = avd_node_get(*iter);
> AVD_SU *su = NULL;
> //If this node has any susi on it.
> - for (su = node->list_of_su; su; su = su->avnd_list_su_next)
> + for (const auto& su : node->list_of_su)
> if (su->list_of_susi != NULL)
> break;
> if ((ng->saAmfNGAdminState == SA_AMF_ADMIN_SHUTTING_DOWN) &&
> (su != NULL))
> diff --git a/osaf/services/saf/amf/amfd/clm.cc
> b/osaf/services/saf/amf/amfd/clm.cc
> --- a/osaf/services/saf/amf/amfd/clm.cc
> +++ b/osaf/services/saf/amf/amfd/clm.cc
> @@ -26,8 +26,6 @@ static SaVersionT clmVersion = { 'B', 4,
>
> static void clm_node_join_complete(AVD_AVND *node)
> {
> - AVD_SU *su;
> -
> TRACE_ENTER();
> /* For each of the SUs calculate the readiness state.
>** call the SG FSM with the new readiness state.
> @@ -39,8 +37,7 @@ static void clm_node_join_complete(AVD_A
> }
>
> avd_node_oper_state_set(node, SA_AMF_OPERATIONAL_ENABLED);
> - su = node->list_of_su;
> - while (su != NULL) {
> + for (const auto& su : node->list_of_su) {
> /* For non-preinstantiable SU unlock-inst will not lead to its
> inst until unlock. */
> if ( su->saAmfSUPreInstantiable == false ) {
> /* Skip the instantiation. */
> @@ -68,8 +65,6 @@ static void clm_node_join_complete(AVD_A
> }
> }
> }
> - /* get the next SU on the node */
> - su = su->avnd_list_su_next;
> }
>
> node_reset_su_try_inst_counter(node);
> @@ -82,7 +77,6 @@ done:
> /* validating this node for a graceful exit */
> static void clm_node_exit_validate(AVD_AVND *node)
> {
> - AVD_SU *su;
> AVD_SU_SI_REL *susi;
> bool reject = false;
> SaAisErrorT rc = SA_AIS_OK;
> @@ -99,8 +93,7 @@ static void clm_node_exit_validate(AVD_A
>
> /* now go through each SU to determine whether
>any SI assigned becomes unassigned due to node exit*/
> - su = node->list_of_su;
> - while (su != NULL) {
> + for (const auto& su : node->list_of_su) {
> susi = su->list_of_susi;
> /* now evalutate each SI that is assigned to this SU */
> while (susi != NULL) {
> @@ -114,7 +107,6 @@ static void clm_node_exit_validate(AVD_A
> }
> susi = susi->su_next;
> }
> - su = su->avnd_list_su_next;
> }
>
> done:
> diff --git a/osaf/services/saf/amf/amfd/include/node.h
> b/osaf/services/saf/amf/amfd/include/node.h
> --- a/osaf/services/saf/amf/amfd/include/node.h
> +++ b/osaf/services/saf/amf/amfd/include/node.h
> @@ -43,6 +43,7 @@
> #include
> #include
> #include
> +#include
>
> class AVD_SU;
> struct avd_cluster_tag;
> @@ -76,6 +77,8 @@ class AVD_AVND {
>public:
> AVD_AVND();
> explicit AVD_AVND(const SaNameT* dn);
> +
> + bool is_node_lock();
> SaNameT name; /* DN */
> char *node_name;/* RDN value, normally the short host name */
> SaClmClusterNodeT_4 node_info;/* the node information of the node on
> @@ -112,10 +115,10 @@ class AVD_AVND {
>* Checkpointing - Sent independent update
>*/
>
[devel] [PATCH 0 of 1] Review Request for amfd: correct logic in si dep flow [#276]
Summary: amfd: correct logic in si dep flow [#276] Review request for Trac Ticket(s): #276 Peer Reviewer(s): Praveen, Hans N, Minh, Gary Pull request to: <> Affected branch(es): Default 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): - I am still testing it. I am floating it for early review because this looks only some logic correction and could be ok to review just the code. changeset 6ccb01038db382e520484eebfbbfe9b3b4bdc9c6 Author: Nagendra Kumar Date: Wed, 26 Aug 2015 11:44:50 +0530 amfd: correct logic in si dep flow [#276] At many places, there has been tautological errors in si dep flow. The fix corrects them Complete diffstat: -- osaf/services/saf/amf/amfd/si_dep.cc | 25 - 1 files changed, 12 insertions(+), 13 deletions(-) Testing Commands: - Started system controller. Testing, Expected Results: -- No errors. Conditions of Submission: - Ack from reviewers. 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. -- ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 1 of 1] amfd: correct logic in si dep flow [#276]
osaf/services/saf/amf/amfd/si_dep.cc | 25 -
1 files changed, 12 insertions(+), 13 deletions(-)
At many places, there has been tautological errors in si dep flow.
The fix corrects them
diff --git a/osaf/services/saf/amf/amfd/si_dep.cc
b/osaf/services/saf/amf/amfd/si_dep.cc
--- a/osaf/services/saf/amf/amfd/si_dep.cc
+++ b/osaf/services/saf/amf/amfd/si_dep.cc
@@ -1468,10 +1468,10 @@ void avd_sidep_unassign_dependents(AVD_S
if
(m_NCS_MDS_DEST_EQUAL(&sisu->su->su_on_node->adest,&su->su_on_node->adest)) {
avd_si_unassign(dep_si);
} else {
- if((dep_si->si_dep_state != AVD_SI_TOL_TIMER_RUNNING) ||
- (dep_si->si_dep_state !=
AVD_SI_READY_TO_UNASSIGN)) {
+ /* Don't start tol timer if dep state are either in
running or unassigned. */
+ if(!((dep_si->si_dep_state == AVD_SI_TOL_TIMER_RUNNING)
||
+ (dep_si->si_dep_state ==
AVD_SI_READY_TO_UNASSIGN))) {
avd_sidep_start_tolerance_timer_for_dependant(dep_si, si);
-
}
}
/* If this dependent SI is sponsor too, then unassign its
dependents also */
@@ -1788,9 +1788,9 @@ void avd_sidep_update_depstate_si_failov
if(su->su_on_node->saAmfNodeOperState ==
SA_AMF_OPERATIONAL_DISABLED) {
if
((m_NCS_MDS_DEST_EQUAL(&sisu->su->su_on_node->adest,&su->su_on_node->adest))) {
- if(((dep_si->si_dep_state !=
AVD_SI_TOL_TIMER_RUNNING) ||
- (dep_si->si_dep_state
!= AVD_SI_READY_TO_UNASSIGN) ||
- (dep_si->si_dep_state
!= AVD_SI_FAILOVER_UNDER_PROGRESS)) &&
+ if((!((dep_si->si_dep_state ==
AVD_SI_TOL_TIMER_RUNNING) ||
+ (dep_si->si_dep_state
== AVD_SI_READY_TO_UNASSIGN) ||
+ (dep_si->si_dep_state
== AVD_SI_FAILOVER_UNDER_PROGRESS))) &&
(avd_sidep_sponsors_assignment_states(dep_si))) {
avd_sidep_si_dep_state_set(dep_si, AVD_SI_FAILOVER_UNDER_PROGRESS);
@@ -1801,10 +1801,9 @@ void avd_sidep_update_depstate_si_failov
}
}
} else if (dep_si->sg_of_si == si->sg_of_si) {
- if((dep_si->si_dep_state !=
AVD_SI_TOL_TIMER_RUNNING) ||
- (dep_si->si_dep_state !=
AVD_SI_READY_TO_UNASSIGN) ||
- (dep_si->si_dep_state !=
AVD_SI_FAILOVER_UNDER_PROGRESS)) {
-
+ if(!((dep_si->si_dep_state ==
AVD_SI_TOL_TIMER_RUNNING) ||
+ (dep_si->si_dep_state ==
AVD_SI_READY_TO_UNASSIGN) ||
+ (dep_si->si_dep_state ==
AVD_SI_FAILOVER_UNDER_PROGRESS))) {
avd_sidep_si_dep_state_set(dep_si, AVD_SI_FAILOVER_UNDER_PROGRESS);
if (dep_si->num_dependents > 0)
{
/* This SI also has
dependents under it, update their state also */
@@ -1842,9 +1841,9 @@ void avd_sidep_update_depstate_si_failov
}
if (sponsor_assignments_state
== true) {
-
if((dep_si->si_dep_state != AVD_SI_TOL_TIMER_RUNNING) ||
-
(dep_si->si_dep_state != AVD_SI_READY_TO_UNASSIGN) ||
-
(dep_si->si_dep_state != AVD_SI_FAILOVER_UNDER_PROGRESS)) {
+
if(!((dep_si->si_dep_state == AVD_SI_TOL_TIMER_RUNNING) ||
+
(dep_si->si_dep_state == AVD_SI_READY_TO_UNASSIGN) ||
+
(dep_si->si_dep_state == AVD_SI_FAILOVER_UNDER_PROGRESS))) {
avd_sidep_si_dep_state_set(dep_si, AVD_SI_FAILOVER_UNDER_PROGRESS);
if
(dep_si->num_dependents > 0) {
--
___
Opensaf-devel maili
Re: [devel] [PATCH 1 of 1] amfd: correct logic in si dep flow [#276]
Code inside the if block was always executing, which means if condition
is not needed.
Why can't simply remove the if condition.
Thanks,
Praveen
On 26-Aug-15 11:51 AM, nagendr...@oracle.com wrote:
> osaf/services/saf/amf/amfd/si_dep.cc | 25 -
> 1 files changed, 12 insertions(+), 13 deletions(-)
>
>
> At many places, there has been tautological errors in si dep flow.
> The fix corrects them
>
> diff --git a/osaf/services/saf/amf/amfd/si_dep.cc
> b/osaf/services/saf/amf/amfd/si_dep.cc
> --- a/osaf/services/saf/amf/amfd/si_dep.cc
> +++ b/osaf/services/saf/amf/amfd/si_dep.cc
> @@ -1468,10 +1468,10 @@ void avd_sidep_unassign_dependents(AVD_S
> if
> (m_NCS_MDS_DEST_EQUAL(&sisu->su->su_on_node->adest,&su->su_on_node->adest)) {
> avd_si_unassign(dep_si);
> } else {
> - if((dep_si->si_dep_state != AVD_SI_TOL_TIMER_RUNNING) ||
> - (dep_si->si_dep_state !=
> AVD_SI_READY_TO_UNASSIGN)) {
> + /* Don't start tol timer if dep state are either in
> running or unassigned. */
> + if(!((dep_si->si_dep_state == AVD_SI_TOL_TIMER_RUNNING)
> ||
> + (dep_si->si_dep_state ==
> AVD_SI_READY_TO_UNASSIGN))) {
>
> avd_sidep_start_tolerance_timer_for_dependant(dep_si, si);
> -
> }
> }
> /* If this dependent SI is sponsor too, then unassign its
> dependents also */
> @@ -1788,9 +1788,9 @@ void avd_sidep_update_depstate_si_failov
>
> if(su->su_on_node->saAmfNodeOperState ==
> SA_AMF_OPERATIONAL_DISABLED) {
> if
> ((m_NCS_MDS_DEST_EQUAL(&sisu->su->su_on_node->adest,&su->su_on_node->adest)))
> {
> - if(((dep_si->si_dep_state !=
> AVD_SI_TOL_TIMER_RUNNING) ||
> - (dep_si->si_dep_state
> != AVD_SI_READY_TO_UNASSIGN) ||
> - (dep_si->si_dep_state
> != AVD_SI_FAILOVER_UNDER_PROGRESS)) &&
> + if((!((dep_si->si_dep_state ==
> AVD_SI_TOL_TIMER_RUNNING) ||
> + (dep_si->si_dep_state
> == AVD_SI_READY_TO_UNASSIGN) ||
> + (dep_si->si_dep_state
> == AVD_SI_FAILOVER_UNDER_PROGRESS))) &&
>
> (avd_sidep_sponsors_assignment_states(dep_si))) {
>
>
> avd_sidep_si_dep_state_set(dep_si, AVD_SI_FAILOVER_UNDER_PROGRESS);
> @@ -1801,10 +1801,9 @@ void avd_sidep_update_depstate_si_failov
> }
> }
> } else if (dep_si->sg_of_si == si->sg_of_si) {
> - if((dep_si->si_dep_state !=
> AVD_SI_TOL_TIMER_RUNNING) ||
> - (dep_si->si_dep_state !=
> AVD_SI_READY_TO_UNASSIGN) ||
> - (dep_si->si_dep_state !=
> AVD_SI_FAILOVER_UNDER_PROGRESS)) {
> -
> + if(!((dep_si->si_dep_state ==
> AVD_SI_TOL_TIMER_RUNNING) ||
> + (dep_si->si_dep_state ==
> AVD_SI_READY_TO_UNASSIGN) ||
> + (dep_si->si_dep_state ==
> AVD_SI_FAILOVER_UNDER_PROGRESS))) {
>
> avd_sidep_si_dep_state_set(dep_si, AVD_SI_FAILOVER_UNDER_PROGRESS);
> if (dep_si->num_dependents > 0)
> {
> /* This SI also has
> dependents under it, update their state also */
> @@ -1842,9 +1841,9 @@ void avd_sidep_update_depstate_si_failov
> }
> if (sponsor_assignments_state
> == true) {
>
> -
> if((dep_si->si_dep_state != AVD_SI_TOL_TIMER_RUNNING) ||
> -
> (dep_si->si_dep_state != AVD_SI_READY_TO_UNASSIGN) ||
> -
> (dep_si->si_dep_state != AVD_SI_FAILOVER_UNDER_PROGRESS)) {
> +
> if(!((dep_si->si_dep_state == AVD_SI_TOL_TIMER_RUNNING) ||
> +
> (dep_si->si_dep_state == AVD_SI_READY_TO_UNASSIGN) ||
> +
> (dep_si->si_dep_state == AVD_SI_FAILOVER_UNDER_PROGRESS))) {
>
>
> avd_sidep_s
