Hi Zoran, I thought that the existing instances with have the new attributes with NULL value (NULL value is not allowed for STRONG_DEFAULT).
I was wrong about that, actually the default value (if included) will be assigned to new attributes when doing migrate instances. And STRONG_DEFAULT always comes with a default value so we should support STRONG_DEFAULT for new attributes. Best Regards, Hung Nguyen - DEK Technologies -------------------------------------------------------------------------------- From: Zoran Milinkovic [email protected] Sent: Friday, September 25, 2015 10:05AM To: Hung Nguyen, Anders Bjornerstedt, Neelakanta Reddy [email protected], [email protected], [email protected] Cc: Opensaf-devel [email protected] Subject: RE: [PATCH 2 of 4] imm: Support SA_IMM_ATTR_STRONG_DEFAULT flag [#1425] Hi Hung, Find my comments inline started with [Zoran] -----Original Message----- From: Hung Nguyen [mailto:[email protected]] Sent: Tuesday, September 22, 2015 7:05 PM To: Anders Björnerstedt; Zoran Milinkovic; [email protected] Cc: [email protected] Subject: [PATCH 2 of 4] imm: Support SA_IMM_ATTR_STRONG_DEFAULT flag [#1425] osaf/services/saf/immsv/immloadd/imm_loader.cc | 7 +- osaf/services/saf/immsv/immnd/ImmModel.cc | 81 +++++++++++++- osaf/services/saf/immsv/immnd/immnd_evt.c | 8 +- osaf/services/saf/immsv/schema/SAI-AIS-IMM-XSD-A.02.16.xsd | 1 + 4 files changed, 94 insertions(+), 3 deletions(-) This flag is only allowed to be set on attributes that have a default value. Attributes with this flag will never be NULL. Adding this flag to an existing attribute (schema change) will be allowed after checking for ccb interference and existing objects of the class. Do not allow to add a new attribute with this flag when doing schema change. When there's an attempt to set NULL value (or to delete all values with ATTR_VALUES_DELETE) to attributes with this flag, default value will be set. ImmAttrValue::operator= is used to assign value to ImmAttrValue. This will also work with ImmAttrMultiValue (head value will be assigned). diff --git a/osaf/services/saf/immsv/immloadd/imm_loader.cc b/osaf/services/saf/immsv/immloadd/imm_loader.cc --- a/osaf/services/saf/immsv/immloadd/imm_loader.cc +++ b/osaf/services/saf/immsv/immloadd/imm_loader.cc @@ -1720,7 +1720,8 @@ static bool loadXsd(const char *xsdFile) strcmp(value, "SA_INITIALIZED") && strcmp(value, "SA_PERSISTENT") && strcmp(value, "SA_CACHED") && strcmp(value, "SA_NOTIFY") && strcmp(value, "SA_NO_DUPLICATES") && strcmp(value, "SA_NO_DANGLING") && - strcmp(value, "SA_DN") && strcmp(value, "SA_DEFAULT_REMOVED")) { + strcmp(value, "SA_DN") && strcmp(value, "SA_DEFAULT_REMOVED") && + strcmp(value, "SA_STRONG_DEFAULT")) { attrFlagSet.insert(value); } } @@ -1797,6 +1798,10 @@ static SaImmAttrFlagsT charsToFlagsHelpe { return SA_IMM_ATTR_DEFAULT_REMOVED; } + else if (len == strlen("SA_STRONG_DEFAULT") && strncmp((const char*)str, "SA_STRONG_DEFAULT", len) == 0) + { + return SA_IMM_ATTR_STRONG_DEFAULT; + } std::string unflag((char *)str, len); if(!isXsdLoaded) { diff --git a/osaf/services/saf/immsv/immnd/ImmModel.cc b/osaf/services/saf/immsv/immnd/ImmModel.cc --- a/osaf/services/saf/immsv/immnd/ImmModel.cc +++ b/osaf/services/saf/immsv/immnd/ImmModel.cc @@ -3243,6 +3243,14 @@ ImmModel::classCreate(const ImmsvOmClass illegal = 1; } + if (attr->attrFlags & SA_IMM_ATTR_STRONG_DEFAULT) { + if (!attr->attrDefaultValue) { [Zoran] These two 'if' statements can be combined into 1 'if' statement. No need to have 'if' statement within an 'if' block. + LOG_NO("ERR_INVALID_PARAM: Attribute '%s' can not have SA_IMM_ATTR_STRONG_DEFAULT flag " + "without having a default value", attNm); + illegal = 1; + } + } + if(attr->attrDefaultValue) { if(attr->attrFlags & SA_IMM_ATTR_RDN) { LOG_NO("ERR_INVALID_PARAM: RDN '%s' can not have a default", attNm); @@ -3866,6 +3874,7 @@ ImmModel::notCompatibleAtt(const std::st bool checkCcb=false; bool checkNoDup=false; bool checkNoDanglingRefs=false; + bool checkStrongDefault=false; osafassert(changedAttrs); if(oldAttr->mValueType != newAttr->mValueType) { LOG_NO("Impossible upgrade, attribute %s:%s changes value type", @@ -4048,6 +4057,22 @@ ImmModel::notCompatibleAtt(const std::st change = true; } } + + if(!(oldAttr->mFlags & SA_IMM_ATTR_STRONG_DEFAULT) && + (newAttr->mFlags & SA_IMM_ATTR_STRONG_DEFAULT)) { + LOG_NO("Allowed upgrade, attribute %s:%s adds flag " + "SA_IMM_ATTR_STRONG_DEFAULT", className.c_str(), attName.c_str()); + checkCcb = true; + checkStrongDefault = true; + change = true; + } + + if((oldAttr->mFlags & SA_IMM_ATTR_STRONG_DEFAULT) && + !(newAttr->mFlags & SA_IMM_ATTR_STRONG_DEFAULT)) { + LOG_NO("Allowed upgrade, attribute %s:%s removes flag " + "SA_IMM_ATTR_STRONG_DEFAULT", className.c_str(), attName.c_str()); + change = true; + } } osafassert(!checkNoDup || checkCcb); //Duplicate-check implies ccb-check @@ -4117,6 +4142,25 @@ ImmModel::notCompatibleAtt(const std::st } } } + + if (checkStrongDefault) { + /* Screen all instances of the class. + * If there's an instance with the attribute being NULL, abort the schema change. */ + ObjectSet::iterator osi = oldClassInfo->mExtent.begin(); + for(;osi!=oldClassInfo->mExtent.end();++osi) { + obj = *osi; + ImmAttrValueMap::iterator oavi = obj->mAttrValueMap.find(attName); + osafassert(oavi!= obj->mAttrValueMap.end()); + if(oavi->second->empty()) { + std::string objName; + getObjectName(obj, objName); + LOG_NO("Impossible upgrade, attribute %s:%s adds SA_IMM_ATTR_STRONG_DEFAULT flag, " + "but that attribute of object '%s' has NULL value", + className.c_str(), attName.c_str(), objName.c_str()); + return true; + } + } + } } /* "changedAttrs != NULL" ensures that this check is only for the schema update */ @@ -4241,6 +4285,12 @@ ImmModel::notCompatibleAtt(const std::st "flag set", className.c_str(), attName.c_str()); return true; } + + if (newAttr->mFlags & SA_IMM_ATTR_STRONG_DEFAULT) { + LOG_NO("Impossible upgrade, new attribute %s:%s has SA_IMM_ATTR_STRONG_DEFAULT " + "flag set", className.c_str(), attName.c_str()); + return true; + } [Zoran] Why cannot new attribute have SA_IMM_ATTR_STRONG_DEFAULT ? Thanks, Zoran } return false; @@ -4373,7 +4423,8 @@ ImmModel::attrCreate(ClassInfo* classInf SA_IMM_ATTR_NOTIFY | SA_IMM_ATTR_NO_DANGLING | SA_IMM_ATTR_DN | - SA_IMM_ATTR_DEFAULT_REMOVED); + SA_IMM_ATTR_DEFAULT_REMOVED | + SA_IMM_ATTR_STRONG_DEFAULT); if(unknownFlags) { /* This error means that at least one attribute flag is not supported by this @@ -8467,6 +8518,11 @@ ImmModel::ccbObjectModify(const ImmsvOmC err = SA_AIS_ERR_INVALID_PARAM; break; //out of switch } + + if (attr->mFlags & SA_IMM_ATTR_STRONG_DEFAULT) { + osafassert(!attr->mDefaultValue.empty()); + (*attrValue) = attr->mDefaultValue; + } continue; //Ok to replace with nothing. } //else intentional fall-through @@ -8633,6 +8689,11 @@ ImmModel::ccbObjectModify(const ImmsvOmC err = SA_AIS_ERR_INVALID_PARAM; break; //out of switch } + + if (attrValue->empty() && (attr->mFlags & SA_IMM_ATTR_STRONG_DEFAULT)) { + osafassert(!attr->mDefaultValue.empty()); + (*attrValue) = attr->mDefaultValue; + } } break; //out of switch @@ -15991,6 +16052,11 @@ ImmModel::rtObjectUpdate(const ImmsvOmCc attrValue->discardValues(); } if(p->attrValue.attrValuesNumber == 0) { + if (attr->mFlags & SA_IMM_ATTR_STRONG_DEFAULT) { + osafassert(!attr->mDefaultValue.empty()); + (*attrValue) = attr->mDefaultValue; + } + p = p->next; continue; //Ok to replace with nothing. } @@ -16114,6 +16180,11 @@ ImmModel::rtObjectUpdate(const ImmsvOmCc al = al->next; } } + + if (attrValue->empty() && (attr->mFlags & SA_IMM_ATTR_STRONG_DEFAULT)) { + osafassert(!attr->mDefaultValue.empty()); + (*attrValue) = attr->mDefaultValue; + } } break; //out of switch @@ -16866,6 +16937,7 @@ ImmModel::objectSync(const ImmsvOmObject } //while(p) //Check that all attributes with INITIALIZED flag have been set. + //Check that all attributes with STRONG_DEFAULT flag have been set. ImmAttrValueMap::iterator i6; for(i6=object->mAttrValueMap.begin(); i6!=object->mAttrValueMap.end() && err==SA_AIS_OK; @@ -16883,6 +16955,13 @@ ImmModel::objectSync(const ImmsvOmObject attrName.c_str()); err = SA_AIS_ERR_INVALID_PARAM; } + + if ((attr->mFlags & SA_IMM_ATTR_STRONG_DEFAULT) && attrValue->empty()) { + LOG_NO("ERR_INVALID_PARAM: attr '%s' has STRONG_DEFAULT flag " + "yet no value provided in the object create call", + attrName.c_str()); + err = SA_AIS_ERR_INVALID_PARAM; + } } if(err == SA_AIS_OK) { diff --git a/osaf/services/saf/immsv/immnd/immnd_evt.c b/osaf/services/saf/immsv/immnd/immnd_evt.c --- a/osaf/services/saf/immsv/immnd/immnd_evt.c +++ b/osaf/services/saf/immsv/immnd/immnd_evt.c @@ -3261,7 +3261,7 @@ static SaAisErrorT immnd_fevs_local_chec } if (!immModel_protocol47Allowed(cb)) { /* IMM supports creating classes with unknown flags. - * When the upgrade process is not completed, a class-create request (with DEFAULT_REMOVED flag) + * When the upgrade process is not completed, a class-create request (with a new flag) * may be accepted on nodes with old version and rejected on nodes with new version. * That will cause an inconsistency between nodes. */ IMMSV_ATTR_DEF_LIST* list = frwrd_evt.info.immnd.info.classDescr.attrDefinitions; @@ -3272,6 +3272,12 @@ static SaAisErrorT immnd_fevs_local_chec error = SA_AIS_ERR_TRY_AGAIN; break; /* while */ } + if (list->d.attrFlags & SA_IMM_ATTR_STRONG_DEFAULT) { + LOG_WA("ERR_TRY_AGAIN: Can not create class with SA_IMM_ATTR_STRONG_DEFAULT " + "when proto47 is not enabled"); + error = SA_AIS_ERR_TRY_AGAIN; + break; /* while */ + } list = list->next; } } diff --git a/osaf/services/saf/immsv/schema/SAI-AIS-IMM-XSD-A.02.16.xsd b/osaf/services/saf/immsv/schema/SAI-AIS-IMM-XSD-A.02.16.xsd --- a/osaf/services/saf/immsv/schema/SAI-AIS-IMM-XSD-A.02.16.xsd +++ b/osaf/services/saf/immsv/schema/SAI-AIS-IMM-XSD-A.02.16.xsd @@ -69,6 +69,7 @@ <xs:enumeration value="SA_NO_DANGLING"/> <xs:enumeration value="SA_DN"/> <xs:enumeration value="SA_DEFAULT_REMOVED"/> + <xs:enumeration value="SA_STRONG_DEFAULT"/> </xs:restriction> </xs:simpleType> <!-- ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
