Re: [devel] [PATCH 1 of 1] amfnd: queue PG track action msgs [#2115]

[Gary Lee]

Hi Praveen

Yeah, but at least it’s added to “diq”. So when amfnd gets set_led again, the 
message will be retransmitted and accepted by amfd.

Thanks
Gary

> On 14 Oct. 2016, at 4:40 pm, praveen malviya  
> wrote:
> 
> 
> But I think same problem will still come as AMFND is trying to send this 
> newly added record only in the same function avnd_di_msg_send():
>  /* add the record to the AvD msg list */
>if ((0 != (rec = avnd_diq_rec_add(cb, msg {
>/* send the message */
>avnd_diq_rec_send(cb, rec);
>} else
>rc = NCSCC_RC_FAILURE;
> 
> Thanks,
> Praveen
> 
> 
> On 14-Oct-16 10:59 AM, Gary Lee wrote:
>> Hi Praveen
>> 
>> Yes, I will change it to make it clearer.
>> 
>> Thanks
>> 
>>> On 14 Oct. 2016, at 4:27 pm, praveen malviya  
>>> wrote:
>>> 
>>> Hi Gary,
>>> 
>>> one query inline with [Praveen]
>>> 
>>> Thanks,
>>> Praveen
>>> 
>>> On 13-Oct-16 4:20 AM, Gary Lee wrote:
 osaf/services/saf/amf/amfnd/di.cc |  8 ++--
 1 files changed, 2 insertions(+), 6 deletions(-)
 
 
 After SC absence, active amfd will reject messages from 'veteran' amfnds 
 until
 its local amfnd has started. During this period, if a PG track action msg
 is sent and rejected by amfd, it will cause the sending amfnd to lose sync
>>> [Praveen] I think here "lose sync" means it will cause message id mismatch 
>>> later.
>>> 
 with amfd. So we should also queue this message to be re-sent.
 
 diff --git a/osaf/services/saf/amf/amfnd/di.cc 
 b/osaf/services/saf/amf/amfnd/di.cc
 --- a/osaf/services/saf/amf/amfnd/di.cc
 +++ b/osaf/services/saf/amf/amfnd/di.cc
 @@ -1033,12 +1033,8 @@ uint32_t avnd_di_msg_send(AVND_CB *cb, A
if (!msg)
goto done;
 
 -  /* Verify Ack nack and PG track action msgs are not buffered */
 -  if (m_AVSV_N2D_MSG_IS_PG_TRACK_ACT(msg->info.avd)) {
 -  /*send the response to AvD */
 -  rc = avnd_mds_send(cb, msg, >avd_dest, 0);
 -  goto done;
 -  } else if (m_AVSV_N2D_MSG_IS_VER_ACK_NACK(msg->info.avd)) {
 +  /* Verify Ack nack msgs are not buffered */
 +  if (m_AVSV_N2D_MSG_IS_VER_ACK_NACK(msg->info.avd)) {
/*send the response to active AvD (In case MDS has not updated 
 its
   tables by this time) */
TRACE_1("%s, Active AVD Adest: %" PRIu64, __FUNCTION__, 
 cb->active_avd_adest);
 
>> 


--
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

Re: [devel] [PATCH 1 of 1] amfnd: queue PG track action msgs [#2115]

[praveen malviya]

But I think same problem will still come as AMFND is trying to send this 
newly added record only in the same function avnd_di_msg_send():
   /* add the record to the AvD msg list */
 if ((0 != (rec = avnd_diq_rec_add(cb, msg {
 /* send the message */
 avnd_diq_rec_send(cb, rec);
 } else
 rc = NCSCC_RC_FAILURE;

Thanks,
Praveen


On 14-Oct-16 10:59 AM, Gary Lee wrote:
> Hi Praveen
>
> Yes, I will change it to make it clearer.
>
> Thanks
>
>> On 14 Oct. 2016, at 4:27 pm, praveen malviya  
>> wrote:
>>
>> Hi Gary,
>>
>> one query inline with [Praveen]
>>
>> Thanks,
>> Praveen
>>
>> On 13-Oct-16 4:20 AM, Gary Lee wrote:
>>> osaf/services/saf/amf/amfnd/di.cc |  8 ++--
>>> 1 files changed, 2 insertions(+), 6 deletions(-)
>>>
>>>
>>> After SC absence, active amfd will reject messages from 'veteran' amfnds 
>>> until
>>> its local amfnd has started. During this period, if a PG track action msg
>>> is sent and rejected by amfd, it will cause the sending amfnd to lose sync
>> [Praveen] I think here "lose sync" means it will cause message id mismatch 
>> later.
>>
>>> with amfd. So we should also queue this message to be re-sent.
>>>
>>> diff --git a/osaf/services/saf/amf/amfnd/di.cc 
>>> b/osaf/services/saf/amf/amfnd/di.cc
>>> --- a/osaf/services/saf/amf/amfnd/di.cc
>>> +++ b/osaf/services/saf/amf/amfnd/di.cc
>>> @@ -1033,12 +1033,8 @@ uint32_t avnd_di_msg_send(AVND_CB *cb, A
>>> if (!msg)
>>> goto done;
>>>
>>> -   /* Verify Ack nack and PG track action msgs are not buffered */
>>> -   if (m_AVSV_N2D_MSG_IS_PG_TRACK_ACT(msg->info.avd)) {
>>> -   /*send the response to AvD */
>>> -   rc = avnd_mds_send(cb, msg, >avd_dest, 0);
>>> -   goto done;
>>> -   } else if (m_AVSV_N2D_MSG_IS_VER_ACK_NACK(msg->info.avd)) {
>>> +   /* Verify Ack nack msgs are not buffered */
>>> +   if (m_AVSV_N2D_MSG_IS_VER_ACK_NACK(msg->info.avd)) {
>>> /*send the response to active AvD (In case MDS has not updated 
>>> its
>>>tables by this time) */
>>> TRACE_1("%s, Active AVD Adest: %" PRIu64, __FUNCTION__, 
>>> cb->active_avd_adest);
>>>
>

--
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

Re: [devel] [PATCH 1 of 1] amfnd: queue PG track action msgs [#2115]

[Gary Lee]

Hi Praveen

Yes, I will change it to make it clearer.

Thanks

> On 14 Oct. 2016, at 4:27 pm, praveen malviya  
> wrote:
> 
> Hi Gary,
> 
> one query inline with [Praveen]
> 
> Thanks,
> Praveen
> 
> On 13-Oct-16 4:20 AM, Gary Lee wrote:
>> osaf/services/saf/amf/amfnd/di.cc |  8 ++--
>> 1 files changed, 2 insertions(+), 6 deletions(-)
>> 
>> 
>> After SC absence, active amfd will reject messages from 'veteran' amfnds 
>> until
>> its local amfnd has started. During this period, if a PG track action msg
>> is sent and rejected by amfd, it will cause the sending amfnd to lose sync
> [Praveen] I think here "lose sync" means it will cause message id mismatch 
> later.
> 
>> with amfd. So we should also queue this message to be re-sent.
>> 
>> diff --git a/osaf/services/saf/amf/amfnd/di.cc 
>> b/osaf/services/saf/amf/amfnd/di.cc
>> --- a/osaf/services/saf/amf/amfnd/di.cc
>> +++ b/osaf/services/saf/amf/amfnd/di.cc
>> @@ -1033,12 +1033,8 @@ uint32_t avnd_di_msg_send(AVND_CB *cb, A
>>  if (!msg)
>>  goto done;
>> 
>> -/* Verify Ack nack and PG track action msgs are not buffered */
>> -if (m_AVSV_N2D_MSG_IS_PG_TRACK_ACT(msg->info.avd)) {
>> -/*send the response to AvD */
>> -rc = avnd_mds_send(cb, msg, >avd_dest, 0);
>> -goto done;
>> -} else if (m_AVSV_N2D_MSG_IS_VER_ACK_NACK(msg->info.avd)) {
>> +/* Verify Ack nack msgs are not buffered */
>> +if (m_AVSV_N2D_MSG_IS_VER_ACK_NACK(msg->info.avd)) {
>>  /*send the response to active AvD (In case MDS has not updated 
>> its
>> tables by this time) */
>>  TRACE_1("%s, Active AVD Adest: %" PRIu64, __FUNCTION__, 
>> cb->active_avd_adest);
>> 


--
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

Re: [devel] [PATCH 1 of 1] amfnd: queue PG track action msgs [#2115]

[praveen malviya]

Hi Gary,

one query inline with [Praveen]

Thanks,
Praveen

On 13-Oct-16 4:20 AM, Gary Lee wrote:
>  osaf/services/saf/amf/amfnd/di.cc |  8 ++--
>  1 files changed, 2 insertions(+), 6 deletions(-)
>
>
> After SC absence, active amfd will reject messages from 'veteran' amfnds until
> its local amfnd has started. During this period, if a PG track action msg
> is sent and rejected by amfd, it will cause the sending amfnd to lose sync
[Praveen] I think here "lose sync" means it will cause message id 
mismatch later.

> with amfd. So we should also queue this message to be re-sent.
>
> diff --git a/osaf/services/saf/amf/amfnd/di.cc 
> b/osaf/services/saf/amf/amfnd/di.cc
> --- a/osaf/services/saf/amf/amfnd/di.cc
> +++ b/osaf/services/saf/amf/amfnd/di.cc
> @@ -1033,12 +1033,8 @@ uint32_t avnd_di_msg_send(AVND_CB *cb, A
>   if (!msg)
>   goto done;
>
> - /* Verify Ack nack and PG track action msgs are not buffered */
> - if (m_AVSV_N2D_MSG_IS_PG_TRACK_ACT(msg->info.avd)) {
> - /*send the response to AvD */
> - rc = avnd_mds_send(cb, msg, >avd_dest, 0);
> - goto done;
> - } else if (m_AVSV_N2D_MSG_IS_VER_ACK_NACK(msg->info.avd)) {
> + /* Verify Ack nack msgs are not buffered */
> + if (m_AVSV_N2D_MSG_IS_VER_ACK_NACK(msg->info.avd)) {
>   /*send the response to active AvD (In case MDS has not updated 
> its
>  tables by this time) */
>   TRACE_1("%s, Active AVD Adest: %" PRIu64, __FUNCTION__, 
> cb->active_avd_adest);
>

--
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

Re: [devel] [PATCH 1 of 1] amfd: fix incorrect ER messages in sg_2n_fsm.cc [#1897]

[praveen malviya]

Ack, code review only.

one minor comment.

Thanks
Praveen

On 10-Oct-16 9:56 AM, Long HB Nguyen wrote:
>  osaf/services/saf/amf/amfd/sg_2n_fsm.cc |  10 +-
>  1 files changed, 5 insertions(+), 5 deletions(-)
>
>
> diff --git a/osaf/services/saf/amf/amfd/sg_2n_fsm.cc 
> b/osaf/services/saf/amf/amfd/sg_2n_fsm.cc
> --- a/osaf/services/saf/amf/amfd/sg_2n_fsm.cc
> +++ b/osaf/services/saf/amf/amfd/sg_2n_fsm.cc
> @@ -745,21 +745,21 @@ SaAisErrorT SG_2N::si_swap(AVD_SI *si, S
>   TRACE_ENTER2("'%s' sg_fsm_state=%u", si->name.c_str(), 
> si->sg_of_si->sg_fsm_state);
>
>   if (si->saAmfSIAdminState != SA_AMF_ADMIN_UNLOCKED) {
> - LOG_ER("%s SWAP failed - wrong admin state=%u", 
> si->name.c_str(),
> + LOG_NO("%s SWAP failed - wrong admin state=%u", 
> si->name.c_str(),
>   si->saAmfSIAdminState);
>   rc = SA_AIS_ERR_TRY_AGAIN;
>   goto done;
[Praveen] I think AMFD should not return TRY_AGAIN in this case. Since 
SI is locked, si_swap will not be successful in any no. of tries. I 
think there is already some ticket for this.
>   }
>
>   if (avd_cb->init_state != AVD_APP_STATE) {
> - LOG_ER("%s SWAP failed - not in app state (%u)", 
> si->name.c_str(),
> + LOG_NO("%s SWAP failed - not in app state (%u)", 
> si->name.c_str(),
>   avd_cb->init_state);
>   rc = SA_AIS_ERR_TRY_AGAIN;
>   goto done;
>   }
>
>   if (si->sg_of_si->sg_fsm_state != AVD_SG_FSM_STABLE) {
> - LOG_ER("%s SWAP failed - SG not stable (%u)", si->name.c_str(),
> + LOG_NO("%s SWAP failed - SG not stable (%u)", si->name.c_str(),
>   si->sg_of_si->sg_fsm_state);
>   rc = SA_AIS_ERR_TRY_AGAIN;
>   goto done;
> @@ -774,14 +774,14 @@ SaAisErrorT SG_2N::si_swap(AVD_SI *si, S
>   if ((si->sg_of_si->sg_ncs_spec) &&
>   ((avd_cb->node_id_avd_other != 0) && 
> (avd_cb->other_avd_adest != 0))) {
>   if (avd_cb->stby_sync_state == AVD_STBY_OUT_OF_SYNC) {
> - LOG_ER("%s SWAP failed - Cold sync in progress", 
> si->name.c_str());
> + LOG_NO("%s SWAP failed - Cold sync in progress", 
> si->name.c_str());
>   rc = SA_AIS_ERR_TRY_AGAIN;
>   goto done;
>   }
>   }
>
>   if (si->list_of_sisu->si_next == nullptr) {
> - LOG_ER("%s SWAP failed - only one assignment", 
> si->name.c_str());
> + LOG_NO("%s SWAP failed - only one assignment", 
> si->name.c_str());
>   rc = SA_AIS_ERR_TRY_AGAIN;
>   goto done;
>   }
>

--
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

Re: [devel] [PATCH 1 of 1] amfd: push notification in existing job queue for retries [#314]

[Gary Lee]

Hi again

> On 14 Oct. 2016, at 2:06 pm, Gary Lee  wrote:
> 
> Hi Praveen
> 
> Great idea, some minor comments:
> 
> - It seems a bit messy to add another parameter [ntfHandle] to 
> Job::exec(SaImmOiHandleT immOiHandle). 
> Can we just change the declaration to Job::exec(const AVD_CL_CB *cb)? And 
> ntfHandle wasn’t used anyway.
> Can we can move Job into its own header file?
> 
> - Can we remove NtfSend::already_sent?

Please ignore the above, I see the reason for it now.

> 
> - I think we should just queue up new notifications, instead of trying to 
> send it once. Otherwise, it might be
> possible to get the order of notifications messed up? This would be in line 
> with how IMM updates are done.
> 
> I’ve attached a modified version of your patch to help illustrate the 
> comments.
> 
> 
> 
> Thanks
> Gary
> 
>> On 13 Oct. 2016, at 9:09 pm, praveen.malv...@oracle.com wrote:
>> 
>> osaf/services/saf/amf/amfd/imm.cc|   28 ++--
>> osaf/services/saf/amf/amfd/include/imm.h |   14 +-
>> osaf/services/saf/amf/amfd/include/ntf.h |9 +-
>> osaf/services/saf/amf/amfd/main.cc   |6 +-
>> osaf/services/saf/amf/amfd/ntf.cc|  182 
>> ++
>> osaf/services/saf/amf/amfd/sgproc.cc |2 +-
>> 6 files changed, 164 insertions(+), 77 deletions(-)
>> 
>> 
>> Currently if AMFD gets TRY_AGAIN or TIMEOUT it logs notification and frees 
>> it.
>> 
>> With this patch, AMFD will give one try. AMFD will push the notification in 
>> the existing
>> job queue if hits with TRY_AGAIN or TIMEOUT in this try. Job queue already 
>> handles
>> TRY_AGAIN and TIMEOUT.
>> 
>> diff --git a/osaf/services/saf/amf/amfd/imm.cc 
>> b/osaf/services/saf/amf/amfd/imm.cc
>> --- a/osaf/services/saf/amf/amfd/imm.cc
>> +++ b/osaf/services/saf/amf/amfd/imm.cc
>> @@ -134,7 +134,7 @@ Job::~Job()
>> }
>> 
>> //
>> -AvdJobDequeueResultT ImmObjCreate::exec(SaImmOiHandleT immOiHandle)
>> +AvdJobDequeueResultT ImmObjCreate::exec(SaImmOiHandleT immOiHandle, 
>> SaNtfHandleT ntfHandle)
>> {
>>  SaAisErrorT rc;
>>  AvdJobDequeueResultT res;
>> @@ -200,7 +200,7 @@ ImmObjCreate::~ImmObjCreate()
>> }
>> 
>> //
>> -AvdJobDequeueResultT ImmObjUpdate::exec(SaImmOiHandleT immOiHandle)
>> +AvdJobDequeueResultT ImmObjUpdate::exec(SaImmOiHandleT immOiHandle, 
>> SaNtfHandleT ntfHandle)
>> {
>>  SaAisErrorT rc;
>>  AvdJobDequeueResultT res;
>> @@ -253,7 +253,7 @@ ImmObjUpdate::~ImmObjUpdate()
>> }
>> 
>> //
>> -AvdJobDequeueResultT ImmObjDelete::exec(SaImmOiHandleT immOiHandle)
>> +AvdJobDequeueResultT ImmObjDelete::exec(SaImmOiHandleT immOiHandle, 
>> SaNtfHandleT ntfHandle)
>> {
>>  SaAisErrorT rc;
>>  AvdJobDequeueResultT res;
>> @@ -290,7 +290,7 @@ ImmObjDelete::~ImmObjDelete()
>> }
>> 
>> //
>> -AvdJobDequeueResultT ImmAdminResponse::exec(const SaImmOiHandleT handle) {
>> +AvdJobDequeueResultT ImmAdminResponse::exec(SaImmOiHandleT handle, 
>> SaNtfHandleT ntfHandle) {
>>  SaAisErrorT rc;
>>  AvdJobDequeueResultT res;
>> 
>> @@ -331,10 +331,10 @@ Job* Fifo::peek()
>> {
>>  Job* tmp;
>>  
>> -if (imm_job_.empty()) {
>> +if (imm_ntf_job_.empty()) {
>>  tmp = 0;
>>  } else {
>> -tmp = imm_job_.front();
>> +tmp = imm_ntf_job_.front();
>>  }
>>  
>>  return tmp;
>> @@ -343,7 +343,7 @@ Job* Fifo::peek()
>> //
>> void Fifo::queue(Job* job)
>> {
>> -imm_job_.push(job);
>> +imm_ntf_job_.push(job);
>> }
>> 
>> //
>> @@ -351,11 +351,11 @@ Job* Fifo::dequeue()
>> {
>>  Job* tmp;
>>  
>> -if (imm_job_.empty()) {
>> +if (imm_ntf_job_.empty()) {
>>  tmp = 0;
>>  } else {
>> -tmp = imm_job_.front();
>> -imm_job_.pop();
>> +tmp = imm_ntf_job_.front();
>> +imm_ntf_job_.pop();
>>  }
>>  
>>  return tmp;
>> @@ -376,7 +376,7 @@ void check_and_flush_job_queue_standby_a
>> }
>> 
>> //
>> -AvdJobDequeueResultT Fifo::execute(SaImmOiHandleT immOiHandle)
>> +AvdJobDequeueResultT Fifo::execute(SaImmOiHandleT immOiHandle, SaNtfHandleT 
>> ntfHandle)
>> {
>>  Job *ajob;
>>  AvdJobDequeueResultT ret;
>> @@ -395,7 +395,7 @@ AvdJobDequeueResultT Fifo::execute(SaImm
>> 
>>  TRACE_ENTER();
>> 
>> -ret = ajob->exec(immOiHandle);
>> +ret = ajob->exec(immOiHandle, ntfHandle);
>>  
>>  TRACE_LEAVE2("%d", ret);
>> 
>> @@ -418,11 +418,11 @@ void Fifo::empty()
>> 
>> uint32_t Fifo::size()
>> {
>> -   return imm_job_.size();
>> +   return imm_ntf_job_.size();
>> }
>> 
>> //
>> -std::queue Fifo::imm_job_;
>> +std::queue Fifo::imm_ntf_job_;
>> //
>> 
>> extern struct ImmutilWrapperProfile immutilWrapperProfile;
>> diff --git a/osaf/services/saf/amf/amfd/include/imm.h 
>> b/osaf/services/saf/amf/amfd/include/imm.h
>> --- a/osaf/services/saf/amf/amfd/include/imm.h
>> +++ b/osaf/services/saf/amf/amfd/include/imm.h
>> @@ -52,7 +52,7 @@ typedef enum {
>> // TODO HANO Write comments
>> class 

Re: [devel] [PATCH 1 of 1] amfd: push notification in existing job queue for retries [#314]

[Gary Lee]

Hi Praveen

Great idea, some minor comments:

- It seems a bit messy to add another parameter [ntfHandle] to 
Job::exec(SaImmOiHandleT immOiHandle). 
Can we just change the declaration to Job::exec(const AVD_CL_CB *cb)? And 
ntfHandle wasn’t used anyway.
Can we can move Job into its own header file?

- Can we remove NtfSend::already_sent?

- I think we should just queue up new notifications, instead of trying to send 
it once. Otherwise, it might be
possible to get the order of notifications messed up? This would be in line 
with how IMM updates are done.

I’ve attached a modified version of your patch to help illustrate the comments.



Thanks
Gary

> On 13 Oct. 2016, at 9:09 pm, praveen.malv...@oracle.com wrote:
> 
> osaf/services/saf/amf/amfd/imm.cc|   28 ++--
> osaf/services/saf/amf/amfd/include/imm.h |   14 +-
> osaf/services/saf/amf/amfd/include/ntf.h |9 +-
> osaf/services/saf/amf/amfd/main.cc   |6 +-
> osaf/services/saf/amf/amfd/ntf.cc|  182 ++
> osaf/services/saf/amf/amfd/sgproc.cc |2 +-
> 6 files changed, 164 insertions(+), 77 deletions(-)
> 
> 
> Currently if AMFD gets TRY_AGAIN or TIMEOUT it logs notification and frees it.
> 
> With this patch, AMFD will give one try. AMFD will push the notification in 
> the existing
> job queue if hits with TRY_AGAIN or TIMEOUT in this try. Job queue already 
> handles
> TRY_AGAIN and TIMEOUT.
> 
> diff --git a/osaf/services/saf/amf/amfd/imm.cc 
> b/osaf/services/saf/amf/amfd/imm.cc
> --- a/osaf/services/saf/amf/amfd/imm.cc
> +++ b/osaf/services/saf/amf/amfd/imm.cc
> @@ -134,7 +134,7 @@ Job::~Job()
> }
> 
> //
> -AvdJobDequeueResultT ImmObjCreate::exec(SaImmOiHandleT immOiHandle)
> +AvdJobDequeueResultT ImmObjCreate::exec(SaImmOiHandleT immOiHandle, 
> SaNtfHandleT ntfHandle)
> {
>   SaAisErrorT rc;
>   AvdJobDequeueResultT res;
> @@ -200,7 +200,7 @@ ImmObjCreate::~ImmObjCreate()
> }
> 
> //
> -AvdJobDequeueResultT ImmObjUpdate::exec(SaImmOiHandleT immOiHandle)
> +AvdJobDequeueResultT ImmObjUpdate::exec(SaImmOiHandleT immOiHandle, 
> SaNtfHandleT ntfHandle)
> {
>   SaAisErrorT rc;
>   AvdJobDequeueResultT res;
> @@ -253,7 +253,7 @@ ImmObjUpdate::~ImmObjUpdate()
> }
> 
> //
> -AvdJobDequeueResultT ImmObjDelete::exec(SaImmOiHandleT immOiHandle)
> +AvdJobDequeueResultT ImmObjDelete::exec(SaImmOiHandleT immOiHandle, 
> SaNtfHandleT ntfHandle)
> {
>   SaAisErrorT rc;
>   AvdJobDequeueResultT res;
> @@ -290,7 +290,7 @@ ImmObjDelete::~ImmObjDelete()
> }
> 
> //
> -AvdJobDequeueResultT ImmAdminResponse::exec(const SaImmOiHandleT handle) {
> +AvdJobDequeueResultT ImmAdminResponse::exec(SaImmOiHandleT handle, 
> SaNtfHandleT ntfHandle) {
>   SaAisErrorT rc;
>   AvdJobDequeueResultT res;
> 
> @@ -331,10 +331,10 @@ Job* Fifo::peek()
> {
>   Job* tmp;
>   
> - if (imm_job_.empty()) {
> + if (imm_ntf_job_.empty()) {
>   tmp = 0;
>   } else {
> - tmp = imm_job_.front();
> + tmp = imm_ntf_job_.front();
>   }
>   
>   return tmp;
> @@ -343,7 +343,7 @@ Job* Fifo::peek()
> //
> void Fifo::queue(Job* job)
> {
> - imm_job_.push(job);
> + imm_ntf_job_.push(job);
> }
> 
> //
> @@ -351,11 +351,11 @@ Job* Fifo::dequeue()
> {
>   Job* tmp;
>   
> - if (imm_job_.empty()) {
> + if (imm_ntf_job_.empty()) {
>   tmp = 0;
>   } else {
> - tmp = imm_job_.front();
> - imm_job_.pop();
> + tmp = imm_ntf_job_.front();
> + imm_ntf_job_.pop();
>   }
>   
>   return tmp;
> @@ -376,7 +376,7 @@ void check_and_flush_job_queue_standby_a
> }
> 
> //
> -AvdJobDequeueResultT Fifo::execute(SaImmOiHandleT immOiHandle)
> +AvdJobDequeueResultT Fifo::execute(SaImmOiHandleT immOiHandle, SaNtfHandleT 
> ntfHandle)
> {
>   Job *ajob;
>   AvdJobDequeueResultT ret;
> @@ -395,7 +395,7 @@ AvdJobDequeueResultT Fifo::execute(SaImm
> 
>   TRACE_ENTER();
> 
> - ret = ajob->exec(immOiHandle);
> + ret = ajob->exec(immOiHandle, ntfHandle);
>   
>   TRACE_LEAVE2("%d", ret);
> 
> @@ -418,11 +418,11 @@ void Fifo::empty()
> 
> uint32_t Fifo::size()
> {
> -   return imm_job_.size();
> +   return imm_ntf_job_.size();
> }
> 
> //
> -std::queue Fifo::imm_job_;
> +std::queue Fifo::imm_ntf_job_;
> //
> 
> extern struct ImmutilWrapperProfile immutilWrapperProfile;
> diff --git a/osaf/services/saf/amf/amfd/include/imm.h 
> b/osaf/services/saf/amf/amfd/include/imm.h
> --- a/osaf/services/saf/amf/amfd/include/imm.h
> +++ b/osaf/services/saf/amf/amfd/include/imm.h
> @@ -52,7 +52,7 @@ typedef enum {
> // TODO HANO Write comments
> class Job {
> public:
> - virtual AvdJobDequeueResultT exec(SaImmOiHandleT immOiHandle) = 0;
> + virtual AvdJobDequeueResultT exec(SaImmOiHandleT immOiHandle, 
> SaNtfHandleT ntfHandle) = 0;
>   virtual ~Job() = 0;
> };
> 
> @@ -63,7 +63,7 @@ public:
>   std::string parentName_;
>   

Re: [devel] [PATCH 0 of 1] Review Request for base: Add classes for UNIX socket operations and log message formatting [#2064]

[ramesh betham]

HI Anders,

Will try to provide my review comments next week.

Regards,
Ramesh.

On 10/13/2016 9:22 PM, Anders Widell wrote:
> Hi Ramesh!
>
> Did you get a chance to look at this yet?
>
> regards,
>
> Anders Widell
>
>
> On 10/10/2016 04:54 PM, Anders Widell wrote:
>> Summary: base: Add classes for UNIX socket operations and log message 
>> formatting [#2064]
>> Review request for Trac Ticket(s): 2064
>> Peer Reviewer(s): Ramesh
>> Pull request to:
>> Affected branch(es): default(5.2)
>> Development branch: default
>>
>> 
>> Impacted area   Impact y/n
>> 
>>   Docsn
>>   Build systemn
>>   RPM/packaging   n
>>   Configuration files n
>>   Startup scripts n
>>   SAF servicesn
>>   OpenSAF servicesn
>>   Core libraries  y
>>   Samples n
>>   Tests   n
>>   Other   n
>>
>>
>> Comments (indicate scope for each "y" above):
>> -
>> v2:
>>
>> * Fix bugs in time stamp formatting (wrong seconds, missing validation
>>of input parameters)
>> * Added a few more test cases for time stamp formatting
>>
>> changeset ea286178e02720d7d8859d4eabfbed785f48321a
>> Author:Anders Widell 
>> Date:Mon, 10 Oct 2016 16:50:15 +0200
>>
>> base: Add classes for UNIX socket operations and log message 
>> formatting
>> [#2064]
>>
>> Add support classes for using UNIX sockets, and for formatting 
>> log messages
>> according to rfc5424.
>>
>>
>> Complete diffstat:
>> --
>>   osaf/libs/core/cplusplus/base/Makefile.am   |   13 
>> -
>>   osaf/libs/core/cplusplus/base/buffer.h  |  100 
>> +++ 
>>
>>   osaf/libs/core/cplusplus/base/log_message.cc|  130 
>> ++
>>  
>>
>>   osaf/libs/core/cplusplus/base/log_message.h |  186 
>> +
>>  
>>
>>   osaf/libs/core/cplusplus/base/tests/Makefile.am |   12 
>> +---
>>   osaf/libs/core/cplusplus/base/tests/log_message_test.cc |  150 
>> +
>>  
>>
>>   osaf/libs/core/cplusplus/base/unix_client_socket.cc |   52 
>> +++
>>   osaf/libs/core/cplusplus/base/unix_client_socket.h  |   41 
>> +++
>>   osaf/libs/core/cplusplus/base/unix_server_socket.cc |   51 
>> ++
>>   osaf/libs/core/cplusplus/base/unix_server_socket.h  |   42 
>> +++
>>   osaf/libs/core/cplusplus/base/unix_socket.cc|   58 
>> +++
>>   osaf/libs/core/cplusplus/base/unix_socket.h |   89 
>> +++
>>   12 files changed, 917 insertions(+), 7 deletions(-)
>>
>>
>> Testing Commands:
>> -
>> make check
>>
>>
>> Testing, Expected Results:
>> --
>> All unit tests should pass
>>
>>
>> Conditions of Submission:
>> -
>> Ack from reviewer(s)
>>
>>
>> Arch  Built StartedLinux distro
>> ---
>> mipsn  n
>> mips64  n  n
>> x86 n  n
>> x86_64  y  y
>> powerpc n  n
>> powerpc64   n  n
>>
>>
>> Reviewer Checklist:
>> ---
>> [Submitters: make sure that your review doesn't trigger any checkmarks!]
>>
>>
>> Your checkin has not passed review because (see checked entries):
>>
>> ___ Your RR template is generally incomplete; it has too many blank 
>> entries
>>  that need proper data filled in.
>>
>> ___ You have failed to nominate the proper persons for review and push.
>>
>> ___ Your patches do not have proper short+long header
>>
>> ___ You have grammar/spelling in your header that is unacceptable.
>>
>> ___ You have exceeded a sensible line length in your 
>> headers/comments/text.
>>
>> ___ You have failed to put in a proper Trac Ticket # into your commits.
>>
>> ___ You have incorrectly put/left internal data in your comments/files
>>  (i.e. internal bug tracking tool IDs, product names etc)
>>
>> ___ You have not given any evidence of testing beyond basic build tests.
>>  Demonstrate some level of runtime or other sanity testing.
>>
>> ___ You have ^M present in some of your files. These have to be removed.
>>
>> ___ You have needlessly changed whitespace or added whitespace crimes
>>  

[devel] [PATCH 1 of 1] build: Make it possible to perform a full OpenSAF installation from RPMs [#2102]

[Anders Widell]

 opensaf.spec.in |  1 +
 1 files changed, 1 insertions(+), 0 deletions(-)


Add the nodeinit.conf.payload file to the controller RPM, so that a node where
this RPM is installed can be configured as either a controller or a payload.

diff --git a/opensaf.spec.in b/opensaf.spec.in
--- a/opensaf.spec.in
+++ b/opensaf.spec.in
@@ -1378,6 +1378,7 @@ fi
 %config(noreplace) %{_pkgsysconfdir}/fmd.conf
 %config(noreplace) %{_pkgsysconfdir}/rde.conf
 %config(noreplace) %{_pkgsysconfdir}/nodeinit.conf.controller
+%config(noreplace) %{_pkgsysconfdir}/nodeinit.conf.payload
 %config(noreplace) %{_pkgsysconfdir}/dtmd.conf
 %{_pkglibdir}/osafrded
 %{_pkgclcclidir}/osaf-rded

--
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

Re: [devel] [PATCH 0 of 1] Review Request for base: Add classes for UNIX socket operations and log message formatting [#2064]

[Anders Widell]

Hi Ramesh!

Did you get a chance to look at this yet?

regards,

Anders Widell


On 10/10/2016 04:54 PM, Anders Widell wrote:
> Summary: base: Add classes for UNIX socket operations and log message 
> formatting [#2064]
> Review request for Trac Ticket(s): 2064
> Peer Reviewer(s): Ramesh
> Pull request to:
> Affected branch(es): default(5.2)
> Development branch: default
>
> 
> Impacted area   Impact y/n
> 
>   Docsn
>   Build systemn
>   RPM/packaging   n
>   Configuration files n
>   Startup scripts n
>   SAF servicesn
>   OpenSAF servicesn
>   Core libraries  y
>   Samples n
>   Tests   n
>   Other   n
>
>
> Comments (indicate scope for each "y" above):
> -
> v2:
>
> * Fix bugs in time stamp formatting (wrong seconds, missing validation
>of input parameters)
> * Added a few more test cases for time stamp formatting
>
> changeset ea286178e02720d7d8859d4eabfbed785f48321a
> Author:   Anders Widell 
> Date: Mon, 10 Oct 2016 16:50:15 +0200
>
>   base: Add classes for UNIX socket operations and log message formatting
>   [#2064]
>
>   Add support classes for using UNIX sockets, and for formatting log 
> messages
>   according to rfc5424.
>
>
> Complete diffstat:
> --
>   osaf/libs/core/cplusplus/base/Makefile.am   |   13 -
>   osaf/libs/core/cplusplus/base/buffer.h  |  100 
> +++
>   osaf/libs/core/cplusplus/base/log_message.cc|  130 
> ++
>   osaf/libs/core/cplusplus/base/log_message.h |  186 
> +
>   osaf/libs/core/cplusplus/base/tests/Makefile.am |   12 +---
>   osaf/libs/core/cplusplus/base/tests/log_message_test.cc |  150 
> +
>   osaf/libs/core/cplusplus/base/unix_client_socket.cc |   52 
> +++
>   osaf/libs/core/cplusplus/base/unix_client_socket.h  |   41 
> +++
>   osaf/libs/core/cplusplus/base/unix_server_socket.cc |   51 
> ++
>   osaf/libs/core/cplusplus/base/unix_server_socket.h  |   42 
> +++
>   osaf/libs/core/cplusplus/base/unix_socket.cc|   58 
> +++
>   osaf/libs/core/cplusplus/base/unix_socket.h |   89 
> +++
>   12 files changed, 917 insertions(+), 7 deletions(-)
>
>
> Testing Commands:
> -
> make check
>
>
> Testing, Expected Results:
> --
> All unit tests should pass
>
>
> Conditions of Submission:
> -
> Ack from reviewer(s)
>
>
> Arch  Built StartedLinux distro
> ---
> mipsn  n
> mips64  n  n
> x86 n  n
> x86_64  y  y
> powerpc n  n
> powerpc64   n  n
>
>
> Reviewer Checklist:
> ---
> [Submitters: make sure that your review doesn't trigger any checkmarks!]
>
>
> Your checkin has not passed review because (see checked entries):
>
> ___ Your RR template is generally incomplete; it has too many blank entries
>  that need proper data filled in.
>
> ___ You have failed to nominate the proper persons for review and push.
>
> ___ Your patches do not have proper short+long header
>
> ___ You have grammar/spelling in your header that is unacceptable.
>
> ___ You have exceeded a sensible line length in your headers/comments/text.
>
> ___ You have failed to put in a proper Trac Ticket # into your commits.
>
> ___ You have incorrectly put/left internal data in your comments/files
>  (i.e. internal bug tracking tool IDs, product names etc)
>
> ___ You have not given any evidence of testing beyond basic build tests.
>  Demonstrate some level of runtime or other sanity testing.
>
> ___ You have ^M present in some of your files. These have to be removed.
>
> ___ You have needlessly changed whitespace or added whitespace crimes
>  like trailing spaces, or spaces before tabs.
>
> ___ You have mixed real technical changes with whitespace and other
>  cosmetic code cleanup changes. These have to be separate commits.
>
> ___ You need to refactor your submission into logical chunks; there is
>  too much content into a single 

[devel] [PATCH 0 of 1] Review Request for amfd: push notification in existing job queue for retries [#314]

[praveen . malviya]

Summary: amfd: push notification in existing job queue for retries [#314] 
Review request for Trac Ticket(s): #314 
Peer Reviewer(s): AMF devs 
Pull request to: <>
Affected branch(es): ALL 
Development branch: <>


Impacted area   Impact y/n

 Docsn
 Build systemn
 RPM/packaging   n
 Configuration files n
 Startup scripts n
 SAF servicesy
 OpenSAF servicesn
 Core libraries  n
 Samples n
 Tests   n
 Other   n


Comments (indicate scope for each "y" above):
-

changeset eb3bef9c5bff38d4ccbfc8f22ee664fb2e1b6311
Author: Praveen Malviya 
Date:   Thu, 13 Oct 2016 15:36:45 +0530

amfd: push notification in existing job queue for retries [#314]

Currently if AMFD gets TRY_AGAIN or TIMEOUT it logs notification and 
frees
it.

With this patch, AMFD will give one try. AMFD will push the 
notification in
the existing job queue if hits with TRY_AGAIN or TIMEOUT in this try. 
Job
queue already handles TRY_AGAIN and TIMEOUT.


Complete diffstat:
--
 osaf/services/saf/amf/amfd/imm.cc|   28 +-
 osaf/services/saf/amf/amfd/include/imm.h |   14 ++--
 osaf/services/saf/amf/amfd/include/ntf.h |9 ++-
 osaf/services/saf/amf/amfd/main.cc   |6 +-
 osaf/services/saf/amf/amfd/ntf.cc|  182 
++--
 osaf/services/saf/amf/amfd/sgproc.cc |2 +-
 6 files changed, 164 insertions(+), 77 deletions(-)


Testing Commands:
-
Not tested the test given in the description.
Brought up one controller.
Brougt up AMF Demo.

Testing, Expected Results:
--
Got notifications.

Conditions of Submission:
-
Ack from Gary and other reviewers.

Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  y  y
powerpc n  n
powerpc64   n  n


Reviewer Checklist:
---
[Submitters: make sure that your review doesn't trigger any checkmarks!]


Your checkin has not passed review because (see checked entries):

___ Your RR template is generally incomplete; it has too many blank entries
that need proper data filled in.

___ You have failed to nominate the proper persons for review and push.

___ Your patches do not have proper short+long header

___ You have grammar/spelling in your header that is unacceptable.

___ You have exceeded a sensible line length in your headers/comments/text.

___ You have failed to put in a proper Trac Ticket # into your commits.

___ You have incorrectly put/left internal data in your comments/files
(i.e. internal bug tracking tool IDs, product names etc)

___ You have not given any evidence of testing beyond basic build tests.
Demonstrate some level of runtime or other sanity testing.

___ You have ^M present in some of your files. These have to be removed.

___ You have needlessly changed whitespace or added whitespace crimes
like trailing spaces, or spaces before tabs.

___ You have mixed real technical changes with whitespace and other
cosmetic code cleanup changes. These have to be separate commits.

___ You need to refactor your submission into logical chunks; there is
too much content into a single commit.

___ You have extraneous garbage in your review (merge commits etc)

___ You have giant attachments which should never have been sent;
Instead you should place your content in a public tree to be pulled.

___ You have too many commits attached to an e-mail; resend as threaded
commits, or place in a public tree for a pull.

___ You have resent this content multiple times without a clear indication
of what has changed between each re-send.

___ You have failed to adequately and individually address all of the
comments and change requests that were proposed in the initial review.

___ You have a misconfigured ~/.hgrc file (i.e. username, email etc)

___ Your computer have a badly configured date and time; confusing the
the threaded patch review.

___ Your changes affect IPC mechanism, and you don't present any results
for in-service upgradability test.

___ Your changes affect user manual and documentation, your patch series
do not contain the patch that updates the Doxygen manual.


--
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

[devel] [PATCH 1 of 1] amfd: push notification in existing job queue for retries [#314]

[praveen . malviya]

 osaf/services/saf/amf/amfd/imm.cc|   28 ++--
 osaf/services/saf/amf/amfd/include/imm.h |   14 +-
 osaf/services/saf/amf/amfd/include/ntf.h |9 +-
 osaf/services/saf/amf/amfd/main.cc   |6 +-
 osaf/services/saf/amf/amfd/ntf.cc|  182 ++
 osaf/services/saf/amf/amfd/sgproc.cc |2 +-
 6 files changed, 164 insertions(+), 77 deletions(-)


Currently if AMFD gets TRY_AGAIN or TIMEOUT it logs notification and frees it.

With this patch, AMFD will give one try. AMFD will push the notification in the 
existing
job queue if hits with TRY_AGAIN or TIMEOUT in this try. Job queue already 
handles
TRY_AGAIN and TIMEOUT.

diff --git a/osaf/services/saf/amf/amfd/imm.cc 
b/osaf/services/saf/amf/amfd/imm.cc
--- a/osaf/services/saf/amf/amfd/imm.cc
+++ b/osaf/services/saf/amf/amfd/imm.cc
@@ -134,7 +134,7 @@ Job::~Job()
 }
 
 //
-AvdJobDequeueResultT ImmObjCreate::exec(SaImmOiHandleT immOiHandle)
+AvdJobDequeueResultT ImmObjCreate::exec(SaImmOiHandleT immOiHandle, 
SaNtfHandleT ntfHandle)
 {
SaAisErrorT rc;
AvdJobDequeueResultT res;
@@ -200,7 +200,7 @@ ImmObjCreate::~ImmObjCreate()
 }
 
 //
-AvdJobDequeueResultT ImmObjUpdate::exec(SaImmOiHandleT immOiHandle)
+AvdJobDequeueResultT ImmObjUpdate::exec(SaImmOiHandleT immOiHandle, 
SaNtfHandleT ntfHandle)
 {
SaAisErrorT rc;
AvdJobDequeueResultT res;
@@ -253,7 +253,7 @@ ImmObjUpdate::~ImmObjUpdate()
 }
 
 //
-AvdJobDequeueResultT ImmObjDelete::exec(SaImmOiHandleT immOiHandle)
+AvdJobDequeueResultT ImmObjDelete::exec(SaImmOiHandleT immOiHandle, 
SaNtfHandleT ntfHandle)
 {
SaAisErrorT rc;
AvdJobDequeueResultT res;
@@ -290,7 +290,7 @@ ImmObjDelete::~ImmObjDelete()
 }
 
 //
-AvdJobDequeueResultT ImmAdminResponse::exec(const SaImmOiHandleT handle) {
+AvdJobDequeueResultT ImmAdminResponse::exec(SaImmOiHandleT handle, 
SaNtfHandleT ntfHandle) {
SaAisErrorT rc;
AvdJobDequeueResultT res;
 
@@ -331,10 +331,10 @@ Job* Fifo::peek()
 {
Job* tmp;

-   if (imm_job_.empty()) {
+   if (imm_ntf_job_.empty()) {
tmp = 0;
} else {
-   tmp = imm_job_.front();
+   tmp = imm_ntf_job_.front();
}

return tmp;
@@ -343,7 +343,7 @@ Job* Fifo::peek()
 //
 void Fifo::queue(Job* job)
 {
-   imm_job_.push(job);
+   imm_ntf_job_.push(job);
 }
 
 //
@@ -351,11 +351,11 @@ Job* Fifo::dequeue()
 {
Job* tmp;

-   if (imm_job_.empty()) {
+   if (imm_ntf_job_.empty()) {
tmp = 0;
} else {
-   tmp = imm_job_.front();
-   imm_job_.pop();
+   tmp = imm_ntf_job_.front();
+   imm_ntf_job_.pop();
}

return tmp;
@@ -376,7 +376,7 @@ void check_and_flush_job_queue_standby_a
 }
 
 //
-AvdJobDequeueResultT Fifo::execute(SaImmOiHandleT immOiHandle)
+AvdJobDequeueResultT Fifo::execute(SaImmOiHandleT immOiHandle, SaNtfHandleT 
ntfHandle)
 {
Job *ajob;
AvdJobDequeueResultT ret;
@@ -395,7 +395,7 @@ AvdJobDequeueResultT Fifo::execute(SaImm
 
TRACE_ENTER();
 
-   ret = ajob->exec(immOiHandle);
+   ret = ajob->exec(immOiHandle, ntfHandle);

TRACE_LEAVE2("%d", ret);
 
@@ -418,11 +418,11 @@ void Fifo::empty()
 
 uint32_t Fifo::size()
 {
-   return imm_job_.size();
+   return imm_ntf_job_.size();
 }
 
 //
-std::queue Fifo::imm_job_;
+std::queue Fifo::imm_ntf_job_;
 //
 
 extern struct ImmutilWrapperProfile immutilWrapperProfile;
diff --git a/osaf/services/saf/amf/amfd/include/imm.h 
b/osaf/services/saf/amf/amfd/include/imm.h
--- a/osaf/services/saf/amf/amfd/include/imm.h
+++ b/osaf/services/saf/amf/amfd/include/imm.h
@@ -52,7 +52,7 @@ typedef enum {
 // TODO HANO Write comments
 class Job {
 public:
-   virtual AvdJobDequeueResultT exec(SaImmOiHandleT immOiHandle) = 0;
+   virtual AvdJobDequeueResultT exec(SaImmOiHandleT immOiHandle, 
SaNtfHandleT ntfHandle) = 0;
virtual ~Job() = 0;
 };
 
@@ -63,7 +63,7 @@ public:
std::string parentName_;
const SaImmAttrValuesT_2 **attrValues_;

-   AvdJobDequeueResultT exec(SaImmOiHandleT immOiHandle);
+   AvdJobDequeueResultT exec(SaImmOiHandleT immOiHandle, SaNtfHandleT 
ntfHandle);

~ImmObjCreate();
 };
@@ -76,7 +76,7 @@ public:
SaImmValueTypeT attrValueType_;
void *value_;

-   AvdJobDequeueResultT exec(SaImmOiHandleT immOiHandle);
+   AvdJobDequeueResultT exec(SaImmOiHandleT immOiHandle, SaNtfHandleT 
ntfHandle);

~ImmObjUpdate();
 };
@@ -86,7 +86,7 @@ class ImmObjDelete : public Job {
 public:
std::string dn;

-   AvdJobDequeueResultT exec(SaImmOiHandleT immOiHandle);
+   AvdJobDequeueResultT exec(SaImmOiHandleT immOiHandle, SaNtfHandleT 
ntfHandle);

~ImmObjDelete();
 };
@@ -98,7 +98,7 @@ class ImmAdminResponse : public Job {

Re: [devel] Review request CLM Programmers Guide [#2053]

[Mathivanan Naickan Palanivelu]

Ack,

If appropriate, We could add a "Note: This admin operation on the cluster 
object is an OpenSAF extension".

Thanks,
Mathi. 

> -Original Message-
> From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
> Sent: Thursday, October 13, 2016 12:55 PM
> To: Mathivanan Naickan Palanivelu
> Cc: opensaf-devel@lists.sourceforge.net; Hans Nordebäck
> Subject: Review request CLM Programmers Guide [#2053]
> 
> Hi Mathi,
> 
> I updated the CLM Programmers Guide with the CLM cluster reboot support.
> 
> /Thanks HansN
> 

--
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

[devel] [PATCH 0 of 1] Review Request for cpd: to correct failover behavior of cpsv [#1765] V2

[Hoang Vo]

Summary: cpd: to correct failover behavior of cpsv [#1765]
Review request for Trac Ticket(s): 1765
Peer Reviewer(s): mahesh.va...@oracle.com; anders.wid...@ericsson.com
Pull request to: mahesh.va...@oracle.com
Affected branch(es): default
Development branch: default


Impacted area   Impact y/n

 Docsn
 Build systemn
 RPM/packaging   n
 Configuration files n
 Startup scripts n
 SAF servicesy
 OpenSAF servicesn
 Core libraries  n
 Samples n
 Tests   n
 Other   n


Comments (indicate scope for each "y" above):
-

changeset 4bf73f541377092c8efd79dc452ebba5db683bb9
Author: Hoang Vo 
Date:   Thu, 13 Oct 2016 15:26:46 +0700

cpd: to correct failover behavior of cpsv [#1765]

problem: In case a failover happens while a checkpoint is being 
unlinked, it
might causes an unfinished unlink operation (i.e the checkpoint IMM 
object
is not deleted). Later on, when the checkpoint is created again, it 
will not
succeed because the CPD detects that the checkpoint IMM object existing.

Fix:
- When error occur delete the existing checkpoint IMM object and 
re-create new
one.
- Stop timer of removed node.
- Update data in patricia trees.


Complete diffstat:
--
 osaf/services/saf/cpsv/cpd/cpd_db.c   |  15 +++
 osaf/services/saf/cpsv/cpd/cpd_proc.c |  18 --
 2 files changed, 31 insertions(+), 2 deletions(-)


Testing Commands:
-
Create checkpoint and set retention to big value
Failover by killing osafamfd multiple times
Check checkpoint information

Testing, Expected Results:
--
Checkpoint information is not change

Conditions of Submission:
-
ACK from maintainer

Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  y  y
powerpc n  n
powerpc64   n  n


Reviewer Checklist:
---
[Submitters: make sure that your review doesn't trigger any checkmarks!]


Your checkin has not passed review because (see checked entries):

___ Your RR template is generally incomplete; it has too many blank entries
that need proper data filled in.

___ You have failed to nominate the proper persons for review and push.

___ Your patches do not have proper short+long header

___ You have grammar/spelling in your header that is unacceptable.

___ You have exceeded a sensible line length in your headers/comments/text.

___ You have failed to put in a proper Trac Ticket # into your commits.

___ You have incorrectly put/left internal data in your comments/files
(i.e. internal bug tracking tool IDs, product names etc)

___ You have not given any evidence of testing beyond basic build tests.
Demonstrate some level of runtime or other sanity testing.

___ You have ^M present in some of your files. These have to be removed.

___ You have needlessly changed whitespace or added whitespace crimes
like trailing spaces, or spaces before tabs.

___ You have mixed real technical changes with whitespace and other
cosmetic code cleanup changes. These have to be separate commits.

___ You need to refactor your submission into logical chunks; there is
too much content into a single commit.

___ You have extraneous garbage in your review (merge commits etc)

___ You have giant attachments which should never have been sent;
Instead you should place your content in a public tree to be pulled.

___ You have too many commits attached to an e-mail; resend as threaded
commits, or place in a public tree for a pull.

___ You have resent this content multiple times without a clear indication
of what has changed between each re-send.

___ You have failed to adequately and individually address all of the
comments and change requests that were proposed in the initial review.

___ You have a misconfigured ~/.hgrc file (i.e. username, email etc)

___ Your computer have a badly configured date and time; confusing the
the threaded patch review.

___ Your changes affect IPC mechanism, and you don't present any results
for in-service upgradability test.

___ Your changes affect user manual and documentation, your patch series
do not contain the patch that updates the Doxygen manual.


--
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

[devel] [PATCH 1 of 1] cpd: to correct failover behavior of cpsv [#1765]

[Hoang Vo]

 osaf/services/saf/cpsv/cpd/cpd_db.c   |  15 +++
 osaf/services/saf/cpsv/cpd/cpd_proc.c |  18 --
 2 files changed, 31 insertions(+), 2 deletions(-)


problem:
In case a failover happens while a checkpoint is being unlinked, it might 
causes an unfinished
unlink operation (i.e the checkpoint IMM object is not deleted). Later on, when 
the checkpoint is
created again, it will not succeed because the CPD detects that the checkpoint 
IMM object existing.

Fix:
- When error occur delete the existing checkpoint IMM object and re-create new 
one.
- Stop timer of removed node.
- Update data in patricia trees.

diff --git a/osaf/services/saf/cpsv/cpd/cpd_db.c 
b/osaf/services/saf/cpsv/cpd/cpd_db.c
--- a/osaf/services/saf/cpsv/cpd/cpd_db.c
+++ b/osaf/services/saf/cpsv/cpd/cpd_db.c
@@ -104,6 +104,21 @@ uint32_t cpd_ckpt_node_add(NCS_PATRICIA_
/*create the imm runtime object */
if (ha_state == SA_AMF_HA_ACTIVE) {
err = create_runtime_ckpt_object(ckpt_node, immOiHandle);
+
+   /* The Checkpoint IMM object exist due to unfinished previous 
opernation (e.g unlink)
+* The action is to delete the old object and create a new one 
*/
+
+   if (err == SA_AIS_ERR_EXIST) {
+   LOG_WA("cpd ckpt node add - the IMM object exits %s", 
ckpt_node->ckpt_name);
+
+   if (delete_runtime_ckpt_object(ckpt_node, immOiHandle) 
!= SA_AIS_OK) {
+   LOG_ER("Deleting run time object %s FAILED", 
ckpt_node->ckpt_name);
+   return NCSCC_RC_FAILURE;
+   }
+
+   err = create_runtime_ckpt_object(ckpt_node, 
immOiHandle);
+   }
+
if (err != SA_AIS_OK) {
LOG_ER("create runtime ckpt object failed with error: 
%u",err);
if (err == SA_AIS_ERR_INVALID_PARAM) {
diff --git a/osaf/services/saf/cpsv/cpd/cpd_proc.c 
b/osaf/services/saf/cpsv/cpd/cpd_proc.c
--- a/osaf/services/saf/cpsv/cpd/cpd_proc.c
+++ b/osaf/services/saf/cpsv/cpd/cpd_proc.c
@@ -348,7 +348,8 @@ uint32_t cpd_ckpt_db_entry_update(CPD_CB
proc_rc = 
cpd_ckpt_reploc_node_add(>ckpt_reploc_tree, reploc_info, cb->ha_state, 
cb->immOiHandle);
if (proc_rc != NCSCC_RC_SUCCESS) {
/* goto reploc_node_add_fail; */
-   TRACE_4("cpd db add failed ");
+   LOG_ER("cpd db replica add failed ");
+   goto replica_node_add_fail;
}
}
 
@@ -367,6 +368,10 @@ uint32_t cpd_ckpt_db_entry_update(CPD_CB
TRACE_LEAVE();
return NCSCC_RC_SUCCESS;
 
+ replica_node_add_fail:
+   cpd_ckpt_node_delete(cb, ckpt_node);
+   ckpt_node = NULL;
+
  ckpt_node_add_fail:
cpd_ckpt_map_node_delete(cb, map_info);
map_info = NULL;
@@ -679,7 +684,8 @@ uint32_t cpd_process_cpnd_down(CPD_CB *c
cpd_cpnd_info_node_find_add(>cpnd_tree, cpnd_dest, _info, 
_flag);
if (!cpnd_info)
return NCSCC_RC_SUCCESS;
-
+   /* Stop timer before processing down */
+   cpd_tmr_stop(_info->cpnd_ret_timer);
cref_info = cpnd_info->ckpt_ref_list;
 
while (cref_info) {
@@ -984,6 +990,14 @@ uint32_t cpd_proc_retention_set(CPD_CB *
 
/* Update the retention Time */
(*ckpt_node)->ret_time = reten_time;
+   (*ckpt_node)->attributes.retentionDuration = reten_time;
+
+   /* Update the related patricia tree */
+   CPD_CKPT_MAP_INFO *map_info = NULL;
+   cpd_ckpt_map_node_get(>ckpt_map_tree, (*ckpt_node)->ckpt_name, 
_info);
+   if (map_info) {
+   map_info->attributes.retentionDuration = reten_time;
+   }
return rc;
 }
 

--
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

Re: [devel] [PATCH 1 of 1] log: fix the usage of command saflogger [#2073]

[A V Mahesh]

ACK

-AVM


On 10/3/2016 3:08 PM, Canh Van Truong wrote:
>   osaf/tools/saflog/saflogger/saf_logger.c |  6 +++---
>   1 files changed, 3 insertions(+), 3 deletions(-)
>
>
> The runtime app stream name must start with "safLgStr=", not "safLgStrCfg=".
> So, creating app stream name with "safLgStrCfg=" will result 
> "SA_AIS_ERR_INVALID_PARAM".
>
> diff --git a/osaf/tools/saflog/saflogger/saf_logger.c 
> b/osaf/tools/saflog/saflogger/saf_logger.c
> --- a/osaf/tools/saflog/saflogger/saf_logger.c
> +++ b/osaf/tools/saflog/saflogger/saf_logger.c
> @@ -101,12 +101,12 @@ static void usage(void)
>   printf("\t-s SEV or --severity=SEV   use severity SEV, default 
> INFO\n");
>   printf("\t\tvalid severity names: emerg, alert, crit, error, warn, 
> notice, info\n");
>   printf("\nNOTES\n");
> - printf("\t1) -f is only applicable for app stream.\n");
> + printf("\t1) -f is only applicable for runtime app stream.\n");
>   printf("\t1)  length must not be longer than 255 
> characters.\n");
>   
>   printf("\nEXAMPLES\n");
> - printf("\tsaflogger -a safLgStrCfg=Test \"Hello world\"\n");
> - printf("\tsaflogger -a safLgStrCfg=Test -f testLogFile \"Hello 
> world\"\n");
> + printf("\tsaflogger -a safLgStr=Test \"Hello world\"\n");
> + printf("\tsaflogger -a safLgStr=Test -f testLogFile \"Hello world\"\n");
>   printf("\tsaflogger -s crit \"I am going down\"\n\n");
>   }
>   


--
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

Re: [devel] [PATCH 1 of 1] log: fix the usage of command saflogger [#2073]

[Vu Minh Nguyen]

Ack.

Regards, Vu

> -Original Message-
> From: Canh Van Truong [mailto:canh.v.tru...@dektech.com.au]
> Sent: Monday, October 3, 2016 4:38 PM
> To: lennart.l...@ericsson.com; mahesh.va...@oracle.com;
> vu.m.ngu...@dektech.com.au
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: [PATCH 1 of 1] log: fix the usage of command saflogger [#2073]
> 
>  osaf/tools/saflog/saflogger/saf_logger.c |  6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> 
> The runtime app stream name must start with "safLgStr=", not
> "safLgStrCfg=".
> So, creating app stream name with "safLgStrCfg=" will result
> "SA_AIS_ERR_INVALID_PARAM".
> 
> diff --git a/osaf/tools/saflog/saflogger/saf_logger.c
> b/osaf/tools/saflog/saflogger/saf_logger.c
> --- a/osaf/tools/saflog/saflogger/saf_logger.c
> +++ b/osaf/tools/saflog/saflogger/saf_logger.c
> @@ -101,12 +101,12 @@ static void usage(void)
>   printf("\t-s SEV or --severity=SEV   use severity SEV, default
> INFO\n");
>   printf("\t\tvalid severity names: emerg, alert, crit, error, warn,
> notice, info\n");
>   printf("\nNOTES\n");
> - printf("\t1) -f is only applicable for app stream.\n");
> + printf("\t1) -f is only applicable for runtime app stream.\n");
>   printf("\t1)  length must not be longer than 255
> characters.\n");
> 
>   printf("\nEXAMPLES\n");
> - printf("\tsaflogger -a safLgStrCfg=Test \"Hello world\"\n");
> - printf("\tsaflogger -a safLgStrCfg=Test -f testLogFile \"Hello
> world\"\n");
> + printf("\tsaflogger -a safLgStr=Test \"Hello world\"\n");
> + printf("\tsaflogger -a safLgStr=Test -f testLogFile \"Hello
world\"\n");
>   printf("\tsaflogger -s crit \"I am going down\"\n\n");
>  }
> 


--
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

[devel] Review request CLM Programmers Guide [#2053]

[Hans Nordebäck]

Hi Mathi,

I updated the CLM Programmers Guide with the CLM cluster reboot support.

/Thanks HansN



OpenSAF_CLMSv_PR.odt
Description: application/vnd.oasis.opendocument.text
--
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

Re: [devel] [PATCH 1 of 1] cpnd: use shared memory based on ckpt name length [#2108]

[Vo Minh Hoang]

Dear Mahesh,

Because of keeping the consistent working behavior of existing function,
only 1 shared memory at a time. If shared memory swapping action occurs, a
new shared memory will replace old one.

Here is the detailed answers to your questions:
>> -The  existing  `small format shm`  will continue to be small , is that
right ?
>> -Only newly created longDN checkpoint will be in `big format shm`, is
that right ? 
No, old checkpoint data is converted to `big format`.
So all of them will be stored in `big format`.

>> - what will be the format of newly joined the PL-5 opens  an existing
`small format shm`
PL-5 still use `small format`.
Only when a long DN replica is added in this node, the shared memory is
converted to `big format`.
>>  the what will be the new replica  on new node `small format shm` or `big
format shm` ?
This implementation only affect the `header` shared memory
(opensaf_CPND_CHECKPOINT_INFO_nodeid). It do not change replica shared
memory (opensaf_ckptname_nodeid_n).

About testing, because of above specification, I tested:
- start new node
- restart ckptnd with existing small shm
- restart ckptnd with existing big shm
- create first long dn (check all node)

Thank you and best regards,
Hoang

-Original Message-
From: A V Mahesh [mailto:mahesh.va...@oracle.com] 
Sent: Thursday, October 13, 2016 1:33 PM
To: Hoang Vo ; anders.wid...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1 of 1] cpnd: use shared memory based on ckpt name
length [#2108]

Hi Hoang,

 >> - Run time cpnd keep using small format shm until first longDN
checkpoint is created.
 >> After that cpnd use big format shm.

While reviewing I am assuming following please confirm  :

-The  existing  `small format shm`  will continue to be small , is that
right ?
-Only newly created longDN checkpoint will be in `big format shm`, is that
right ?
- what will be the format of newly joined the PL-5 opens  an existing `small
format shm`
   the what will be the new replica  on new node `small format shm` or `big
format shm` ?


I hope you  tested following :
==
- combination of some `small format shm`  and some  `big format shm`  ckpts
- Joined a New node ( say PL-5)  and then opened the existing `small format
shm` ckpt from the new Node
- Restating controller which has combination of  `small format shm` and `big
format shm` and how the restored non-collocated ckpt`s

-AVM

On 10/11/2016 1:15 PM, Hoang Vo wrote:
>   osaf/libs/common/cpsv/include/cpsv_shm.h |9 +-
>   osaf/services/saf/cpsv/cpnd/cpnd_res.c   |  565
--
>   2 files changed, 536 insertions(+), 38 deletions(-)
>
>
> problem: In the case of CKPT osafckptnd increased 3,5Mb - 240 percent 
> on all nodes CKPT_INFO size inscrease when support longDN lead to total
size increase.
>
> solution:
> - From start, cpnd use small format shm.
> - Run time cpnd keep using small format shm until first longDN checkpoint
is created.
> After that cpnd use big format shm.
>
> diff --git a/osaf/libs/common/cpsv/include/cpsv_shm.h 
> b/osaf/libs/common/cpsv/include/cpsv_shm.h
> --- a/osaf/libs/common/cpsv/include/cpsv_shm.h
> +++ b/osaf/libs/common/cpsv/include/cpsv_shm.h
> @@ -27,7 +27,8 @@
>   #define SHM_NEXT -3
>   #define SHM_INIT -1
>   
> -#define CPSV_CPND_SHM_VERSION1
> +#define CPSV_CPND_SHM_VERSION_SHORT_DN   0
> +#define CPSV_CPND_SHM_VERSION_LONG_DN1
>   
>   typedef struct cpsv_ckpt_hdr {
>   SaCkptCheckpointHandleT ckpt_id;/* Index for identifying the
checkpoint */
> @@ -134,4 +135,10 @@ typedef enum cpnd_type_info {
>   CPND_CKPT_INFO
>   } CPND_TYPE_INFO;
>   
> +#define cpsv_cpnd_shm_size(x) x == CPSV_CPND_SHM_VERSION_LONG_DN ?   \
> + sizeof(CLIENT_HDR) + (MAX_CLIENTS * sizeof(CLIENT_INFO)) +
\
> + sizeof(CKPT_HDR) + (MAX_CKPTS * sizeof(CKPT_INFO)) :
\
> + sizeof(CLIENT_HDR) + (MAX_CLIENTS * sizeof(CLIENT_INFO)) +
\
> + sizeof(CKPT_HDR) + (MAX_CKPTS * sizeof(CKPT_INFO_V0))
\
> +
>   #endif
> diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_res.c 
> b/osaf/services/saf/cpsv/cpnd/cpnd_res.c
> --- a/osaf/services/saf/cpsv/cpnd/cpnd_res.c
> +++ b/osaf/services/saf/cpsv/cpnd/cpnd_res.c
> @@ -44,20 +44,34 @@
>   
>   #define m_CPND_CKPTINFO_UPDATE(addr,ckpt_info,offset) 
> memcpy(addr+offset,_info,sizeof(CKPT_INFO))
>   
> +#define m_CPND_CKPTINFO_V0_UPDATE(addr,ckpt_info,offset) 
> +memcpy(addr+offset,_info,sizeof(CKPT_INFO_V0))
> +
>   #define m_CPND_CKPTHDR_UPDATE(ckpt_hdr,offset)  
> memcpy(offset,_hdr,sizeof(CKPT_HDR))
>   
> +void *cpnd_restart_shm(NCS_OS_POSIX_SHM_REQ_INFO *cpnd_open_req, 
> +CPND_CB *cb, SaClmNodeIdT nodeid); uint32_t 
> +cpnd_update_ckpt_with_clienthdl_v1(CPND_CB *cb, CPND_CKPT_NODE 
> +*cp_node, SaCkptHandleT client_hdl); uint32_t 
> +cpnd_update_ckpt_with_clienthdl_v0(CPND_CB *cb, CPND_CKPT_NODE 
> +*cp_node, SaCkptHandleT client_hdl); uint32_t 
> +cpnd_write_ckpt_info_v1(CPND_CB *cb, 

[devel] [PATCH 0 of 1] Review Request for log: improve test cases for log service [#1913] V2

[Canh Van Truong]

Summary: log: improve test cases for log service [#1913] V2
Review request for Trac Ticket(s): #1913
Peer Reviewer(s): Vu, Lennart, Mahesh
Pull request to: Vu
Affected branch(es): default
Development branch: default


Impacted area   Impact y/n

 Docsn
 Build systemn
 RPM/packaging   n
 Configuration files n
 Startup scripts n
 SAF servicesn
 OpenSAF servicesn
 Core libraries  n
 Samples n
 Tests   y
 Other   n


Comments (indicate scope for each "y" above):
-
 <>

changeset 2fd2c08225292a0afe38a583bf71fbde137909a8
Author: Canh Van Truong 
Date:   Thu, 06 Oct 2016 18:55:51 +0700

log: improve test cases for log service [#1913]

The patch fixes to improve following cases:

1) The APIs may return TRY_AGAIN, this may not be a fault. Writing some
wrapper functions here to handle TRY_AGAIN case for log API.

2) Remove abort (safassert) in test case and handle it as test case 
failed.

3) Some test cases (in testsuite 4) have dependence on other test cases.
Make them independency.

4) Some test cases (in testsuite 4, 5, 6) change setting such as IMM
attributes values and don't restore them back to previous. Get the
attributes values before changing attributes, then restore its to 
previous
values at end of test cases.

5) Some test cases in testsuite 6 change attributes base on the present
value of logMaxLogrecsize without read the present value of its. They 
use
constanst values (=1024). Changing to read logMaxLogrecsize value and 
use
it.


Added Files:

 tests/logsv/logutil.c
 tests/logsv/logutil.h


Complete diffstat:
--
 tests/logsv/Makefile.am   | 1 +
 tests/logsv/logtest.c |12 +-
 tests/logsv/logtest.h | 5 +-
 tests/logsv/logutil.c |   129 
 tests/logsv/logutil.h |51 +
 tests/logsv/tet_LogOiOps.c|  2285 
++-
 tests/logsv/tet_Log_recov.c   | 2 +-
 tests/logsv/tet_saLogDispatch.c   | 9 +-
 tests/logsv/tet_saLogFinalize.c   |11 +-
 tests/logsv/tet_saLogInitialize.c | 8 +-
 tests/logsv/tet_saLogLimitGet.c   |19 +-
 tests/logsv/tet_saLogSelectionObjectGet.c |13 +-
 tests/logsv/tet_saLogStreamClose.c|21 +-
 tests/logsv/tet_saLogStreamOpenAsync_2.c  |14 +-
 tests/logsv/tet_saLogStreamOpen_2.c   |   541 ++--
 tests/logsv/tet_saLogWriteLog.c   |21 +-
 tests/logsv/tet_saLogWriteLogAsync.c  |   478 +-
 tests/logsv/tet_saLogWriteLogCallbackT.c  |   116 ++-
 18 files changed, 2229 insertions(+), 1507 deletions(-)


Testing Commands:
-
Re-run all test cases


Testing, Expected Results:
--
All test cases passed


Conditions of Submission:
-
Ack from reviewers


Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  n  n
powerpc n  n
powerpc64   n  n


Reviewer Checklist:
---
[Submitters: make sure that your review doesn't trigger any checkmarks!]


Your checkin has not passed review because (see checked entries):

___ Your RR template is generally incomplete; it has too many blank entries
that need proper data filled in.

___ You have failed to nominate the proper persons for review and push.

___ Your patches do not have proper short+long header

___ You have grammar/spelling in your header that is unacceptable.

___ You have exceeded a sensible line length in your headers/comments/text.

___ You have failed to put in a proper Trac Ticket # into your commits.

___ You have incorrectly put/left internal data in your comments/files
(i.e. internal bug tracking tool IDs, product names etc)

___ You have not given any evidence of testing beyond basic build tests.
Demonstrate some level of runtime or other sanity testing.

___ You have ^M present in some of your files. These have to be removed.

___ You have needlessly changed whitespace or added whitespace crimes
like trailing spaces, or spaces before tabs.

___ You have mixed real technical changes with whitespace and other
cosmetic code cleanup changes. These have to be separate commits.

___ You need to refactor your submission into logical chunks; there is
too much content into a single commit.

___ You have extraneous garbage in your review (merge commits etc)

___ You have giant 

Re: [devel] [PATCH 1 of 1] cpsv: Update ckpt_reploc_tree when unlinking a checkpoint [#1655]

[A V Mahesh]

Hi Nhat Pham/Hoang,

Can you please clarify following :

>>Solution:
>>-
>> The solution is to remove replica location node of that checkpoint from the 
>> ckpt_reploc_tree
>> when unlinking the checkpoint.

If replica location node of that checkpoint from the ckpt_reploc_tree remove
how will be existing application still accessing will behave ?

say for example :

Cpa-1 on node-A  did unlinking a ckpt-1
Cpa-2 on node-A still accessing reading/writing/creating sections/ect .. for 
the unlinked ckpt-1

Can you please test this case

-AVM

On 1/6/2016 9:42 AM, Nhat Pham wrote:
>   osaf/services/saf/cpsv/cpd/cpd_db.c   |   4 
>   osaf/services/saf/cpsv/cpd/cpd_proc.c |  30 ++
>   2 files changed, 34 insertions(+), 0 deletions(-)
>
>
> Problem:
> 
> The replica IMM objects are not created after opening a checkpoint in 
> following scenario:
>
> 1. Open a checkpoint with flag SA_CKPT_CHECKPOINT_CREATE
> 2. Unlink the checkpoint ( the checkpoint is still being used)
> 3. Open a checkpoint with flag SA_CKPT_CHECKPOINT_CREATE with same name as 
> the one in 1.
>
> After step 3. although the checkpoint is opened successfully, the replica IMM 
> objects
> are not created.
>
> The problem happens because the CPD does not delete relating nodes from 
> ckpt_reploc_tree
> when it unlinks the checkpoint in step 2.
>
> Solution:
> -
> The solution is to remove replica location node of that checkpoint from the 
> ckpt_reploc_tree
> when unlinking the checkpoint.
>
> diff --git a/osaf/services/saf/cpsv/cpd/cpd_db.c 
> b/osaf/services/saf/cpsv/cpd/cpd_db.c
> --- a/osaf/services/saf/cpsv/cpd/cpd_db.c
> +++ b/osaf/services/saf/cpsv/cpd/cpd_db.c
> @@ -420,6 +420,8 @@ uint32_t cpd_ckpt_reploc_node_delete(CPD
>   {
>   uint32_t rc = NCSCC_RC_SUCCESS;
>   
> + TRACE_ENTER();
> +
>   if (cb->ha_state == SA_AMF_HA_ACTIVE) {
>   
>   rc = cpd_ckpt_reploc_imm_object_delete(cb, ckpt_reploc_node, 
> is_unlink_set);
> @@ -441,6 +443,7 @@ uint32_t cpd_ckpt_reploc_node_delete(CPD
>   TRACE_4("cpd db node add failed ");
>   }
>   
> + TRACE_LEAVE();
>   return NCSCC_RC_FAILURE;
>   }
>   
> @@ -448,6 +451,7 @@ uint32_t cpd_ckpt_reploc_node_delete(CPD
>   if (ckpt_reploc_node)
>   m_MMGR_FREE_CPD_CKPT_REPLOC_INFO(ckpt_reploc_node);
>   
> + TRACE_LEAVE();
>   return rc;
>   }
>   
> diff --git a/osaf/services/saf/cpsv/cpd/cpd_proc.c 
> b/osaf/services/saf/cpsv/cpd/cpd_proc.c
> --- a/osaf/services/saf/cpsv/cpd/cpd_proc.c
> +++ b/osaf/services/saf/cpsv/cpd/cpd_proc.c
> @@ -1021,6 +1021,36 @@ uint32_t cpd_proc_unlink_set(CPD_CB *cb,
>   (*ckpt_node)->attributes = map_info->attributes;
>   /* Delete the MAP Info */
>   rc = cpd_ckpt_map_node_delete(cb, map_info);
> +
> + /* Delete replica location of the unlinked checkpoint from the 
> ckpt_reploc_tree */
> + CPD_NODE_REF_INFO *nref_info = (*ckpt_node)->node_list;
> +
> + while (nref_info) {
> + SaClmNodeIdT node_id = 
> m_NCS_NODE_ID_FROM_MDS_DEST(nref_info->dest);
> + SaClmClusterNodeT cluster_node;
> + CPD_REP_KEY_INFO key_info;
> + CPD_CKPT_REPLOC_INFO *reploc_info = NULL;
> + 
> + memset(_node, 0, sizeof(SaClmClusterNodeT));
> + memset(_info, 0, sizeof(CPD_REP_KEY_INFO));
> +
> + if (saClmClusterNodeGet(cb->clm_hdl, node_id, 
> CPD_CLM_API_TIMEOUT, _node) != SA_AIS_OK) {
> + LOG_ER("cpd unlink set - saClmClusterNodeGet failed for 
> node_id %u",node_id);
> + rc = SA_AIS_ERR_LIBRARY;
> + break;
> + }
> +
> + key_info.node_name = cluster_node.nodeName;
> + key_info.ckpt_name = *ckpt_name;
> +
> + cpd_ckpt_reploc_get(>ckpt_reploc_tree, _info, 
> _info);
> + if (reploc_info) {
> + cpd_ckpt_reploc_node_delete(cb, reploc_info, 
> (*ckpt_node)->is_unlink_set);
> + }
> +
> + nref_info = nref_info->next;
> + }
> +
>   TRACE_LEAVE();
>   return rc;
>   }


--
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

Re: [devel] [PATCH 1 of 1] cpnd: use shared memory based on ckpt name length [#2108]

[A V Mahesh]

Hi Hoang,

 >> - Run time cpnd keep using small format shm until first longDN 
checkpoint is created.
 >> After that cpnd use big format shm.

While reviewing I am assuming following please confirm  :

-The  existing  `small format shm`  will continue to be small , is that 
right ?
-Only newly created longDN checkpoint will be in `big format shm`, is 
that right ?
- what will be the format of newly joined the PL-5 opens  an existing  
`small format shm`
   the what will be the new replica  on new node `small format shm` or  
`big format shm` ?


I hope you  tested following :
==
- combination of some `small format shm`  and some  `big format shm`  ckpts
- Joined a New node ( say PL-5)  and then opened the existing `small 
format shm` ckpt from the new Node
- Restating controller which has combination of  `small format shm` and 
`big format shm` and how the restored non-collocated ckpt`s

-AVM

On 10/11/2016 1:15 PM, Hoang Vo wrote:
>   osaf/libs/common/cpsv/include/cpsv_shm.h |9 +-
>   osaf/services/saf/cpsv/cpnd/cpnd_res.c   |  565 
> --
>   2 files changed, 536 insertions(+), 38 deletions(-)
>
>
> problem: In the case of CKPT osafckptnd increased 3,5Mb - 240 percent on all 
> nodes
> CKPT_INFO size inscrease when support longDN lead to total size increase.
>
> solution:
> - From start, cpnd use small format shm.
> - Run time cpnd keep using small format shm until first longDN checkpoint is 
> created.
> After that cpnd use big format shm.
>
> diff --git a/osaf/libs/common/cpsv/include/cpsv_shm.h 
> b/osaf/libs/common/cpsv/include/cpsv_shm.h
> --- a/osaf/libs/common/cpsv/include/cpsv_shm.h
> +++ b/osaf/libs/common/cpsv/include/cpsv_shm.h
> @@ -27,7 +27,8 @@
>   #define SHM_NEXT -3
>   #define SHM_INIT -1
>   
> -#define CPSV_CPND_SHM_VERSION1
> +#define CPSV_CPND_SHM_VERSION_SHORT_DN   0
> +#define CPSV_CPND_SHM_VERSION_LONG_DN1
>   
>   typedef struct cpsv_ckpt_hdr {
>   SaCkptCheckpointHandleT ckpt_id;/* Index for identifying the 
> checkpoint */
> @@ -134,4 +135,10 @@ typedef enum cpnd_type_info {
>   CPND_CKPT_INFO
>   } CPND_TYPE_INFO;
>   
> +#define cpsv_cpnd_shm_size(x) x == CPSV_CPND_SHM_VERSION_LONG_DN ?   \
> + sizeof(CLIENT_HDR) + (MAX_CLIENTS * sizeof(CLIENT_INFO)) +  
> \
> + sizeof(CKPT_HDR) + (MAX_CKPTS * sizeof(CKPT_INFO)) :
> \
> + sizeof(CLIENT_HDR) + (MAX_CLIENTS * sizeof(CLIENT_INFO)) +  
> \
> + sizeof(CKPT_HDR) + (MAX_CKPTS * sizeof(CKPT_INFO_V0))   
> \
> +
>   #endif
> diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_res.c 
> b/osaf/services/saf/cpsv/cpnd/cpnd_res.c
> --- a/osaf/services/saf/cpsv/cpnd/cpnd_res.c
> +++ b/osaf/services/saf/cpsv/cpnd/cpnd_res.c
> @@ -44,20 +44,34 @@
>   
>   #define m_CPND_CKPTINFO_UPDATE(addr,ckpt_info,offset) 
> memcpy(addr+offset,_info,sizeof(CKPT_INFO))
>   
> +#define m_CPND_CKPTINFO_V0_UPDATE(addr,ckpt_info,offset) 
> memcpy(addr+offset,_info,sizeof(CKPT_INFO_V0))
> +
>   #define m_CPND_CKPTHDR_UPDATE(ckpt_hdr,offset)  
> memcpy(offset,_hdr,sizeof(CKPT_HDR))
>   
> +void *cpnd_restart_shm(NCS_OS_POSIX_SHM_REQ_INFO *cpnd_open_req, CPND_CB 
> *cb, SaClmNodeIdT nodeid);
> +uint32_t cpnd_update_ckpt_with_clienthdl_v1(CPND_CB *cb, CPND_CKPT_NODE 
> *cp_node, SaCkptHandleT client_hdl);
> +uint32_t cpnd_update_ckpt_with_clienthdl_v0(CPND_CB *cb, CPND_CKPT_NODE 
> *cp_node, SaCkptHandleT client_hdl);
> +uint32_t cpnd_write_ckpt_info_v1(CPND_CB *cb, CPND_CKPT_NODE *cp_node, 
> int32_t offset, SaCkptHandleT client_hdl);
> +uint32_t cpnd_write_ckpt_info_v0(CPND_CB *cb, CPND_CKPT_NODE *cp_node, 
> int32_t offset, SaCkptHandleT client_hdl);
> +
>   static uint32_t cpnd_res_ckpt_sec_add(CPND_CKPT_SECTION_INFO *pSecPtr, 
> CPND_CKPT_NODE *cp_node);
>   static bool cpnd_find_exact_ckptinfo(CPND_CB *cb, CKPT_INFO *ckpt_info, 
> uint32_t bitmap_offset,
>uint32_t *offset, uint32_t 
> *prev_offset);
> +static bool cpnd_find_exact_ckptinfo_v0(CPND_CB *cb, CKPT_INFO_V0 
> *ckpt_info, uint32_t bitmap_offset,
> +  uint32_t *offset, uint32_t 
> *prev_offset);
>   static void cpnd_clear_ckpt_info(CPND_CB *cb, CPND_CKPT_NODE *cp_node, 
> uint32_t curr_offset, uint32_t prev_offset);
>   static uint32_t cpnd_restore_client_info(CPND_CB *cb, uint8_t *cli_addr);
>   static uint32_t cpnd_restore_ckpt_info_v1(CPND_CB *cb, uint8_t *ckpt_addr, 
> SaClmNodeIdT nodeid);
>   static uint32_t cpnd_restore_ckpt_info_v0(CPND_CB *cb, uint8_t *ckpt_addr, 
> SaClmNodeIdT nodeid);
> +static void cpnd_restart_client_reset_v1(CPND_CB *cb, CPND_CKPT_NODE 
> *cp_node, CPND_CKPT_CLIENT_NODE *cl_node);
> +static void cpnd_restart_client_reset_v0(CPND_CB *cb, CPND_CKPT_NODE 
> *cp_node, CPND_CKPT_CLIENT_NODE *cl_node);
>   static void cpnd_destroy_shm_cpnd_cp_info(NCS_OS_POSIX_SHM_REQ_OPEN_INFO 
> *open_req);
>   static void 

[devel] [PATCH 0 of 1] Review Request for amfnd: fix amfnd exit when a NPI SU comp faults during shutdown phase [#2098].

[praveen . malviya]

Summary: amfnd: fix amfnd exit when a NPI SU comp faults during shutdown phase 
[#2098]. 
Review request for Trac Ticket(s): #2098 
Peer Reviewer(s): AMF devs 
Pull request to: <>
Affected branch(es): ALL 
Development branch: <>


Impacted area   Impact y/n

 Docsn
 Build systemn
 RPM/packaging   n
 Configuration files n
 Startup scripts n
 SAF servicesy
 OpenSAF servicesn
 Core libraries  n
 Samples n
 Tests   n
 Other   n


Comments (indicate scope for each "y" above):
-

changeset 59e1d93081884dd2f5500eb6c1edda72673379ed
Author: Praveen Malviya 
Date:   Thu, 13 Oct 2016 11:40:04 +0530

amfnd: fix amfnd exit when a NPI SU comp faults during shutdown phase
[#2098].

AMFND does not exit at OpenSAF stop when a NPI application comp faults
during removal of assignment.

As a part of OpenSAF shutdwon, AMFND first removes assignment of all
application SIs honoring their ranks. Since application is NPI, AMFND 
tries
to terminate the only application comp hosted on it by launching 
TERMINATE
command. Command fails and AMFND launches clean up of component. Clean 
up
command also fails and both comp and SU are marked TERM_FAILED. Since no
more applicaion SIs remained to perform their removal, AMFND should have
continued next phase of shutdown sequence viz clean up of all the comps 
(PI
apps and MW). This sequence was not triggered because when faulted app 
SU
was moved to TERM_FAILED state, AMFND does not trigger removal done 
logic.
Removal done logic takes care of further removal of remaining assigned 
SIs
or clean up of PI app comps or MW comps.

Patch ensures when SU is marked TERM_FAILED, AMFND should continue 
shutdown
sequence.


Complete diffstat:
--
 osaf/services/saf/amf/amfnd/clc.cc  |  13 +
 osaf/services/saf/amf/amfnd/mds.cc  |   4 +++-
 osaf/services/saf/amf/amfnd/susm.cc |  28 
 3 files changed, 44 insertions(+), 1 deletions(-)


Testing Commands:
-
Tested based on ticket description.

Testing, Expected Results:
--
AMFND exits after cleaning up all the comps.

Conditions of Submission:
-
Ack from one reviewer.

Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  y  y
powerpc n  n
powerpc64   n  n


Reviewer Checklist:
---
[Submitters: make sure that your review doesn't trigger any checkmarks!]


Your checkin has not passed review because (see checked entries):

___ Your RR template is generally incomplete; it has too many blank entries
that need proper data filled in.

___ You have failed to nominate the proper persons for review and push.

___ Your patches do not have proper short+long header

___ You have grammar/spelling in your header that is unacceptable.

___ You have exceeded a sensible line length in your headers/comments/text.

___ You have failed to put in a proper Trac Ticket # into your commits.

___ You have incorrectly put/left internal data in your comments/files
(i.e. internal bug tracking tool IDs, product names etc)

___ You have not given any evidence of testing beyond basic build tests.
Demonstrate some level of runtime or other sanity testing.

___ You have ^M present in some of your files. These have to be removed.

___ You have needlessly changed whitespace or added whitespace crimes
like trailing spaces, or spaces before tabs.

___ You have mixed real technical changes with whitespace and other
cosmetic code cleanup changes. These have to be separate commits.

___ You need to refactor your submission into logical chunks; there is
too much content into a single commit.

___ You have extraneous garbage in your review (merge commits etc)

___ You have giant attachments which should never have been sent;
Instead you should place your content in a public tree to be pulled.

___ You have too many commits attached to an e-mail; resend as threaded
commits, or place in a public tree for a pull.

___ You have resent this content multiple times without a clear indication
of what has changed between each re-send.

___ You have failed to adequately and individually address all of the
comments and change requests that were proposed in the initial review.

___ You have a misconfigured ~/.hgrc file (i.e. username, email etc)

___ Your computer have a badly configured date and time; confusing the
the threaded patch review.

___ Your changes affect IPC mechanism, and you 

[devel] [PATCH 1 of 1] amfnd: fix amfnd exit when a NPI SU comp faults during shutdown phase [#2098]

[praveen . malviya]

 osaf/services/saf/amf/amfnd/clc.cc  |  13 +
 osaf/services/saf/amf/amfnd/mds.cc  |   4 +++-
 osaf/services/saf/amf/amfnd/susm.cc |  28 
 3 files changed, 44 insertions(+), 1 deletions(-)


AMFND does not exit at OpenSAF stop when a NPI application comp faults during 
removal
of assignment.

As a part of OpenSAF shutdwon, AMFND first removes assignment of all application
SIs honoring their ranks. Since application is NPI, AMFND tries to terminate 
the only
application comp hosted on it by launching TERMINATE command. Command fails and 
AMFND
launches clean up of component. Clean up command also fails and both comp and 
SU are
marked TERM_FAILED.  Since no more applicaion SIs remained to perform their 
removal,
AMFND should have continued next phase of shutdown sequence viz clean up of all 
the comps
(PI apps and MW). This sequence was not triggered because when faulted app SU 
was moved to
TERM_FAILED state, AMFND does not trigger removal done logic. Removal done 
logic takes care
of further removal of remaining assigned SIs or clean up of PI app comps or MW 
comps.

Patch ensures when SU is marked TERM_FAILED, AMFND should continue shutdown 
sequence.

diff --git a/osaf/services/saf/amf/amfnd/clc.cc 
b/osaf/services/saf/amf/amfnd/clc.cc
--- a/osaf/services/saf/amf/amfnd/clc.cc
+++ b/osaf/services/saf/amf/amfnd/clc.cc
@@ -1315,6 +1315,19 @@ uint32_t avnd_comp_clc_st_chng_prc(AVND_
}
 
 }
+   //Terminating -> Terminationfailed.
+if ((prv_st == SA_AMF_PRESENCE_TERMINATING) && (final_st == 
SA_AMF_PRESENCE_TERMINATION_FAILED)) {
+   /*
+  Shutdown phase and NPI comp faulted during its 
termination as a part of 
+  of removal of assignment.
+*/
+   csi = 
m_AVND_CSI_REC_FROM_COMP_DLL_NODE_GET(m_NCS_DBLIST_FIND_FIRST(>csi_list));
+   if ((m_AVND_IS_SHUTTING_DOWN(cb)) && 
+   
(m_AVND_COMP_CSI_CURR_ASSIGN_STATE_IS_REMOVING(csi))) {
+   TRACE_1("CSI marked Removed.");
+   m_AVND_COMP_CSI_CURR_ASSIGN_STATE_SET(csi, 
AVND_COMP_CSI_ASSIGN_STATE_REMOVED);
+   }
+   }
 
}
 
diff --git a/osaf/services/saf/amf/amfnd/mds.cc 
b/osaf/services/saf/amf/amfnd/mds.cc
--- a/osaf/services/saf/amf/amfnd/mds.cc
+++ b/osaf/services/saf/amf/amfnd/mds.cc
@@ -1416,8 +1416,10 @@ uint32_t avnd_mds_send(AVND_CB *cb, AVND
case AVND_MSG_AVD:
send_info->i_to_svc = NCSMDS_SVC_ID_AVD;
/* Don't send any messages if we are shutting down */
-   if (m_AVND_IS_SHUTTING_DOWN(cb))
+   if (m_AVND_IS_SHUTTING_DOWN(cb)) {
+   TRACE_1("Shutting down, not sending msg to AMFD.");
goto done;
+   }
break;
 
case AVND_MSG_AVA:
diff --git a/osaf/services/saf/amf/amfnd/susm.cc 
b/osaf/services/saf/amf/amfnd/susm.cc
--- a/osaf/services/saf/amf/amfnd/susm.cc
+++ b/osaf/services/saf/amf/amfnd/susm.cc
@@ -1884,6 +1884,16 @@ uint32_t avnd_su_pres_st_chng_prc(AVND_C
/* Send response to Amfd only when there is a pending 
assignment. */
if (m_AVND_SU_IS_ASSIGN_PEND(su))
rc = avnd_di_susi_resp_send(cb, su, 
m_AVND_SU_IS_ALL_SI(su) ? 0 : si);
+   /*
+  During shutdown phase, all comps of NPI SU are 
terminated as a part of 
+  removal of assignments. If a SU enters in 
TERM_FAILED state then in order
+  to complete shutdown sequence generate a si-oper 
done indication.
+*/
+   if ((si != nullptr) && (m_AVND_IS_SHUTTING_DOWN(cb)) &&
+   
(m_AVND_SU_SI_CURR_ASSIGN_STATE_IS_REMOVING(si)) && 
+   (all_comps_terminated_in_su(su, true) 
== true)) {
+   rc = avnd_su_si_oper_done(cb, su, si);
+   }
}
 
/* instantiating -> term-failed */
@@ -3911,6 +3921,24 @@ static uint32_t avnd_su_pres_termfailed_
   avnd_su_si_del(cb, su->name);
 }
   }
+
+  //NPI SU case. 
+  if (!m_AVND_SU_IS_PREINSTANTIABLE(su)) {
+TRACE_1("NPI SU");
+AVND_SU_SI_REC *si = (AVND_SU_SI_REC 
*)m_NCS_DBLIST_FIND_FIRST(>si_list);
+
+/*
+   During shutdown phase, all comps of NPI SU are terminated as a part of
+   removal of assignments. If a SU enters in TERM_FAILED state then in 
order
+   to complete shutdown sequence generate a si-oper done indication.
+ */
+if ((si != nullptr) && (m_AVND_IS_SHUTTING_DOWN(cb)) &&
+  (m_AVND_SU_SI_CURR_ASSIGN_STATE_IS_REMOVING(si)) && 
+  (all_comps_terminated_in_su(su, true)