Dear Mahesh, Thank you very much for your comment.
Would you please verify again my understanding about this. Old cpnd_ckpt_sec_del() function and new cpnd_ckpt_sec_del_db() search for the sectionInfo in 2 maps by its id and remove if found. In case ckpt_hdr_update_fails, the cpnd_ckpt_sec_del_db() is called twice, the first one remove sectionInfo from maps, the second one does not found sectionInfo and cannot remove anything, will not generate error. Sincerely, Hoang -----Original Message----- From: A V Mahesh [mailto:mahesh.va...@oracle.com] Sent: Friday, November 25, 2016 12:12 PM To: Hoang Vo <hoang.m...@dektech.com.au>; anders.wid...@ericsson.com Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [PATCH 1 of 1] cpnd: fix error handling while section_hdr_update_fail [#2207] Hi Hoang, NACK This patch has some fundamental problem, this will corrupt the cpsv database . This patch instead of resolving the issue , increase the percentage of problem occurrence . Basically this patch splits cpnd_ckpt_sec_del() in to two parts cpnd_ckpt_sec_del_db() & cpnd_ckpt_sec_del() and the new cpnd_ckpt_sec_del() invokes cpnd_ckpt_sec_del_db() to fullfil the old function behavior , so that to support other old invocations and changed the error cleanup in cpnd_ckpt_sec_add() as follows : ckpt_hdr_update_fails: cpnd_ckpt_sec_del(cp_node, id); section_hdr_update_fails: cpnd_ckpt_sec_del_db(cp_node, id); This means cpnd_ckpt_sec_del_db() is always called twice in case of ckpt_hdr_update_fails , which will generate core -AVM On 11/24/2016 3:26 PM, Hoang Vo wrote: > osaf/libs/common/cpsv/include/cpnd_sec.h | 3 +++ > osaf/services/saf/cpsv/cpnd/cpnd_db.c | 4 +++- > osaf/services/saf/cpsv/cpnd/cpnd_sec.cc | 31 +++++++++++++++++++++++++++---- > 3 files changed, 33 insertions(+), 5 deletions(-) > > > problem: > the steps to add a section is add_db_tree -> update_sec_hdr -> > update_ckpt_hdr so if an error occur cpsv should handle error in reverse order. > currently, section_hdr_update_fails, cpsv revert ckpt_hdr also that > case error > > solution: > only revert db_tree in case section_hdr_update_fails > > diff --git a/osaf/libs/common/cpsv/include/cpnd_sec.h > b/osaf/libs/common/cpsv/include/cpnd_sec.h > --- a/osaf/libs/common/cpsv/include/cpnd_sec.h > +++ b/osaf/libs/common/cpsv/include/cpnd_sec.h > @@ -39,6 +39,9 @@ CPND_CKPT_SECTION_INFO * > cpnd_ckpt_sec_get_create(const CPND_CKPT_NODE *, const > SaCkptSectionIdT *); > > CPND_CKPT_SECTION_INFO * > +cpnd_ckpt_sec_del_db(CPND_CKPT_NODE *, SaCkptSectionIdT *); > + > +CPND_CKPT_SECTION_INFO * > cpnd_ckpt_sec_del(CPND_CKPT_NODE *, SaCkptSectionIdT *); > > CPND_CKPT_SECTION_INFO * > diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_db.c > b/osaf/services/saf/cpsv/cpnd/cpnd_db.c > --- a/osaf/services/saf/cpsv/cpnd/cpnd_db.c > +++ b/osaf/services/saf/cpsv/cpnd/cpnd_db.c > @@ -468,10 +468,12 @@ CPND_CKPT_SECTION_INFO *cpnd_ckpt_sec_ad > TRACE_LEAVE(); > return pSecPtr; > > - section_hdr_update_fails: > ckpt_hdr_update_fails: > cpnd_ckpt_sec_del(cp_node, id); > > + section_hdr_update_fails: > + cpnd_ckpt_sec_del_db(cp_node, id); > + > section_add_fails: > if (pSecPtr->sec_id.id != NULL) > m_MMGR_FREE_CPND_DEFAULT(pSecPtr->sec_id.id); > diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_sec.cc > b/osaf/services/saf/cpsv/cpnd/cpnd_sec.cc > --- a/osaf/services/saf/cpsv/cpnd/cpnd_sec.cc > +++ b/osaf/services/saf/cpsv/cpnd/cpnd_sec.cc > @@ -157,19 +157,19 @@ cpnd_ckpt_sec_find(const CPND_CKPT_NODE > } > > /*************************************************************************** * > - * Name : cpnd_ckpt_sec_del > + * Name : cpnd_ckpt_sec_del_db > * > - * Description : Function to remove the section from a checkpoint. > + * Description : Function to remove the section from a checkpoint map db. > * > * Arguments : CPND_CKPT_NODE *cp_node - Check point node. > * : SaCkptSectionIdT id - Section Identifier > - * > + * > * Return Values : ptr to CPND_CKPT_SECTION_INFO/NULL; > * > * Notes : None. > **************************************************************************** */ > CPND_CKPT_SECTION_INFO * > -cpnd_ckpt_sec_del(CPND_CKPT_NODE *cp_node, SaCkptSectionIdT *id) > +cpnd_ckpt_sec_del_db(CPND_CKPT_NODE *cp_node, SaCkptSectionIdT *id) > { > CPND_CKPT_SECTION_INFO *sectionInfo(0); > > @@ -206,6 +206,29 @@ cpnd_ckpt_sec_del(CPND_CKPT_NODE *cp_nod > osafassert(false); > } > > + TRACE_LEAVE(); > + > + return sectionInfo; > +} > + > +/************************************************************************** ** > + * Name : cpnd_ckpt_sec_del > + * > + * Description : Function to remove the section from a checkpoint. > + * > + * Arguments : CPND_CKPT_NODE *cp_node - Check point node. > + * : SaCkptSectionIdT id - Section Identifier > + * > + * Return Values : ptr to CPND_CKPT_SECTION_INFO/NULL; > + * > + * Notes : None. > + > +********************************************************************* > +********/ > +CPND_CKPT_SECTION_INFO * > +cpnd_ckpt_sec_del(CPND_CKPT_NODE *cp_node, SaCkptSectionIdT *id) { > + TRACE_ENTER(); > + CPND_CKPT_SECTION_INFO *sectionInfo = cpnd_ckpt_sec_del_db(cp_node, > +id); > + > if (sectionInfo) { > cp_node->replica_info.n_secs--; > cp_node->replica_info.mem_used = cp_node->replica_info.mem_used > - (sectionInfo->sec_size); ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
