Ack from me 4.3 patch 1 with same adjustment to the comment marked below.
Zoran Milinkovic wrote:
> osaf/libs/common/immsv/immpbe_dump.cc | 2 +-
> osaf/services/saf/immsv/immloadd/imm_loader.cc | 140
> ++++++++++++++++++++++++-
> osaf/services/saf/immsv/immnd/ImmModel.cc | 36 +++--
> osaf/services/saf/immsv/immnd/ImmSearchOp.cc | 2 +-
> osaf/services/saf/immsv/immnd/ImmSearchOp.hh | 4 +-
> 5 files changed, 165 insertions(+), 19 deletions(-)
>
>
> Add support for 64-bit attribute flags to IMM service.
> If immload finds unknown attribute flag, immload will try to find the
> attribute flag in the schema defined in the top element of the loading IMM
> XML file.
>
> diff --git a/osaf/libs/common/immsv/immpbe_dump.cc
> b/osaf/libs/common/immsv/immpbe_dump.cc
> --- a/osaf/libs/common/immsv/immpbe_dump.cc
> +++ b/osaf/libs/common/immsv/immpbe_dump.cc
> @@ -873,7 +873,7 @@ ClassInfo* classToPBE(std::string classN
> LOG_ER("Failed to bind attr_type with error code: %d",
> rc);
> goto bailout;
> }
> - if((rc = sqlite3_bind_int(stmt, 4, (*p)->attrFlags)) !=
> SQLITE_OK) {
> + if((rc = sqlite3_bind_int64(stmt, 4, (*p)->attrFlags)) !=
> SQLITE_OK) {
> LOG_ER("Failed to bind attr_flags with error code: %d",
> rc);
> goto bailout;
> }
> 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
> @@ -17,7 +17,9 @@
>
> #include "imm_loader.hh"
> #include <iostream>
> +#include <set>
> #include <libxml/parser.h>
> +#include <libxml/xpath.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> @@ -26,6 +28,7 @@
> #include <configmake.h>
> #include <logtrace.h>
> #include <sys/stat.h>
> +#include <errno.h>
>
> #define MAX_DEPTH 10
> #define MAX_CHAR_BUFFER_SIZE 8192 //8k
> @@ -111,6 +114,12 @@ typedef struct ParserStateStruct
> SaImmHandleT ccbHandle;
> } ParserState;
>
> +bool isXsdLoaded;
> +std::string xsddir;
> +std::string xsd;
> +typedef std::set<std::string> AttrFlagSet;
> +AttrFlagSet attrFlagSet;
> +
> /* Helper functions */
>
> static void addToAttrTypeCache(ParserState*, SaImmValueTypeT);
> @@ -874,6 +883,13 @@ static void startElementHandler(void* us
> else if (strcmp((const char*)name, "imm:IMM-contents") == 0)
> {
> state->state[state->depth] = IMM_CONTENTS;
> + char *schema = (char *)getAttributeValue(attrs, (xmlChar
> *)"noNamespaceSchemaLocation");
> + if(!schema) {
> + schema = (char *)getAttributeValue(attrs, (xmlChar
> *)"xsi:noNamespaceSchemaLocation");
> + }
> + if(schema) {
> + xsd = schema;
> + }
> }
> else
> {
> @@ -1424,6 +1440,106 @@ static xmlEntityPtr
> return xmlGetPredefinedEntity(name);
> }
>
> +static inline char *getAttrValue(xmlAttributePtr attr) {
> + if(!attr || !attr->children) {
> + return NULL;
> + }
> +
> + return (char *)attr->children->content;
> +}
> +
> +static bool loadXsd(const char *xsdFile) {
> + struct stat st;
> + if(stat(xsdFile, &st)) {
> + if(errno == ENOENT) {
> + LOG_ER("%s does not exist", xsdFile);
> + } else {
> + LOG_ER("stat of %s return error: %d", xsdFile, errno);
> + }
> +
> + return false;
> + }
> + // It should be a file or a directory
> + if(!S_ISREG(st.st_mode)) {
> + LOG_ER("%s is not a file", xsdFile);
> + return false;
> + }
> +
> + xmlNodePtr xsdDocRoot;
> + xmlDocPtr xsdDoc = xmlParseFile(xsdFile);
> + if(!xsdDoc) {
> + return false;
> + }
> +
> + bool rc = true;
> + xmlXPathContextPtr ctx = xmlXPathNewContext(xsdDoc);
> + if(!ctx) {
> + rc = false;
> + goto freedoc;
> + }
> +
> + // Add namespace of the first element
> + xsdDocRoot = xmlDocGetRootElement(xsdDoc);
> + if(xsdDocRoot->ns) {
> + ctx->namespaces = (xmlNsPtr *)malloc(sizeof(xmlNsPtr));
> + ctx->namespaces[0] = xsdDocRoot->ns;
> + ctx->nsNr = 1;
> + }
> +
> + xmlXPathObjectPtr xpathObj;
> + xpathObj = xmlXPathEval((const
> xmlChar*)"/xs:schema/xs:simpleType[@name=\"attr-flags\"]/xs:restriction/xs:enumeration",
> ctx);
> + if(!xpathObj || !xpathObj->nodesetval) {
> + rc = false;
> + goto freectx;
> + }
> +
> + xmlElementPtr element;
> + xmlAttributePtr attr;
> + char *value;
> + int size;
> +
> + size = xpathObj->nodesetval->nodeNr;
> + for(int i=0; i<size; i++) {
> + value = NULL;
> + element = (xmlElementPtr)xpathObj->nodesetval->nodeTab[i];
> + attr = element->attributes;
> + while(attr) {
> + if(!strcmp((char *)attr->name, "value")) {
> + value = getAttrValue(attr);
> + }
> +
> + if(value) {
> + break;
> + }
> +
> + attr = (xmlAttributePtr)attr->next;
> + }
> +
> + if(value) {
> + if(strcmp(value, "SA_RUNTIME") && strcmp(value, "SA_CONFIG") &&
> + strcmp(value, "SA_MULTI_VALUE") && strcmp(value,
> "SA_WRITABLE") &&
> + strcmp(value, "SA_INITIALIZED") && strcmp(value,
> "SA_PERSISTENT") &&
> + strcmp(value, "SA_CACHED") && strcmp(value, "SA_NOTIFY")
> &&
> + strcmp(value, "SA_NO_DUPLICATES")) {
> + attrFlagSet.insert(value);
> + }
> + }
> + }
> +
> + isXsdLoaded = true;
> +
> + xmlXPathFreeObject(xpathObj);
> +freectx:
> + if(ctx->nsNr) {
> + free(ctx->namespaces);
> + }
> + xmlXPathFreeContext(ctx);
> +freedoc:
> + xmlFreeDoc(xsdDoc);
> +
> + return rc;
> +}
> +
> /**
> * Takes a string and returns the corresponding flag
> */
> @@ -1470,7 +1586,24 @@ static SaImmAttrFlagsT charsToFlagsHelpe
> return SA_IMM_ATTR_NO_DUPLICATES;
> }
>
> - LOG_ER("UNKNOWN FLAGS, %s", str);
> + std::string unflag((char *)str, len);
> + if(!isXsdLoaded) {
> + std::string xsdPath = xsddir;
> + if(xsdPath.size() > 0)
> + xsdPath.append("/");
> + xsdPath.append(xsd);
> + LOG_WA("Found unknown flag (%s). Trying to load a schema %s",
> unflag.c_str(), xsdPath.c_str());
> + loadXsd(xsdPath.c_str());
> + }
> +
> + if(isXsdLoaded) {
> + AttrFlagSet::iterator it = attrFlagSet.find(unflag);
> + if(it != attrFlagSet.end()) {
> + return 0;
> + }
> + }
> +
> + LOG_ER("UNKNOWN FLAGS, %s", unflag.c_str());
>
> exit(1);
> }
> @@ -1879,6 +2012,11 @@ int loadImmXML(std::string xmldir, std::
> state.adminInit = 0;
> state.ccbInit = 0;
>
> + isXsdLoaded = false;
> + xsddir = xmldir;
> + xsd = "";
> + attrFlagSet.clear();
> +
> /* Build the filename */
> filename = xmldir;
> filename.append("/");
> 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
> @@ -70,7 +70,7 @@ struct AttrInfo
> {
> AttrInfo():mValueType(0), mFlags(0), mNtfId(0){}
> SaUint32T mValueType;
> - SaUint32T mFlags;
> + SaImmAttrFlagsT mFlags;
> SaUint32T mNtfId;
> ImmAttrValue mDefaultValue;
> };
> @@ -2429,6 +2429,7 @@ ImmModel::classCreate(const ImmsvOmClass
> int illegal=0;
> bool persistentRt = false;
> bool persistentRdn = false;
> + bool useUnknownFlag = false;
> ClassInfo* classInfo = NULL;
> ClassInfo* prevClassInfo = NULL;
> ClassInfo dummyClass(req->classCategory);
> @@ -2647,7 +2648,10 @@ ImmModel::classCreate(const ImmsvOmClass
> }
> }
> err = attrCreate(classInfo, attr, attrName);
> - if(err != SA_AIS_OK) {
> + if(err == SA_AIS_ERR_NOT_SUPPORTED) {
> + useUnknownFlag = true;
> + err = SA_AIS_OK;
> + } else if(err != SA_AIS_OK) {
> illegal = 1;
> }
> list = list->next;
> @@ -2700,6 +2704,10 @@ ImmModel::classCreate(const ImmsvOmClass
> goto done;
> }
>
> + if(useUnknownFlag) {
> + LOG_NO("At least one attribute in class %s has unsupported attribute
> flag", className.c_str());
> + }
> +
> /* All checks passed, now install the class def. */
>
> if(!schemaChange) {
> @@ -3420,18 +3428,18 @@ ImmModel::attrCreate(ClassInfo* classInf
> SA_IMM_ATTR_NOTIFY);
>
> if(unknownFlags) {
> - LOG_NO("ERR_INVALID_PARAM: invalid search option 0x%llx",
> - unknownFlags);
> - err = SA_AIS_ERR_INVALID_PARAM;
> - goto done;
> - }
> -
> - TRACE_5("create attribute '%s'", attrName.c_str());
> -
> + /* This error means that at least one attribute flag is not
> supported by OpenSAF,
> + * but because of forward compatibility, future flags must be
> supported.
> + */
>
I have two problems with the comment.
First, he flag is not supported by this >>release<< of OpenSAF.
Second, its not that we must "support" future flags (contradicts that it
is not supported by this release)
but that this release must try to "cope" with future flags by
essentially ignoring them.
I suggest instead:
/* This error means that at least one attribute flag is not supported by
this
OpenSAF release. This release will still try to cope with such flags by
ignoring them (e.g. not failing to load the rest of the data). This
can only
be done if either the XSD pointed at by the file can be loaded and the
unsupported flag is confirmed as a valid flag name in that XSD; or if the
recognition of the unsupported flag has been hardcoded into this older
release.
Hardcoding is only done for non-integrity related flags such as
SA_IMM_ATTR_NOTIFY
and allows the flag to pass through this system (e.g. be included in a
dump).
*/
/AndersBj
> + err = SA_AIS_ERR_NOT_SUPPORTED;
> + TRACE_5("create attribute '%s' with unknown flag(s), attribute
> flag: %llu", attrName.c_str(), attr->attrFlags);
> + } else {
> + TRACE_5("create attribute '%s'", attrName.c_str());
> + }
> +
> AttrInfo* attrInfo = new AttrInfo;
> attrInfo->mValueType = attr->attrValueType;
> - osafassert(attr->attrFlags < 0xffffffff);
> - attrInfo->mFlags = (SaUint32T) attr->attrFlags;
> + attrInfo->mFlags = attr->attrFlags;
> attrInfo->mNtfId = attr->attrNtfId;
> if(attr->attrDefaultValue) {
> IMMSV_OCTET_STRING tmpos; //temporary octet string
> @@ -3514,13 +3522,13 @@ ImmModel::classDescriptionGet(const IMMS
>
> struct AttrFlagIncludes
> {
> - AttrFlagIncludes(SaUint32T attrFlag) : mFlag(attrFlag) { }
> + AttrFlagIncludes(SaImmAttrFlagsT attrFlag) : mFlag(attrFlag) { }
>
> bool operator() (AttrMap::value_type& item) const {
> return (item.second->mFlags & mFlag) != 0;
> }
>
> - SaUint32T mFlag;
> + SaImmAttrFlagsT mFlag;
> };
>
> struct IdIs
> diff --git a/osaf/services/saf/immsv/immnd/ImmSearchOp.cc
> b/osaf/services/saf/immsv/immnd/ImmSearchOp.cc
> --- a/osaf/services/saf/immsv/immnd/ImmSearchOp.cc
> +++ b/osaf/services/saf/immsv/immnd/ImmSearchOp.cc
> @@ -51,7 +51,7 @@ ImmSearchOp::addObject(
> void
> ImmSearchOp::addAttribute(const std::string& attributeName,
> SaUint32T valueType,
> - SaUint32T flags)
> + SaImmAttrFlagsT flags)
>
> {
> SearchObject& obj = mResultList.back();
> diff --git a/osaf/services/saf/immsv/immnd/ImmSearchOp.hh
> b/osaf/services/saf/immsv/immnd/ImmSearchOp.hh
> --- a/osaf/services/saf/immsv/immnd/ImmSearchOp.hh
> +++ b/osaf/services/saf/immsv/immnd/ImmSearchOp.hh
> @@ -32,7 +32,7 @@ struct SearchAttribute
> std::string name;
> ImmAttrValue* valuep;
> SaImmValueTypeT valueType;
> - SaUint32T flags;
> + SaImmAttrFlagsT flags;
>
> ~SearchAttribute();
> };
> @@ -63,7 +63,7 @@ public:
> void addAttribute(
> const std::string& attributeName,
> SaUint32T valueType,
> - SaUint32T flags);
> + SaImmAttrFlagsT flags);
> void addAttrValue(const ImmAttrValue& value);
> void setImplementer(
> SaUint32T conn,
>
> ------------------------------------------------------------------------------
> Managing the Performance of Cloud-Based Applications
> Take advantage of what the Cloud has to offer - Avoid Common Pitfalls.
> Read the Whitepaper.
> http://pubads.g.doubleclick.net/gampad/clk?id=121054471&iu=/4140/ostg.clktrk
> _______________________________________________
> Opensaf-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/opensaf-devel
>
------------------------------------------------------------------------------
Flow-based real-time traffic analytics software. Cisco certified tool.
Monitor traffic, SLAs, QoS, Medianet, WAAS etc. with NetFlow Analyzer
Customize your own dashboards, set traffic alerts and generate reports.
Network behavioral analysis & security monitoring. All-in-one tool.
http://pubads.g.doubleclick.net/gampad/clk?id=126839071&iu=/4140/ostg.clktrk
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
