Hi,
Any comment from you? I will push this ticket B/next week if no comment.
Regards, Vu
> -----Original Message-----
> From: Vu Minh Nguyen <vu.m.ngu...@dektech.com.au>
> Sent: Wednesday, December 19, 2018 6:11 PM
> To: hans.nordeb...@ericsson.com; lennart.l...@ericsson.com;
> gary....@dektech.com.au
> Cc: opensaf-devel@lists.sourceforge.net; Vu Minh Nguyen
> <vu.m.ngu...@dektech.com.au>
> Subject: [PATCH 1/1] imm: allow empty-value attribute with default-tag
> persisted [#2985]
>
> During runtime, when replacing value of an attribute which has
default-value
> tag with NULL, this NULL value is not persistent after cluster is rebooted
-
> NULL value will be automatically replaced with its default value by IMM.
>
> This behavior causes several unexpected results. Below is a use case.
>
> User defines an attribute with name `maxAge`. It is used to shows how many
> days
> user passwords will be expired, default value is 30 days; if replacing
with
> a NULL/empty, it means the passwords will never get expired.
>
> During runtime, user changes the existing value with NULL - expect the
> passwords
> never get expired. Later on, then cluster is rebooted, that value is
silently
> replaced with the default without notice of user.
>
> This patch makes some changes in immdump/immloader/imm om
> library/immnd
> to make NULL value in such case persisted even after cluster is rebooted.
> ---
> src/imm/README | 10 +---
> src/imm/agent/imma_om_api.cc | 7 ++-
> src/imm/immloadd/imm_loader.cc | 23 +++++---
> src/imm/immloadd/imm_pbe_load.cc | 16 ++++--
> src/imm/immnd/ImmModel.cc | 9 ++-
> src/imm/tools/imm_xmlw_dump.cc | 96 ++++++++++++++++++++------------
> 6 files changed, 100 insertions(+), 61 deletions(-)
>
> diff --git a/src/imm/README b/src/imm/README
> index 7ace34741..132ee0ac0 100644
> --- a/src/imm/README
> +++ b/src/imm/README
> @@ -351,14 +351,8 @@ as part of an attribute definition.
> (i) A default declaration is only allowed for single valued attributes
(no
> concept of a multivalued default exists).
>
> -(ii) Default values are assigned at object creation. Default values are
NOT
> -assigned if an attribute is set to the empty/null value by a
modification.
> -
> -(iii) Default values are assigned at cluster restart for any attributes
that
> -are null/empty and that have a default. This is a special case of (i)
because
> -imm loading actually uses the regular imm API to recreate the imm
contents.
> -In particular, saImmOmCcbObjectCreate is used to recreate all objects
from
> -the file-system image.
> +(ii) Default values are assigned at object creation, except the creation
> comes
> +from IMM loader.
>
> Common missunderstandings about "system attributes" of an imm object.
> ---------------------------------------------------------------------
> diff --git a/src/imm/agent/imma_om_api.cc
> b/src/imm/agent/imma_om_api.cc
> index 7155799d9..0d24b2335 100644
> --- a/src/imm/agent/imma_om_api.cc
> +++ b/src/imm/agent/imma_om_api.cc
> @@ -2037,7 +2037,7 @@ static SaAisErrorT ccb_object_create_common(
> TRACE_2("ERR_INVALID_PARAM: Not allowed to set attribute %s",
> sysaImplName);
> goto mds_send_fail;
> - } else if (attr->attrValuesNumber == 0) {
> + } else if (attr->attrValuesNumber == 0 && !immOmIsLoader) {
> TRACE("CcbObjectCreate ignoring attribute %s with no values",
> attr->attrName);
> continue;
> @@ -2065,7 +2065,9 @@ static SaAisErrorT ccb_object_create_common(
>
> const SaImmAttrValueT *avarr = attr->attrValues;
> /*alloc-5 */
> - imma_copyAttrValue(&(p->n.attrValue), attr->attrValueType,
avarr[0]);
> + if (attr->attrValuesNumber > 0) {
> + imma_copyAttrValue(&(p->n.attrValue), attr->attrValueType,
avarr[0]);
> + }
>
> if (attr->attrValuesNumber > 1) {
> unsigned int numAdded = attr->attrValuesNumber - 1;
> @@ -2087,6 +2089,7 @@ static SaAisErrorT ccb_object_create_common(
> }
> }
>
> +
> rc = imma_evt_fake_evs(cb, &evt, &out_evt, cl_node->syncr_timeout,
> cl_node->handle, &locked, false);
> cl_node = NULL;
> diff --git a/src/imm/immloadd/imm_loader.cc
> b/src/imm/immloadd/imm_loader.cc
> index e9a985c22..3bd3e2b2e 100644
> --- a/src/imm/immloadd/imm_loader.cc
> +++ b/src/imm/immloadd/imm_loader.cc
> @@ -117,6 +117,7 @@ typedef struct ParserStateStruct {
> SaImmAttrFlagsT attrFlags;
> SaUint32T attrNtfId;
> char *attrDefaultValueBuffer;
> + bool is_null_value;
>
> int attrValueTypeSet;
> int attrNtfIdSet;
> @@ -925,6 +926,13 @@ static void startElementHandler(void *userData,
> const xmlChar *name,
> state->state[state->depth] = VALUE;
> state->valueContinue = 0;
> state->isBase64Encoded = isBase64Encoded(attrs);
> + state->is_null_value = false;
> + if (attrs) {
> + char* null_value = getAttributeValue("xsi:nil", attrs);
> + if (null_value && std::string{null_value} == "true") {
> + state->is_null_value = true;
> + }
> + }
> /* <category> */
> } else if (strcmp((const char *)name, "category") == 0) {
> state->state[state->depth] = CATEGORY;
> @@ -982,7 +990,7 @@ static void endElementHandler(void *userData, const
> xmlChar *name) {
>
> /* </value> */
> if (strcmp((const char *)name, "value") == 0) {
> - if (state->attrValueBuffers.empty()) {
> + if (state->attrValueBuffers.empty() && !state->is_null_value) {
> char *str = (char *)malloc(1);
>
> str[0] = '\0';
> @@ -1759,14 +1767,13 @@ void addObjectAttributeDefinition(
> SaImmAttrValuesT_2 attrValues;
> int i;
> size_t len;
> + bool null_value = attrValueBuffers->empty();
> +
> TRACE_ENTER2("attrValueBuffers size:%u",
> (unsigned int)attrValueBuffers->size());
> /* The attrName must be set */
> assert(attrName);
>
> - /* The value array can not be empty */
> - assert(!attrValueBuffers->empty());
> -
> /* The object class must be set */
> assert(objectClass);
>
> @@ -1778,15 +1785,15 @@ void addObjectAttributeDefinition(
>
> /* For each value, convert from char* to SaImmAttrValuesT_2 and
> store an array pointing to all in attrValues */
> - attrValues.attrValuesNumber = attrValueBuffers->size();
> - attrValues.attrValues = (SaImmAttrValueT *)malloc(
> + attrValues.attrValuesNumber = null_value ? 0 :
attrValueBuffers->size();
> + attrValues.attrValues = null_value ? nullptr : (SaImmAttrValueT
*)malloc(
> sizeof(SaImmAttrValuesT_2) * attrValues.attrValuesNumber + 1);
>
> - attrValues.attrValues[attrValues.attrValuesNumber] = NULL;
> + if (!null_value) attrValues.attrValues[attrValues.attrValuesNumber] =
> NULL;
>
> it = attrValueBuffers->begin();
> i = 0;
> - while (it != attrValueBuffers->end()) {
> + while (!null_value && it != attrValueBuffers->end()) {
> charsToValueHelper(&attrValues.attrValues[i],
attrValues.attrValueType,
> *it);
> i++;
> diff --git a/src/imm/immloadd/imm_pbe_load.cc
> b/src/imm/immloadd/imm_pbe_load.cc
> index d9e1c4d83..72b926383 100644
> --- a/src/imm/immloadd/imm_pbe_load.cc
> +++ b/src/imm/immloadd/imm_pbe_load.cc
> @@ -524,7 +524,8 @@ bool loadObjectFromPbe(void *pbeHandle,
> SaImmHandleT immHandle,
> const unsigned char *res;
> for (c = 0; c < ncols; ++c) {
> res = sqlite3_column_text(stmt, c);
> - if (res) {
> + bool is_null = false;
> + if (true) {
> SaImmValueTypeT attrType = (SaImmValueTypeT)0;
> const char *colname = sqlite3_column_name(stmt, c);
> assert(colname != NULL);
> @@ -544,7 +545,7 @@ bool loadObjectFromPbe(void *pbeHandle,
> SaImmHandleT immHandle,
> assert(it != class_info->attrInfoVector.end());
>
> char *val;
> - if (attrType == SA_IMM_ATTR_SADOUBLET) {
> + if (res && attrType == SA_IMM_ATTR_SADOUBLET) {
> double dbl = sqlite3_column_double(stmt, c);
>
> val = (char *)malloc(30);
> @@ -555,12 +556,16 @@ bool loadObjectFromPbe(void *pbeHandle,
> SaImmHandleT immHandle,
> snprintf(val, size, "%.17g", dbl);
> }
> } else {
> - val = strdup((const char *)res);
> + if (res) {
> + val = strdup((const char *)res);
> + }
> }
>
> + is_null = (res == nullptr);
> std::list<char *> attrValueBuffers;
> - attrValueBuffers.push_front(val);
> -
> + if (!is_null) {
> + attrValueBuffers.push_front(val);
> + }
> addObjectAttributeDefinition((char *)class_info->className.c_str(),
> (SaImmAttrNameT)colname,
&attrValueBuffers,
> attrType, &attrValuesList);
> @@ -669,7 +674,6 @@ bool loadObjectFromPbe(void *pbeHandle,
> SaImmHandleT immHandle,
> (char *)(*it)->attrName.c_str(),
> &attrValueBuffers,
(*it)->attrValueType,
> &attrValuesList);
> -
> sqlite3_reset(stmt);
> } /*if(attr_is_multi && !attr_is_pure_rt)*/
> ++it;
> diff --git a/src/imm/immnd/ImmModel.cc b/src/imm/immnd/ImmModel.cc
> index 8e3f338dc..747b406c6 100644
> --- a/src/imm/immnd/ImmModel.cc
> +++ b/src/imm/immnd/ImmModel.cc
> @@ -8393,8 +8393,13 @@ SaAisErrorT ImmModel::ccbObjectCreate(
>
> ImmAttrValue* attrValue = i6->second;
> IMMSV_OCTET_STRING tmpos; // temporary octet string
> - eduAtValToOs(&tmpos, &(p->n.attrValue),
> - (SaImmValueTypeT)p->n.attrValueType);
> + memset(&tmpos, 0, sizeof(tmpos));
> + // Attribute value will be assigned null value or <empty>
> + // if having `attrValuesNumber=0` goes with it.
> + if (p->n.attrValuesNumber) {
> + eduAtValToOs(&tmpos, &(p->n.attrValue),
> + (SaImmValueTypeT)p->n.attrValueType);
> + }
> attrValue->setValue(tmpos);
> if (p->n.attrValuesNumber > 1) {
> /*
> diff --git a/src/imm/tools/imm_xmlw_dump.cc
> b/src/imm/tools/imm_xmlw_dump.cc
> index 2e7fcda86..e6fdd279f 100644
> --- a/src/imm/tools/imm_xmlw_dump.cc
> +++ b/src/imm/tools/imm_xmlw_dump.cc
> @@ -419,11 +419,6 @@ void objectToXMLw(std::string objectNameString,
> SaImmAttrValuesT_2** attrs,
> exit(1);
> }
> for (SaImmAttrValuesT_2** p = attrs; *p != NULL; p++) {
> - /* Skip attributes with attrValues = NULL */
> - if ((*p)->attrValues == NULL) {
> - continue;
> - }
> -
> if (classRDNMap.find(classNameString) != classRDNMap.end() &&
> classRDNMap[classNameString] == std::string((*p)->attrName)) {
> continue;
> @@ -469,10 +464,6 @@ static void StoreObject(const std::string&
> objectName,
>
> /* Add attributes to list */
> for (SaImmAttrValuesT_2** p = attrs; *p != NULL; p++) {
> - /* Skip attributes with attrValues = NULL */
> - if ((*p)->attrValues == NULL) {
> - continue;
> - }
> /* Skip RDN */
> if (classRDNMap.find(obj.className) != classRDNMap.end() &&
> classRDNMap[obj.className] == std::string((*p)->attrName)) {
> @@ -544,42 +535,62 @@ static void ObjectSetToXMLw(std::set<Object,
> ObjectComp>& objectSet,
> }
>
> /* Write attribute values */
> - for (std::list<std::string>::const_iterator value_it =
> - attr_it->second.begin();
> - value_it != attr_it->second.end(); ++value_it) {
> + std::list<std::string> values = attr_it->second;
> + if (values.empty()) {
> if (xmlTextWriterStartElement(writer, (xmlChar*)"value") < 0) {
> std::cout << "Error at xmlTextWriterStartElement (value)"
> << std::endl;
> exit(1);
> }
> - if (osaf_is_valid_xml_utf8((*value_it).c_str())) {
> - if (xmlTextWriterWriteString(writer,
(xmlChar*)(*value_it).c_str()) <
> - 0) {
> - std::cout << "Error at xmlTextWriterWriteString (value)"
> - << std::endl;
> - exit(1);
> - }
> - } else {
> - if (xmlTextWriterWriteAttribute(writer, (xmlChar*)"xsi:type",
> - (xmlChar*)"xs:base64Binary") <
0) {
> - std::cout << "Error at xmlTextWriterWriteAttribute (value)"
> - << std::endl;
> - exit(1);
> - }
> - if (xmlTextWriterWriteBase64(writer, (*value_it).c_str(), 0,
> - (*value_it).size()) < 0) {
> - std::cout << "Error at xmlTextWriterWriteBase64 (value)"
> - << std::endl;
> - exit(1);
> - }
> + if (xmlTextWriterWriteAttribute(writer, (xmlChar*)"xsi:nil",
> + (xmlChar*)"true") < 0) {
> + std::cout << "Error at xmlTextWriterWriteAttribute (value)"
> + << std::endl;
> + exit(1);
> }
> if (xmlTextWriterEndElement(writer) < 0) {
> std::cout << "Error at xmlTextWriterWriteEndElement (value)"
> << std::endl;
> exit(1);
> }
> - } /* Attribute values loop */
>
> + } else {
> + for (std::list<std::string>::const_iterator value_it =
> + attr_it->second.begin();
> + value_it != attr_it->second.end(); ++value_it) {
> + if (xmlTextWriterStartElement(writer, (xmlChar*)"value") < 0) {
> + std::cout << "Error at xmlTextWriterStartElement (value)"
> + << std::endl;
> + exit(1);
> + }
> + if (osaf_is_valid_xml_utf8((*value_it).c_str())) {
> + if (xmlTextWriterWriteString(writer,
(xmlChar*)(*value_it).c_str()) <
> + 0) {
> + std::cout << "Error at xmlTextWriterWriteString (value)"
> + << std::endl;
> + exit(1);
> + }
> + } else {
> + if (xmlTextWriterWriteAttribute(writer, (xmlChar*)"xsi:type",
> + (xmlChar*)"xs:base64Binary")
< 0) {
> + std::cout << "Error at xmlTextWriterWriteAttribute (value)"
> + << std::endl;
> + exit(1);
> + }
> + if (xmlTextWriterWriteBase64(writer, (*value_it).c_str(), 0,
> + (*value_it).size()) < 0) {
> + std::cout << "Error at xmlTextWriterWriteBase64 (value)"
> + << std::endl;
> + exit(1);
> + }
> + }
> + if (xmlTextWriterEndElement(writer) < 0) {
> + std::cout << "Error at xmlTextWriterWriteEndElement (value)"
> + << std::endl;
> + exit(1);
> + }
> + } /* Attribute values loop */
> + }
> if (xmlTextWriterEndElement(writer) < 0) {
> std::cout << "Error at xmlTextWriterEndElement (attr-object)"
> << std::endl;
> @@ -597,8 +608,23 @@ static void ObjectSetToXMLw(std::set<Object,
> ObjectComp>& objectSet,
> }
>
> void valuesToXMLw(SaImmAttrValuesT_2* p, xmlTextWriterPtr writer) {
> - if (!p->attrValues) {
> - // std::cout << "No values!" << std::endl;
> + if (p->attrValuesNumber == 0) {
> + if (xmlTextWriterStartElement(writer, (xmlChar*)"value") < 0) {
> + std::cout << "Error at xmlTextWriterStartElement (value)"
> + << std::endl;
> + exit(1);
> + }
> + if (xmlTextWriterWriteAttribute(writer, (xmlChar*)"xsi:nil",
> + (xmlChar*)"true") < 0) {
> + std::cout << "Error at xmlTextWriterWriteAttribute (value)"
> + << std::endl;
> + exit(1);
> + }
> + if (xmlTextWriterEndElement(writer) < 0) {
> + std::cout << "Error at xmlTextWriterWriteEndElement (value)"
> + << std::endl;
> + exit(1);
> + }
> return;
> }
>
> --
> 2.19.2
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
