Currently, if an upgrade DELETE or MODIFY CCB contains empty-value
attributes, SMF will create rollback data for those attributes with
the hard-coded "<_empty_>" string value. However, this hard-coded
string will actually make the later rollback fail, especially for
numeric-type attributes, when trying to convert "<_empty_>" to the
real numeric type, which is considered an invalid conversion.
This commit fixes that kind of rollback issue by:
- Not re-create any originally empty-value attributes when rolling
back DELETE CCBs.
- Check for the hard-coded marker string "<_empty_>" and not add it
to an attribute descriptor when rolling back empty attributes that
got added with values by MODIFY CCBs in the upgrade.
This commit also fixes a few semantic errors in the code, and improve
some log/trace printouts.
---
src/smf/smfd/SmfImmOperation.cc | 5 ++---
src/smf/smfd/SmfImmOperation.h | 3 ++-
src/smf/smfd/SmfRollback.cc | 2 +-
src/smf/smfd/imm_modify_config/add_operation_to_ccb.cc | 13 ++++++++-----
src/smf/smfd/imm_modify_config/attribute.cc | 10 +++++++---
src/smf/smfd/imm_modify_config/immccb.cc | 6 +++---
6 files changed, 23 insertions(+), 16 deletions(-)
diff --git a/src/smf/smfd/SmfImmOperation.cc b/src/smf/smfd/SmfImmOperation.cc
index a8e65c5..fc127fa 100644
--- a/src/smf/smfd/SmfImmOperation.cc
+++ b/src/smf/smfd/SmfImmOperation.cc
@@ -275,9 +275,8 @@ SaAisErrorT SmfImmDeleteOperation::PrepareRollback(
if (saveAttribute == true) {
if (attr->attrValuesNumber == 0) {
- o_rollbackData->addAttrValue(attr->attrName,
- smf_immTypeToString(attr->attrValueType),
- "<_empty_>");
+ // No need to re-create originally empty attribute
+ continue;
} else {
for (unsigned int j = 0; j < attr->attrValuesNumber; j++) {
o_rollbackData->addAttrValue(
diff --git a/src/smf/smfd/SmfImmOperation.h b/src/smf/smfd/SmfImmOperation.h
index fc855d4..bc91431 100644
--- a/src/smf/smfd/SmfImmOperation.h
+++ b/src/smf/smfd/SmfImmOperation.h
@@ -49,7 +49,8 @@ class SmfImmAttribute {
// Note: More than one value can be added (multi-value)
void AddAttributeValue(const std::string& i_value) {
m_values.push_back(i_value);
- attribute_.AddValue(i_value);
+ if (i_value.compare("<_empty_>") != 0)
+ attribute_.AddValue(i_value);
}
// Note: This is the legacy function that returns a list of strings. Is kept
diff --git a/src/smf/smfd/SmfRollback.cc b/src/smf/smfd/SmfRollback.cc
index 5aedd62..299a1cb 100644
--- a/src/smf/smfd/SmfRollback.cc
+++ b/src/smf/smfd/SmfRollback.cc
@@ -408,7 +408,7 @@ SaAisErrorT SmfRollbackData::rollbackModifyOperation(
SmfImmModifyOperation* immOp = new (std::nothrow) SmfImmModifyOperation();
if (immOp == NULL) {
LOG_ER(
- "SmfRollbackData::rollbackModifyOperation, could not create
SmfImmCreateOperation");
+ "SmfRollbackData::rollbackModifyOperation, could not create
SmfImmModifyOperation");
return SA_AIS_ERR_FAILED_OPERATION;
}
diff --git a/src/smf/smfd/imm_modify_config/add_operation_to_ccb.cc
b/src/smf/smfd/imm_modify_config/add_operation_to_ccb.cc
index 44373cd..e9dea79 100644
--- a/src/smf/smfd/imm_modify_config/add_operation_to_ccb.cc
+++ b/src/smf/smfd/imm_modify_config/add_operation_to_ccb.cc
@@ -284,12 +284,15 @@ int AddModifyToCcb(const SaImmCcbHandleT& ccb_handle,
ais_error_ = ais_rc;
break;
}
+ } else {
+ // Unrecoverable Fail
+ LOG_NO("%s: AddObjectModifyToCcb() Fail, %s", __FUNCTION__,
+ saf_error(ais_rc));
+ recovery_info = kFail;
+ api_name_ = "saImmOmCcbObjectModify_2";
+ ais_error_ = ais_rc;
+ break;
}
- } else {
- // Unrecoverable Fail
- recovery_info = kFail;
- api_name_ = "saImmOmCcbObjectModify_2";
- ais_error_ = ais_rc;
}
// Add Modify to CCB Success
diff --git a/src/smf/smfd/imm_modify_config/attribute.cc
b/src/smf/smfd/imm_modify_config/attribute.cc
index 9329110..d31524f 100644
--- a/src/smf/smfd/imm_modify_config/attribute.cc
+++ b/src/smf/smfd/imm_modify_config/attribute.cc
@@ -88,10 +88,10 @@ static bool StringToNumericValue(const std::string&
str_value,
}
}
} catch(std::invalid_argument& e) {
- LOG_NO("%s: Invalid argument", __FUNCTION__);
+ LOG_NO("%s Fail: Invalid argument", __FUNCTION__);
rc = false;
} catch(std::out_of_range& e) {
- LOG_NO("%s: Out of range", __FUNCTION__);
+ LOG_NO("%s Fail: Out of range", __FUNCTION__);
rc = false;
}
@@ -140,6 +140,9 @@ bool AttributeHandler::AddAttributesForModification(const
ModifyDescriptor&
rc = false;
break;
}
+ // break out of the 'for' loop if a 'switch case' fails
+ if (rc == false)
+ break;
}
TRACE_LEAVE();
@@ -234,7 +237,8 @@ StoreNumericAttribute(const AttributeDescriptor& attribute,
Request request) {
// Create a vector containing all values for this attribute
if (StringToNumericValue<T>
(value_str, numeric_value, value_type) == false) {
- LOG_NO("%s: StringToNumericValue() Fail", __FUNCTION__);
+ LOG_NO("%s Fail: name=%s, value=%s", __FUNCTION__,
+ attribute.attribute_name.c_str(), value_str.c_str());
rc = false;
break;
}
diff --git a/src/smf/smfd/imm_modify_config/immccb.cc
b/src/smf/smfd/imm_modify_config/immccb.cc
index f945bdc..cc62911 100644
--- a/src/smf/smfd/imm_modify_config/immccb.cc
+++ b/src/smf/smfd/imm_modify_config/immccb.cc
@@ -189,9 +189,9 @@ int ModelModification::CreateHandles() {
if (recovery_info == kContinue) {
recovery_info = CreateCcb();
if (recovery_info == kFail) {
- LOG_NO("%s: CreateAdminOwner() Fail", __FUNCTION__);
+ LOG_NO("%s: CreateCcb() Fail", __FUNCTION__);
} else if (recovery_info == kRestartOm) {
- TRACE("%s: CreateAdminOwner() Restart", __FUNCTION__);
+ TRACE("%s: CreateCcb() Restart", __FUNCTION__);
}
}
TRACE_LEAVE();
@@ -692,4 +692,4 @@ int ModelModification::ApplyModifications() {
return recovery_info;
}
-} // namespace modelmodify
\ No newline at end of file
+} // namespace modelmodify
--
2.7.4
------------------------------------------------------------------------------
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
