See inline Thanks, Hans > -----Original Message----- > From: Nagendra Kumar [mailto:[email protected]] > Sent: den 1 juli 2014 13:29 > To: Hans Feldt; Praveen Malviya > Cc: [email protected] > Subject: RE: [devel] [PATCH 1 of 1] amfd: allow creation of UNLOCKED service > units [#911] > > Thanks. > > [Hans] I tried to keep mw SUs as it is today. What restrictions do we > > have/need? > We need to document the changes created by this ticket in Amf PR. > > Following comments: > 1. traces should be converted to saflog or returned to user. Like : > TRACE("Create '%s', saAmfSUHostNodeOrNodeGroup not > configured", su_dn->value); [Hans] No because it is not an error, see below
> 2. Check for "saAmfSUHostNodeOrNodeGroup" and "safAmfNode are already in > is_config_valid, could be removed. [Hans] No because with this feature an SU is required to be configured to a node whereas in general it can be mapped in the SG to a node group and this attribute can be empty. We cannot check the node admin state when an SU is mapped to a nodegroup since the mapping to node has not been done yet. Will be at apply. > 3. Check for "safSg" is also in is_config_valid, could be removed. [Hans] could be replaced with osafassert I guess > 4. node_admin_state_is_valid_for_su_create and > sg_admin_state_is_valid_for_su_create could be combined. [Hans] just for clarity I kept them separate, the logic was not so easy to come up with.. > > Thanks > -Nagu > > > -----Original Message----- > > From: Hans Feldt [mailto:[email protected]] > > Sent: 01 July 2014 16:37 > > To: Nagendra Kumar; Praveen Malviya > > Cc: [email protected] > > Subject: RE: [devel] [PATCH 1 of 1] amfd: allow creation of UNLOCKED service > > units [#911] > > > > See inline > > Thanks, > > Hans > > > > > -----Original Message----- > > > From: Nagendra Kumar [mailto:[email protected]] > > > Sent: den 1 juli 2014 13:00 > > > To: Hans Feldt; Praveen Malviya > > > Cc: [email protected] > > > Subject: RE: [devel] [PATCH 1 of 1] amfd: allow creation of UNLOCKED > > > service units [#911] > > > > > > Two comments: > > > > > > 1. Ticket doesn't talk about mw restrictions, so the ticket need update. > > [Hans] I tried to keep mw SUs as it is today. What restrictions do we > > have/need? > > > > > 2. If SU can be allowed in unlocked state, it could be allowed into > > > locked state > > also. Is there any problem? > > [Hans] yes time, testing, lack of use case etc. > > > > > > Thanks > > > -Nagu > > > > > > > -----Original Message----- > > > > From: Hans Feldt [mailto:[email protected]] > > > > Sent: 30 June 2014 14:26 > > > > To: Praveen Malviya > > > > Cc: [email protected] > > > > Subject: [devel] [PATCH 1 of 1] amfd: allow creation of UNLOCKED > > > > service units [#911] > > > > > > > > osaf/services/saf/amf/amfd/su.cc | 158 > > > > +++++++++++++++++++++++++++++++++++--- > > > > 1 files changed, 143 insertions(+), 15 deletions(-) > > > > > > > > > > > > Allow creation of UNLOCKED SUs if the parent SG is LOCKED-IN or the > > > > hosting node is LOCKED-IN > > > > > > > > diff --git a/osaf/services/saf/amf/amfd/su.cc > > > > b/osaf/services/saf/amf/amfd/su.cc > > > > --- a/osaf/services/saf/amf/amfd/su.cc > > > > +++ b/osaf/services/saf/amf/amfd/su.cc > > > > @@ -1380,36 +1380,164 @@ done: > > > > } > > > > > > > > /** > > > > + * Validates if a node's admin state is valid for creating an SU > > > > + * Should only be called if the SU admin state is UNLOCKED, > > > > +otherwise the > > > > + * logged errors could be misleading. > > > > + * @param su_dn > > > > + * @param attributes > > > > + * @param opdata > > > > + * @return true if so > > > > + */ > > > > +static bool node_admin_state_is_valid_for_su_create(const SaNameT > > *su_dn, > > > > + const SaImmAttrValuesT_2 **attributes, > > > > + const CcbUtilOperationData_t *opdata) { > > > > + SaNameT node_name = {0}; > > > > + (void) immutil_getAttr("saAmfSUHostNodeOrNodeGroup", attributes, > > > > 0, &node_name); > > > > + if (node_name.length == 0) { > > > > + // attribute empty but this is probably not an error, > > > > just trace > > > > + TRACE("Create '%s', saAmfSUHostNodeOrNodeGroup not > > > > configured", > > > > + su_dn->value); > > > > + return false; > > > > + } > > > > + > > > > + if (strncmp((char*)node_name.value, "safAmfNode=", 11) != 0) { > > > > + // attribute non empty but does not contain a node DN, > > > > not OK > > > > + amflog(SA_LOG_SEV_NOTICE, > > > > + "Create '%s', saAmfSUHostNodeOrNodeGroup not > > > > configured with a node (%s)", > > > > + su_dn->value, node_name.value); > > > > + return false; > > > > + } > > > > + > > > > + const AVD_AVND *node = avd_node_get(&node_name); > > > > + if (node == NULL) { > > > > + // node must exist in the current model, not created in > > > > the same > > > > CCB > > > > + amflog(SA_LOG_SEV_WARNING, > > > > + "Create '%s', configured with a non existing > > > > node (%s)", > > > > + su_dn->value, node_name.value); > > > > + return false; > > > > + } > > > > + > > > > + // configured with a node DN, accept only locked-in state > > > > + if (node->saAmfNodeAdminState != > > > > SA_AMF_ADMIN_LOCKED_INSTANTIATION) { > > > > + TRACE("Create '%s', configured node '%s' is not locked > > > > instantiation", > > > > + su_dn->value, node_name.value); > > > > + return false; > > > > + } > > > > + > > > > + return true; > > > > +} > > > > + > > > > +/** > > > > + * Validates if an SG's admin state is valid for creating an SU > > > > + * Should only be called if the SU admin state is UNLOCKED, > > > > +otherwise the > > > > + * logged errors could be misleading. > > > > + * @param su_dn > > > > + * @param attributes > > > > + * @param opdata > > > > + * @return true if so > > > > + */ > > > > +static bool sg_admin_state_is_valid_for_su_create(const SaNameT > > *su_dn, > > > > + const SaImmAttrValuesT_2 **attributes, > > > > + const CcbUtilOperationData_t *opdata) { > > > > + SaNameT sg_name = {0}; > > > > + SaAmfAdminStateT admin_state; > > > > + > > > > + avsv_sanamet_init(su_dn, &sg_name, "safSg"); > > > > + const AVD_SG *sg = sg_db->find(Amf::to_string(&sg_name)); > > > > + if (sg != NULL) { > > > > + admin_state = sg->saAmfSGAdminState; > > > > + } else { > > > > + // SG does not exist in current model, check CCB > > > > + > > > > + const CcbUtilOperationData_t *tmp = > > > > + ccbutil_getCcbOpDataByDN(opdata->ccbId, > > > > &sg_name); > > > > + if (tmp == NULL) { > > > > + TRACE("Create '%s', parent SG does not exist", > > > > su_dn- > > > > >value); > > > > + return false; > > > > + } > > > > + > > > > + // SG created in the same CCB, get its admin state > > > > + (void) immutil_getAttr("saAmfSGAdminState", > > > > + tmp->param.create.attrValues, 0, &admin_state); > > > > + } > > > > + > > > > + if (admin_state != SA_AMF_ADMIN_LOCKED_INSTANTIATION) { > > > > + TRACE("'%s' created UNLOCKED but '%s' is not locked > > > > instantiation", > > > > + su_dn->value, sg_name.value); > > > > + return false; > > > > + } > > > > + > > > > + return true; > > > > +} > > > > + > > > > +/** > > > > * Validation performed when an SU is dynamically created with a CCB. > > > > * @param dn > > > > * @param attributes > > > > * @param opdata > > > > * > > > > - * @return int > > > > + * @return bool > > > > */ > > > > -static int is_ccb_create_config_valid(const SaNameT *dn, const > > > > SaImmAttrValuesT_2 **attributes, > > > > - const CcbUtilOperationData_t *opdata) > > > > +static bool is_ccb_create_config_valid(const SaNameT *dn, > > > > + const SaImmAttrValuesT_2 > > > > **attributes, > > > > + const CcbUtilOperationData_t > > > > +*opdata) > > > > { > > > > SaAmfAdminStateT admstate; > > > > - int is_app_su = 1; > > > > + SaAisErrorT rc; > > > > > > > > + assert(opdata != NULL); // must be called in CCB context > > > > + > > > > + bool is_mw_su = false; > > > > if (strstr((char *)dn->value, "safApp=OpenSAF") != NULL) > > > > - is_app_su = 0; > > > > + is_mw_su = true; > > > > > > > > - if (is_app_su && > > > > (immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfSUAdminState"), > > > > attributes, 0, &admstate) != SA_AIS_OK)) { > > > > - report_ccb_validation_error(opdata, "saAmfSUAdminState > > > > not > > > > configured for '%s'", dn->value); > > > > - return 0; > > > > - } > > > > + rc = immutil_getAttr("saAmfSUAdminState", attributes, 0, > > > > +&admstate); > > > > > > > > - if > > > > (immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfSUAdminState"), > > > > attributes, 0, &admstate) == SA_AIS_OK) { > > > > - if (admstate != SA_AMF_ADMIN_LOCKED_INSTANTIATION) { > > > > - report_ccb_validation_error(opdata, > > > > "saAmfSUAdminState must be LOCKED_INSTANTIATION(%u) for" > > > > - " dynamically created SUs", > > > > SA_AMF_ADMIN_LOCKED_INSTANTIATION); > > > > - return 0; > > > > + if (is_mw_su == true) { > > > > + // Allow MW SUs to be created without an admin state > > > > + if (rc != SA_AIS_OK) > > > > + return true; > > > > + > > > > + // value exist, it must be unlocked > > > > + if (admstate == SA_AMF_ADMIN_UNLOCKED) > > > > + return true; > > > > + else { > > > > + report_ccb_validation_error(opdata, > > > > + "admin state must be UNLOCKED for > > > > dynamically created MW SUs"); > > > > + return false; > > > > } > > > > } > > > > > > > > - return 1; > > > > + // A non MW SU (application SU), check admin state > > > > + // Default value is UNLOCKED if created without a value > > > > + if (rc != SA_AIS_OK) > > > > + admstate = SA_AMF_ADMIN_UNLOCKED; > > > > + > > > > + // locked-in state is fine > > > > + if (admstate == SA_AMF_ADMIN_LOCKED_INSTANTIATION) > > > > + return true; > > > > + > > > > + if (admstate != SA_AMF_ADMIN_UNLOCKED) { > > > > + report_ccb_validation_error(opdata, > > > > + "'%s' created with invalid saAmfSUAdminState > > > > (%u)", > > > > + dn->value, admstate); > > > > + return false; > > > > + } > > > > + > > > > + if (node_admin_state_is_valid_for_su_create(dn, attributes, > > > > opdata)) > > > > + return true; > > > > + > > > > + if (sg_admin_state_is_valid_for_su_create(dn, attributes, > > > > opdata)) > > > > + return true; > > > > + > > > > + amflog(SA_LOG_SEV_NOTICE, "CCB %d creation of '%s' failed", > > > > + opdata->ccbId, dn->value); > > > > + report_ccb_validation_error(opdata, > > > > + "SG or node not configured properly to allow creation of > > > > UNLOCKED SU"); > > > > + > > > > + return false; > > > > } > > > > > > > > static SaAisErrorT su_ccb_completed_cb(CcbUtilOperationData_t > > > > *opdata) > > > > > > > > -------------------------------------------------------------------- > > > > ---------- Open source business process management suite built on > > > > Java and Eclipse Turn processes into business applications with > > > > Bonita BPM Community Edition Quickly connect people, data, and > > > > systems into organized workflows Winner of BOSSIE, CODIE, OW2 and > > > > Gartner awards http://p.sf.net/sfu/Bonitasoft > > > > _______________________________________________ > > > > Opensaf-devel mailing list > > > > [email protected] > > > > https://lists.sourceforge.net/lists/listinfo/opensaf-devel ------------------------------------------------------------------------------ Open source business process management suite built on Java and Eclipse Turn processes into business applications with Bonita BPM Community Edition Quickly connect people, data, and systems into organized workflows Winner of BOSSIE, CODIE, OW2 and Gartner awards http://p.sf.net/sfu/Bonitasoft _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
