date:20170216

Re: [devel] [PATCH 1 of 1] AMFND: Ensure su operational message synchronizes with component failover sequence [#2233]

2017-02-16 Thread Minh Hon CHAU

Hi Praveen,

Yes, you are right, I will update the description.

Thanks, Minh

Quoting praveen malviya :

> Hi Minh,
>
> One quick question:
> Ticket description says:
> "Si deps safSi=AmfDemoTwon2 depends safSi=AmfDemoTwon1 depends  
> safSi=AmfDemoTwon"
> But logs are related to without SIdep. Also in the configuration  
> app3_twon3su3si.xml, SI dep classes are commented.
> I think ticket description needs correction as problem is without SI dep.
> Please confirm.
>
> Thanks,
> Praveen
>
>
> On 17-Feb-17 10:58 AM, praveen malviya wrote:
>> Hi Minh,
>>
>> I have started reviewing this patch.
>>
>> Thanks,
>> Praveen
>>
>> On 15-Feb-17 9:22 AM, minh chau wrote:
>>> Hi all,
>>>
>>> Have you had time to review this patch?
>>> It changes the component failover sequence, so I think we need more time
>>> to look at it.
>>>
>>> Thanks,
>>> Minh
>>>
>>> On 23/01/17 12:28, Minh Hon Chau wrote:
  src/amf/amfnd/avnd_su.h |   1 +
  src/amf/amfnd/clc.cc|   3 ---
  src/amf/amfnd/di.cc |  12 +++-
  src/amf/amfnd/susm.cc   |  32 +---
  4 files changed, 41 insertions(+), 7 deletions(-)

 In case component failover, faulty component will be terminated. When
 the reinstantiation
 is done, amfnd will send su_oper_message (enabled) to amfd which is
 running along with
 component failover. In the reported problem, if su_oper_message
 (enabled) comes to amfd
 before the quiesced assignment response (as part of component failover
 sequence) comes to
 amfd, then this quiesced assignment response is ignored, thus
 component failover will not
 finish.

 The problem is in function susi_success_sg_realign with act=5,
 state=3, amfd always assumes
 su having faulty component is OUT_OF_SERVICE. This assumption is true
 in most of the time
 when su_oper_message (enabled) comes a little later than quiesced
 assignment response. In fact
 the su_oper_message (enabled) is not designed as part of component
 failover sequence, thus it
 can come any time during the failover. If amfd is getting a bit busier
 with RTA update then
 the faulty component has enough to reinstiantiate so that amfnd sends
 su_oper_message (enabled)
 before quiesced assignment response, the reported problem will be seen.

 This patch hardens the component failover sequence by ensuring the
 su_oper_message (enabled) to
 be sent after su completes to remove assignment. This approach comes
 from the similarity in
 su failover, where the su_oper_message (enabled) is sent in repair phase.

 diff --git a/src/amf/amfnd/avnd_su.h b/src/amf/amfnd/avnd_su.h
 --- a/src/amf/amfnd/avnd_su.h
 +++ b/src/amf/amfnd/avnd_su.h
 @@ -393,6 +393,7 @@ extern struct avnd_su_si_rec *avnd_silis
  extern struct avnd_su_si_rec *avnd_silist_getprev(const struct
 avnd_su_si_rec *);
  extern struct avnd_su_si_rec *avnd_silist_getlast(void);
  extern bool sufailover_in_progress(const AVND_SU *su);
 +extern bool componentfailover_in_progress(const AVND_SU *su);
  extern bool sufailover_during_nodeswitchover(const AVND_SU *su);
  extern bool all_csis_in_removed_state(const AVND_SU *su);
  extern void su_reset_restart_count_in_comps(const struct avnd_cb_tag
 *cb, const AVND_SU *su);
 diff --git a/src/amf/amfnd/clc.cc b/src/amf/amfnd/clc.cc
 --- a/src/amf/amfnd/clc.cc
 +++ b/src/amf/amfnd/clc.cc
 @@ -2381,9 +2381,6 @@ uint32_t avnd_comp_clc_terming_cleansucc
  (m_AVND_SU_IS_FAILOVER(su))) {
  /* yes, request director to orchestrate component failover */
  rc = avnd_di_oper_send(cb, su, SA_AMF_COMPONENT_FAILOVER);
 -
 -//Reset component-failover here. SU failover is reset as part
 of REPAIRED admin op.
 -m_AVND_SU_FAILOVER_RESET(su);
  }
/*
 diff --git a/src/amf/amfnd/di.cc b/src/amf/amfnd/di.cc
 --- a/src/amf/amfnd/di.cc
 +++ b/src/amf/amfnd/di.cc
 @@ -894,7 +894,17 @@ uint32_t avnd_di_susi_resp_send(AVND_CB
  }
  m_AVND_SU_ALL_SI_RESET(su);
  }
 -
 +if (componentfailover_in_progress(su)) {
 +if (all_csis_in_removed_state(su) == true) {
 +bool is_en;
 +m_AVND_SU_IS_ENABLED(su, is_en);
 +if (is_en) {
 +if (avnd_di_oper_send(cb, su, 0) ==
 NCSCC_RC_SUCCESS) {
 +m_AVND_SU_FAILOVER_RESET(su);
 +}
 +}
 +}
 +}
  /* free the contents of avnd message */
  avnd_msg_content_free(cb, );
  diff --git a/src/amf/amfnd/susm.cc b/src/amf/amfnd/susm.cc
 --- a/src/amf/amfnd/susm.cc
 +++ b/src/amf/amfnd/susm.cc
 @@ -1633,10 +1633,22

Re: [devel] [PATCH 1 of 1] AMFND: Ensure su operational message synchronizes with component failover sequence [#2233]

2017-02-16 Thread praveen malviya

Hi Minh,

One quick question:
Ticket description says:
"Si deps safSi=AmfDemoTwon2 depends safSi=AmfDemoTwon1 depends 
safSi=AmfDemoTwon"
But logs are related to without SIdep. Also in the configuration 
app3_twon3su3si.xml, SI dep classes are commented.
I think ticket description needs correction as problem is without SI dep.
Please confirm.

Thanks,
Praveen


On 17-Feb-17 10:58 AM, praveen malviya wrote:
> Hi Minh,
>
> I have started reviewing this patch.
>
> Thanks,
> Praveen
>
> On 15-Feb-17 9:22 AM, minh chau wrote:
>> Hi all,
>>
>> Have you had time to review this patch?
>> It changes the component failover sequence, so I think we need more time
>> to look at it.
>>
>> Thanks,
>> Minh
>>
>> On 23/01/17 12:28, Minh Hon Chau wrote:
>>>   src/amf/amfnd/avnd_su.h |   1 +
>>>   src/amf/amfnd/clc.cc|   3 ---
>>>   src/amf/amfnd/di.cc |  12 +++-
>>>   src/amf/amfnd/susm.cc   |  32 +---
>>>   4 files changed, 41 insertions(+), 7 deletions(-)
>>>
>>>
>>> In case component failover, faulty component will be terminated. When
>>> the reinstantiation
>>> is done, amfnd will send su_oper_message (enabled) to amfd which is
>>> running along with
>>> component failover. In the reported problem, if su_oper_message
>>> (enabled) comes to amfd
>>> before the quiesced assignment response (as part of component failover
>>> sequence) comes to
>>> amfd, then this quiesced assignment response is ignored, thus
>>> component failover will not
>>> finish.
>>>
>>> The problem is in function susi_success_sg_realign with act=5,
>>> state=3, amfd always assumes
>>> su having faulty component is OUT_OF_SERVICE. This assumption is true
>>> in most of the time
>>> when su_oper_message (enabled) comes a little later than quiesced
>>> assignment response. In fact
>>> the su_oper_message (enabled) is not designed as part of component
>>> failover sequence, thus it
>>> can come any time during the failover. If amfd is getting a bit busier
>>> with RTA update then
>>> the faulty component has enough to reinstiantiate so that amfnd sends
>>> su_oper_message (enabled)
>>> before quiesced assignment response, the reported problem will be seen.
>>>
>>> This patch hardens the component failover sequence by ensuring the
>>> su_oper_message (enabled) to
>>> be sent after su completes to remove assignment. This approach comes
>>> from the similarity in
>>> su failover, where the su_oper_message (enabled) is sent in repair phase.
>>>
>>> diff --git a/src/amf/amfnd/avnd_su.h b/src/amf/amfnd/avnd_su.h
>>> --- a/src/amf/amfnd/avnd_su.h
>>> +++ b/src/amf/amfnd/avnd_su.h
>>> @@ -393,6 +393,7 @@ extern struct avnd_su_si_rec *avnd_silis
>>>   extern struct avnd_su_si_rec *avnd_silist_getprev(const struct
>>> avnd_su_si_rec *);
>>>   extern struct avnd_su_si_rec *avnd_silist_getlast(void);
>>>   extern bool sufailover_in_progress(const AVND_SU *su);
>>> +extern bool componentfailover_in_progress(const AVND_SU *su);
>>>   extern bool sufailover_during_nodeswitchover(const AVND_SU *su);
>>>   extern bool all_csis_in_removed_state(const AVND_SU *su);
>>>   extern void su_reset_restart_count_in_comps(const struct avnd_cb_tag
>>> *cb, const AVND_SU *su);
>>> diff --git a/src/amf/amfnd/clc.cc b/src/amf/amfnd/clc.cc
>>> --- a/src/amf/amfnd/clc.cc
>>> +++ b/src/amf/amfnd/clc.cc
>>> @@ -2381,9 +2381,6 @@ uint32_t avnd_comp_clc_terming_cleansucc
>>>   (m_AVND_SU_IS_FAILOVER(su))) {
>>>   /* yes, request director to orchestrate component failover */
>>>   rc = avnd_di_oper_send(cb, su, SA_AMF_COMPONENT_FAILOVER);
>>> -
>>> -//Reset component-failover here. SU failover is reset as part
>>> of REPAIRED admin op.
>>> -m_AVND_SU_FAILOVER_RESET(su);
>>>   }
>>> /*
>>> diff --git a/src/amf/amfnd/di.cc b/src/amf/amfnd/di.cc
>>> --- a/src/amf/amfnd/di.cc
>>> +++ b/src/amf/amfnd/di.cc
>>> @@ -894,7 +894,17 @@ uint32_t avnd_di_susi_resp_send(AVND_CB
>>>   }
>>>   m_AVND_SU_ALL_SI_RESET(su);
>>>   }
>>> -
>>> +if (componentfailover_in_progress(su)) {
>>> +if (all_csis_in_removed_state(su) == true) {
>>> +bool is_en;
>>> +m_AVND_SU_IS_ENABLED(su, is_en);
>>> +if (is_en) {
>>> +if (avnd_di_oper_send(cb, su, 0) ==
>>> NCSCC_RC_SUCCESS) {
>>> +m_AVND_SU_FAILOVER_RESET(su);
>>> +}
>>> +}
>>> +}
>>> +}
>>>   /* free the contents of avnd message */
>>>   avnd_msg_content_free(cb, );
>>>   diff --git a/src/amf/amfnd/susm.cc b/src/amf/amfnd/susm.cc
>>> --- a/src/amf/amfnd/susm.cc
>>> +++ b/src/amf/amfnd/susm.cc
>>> @@ -1633,10 +1633,22 @@ uint32_t avnd_su_pres_st_chng_prc(AVND_C
>>>   m_AVND_SU_IS_ENABLED(su, is_en);
>>>   if (true == is_en) {
>>>   TRACE("SU oper state is enabled");
>>> +// do not send su_oper state if

Re: [devel] [PATCH 1 of 1] AMFND: Ensure su operational message synchronizes with component failover sequence [#2233]

2017-02-16 Thread praveen malviya

Hi Minh,

I have started reviewing this patch.

Thanks,
Praveen

On 15-Feb-17 9:22 AM, minh chau wrote:
> Hi all,
>
> Have you had time to review this patch?
> It changes the component failover sequence, so I think we need more time
> to look at it.
>
> Thanks,
> Minh
>
> On 23/01/17 12:28, Minh Hon Chau wrote:
>>   src/amf/amfnd/avnd_su.h |   1 +
>>   src/amf/amfnd/clc.cc|   3 ---
>>   src/amf/amfnd/di.cc |  12 +++-
>>   src/amf/amfnd/susm.cc   |  32 +---
>>   4 files changed, 41 insertions(+), 7 deletions(-)
>>
>>
>> In case component failover, faulty component will be terminated. When
>> the reinstantiation
>> is done, amfnd will send su_oper_message (enabled) to amfd which is
>> running along with
>> component failover. In the reported problem, if su_oper_message
>> (enabled) comes to amfd
>> before the quiesced assignment response (as part of component failover
>> sequence) comes to
>> amfd, then this quiesced assignment response is ignored, thus
>> component failover will not
>> finish.
>>
>> The problem is in function susi_success_sg_realign with act=5,
>> state=3, amfd always assumes
>> su having faulty component is OUT_OF_SERVICE. This assumption is true
>> in most of the time
>> when su_oper_message (enabled) comes a little later than quiesced
>> assignment response. In fact
>> the su_oper_message (enabled) is not designed as part of component
>> failover sequence, thus it
>> can come any time during the failover. If amfd is getting a bit busier
>> with RTA update then
>> the faulty component has enough to reinstiantiate so that amfnd sends
>> su_oper_message (enabled)
>> before quiesced assignment response, the reported problem will be seen.
>>
>> This patch hardens the component failover sequence by ensuring the
>> su_oper_message (enabled) to
>> be sent after su completes to remove assignment. This approach comes
>> from the similarity in
>> su failover, where the su_oper_message (enabled) is sent in repair phase.
>>
>> diff --git a/src/amf/amfnd/avnd_su.h b/src/amf/amfnd/avnd_su.h
>> --- a/src/amf/amfnd/avnd_su.h
>> +++ b/src/amf/amfnd/avnd_su.h
>> @@ -393,6 +393,7 @@ extern struct avnd_su_si_rec *avnd_silis
>>   extern struct avnd_su_si_rec *avnd_silist_getprev(const struct
>> avnd_su_si_rec *);
>>   extern struct avnd_su_si_rec *avnd_silist_getlast(void);
>>   extern bool sufailover_in_progress(const AVND_SU *su);
>> +extern bool componentfailover_in_progress(const AVND_SU *su);
>>   extern bool sufailover_during_nodeswitchover(const AVND_SU *su);
>>   extern bool all_csis_in_removed_state(const AVND_SU *su);
>>   extern void su_reset_restart_count_in_comps(const struct avnd_cb_tag
>> *cb, const AVND_SU *su);
>> diff --git a/src/amf/amfnd/clc.cc b/src/amf/amfnd/clc.cc
>> --- a/src/amf/amfnd/clc.cc
>> +++ b/src/amf/amfnd/clc.cc
>> @@ -2381,9 +2381,6 @@ uint32_t avnd_comp_clc_terming_cleansucc
>>   (m_AVND_SU_IS_FAILOVER(su))) {
>>   /* yes, request director to orchestrate component failover */
>>   rc = avnd_di_oper_send(cb, su, SA_AMF_COMPONENT_FAILOVER);
>> -
>> -//Reset component-failover here. SU failover is reset as part
>> of REPAIRED admin op.
>> -m_AVND_SU_FAILOVER_RESET(su);
>>   }
>> /*
>> diff --git a/src/amf/amfnd/di.cc b/src/amf/amfnd/di.cc
>> --- a/src/amf/amfnd/di.cc
>> +++ b/src/amf/amfnd/di.cc
>> @@ -894,7 +894,17 @@ uint32_t avnd_di_susi_resp_send(AVND_CB
>>   }
>>   m_AVND_SU_ALL_SI_RESET(su);
>>   }
>> -
>> +if (componentfailover_in_progress(su)) {
>> +if (all_csis_in_removed_state(su) == true) {
>> +bool is_en;
>> +m_AVND_SU_IS_ENABLED(su, is_en);
>> +if (is_en) {
>> +if (avnd_di_oper_send(cb, su, 0) ==
>> NCSCC_RC_SUCCESS) {
>> +m_AVND_SU_FAILOVER_RESET(su);
>> +}
>> +}
>> +}
>> +}
>>   /* free the contents of avnd message */
>>   avnd_msg_content_free(cb, );
>>   diff --git a/src/amf/amfnd/susm.cc b/src/amf/amfnd/susm.cc
>> --- a/src/amf/amfnd/susm.cc
>> +++ b/src/amf/amfnd/susm.cc
>> @@ -1633,10 +1633,22 @@ uint32_t avnd_su_pres_st_chng_prc(AVND_C
>>   m_AVND_SU_IS_ENABLED(su, is_en);
>>   if (true == is_en) {
>>   TRACE("SU oper state is enabled");
>> +// do not send su_oper state if component failover is
>> in progress
>>   m_AVND_SU_OPER_STATE_SET(su,
>> SA_AMF_OPERATIONAL_ENABLED);
>> -rc = avnd_di_oper_send(cb, su, 0);
>> -if (NCSCC_RC_SUCCESS != rc)
>> -goto done;
>> +if (componentfailover_in_progress(su) == true) {
>> +si = reinterpret_cast
>> +(m_NCS_DBLIST_FIND_FIRST(>si_list));
>> +if (si == nullptr ||
>>

Re: [devel] [PATCH 1 of 1] mds: Use sequenceId for message sequence numbers [#2292]

2017-02-16 Thread A V Mahesh

Hi Anders Widell,

Can you please clarify below :

1) is this  meta sequenceId is rotates after reaching some value ? (   
Max value )

2) is this  meta sequenceId  persistent even after application respawn  ?

=

<143>1 2017-02-17T10:44:21.59772+05:30 PL-1 osafimmloadd 10183 - [meta 
sequenceId="431572"] MDS_SND_RCV : calling cb ptr enc or enc flatin 
mcm_msg_encode_full_or_flat_and_send
<143>1 2017-02-17T10:44:21.597733+05:30 PL-1 osafimmloadd 10183 - [meta 
sequenceId="431573"] >> mds_get_subtn_res_tbl_by_adest

<143>1 2017-02-17T10:44:21.597771+05:30 PL-1 osafimmloadd 10183 - [meta 
sequenceId="431576"] << mcm_msg_encode_full_or_flat_and_send
<142>1 2017-02-17T10:44:21.597784+05:30 PL-1 osafimmloadd 10183 - [meta 
sequenceId="431577"] MDTM: User Sending Data lenght=384 From svc_id = 
IMMA_OM(26) to svc_id = IMMND(25)
<143>1 2017-02-17T10:44:21.597798+05:30 PL-1 osafimmloadd 10183 - [meta 
sequenceId="431578"] MDTM: Sending message with Service Seqno=7482, TO 
Dest_id=<0x0002020f:9253>

<143>1 2017-02-17T10:44:21.597875+05:30 PL-1 osafimmnd 9253 - [meta 
sequenceId="1532911"] >> mds_vdest_tbl_get_role
<143>1 2017-02-17T10:44:21.597879+05:30 PL-1 osafimmnd 9253 - [meta 
sequenceId="1532912"] << mds_vdest_tbl_get_role

=
 


-AVM


On 2/13/2017 8:11 PM, Anders Widell wrote:
>   src/mds/mds_log.cc |  24 
>   1 files changed, 16 insertions(+), 8 deletions(-)
>
>
> Use the meta sequenceId structured element for message sequence numbers 
> instead
> of the MSGID field, as per RFC-5424.
>
> Also use a mutex to protect the UnixSocket instance, since users of this class
> will be reponsible for thread safety after ticket [#2293].
>
> diff --git a/src/mds/mds_log.cc b/src/mds/mds_log.cc
> --- a/src/mds/mds_log.cc
> +++ b/src/mds/mds_log.cc
> @@ -27,7 +27,6 @@
>   #include 
>   #include 
>   #include 
> -#include 
>   #include 
>   #include 
>   #include 
> @@ -35,6 +34,7 @@
>   #include "base/buffer.h"
>   #include "base/log_message.h"
>   #include "base/macros.h"
> +#include "base/mutex.h"
>   #include "base/ncsgl_defs.h"
>   #include "base/osaf_utility.h"
>   #include "base/time.h"
> @@ -54,12 +54,14 @@ class MdsLog {
>   uint32_t proc_id, const char* socket_name);
> void LogInternal(base::LogMessage::Severity severity, const char *fmt,
>  va_list ap);
> +  static constexpr const uint32_t kMaxSequenceId = uint32_t{0x7fff};
> static MdsLog* instance_;
> const base::LogMessage::HostName host_name_;
> const base::LogMessage::AppName app_name_;
> const base::LogMessage::ProcId proc_id_;
> -  std::atomic msg_id_;
> +  uint32_t sequence_id_;
> base::UnixClientSocket log_socket_;
> +  base::Mutex mutex_;
>   
> DELETE_COPY_AND_MOVE_OPERATORS(MdsLog);
>   };
> @@ -72,8 +74,9 @@ MdsLog::MdsLog(const char* host_name, co
>   host_name_{base::LogMessage::HostName{host_name}},
>   app_name_{base::LogMessage::AppName{app_name}},
>   proc_id_{base::LogMessage::ProcId{std::to_string(proc_id)}},
> -msg_id_{0},
> -log_socket_{socket_name} {
> +sequence_id_{1},
> +log_socket_{socket_name},
> +mutex_{} {
>   }
>   
>   /*
> @@ -130,16 +133,21 @@ void MdsLog::Log(base::LogMessage::Sever
>   
>   void MdsLog::LogInternal(base::LogMessage::Severity severity, const char 
> *fmt,
>va_list ap) {
> -  uint64_t id = msg_id_++;
> +  base::Lock lock(mutex_);
> +  uint32_t id = sequence_id_;
> +  sequence_id_ = id < kMaxSequenceId ? id + 1 : 1;
> base::Buffer<256> buffer;
> -  base::LogMessage::Write(base::LogMessage::Facility::kLocal0,
> +  base::LogMessage::Write(base::LogMessage::Facility::kLocal1,
> severity,
> base::ReadRealtimeClock(),
> host_name_,
> app_name_,
> proc_id_,
> -  base::LogMessage::MsgId{std::to_string(id)},
> -  {},
> +  base::LogMessage::MsgId{""},
> +  {{base::LogMessage::SdName{"meta"},
> +  {base::LogMessage::Parameter{
> +  base::LogMessage::SdName{"sequenceId"},
> +  std::to_string(id),
> fmt,
> ap,
> );


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org!

Re: [devel] [PATCH 1 of 1] amfd: do not assert on saAmfSUMaintenanceCampaign modification [#2305]

2017-02-16 Thread praveen malviya

Ack with one minor comment.

Thanks,
Praveen



On 15-Feb-17 7:50 AM, Gary Lee wrote:
>  src/amf/amfd/su.cc |  12 ++--
>  1 files changed, 10 insertions(+), 2 deletions(-)
>
>
> Someone could change saAmfSUMaintenanceCampaign more than once in a CCB.
> We should not assert / cause a node reboot because of this.
>
> diff --git a/src/amf/amfd/su.cc b/src/amf/amfd/su.cc
> --- a/src/amf/amfd/su.cc
> +++ b/src/amf/amfd/su.cc
> @@ -1869,9 +1869,17 @@ static void su_ccb_apply_modify_hdlr(str
>   su->saAmfSUMaintenanceCampaign = "";
>   TRACE("saAmfSUMaintenanceCampaign cleared for 
> '%s'", su->name.c_str());
>   } else {
> - 
> osafassert(su->saAmfSUMaintenanceCampaign.empty() == true);
> - su->saAmfSUMaintenanceCampaign = Amf::to_string(
> + const std::string saAmfSUMaintenanceCampaign = 
> Amf::to_string(
I think the variable name can be kept tmp_saAmfSUMaintenanceCampaign.
>   
> reinterpret_cast(attr_mod->modAttr.attrValues[0]));
> + // there is a check in completed callback to 
> ensure saAmfSUMaintenanceCampaign is empty
> + // before allowing modification but 
> saAmfSUMaintenanceCampaign could be changed
> + // multiple times in a CCB
> + if (su->saAmfSUMaintenanceCampaign.empty() == 
> false &&
> + 
> su->saAmfSUMaintenanceCampaign.compare(saAmfSUMaintenanceCampaign) != 0) {
> + LOG_WA("saAmfSUMaintenanceCampaign set 
> multiple times in CCB for '%s'",
> + su->name.c_str());
> + }
> + su->saAmfSUMaintenanceCampaign = 
> saAmfSUMaintenanceCampaign;
>   TRACE("saAmfSUMaintenanceCampaign set to '%s' 
> for '%s'",
> 
> su->saAmfSUMaintenanceCampaign.c_str(), su->name.c_str());
>   }
>

--
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: Recover the loss of update Admin State after headless [#2210]

2017-02-16 Thread minh chau

Hi Nagu,

This patch is not a full solution for loss of RTA, it's only for problem 
reported in #2210, so I only tested the scenario described in this ticket.
It's probably hard to see the loss if just following the test scenario. 
I had to change AVD_SI::set_admin_state in AMFD code not to update Admin 
State to simulate the loss.
The fix can be seen something like this in syslog

2017-02-17 09:42:24 SC-1 osafamfd[474]: NO Enter restore headless cached 
RTAs from IMM
2017-02-17 09:42:24 SC-1 osafamfd[474]: WA 
SISU:'safSi=AmfDemoTwon,safApp=AmfDemoTwon,safSu=SU4,safSg=AmfDemoTwon,safApp=AmfDemoTwon',
 
ha:'3', but one of [node/sg/su/si] is not in LOCKED
2017-02-17 09:42:24 SC-1 osafamfd[474]: NO Leave reading headless cached 
RTAs from IMM: SUCCESS
2017-02-17 09:42:24 SC-1 osafamfd[474]: NO Node 'SC-1' joined the cluster

I will update "Testing commands:" for next times

Thanks,
Minh

On 16/02/17 19:28, Nagendra Kumar wrote:
> Ack(Not tested). Please do mention the test cases in " Testing Commands:"
>
> Thanks
> -Nagu
>
>> -Original Message-
>> From: Minh Hon Chau [mailto:minh.c...@dektech.com.au]
>> Sent: 07 February 2017 08:56
>> To: hans.nordeb...@ericsson.com; Nagendra Kumar; Praveen Malviya;
>> gary@dektech.com.au; long.hb.ngu...@dektech.com.au;
>> minh.c...@dektech.com.au
>> Cc: opensaf-devel@lists.sourceforge.net
>> Subject: [PATCH 1 of 1] amfd: Recover the loss of update Admin State after
>> headless [#2210]
>>
>>   src/amf/amfd/siass.cc |  53
>> --
>>   1 files changed, 42 insertions(+), 11 deletions(-)
>>
>>
>> Both controllers go down at the time AMFD is about update Admin State,
>> after headless the admin state read from IMM conflicts with the existing
>> assignment. In ticket #2210, saAmfSIAdminState is UNLOCKED while the
>> SUSI assignment is QUIESCED.
>>
>> The patch helps to recover the admin state based on the HA State of
>> existing assignment. The loss of update RTA is still open issue for
>> many other attributes that are possibly missed to update IMM at the time
>> cluster going headless.
>>
>> diff --git a/src/amf/amfd/siass.cc b/src/amf/amfd/siass.cc
>> --- a/src/amf/amfd/siass.cc
>> +++ b/src/amf/amfd/siass.cc
>> @@ -1,6 +1,7 @@
>>   /*  -*- OpenSAF  -*-
>>*
>>* (C) Copyright 2008 The OpenSAF Foundation
>> + * (C) Copyright 2017 Ericsson AB - All Rights Reserved.
>>*
>>* This program is distributed in the hope that it will be useful, but
>>* WITHOUT ANY WARRANTY; without even the implied warranty of
>> MERCHANTABILITY
>> @@ -346,19 +347,49 @@ bool avd_susi_validate_headless_cached_r
>>  }
>>  }
>>  // rule 2: if ha_fr_imm is QUIESCING, one of relevant entities must
>> -// have adminState is SHUTTINGDOWN
>> +// have adminState is SHUTTINGDOWN, otherwise re-adjust if
>> possible
>>  if (ha_fr_imm == SA_AMF_HA_QUIESCING) {
>> -if (present_susi->su->saAmfSUAdminState ==
>> SA_AMF_ADMIN_SHUTTING_DOWN ||
>> -present_susi->si->saAmfSIAdminState ==
>> SA_AMF_ADMIN_SHUTTING_DOWN ||
>> -present_susi->su->sg_of_su->saAmfSGAdminState ==
>> SA_AMF_ADMIN_SHUTTING_DOWN ||
>> -present_susi->su->su_on_node-
>>> saAmfNodeAdminState == SA_AMF_ADMIN_SHUTTING_DOWN) {
>> -// That's fine
>> -;
>> -} else {
>> -LOG_ER("SISU:'%s', ha:'%u', but one of
>> [node/sg/su/si] is not in SHUTTING_DOWN",
>> +if (present_susi->su->saAmfSUAdminState !=
>> SA_AMF_ADMIN_SHUTTING_DOWN &&
>> +present_susi->si->saAmfSIAdminState !=
>> SA_AMF_ADMIN_SHUTTING_DOWN &&
>> +present_susi->su->sg_of_su->saAmfSGAdminState !=
>> SA_AMF_ADMIN_SHUTTING_DOWN &&
>> +present_susi->su->su_on_node-
>>> saAmfNodeAdminState != SA_AMF_ADMIN_SHUTTING_DOWN) {
>> +LOG_WA("SISU:'%s', ha:'%u', but one of
>> [node/sg/su/si] is not in SHUTTING_DOWN",
>>  dn.c_str(), ha_fr_imm);
>> -valid = false;
>> -goto done;
>> +if (present_susi->su->sg_of_su->sg_fsm_state ==
>> AVD_SG_FSM_SU_OPER)
>> +present_susi->su-
>>> set_admin_state(SA_AMF_ADMIN_SHUTTING_DOWN);
>> +else if (present_susi->su->sg_of_su->sg_fsm_state
>> == AVD_SG_FSM_SI_OPER)
>> +present_susi->si-
>>> set_admin_state(SA_AMF_ADMIN_SHUTTING_DOWN);
>> +else if (present_susi->su->sg_of_su->sg_fsm_state
>> == AVD_SG_FSM_SG_ADMIN)
>> +present_susi->su->sg_of_su-
>>> set_admin_state(SA_AMF_ADMIN_SHUTTING_DOWN);
>> +else {
>> +LOG_ER("Failed to adjust the Admin State of
>> [sg/su/si] with sg fsm state:'%u'",
>> +present_susi->su->sg_of_su-
>>>

Re: [devel] [PATCH 2 of 2] AMFND: Fix SC failover during headless sync before standby AMFD comes up [#2162]

2017-02-16 Thread minh chau

Hi Nagu,

This patch is just for a corner case, where failover happens in between 
AVD_INIT_DONE and AVD_APP_STATE, we still have to reboot the node if out 
of cold sync happens.
So I think we still have to keep that sentence in Compliance Table.

Thanks,
Minh

On 16/02/17 17:24, Nagendra Kumar wrote:
>
> Hi Minh,
>
> Good catch !! Yes, please push, but as such we have documented in 
> Compliance Table that “Before the timer expiry, failover and 
> switchover are not supported.”
>
> Thanks
>
> -Nagu
>
> *From:*minh chau [mailto:minh.c...@dektech.com.au]
> *Sent:* 16 February 2017 07:36
> *To:* Nagendra Kumar; hans.nordeb...@ericsson.com; Praveen Malviya; 
> gary@dektech.com.au
> *Cc:* opensaf-devel@lists.sourceforge.net
> *Subject:* Re: [PATCH 2 of 2] AMFND: Fix SC failover during headless 
> sync before standby AMFD comes up [#2162]
>
> Hi Nagu,
>
> Thanks for reminding, there's one change in the patch that could 
> affect on upgrade too, it is:
>
> +// The cb->init_state must be AVD_INIT_DONE or 
> AVD_APP_STATE
> +// If AVD_INIT_DONE, there was a SC failover during 
> cluster
> +// instantiation phase in cluster (after all NCS SU 
> is assigned)
> +// If AVD_APP_STATE, this should be come from 2N-MW 
> SI swap
> *+if (cb->init_state >= AVD_INIT_DONE) {*
> +if (cluster_su_instantiation_done(cb, nullptr) == 
> true) {
> + cluster_startup_expiry_event_generate(cb);
> +} else {
> +m_AVD_CLINIT_TMR_START(cb);
> +}
> +}
>
> So, I would like to make it for AVD_INIT_DONE only, it looks like
>
> +// The cb->init_state must be AVD_INIT_DONE or 
> AVD_APP_STATE
> +// If AVD_INIT_DONE, there was a SC failover during 
> cluster
> +// instantiation phase in cluster (after all NCS SU 
> is assigned)
> *+if (cb->init_state == AVD_INIT_DONE) {*
> +if (cluster_su_instantiation_done(cb, nullptr) == 
> true) {
> + cluster_startup_expiry_event_generate(cb);
> +} else {
> +m_AVD_CLINIT_TMR_START(cb);
> +}
> +}
>
> If you agree, I can push the patches with new change.
>
> Thanks,
> Minh
>
> On 15/02/17 15:13, Nagendra Kumar wrote:
>
> Yes, ack for both the patches. I assume you would have tested upgrade 
> scenarios.
>
> Thanks
>
> -Nagu
>
> -Original Message-
>
> From: minh chau [mailto:minh.c...@dektech.com.au]
>
> Sent: 15 February 2017 08:52
>
> To: Nagendra Kumar;hans.nordeb...@ericsson.com 
> ; Praveen Malviya;
>
> gary@dektech.com.au 
>
> Cc:opensaf-devel@lists.sourceforge.net
> 
>
> Subject: Re: [PATCH 2 of 2] AMFND: Fix SC failover during headless 
> sync
>
> before standby AMFD comes up [#2162]
>
> Hi Nagu,
>
> The #2162 has two patches. I think your ack is for [PATCH 2 of 2] 
> AMFND:
>
> Fix SC failover during headless sync before standby AMFD comes up 
> [#2162].
>
> Does the other one ([PATCH 1 of 2] AMFD: Fix SC failover during 
> headless
>
> sync at INIT_DONE state [#2162]) look ok?
>
> Thanks,
>
> Minh
>
> On 14/02/17 20:40, Nagendra Kumar wrote:
>
> Ack.
>
> Tested the scenarios.
>
> Thanks
>
> -Nagu
>
> -Original Message-
>
> From: minh chau [mailto:minh.c...@dektech.com.au]
>
> Sent: 23 January 2017 16:24
>
> To: Nagendra Kumar;hans.nordeb...@ericsson.com
> ; Praveen Malviya;
>
> gary@dektech.com.au 
>
> Cc:opensaf-devel@lists.sourceforge.net
> 
>
> Subject: Re: [PATCH 2 of 2] AMFND: Fix SC failover during 
> headless
>
> sync before standby AMFD comes up [#2162]
>
> Hi Nagu,
>
> I am checking the logs now.
>
> Thanks, Minh
>
> On 23/01/17 17:47, Nagendra Kumar wrote:
>
> The logs (Logs-tc.rar) attached in the ticket.
>
> Thanks
>
> -Nagu
>
> -Original Message-
>
> From: minh chau [mailto:minh.c...@dektech.com.au]
>
> Sent: 16 January 2017 05:47
>
> To: Nagendra Kumar;hans.nordeb...@ericsson.com
> ; Praveen Malviya;
>
> gary@dektech.com.au
>

Re: [devel] [PATCH 1 of 3] log: add alternative destinations of log records [#2258]

2017-02-16 Thread Lennart Lund

Hi Vu

See comments inline tagged [Lennart]

General comments:
Update code and comments regarding changed delimiters in configuration string 
from '\n' to ';'

Fix incorrect changes and usage of the configuration handler

Regards
Lennart


> -Original Message-
> From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> Sent: den 16 februari 2017 11:30
> To: Lennart Lund ; mahesh.va...@oracle.com;
> Canh Van Truong 
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: [PATCH 1 of 3] log: add alternative destinations of log records
> [#2258]
> 
>  src/log/Makefile.am  |8 +-
>  src/log/config/logsv_classes.xml |7 +-
>  src/log/logd/lgs_config.cc   |  193 +++--
>  src/log/logd/lgs_config.h|3 +-
>  src/log/logd/lgs_dest.cc |  746
> +++
>  src/log/logd/lgs_dest.h  |  580 ++
>  src/log/logd/lgs_evt.cc  |   33 +
>  src/log/logd/lgs_imm.cc  |  140 ++-
>  src/log/logd/lgs_mbcsv.cc|   91 -
>  src/log/logd/lgs_mbcsv.h |6 +-
>  src/log/logd/lgs_mbcsv_v7.cc |  177 +
>  src/log/logd/lgs_mbcsv_v7.h  |   67 +++
>  src/log/logd/lgs_stream.cc   |   54 ++-
>  src/log/logd/lgs_stream.h|   16 +
>  14 files changed, 2041 insertions(+), 80 deletions(-)
> 
> 
> Here are major info, detailed info will be added to PR doc soon.
> 1) Add attribute "saLogRecordDestination" to log stream.
> 2) Add Local socket destintion handler
> 3) Integrate into first increment made by Lennart
> 
> diff --git a/src/log/Makefile.am b/src/log/Makefile.am
> --- a/src/log/Makefile.am
> +++ b/src/log/Makefile.am
> @@ -71,9 +71,11 @@ noinst_HEADERS += \
>   src/log/logd/lgs_mbcsv_v2.h \
>   src/log/logd/lgs_mbcsv_v3.h \
>   src/log/logd/lgs_mbcsv_v5.h \
> + src/log/logd/lgs_mbcsv_v7.h \
>   src/log/logd/lgs_recov.h \
>   src/log/logd/lgs_stream.h \
> - src/log/logd/lgs_util.h
> + src/log/logd/lgs_util.h \
> + src/log/logd/lgs_dest.h
> 
>  bin_PROGRAMS += bin/saflogger
>  osaf_execbin_PROGRAMS += bin/osaflogd
> @@ -117,10 +119,12 @@ bin_osaflogd_SOURCES = \
>   src/log/logd/lgs_mbcsv_v2.cc \
>   src/log/logd/lgs_mbcsv_v3.cc \
>   src/log/logd/lgs_mbcsv_v5.cc \
> + src/log/logd/lgs_mbcsv_v7.cc \
>   src/log/logd/lgs_mds.cc \
>   src/log/logd/lgs_recov.cc \
>   src/log/logd/lgs_stream.cc \
> - src/log/logd/lgs_util.cc
> + src/log/logd/lgs_util.cc \
> + src/log/logd/lgs_dest.cc
> 
>  bin_osaflogd_LDADD = \
>   lib/libosaf_common.la \
> diff --git a/src/log/config/logsv_classes.xml
> b/src/log/config/logsv_classes.xml
> --- a/src/log/config/logsv_classes.xml
> +++ b/src/log/config/logsv_classes.xml
> @@ -147,12 +147,13 @@
>   SA_CONFIG
>   SA_WRITABLE
>   
> - 
> + 
>   saLogRecordDestination
> - SA_UINT32_T
> + SA_STRING_T
>   SA_CONFIG
>   SA_WRITABLE
> -SA_MULTI_VALUE
> + SA_MULTI_VALUE
> + SA_NO_DUPLICATES
>   
>   
>   saLogStreamCreationTimestamp
> diff --git a/src/log/logd/lgs_config.cc b/src/log/logd/lgs_config.cc
> --- a/src/log/logd/lgs_config.cc
> +++ b/src/log/logd/lgs_config.cc
> @@ -40,6 +40,7 @@
>  #include "osaf/immutil/immutil.h"
>  #include "log/logd/lgs_file.h"
>  #include "log/logd/lgs.h"
> +#include "log/logd/lgs_dest.h"
> 
>  static SaVersionT immVersion = { 'A', 2, 11 };
> 
> @@ -117,10 +118,6 @@ typedef struct _lgs_conf_t {
>std::vector logRecordDestinationConfiguration; // Default
> empty
>/* --- end correspond to IMM Class --- */
> 
> -  /* --- Used with OpenSafLogCurrentConfig runtime object only --- */
> -  /* Note: Has no cnfflag */
> -  std::vector logRecordDestinationStatus; // Default empty
> -
>/* Used for checkpointing time when files are closed */
>time_t chkp_file_close_time;
> 
> @@ -254,6 +251,9 @@ void lgs_cfgupd_multival_add(const std::
>   const std::vector& value_list,
>   lgs_config_chg_t *config_data) {
>TRACE_ENTER();
> +
> +  CfgDestination(value_list, ModifyType::kAdd);
> +
>// Get the existing multi-values and add them to the config data list
>lgs_logconfGet_t param_id = param_name_to_id(attribute_name);
>const std::vector *exist_list =
> @@ -294,6 +294,9 @@ void lgs_cfgupd_multival_delete(std::str
>  std::vector value_list,
>  lgs_config_chg_t *config_data) {
>TRACE_ENTER();
> +
> +  CfgDestination(value_list, ModifyType::kDelete);
> +
>// Get the existing multi-values
>lgs_logconfGet_t param_id = param_name_to_id(attribute_name);
>const

[devel] [PATCH 0 of 3] Review Request for log: add alternative destinations of log records [#2258] V3

2017-02-16 Thread Vu Minh Nguyen

Summary: log: add alternative destinations of log records [#2258]
Review request for Trac Ticket(s): #2258
Peer Reviewer(s): Lennart, Mahesh, Canh
Pull request to: <>
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   y
 Other   n


Comments (indicate scope for each "y" above):
-
 Must be applied on top of Lennart's patch.
 Email title: [PATCH 1 of 1] lgs: Add new multivalue attributes to 
configuration handler [#2258]

changeset 2de480b4565786174667918f3c760d562f4f6ce1
Author: Vu Minh Nguyen 
Date:   Wed, 15 Feb 2017 14:36:00 +0700

log: add alternative destinations of log records [#2258]

Here are major info, detailed info will be added to PR doc soon. 1) Add
attribute "saLogRecordDestination" to log stream. 2) Add Local socket
destintion handler 3) Integrate into first increment made by Lennart

changeset 66141079edb50243ac4c0d122fb3dd7916933cec
Author: Vu Minh Nguyen 
Date:   Wed, 15 Feb 2017 15:07:09 +0700

log: add UML test case to verify alternative destination [#2258]

Major changes: 1) Modify Lennart's test cases because enhancing 
destination
configuration validation rules. 2) Add test suite #17 to verify 
alternative
destination

changeset 325a2327381f360a8bd024babf2968c881e71125
Author: Vu Minh Nguyen 
Date:   Thu, 16 Feb 2017 17:22:13 +0700

log: add unit tests to verify interfaces provided by destination handler
[#2258]

Unit tests to verify major interfaces: 1) CfgDestination() 2)
WriteToDestination()


Added Files:

 src/log/logd/lgs_dest.cc
 src/log/logd/lgs_dest.h
 src/log/logd/lgs_mbcsv_v7.cc
 src/log/logd/lgs_mbcsv_v7.h
 src/log/tests/lgs_dest_test.cc
 src/log/tests/Makefile


Complete diffstat:
--
 src/log/Makefile |4 +
 src/log/Makefile.am  |   31 +-
 src/log/apitest/tet_LogOiOps.c   |8 +-
 src/log/config/logsv_classes.xml |7 +-
 src/log/logd/lgs_config.cc   |  193 --
 src/log/logd/lgs_config.h|3 +-
 src/log/logd/lgs_dest.cc |  746 
+
 src/log/logd/lgs_dest.h  |  580 
+++
 src/log/logd/lgs_evt.cc  |   33 ++
 src/log/logd/lgs_imm.cc  |  140 +--
 src/log/logd/lgs_mbcsv.cc|   91 -
 src/log/logd/lgs_mbcsv.h |6 +-
 src/log/logd/lgs_mbcsv_v7.cc |  177 +++
 src/log/logd/lgs_mbcsv_v7.h  |   67 +
 src/log/logd/lgs_stream.cc   |   54 ++-
 src/log/logd/lgs_stream.h|   16 +++
 src/log/tests/Makefile   |   20 +++
 src/log/tests/lgs_dest_test.cc   |  209 
+
 18 files changed, 2300 insertions(+), 85 deletions(-)


Testing Commands:
-
 Unit test:
 - export enviroment GTEST_DIR, point to googletest
 - run "make check"

 UML test:
  Run test suite #17

Testing, Expected Results:
--
 All tests PASS


Conditions of Submission:
-
 <>


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

[devel] [PATCH 1 of 3] log: add alternative destinations of log records [#2258]

2017-02-16 Thread Vu Minh Nguyen

 src/log/Makefile.am  |8 +-
 src/log/config/logsv_classes.xml |7 +-
 src/log/logd/lgs_config.cc   |  193 +++--
 src/log/logd/lgs_config.h|3 +-
 src/log/logd/lgs_dest.cc |  746 +++
 src/log/logd/lgs_dest.h  |  580 ++
 src/log/logd/lgs_evt.cc  |   33 +
 src/log/logd/lgs_imm.cc  |  140 ++-
 src/log/logd/lgs_mbcsv.cc|   91 -
 src/log/logd/lgs_mbcsv.h |6 +-
 src/log/logd/lgs_mbcsv_v7.cc |  177 +
 src/log/logd/lgs_mbcsv_v7.h  |   67 +++
 src/log/logd/lgs_stream.cc   |   54 ++-
 src/log/logd/lgs_stream.h|   16 +
 14 files changed, 2041 insertions(+), 80 deletions(-)


Here are major info, detailed info will be added to PR doc soon.
1) Add attribute "saLogRecordDestination" to log stream.
2) Add Local socket destintion handler
3) Integrate into first increment made by Lennart

diff --git a/src/log/Makefile.am b/src/log/Makefile.am
--- a/src/log/Makefile.am
+++ b/src/log/Makefile.am
@@ -71,9 +71,11 @@ noinst_HEADERS += \
src/log/logd/lgs_mbcsv_v2.h \
src/log/logd/lgs_mbcsv_v3.h \
src/log/logd/lgs_mbcsv_v5.h \
+   src/log/logd/lgs_mbcsv_v7.h \
src/log/logd/lgs_recov.h \
src/log/logd/lgs_stream.h \
-   src/log/logd/lgs_util.h
+   src/log/logd/lgs_util.h \
+   src/log/logd/lgs_dest.h
 
 bin_PROGRAMS += bin/saflogger
 osaf_execbin_PROGRAMS += bin/osaflogd
@@ -117,10 +119,12 @@ bin_osaflogd_SOURCES = \
src/log/logd/lgs_mbcsv_v2.cc \
src/log/logd/lgs_mbcsv_v3.cc \
src/log/logd/lgs_mbcsv_v5.cc \
+   src/log/logd/lgs_mbcsv_v7.cc \
src/log/logd/lgs_mds.cc \
src/log/logd/lgs_recov.cc \
src/log/logd/lgs_stream.cc \
-   src/log/logd/lgs_util.cc
+   src/log/logd/lgs_util.cc \
+   src/log/logd/lgs_dest.cc
 
 bin_osaflogd_LDADD = \
lib/libosaf_common.la \
diff --git a/src/log/config/logsv_classes.xml b/src/log/config/logsv_classes.xml
--- a/src/log/config/logsv_classes.xml
+++ b/src/log/config/logsv_classes.xml
@@ -147,12 +147,13 @@
SA_CONFIG
SA_WRITABLE

-   
+   
saLogRecordDestination
-   SA_UINT32_T
+   SA_STRING_T
SA_CONFIG
SA_WRITABLE
-SA_MULTI_VALUE
+   SA_MULTI_VALUE
+   SA_NO_DUPLICATES


saLogStreamCreationTimestamp
diff --git a/src/log/logd/lgs_config.cc b/src/log/logd/lgs_config.cc
--- a/src/log/logd/lgs_config.cc
+++ b/src/log/logd/lgs_config.cc
@@ -40,6 +40,7 @@
 #include "osaf/immutil/immutil.h"
 #include "log/logd/lgs_file.h"
 #include "log/logd/lgs.h"
+#include "log/logd/lgs_dest.h"
 
 static SaVersionT immVersion = { 'A', 2, 11 };
 
@@ -117,10 +118,6 @@ typedef struct _lgs_conf_t {
   std::vector logRecordDestinationConfiguration; // Default empty
   /* --- end correspond to IMM Class --- */
 
-  /* --- Used with OpenSafLogCurrentConfig runtime object only --- */
-  /* Note: Has no cnfflag */
-  std::vector logRecordDestinationStatus; // Default empty
-
   /* Used for checkpointing time when files are closed */
   time_t chkp_file_close_time;
 
@@ -254,6 +251,9 @@ void lgs_cfgupd_multival_add(const std::
  const std::vector& value_list,
  lgs_config_chg_t *config_data) {
   TRACE_ENTER();
+
+  CfgDestination(value_list, ModifyType::kAdd);
+
   // Get the existing multi-values and add them to the config data list
   lgs_logconfGet_t param_id = param_name_to_id(attribute_name);
   const std::vector *exist_list =
@@ -294,6 +294,9 @@ void lgs_cfgupd_multival_delete(std::str
 std::vector value_list,
 lgs_config_chg_t *config_data) {
   TRACE_ENTER();
+
+  CfgDestination(value_list, ModifyType::kDelete);
+
   // Get the existing multi-values
   lgs_logconfGet_t param_id = param_name_to_id(attribute_name);
   const std::vector *exist_list =
@@ -325,6 +328,9 @@ void lgs_cfgupd_mutival_replace(std::str
 std::vector value_list,
 lgs_config_chg_t *config_data) {
   TRACE_ENTER();
+
+  CfgDestination(value_list, ModifyType::kReplace);
+
   // Add given value-list to the config data list
   for (auto& value : value_list) {
 lgs_cfgupd_list_create(attribute_name.c_str(),
@@ -373,7 +379,6 @@ int lgs_cfg_update(const lgs_config_chg_
   char *saveptr = NULL;
   int rc = 0;
   bool logRecordDestinationConfiguration_list_clear = true;
-  bool logRecordDestinationStatus_list_clear = true;
 
   TRACE_ENTER();
   /* Validate config_data */
@@ -451,12 +456,6 @@ int lgs_cfg_update(const lgs_config_chg_

[devel] [PATCH 3 of 3] log: add unit tests to verify interfaces provided by destination handler [#2258]

2017-02-16 Thread Vu Minh Nguyen

 src/log/Makefile   |4 +
 src/log/Makefile.am|   19 +++
 src/log/tests/Makefile |   20 +++
 src/log/tests/lgs_dest_test.cc |  209 +
 4 files changed, 252 insertions(+), 0 deletions(-)


Unit tests to verify major interfaces:
1) CfgDestination()
2) WriteToDestination()

diff --git a/src/log/Makefile b/src/log/Makefile
--- a/src/log/Makefile
+++ b/src/log/Makefile
@@ -1,6 +1,7 @@
 #  -*- OpenSAF  -*-
 #
 # (C) Copyright 2016 The OpenSAF Foundation
+# Copyright Ericsson AB 2016, 2017 - All Rights Reserved.
 #
 # This program is distributed in the hope that it will be useful, but
 # WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
@@ -17,3 +18,6 @@
 all:
$(MAKE) -C ../.. lib/libSaLog.la bin/saflogger bin/osaflogd
 
+check:
+   $(MAKE) -C ../.. bin/testlogd
+   ../../bin/testlogd
diff --git a/src/log/Makefile.am b/src/log/Makefile.am
--- a/src/log/Makefile.am
+++ b/src/log/Makefile.am
@@ -80,6 +80,7 @@ noinst_HEADERS += \
 bin_PROGRAMS += bin/saflogger
 osaf_execbin_PROGRAMS += bin/osaflogd
 CORE_INCLUDES += -I$(top_srcdir)/src/log/saf
+TESTS += bin/testlogd
 pkgconfig_DATA += src/log/saf/opensaf-log.pc
 
 nodist_pkgclccli_SCRIPTS += \
@@ -217,4 +218,22 @@ bin_logtestfr_LDADD = \
lib/libSaImmOm.la \
lib/libopensaf_core.la
 
+bin_testlogd_CXXFLAGS =$(AM_CXXFLAGS)
+
+bin_testlogd_CPPFLAGS = \
+   $(AM_CPPFLAGS) \
+   -I$(GTEST_DIR)/include
+
+bin_testlogd_LDFLAGS = \
+   $(AM_LDFLAGS)
+
+bin_testlogd_SOURCES = \
+   src/log/tests/lgs_dest_test.cc \
+   src/log/logd/lgs_dest.cc
+
+bin_testlogd_LDADD = \
+   lib/libopensaf_core.la \
+   $(GTEST_DIR)/lib/libgtest.la \
+   $(GTEST_DIR)/lib/libgtest_main.la
+
 endif
diff --git a/src/log/tests/Makefile b/src/log/tests/Makefile
new file mode 100644
--- /dev/null
+++ b/src/log/tests/Makefile
@@ -0,0 +1,20 @@
+#  -*- OpenSAF  -*-
+#
+# (C) Copyright 2017 The OpenSAF Foundation
+# Copyright Ericsson AB 2017 - All Rights Reserved.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+# or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed
+# under the GNU Lesser General Public License Version 2.1, February 1999.
+# The complete license can be accessed from the following location:
+# http://opensource.org/licenses/lgpl-license.php
+# See the Copying file included with the OpenSAF distribution for full
+# licensing terms.
+#
+# Author(s): Ericsson AB
+#
+
+check:
+   $(MAKE) -C ../../.. bin/testlogd
+   ../../../bin/testlogd
diff --git a/src/log/tests/lgs_dest_test.cc b/src/log/tests/lgs_dest_test.cc
new file mode 100644
--- /dev/null
+++ b/src/log/tests/lgs_dest_test.cc
@@ -0,0 +1,209 @@
+/*  -*- OpenSAF  -*-
+ *
+ * (C) Copyright 2017 The OpenSAF Foundation
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed
+ * under the GNU Lesser General Public License Version 2.1, February 1999.
+ * The complete license can be accessed from the following location:
+ * http://opensource.org/licenses/lgpl-license.php
+ * See the Copying file included with the OpenSAF distribution for full
+ * licensing terms.
+ *
+ * Author(s): Ericsson AB
+ *
+ */
+
+#define protected public
+#define private public
+
+#include "log/logd/lgs_dest.h"
+#include 
+#include 
+#include "base/unix_server_socket.h"
+#include "gtest/gtest.h"
+
+// Verify it is OK to add one valid destination configuration
+TEST(CfgDestination, AddOneDestination) {
+  const std::vector vdest {"test;localsocket;/tmp/sock.sock"};
+  ASSERT_EQ(CfgDestination(vdest, ModifyType::kAdd), true);
+}
+
+// Verify it is Ok to add multiple destination configurations
+TEST(CfgDestination, AddMultipleDestination) {
+  const std::vector vdest {"test;localsocket;/tmp/sock.sock",
+"test1;localsocket;/tmp/sock1.sock", 
"test2;localsocket;/tmp/sock2.sock"};
+  ASSERT_EQ(CfgDestination(vdest, ModifyType::kAdd), true);
+}
+
+// Verify it is Ok to add NIL destination
+TEST(CfgDestination, AddNilDestination) {
+  const std::vector vdest {"test;localsocket;/tmp/sock.sock",
+"test1;localsocket;/tmp/sock1.sock", "test2;localsocket;"};
+  ASSERT_EQ(CfgDestination(vdest, ModifyType::kAdd), true);
+}
+
+// Verify it is OK to delete one destination configuration
+TEST(CfgDestination, DelOneDestination) {
+  const std::vector vdest {"test;localsocket;/tmp/sock.sock",
+"test1;localsocket;/tmp/sock1.sock", "test2;localsocket;"};
+  CfgDestination(vdest, ModifyType::kAdd);
+
+  // Delete destination
+  const std::vector vdeldest 
{"test1;localsocket;/tmp/sock1.sock"};
+  ASSERT_EQ(CfgDestination(vdeldest,

[devel] [PATCH 2 of 3] log: add UML test case to verify alternative destination [#2258]

2017-02-16 Thread Vu Minh Nguyen

 src/log/Makefile.am|  4 +++-
 src/log/apitest/tet_LogOiOps.c |  8 
 2 files changed, 7 insertions(+), 5 deletions(-)


Major changes:
1) Modify Lennart's test cases because enhancing destination
   configuration validation rules.
2) Add test suite #17 to verify alternative destination

diff --git a/src/log/Makefile.am b/src/log/Makefile.am
--- a/src/log/Makefile.am
+++ b/src/log/Makefile.am
@@ -182,7 +182,9 @@ bin_logtest_SOURCES = \
src/log/apitest/tet_Log_recov.c \
src/log/apitest/tet_log_runtime_cfgobj.c \
src/log/apitest/tet_log_longDN.c \
-   src/log/apitest/tet_Log_clm.c
+   src/log/apitest/tet_Log_clm.c \
+   src/log/apitest/tet_cfg_destination.c
+
 
 bin_logtest_LDADD = \
lib/libapitest.la \
diff --git a/src/log/apitest/tet_LogOiOps.c b/src/log/apitest/tet_LogOiOps.c
--- a/src/log/apitest/tet_LogOiOps.c
+++ b/src/log/apitest/tet_LogOiOps.c
@@ -1913,7 +1913,7 @@ void check_logRecordDestinationConfigura
do {
// Add values
for (int i = 0; i < num_values; i++) {
-   sprintf(set_values[i], "Name%d;Type%d;Setting%d", i, i, 
i);
+   sprintf(set_values[i], "Name%d;localsocket;Setting%d", 
i, i);
sprintf(command, "immcfg "
"-a logRecordDestinationConfiguration+="
"'%s' "
@@ -1969,7 +1969,7 @@ void check_logRecordDestinationConfigura
do {
// Add values
for (int i = 0; i < num_values; i++) {
-   sprintf(set_values[i], "Name%d;Type%d;Setting%d", i, i, 
i);
+   sprintf(set_values[i], "Name%d;localsocket;Setting%d", 
i, i);
sprintf(command, "immcfg "
"-a logRecordDestinationConfiguration+="
"'%s' "
@@ -2057,7 +2057,7 @@ void check_logRecordDestinationConfigura
do {
// Add values that will be replaced
for (int i = 0; i < num_values; i++) {
-   sprintf(set_values[i], "Name%d;Type%d;Setting%d", i, i, 
i);
+   sprintf(set_values[i], "Name%d;localsocket;Setting%d", 
i, i);
sprintf(command, "immcfg "
"-a logRecordDestinationConfiguration+="
"'%s' "
@@ -2084,7 +2084,7 @@ void check_logRecordDestinationConfigura
int num_new_values = 3;
for (int i = 0; i < num_new_values; i++) {
sprintf(set_values[i],
-   "NewName%d;NewType%d;NewSetting%d", i, i, i);
+   "NewName%d;localsocket;NewSetting%d", i, i);
}
sprintf(command, "immcfg "
"-a logRecordDestinationConfiguration='%s' "

--
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: Recover the loss of update Admin State after headless [#2210]

2017-02-16 Thread Nagendra Kumar

Ack(Not tested). Please do mention the test cases in " Testing Commands:"

Thanks
-Nagu

> -Original Message-
> From: Minh Hon Chau [mailto:minh.c...@dektech.com.au]
> Sent: 07 February 2017 08:56
> To: hans.nordeb...@ericsson.com; Nagendra Kumar; Praveen Malviya;
> gary@dektech.com.au; long.hb.ngu...@dektech.com.au;
> minh.c...@dektech.com.au
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: [PATCH 1 of 1] amfd: Recover the loss of update Admin State after
> headless [#2210]
> 
>  src/amf/amfd/siass.cc |  53
> --
>  1 files changed, 42 insertions(+), 11 deletions(-)
> 
> 
> Both controllers go down at the time AMFD is about update Admin State,
> after headless the admin state read from IMM conflicts with the existing
> assignment. In ticket #2210, saAmfSIAdminState is UNLOCKED while the
> SUSI assignment is QUIESCED.
> 
> The patch helps to recover the admin state based on the HA State of
> existing assignment. The loss of update RTA is still open issue for
> many other attributes that are possibly missed to update IMM at the time
> cluster going headless.
> 
> diff --git a/src/amf/amfd/siass.cc b/src/amf/amfd/siass.cc
> --- a/src/amf/amfd/siass.cc
> +++ b/src/amf/amfd/siass.cc
> @@ -1,6 +1,7 @@
>  /*  -*- OpenSAF  -*-
>   *
>   * (C) Copyright 2008 The OpenSAF Foundation
> + * (C) Copyright 2017 Ericsson AB - All Rights Reserved.
>   *
>   * This program is distributed in the hope that it will be useful, but
>   * WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY
> @@ -346,19 +347,49 @@ bool avd_susi_validate_headless_cached_r
>   }
>   }
>   // rule 2: if ha_fr_imm is QUIESCING, one of relevant entities must
> - // have adminState is SHUTTINGDOWN
> + // have adminState is SHUTTINGDOWN, otherwise re-adjust if
> possible
>   if (ha_fr_imm == SA_AMF_HA_QUIESCING) {
> - if (present_susi->su->saAmfSUAdminState ==
> SA_AMF_ADMIN_SHUTTING_DOWN ||
> - present_susi->si->saAmfSIAdminState ==
> SA_AMF_ADMIN_SHUTTING_DOWN ||
> - present_susi->su->sg_of_su->saAmfSGAdminState ==
> SA_AMF_ADMIN_SHUTTING_DOWN ||
> - present_susi->su->su_on_node-
> >saAmfNodeAdminState == SA_AMF_ADMIN_SHUTTING_DOWN) {
> - // That's fine
> - ;
> - } else {
> - LOG_ER("SISU:'%s', ha:'%u', but one of
> [node/sg/su/si] is not in SHUTTING_DOWN",
> + if (present_susi->su->saAmfSUAdminState !=
> SA_AMF_ADMIN_SHUTTING_DOWN &&
> + present_susi->si->saAmfSIAdminState !=
> SA_AMF_ADMIN_SHUTTING_DOWN &&
> + present_susi->su->sg_of_su->saAmfSGAdminState !=
> SA_AMF_ADMIN_SHUTTING_DOWN &&
> + present_susi->su->su_on_node-
> >saAmfNodeAdminState != SA_AMF_ADMIN_SHUTTING_DOWN) {
> + LOG_WA("SISU:'%s', ha:'%u', but one of
> [node/sg/su/si] is not in SHUTTING_DOWN",
>   dn.c_str(), ha_fr_imm);
> - valid = false;
> - goto done;
> + if (present_susi->su->sg_of_su->sg_fsm_state ==
> AVD_SG_FSM_SU_OPER)
> + present_susi->su-
> >set_admin_state(SA_AMF_ADMIN_SHUTTING_DOWN);
> + else if (present_susi->su->sg_of_su->sg_fsm_state
> == AVD_SG_FSM_SI_OPER)
> + present_susi->si-
> >set_admin_state(SA_AMF_ADMIN_SHUTTING_DOWN);
> + else if (present_susi->su->sg_of_su->sg_fsm_state
> == AVD_SG_FSM_SG_ADMIN)
> + present_susi->su->sg_of_su-
> >set_admin_state(SA_AMF_ADMIN_SHUTTING_DOWN);
> + else {
> + LOG_ER("Failed to adjust the Admin State of
> [sg/su/si] with sg fsm state:'%u'",
> + present_susi->su->sg_of_su-
> >sg_fsm_state);
> + valid = false;
> + goto done;
> + }
> + }
> + }
> + // rule 3: if ha_fr_imm is QUIESCED, one of relevant entities must
> + // have adminState is LOCKED, otherwise re-adjust if possible
> + if (ha_fr_imm == SA_AMF_HA_QUIESCED) {
> + if (present_susi->su->saAmfSUAdminState !=
> SA_AMF_ADMIN_LOCKED &&
> + present_susi->si->saAmfSIAdminState !=
> SA_AMF_ADMIN_LOCKED &&
> + present_susi->su->sg_of_su->saAmfSGAdminState !=
> SA_AMF_ADMIN_LOCKED &&
> + present_susi->su->su_on_node-
> >saAmfNodeAdminState != SA_AMF_ADMIN_LOCKED) {
> + LOG_WA("SISU:'%s', ha:'%u', but one of
> [node/sg/su/si] is not in LOCKED",
> + dn.c_str(), ha_fr_imm);
> + if (present_susi->su->sg_of_su->sg_fsm_state ==
> AVD_SG_FSM_SU_OPER)
> +

Re: [devel] [PATCH 1 of 1] AMFND: Ensure su operational message synchronizes with component failover sequence [#2233]

Re: [devel] [PATCH 1 of 1] AMFND: Ensure su operational message synchronizes with component failover sequence [#2233]

Re: [devel] [PATCH 1 of 1] AMFND: Ensure su operational message synchronizes with component failover sequence [#2233]

Re: [devel] [PATCH 1 of 1] mds: Use sequenceId for message sequence numbers [#2292]

Re: [devel] [PATCH 1 of 1] amfd: do not assert on saAmfSUMaintenanceCampaign modification [#2305]

Re: [devel] [PATCH 1 of 1] amfd: Recover the loss of update Admin State after headless [#2210]

Re: [devel] [PATCH 2 of 2] AMFND: Fix SC failover during headless sync before standby AMFD comes up [#2162]

Re: [devel] [PATCH 1 of 3] log: add alternative destinations of log records [#2258]

[devel] [PATCH 0 of 3] Review Request for log: add alternative destinations of log records [#2258] V3

[devel] [PATCH 1 of 3] log: add alternative destinations of log records [#2258]

[devel] [PATCH 3 of 3] log: add unit tests to verify interfaces provided by destination handler [#2258]

[devel] [PATCH 2 of 3] log: add UML test case to verify alternative destination [#2258]

Re: [devel] [PATCH 1 of 1] amfd: Recover the loss of update Admin State after headless [#2210]

13 matches

Site Navigation

Mail list logo

Footer information