Hi,
A failed calloc() breaks the main flow and intended purpose of the
function, and perhaps the original author saw no better way to
gracefully handle and signify the failure except to assert. In our case,
the failure can be gracefully handled and signified as a failed
campaign. Again, it's just a matter of opinion whether to use assert. In
case SMF is not told in the campaign to wait for callback response from
the client, why crashing the client if it misses a callback whose
response is not important to the upgrade progress?
I guess the agent you mentioned is the cbk_util thread of SMFD. If it
crashes, no rollback will be triggered, and the campaign will fail.
Thanks,
Nguyen
On 7/6/2018 3:04 PM, Tran Thuan wrote:
Hi Nguyen,
In that function, some osafassert(0) is used when fail calloc(), why don't just
ignore as your idea?
Just wonder one point:
Agent is a thread of SMFD, it crash mean SMFD crash (node reboot?), is there
any rollback trigger after that?
If it cannot, then I should handle posting fail as your comment.
Best Regards,
Thuan
-----Original Message-----
From: Nguyen Luu <nguyen.tk....@dektech.com.au>
Sent: Thursday, July 5, 2018 8:00 PM
To: Tran Thuan <thuan.t...@dektech.com.au>; lennart.l...@ericsson.com; Gary Lee
<gary....@dektech.com.au>
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [devel] [PATCH 1/1] smf: Increase cbk count before post the evt to
client [#2882]
Hi Thuan,
I think it's not a good idea and also not that critical to crash the SMF client
just because of the agent failing to send a callback message. It also depends
on whether the client expects SMF to wait for its response on callback in order
to proceed with the upgrade or not, which behavior is specified by the callback
timeout value in the campaign file.
With the current handling, SMF will not wait for the client response on
callback if the timeout is '0', and will just continue executing the campaign;
otherwise it will wait for as long as the specified timeout and will fail the
campaign if not receiving the callback response within that period. A failed
campaign and an error message in the log should be good enough for SMF to
communicate the failure in this situation. It's not that severe to call abort()
and generate core dump in this case.
Perhaps Lennart and Gary can have further comments/ideas on this solution.
Thanks,
Nguyen
On 7/5/2018 4:11 PM, Tran Thuan wrote:
Hi Nguyen,
Thanks for your comments. I agree with general comment.
About the patch (new flow), when posting to the client mailbox fail,
osafassert(0) is called.
I think the posting fail is critical problem since agent will miss a callback.
Also avoid smfnd is keeping waiting for agent response for that callback.
Please correct if I was wrong.
Best Regards,
Thuan (UFO – Unique FBI Opensaf)
CoreMW Maintenance, DEK VietNam
-----Original Message-----
From: Nguyen Luu <nguyen.tk....@dektech.com.au>
Sent: Wednesday, July 4, 2018 5:08 PM
To: thuan.tran <thuan.t...@dektech.com.au>; lennart.l...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [devel] [PATCH 1/1] smf: Increase cbk count before post
the evt to client [#2882]
Hi,
One comment from me for the patch: With the new flow, the matched hdl should be
removed from the cbk_list if posting the message to the client mailbox fails.
A general comment: The original issue happened because of a race condition in
which the smfa_cb was used in different threads without proper use of mutex.
This patch may solve the issue, but the agent code should be improved to use
the mutex better, to avoid any other hidden race conditions. Perhaps a future
enhancement.
Thanks,
Nguyen
On 6/26/2018 11:32 AM, thuan.tran wrote:
Sometimes, callback agent dispatch and fail at saSmfReponse() because
cbk list is empty, agent by somehow handle evt before increase cbk
count. To avoid this, increase cbk count before post the evt.
---
src/smf/agent/smfa_utils.c | 38 ++++++++++++++++++++------------------
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/src/smf/agent/smfa_utils.c b/src/smf/agent/smfa_utils.c
index fb31a9a..917b6b8 100644
--- a/src/smf/agent/smfa_utils.c
+++ b/src/smf/agent/smfa_utils.c
@@ -615,8 +615,8 @@ SMFA_CBK_HDL_LIST *smfa_inv_hdl_add(SaInvocationT inv_id,
SaSmfHandleT hdl)
}
/***************************************************************************
-@brief : Match the filter. If matches, post the evt to the client MBX
- and increment the cbk count of the the corresponding hdl node.
+@brief : Match the filter. If matches, increment the cbk count of
+ the corresponding hdl node and post the evt to the client MBX.
If for a client, more than one scope matches, then those many
no of evts are posted to the MBX.
@param[in] : client_info - For which filter match to be performed.
@@ -694,27 +694,29 @@ uint32_t smfa_cbk_filter_match(SMFA_CLIENT_INFO
*client_info,
&cbk_evt->object_name),
&evt->evt.cbk_evt.object_name);
- if (m_NCS_IPC_SEND(&client_info->cbk_mbx,
- (NCSCONTEXT)evt,
- NCS_IPC_PRIORITY_NORMAL)) {
- /* Increment the cbk count.*/
- if (NULL != hdl_list) {
- /* There are two scope id
- * matching for the same hdl.*/
- } else {
- /* First scope id matching for
- * this hdl.*/
- hdl_list = smfa_inv_hdl_add(
- cbk_evt->inv_id,
- client_info->client_hdl);
- }
- hdl_list->cnt++;
- rc = NCSCC_RC_SUCCESS;
+ /* Increment the cbk count.*/
+ if (NULL != hdl_list) {
+ /* There are two scope id
+ * matching for the same hdl.*/
} else {
+ /* First scope id matching for
+ * this hdl.*/
+ hdl_list = smfa_inv_hdl_add(
+ cbk_evt->inv_id,
+ client_info->client_hdl);
+ }
+ hdl_list->cnt++;
+ rc = NCSCC_RC_SUCCESS;
+ if (m_NCS_IPC_SEND(
+ &client_info->cbk_mbx,
+ (NCSCONTEXT)evt,
+ NCS_IPC_PRIORITY_NORMAL)
+ != NCSCC_RC_SUCCESS) {
LOG_ER(
"SMFA: Posting to MBX failed. hdl: %llu,
scoe_id: %u",
client_info->client_hdl,
cbk_evt->scope_id);
+ osafassert(0);
}
/* If one of the filter matches then go to the
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
