Hi Neelakanta, Find my comments inline
-----Original Message----- From: Neelakanta Reddy [mailto:[email protected]] Sent: Wednesday, May 13, 2015 1:33 PM To: Zoran Milinkovic Cc: [email protected] Subject: Re: [PATCH 1 of 1] immtools: add support for ccbObjectModify after ccbObjectCreate in the same CCB in immcfg [#1283] Hi zoran, Reviewed and tested the patch. Ack with the following comments. 1. object_info_add must be after saImmOmCcbObjectCreate_2 is successful in both imm_cfg.c and imm_import.cc [Zoran] It's not important if object_info_add is called before or after ccbObjectCreate. immcfg aborts CCB on any non SA_AIS_OK error code. So, if object_info_add is added before ccbObjectCreate, and ccbObjectCreate fails, object info will be deleted next time a new CCB is initialized. In this case with ccbObjectCreate, more codes need to be added to put object_info_add after ccbObjectCreate 2. add "object_info_clear" aftersaImmOmCcbApply in imm_import.cc [Zoran] object_info_clear is not needed in this place. ccbApply in imm_import.cc is done only for non-explicit commit mode. It will execute only once, and object info is not need to be saved. 3. add "object_info_clear" in ccb_apply and ccb_abort [Zoran] object_info_clear is also not needed in these two functions. In both functions CCB is finalized, and for next CCB operation a new CCB needs to be initialized. In the patch, object_info_clear is called with each ccbInitialize. So, object_info_clear will be called with next CCB operation, and adding object_info_clear in ccb_apply and ccb_abort is not necessary. Best regards, Zoran /Neel. On Friday 08 May 2015 06:48 PM, Zoran Milinkovic wrote: > osaf/tools/safimm/immcfg/imm_cfg.c | 90 > +++++++++++++++++++++++++++++++++- > osaf/tools/safimm/immcfg/imm_import.cc | 14 +++- > 2 files changed, 99 insertions(+), 5 deletions(-) > > > Add support for ccbObjectModify operation on an object created by > ccbObjectCreate in the same CCB. > > diff --git a/osaf/tools/safimm/immcfg/imm_cfg.c > b/osaf/tools/safimm/immcfg/imm_cfg.c > --- a/osaf/tools/safimm/immcfg/imm_cfg.c > +++ b/osaf/tools/safimm/immcfg/imm_cfg.c > @@ -46,6 +46,12 @@ > > #include "osaf_extended_name.h" > > +typedef struct ObjectInfo { > + char *objectName; > + char *className; > + struct ObjectInfo *next; > +} ObjectInfoT; > + > static SaVersionT immVersion = { 'A', 2, 15 }; > int verbose = 0; > int ccb_safe = 1; > @@ -56,6 +62,8 @@ SaImmAdminOwnerNameT adminOwnerName = NU > SaImmAdminOwnerHandleT ownerHandle = 0; > SaImmCcbHandleT ccbHandle = -1; > > +static ObjectInfoT *objectInfo = NULL; > + > extern struct ImmutilWrapperProfile immutilWrapperProfile; > typedef enum { > INVALID = 0, > @@ -182,6 +190,76 @@ void sigalarmh(int sig) > exit(EXIT_FAILURE); > } > > +void object_info_add(const char *objectName, const char *className) { > + ObjectInfoT *oi; > + ObjectInfoT *prev; > + int rc; > + > + prev = NULL; > + oi = objectInfo; > + while(oi) { > + if((rc = strcmp(oi->objectName, objectName)) >= 0) { > + if(!rc) { > + // Object is already in the list > + return; > + } > + > + break; > + } > + > + prev = oi; > + oi = oi->next; > + } > + > + if(prev) { > + oi = (ObjectInfoT *)calloc(1, sizeof(ObjectInfoT)); > + oi->objectName = strdup(objectName); > + oi->className = strdup(className); > + oi->next = prev->next; > + prev->next = oi; > + } else { > + objectInfo = (ObjectInfoT *)calloc(1, sizeof(ObjectInfoT)); > + objectInfo->objectName = strdup(objectName); > + objectInfo->className = strdup(className); > + objectInfo->next = oi; > + } > +} > + > +void object_info_clear() { > + ObjectInfoT *oi = objectInfo; > + ObjectInfoT *next; > + > + while(oi) { > + next = oi->next; > + free(oi->objectName); > + free(oi->className); > + free(oi); > + oi = next; > + } > + > + objectInfo = NULL; > +} > + > +char *object_info_get_class(const char *objectName) { > + ObjectInfoT *oi; > + int rc; > + > + oi = objectInfo; > + while(oi) { > + if((rc = strcmp(oi->objectName, objectName)) >= 0) { > + if(!rc) { > + return strdup(oi->className); > + } > + > + break; > + } > + > + oi = oi->next; > + } > + > + return NULL; > +} > + > static void free_attr_value(SaImmValueTypeT attrValueType, SaImmAttrValueT > attrValue) { > if(attrValue) { > if(attrValueType == SA_IMM_ATTR_SASTRINGT) > @@ -233,10 +311,15 @@ static SaImmAttrModificationT_2 *new_att > char *name = strdup(nameval); > char *value; > SaImmAttrModificationT_2 *attrMod = NULL; > - SaImmClassNameT className = immutil_get_className(objectName); > + SaImmClassNameT className; > SaAisErrorT error; > SaImmAttrModificationTypeT modType = SA_IMM_ATTR_VALUES_REPLACE; > > + className = > object_info_get_class(osaf_extended_name_borrow(objectName)); > + if(!className) { > + className = immutil_get_className(objectName); > + } > + > if (className == NULL) { > fprintf(stderr, "Object with DN '%s' does not exist\n", > osaf_extended_name_borrow(objectName)); > res = -1; > @@ -435,10 +518,13 @@ int object_create(const SaNameT **object > fprintf(stderr, "error - saImmOmCcbInitialize FAILED: > %s\n", saf_error(error)); > goto done; > } > + object_info_clear(); > } > > i = 0; > while (objectNames[i] != NULL) { > + object_info_add(osaf_extended_name_borrow(objectNames[i]), > className); > + > str = strdup(osaf_extended_name_borrow(objectNames[i])); > if ((delim = strchr(str, ',')) != NULL) { > /* a parent exist */ > @@ -584,6 +670,7 @@ int object_modify(const SaNameT **object > fprintf(stderr, "error - saImmOmCcbInitialize FAILED: > %s\n", saf_error(error)); > goto done; > } > + object_info_clear(); > } > > i = 0; > @@ -657,6 +744,7 @@ int object_delete(const SaNameT **object > fprintf(stderr, "error - saImmOmCcbInitialize FAILED: > %s\n", saf_error(error)); > goto done; > } > + object_info_clear(); > } > > while (objectNames[i] != NULL) { > diff --git a/osaf/tools/safimm/immcfg/imm_import.cc > b/osaf/tools/safimm/immcfg/imm_import.cc > --- a/osaf/tools/safimm/immcfg/imm_import.cc > +++ b/osaf/tools/safimm/immcfg/imm_import.cc > @@ -68,6 +68,10 @@ extern "C" > SaImmHandleT *immHandle, SaImmAdminOwnerHandleT > *ownerHandle, > SaImmCcbHandleT *ccbHandle, int mode, const char > *xsdPath, int strictParse); > int validateImmXML(const char *xmlfile, int verbose, int mode, int > strictParse); > + > + void object_info_add(const char *objectName, const char *className); > + void object_info_clear(); > + char *object_info_get_class(const char *objectName); > } > > extern ImmutilErrorFnT immutilError; > @@ -638,7 +642,6 @@ static void createImmObject(ParserState* > SaImmAttrValuesT_2** attrValues; > SaAisErrorT errorCode = SA_AIS_OK; > int i = 0; > - size_t DNlen; > SaNameT objectName; > std::list<SaImmAttrValuesT_2>::iterator it; > > @@ -686,15 +689,17 @@ static void createImmObject(ParserState* > if(state->ctxt->instate == XML_PARSER_EOF) > return; > > + object_info_add(state->objectName, className); > + > #ifdef TRACE_8 > /* Get the length of the DN and truncate state->objectName */ > if (!osaf_is_extended_name_empty(&parentName)) { > - DNlen = strlen(state->objectName) - > (strlen(osaf_extended_name_borrow(&parentName)) + 1); > + i = strlen(state->objectName) - > (strlen(osaf_extended_name_borrow(&parentName)) + 1); > } else { > - DNlen = strlen(state->objectName); > + i = strlen(state->objectName); > } > > - state->objectName[DNlen] = '\0'; > + state->objectName[i] = '\0'; > TRACE_8("OBJECT NAME: %s", state->objectName); > #endif > > @@ -1704,6 +1709,7 @@ static void endElementHandler(void* user > state->parsingStatus = 1; > return; > } > + object_info_clear(); > state->ccbInit = 1; > > } ------------------------------------------------------------------------------ One dashboard for servers and applications across Physical-Virtual-Cloud Widest out-of-the-box monitoring support with 50+ applications Performance metrics, stats and reports that give you Actionable Insights Deep dive visibility with transaction tracing using APM Insight. http://ad.doubleclick.net/ddm/clk/290420510;117567292;y _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
