Hi Hung,
Reviewed and tested the patch series.
Following are the comments:
There must be check that, the class attribute does not have both
STRONG_DEFAULT and DEFAULT REMOVED (even though the present code does
not allow to have both attribute flags)
more comments inline.
Ack, when the comments are addressed(no need to send another review)
On Wednesday 14 October 2015 02:33 PM, Hung Nguyen wrote:
> osaf/services/saf/immsv/immloadd/imm_loader.cc | 11 +-
> osaf/services/saf/immsv/immloadd/imm_pbe_load.cc | 2 +-
> osaf/services/saf/immsv/immnd/ImmModel.cc | 82 +++++-
> osaf/services/saf/immsv/immnd/immnd_evt.c | 12 +
> osaf/services/saf/immsv/schema/SAI-AIS-IMM-XSD-A.02.17.xsd | 183
> +++++++++++++
> 5 files changed, 285 insertions(+), 5 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.
>
> 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.
> The AttrMod will be changed to ATTR_VALUES_REPLACE (attrValuesNumber = 1,
> attrValues is default value) and sent to OI.
>
> 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) {
> @@ -2187,7 +2192,7 @@ int loadImmXML(std::string xmldir, std::
>
> version.releaseCode = 'A';
> version.majorVersion = 2;
> - version.minorVersion = 16;
> + version.minorVersion = 17;
>
> TRACE("Loading from %s/%s", xmldir.c_str(), file.c_str());
>
> @@ -2638,7 +2643,7 @@ int immsync(int maxBatchSize)
>
> version.releaseCode = 'A';
> version.majorVersion = 2;
> - version.minorVersion = 16;
> + version.minorVersion = 17;
>
> int retries = 0;
> do
> diff --git a/osaf/services/saf/immsv/immloadd/imm_pbe_load.cc
> b/osaf/services/saf/immsv/immloadd/imm_pbe_load.cc
> --- a/osaf/services/saf/immsv/immloadd/imm_pbe_load.cc
> +++ b/osaf/services/saf/immsv/immloadd/imm_pbe_load.cc
> @@ -745,7 +745,7 @@ bool loadObjectsFromPbe(void* pbeHandle,
>
> int loadImmFromPbe(void* pbeHandle, bool preload)
> {
> - SaVersionT version = {'A', 2, 16};
> + SaVersionT version = {'A', 2, 17};
> SaImmHandleT immHandle=0LL;
> SaImmAdminOwnerHandleT ownerHandle=0LL;
> SaImmCcbHandleT ccbHandle=0LL;
> 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
> @@ -3215,6 +3215,11 @@ ImmModel::classCreate(const ImmsvOmClass
> LOG_NO("ERR_INVALID_PARAM: RDN '%s' cannot have
> SA_IMM_ATTR_DEFAULT_REMOVED flag", attNm);
> illegal = 1;
> }
> +
> + if (attr->attrFlags & SA_IMM_ATTR_STRONG_DEFAULT) {
> + LOG_NO("ERR_INVALID_PARAM: RDN '%s' cannot have
> SA_IMM_ATTR_STRONG_DEFAULT flag", attNm);
> + illegal = 1;
> + }
> }
>
> if(attr->attrFlags & SA_IMM_ATTR_NO_DANGLING) {
> @@ -3251,6 +3256,12 @@ ImmModel::classCreate(const ImmsvOmClass
> illegal = 1;
> }
>
> + if ((attr->attrFlags & SA_IMM_ATTR_STRONG_DEFAULT) &&
> !attr->attrDefaultValue) {
> + 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);
> @@ -3900,6 +3911,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",
> @@ -4082,6 +4094,22 @@ ImmModel::notCompatibleAtt(const std::st
> change = true;
> }
> }
> +
> + if (!(oldAttr->mFlags & SA_IMM_ATTR_STRONG_DEFAULT) &&
> + (newAttr->mFlags & SA_IMM_ATTR_STRONG_DEFAULT)) {
Here the check must also be there for protocol50Allowed.
> + 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
> @@ -4153,6 +4181,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 */
> @@ -4410,7 +4457,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
> @@ -8560,6 +8608,16 @@ 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;
> +
> + TRACE("Canonicalizing attr-mod for attribute '%s'",
> attrName.c_str());
> + p->attrValue.attrValuesNumber = 1;
> + attrValue->copyValueToEdu(&(p->attrValue.attrValue),
> + (SaImmValueTypeT)
> p->attrValue.attrValueType);
> + }
> continue; //Ok to replace with nothing.
> }
> //else intentional fall-through
> @@ -8726,6 +8784,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;
Add trace/LOG_WA
> + }
> }
>
> break; //out of switch
> @@ -16102,6 +16165,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;
Add trace/LOG_WA
> + }
> +
> p = p->next;
> continue; //Ok to replace with nothing.
> }
> @@ -16225,6 +16293,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;
Add trace/LOG_WA
/Neel.
> + }
> }
> break; //out of switch
>
> @@ -16977,6 +17050,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;
> @@ -16994,6 +17068,12 @@ ImmModel::objectSync(const ImmsvOmObject
> attrName.c_str());
> err = SA_AIS_ERR_INVALID_PARAM;
> }
> +
> + if ((attr->mFlags & SA_IMM_ATTR_STRONG_DEFAULT) &&
> attrValue->empty()) {
> + LOG_WA("ERR_INVALID_PARAM: attr '%s' has STRONG_DEFAULT flag
> "
> + "but has no value", 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
> @@ -3275,6 +3275,18 @@ static SaAisErrorT immnd_fevs_local_chec
> list = list->next;
> }
> }
> + if (!immModel_protocol50Allowed(cb)) {
> + IMMSV_ATTR_DEF_LIST* list =
> frwrd_evt.info.immnd.info.classDescr.attrDefinitions;
> + while (list) {
> + 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 proto50 is not
> enabled");
> + error = SA_AIS_ERR_TRY_AGAIN;
> + break; /* while */
> + }
> + list = list->next;
> + }
> + }
>
> break;
>
> diff --git a/osaf/services/saf/immsv/schema/SAI-AIS-IMM-XSD-A.02.17.xsd
> b/osaf/services/saf/immsv/schema/SAI-AIS-IMM-XSD-A.02.17.xsd
> new file mode 100644
> --- /dev/null
> +++ b/osaf/services/saf/immsv/schema/SAI-AIS-IMM-XSD-A.02.17.xsd
> @@ -0,0 +1,183 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<!-- edited with XMLSpy v2006 sp1 U (http://www.altova.com) by Maria Toeroe
> (Ericsson Canada/Open Systems Research) -->
> +<xs:schema xmlns:imm="http://www.saforum.org/IMMSchema";
> xmlns:xs="http://www.w3.org/2001/XMLSchema";
> targetNamespace="http://www.saforum.org/IMMSchema";
> elementFormDefault="unqualified" attributeFormDefault="unqualified">
> + <!--
> + OWNERSHIP OF SPECIFICATION AND COPYRIGHTS
> +
> + Copyright(c) 2009, Service Availability(TM) Forum. All rights reserved.
> +
> + Permission to use, copy, modify, and distribute this software for any
> + purpose without fee is hereby granted, provided that this entire notice
> + is included in all copies of any software which is or includes a copy
> + or modification of this software and in all copies of the supporting
> + documentation for such software.
> +
> + THIS SOFTWARE IS BEING PROVIDED "AS IS", WITHOUT ANY EXPRESS OR IMPLIED
> + WARRANTY. IN PARTICULAR, THE SERVICE AVAILABILITY FORUM DOES NOT MAKE ANY
> + REPRESENTATION OR WARRANTY OF ANY KIND CONCERNING THE MERCHANTABILITY
> + OF THIS SOFTWARE OR ITS FITNESS FOR ANY PARTICULAR PURPOSE.
> +
> + -->
> + <!-- PART I - DEFINITION OF AN IMM OBJECT CLASS -->
> + <!--
> + Various types of IMM object attributes (SaImmValueTypeT)
> + -->
> + <xs:simpleType name="attr-value-type">
> + <xs:annotation>
> + <xs:documentation>Types of attribute's
> values</xs:documentation>
> + </xs:annotation>
> + <xs:restriction base="xs:string">
> + <xs:enumeration value="SA_INT32_T"/>
> + <xs:enumeration value="SA_UINT32_T"/>
> + <xs:enumeration value="SA_INT64_T"/>
> + <xs:enumeration value="SA_UINT64_T"/>
> + <xs:enumeration value="SA_NAME_T"/>
> + <xs:enumeration value="SA_TIME_T"/>
> + <xs:enumeration value="SA_FLOAT_T"/>
> + <xs:enumeration value="SA_DOUBLE_T"/>
> + <xs:enumeration value="SA_STRING_T"/>
> + <xs:enumeration value="SA_ANY_T"/>
> + </xs:restriction>
> + </xs:simpleType>
> + <!--
> + Two categories of IMM classes & attributes)
> + -->
> + <xs:simpleType name="category">
> + <xs:annotation>
> + <xs:documentation>Class or attribute
> category</xs:documentation>
> + </xs:annotation>
> + <xs:restriction base="xs:string">
> + <xs:enumeration value="SA_CONFIG"/>
> + <xs:enumeration value="SA_RUNTIME"/>
> + </xs:restriction>
> + </xs:simpleType>
> + <!--
> + Flags for object attributes (SaImmAttrFlagsT)
> + -->
> + <xs:simpleType name="attr-flags">
> + <xs:annotation>
> + <xs:documentation>Attribute flags</xs:documentation>
> + </xs:annotation>
> + <xs:restriction base="xs:string">
> + <xs:enumeration value="SA_MULTI_VALUE"/>
> + <xs:enumeration value="SA_WRITABLE"/>
> + <xs:enumeration value="SA_INITIALIZED"/>
> + <xs:enumeration value="SA_PERSISTENT"/>
> + <xs:enumeration value="SA_CACHED"/>
> + <xs:enumeration value="SA_NO_DUPLICATES"/>
> + <xs:enumeration value="SA_NOTIFY"/>
> + <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>
> + <!--
> + Union of types for "value" and "default-value" elements
> + -->
> + <xs:simpleType name="value-type">
> + <xs:annotation>
> + <xs:documentation>Value type</xs:documentation>
> + </xs:annotation>
> + <xs:union memberTypes="xs:string xs:base64Binary"/>
> + </xs:simpleType>
> + <!--
> + Group of XML elements common to RDN and other object attributes
> + -->
> + <xs:group name="attr-elements">
> + <xs:annotation>
> + <xs:documentation>Common elements to RDN and other
> attributes</xs:documentation>
> + </xs:annotation>
> + <xs:sequence>
> + <xs:element name="name" type="xs:string"/>
> + <xs:element name="type" type="imm:attr-value-type"/>
> + <xs:element name="category" type="imm:category"/>
> + <xs:element name="flag" type="imm:attr-flags"
> minOccurs="0" maxOccurs="5"/>
> + <xs:element name="ntfid" type="xs:unsignedInt"
> default="0" minOccurs="0"/>
> + </xs:sequence>
> + </xs:group>
> + <!--
> + Definition of an object's RDN attribute
> + -->
> + <xs:complexType name="rdn-def">
> + <xs:annotation>
> + <xs:documentation>RDN attribute
> definition</xs:documentation>
> + </xs:annotation>
> + <xs:sequence>
> + <xs:group ref="imm:attr-elements"/>
> + </xs:sequence>
> + </xs:complexType>
> + <!--
> + Definition of an object's attribute (other than RDN)
> + In addition to an RDN attribute, other attributes can be multi-value
> and can have a default value.
> + -->
> + <xs:complexType name="attr-def">
> + <xs:annotation>
> + <xs:documentation>Attribute
> definition</xs:documentation>
> + </xs:annotation>
> + <xs:sequence>
> + <xs:group ref="imm:attr-elements"/>
> + <xs:element name="default-value" type="imm:value-type"
> minOccurs="0"/>
> + </xs:sequence>
> + </xs:complexType>
> + <!--
> + Definition of an object class
> + -->
> + <xs:complexType name="class-def">
> + <xs:annotation>
> + <xs:documentation>Object class
> definition</xs:documentation>
> + </xs:annotation>
> + <xs:sequence>
> + <xs:element name="category" type="imm:category"/>
> + <xs:element name="rdn" type="imm:rdn-def"/>
> + <xs:element name="attr" type="imm:attr-def"
> minOccurs="0" maxOccurs="unbounded"/>
> + </xs:sequence>
> + <xs:attribute name="name" type="xs:string"/>
> + </xs:complexType>
> + <!-- PART II - DEFINITION OF AN IMMS OBJECT INSTANCE -->
> + <!--
> + The object is defined by:
> + - its object class
> + - its DN (XML attribute)
> + - all its attributes (name/values)
> + -->
> + <xs:complexType name="object-def">
> + <xs:annotation>
> + <xs:documentation>Object instance
> definition</xs:documentation>
> + </xs:annotation>
> + <xs:sequence>
> + <xs:element name="dn" type="xs:string">
> + <xs:annotation>
> + <xs:documentation>Object Distinguished
> Name</xs:documentation>
> + </xs:annotation>
> + </xs:element>
> + <xs:element name="attr" minOccurs="0"
> maxOccurs="unbounded">
> + <xs:annotation>
> + <xs:documentation>Object's
> attributes</xs:documentation>
> + </xs:annotation>
> + <xs:complexType>
> + <xs:sequence>
> + <xs:element name="name"
> type="xs:string"/>
> + <xs:element name="value"
> type="imm:value-type" maxOccurs="unbounded"/>
> + </xs:sequence>
> + </xs:complexType>
> + </xs:element>
> + </xs:sequence>
> + <xs:attribute name="class" type="xs:string"/>
> + </xs:complexType>
> + <!-- PART III- DEFINITION OF IMM Service CONTENTS-->
> + <!--
> + The IMM Service contents is defined as a list of class definitions
> followed by a list of objects
> + -->
> + <xs:element name="IMM-contents">
> + <xs:annotation>
> + <xs:documentation>IMM Service
> contents</xs:documentation>
> + </xs:annotation>
> + <xs:complexType>
> + <xs:sequence>
> + <xs:element name="class" type="imm:class-def"
> maxOccurs="unbounded"/>
> + <xs:element name="object" type="imm:object-def"
> minOccurs="0" maxOccurs="unbounded"/>
> + </xs:sequence>
> + </xs:complexType>
> + </xs:element>
> +</xs:schema>
------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
