On 15-Aug-16 5:05 PM, Long Nguyen wrote:
Hi Praveen,
Thanks for your suggestions.
Please see my comments marked with [Long[.
Best regards,
Long Nguyen.
On 8/12/2016 5:03 PM, praveen malviya wrote:
On 12-Aug-16 1:27 PM, Long Nguyen wrote:
Hi Praveen,
Actually, since Anders introduced the extended SaNameT in leap core, he
also added the osaf_extended_name_init() into leap.
Amf agent uses leap library (i.e. saAmfInitialize()). So, applications
under amf control enabled long DN implicitly.
The SA_AIS_ERR_NAME_TOO_LONG return code is not returned in our case
(still SA_AIS_OK) due to the above reason. Consequently, the
applications are not crashed.
I am not sure if it is right to add osaf_extended_name_init() into leap.
However, it is the case now.
Addition of osaf_extended_name_init() into leap is right because
every agent has to support long and short dn applications. But
osaf_extended_name_init() distinguish a long or short dn application
based on whether application has set "SA_ENABLE_EXTENDED_NAMES" or
not.If you see the implementation of this API, based on this exported
variable it remembers whether application is a long dn one or short dn
one. For this only only applications are recommended via documentation
to set this environment variable before calling any SAF API if they
want handle longdn.In this way when first SAF API is called (generally
sa,*>Initialize()), leap initialization will be done as a part of
agent creation. Inside leap init it will call
osaf_extended_name_init() which will use the environment variable to
remember about the nature of application.
The idea of CCB rejection was based on this only that an agent knows
via osaf_extended_* APIs that it is created by a longdn or shortdn
application. And this information can be passed to amfnd and hence to
AMFD.
But now what needs to be explored is why AMF Dispatch() is not
returning NAME_TOO_LONG when you have already coded for it? Since old
AMF demo has not enabled long dn, Dispatch() must return this error code.
Is there any problem with ava_sanamet_is_valid()?
Let us debug that.
Can you please share with us your ideas? Do we still need to truncate
long names in this case?
As we have discussed, rejection of CCB can be postponed as it requires
new messages or introduction of new fields in old messages, all the
way from AMFA to AMFD.
So we are left with three options:
1) Dispatch() will return NAME_TOO_LONG without invoking callback.
Current patch is doing this.
This has one disadvantage that comp/application may exit/crash.
2)Dispatch will return SA_AIS_OK "without" invoking the callback.
This will not confuse application.
There is one disadvantage here, AMF has an illegal COMPCSI.
3)Dispatch will return SA_AIS_OK "by" invoking the callback with
truncated values.
This has one advantage that AMF does not have illegal COMPCSI.
One disadvantage is that if comp is serving more CSI and then it
will not be able to rightly distinguish between the CSI names.
Option 2) sounds Ok to me as it will not create problems with
application as AMF is not invoking any callback and atleast
application is serving existing CSIs. Regarding the illegal COMPCSI in
AMF: A user can always delete it and deletion will not again result in
CSI remove callback because of the same reasons. At the same time we
are cautioning the user via documentation (REAMDME and PR doc).
[Long] I think the option 3 is better than the option 2. If we ignore
the callback, amf does not receive the responses from applications and
then applications get restarted due to callback timeout.
[Praveen] That was the problem with the current patch also, in which
callback is not invoked and NAME_TOO_LONG is returned in dispatch(). So
AMFND was already getting time out for the callback.
For option 2) there can be other consideration like generating fake
Response() to AMFND or blocking the callback at AMFND itself (I was
trying this blocking at amfnd approach crude patch in attachment. From
agent to amfnd it is flat encode/decode advantage. But this needs to be
explored further and will be complex. But it can act as first step
towards CCB rejection.).
For option 3) how the truncation should be done to make it unique? Also
there can still be one challenge: if a component on receiving this
truncated CSI use it for PG tracking by calling the PG tracking API(with
truncated CSI being an input param). So agent/amfnd will have to
remember this reverse mapping between truncated CSI and actual CSI.
I think we can look for ideas from other services in which long dn
support is there and similar situation was faced.
Thanks,
Praveen
thanks,
Praveen
Best regards,
Long Nguyen.
On 8/11/2016 3:04 PM, Long Nguyen wrote:
Hi Praveen,
Thanks for your suggestion.
In the situation you described below, you add a csi dynamically (long
DN) to an application (not support long DN).
So, we only need to truncate the csi name. This will help the
application not crashed.
In my opinion, we only need to truncate in case of CSISet or
CSIRemove. There may be some problems with applications associated
with more than one csi, if the applications base on csi names to do
special things.
In case of protection group, when we call saAmfProtectionGroupTrack,
we have to specify the csi_name. It is not in the case of long DN
since the application does not support long DN.
Best regards,
Long Nguyen.
On 8/11/2016 1:29 PM, praveen malviya wrote:
Hi,
I think, as of now, approaches discussed in this mail thread can be
mixed for a temporary solution: Dispatch() can return SA_AIS_OK by
invoking the callback(s) with truncated values for any long field
(like comp name, csi name etc). In this way comp will have callbacks
and AMF will also be maintaining legal COMPCSIs. Actual values can be
logged. Also the the PR doc and Readme should talk how truncation
will be performed.
I think, this can be done in this release.
Other solution of rejecting the CCB itself can be postponed as of now.
Thanks,
Praveen
On 11-Aug-16 10:03 AM, Long Nguyen wrote:
Hi Praveen and others,
I have an idea marked with [Long] below.
Best regards,
Long Nguyen.
On 8/3/2016 1:45 PM, praveen malviya wrote:
On 02-Aug-16 3:39 AM, minh chau wrote:
Hi Praveen,
One comment with [Minh] in line.
Thanks,
Minh
On 01/08/16 17:22, Gary Lee wrote:
Hi Praveen
On 1/08/2016 4:29 PM, praveen malviya wrote:
Hi Gary, Long,
Some comments/observations:
-In AMFD saAisNameBorrow() is used in logging and AMFND uses
osaf_extended_name_borrow().
For osaf_extended_name_borrow() note in osaf_extended_name.h
says it
is intended for mainly agent libraries. But middle-ware services
always use core libs. At the same time saAisNameBorrow(), I
think, is
for application.
any reason of using them this way and what is the recommended
way?
I think I used both styles in amfd. I think we can change
saAisNameXX
to osaf_extended_name_XX just before pushing, to make it
consistent
with the rest of the OpenSAF services.
-I think, one case may arrive from upgrade perspective.
Suppose any application (say amf_demo app) is running without
enabling long dn and a csi, with its RDn greater than 256, is
added
dynamically (long dn enabled in IMM). In this case AMFD will
assign
this csi to the running component. Component will not be able to
read
the CSI and may crash.
This is related to invocation of CSI_SET callback but same
will be
valid for PG tracking also. There may be other cases also.
Even truncation will not work in this case.
[Minh] I think the agent patch that Gary submitted currently
returns
SA_AIS_ERR_NAME_TOO_LONG in saAmfDispatch() if long DN callback
comes to
legacy application (unadapted long DN app). The real callback
won't be
issued but application may crash if it exit() on non-SA_AIS_OK from
Dispatch(). I guess you have seen this with #1553? Do you think
it's
good way if amf agent drops the long DN callback and also
Dispatch()
returns OK to legacy app, and print error in syslog?
Not observed in the context of #1553, but I remember about such a
fix
in NTF long dn changes.
Returning SA_AIS_OK in Dispatch() call saves application from crash,
but the problem in AMF is still different as it will maintain a
COMPCSI relationship without comp being actually assigned for that.
Can we think of a way where AMF will not allow this conf change by
rejecting the CCB operation itself. For this AMF should know that
this
application supports Long dn or not. But this information needs
to be
carried from agent to AMFD.
Before going for such a way, I think, we can ask opinion from other
AMF maintainers.
[Long] You have the good solution. However, it is hard to know if an
application supports long DN or not at the time of CCB
operration. Can
we make your suggestion as an enhancement later?
Thanks,
Praveen
- While running some tests observed crashes in amfnd and amfd.
I will update #1642 with bt information.
Minh will answer this bit.
Thanks
Gary
diff --git a/osaf/libs/agents/saf/amfa/amf_agent.cc
b/osaf/libs/agents/saf/amfa/amf_agent.cc
--- a/osaf/libs/agents/saf/amfa/amf_agent.cc
+++ b/osaf/libs/agents/saf/amfa/amf_agent.cc
@@ -525,6 +525,7 @@ SaAisErrorT AmfAgent::ComponentRegister(
else
memset(&pcomp_name, 0, sizeof(SaNameT));
ava_fill_comp_reg_msg(&msg, cb->ava_dest, hdl, *comp_name, pcomp_name);
+ msg.info.api_info.param.reg.longdn_enabled_comp =
osaf_extended_names_enabled ;
rc = static_cast<SaAisErrorT>(ava_mds_send(cb, &msg, &msg_rsp));
if (NCSCC_RC_SUCCESS == rc) {
osafassert(AVSV_AVND_AMF_API_RESP_MSG == msg_rsp->type);
diff --git a/osaf/libs/agents/saf/amfa/include/ava.h
b/osaf/libs/agents/saf/amfa/include/ava.h
--- a/osaf/libs/agents/saf/amfa/include/ava.h
+++ b/osaf/libs/agents/saf/amfa/include/ava.h
@@ -49,5 +49,7 @@
/* AvA CB global handle declaration */
extern uint32_t gl_ava_hdl;
+extern bool osaf_extended_names_enabled;
+
#endif /* !AVA_H */
diff --git a/osaf/libs/common/amf/include/amf_amfparam.h
b/osaf/libs/common/amf/include/amf_amfparam.h
--- a/osaf/libs/common/amf/include/amf_amfparam.h
+++ b/osaf/libs/common/amf/include/amf_amfparam.h
@@ -103,6 +103,7 @@ typedef struct avsv_amf_comp_reg_param_t
SaAmfHandleT hdl; /* AMF handle */
SaNameT comp_name; /* comp name */
SaNameT proxy_comp_name; /* proxy comp name */
+ bool longdn_enabled_comp;
} AVSV_AMF_COMP_REG_PARAM;
/* component unregister */
diff --git a/osaf/services/saf/amf/amfnd/comp.cc
b/osaf/services/saf/amf/amfnd/comp.cc
--- a/osaf/services/saf/amf/amfnd/comp.cc
+++ b/osaf/services/saf/amf/amfnd/comp.cc
@@ -725,6 +725,7 @@ uint32_t avnd_comp_reg_prc(AVND_CB *cb,
comp->reg_hdl = reg->hdl;
comp->reg_dest = *dest;
m_AVND_COMP_REG_SET(comp);
+ comp->is_longdn_enabled_comp = reg->longdn_enabled_comp;
/* if proxied comp, do add to the pxied_list of pxy */
if (m_AVND_COMP_TYPE_IS_PROXIED(comp))
@@ -1888,6 +1889,7 @@ uint32_t avnd_comp_curr_info_del(AVND_CB
return rc;
}
+
/****************************************************************************
Name : avnd_comp_cbk_send
@@ -1950,6 +1952,15 @@ uint32_t avnd_comp_cbk_send(AVND_CB *cb,
rc = NCSCC_RC_FAILURE;
goto done;
}
+ if ((curr_csi->name.length() >
SA_MAX_UNEXTENDED_NAME_LENGTH)
+ && !comp->is_longdn_enabled_comp) {
+ LOG_NO("'%s' is an extended name and comp does
not support longdn "
+ "so not invoking CSI set
callback for '%s'",
+
curr_csi->name.c_str(),comp->name.c_str());
+ rc = NCSCC_RC_FAILURE;
+ goto done;
+ }
+
/*
* Populate the csi-desc structure.
diff --git a/osaf/services/saf/amf/amfnd/compdb.cc
b/osaf/services/saf/amf/amfnd/compdb.cc
--- a/osaf/services/saf/amf/amfnd/compdb.cc
+++ b/osaf/services/saf/amf/amfnd/compdb.cc
@@ -1463,6 +1463,7 @@ static AVND_COMP *avnd_comp_create(const
comp->su = su;
comp->error_report_sent = false;
comp->admin_oper = false;
+ comp->is_longdn_enabled_comp = true;
if (true == su->su_is_external) {
m_AVND_COMP_TYPE_SET_EXT_CLUSTER(comp);
diff --git a/osaf/services/saf/amf/amfnd/include/avnd_comp.h
b/osaf/services/saf/amf/amfnd/include/avnd_comp.h
--- a/osaf/services/saf/amf/amfnd/include/avnd_comp.h
+++ b/osaf/services/saf/amf/amfnd/include/avnd_comp.h
@@ -400,7 +400,7 @@ typedef struct avnd_comp_tag {
std::bitset<NumAttrs> *use_comptype_attr;
SaInvocationT term_cbq_inv_value; /* invocation value for termination
callback. */
-
+ bool is_longdn_enabled_comp;
} AVND_COMP;
#define AVND_COMP_NULL ((AVND_COMP *)0)
------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
